From 7df9245c819087b5fcbbb4ae626aec66e05a2119 Mon Sep 17 00:00:00 2001
From: Yuxuan Shui <yshuiv7@gmail.com>
Date: Thu, 19 Sep 2019 16:59:36 +0100
Subject: [PATCH 01/11] win: move image update code into a function

Signed-off-by: Yuxuan Shui <yshuiv7@gmail.com>
---
 src/compton.c | 14 +-------------
 src/win.c     | 15 +++++++++++++++
 src/win.h     |  3 +++
 3 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/src/compton.c b/src/compton.c
index 8a544bc..7f8d514 100644
--- a/src/compton.c
+++ b/src/compton.c
@@ -1283,19 +1283,7 @@ static void handle_new_windows(session_t *ps) {
 
 static void refresh_stale_images(session_t *ps) {
 	win_stack_foreach_managed(w, &ps->window_stack) {
-		if ((w->flags & WIN_FLAGS_IMAGE_STALE) != 0 &&
-		    (w->flags & WIN_FLAGS_IMAGE_ERROR) == 0) {
-			// Image needs to be updated, update it.
-			w->flags &= ~WIN_FLAGS_IMAGE_STALE;
-			if (w->state != WSTATE_UNMAPPING &&
-			    w->state != WSTATE_DESTROYING && ps->backend_data) {
-				// Rebind image only when the window does have an image
-				// available
-				if (!win_try_rebind_image(ps, w)) {
-					w->flags |= WIN_FLAGS_IMAGE_ERROR;
-				}
-			}
-		}
+		win_process_flags(ps, w);
 	}
 }
 
diff --git a/src/win.c b/src/win.c
index 1c41214..697405b 100644
--- a/src/win.c
+++ b/src/win.c
@@ -301,6 +301,21 @@ bool win_try_rebind_image(session_t *ps, struct managed_win *w) {
 	return win_bind_image(ps, w);
 }
 
+void win_process_flags(struct session *ps, struct managed_win *w) {
+	if ((w->flags & WIN_FLAGS_IMAGE_STALE) != 0 && (w->flags & WIN_FLAGS_IMAGE_ERROR) == 0) {
+		// Image needs to be updated, update it.
+		w->flags &= ~WIN_FLAGS_IMAGE_STALE;
+		if (w->state != WSTATE_UNMAPPING && w->state != WSTATE_DESTROYING &&
+		    ps->backend_data) {
+			// Rebind image only when the window does have an image
+			// available
+			if (!win_try_rebind_image(ps, w)) {
+				w->flags |= WIN_FLAGS_IMAGE_ERROR;
+			}
+		}
+	}
+}
+
 /**
  * Check if a window has rounded corners.
  * XXX This is really dumb
diff --git a/src/win.h b/src/win.h
index 6171a8d..fb5086c 100644
--- a/src/win.h
+++ b/src/win.h
@@ -253,6 +253,9 @@ struct managed_win {
 void win_release_image(struct backend_base *base, struct managed_win *w);
 bool must_use win_bind_image(session_t *ps, struct managed_win *w);
 
+/// Process pending images flags on a window. Has to be called in X critical section
+void win_process_flags(session_t *ps, struct managed_win *w);
+
 /// Attempt a rebind of window's images. If that failed, the original images are kept.
 bool must_use win_try_rebind_image(session_t *ps, struct managed_win *w);
 int win_get_name(session_t *ps, struct managed_win *w);

From ebb52567c019fdfc7b0e494b368d430b08dfbd48 Mon Sep 17 00:00:00 2001
From: Yuxuan Shui <yshuiv7@gmail.com>
Date: Thu, 19 Sep 2019 17:20:18 +0100
Subject: [PATCH 02/11] core: move the X critical section into a function

Also move handle_root_flags into the critical section.

Signed-off-by: Yuxuan Shui <yshuiv7@gmail.com>
---
 src/compton.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/src/compton.c b/src/compton.c
index 7f8d514..98af27b 100644
--- a/src/compton.c
+++ b/src/compton.c
@@ -1301,7 +1301,7 @@ static void fade_timer_callback(EV_P attr_unused, ev_timer *w, int revents attr_
 	queue_redraw(ps);
 }
 
-static void _draw_callback(EV_P_ session_t *ps, int revents attr_unused) {
+static void handle_pending_updates(EV_P_ struct session *ps) {
 	if (ps->pending_updates) {
 		log_debug("Delayed handling of events, entering critical section");
 		auto e = xcb_request_check(ps->c, xcb_grab_server_checked(ps->c));
@@ -1325,10 +1325,11 @@ static void _draw_callback(EV_P_ session_t *ps, int revents attr_unused) {
 			recheck_focus(ps);
 		}
 
-		// Refresh pixmaps
+		// Refresh pixmaps and shadows
 		refresh_stale_images(ps);
 
-		ps->server_grabbed = false;
+		// Handle screen changes
+		handle_root_flags(ps);
 
 		e = xcb_request_check(ps->c, xcb_ungrab_server_checked(ps->c));
 		if (e) {
@@ -1338,9 +1339,14 @@ static void _draw_callback(EV_P_ session_t *ps, int revents attr_unused) {
 			return quit_compton(ps);
 		}
 
+		ps->server_grabbed = false;
 		ps->pending_updates = false;
-		log_debug("Exiting critical section");
+		log_debug("Exited critical section");
 	}
+}
+
+static void _draw_callback(EV_P_ session_t *ps, int revents attr_unused) {
+	handle_pending_updates(EV_A_ ps);
 
 	if (ps->o.benchmark) {
 		if (ps->o.benchmark_wid) {
@@ -1355,11 +1361,6 @@ static void _draw_callback(EV_P_ session_t *ps, int revents attr_unused) {
 		}
 	}
 
-	// TODO xcb_grab_server
-	// TODO clean up event queue
-
-	handle_root_flags(ps);
-
 	// TODO have a stripped down version of paint_preprocess that is used when screen
 	// is not redirected. its sole purpose should be to decide whether the screen
 	// should be redirected.

From 5787c17ebec29213efc73c2ce3d64d61b4beb3c8 Mon Sep 17 00:00:00 2001
From: Yuxuan Shui <yshuiv7@gmail.com>
Date: Thu, 19 Sep 2019 17:23:48 +0100
Subject: [PATCH 03/11] Promote some log_trace to log_debug

Signed-off-by: Yuxuan Shui <yshuiv7@gmail.com>
---
 src/compton.c | 4 ++--
 src/win.c     | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/compton.c b/src/compton.c
index 98af27b..42f0d4a 100644
--- a/src/compton.c
+++ b/src/compton.c
@@ -2011,9 +2011,9 @@ static session_t *session_init(int argc, char **argv, Display *dpy,
 		free(query_tree_reply);
 	}
 
-	log_trace("Initial stack:");
+	log_debug("Initial stack:");
 	list_foreach(struct win, w, &ps->window_stack, stack_neighbour) {
-		log_trace("%#010x", w->id);
+		log_debug("%#010x", w->id);
 	}
 
 	ps->pending_updates = true;
diff --git a/src/win.c b/src/win.c
index 697405b..814b57c 100644
--- a/src/win.c
+++ b/src/win.c
@@ -1877,7 +1877,7 @@ bool win_skip_fading(session_t *ps, struct managed_win *w) {
 		assert(w->opacity_target == w->opacity);
 		return false;
 	}
-	log_trace("Skipping fading process of window %#010x (%s)", w->base.id, w->name);
+	log_debug("Skipping fading process of window %#010x (%s)", w->base.id, w->name);
 	w->opacity = w->opacity_target;
 	return win_check_fade_finished(ps, w);
 }

From 1549997811fb12bf66c9f0f9104add06bbabc608 Mon Sep 17 00:00:00 2001
From: Yuxuan Shui <yshuiv7@gmail.com>
Date: Thu, 19 Sep 2019 17:26:09 +0100
Subject: [PATCH 04/11] win: remove map_win_by_id

Since it has only one user.

Signed-off-by: Yuxuan Shui <yshuiv7@gmail.com>
---
 src/event.c | 21 ++++++++++++++++++++-
 src/win.c   | 22 ----------------------
 src/win.h   |  1 -
 3 files changed, 20 insertions(+), 24 deletions(-)

diff --git a/src/event.c b/src/event.c
index de5b298..de98f88 100644
--- a/src/event.c
+++ b/src/event.c
@@ -271,7 +271,26 @@ static inline void ev_destroy_notify(session_t *ps, xcb_destroy_notify_event_t *
 }
 
 static inline void ev_map_notify(session_t *ps, xcb_map_notify_event_t *ev) {
-	map_win_by_id(ps, ev->window);
+	// Unmap overlay window if it got mapped but we are currently not
+	// in redirected state.
+	if (ps->overlay && ev->window == ps->overlay && !ps->redirected) {
+		log_debug("Overlay is mapped while we are not redirected");
+		auto e = xcb_request_check(ps->c, xcb_unmap_window(ps->c, ps->overlay));
+		if (e) {
+			log_error("Failed to unmap the overlay window");
+			free(e);
+		}
+		// We don't track the overlay window, so we can return
+		return;
+	}
+
+	auto w = find_managed_win(ps, ev->window);
+	if (!w) {
+		return;
+	}
+
+	map_win(ps, w);
+
 	// FocusIn/Out may be ignored when the window is unmapped, so we must
 	// recheck focus here
 	ps->pending_updates = true;        // to update focus
diff --git a/src/win.c b/src/win.c
index 814b57c..c82eb1a 100644
--- a/src/win.c
+++ b/src/win.c
@@ -2042,28 +2042,6 @@ void map_win(session_t *ps, struct managed_win *w) {
 	}
 }
 
-void map_win_by_id(session_t *ps, xcb_window_t id) {
-	// Unmap overlay window if it got mapped but we are currently not
-	// in redirected state.
-	if (ps->overlay && id == ps->overlay && !ps->redirected) {
-		log_debug("Overlay is mapped while we are not redirected");
-		auto e = xcb_request_check(ps->c, xcb_unmap_window(ps->c, ps->overlay));
-		if (e) {
-			log_error("Failed to unmap the overlay window");
-			free(e);
-		}
-		// We don't track the overlay window, so we can return
-		return;
-	}
-
-	auto w = find_managed_win(ps, id);
-	if (!w) {
-		return;
-	}
-
-	map_win(ps, w);
-}
-
 /**
  * Find a managed window from window id in window linked list of the session.
  */
diff --git a/src/win.h b/src/win.h
index fb5086c..6e3163c 100644
--- a/src/win.h
+++ b/src/win.h
@@ -365,7 +365,6 @@ bool must_use unmap_win(session_t *ps, struct managed_win *, bool destroy);
 void destroy_unmanaged_win(session_t *ps, struct win *w);
 
 void map_win(session_t *ps, struct managed_win *w);
-void map_win_by_id(session_t *ps, xcb_window_t id);
 
 /**
  * Execute fade callback of a window if fading finished.

From eecee130b8d7f5b5e9bb735a1e7979c35177729f Mon Sep 17 00:00:00 2001
From: Yuxuan Shui <yshuiv7@gmail.com>
Date: Thu, 19 Sep 2019 18:48:12 +0100
Subject: [PATCH 05/11] win: split WIN_FLAGS_IMAGE_STALE

Split it into PIXMAP_STALE and SHADOW_STALE, this allows us to update
pixmaps and shadow images separately.

Also added PIXMAP_NONE and SHADOW_NONE, as redundancy to detect logic
errors.

Convenient constants and functions are provided for updating pixmap and
shadow together.

Signed-off-by: Yuxuan Shui <yshuiv7@gmail.com>
---
 src/backend/backend.c |   3 +
 src/compton.c         |  11 +--
 src/win.c             | 164 ++++++++++++++++++++++++++++--------------
 src/win.h             |  10 +--
 src/win_defs.h        |  21 +++++-
 5 files changed, 143 insertions(+), 66 deletions(-)

diff --git a/src/backend/backend.c b/src/backend/backend.c
index 46bfd1c..224c369 100644
--- a/src/backend/backend.c
+++ b/src/backend/backend.c
@@ -137,6 +137,8 @@ void paint_all_new(session_t *ps, struct managed_win *t, bool ignore_damage) {
 	for (auto w = t; w; w = w->prev_trans) {
 		pixman_region32_subtract(&reg_visible, &ps->screen_reg, w->reg_ignore);
 		assert(!(w->flags & WIN_FLAGS_IMAGE_ERROR));
+		assert(!(w->flags & WIN_FLAGS_PIXMAP_STALE));
+		assert(!(w->flags & WIN_FLAGS_PIXMAP_NONE));
 
 		// The bounding shape of the window, in global/target coordinates
 		// reminder: bounding shape contains the WM frame
@@ -201,6 +203,7 @@ void paint_all_new(session_t *ps, struct managed_win *t, bool ignore_damage) {
 
 		// Draw shadow on target
 		if (w->shadow) {
+			assert(!(w->flags & WIN_FLAGS_SHADOW_NONE));
 			// Clip region for the shadow
 			// reg_shadow \in reg_paint
 			auto reg_shadow = win_extents_by_val(w);
diff --git a/src/compton.c b/src/compton.c
index 42f0d4a..1b2410d 100644
--- a/src/compton.c
+++ b/src/compton.c
@@ -662,7 +662,7 @@ static void destroy_backend(session_t *ps) {
 
 		if (ps->backend_data) {
 			if (w->state == WSTATE_MAPPED) {
-				win_release_image(ps->backend_data, w);
+				win_release_images(ps->backend_data, w);
 			} else {
 				assert(!w->win_image);
 				assert(!w->shadow_image);
@@ -753,9 +753,12 @@ static bool initialize_backend(session_t *ps) {
 			}
 			auto w = (struct managed_win *)_w;
 			if (w->a.map_state == XCB_MAP_STATE_VIEWABLE) {
-				if (!win_bind_image(ps, w)) {
-					w->flags |= WIN_FLAGS_IMAGE_ERROR;
-				}
+				win_bind_image(ps->backend_data, w,
+				               (struct color){.red = ps->o.shadow_red,
+				                              .green = ps->o.shadow_green,
+				                              .blue = ps->o.shadow_blue,
+				                              .alpha = ps->o.shadow_opacity},
+				               ps->gaussian_map);
 			}
 		}
 	}
diff --git a/src/win.c b/src/win.c
index c82eb1a..a502b2a 100644
--- a/src/win.c
+++ b/src/win.c
@@ -241,78 +241,127 @@ void add_damage_from_win(session_t *ps, const struct managed_win *w) {
 }
 
 /// Release the images attached to this window
-void win_release_image(backend_t *base, struct managed_win *w) {
-	log_debug("Releasing image of window %#010x (%s)", w->base.id, w->name);
-	assert(w->win_image || (w->flags & WIN_FLAGS_IMAGE_ERROR));
+static inline void win_release_pixmap(backend_t *base, struct managed_win *w) {
+	log_debug("Releasing pixmap of window %#010x (%s)", w->base.id, w->name);
+	assert(w->win_image);
 	if (w->win_image) {
 		base->ops->release_image(base, w->win_image);
 		w->win_image = NULL;
+		w->flags |= WIN_FLAGS_PIXMAP_NONE;
 	}
-	if (w->shadow) {
-		assert(w->shadow_image || (w->flags & WIN_FLAGS_IMAGE_ERROR));
-		if (w->shadow_image) {
-			base->ops->release_image(base, w->shadow_image);
-			w->shadow_image = NULL;
-		}
+}
+static inline void win_release_shadow(backend_t *base, struct managed_win *w) {
+	log_debug("Releasing shadow of window %#010x (%s)", w->base.id, w->name);
+	assert(w->shadow_image);
+	if (w->shadow_image) {
+		base->ops->release_image(base, w->shadow_image);
+		w->shadow_image = NULL;
+		w->flags |= WIN_FLAGS_SHADOW_NONE;
 	}
 }
 
-static inline bool
-_win_bind_image(session_t *ps, struct managed_win *w, void **win_image, void **shadow_image) {
-	auto pixmap = x_new_id(ps->c);
+static inline bool win_bind_pixmap(struct backend_base *b, struct managed_win *w) {
+	assert(!w->win_image);
+	auto pixmap = x_new_id(b->c);
 	auto e = xcb_request_check(
-	    ps->c, xcb_composite_name_window_pixmap_checked(ps->c, w->base.id, pixmap));
+	    b->c, xcb_composite_name_window_pixmap_checked(b->c, w->base.id, pixmap));
 	if (e) {
 		log_error("Failed to get named pixmap for window %#010x(%s)", w->base.id,
 		          w->name);
 		free(e);
 		return false;
 	}
-	log_trace("New named pixmap for %#010x (%s) : %#010x", w->base.id, w->name, pixmap);
-	*win_image = ps->backend_data->ops->bind_pixmap(
-	    ps->backend_data, pixmap, x_get_visual_info(ps->c, w->a.visual), true);
-	if (!*win_image) {
+	log_debug("New named pixmap for %#010x (%s) : %#010x", w->base.id, w->name, pixmap);
+	w->win_image =
+	    b->ops->bind_pixmap(b, pixmap, x_get_visual_info(b->c, w->a.visual), true);
+	if (!w->win_image) {
+		log_error("Failed to bind pixmap");
+		w->flags |= WIN_FLAGS_IMAGE_ERROR;
 		return false;
 	}
-	if (w->shadow) {
-		*shadow_image = ps->backend_data->ops->render_shadow(
-		    ps->backend_data, w->widthb, w->heightb, ps->gaussian_map, ps->o.shadow_red,
-		    ps->o.shadow_green, ps->o.shadow_blue, ps->o.shadow_opacity);
-		if (!*shadow_image) {
-			log_error("Failed to bind shadow image");
-			ps->backend_data->ops->release_image(ps->backend_data, *win_image);
-			*win_image = NULL;
-			return false;
-		}
-	}
+
+	w->flags &= ~WIN_FLAGS_PIXMAP_NONE;
 	return true;
 }
 
-bool win_bind_image(session_t *ps, struct managed_win *w) {
-	assert(!w->win_image && !w->shadow_image);
-	return _win_bind_image(ps, w, &w->win_image, &w->shadow_image);
+static inline bool win_bind_shadow(struct backend_base *b, struct managed_win *w,
+                                   struct color c, struct conv *kernel) {
+	assert(!w->shadow_image);
+	assert(w->shadow);
+	w->shadow_image = b->ops->render_shadow(b, w->widthb, w->heightb, kernel, c.red,
+	                                        c.green, c.blue, c.alpha);
+	if (!w->shadow_image) {
+		log_error("Failed to bind shadow image");
+		w->flags |= WIN_FLAGS_IMAGE_ERROR;
+		return false;
+	}
+
+	log_debug("New shadow for %#010x (%s)", w->base.id, w->name);
+	w->flags &= ~WIN_FLAGS_SHADOW_NONE;
+	return true;
 }
 
-bool win_try_rebind_image(session_t *ps, struct managed_win *w) {
-	log_trace("Freeing old window image");
-	// Must release first, otherwise breaks NVIDIA driver
-	win_release_image(ps->backend_data, w);
+void win_release_images(struct backend_base *backend, struct managed_win *w) {
+	// we don't want to consider what we should do if the image we want to release is
+	// stale (do we clear the stale flags or not?)
+	assert((w->flags & WIN_FLAGS_IMAGES_STALE) == 0);
 
-	return win_bind_image(ps, w);
+	if ((w->flags & WIN_FLAGS_PIXMAP_NONE) == 0) {
+		win_release_pixmap(backend, w);
+	}
+
+	if ((w->flags & WIN_FLAGS_SHADOW_NONE) == 0) {
+		win_release_shadow(backend, w);
+	}
 }
 
-void win_process_flags(struct session *ps, struct managed_win *w) {
-	if ((w->flags & WIN_FLAGS_IMAGE_STALE) != 0 && (w->flags & WIN_FLAGS_IMAGE_ERROR) == 0) {
+void win_bind_image(struct backend_base *backend, struct managed_win *w, struct color c,
+                    struct conv *kernel) {
+	win_bind_pixmap(backend, w);
+	if (w->shadow) {
+		win_bind_shadow(backend, w, c, kernel);
+	}
+}
+
+void win_process_flags(session_t *ps, struct managed_win *w) {
+	if (!w->flags || (w->flags & WIN_FLAGS_IMAGE_ERROR) != 0) {
+		return;
+	}
+
+	// Not a loop
+	while ((w->flags & WIN_FLAGS_IMAGES_STALE) != 0) {
 		// Image needs to be updated, update it.
-		w->flags &= ~WIN_FLAGS_IMAGE_STALE;
-		if (w->state != WSTATE_UNMAPPING && w->state != WSTATE_DESTROYING &&
-		    ps->backend_data) {
-			// Rebind image only when the window does have an image
-			// available
-			if (!win_try_rebind_image(ps, w)) {
-				w->flags |= WIN_FLAGS_IMAGE_ERROR;
+		if (w->state == WSTATE_UNMAPPING || w->state == WSTATE_DESTROYING ||
+		    !ps->backend_data) {
+			// Window is already gone, or we are using the legacy backend
+			// we cannot rebind image
+			break;
+		}
+
+		// Must release images first, otherwise breaks NVIDIA driver
+		if ((w->flags & WIN_FLAGS_PIXMAP_STALE) != 0) {
+			if ((w->flags & WIN_FLAGS_PIXMAP_NONE) == 0) {
+				win_release_pixmap(ps->backend_data, w);
+			}
+			win_bind_pixmap(ps->backend_data, w);
+		}
+
+		if ((w->flags & WIN_FLAGS_SHADOW_STALE) != 0) {
+			if ((w->flags & WIN_FLAGS_SHADOW_NONE) == 0) {
+				win_release_shadow(ps->backend_data, w);
+			}
+			if (w->shadow) {
+				win_bind_shadow(ps->backend_data, w,
+				                (struct color){.red = ps->o.shadow_red,
+				                               .green = ps->o.shadow_green,
+				                               .blue = ps->o.shadow_blue,
+				                               .alpha = ps->o.shadow_opacity},
+				                ps->gaussian_map);
 			}
 		}
+
+		// Flags are cleared here, loop always run only once
+		w->flags &= ~WIN_FLAGS_IMAGES_STALE;
 	}
 }
 
@@ -865,7 +914,7 @@ void win_on_win_size_change(session_t *ps, struct managed_win *w) {
 	// Invalidate the shadow we built
 	if (w->state == WSTATE_MAPPED || w->state == WSTATE_MAPPING ||
 	    w->state == WSTATE_FADING) {
-		w->flags |= WIN_FLAGS_IMAGE_STALE;
+		w->flags |= WIN_FLAGS_IMAGES_STALE;
 		ps->pending_updates = true;
 	} else {
 		assert(w->state == WSTATE_UNMAPPED);
@@ -1097,7 +1146,7 @@ struct win *fill_win(session_t *ps, struct win *w) {
 	    .in_openclose = true,             // set to false after first map is done,
 	                                      // true here because window is just created
 	    .reg_ignore_valid = false,        // set to true when damaged
-	    .flags = 0,                       // updated by property/attributes/etc change
+	    .flags = WIN_FLAGS_IMAGES_NONE,   // updated by property/attributes/etc change
 
 	    // Runtime variables, updated by dbus
 	    .fade_force = UNSET,
@@ -1504,7 +1553,7 @@ void win_update_bounding_shape(session_t *ps, struct managed_win *w) {
 		// Note we only do this when screen is redirected, because
 		// otherwise win_data is not valid
 		assert(w->state != WSTATE_UNMAPPING && w->state != WSTATE_DESTROYING);
-		w->flags |= WIN_FLAGS_IMAGE_STALE;
+		w->flags |= WIN_FLAGS_IMAGES_STALE;
 		ps->pending_updates = true;
 	}
 	free_paint(ps, &w->paint);
@@ -1608,12 +1657,13 @@ static void finish_unmap_win(session_t *ps, struct managed_win *w) {
 
 	// We are in unmap_win, this window definitely was viewable
 	if (ps->backend_data) {
-		win_release_image(ps->backend_data, w);
+		win_release_images(ps->backend_data, w);
 	}
 	free_paint(ps, &w->paint);
 	free_paint(ps, &w->shadow_paint);
 
-	w->flags = 0;
+	// Try again at binding images when the window is mapped next time
+	w->flags &= ~WIN_FLAGS_IMAGE_ERROR;
 }
 
 /// Finish the destruction of a window (e.g. after fading has finished).
@@ -1929,6 +1979,9 @@ void map_win(session_t *ps, struct managed_win *w) {
 		assert(w);
 	}
 
+	assert((w->flags & WIN_FLAGS_IMAGES_NONE) == WIN_FLAGS_IMAGES_NONE ||
+	       !ps->o.experimental_backends);
+
 	// We stopped processing window size change when we were unmapped, refresh the
 	// size of the window
 	xcb_get_geometry_cookie_t gcookie = xcb_get_geometry(ps->c, w->base.id);
@@ -2020,13 +2073,16 @@ void map_win(session_t *ps, struct managed_win *w) {
 	//
 	// Also because NVIDIA driver doesn't like seeing the same pixmap under different
 	// ids, so avoid naming the pixmap again when it didn't actually change.
-	w->flags &= ~WIN_FLAGS_IMAGE_STALE;
+	w->flags &= ~WIN_FLAGS_IMAGES_STALE;
 
 	// Bind image after update_bounding_shape, so the shadow has the correct size.
 	if (ps->backend_data) {
-		if (!win_bind_image(ps, w)) {
-			w->flags |= WIN_FLAGS_IMAGE_ERROR;
-		}
+		win_bind_image(ps->backend_data, w,
+		               (struct color){.red = ps->o.shadow_red,
+		                              .green = ps->o.shadow_green,
+		                              .blue = ps->o.shadow_blue,
+		                              .alpha = ps->o.shadow_opacity},
+		               ps->gaussian_map);
 	}
 
 #ifdef CONFIG_DBUS
diff --git a/src/win.h b/src/win.h
index 6e3163c..b0e1272 100644
--- a/src/win.h
+++ b/src/win.h
@@ -250,14 +250,14 @@ struct managed_win {
 #endif
 };
 
-void win_release_image(struct backend_base *base, struct managed_win *w);
-bool must_use win_bind_image(session_t *ps, struct managed_win *w);
-
 /// Process pending images flags on a window. Has to be called in X critical section
 void win_process_flags(session_t *ps, struct managed_win *w);
 
-/// Attempt a rebind of window's images. If that failed, the original images are kept.
-bool must_use win_try_rebind_image(session_t *ps, struct managed_win *w);
+/// Release images bound with a window, set the *_NONE flags on the window. Only to be
+/// used when de-initializing the backend outside of win.c
+void win_release_images(struct backend_base *base, struct managed_win *w);
+void win_bind_image(struct backend_base *backend, struct managed_win *w, struct color c,
+                    struct conv *kernel);
 int win_get_name(session_t *ps, struct managed_win *w);
 int win_get_role(session_t *ps, struct managed_win *w);
 winmode_t attr_pure win_calc_mode(const struct managed_win *w);
diff --git a/src/win_defs.h b/src/win_defs.h
index 423ea9b..b86f3ce 100644
--- a/src/win_defs.h
+++ b/src/win_defs.h
@@ -1,4 +1,5 @@
 #pragma once
+#include <stdint.h>
 
 typedef enum {
 	WINTYPE_UNKNOWN,
@@ -67,8 +68,22 @@ typedef enum {
 } winstate_t;
 
 enum win_flags {
-	/// win_image/shadow_image is out of date
-	WIN_FLAGS_IMAGE_STALE = 1,
+	// Note: *_NONE flags are mostly redudant and meant for detecting logical errors
+	// in the code
+
+	/// pixmap is out of date, will be update in win_process_flags
+	WIN_FLAGS_PIXMAP_STALE = 1,
+	/// window does not have pixmap bound
+	WIN_FLAGS_PIXMAP_NONE = 2,
 	/// there was an error trying to bind the images
-	WIN_FLAGS_IMAGE_ERROR = 2,
+	WIN_FLAGS_IMAGE_ERROR = 4,
+	/// shadow is out of date, will be updated in win_process_flags
+	WIN_FLAGS_SHADOW_STALE = 8,
+	/// shadow has not been generated
+	WIN_FLAGS_SHADOW_NONE = 16,
 };
+
+static const int_fast16_t WIN_FLAGS_IMAGES_STALE =
+    WIN_FLAGS_PIXMAP_STALE | WIN_FLAGS_SHADOW_STALE;
+
+static const int_fast16_t WIN_FLAGS_IMAGES_NONE = WIN_FLAGS_PIXMAP_NONE | WIN_FLAGS_SHADOW_NONE;

From fd570e1b78f003aabb193b9d656e1024e92131e4 Mon Sep 17 00:00:00 2001
From: Yuxuan Shui <yshuiv7@gmail.com>
Date: Fri, 20 Sep 2019 01:23:44 +0100
Subject: [PATCH 06/11] win: split unmap_win into unmap and destroy

Instead of doing both unmapping and destroying in the same function,
since that complicates the logic.

Also since now destroy_win handles both the managed and unmanaged cases,
remove destroy_unmanage_win function.

Signed-off-by: Yuxuan Shui <yshuiv7@gmail.com>
---
 src/event.c |  19 ++---
 src/win.c   | 202 +++++++++++++++++++++++++++-------------------------
 src/win.h   |  14 ++--
 3 files changed, 118 insertions(+), 117 deletions(-)

diff --git a/src/event.c b/src/event.c
index de98f88..9bf95da 100644
--- a/src/event.c
+++ b/src/event.c
@@ -262,11 +262,7 @@ static inline void ev_configure_notify(session_t *ps, xcb_configure_notify_event
 static inline void ev_destroy_notify(session_t *ps, xcb_destroy_notify_event_t *ev) {
 	auto w = find_win(ps, ev->window);
 	if (w) {
-		if (w->managed) {
-			auto _ attr_unused = unmap_win(ps, (struct managed_win *)w, true);
-		} else {
-			destroy_unmanaged_win(ps, w);
-		}
+		auto _ attr_unused = destroy_win(ps, w);
 	}
 }
 
@@ -299,7 +295,7 @@ static inline void ev_map_notify(session_t *ps, xcb_map_notify_event_t *ev) {
 static inline void ev_unmap_notify(session_t *ps, xcb_unmap_notify_event_t *ev) {
 	auto w = find_managed_win(ps, ev->window);
 	if (w) {
-		auto _ attr_unused = unmap_win(ps, w, false);
+		unmap_win(ps, w);
 	}
 }
 
@@ -322,13 +318,10 @@ static inline void ev_reparent_notify(session_t *ps, xcb_reparent_notify_event_t
 		}
 	} else {
 		// otherwise, find and destroy the window first
-		auto w = find_win(ps, ev->window);
-		if (w) {
-			if (w->managed) {
-				auto _ attr_unused =
-				    unmap_win(ps, (struct managed_win *)w, true);
-			} else {
-				destroy_unmanaged_win(ps, w);
+		{
+			auto w = find_win(ps, ev->window);
+			if (w) {
+				auto _ attr_unused = destroy_win(ps, w);
 			}
 		}
 
diff --git a/src/win.c b/src/win.c
index a502b2a..f39a9c6 100644
--- a/src/win.c
+++ b/src/win.c
@@ -1668,51 +1668,58 @@ static void finish_unmap_win(session_t *ps, struct managed_win *w) {
 
 /// Finish the destruction of a window (e.g. after fading has finished).
 /// Frees `w`
-static void finish_destroy_win(session_t *ps, struct managed_win *w) {
-	log_trace("Trying to finish destroying (%#010x)", w->base.id);
+static void finish_destroy_win(session_t *ps, struct win *w) {
+	log_trace("Trying to finish destroying (%#010x)", w->id);
 
-	if (w->state != WSTATE_UNMAPPED) {
-		// Only UNMAPPED state has window resources freed, otherwise
-		// we need to call finish_unmap_win to free them.
-		finish_unmap_win(ps, w);
-	}
+	auto next_w = win_stack_find_next_managed(ps, &w->stack_neighbour);
+	list_remove(&w->stack_neighbour);
 
-	// Invalidate reg_ignore of windows below this one
-	// TODO what if w->next is not mapped??
-	// TODO seriously figure out how reg_ignore behaves.
-	//      I think if `w` is unmapped, and destroyed after
-	//      paint happened at least once, w->reg_ignore_valid would
-	//      be true, and there is no need to invalid w->next->reg_ignore
-	//      when w is destroyed.
-	auto next_w = win_stack_find_next_managed(ps, &w->base.stack_neighbour);
-	if (next_w) {
-		rc_region_unref(&next_w->reg_ignore);
-		next_w->reg_ignore_valid = false;
-	}
+	if (w->managed) {
+		auto mw = (struct managed_win *)w;
 
-	log_trace("Trying to destroy (%#010x)", w->base.id);
-	list_remove(&w->base.stack_neighbour);
+		if (mw->state != WSTATE_UNMAPPED) {
+			// Only UNMAPPED state has window resources freed, otherwise
+			// we need to call unmap_win_finish to free them.
+			// XXX actually we unmap_win_finish only frees the rendering
+			//     resources, we still need to call free_win_res. will fix
+			//     later.
+			finish_unmap_win(ps, mw);
+		}
 
-	if (w == ps->active_win) {
-		// Usually, the window cannot be the focused at destruction. FocusOut
-		// should be generated before the window is destroyed.
-		// We do this check just to be completely sure we don't have dangling
-		// references.
-		log_debug("window %#010x (%s) is destroyed while being focused",
-		          w->base.id, w->name);
-		ps->active_win = NULL;
-	}
+		// Invalidate reg_ignore of windows below this one
+		// TODO what if next_w is not mapped??
+		// TODO seriously figure out how reg_ignore behaves.
+		//      I think if `w` is unmapped, and destroyed after
+		//      paint happened at least once, w->reg_ignore_valid would
+		//      be true, and there is no need to invalid w->next->reg_ignore
+		//      when w is destroyed.
+		if (next_w) {
+			rc_region_unref(&next_w->reg_ignore);
+			next_w->reg_ignore_valid = false;
+		}
 
-	free_win_res(ps, w);
+		if (mw == ps->active_win) {
+			// Usually, the window cannot be the focused at destruction.
+			// FocusOut should be generated before the window is destroyed. We
+			// do this check just to be completely sure we don't have dangling
+			// references.
+			log_debug("window %#010x (%s) is destroyed while being focused",
+			          w->id, mw->name);
+			ps->active_win = NULL;
+		}
 
-	// Drop w from all prev_trans to avoid accessing freed memory in
-	// repair_win()
-	// TODO there can only be one prev_trans pointing to w
-	win_stack_foreach_managed(w2, &ps->window_stack) {
-		if (w == w2->prev_trans) {
-			w2->prev_trans = NULL;
+		free_win_res(ps, mw);
+
+		// Drop w from all prev_trans to avoid accessing freed memory in
+		// repair_win()
+		// TODO there can only be one prev_trans pointing to w
+		win_stack_foreach_managed(w2, &ps->window_stack) {
+			if (mw == w2->prev_trans) {
+				w2->prev_trans = NULL;
+			}
 		}
 	}
+
 	free(w);
 }
 
@@ -1721,14 +1728,6 @@ static void finish_map_win(struct managed_win *w) {
 	w->state = WSTATE_MAPPED;
 }
 
-void destroy_unmanaged_win(session_t *ps, struct win *w) {
-	assert(!w->managed);
-	assert(!w->destroyed);
-	list_remove(&w->stack_neighbour);
-	HASH_DEL(ps->windows, w);
-	free(w);
-}
-
 /// Move window `w` so it's before `next` in the list
 static inline void restack_win(session_t *ps, struct win *w, struct list_node *next) {
 	struct managed_win *mw = NULL;
@@ -1812,86 +1811,93 @@ void restack_top(session_t *ps, struct win *w) {
 	restack_win(ps, w, ps->window_stack.next);
 }
 
-/// Unmap or destroy a window
-/// @return whether the window is destroyed and freed
-bool unmap_win(session_t *ps, struct managed_win *w, bool destroy) {
-	winstate_t target_state = destroy ? WSTATE_DESTROYING : WSTATE_UNMAPPING;
+/// Start destroying a window. Windows cannot always be destroyed immediately
+/// because of fading and such.
+///
+/// @return whether the window has finished destroying and is freed
+bool destroy_win(session_t *ps, struct win *w) {
+	auto mw = (struct managed_win *)w;
+	assert(w);
 
-	if (unlikely(!w)) {
-		return false;
+	log_debug("Destroying %#010x \"%s\", managed = %d", w->id,
+	          (w->managed ? mw->name : NULL), w->managed);
+
+	// Delete destroyed window from the hash table, even though the window might still
+	// be rendered for a while. We need to make sure future window with the same
+	// window id won't confuse us. Keep the window in the window stack if it's managed
+	// and mapped, since we might still need to render it (e.g. fading out). Window
+	// will be removed from the stack when it finishes destroying.
+	HASH_DEL(ps->windows, w);
+
+	if (!w->managed || mw->state == WSTATE_UNMAPPED) {
+		// Window is already unmapped, or is an unmanged window, just destroy it
+		finish_destroy_win(ps, w);
+		return true;
 	}
 
-	if (!destroy && w->a._class == XCB_WINDOW_CLASS_INPUT_ONLY) {
-		// We don't care about mapping / unmapping of Input Only windows.
-		// But we need to remember to destroy them, so future window with
-		// the same id won't be handled incorrectly.
-		// Issue #119 was caused by this.
-		return false;
+	if (w->managed) {
+		// Update state flags of a managed window
+		mw->state = WSTATE_DESTROYING;
+		mw->a.map_state = XCB_MAP_STATE_UNMAPPED;
+		mw->in_openclose = true;
 	}
 
-	log_trace("Unmapping %#010x \"%s\", destroy = %d", w->base.id,
-	          (w ? w->name : NULL), destroy);
+	// don't need win_ev_stop because the window is gone anyway
+#ifdef CONFIG_DBUS
+	// Send D-Bus signal
+	if (ps->o.dbus) {
+		cdbus_ev_win_destroyed(ps, w);
+	}
+#endif
 
-	if (unlikely(w->state == WSTATE_DESTROYING && !destroy)) {
+	if (!ps->redirected) {
+		// Skip transition if we are not rendering
+		return win_skip_fading(ps, mw);
+	}
+
+	return false;
+}
+
+void unmap_win(session_t *ps, struct managed_win *w) {
+	assert(w);
+	assert(w->base.managed);
+	assert(w->a._class != XCB_WINDOW_CLASS_INPUT_ONLY);
+
+	log_debug("Unmapping %#010x \"%s\"", w->base.id, w->name);
+
+	if (unlikely(w->state == WSTATE_DESTROYING)) {
 		log_warn("Trying to undestroy a window?");
 		assert(false);
 	}
 
-	// If the window is already in the state we want
-	if (unlikely(w->state == target_state)) {
-		log_warn("%s a window twice", destroy ? "Destroying" : "Unmapping");
-		return false;
-	}
-
-	if (destroy) {
-		// Delete destroyed window from the hash table, so future window with the
-		// same window id won't confuse us.
-		// Keep the window in the window stack, since we might still need to
-		// render it (fading out). Window will be removed from the stack when
-		// fading finishes.
-		HASH_DEL(ps->windows, &w->base);
-	}
-
-	if (unlikely(w->state == WSTATE_UNMAPPED) || w->a._class == XCB_WINDOW_CLASS_INPUT_ONLY) {
-		if (unlikely(!destroy)) {
-			log_warn("Unmapping an already unmapped window %#010x %s twice",
-			         w->base.id, w->name);
-			return false;
-		}
-		// Window is already unmapped, or is an Input Only window, just destroy it
-		finish_destroy_win(ps, w);
-		return true;
+	if (unlikely(w->state == WSTATE_UNMAPPING || w->state == WSTATE_UNMAPPED)) {
+		log_warn("Trying to unmapping an already unmapped window %#010x "
+		         "\"%s\"",
+		         w->base.id, w->name);
+		assert(false);
+		return;
 	}
 
 	// Note we don't update focused window here. This will either be
 	// triggered by subsequence Focus{In, Out} event, or by recheck_focus
 
 	w->a.map_state = XCB_MAP_STATE_UNMAPPED;
-	w->state = target_state;
+	w->state = WSTATE_UNMAPPING;
 	w->opacity_target = win_calc_opacity_target(ps, w, false);
 
-	w->in_openclose = destroy;
-
 	// don't care about properties anymore
-	if (!destroy) {
-		win_ev_stop(ps, &w->base);
-	}
+	win_ev_stop(ps, &w->base);
 
 #ifdef CONFIG_DBUS
 	// Send D-Bus signal
 	if (ps->o.dbus) {
-		if (destroy) {
-			cdbus_ev_win_destroyed(ps, &w->base);
-		} else {
-			cdbus_ev_win_unmapped(ps, &w->base);
-		}
+		cdbus_ev_win_unmapped(ps, &w->base);
 	}
 #endif
 
 	if (!ps->redirected) {
-		return win_skip_fading(ps, w);
+		CHECK(!win_skip_fading(ps, w));
 	}
-	return false;
 }
 
 /**
@@ -1908,7 +1914,7 @@ bool win_check_fade_finished(session_t *ps, struct managed_win *w) {
 	if (w->opacity == w->opacity_target) {
 		switch (w->state) {
 		case WSTATE_UNMAPPING: finish_unmap_win(ps, w); return false;
-		case WSTATE_DESTROYING: finish_destroy_win(ps, w); return true;
+		case WSTATE_DESTROYING: finish_destroy_win(ps, &w->base); return true;
 		case WSTATE_MAPPING: finish_map_win(w); return false;
 		case WSTATE_FADING: w->state = WSTATE_MAPPED; break;
 		default: unreachable;
diff --git a/src/win.h b/src/win.h
index b0e1272..5133967 100644
--- a/src/win.h
+++ b/src/win.h
@@ -253,6 +253,14 @@ struct managed_win {
 /// Process pending images flags on a window. Has to be called in X critical section
 void win_process_flags(session_t *ps, struct managed_win *w);
 
+/// Start the unmap of a window. We cannot unmap immediately since we might need to fade
+/// the window out.
+void unmap_win(struct session *, struct managed_win *);
+
+/// Start the destroying of a window. Windows cannot always be destroyed immediately
+/// because of fading and such.
+bool must_use destroy_win(session_t *ps, struct win *w);
+
 /// Release images bound with a window, set the *_NONE flags on the window. Only to be
 /// used when de-initializing the backend outside of win.c
 void win_release_images(struct backend_base *base, struct managed_win *w);
@@ -358,12 +366,6 @@ void restack_above(session_t *ps, struct win *w, xcb_window_t below);
 void restack_bottom(session_t *ps, struct win *w);
 /// Move window `w` to the top of the stack
 void restack_top(session_t *ps, struct win *w);
-/// Unmap or destroy a window
-/// @return whether the window is destroyed and freed
-bool must_use unmap_win(session_t *ps, struct managed_win *, bool destroy);
-/// Destroy and free an unmanaged window
-void destroy_unmanaged_win(session_t *ps, struct win *w);
-
 void map_win(session_t *ps, struct managed_win *w);
 
 /**

From b9bf4fed608dac402b164ca391947eaa9610856c Mon Sep 17 00:00:00 2001
From: Yuxuan Shui <yshuiv7@gmail.com>
Date: Fri, 20 Sep 2019 01:41:56 +0100
Subject: [PATCH 07/11] win: rename some functions

rename map_win, unmap_win, destroy_win to map_win_start,
unmap_win_start, destroy_win_start respectively. To clarify their
intended functions. Also rename the corresponding finish_* functions to
*_finish so they are consistent.

Also some very minor code clean ups.

Signed-off-by: Yuxuan Shui <yshuiv7@gmail.com>
---
 src/compton.c |  2 +-
 src/event.c   |  8 ++++----
 src/win.c     | 28 +++++++++++++++-------------
 src/win.h     |  9 ++++++---
 4 files changed, 26 insertions(+), 21 deletions(-)

diff --git a/src/compton.c b/src/compton.c
index 1b2410d..84ffb80 100644
--- a/src/compton.c
+++ b/src/compton.c
@@ -1272,7 +1272,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) {
-				map_win(ps, mw);
+				map_win_start(ps, mw);
 
 				// This window might be damaged before we called fill_win
 				// and created the damage handle. And there is no way for
diff --git a/src/event.c b/src/event.c
index 9bf95da..9933c6e 100644
--- a/src/event.c
+++ b/src/event.c
@@ -262,7 +262,7 @@ static inline void ev_configure_notify(session_t *ps, xcb_configure_notify_event
 static inline void ev_destroy_notify(session_t *ps, xcb_destroy_notify_event_t *ev) {
 	auto w = find_win(ps, ev->window);
 	if (w) {
-		auto _ attr_unused = destroy_win(ps, w);
+		auto _ attr_unused = destroy_win_start(ps, w);
 	}
 }
 
@@ -285,7 +285,7 @@ static inline void ev_map_notify(session_t *ps, xcb_map_notify_event_t *ev) {
 		return;
 	}
 
-	map_win(ps, w);
+	map_win_start(ps, w);
 
 	// FocusIn/Out may be ignored when the window is unmapped, so we must
 	// recheck focus here
@@ -295,7 +295,7 @@ static inline void ev_map_notify(session_t *ps, xcb_map_notify_event_t *ev) {
 static inline void ev_unmap_notify(session_t *ps, xcb_unmap_notify_event_t *ev) {
 	auto w = find_managed_win(ps, ev->window);
 	if (w) {
-		unmap_win(ps, w);
+		unmap_win_start(ps, w);
 	}
 }
 
@@ -321,7 +321,7 @@ static inline void ev_reparent_notify(session_t *ps, xcb_reparent_notify_event_t
 		{
 			auto w = find_win(ps, ev->window);
 			if (w) {
-				auto _ attr_unused = destroy_win(ps, w);
+				auto _ attr_unused = destroy_win_start(ps, w);
 			}
 		}
 
diff --git a/src/win.c b/src/win.c
index f39a9c6..2887eff 100644
--- a/src/win.c
+++ b/src/win.c
@@ -1650,7 +1650,7 @@ void win_ev_stop(session_t *ps, const struct win *w) {
 
 /// Finish the unmapping of a window (e.g. after fading has finished).
 /// Doesn't free `w`
-static void finish_unmap_win(session_t *ps, struct managed_win *w) {
+static void unmap_win_finish(session_t *ps, struct managed_win *w) {
 	w->ever_damaged = false;
 	w->reg_ignore_valid = false;
 	w->state = WSTATE_UNMAPPED;
@@ -1668,7 +1668,7 @@ static void finish_unmap_win(session_t *ps, struct managed_win *w) {
 
 /// Finish the destruction of a window (e.g. after fading has finished).
 /// Frees `w`
-static void finish_destroy_win(session_t *ps, struct win *w) {
+static void destroy_win_finish(session_t *ps, struct win *w) {
 	log_trace("Trying to finish destroying (%#010x)", w->id);
 
 	auto next_w = win_stack_find_next_managed(ps, &w->stack_neighbour);
@@ -1683,7 +1683,7 @@ static void finish_destroy_win(session_t *ps, struct win *w) {
 			// XXX actually we unmap_win_finish only frees the rendering
 			//     resources, we still need to call free_win_res. will fix
 			//     later.
-			finish_unmap_win(ps, mw);
+			unmap_win_finish(ps, mw);
 		}
 
 		// Invalidate reg_ignore of windows below this one
@@ -1723,7 +1723,7 @@ static void finish_destroy_win(session_t *ps, struct win *w) {
 	free(w);
 }
 
-static void finish_map_win(struct managed_win *w) {
+static void map_win_finish(struct managed_win *w) {
 	w->in_openclose = false;
 	w->state = WSTATE_MAPPED;
 }
@@ -1815,7 +1815,7 @@ void restack_top(session_t *ps, struct win *w) {
 /// because of fading and such.
 ///
 /// @return whether the window has finished destroying and is freed
-bool destroy_win(session_t *ps, struct win *w) {
+bool destroy_win_start(session_t *ps, struct win *w) {
 	auto mw = (struct managed_win *)w;
 	assert(w);
 
@@ -1831,7 +1831,7 @@ bool destroy_win(session_t *ps, struct win *w) {
 
 	if (!w->managed || mw->state == WSTATE_UNMAPPED) {
 		// Window is already unmapped, or is an unmanged window, just destroy it
-		finish_destroy_win(ps, w);
+		destroy_win_finish(ps, w);
 		return true;
 	}
 
@@ -1858,7 +1858,7 @@ bool destroy_win(session_t *ps, struct win *w) {
 	return false;
 }
 
-void unmap_win(session_t *ps, struct managed_win *w) {
+void unmap_win_start(session_t *ps, struct managed_win *w) {
 	assert(w);
 	assert(w->base.managed);
 	assert(w->a._class != XCB_WINDOW_CLASS_INPUT_ONLY);
@@ -1913,9 +1913,9 @@ bool win_check_fade_finished(session_t *ps, struct managed_win *w) {
 	}
 	if (w->opacity == w->opacity_target) {
 		switch (w->state) {
-		case WSTATE_UNMAPPING: finish_unmap_win(ps, w); return false;
-		case WSTATE_DESTROYING: finish_destroy_win(ps, &w->base); return true;
-		case WSTATE_MAPPING: finish_map_win(w); return false;
+		case WSTATE_UNMAPPING: unmap_win_finish(ps, w); return false;
+		case WSTATE_DESTROYING: destroy_win_finish(ps, &w->base); return true;
+		case WSTATE_MAPPING: map_win_finish(w); return false;
 		case WSTATE_FADING: w->state = WSTATE_MAPPED; break;
 		default: unreachable;
 		}
@@ -1960,7 +1960,7 @@ void win_update_screen(session_t *ps, struct managed_win *w) {
 }
 
 /// Map an already registered window
-void map_win(session_t *ps, struct managed_win *w) {
+void map_win_start(session_t *ps, struct managed_win *w) {
 	assert(w);
 
 	// Don't care about window mapping if it's an InputOnly window
@@ -1980,7 +1980,10 @@ void map_win(session_t *ps, struct managed_win *w) {
 	if (w->state == WSTATE_UNMAPPING) {
 		CHECK(!win_skip_fading(ps, w));
 		// We skipped the unmapping process, the window was rendered, now it is
-		// not anymore. So we need to mark then unmapping window as damaged.
+		// not anymore. So we need to mark the then unmapping window as damaged.
+		//
+		// Solves problem when, for example, a window is unmapped then mapped in a
+		// different location
 		add_damage_from_win(ps, w);
 		assert(w);
 	}
@@ -2100,7 +2103,6 @@ void map_win(session_t *ps, struct managed_win *w) {
 
 	if (!ps->redirected) {
 		CHECK(!win_skip_fading(ps, w));
-		assert(w);
 	}
 }
 
diff --git a/src/win.h b/src/win.h
index 5133967..5c6d851 100644
--- a/src/win.h
+++ b/src/win.h
@@ -255,11 +255,15 @@ void win_process_flags(session_t *ps, struct managed_win *w);
 
 /// Start the unmap of a window. We cannot unmap immediately since we might need to fade
 /// the window out.
-void unmap_win(struct session *, struct managed_win *);
+void unmap_win_start(struct session *, struct managed_win *);
+
+/// Start the mapping of a window. We cannot map immediately since we might need to fade
+/// the window in.
+void map_win_start(struct session *, struct managed_win *);
 
 /// Start the destroying of a window. Windows cannot always be destroyed immediately
 /// because of fading and such.
-bool must_use destroy_win(session_t *ps, struct win *w);
+bool must_use destroy_win_start(session_t *ps, struct win *w);
 
 /// Release images bound with a window, set the *_NONE flags on the window. Only to be
 /// used when de-initializing the backend outside of win.c
@@ -366,7 +370,6 @@ void restack_above(session_t *ps, struct win *w, xcb_window_t below);
 void restack_bottom(session_t *ps, struct win *w);
 /// Move window `w` to the top of the stack
 void restack_top(session_t *ps, struct win *w);
-void map_win(session_t *ps, struct managed_win *w);
 
 /**
  * Execute fade callback of a window if fading finished.

From 16bff8ff41d933604f05ba6d75cacccf5617c10b Mon Sep 17 00:00:00 2001
From: Yuxuan Shui <yshuiv7@gmail.com>
Date: Fri, 20 Sep 2019 01:54:14 +0100
Subject: [PATCH 08/11] win: add managed_win_internal

Which includes the pending update flags for the window.

Using an internal struct makes sure the window update flags are opaque outside
of win.c

Signed-off-by: Yuxuan Shui <yshuiv7@gmail.com>
---
 src/win.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/src/win.c b/src/win.c
index 2887eff..f384ea6 100644
--- a/src/win.c
+++ b/src/win.c
@@ -43,6 +43,14 @@
 
 #include "win.h"
 
+// TODO Make more window states internal
+struct managed_win_internal {
+	struct managed_win base;
+
+	/// A bit mask of unhandled window updates
+	uint_fast32_t pending_updates;
+};
+
 #define OPAQUE (0xffffffff)
 static const int WIN_GET_LEADER_MAX_RECURSION = 20;
 static const int ROUNDED_PIXELS = 1;
@@ -1244,7 +1252,9 @@ struct win *fill_win(session_t *ps, struct win *w) {
 	}
 
 	// Allocate and initialize the new win structure
-	auto new = cmalloc(struct managed_win);
+	auto new_internal = cmalloc(struct managed_win_internal);
+	auto new = (struct managed_win *)new_internal;
+	new_internal->pending_updates = 0;
 
 	// Fill structure
 	// We only need to initialize the part that are not initialized

From 9f3d3f2fbadb5fee868bb08bb1e0ba542a3a4cb8 Mon Sep 17 00:00:00 2001
From: Yuxuan Shui <yshuiv7@gmail.com>
Date: Fri, 20 Sep 2019 01:59:25 +0100
Subject: [PATCH 09/11] win: add functions for delayed window updates

And a window update flag for mapping the window.

Also make sure related functions consider the case where the given window
has pending updates.

Signed-off-by: Yuxuan Shui <yshuiv7@gmail.com>
---
 src/compton.c  | 22 +++++++++++++++++-----
 src/event.c    |  4 ++--
 src/win.c      | 51 +++++++++++++++++++++++++++++++++++++++++++++-----
 src/win.h      |  7 +++++++
 src/win_defs.h |  5 +++++
 5 files changed, 77 insertions(+), 12 deletions(-)

diff --git a/src/compton.c b/src/compton.c
index 84ffb80..41dab06 100644
--- a/src/compton.c
+++ b/src/compton.c
@@ -1284,6 +1284,12 @@ static void handle_new_windows(session_t *ps) {
 	}
 }
 
+static void refresh_windows(session_t *ps) {
+	win_stack_foreach_managed_safe(w, &ps->window_stack) {
+		win_process_updates(ps, w);
+	}
+}
+
 static void refresh_stale_images(session_t *ps) {
 	win_stack_foreach_managed(w, &ps->window_stack) {
 		win_process_flags(ps, w);
@@ -1328,6 +1334,9 @@ static void handle_pending_updates(EV_P_ struct session *ps) {
 			recheck_focus(ps);
 		}
 
+		// Process window updates
+		refresh_windows(ps);
+
 		// Refresh pixmaps and shadows
 		refresh_stale_images(ps);
 
@@ -1374,11 +1383,14 @@ static void _draw_callback(EV_P_ session_t *ps, int revents attr_unused) {
 
 	if (!was_redirected && ps->redirected) {
 		// paint_preprocess redirected the screen, which might change the state of
-		// some of the windows (e.g. the window image might fail to bind, and the
-		// window would be put into an error state). so we rerun paint_preprocess
-		// here to make sure the rendering decision we make is up-to-date
-		log_debug("Re-run paint_preprocess");
-		bottom = paint_preprocess(ps, &fade_running);
+		// some of the windows (e.g. the window image might become stale).
+		// so we rerun _draw_callback to make sure the rendering decision we make
+		// is up-to-date, and all the new flags got handled.
+		//
+		// TODO This is not ideal, we should try to avoid setting window flags in
+		// paint_preprocess.
+		log_debug("Re-run _draw_callback");
+		return _draw_callback(EV_A_ ps, revents);
 	}
 
 	// Start/stop fade timer depends on whether window are fading
diff --git a/src/event.c b/src/event.c
index 9933c6e..a92fcf9 100644
--- a/src/event.c
+++ b/src/event.c
@@ -546,8 +546,8 @@ 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) {
-	if (w->a.map_state != XCB_MAP_STATE_VIEWABLE)
-		return;
+	// Only mapped window can receive damages
+	assert(win_is_mapped_in_x(w));
 
 	region_t parts;
 	pixman_region32_init(&parts);
diff --git a/src/win.c b/src/win.c
index f384ea6..c83992b 100644
--- a/src/win.c
+++ b/src/win.c
@@ -1869,6 +1869,7 @@ bool destroy_win_start(session_t *ps, struct win *w) {
 }
 
 void unmap_win_start(session_t *ps, struct managed_win *w) {
+	auto internal_w = (struct managed_win_internal *)w;
 	assert(w);
 	assert(w->base.managed);
 	assert(w->a._class != XCB_WINDOW_CLASS_INPUT_ONLY);
@@ -1881,10 +1882,14 @@ void unmap_win_start(session_t *ps, struct managed_win *w) {
 	}
 
 	if (unlikely(w->state == WSTATE_UNMAPPING || w->state == WSTATE_UNMAPPED)) {
-		log_warn("Trying to unmapping an already unmapped window %#010x "
-		         "\"%s\"",
-		         w->base.id, w->name);
-		assert(false);
+		if (internal_w->pending_updates & WIN_UPDATE_MAP) {
+			internal_w->pending_updates &= ~(unsigned long)WIN_UPDATE_MAP;
+		} else {
+			log_warn("Trying to unmapping an already unmapped window %#010x "
+			         "\"%s\"",
+			         w->base.id, w->name);
+			assert(false);
+		}
 		return;
 	}
 
@@ -2079,7 +2084,10 @@ void map_win_start(session_t *ps, struct managed_win *w) {
 
 	win_determine_blur_background(ps, w);
 
-	w->ever_damaged = false;
+	// Cannot set w->ever_damaged = false here, since window mapping could be
+	// delayed, so a damage event might have already arrived before this function
+	// is called. But this should be unnecessary in the first place, since
+	// ever_damaged is set to false in unmap_win_finish anyway.
 
 	// We stopped listening on ShapeNotify events
 	// when the window is unmapped (XXX we shouldn't),
@@ -2236,6 +2244,32 @@ win_is_fullscreen_xcb(xcb_connection_t *c, const struct atom *a, const xcb_windo
 	return false;
 }
 
+/// Queue an update on a window. A series of sanity checks are performed
+void win_queue_update(struct managed_win *_w, enum win_update update) {
+	auto w = (struct managed_win_internal *)_w;
+	assert(popcount(update) == 1);
+
+	if (unlikely(_w->state == WSTATE_DESTROYING)) {
+		log_error("Updates queued on a destroyed window %#010x (%s)", _w->base.id,
+		          _w->name);
+		return;
+	}
+
+	w->pending_updates |= WIN_UPDATE_MAP;
+}
+
+/// Process pending updates on a window. Has to be called in X critical section
+void win_process_updates(struct session *ps, struct managed_win *_w) {
+	assert(ps->server_grabbed);
+	auto w = (struct managed_win_internal *)_w;
+
+	if (w->pending_updates & WIN_UPDATE_MAP) {
+		map_win_start(ps, _w);
+	}
+
+	w->pending_updates = 0;
+}
+
 /**
  * Check if a window is a fullscreen window.
  *
@@ -2267,3 +2301,10 @@ win_stack_find_next_managed(const session_t *ps, const struct list_node *i) {
 	}
 	return NULL;
 }
+
+/// Return whether this window is mapped on the X server side
+bool win_is_mapped_in_x(const struct managed_win *w) {
+	auto iw = (const struct managed_win_internal *)w;
+	return w->state == WSTATE_MAPPING || w->state == WSTATE_FADING ||
+	       w->state == WSTATE_MAPPED || (iw->pending_updates & WIN_UPDATE_MAP);
+}
diff --git a/src/win.h b/src/win.h
index 5c6d851..e8dbd66 100644
--- a/src/win.h
+++ b/src/win.h
@@ -250,8 +250,12 @@ struct managed_win {
 #endif
 };
 
+/// Process pending updates on a window. Has to be called in X critical section
+void win_process_updates(struct session *ps, struct managed_win *_w);
 /// Process pending images flags on a window. Has to be called in X critical section
 void win_process_flags(session_t *ps, struct managed_win *w);
+/// Queue an update on a window. A series of sanity checks are performed
+void win_queue_update(struct managed_win *_w, enum win_update update);
 
 /// Start the unmap of a window. We cannot unmap immediately since we might need to fade
 /// the window out.
@@ -417,6 +421,9 @@ bool attr_pure win_has_alpha(const struct managed_win *w);
 /// check if reg_ignore_valid is true for all windows above us
 bool attr_pure win_is_region_ignore_valid(session_t *ps, const struct managed_win *w);
 
+/// Whether a given window is mapped on the X server side
+bool win_is_mapped_in_x(const struct managed_win *w);
+
 // Find the managed window immediately below `w` in the window stack
 struct managed_win *attr_pure win_stack_find_next_managed(const session_t *ps,
                                                           const struct list_node *w);
diff --git a/src/win_defs.h b/src/win_defs.h
index b86f3ce..bc43fc3 100644
--- a/src/win_defs.h
+++ b/src/win_defs.h
@@ -27,6 +27,11 @@ typedef enum {
 	WMODE_SOLID,              // The window is opaque including the frame
 } winmode_t;
 
+/// Pending window updates
+enum win_update {
+	WIN_UPDATE_MAP = 1,
+};
+
 /// Transition table:
 /// (DESTROYED is when the win struct is destroyed and freed)
 /// ('o' means in all other cases)

From 39a609acb0a8cc774641f00307889b033de907f7 Mon Sep 17 00:00:00 2001
From: Yuxuan Shui <yshuiv7@gmail.com>
Date: Fri, 20 Sep 2019 02:15:41 +0100
Subject: [PATCH 10/11] event: do delayed window mapping

This remove the only case where map_win_start is called outside of the
X critical section.

Signed-off-by: Yuxuan Shui <yshuiv7@gmail.com>
---
 src/compton.c | 3 +++
 src/event.c   | 2 +-
 src/win.c     | 2 ++
 3 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/compton.c b/src/compton.c
index 41dab06..f4cc3aa 100644
--- a/src/compton.c
+++ b/src/compton.c
@@ -1272,6 +1272,9 @@ 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);
 
 				// This window might be damaged before we called fill_win
diff --git a/src/event.c b/src/event.c
index a92fcf9..873d39d 100644
--- a/src/event.c
+++ b/src/event.c
@@ -285,7 +285,7 @@ static inline void ev_map_notify(session_t *ps, xcb_map_notify_event_t *ev) {
 		return;
 	}
 
-	map_win_start(ps, w);
+	win_queue_update(w, WIN_UPDATE_MAP);
 
 	// FocusIn/Out may be ignored when the window is unmapped, so we must
 	// recheck focus here
diff --git a/src/win.c b/src/win.c
index c83992b..23e6ad6 100644
--- a/src/win.c
+++ b/src/win.c
@@ -1976,6 +1976,7 @@ void win_update_screen(session_t *ps, struct managed_win *w) {
 
 /// Map an already registered window
 void map_win_start(session_t *ps, struct managed_win *w) {
+	assert(ps->server_grabbed);
 	assert(w);
 
 	// Don't care about window mapping if it's an InputOnly window
@@ -2003,6 +2004,7 @@ void map_win_start(session_t *ps, struct managed_win *w) {
 		assert(w);
 	}
 
+	assert(w->state == WSTATE_UNMAPPED);
 	assert((w->flags & WIN_FLAGS_IMAGES_NONE) == WIN_FLAGS_IMAGES_NONE ||
 	       !ps->o.experimental_backends);
 

From 3766aa2c793c1b529460410b942a293cd59da69f Mon Sep 17 00:00:00 2001
From: Yuxuan Shui <yshuiv7@gmail.com>
Date: Fri, 20 Sep 2019 02:18:16 +0100
Subject: [PATCH 11/11] win: make sure delayed image update is always used

Convert several places where the window image is bound/unbound directly
to use image flags. Make sure window image updates only happen in one
place.

Remove win_bind_image function since its no longer used after this.

Signed-off-by: Yuxuan Shui <yshuiv7@gmail.com>
---
 src/compton.c | 15 ++++++++-------
 src/win.c     | 46 +++++++++++-----------------------------------
 src/win.h     |  2 --
 3 files changed, 19 insertions(+), 44 deletions(-)

diff --git a/src/compton.c b/src/compton.c
index f4cc3aa..0af7ed3 100644
--- a/src/compton.c
+++ b/src/compton.c
@@ -752,13 +752,14 @@ static bool initialize_backend(session_t *ps) {
 				continue;
 			}
 			auto w = (struct managed_win *)_w;
-			if (w->a.map_state == XCB_MAP_STATE_VIEWABLE) {
-				win_bind_image(ps->backend_data, w,
-				               (struct color){.red = ps->o.shadow_red,
-				                              .green = ps->o.shadow_green,
-				                              .blue = ps->o.shadow_blue,
-				                              .alpha = ps->o.shadow_opacity},
-				               ps->gaussian_map);
+			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);
+				w->flags |= WIN_FLAGS_IMAGES_STALE;
+				ps->pending_updates = true;
 			}
 		}
 	}
diff --git a/src/win.c b/src/win.c
index 23e6ad6..ea84837 100644
--- a/src/win.c
+++ b/src/win.c
@@ -323,14 +323,6 @@ void win_release_images(struct backend_base *backend, struct managed_win *w) {
 	}
 }
 
-void win_bind_image(struct backend_base *backend, struct managed_win *w, struct color c,
-                    struct conv *kernel) {
-	win_bind_pixmap(backend, w);
-	if (w->shadow) {
-		win_bind_shadow(backend, w, c, kernel);
-	}
-}
-
 void win_process_flags(session_t *ps, struct managed_win *w) {
 	if (!w->flags || (w->flags & WIN_FLAGS_IMAGE_ERROR) != 0) {
 		return;
@@ -701,18 +693,10 @@ static void win_set_shadow(session_t *ps, struct managed_win *w, bool shadow_new
 	if (w->shadow) {
 		win_extents(w, &extents);
 		add_damage_from_win(ps, w);
-		if (ps->backend_data && w->state != WSTATE_UNMAPPED &&
-		    !(w->flags & WIN_FLAGS_IMAGE_ERROR)) {
+		if (w->state != WSTATE_UNMAPPED) {
 			assert(!w->shadow_image);
-			// Create shadow image
-			w->shadow_image = ps->backend_data->ops->render_shadow(
-			    ps->backend_data, w->widthb, w->heightb, ps->gaussian_map,
-			    ps->o.shadow_red, ps->o.shadow_green, ps->o.shadow_blue,
-			    ps->o.shadow_opacity);
-			if (!w->shadow_image) {
-				log_error("Failed to bind shadow image");
-				w->shadow_force = OFF;
-			}
+			// Delayed creation of shadow image
+			w->flags |= WIN_FLAGS_SHADOW_STALE;
 		}
 	}
 	pixman_region32_fini(&extents);
@@ -1668,7 +1652,11 @@ static void unmap_win_finish(session_t *ps, struct managed_win *w) {
 	// We are in unmap_win, this window definitely was viewable
 	if (ps->backend_data) {
 		win_release_images(ps->backend_data, w);
+	} else {
+		assert(!w->win_image);
+		assert(!w->shadow_image);
 	}
+
 	free_paint(ps, &w->paint);
 	free_paint(ps, &w->shadow_paint);
 
@@ -2095,24 +2083,12 @@ void map_win_start(session_t *ps, struct managed_win *w) {
 	// when the window is unmapped (XXX we shouldn't),
 	// so the shape of the window might have changed,
 	// update. (Issue #35)
+	//
+	// Also this sets the WIN_FLAGS_IMAGES_STALE flag so later in the critical section
+	// the window's image will be bound
 	win_update_bounding_shape(ps, w);
 
-	// Reset the STALE_IMAGE flag set by win_update_bounding_shape. Because we are
-	// just about to bind the image, no way that's stale.
-	//
-	// Also because NVIDIA driver doesn't like seeing the same pixmap under different
-	// ids, so avoid naming the pixmap again when it didn't actually change.
-	w->flags &= ~WIN_FLAGS_IMAGES_STALE;
-
-	// Bind image after update_bounding_shape, so the shadow has the correct size.
-	if (ps->backend_data) {
-		win_bind_image(ps->backend_data, w,
-		               (struct color){.red = ps->o.shadow_red,
-		                              .green = ps->o.shadow_green,
-		                              .blue = ps->o.shadow_blue,
-		                              .alpha = ps->o.shadow_opacity},
-		               ps->gaussian_map);
-	}
+	assert((w->flags & WIN_FLAGS_IMAGES_STALE) == WIN_FLAGS_IMAGES_STALE);
 
 #ifdef CONFIG_DBUS
 	// Send D-Bus signal
diff --git a/src/win.h b/src/win.h
index e8dbd66..38f015a 100644
--- a/src/win.h
+++ b/src/win.h
@@ -272,8 +272,6 @@ bool must_use destroy_win_start(session_t *ps, struct win *w);
 /// Release images bound with a window, set the *_NONE flags on the window. Only to be
 /// used when de-initializing the backend outside of win.c
 void win_release_images(struct backend_base *base, struct managed_win *w);
-void win_bind_image(struct backend_base *backend, struct managed_win *w, struct color c,
-                    struct conv *kernel);
 int win_get_name(session_t *ps, struct managed_win *w);
 int win_get_role(session_t *ps, struct managed_win *w);
 winmode_t attr_pure win_calc_mode(const struct managed_win *w);