win: restructure window states

Remove transitional window states, such as UNMAPPING, MAPPING, etc. We
are trying to generialize animation, keeping all those states for
animation/fading will only make things more complicated. Instead the
window state now exactly reflect how the window is to the X server, and
we check the animatable states to determine the animation progress.

Signed-off-by: Yuxuan Shui <yshuiv7@gmail.com>

src/dbus.c

@@ -29,6 +29,7 @@
 #include "uthash_extra.h"
 #include "utils.h"
 #include "win.h"
+#include "win_defs.h"
 
 #include "dbus.h"
 
@@ -877,7 +878,7 @@ cdbus_process_window_property_get(session_t *ps, DBusMessage *msg, cdbus_window_
 
 	if (!strcmp("Mapped", target)) {
 		cdbus_reply(ps, msg, cdbus_append_bool_variant,
-		            (bool[]){win_is_mapped_in_x(w)});
+		            (bool[]){w->state == WSTATE_MAPPED});
 		return true;
 	}
 

src/event.c

@@ -21,6 +21,7 @@
 #include "region.h"
 #include "utils.h"
 #include "win.h"
+#include "win_defs.h"
 #include "x.h"
 
 /// Event handling with X is complicated. Handling events with other events possibly
@@ -273,18 +274,27 @@ static inline void ev_destroy_notify(session_t *ps, xcb_destroy_notify_event_t *
 		assert(&mw->base == w);
 		mw = NULL;
 	}
+
+	// A window can't be a client window and a top-level window at the same time,
+	// so only one of `w` and `mw` can be non-NULL
 	assert(w == NULL || mw == NULL);
 
 	if (w != NULL) {
-		auto _ attr_unused = destroy_win_start(ps, w);
-	} else if (mw != NULL) {
+		destroy_win_start(ps, w);
+		if (!w->managed || !((struct managed_win *)w)->to_paint) {
+			// If the window wasn't managed, or was already not rendered,
+			// we don't need to fade it out.
+			destroy_win_finish(ps, w);
+		}
+		return;
+	}
+	if (mw != NULL) {
 		win_unmark_client(ps, mw);
 		win_set_flags(mw, WIN_FLAGS_CLIENT_STALE);
 		ps->pending_updates = true;
-	} else {
-		log_debug("Received a destroy notify from an unknown window, %#010x",
-		          ev->window);
+		return;
 	}
+	log_debug("Received a destroy notify from an unknown window, %#010x", ev->window);
 }
 
 static inline void ev_map_notify(session_t *ps, xcb_map_notify_event_t *ev) {
@@ -308,6 +318,13 @@ static inline void ev_map_notify(session_t *ps, xcb_map_notify_event_t *ev) {
 	}
 
 	win_set_flags(w, WIN_FLAGS_MAPPED);
+	// We set `ever_damaged` to false here, instead of in `map_win_start`,
+	// because we might receive damage events before that function is called
+	// (which is called when we handle the `WIN_FLAGS_MAPPED` flag), in
+	// which case `repair_win` will be called, which uses `ever_damaged` so
+	// it needs to be correct. This also covers the case where the window is
+	// unmapped before `map_win_start` is called.
+	w->ever_damaged = false;
 
 	// FocusIn/Out may be ignored when the window is unmapped, so we must
 	// recheck focus here
@@ -349,10 +366,8 @@ static inline void ev_reparent_notify(session_t *ps, xcb_reparent_notify_event_t
 		{
 			auto w = find_win(ps, ev->window);
 			if (w) {
-				auto ret = destroy_win_start(ps, w);
-				if (!ret && w->managed) {
+				if (w->managed) {
 					auto mw = (struct managed_win *)w;
-					CHECK(win_skip_fading(ps, mw));
 					// Usually, damage for unmapped windows
 					// are added in `paint_preprocess`, when
 					// a window was painted before and isn't
@@ -363,7 +378,17 @@ static inline void ev_reparent_notify(session_t *ps, xcb_reparent_notify_event_t
 					if (mw->to_paint) {
 						add_damage_from_win(ps, mw);
 					}
+					// Emulating what X server does: a destroyed
+					// window is always unmapped first.
+					if (mw->state == WSTATE_MAPPED) {
+						unmap_win_start(ps, mw);
+					}
 				}
+				// Window reparenting is unlike normal window destruction,
+				// This window is going to be rendered under another
+				// parent, so we don't fade here.
+				destroy_win_start(ps, w);
+				destroy_win_finish(ps, w);
 			}
 		}
 
@@ -378,25 +403,14 @@ static inline void ev_reparent_notify(session_t *ps, xcb_reparent_notify_event_t
 			evmask |= XCB_EVENT_MASK_PROPERTY_CHANGE;
 		} else {
 			auto w_real_top = find_managed_window_or_parent(ps, ev->parent);
-			if (w_real_top && w_real_top->state != WSTATE_UNMAPPED &&
-			    w_real_top->state != WSTATE_UNMAPPING) {
+			if (w_real_top) {
 				log_debug("Mark window %#010x (%s) as having a stale "
 				          "client",
 				          w_real_top->base.id, w_real_top->name);
 				win_set_flags(w_real_top, WIN_FLAGS_CLIENT_STALE);
 				ps->pending_updates = true;
 			} else {
-				if (!w_real_top) {
-					log_debug("parent %#010x not found", ev->parent);
-				} else {
-					// Window is not currently mapped, unmark its
-					// client to trigger a client recheck when it is
-					// mapped later.
-					win_unmark_client(ps, w_real_top);
-					log_debug("parent %#010x (%s) is in state %d",
-					          w_real_top->base.id, w_real_top->name,
-					          w_real_top->state);
-				}
+				log_debug("parent %#010x not found", ev->parent);
 			}
 		}
 		XCB_AWAIT_VOID(xcb_change_window_attributes, ps->c.c, ev->window,
@@ -605,7 +619,7 @@ static inline void ev_property_notify(session_t *ps, xcb_property_notify_event_t
 
 static inline void repair_win(session_t *ps, struct managed_win *w) {
 	// Only mapped window can receive damages
-	assert(win_is_mapped_in_x(w));
+	assert(w->state == WSTATE_MAPPED || win_check_flags_all(w, WIN_FLAGS_MAPPED));
 
 	region_t parts;
 	pixman_region32_init(&parts);
@@ -628,6 +642,10 @@ static inline void repair_win(session_t *ps, struct managed_win *w) {
 			free(e);
 		}
 		win_extents(w, &parts);
+
+		// We only binds the window pixmap once the window is damaged.
+		win_set_flags(w, WIN_FLAGS_PIXMAP_STALE);
+		ps->pending_updates = true;
 	} else {
 		auto cookie = xcb_damage_subtract(ps->c.c, w->damage, XCB_NONE,
 		                                  ps->damage_ring.x_region);

src/picom.c

@@ -496,22 +496,10 @@ static double fade_timeout(session_t *ps) {
  * @param steps steps of fading
  * @return whether we are still in fading mode
  */
-static bool run_fade(session_t *ps, struct managed_win **_w, unsigned int steps) {
+static bool run_fade(struct managed_win **_w, unsigned int steps) {
 	auto w = *_w;
 	log_trace("Process fading for window %s (%#010x), steps: %u", w->name, w->base.id,
 	          steps);
-	if (w->state == WSTATE_MAPPED || w->state == WSTATE_UNMAPPED) {
-		// We are not fading
-		assert(w->number_of_animations == 0);
-		log_trace("|- not animated");
-		return false;
-	}
-
-	if (!win_should_fade(ps, w)) {
-		log_trace("|- in transition but doesn't need fading");
-		animatable_early_stop(&w->opacity);
-		animatable_early_stop(&w->blur_opacity);
-	}
 	if (w->number_of_animations == 0) {
 		// We have reached target opacity.
 		// We don't call win_check_fade_finished here because that could destroy
@@ -634,10 +622,7 @@ static void rebuild_shadow_exclude_reg(session_t *ps) {
 static void destroy_backend(session_t *ps) {
 	win_stack_foreach_managed_safe(w, &ps->window_stack) {
 		// Wrapping up fading in progress
-		if (win_skip_fading(ps, w)) {
-			// `w` is freed by win_skip_fading
-			continue;
-		}
+		win_skip_fading(w);
 
 		if (ps->backend_data) {
 			// Unmapped windows could still have shadow images, but not pixmap
@@ -653,6 +638,10 @@ static void destroy_backend(session_t *ps) {
 			win_release_images(ps->backend_data, w);
 		}
 		free_paint(ps, &w->paint);
+
+		if (w->state == WSTATE_DESTROYED) {
+			destroy_win_finish(ps, &w->base);
+		}
 	}
 
 	HASH_ITER2(ps->shaders, shader) {
@@ -954,12 +943,14 @@ static bool paint_preprocess(session_t *ps, bool *fade_running, bool *animation,
 		}
 
 		// Run fading
-		if (run_fade(ps, &w, steps)) {
+		if (run_fade(&w, steps)) {
 			*fade_running = true;
 		}
 
-		if (win_maybe_finalize_fading(ps, w)) {
-			// the window has been destroyed because fading finished
+		if (w->state == WSTATE_DESTROYED && w->number_of_animations == 0) {
+			// the window should be destroyed because it was destroyed
+			// by X server and now its animations are finished
+			destroy_win_finish(ps, &w->base);
 			continue;
 		}
 
@@ -992,6 +983,7 @@ static bool paint_preprocess(session_t *ps, bool *fade_running, bool *animation,
 		// w->to_paint remembers whether this window is painted last time
 		const bool was_painted = w->to_paint;
 		const double window_opacity = animatable_get(&w->opacity);
+		const double blur_opacity = animatable_get(&w->blur_opacity);
 
 		// Destroy reg_ignore if some window above us invalidated it
 		if (!reg_ignore_valid) {
@@ -1005,30 +997,31 @@ static bool paint_preprocess(session_t *ps, bool *fade_running, bool *animation,
 		// Give up if it's not damaged or invisible, or it's unmapped and its
 		// pixmap is gone (for example due to a ConfigureNotify), or when it's
 		// excluded
-		if (w->state == WSTATE_UNMAPPED) {
+		if (w->state == WSTATE_UNMAPPED && w->number_of_animations == 0) {
+			if (window_opacity != 0 || blur_opacity != 0) {
+				log_warn("Window %#010x (%s) is unmapped but still has "
+				         "opacity",
+				         w->base.id, w->name);
+			}
 			log_trace("|- is unmapped");
 			to_paint = false;
 		} else if (unlikely(ps->debug_window != XCB_NONE) &&
 		           (w->base.id == ps->debug_window ||
 		            w->client_win == ps->debug_window)) {
 			log_trace("|- is the debug window");
 			to_paint = false;
-		} else if (!w->ever_damaged && w->state != WSTATE_UNMAPPING &&
-		           w->state != WSTATE_DESTROYING) {
-			// Unmapping clears w->ever_damaged, but the fact that the window
-			// is fading out means it must have been damaged when it was still
-			// mapped (because unmap_win_start will skip fading if it wasn't),
-			// so we still need to paint it.
+		} else if (!w->ever_damaged) {
 			log_trace("|- has not received any damages");
 			to_paint = false;
 		} else if (unlikely(w->g.x + w->g.width < 1 || w->g.y + w->g.height < 1 ||
 		                    w->g.x >= ps->root_width || w->g.y >= ps->root_height)) {
 			log_trace("|- is positioned outside of the screen");
 			to_paint = false;
-		} else if (unlikely(window_opacity * MAX_ALPHA < 1 && !w->blur_background)) {
-			/* TODO(yshui) for consistency, even a window has 0 opacity, we
-			 * still probably need to blur its background, so to_paint
-			 * shouldn't be false for them. */
+		} else if (unlikely(window_opacity * MAX_ALPHA < 1 &&
+		                    (!w->blur_background || blur_opacity * MAX_ALPHA < 1))) {
+			// For consistency, even a window has 0 opacity, we would still
+			// blur its background. (unless it's background is not blurred, or
+			// the blur opacity is 0)
 			log_trace("|- has 0 opacity");
 			to_paint = false;
 		} else if (w->paint_excluded) {
@@ -1821,7 +1814,10 @@ static void draw_callback_impl(EV_P_ session_t *ps, int revents attr_unused) {
 		// Using foreach_safe here since skipping fading can cause window to be
 		// freed if it's destroyed.
 		win_stack_foreach_managed_safe(w, &ps->window_stack) {
-			auto _ attr_unused = win_skip_fading(ps, w);
+			win_skip_fading(w);
+			if (w->state == WSTATE_DESTROYED) {
+				destroy_win_finish(ps, &w->base);
+			}
 		}
 	}