Merge pull request #546 from yshui/window-state-update

win: rework how events for unmapped windows are handled

src/event.c

@@ -201,58 +201,55 @@ static void configure_win(session_t *ps, xcb_configure_notify_event_t *ce) {
 
 	auto mw = (struct managed_win *)w;
 
-	if (mw->state == WSTATE_UNMAPPED || mw->state == WSTATE_UNMAPPING ||
-	    mw->state == WSTATE_DESTROYING) {
-		// Only restack the window to make sure we can handle future restack
-		// notification correctly
-		restack_above(ps, w, ce->above_sibling);
-	} else {
-		restack_above(ps, w, ce->above_sibling);
-
-		// If window geometry change, free old extents
-		if (mw->g.x != ce->x || mw->g.y != ce->y || mw->g.width != ce->width ||
-		    mw->g.height != ce->height || mw->g.border_width != ce->border_width) {
-			// We don't mark the old region as damaged if we have stale
-			// shape/size/position information. The old region should have
-			// already been add to damage when the information became stale.
-			if (!win_check_flags_any(
-			        mw, WIN_FLAGS_SIZE_STALE | WIN_FLAGS_POSITION_STALE)) {
-				// Mark the old extents as damaged.
-				// The new extents will be marked damaged when processing
-				// window flags.
+	restack_above(ps, w, ce->above_sibling);
+
+	// If window geometry change, free old extents
+	if (mw->g.x != ce->x || mw->g.y != ce->y || mw->g.width != ce->width ||
+	    mw->g.height != ce->height || mw->g.border_width != ce->border_width) {
+		// We don't mark the old region as damaged if we have stale
+		// shape/size/position information. The old region should have
+		// already been add to damage when the information became stale.
+		if (!win_check_flags_any(mw, WIN_FLAGS_SIZE_STALE | WIN_FLAGS_POSITION_STALE)) {
+			if (mw->state != WSTATE_UNMAPPED && mw->state != WSTATE_UNMAPPING &&
+			    mw->state != WSTATE_DESTROYING) {
+				// Mark the old extents as damaged. The new extents will
+				// be marked damaged when processing window flags.
+				// If the window is not mapped, we don't care
 				region_t damage;
 				pixman_region32_init(&damage);
 				win_extents(mw, &damage);
 				add_damage(ps, &damage);
 				pixman_region32_fini(&damage);
 			}
+		}
 
-			// Queue pending updates
-			win_set_flags(mw, WIN_FLAGS_FACTOR_CHANGED);
-			ps->pending_updates = true;
-
-			// At least one of the following if's is true
-			if (mw->g.x != ce->x || mw->g.y != ce->y) {
-				log_trace("Window position changed, %dx%d -> %dx%d",
-				          mw->g.x, mw->g.y, ce->x, ce->y);
-				mw->g.x = ce->x;
-				mw->g.y = ce->y;
-				win_set_flags(mw, WIN_FLAGS_POSITION_STALE);
-			}
+		// Queue pending updates
+		win_set_flags(mw, WIN_FLAGS_FACTOR_CHANGED);
+		// TODO(yshui) don't set pending_updates if the window is not
+		// visible/mapped
+		ps->pending_updates = true;
 
-			if (mw->g.width != ce->width || mw->g.height != ce->height ||
-			    mw->g.border_width != ce->border_width) {
-				log_trace("Window size changed, %dx%d -> %dx%d",
-				          mw->g.width, mw->g.height, ce->width, ce->height);
-				mw->g.width = ce->width;
-				mw->g.height = ce->height;
-				mw->g.border_width = ce->border_width;
-				win_set_flags(mw, WIN_FLAGS_SIZE_STALE);
-			}
+		// At least one of the following if's is true
+		if (mw->g.x != ce->x || mw->g.y != ce->y) {
+			log_trace("Window position changed, %dx%d -> %dx%d", mw->g.x,
+			          mw->g.y, ce->x, ce->y);
+			mw->g.x = ce->x;
+			mw->g.y = ce->y;
+			win_set_flags(mw, WIN_FLAGS_POSITION_STALE);
+		}
 
-			// Recalculate which screen this window is on
-			win_update_screen(ps->xinerama_nscrs, ps->xinerama_scr_regs, mw);
+		if (mw->g.width != ce->width || mw->g.height != ce->height ||
+		    mw->g.border_width != ce->border_width) {
+			log_trace("Window size changed, %dx%d -> %dx%d", mw->g.width,
+			          mw->g.height, ce->width, ce->height);
+			mw->g.width = ce->width;
+			mw->g.height = ce->height;
+			mw->g.border_width = ce->border_width;
+			win_set_flags(mw, WIN_FLAGS_SIZE_STALE);
 		}
+
+		// Recalculate which screen this window is on
+		win_update_screen(ps->xinerama_nscrs, ps->xinerama_scr_regs, mw);
 	}
 
 	// override_redirect flag cannot be changed after window creation, as far

src/picom.c

@@ -402,12 +402,20 @@ static void destroy_backend(session_t *ps) {
 		}
 
 		if (ps->backend_data) {
-			if (w->state == WSTATE_MAPPED) {
-				win_release_images(ps->backend_data, w);
-			} else {
-				assert(!w->win_image);
-				assert(!w->shadow_image);
+			// Unmapped windows could still have shadow images, but not pixmap
+			// images
+			assert(!w->win_image || w->state != WSTATE_UNMAPPED);
+			if (win_check_flags_any(w, WIN_FLAGS_IMAGES_STALE) &&
+			    w->state == WSTATE_MAPPED) {
+				log_warn("Stale flags set for mapped window %#010x "
+				         "during backend destruction",
+				         w->base.id);
+				assert(false);
 			}
+			// Unmapped windows can still have stale flags set, because their
+			// stale flags aren't handled until they are mapped.
+			win_clear_flags(w, WIN_FLAGS_IMAGES_STALE);
+			win_release_images(ps->backend_data, w);
 		}
 		free_paint(ps, &w->paint);
 	}
@@ -495,25 +503,12 @@ static bool initialize_backend(session_t *ps) {
 			}
 			auto w = (struct managed_win *)_w;
 			assert(w->state == WSTATE_MAPPED || w->state == WSTATE_UNMAPPED);
-			if (w->state == WSTATE_MAPPED) {
-				// We need to reacquire image
-				log_debug("Marking window %#010x (%s) for update after "
-				          "redirection",
-				          w->base.id, w->name);
-				if (w->shadow) {
-					struct color c = {
-					    .red = ps->o.shadow_red,
-					    .green = ps->o.shadow_green,
-					    .blue = ps->o.shadow_blue,
-					    .alpha = ps->o.shadow_opacity,
-					};
-					win_bind_shadow(ps->backend_data, w, c,
-					                ps->gaussian_map);
-				}
-
-				w->flags |= WIN_FLAGS_PIXMAP_STALE;
-				ps->pending_updates = true;
-			}
+			// We need to reacquire image
+			log_debug("Marking window %#010x (%s) for update after "
+			          "redirection",
+			          w->base.id, w->name);
+			win_set_flags(w, WIN_FLAGS_IMAGES_STALE);
+			ps->pending_updates = true;
 		}
 	}
 
@@ -1354,10 +1349,7 @@ static void handle_new_windows(session_t *ps) {
 			}
 			auto mw = (struct managed_win *)new_w;
 			if (mw->a.map_state == XCB_MAP_STATE_VIEWABLE) {
-				// Have to map immediately instead of queue window update
-				// because we need the window's extent right now.
-				// We can do this because we are in the critical section.
-				map_win_start(ps, mw);
+				win_set_flags(mw, WIN_FLAGS_MAPPED);
 
 				// This window might be damaged before we called fill_win
 				// and created the damage handle. And there is no way for