wip

meson.build

@@ -68,7 +68,7 @@ if get_option('warning_level') != '0'
             'sign-compare', 'type-limits', 'uninitialized', 'shift-negative-value',
             'unused-but-set-parameter', 'unused-parameter', 'implicit-fallthrough=2',
             'no-unknown-warning-option', 'no-missing-braces', 'conversion', 'empty-body',
-            'no-c2x-extensions' ]
+            'no-c2x-extensions', 'no-discarded-qualifiers']
   bad_warns_ubsan = [
     'no-format-overflow' # see gcc bug 87884, enabling UBSAN makes gcc spit out spurious warnings
                          # (if you saw this comment, went and checked gcc bugzilla, and found out

src/event.c

@@ -281,7 +281,7 @@ static inline void ev_configure_notify(session_t *ps, xcb_configure_notify_event
 	          ev->event, ev->window, ev->above_sibling, ev->override_redirect);
 	if (ev->window == ps->c.screen_info->root) {
 		set_root_flags(ps, ROOT_FLAGS_CONFIGURED);
-	} else {
+	} else if (!wm_is_wid_masked(ps->wm, ev->event)) {
 		configure_win(ps, ev);
 	}
 }
@@ -307,6 +307,10 @@ static inline void ev_map_notify(session_t *ps, xcb_map_notify_event_t *ev) {
 		return;
 	}
 
+	if (wm_is_wid_masked(ps->wm, ev->event)) {
+		return;
+	}
+
 	auto cursor = wm_find(ps->wm, ev->window);
 	if (cursor == NULL) {
 		log_error("Map event received for unknown window %#010x, overlay is "
@@ -338,6 +342,10 @@ static inline void ev_unmap_notify(session_t *ps, xcb_unmap_notify_event_t *ev)
 		return;
 	}
 
+	if (wm_is_wid_masked(ps->wm, ev->event)) {
+		return;
+	}
+
 	auto cursor = wm_find(ps->wm, ev->window);
 	if (cursor == NULL) {
 		log_error("Unmap event received for unknown window %#010x", ev->window);
@@ -350,27 +358,16 @@ static inline void ev_unmap_notify(session_t *ps, xcb_unmap_notify_event_t *ev)
 }
 
 static inline void ev_reparent_notify(session_t *ps, xcb_reparent_notify_event_t *ev) {
-	log_debug("Window %#010x has new parent: %#010x, override_redirect: %d",
-	          ev->window, ev->parent, ev->override_redirect);
+	log_debug("Window %#010x has new parent: %#010x, override_redirect: %d, send_event: %#010x",
+	          ev->window, ev->parent, ev->override_redirect, ev->event);
+	wm_reparent(ps->wm, ev->window, ev->parent);
+}
 
-	auto cursor = wm_find(ps->wm, ev->window);
-	if (cursor == NULL) {
-		log_error("Reparent event received for unknown window %#010x", ev->window);
-		return;
-	}
-	auto new_parent = wm_find(ps->wm, ev->parent);
-	if (new_parent == NULL) {
-		log_error("Reparent event received for window %#010x, but its new parent "
-		          "window %#010x is not in our tree, ignoring. Expect "
-		          "malfunction.",
-		          ev->window, ev->parent);
+static inline void ev_circulate_notify(session_t *ps, xcb_circulate_notify_event_t *ev) {
+	if (wm_is_wid_masked(ps->wm, ev->event)) {
 		return;
 	}
 
-	wm_reparent(ps->wm, cursor, new_parent);
-}
-
-static inline void ev_circulate_notify(session_t *ps, xcb_circulate_notify_event_t *ev) {
 	auto cursor = wm_find(ps->wm, ev->window);
 
 	if (cursor == NULL) {
@@ -446,6 +443,10 @@ static inline void ev_property_notify(session_t *ps, xcb_property_notify_event_t
 		return;
 	}
 
+	if (wm_is_wid_masked(ps->wm, ev->window)) {
+		return;
+	}
+
 	ps->pending_updates = true;
 	auto cursor = wm_find(ps->wm, ev->window);
 	if (cursor == NULL) {

src/picom.c

@@ -1660,7 +1660,7 @@ static void handle_pending_updates(EV_P_ struct session *ps, double delta_t) {
 
 	// Catching up with X server
 	handle_queued_x_events(EV_A, &ps->event_check, 0);
-	if (ps->pending_updates || wm_has_incomplete_imports(ps->wm)) {
+	if (ps->pending_updates || wm_has_incomplete_imports(ps->wm) || wm_has_tree_changes(ps->wm)) {
 		log_debug("Delayed handling of events, entering critical section");
 		// Process new windows, and maybe allocate struct managed_win for them
 		handle_new_windows(ps);
@@ -1692,6 +1692,7 @@ static void handle_pending_updates(EV_P_ struct session *ps, double delta_t) {
 
 	wm_stack_foreach_safe(ps->wm, cursor, tmp) {
 		auto w = wm_ref_deref(cursor);
+		BUG_ON(w->tree_ref != cursor);
 		// Window might be freed by this function, if it's destroyed and its
 		// animation finished
 		if (w != NULL && win_process_animation_and_state_change(ps, w, delta_t)) {

src/wm/tree.c

@@ -254,6 +254,7 @@ wm_tree_new_window(struct wm_tree *tree, xcb_window_t id, struct wm_tree_node *p
 			wm_tree_enqueue_change(tree, (struct wm_tree_change){
 			                                 .toplevel = node->id,
 			                                 .type = WM_TREE_CHANGE_TOPLEVEL_NEW,
+											 .new_ = node,
 			                             });
 		}
 	}
@@ -267,10 +268,11 @@ wm_tree_refresh_client_and_queue_change(struct wm_tree *tree, struct wm_tree_nod
 	BUG_ON(toplevel->parent->parent != NULL);
 	auto new_client = wm_tree_find_client(toplevel);
 	if (new_client != toplevel->client_window) {
-		struct wm_tree_change change = {
-		    .toplevel = toplevel->id,
-		    .type = WM_TREE_CHANGE_CLIENT,
-		    .client = {.old = WM_TREEID_NONE, .new_ = WM_TREEID_NONE}};
+		struct wm_tree_change change = {.toplevel = toplevel->id,
+		                                .type = WM_TREE_CHANGE_CLIENT,
+		                                .client = {.toplevel = toplevel,
+		                                           .old = WM_TREEID_NONE,
+		                                           .new_ = WM_TREEID_NONE}};
 		if (toplevel->client_window != NULL) {
 			change.client.old = toplevel->client_window->id;
 		}
@@ -403,9 +405,13 @@ void wm_tree_reparent(struct wm_tree *tree, struct wm_tree_node *node,
 
 	auto toplevel = wm_tree_find_toplevel_for(node);
 	if (node == toplevel) {
+		// This node could have a stale `->win` if it was a toplevel at
+		// some point in the past.
+		node->win = NULL;
 		wm_tree_enqueue_change(tree, (struct wm_tree_change){
 		                                 .toplevel = node->id,
 		                                 .type = WM_TREE_CHANGE_TOPLEVEL_NEW,
+										 .new_ = node,
 		                             });
 	} else {
 		wm_tree_refresh_client_and_queue_change(tree, toplevel);

src/wm/wm.c

@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: MPL-2.0
 // Copyright (c) Yuxuan Shui
 
+#include <stddef.h>
 #include <uthash.h>
 #include <xcb/xproto.h>
 
@@ -31,6 +32,9 @@ struct wm {
 	/// Incomplete imports. See `wm_import_incomplete` for an explanation.
 	/// This is a dynarr.
 	struct wm_tree_node **incompletes;
+	/// Tree nodes that we have chosen to forget, but we might still receive some
+	/// events from, we keep them here to ignore those events.
+	struct wm_tree_node **masked;
 };
 
 // TODO(yshui): this is a bit weird and I am not decided on it yet myself. Maybe we can
@@ -56,8 +60,31 @@ static inline struct wm_tree_node *to_tree_node_mut(struct wm_ref *cursor) {
 	                      : NULL;
 }
 
+static ptrdiff_t wm_find_masked(struct wm *wm, xcb_window_t wid) {
+	dynarr_foreach(wm->masked, m) {
+		if ((*m)->id.x == wid) {
+			return m - wm->masked;
+		}
+	}
+	return -1;
+}
+
+bool wm_is_wid_masked(struct wm *wm, xcb_window_t wid) {
+	return wm_find_masked(wm, wid) != -1;
+}
+
 struct win *wm_ref_deref(const struct wm_ref *cursor) {
-	return to_tree_node(cursor)->win;
+	auto node = to_tree_node(cursor);
+	if (node->parent == NULL) {
+		log_error("Trying to dereference a root node. Expect malfunction.");
+		return NULL;
+	}
+	if (node->parent->parent != NULL) {
+		// Don't return the client window if this is not a toplevel node. This
+		// saves us from needing to clear `->win` when a window is reparented.
+		return NULL;
+	}
+	return node->win;
 }
 
 void wm_ref_set(struct wm_ref *cursor, struct win *w) {
@@ -160,6 +187,7 @@ struct wm *wm_new(void) {
 	auto wm = ccalloc(1, struct wm);
 	wm_tree_init(&wm->tree);
 	wm->incompletes = dynarr_new(struct wm_tree_node *, 4);
+	wm->masked = dynarr_new(struct wm_tree_node *, 8);
 	return wm;
 }
 
@@ -181,11 +209,18 @@ void wm_free(struct wm *wm) {
 	}
 	wm_tree_clear(&wm->tree);
 	dynarr_free_pod(wm->incompletes);
+	dynarr_free_pod(wm->masked);
 
 	free(wm);
 }
 
 void wm_destroy(struct wm *wm, xcb_window_t wid) {
+	auto masked = wm_find_masked(wm, wid);
+	if (masked != -1) {
+		dynarr_remove_swap(wm->masked, (size_t)masked);
+		return;
+	}
+
 	struct wm_tree_node *node = wm_tree_find(&wm->tree, wid);
 	if (!node) {
 		log_error("Trying to destroy window %#010x, but it's not in the tree. "
@@ -206,19 +241,60 @@ void wm_reap_zombie(struct wm_ref *zombie) {
 	wm_tree_reap_zombie(to_tree_node_mut(zombie));
 }
 
-void wm_reparent(struct wm *wm, struct wm_ref *cursor, struct wm_ref *new_parent) {
-	wm_tree_reparent(&wm->tree, to_tree_node_mut(cursor), to_tree_node_mut(new_parent));
+void wm_reparent(struct wm *wm, xcb_window_t wid, xcb_window_t parent) {
+	auto window = wm_tree_find(&wm->tree, wid);
+	auto new_parent = wm_tree_find(&wm->tree, parent);
+	// We delete the window here if parent is not found, or if the parent is
+	// an incomplete import. We will rediscover this window later in
+	// `wm_complete_import`. Keeping it around will only confuse us.
+	bool should_forget =
+	    new_parent == NULL || dynarr_find_pod(wm->incompletes, new_parent) != -1;
+	if (window == NULL) {
+		log_debug("Reparenting window %#010x which is not in our tree. Assuming "
+		          "it came from fog of war.",
+		          wid);
+		if (!should_forget) {
+			wm_import_incomplete(wm, wid, parent);
+		}
+		return;
+	}
+	if (should_forget) {
+		log_debug("Window %#010x reparented to window %#010x which is "
+		          "%s, forgetting and masking instead.",
+		          window->id.x, parent,
+		          new_parent == NULL ? "not in our tree" : "an incomplete import");
+
+		wm_tree_detach(&wm->tree, window);
+		for (auto curr = window; curr != NULL; curr = wm_tree_next(curr, window)) {
+			HASH_DEL(wm->tree.nodes, curr);
+			dynarr_push(wm->masked, curr);
+		}
+	} else {
+		wm_tree_reparent(&wm->tree, window, new_parent);
+	}
 }
 
 void wm_set_has_wm_state(struct wm *wm, struct wm_ref *cursor, bool has_wm_state) {
 	wm_tree_set_wm_state(&wm->tree, to_tree_node_mut(cursor), has_wm_state);
 }
 
 void wm_import_incomplete(struct wm *wm, xcb_window_t wid, xcb_window_t parent) {
+	auto masked = wm_find_masked(wm, wid);
+	if (masked != -1) {
+		// A new window created with the same wid means the window we chose to
+		// forget has been deleted (without us knowing), and its ID was then
+		// reused.
+		dynarr_remove_swap(wm->masked, (size_t)masked);
+	}
 	auto parent_cursor = NULL;
 	if (parent != XCB_NONE) {
 		parent_cursor = wm_tree_find(&wm->tree, parent);
-		BUG_ON_NULL(parent_cursor);
+		if (parent_cursor == NULL) {
+			log_error("Importing window %#010x, but its parent %#010x is not "
+			          "in our tree, ignoring. Expect malfunction.",
+			          wid, parent);
+			return;
+		}
 	}
 	log_debug("Importing window %#010x with parent %#010x", wid, parent);
 	auto new = wm_tree_new_window(&wm->tree, wid, parent_cursor);
@@ -267,48 +343,36 @@ static void wm_complete_import_subtree(struct wm *wm, struct x_connection *c,
 			// means the windows will naturally be in the correct stacking
 			// order.
 			auto existing = wm_tree_find(&wm->tree, children[i]);
-			bool is_incomplete = false;
 			if (existing != NULL) {
-				// We might have already known about this window, but that
-				// doesn't mean we have setup, for example, event masks
-				// for it, if it's on the incomplete list. And if it is,
-				// we also need to remove it.
+				// This should never happen: we haven't subscribed to
+				// child creation events yet, and any window reparented to
+				// an incomplete is deleted. report an error and try to
+				// recover.
 				auto index = dynarr_find_pod(wm->incompletes, existing);
-				is_incomplete = index != -1;
-				if (is_incomplete) {
+				if (index != -1) {
 					dynarr_remove_swap(wm->incompletes, (size_t)index);
 				}
-
-				xcb_window_t parent_id = existing->parent != NULL
-				                             ? existing->parent->id.x
-				                             : XCB_NONE;
-				if (existing->parent != curr) {
-					log_error("X says %#010x is a child of %#010x, "
-					          "but according to us it's a child of "
-					          "%#010x. Trying to recover, but expect "
-					          "malfunction.",
-					          children[i], curr->id.x, parent_id);
-					wm_tree_reparent(&wm->tree, existing, curr);
-				} else {
-					log_debug("We already know about window %#010x, "
-					          "moving it to the top.",
-					          children[i]);
-					wm_tree_move_to_end(&wm->tree, existing, false);
-				}
-			} else {
-				is_incomplete = true;
-				existing = wm_tree_new_window(&wm->tree, children[i], curr);
-			}
-
-			if (is_incomplete) {
-				wm_complete_import_single(wm, c, atoms, existing);
+				log_error("Window %#010x already exists in the tree, but "
+				          "it appeared again as a "
+				          "child of window %#010x. Deleting the old one, "
+				          "but expect malfunction.",
+				          children[i], curr->id.x);
+				wm_tree_destroy_window(&wm->tree, existing);
 			}
+			existing = wm_tree_new_window(&wm->tree, children[i], curr);
+			wm_complete_import_single(wm, c, atoms, existing);
 		}
 		free(tree);
 	}
 }
 
 void wm_complete_import(struct wm *wm, struct x_connection *c, struct atom *atoms) {
+	// Unveil the fog of war
+	dynarr_foreach(wm->masked, m) {
+		free(*m);
+	}
+	dynarr_clear_pod(wm->masked);
+
 	while (!dynarr_is_empty(wm->incompletes)) {
 		auto i = dynarr_pop(wm->incompletes);
 		// This function modifies `wm->incompletes`, so we can't use
@@ -321,6 +385,10 @@ bool wm_has_incomplete_imports(const struct wm *wm) {
 	return !dynarr_is_empty(wm->incompletes);
 }
 
+bool wm_has_tree_changes(const struct wm *wm) {
+	return !list_is_empty(&wm->tree.changes);
+}
+
 struct wm_change wm_dequeue_change(struct wm *wm) {
 	auto tree_change = wm_tree_dequeue_change(&wm->tree);
 	struct wm_change ret = {