From 3397077e961e27dd536842a47e8a80267b1cc0d2 Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Thu, 13 Jun 2024 07:46:10 +0100 Subject: [PATCH] wip --- meson.build | 2 +- src/event.c | 37 +++++++------- src/picom.c | 3 +- src/wm/tree.c | 14 +++-- src/wm/wm.c | 138 +++++++++++++++++++++++++++++++++++++------------- src/wm/wm.h | 35 ++++++++++--- 6 files changed, 164 insertions(+), 65 deletions(-) diff --git a/meson.build b/meson.build index 6bdd4eda57..8bf00a5467 100644 --- a/meson.build +++ b/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 diff --git a/src/event.c b/src/event.c index e56607ebed..a98eccd306 100644 --- a/src/event.c +++ b/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) { diff --git a/src/picom.c b/src/picom.c index c9c30854e4..e194e9151b 100644 --- a/src/picom.c +++ b/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)) { diff --git a/src/wm/tree.c b/src/wm/tree.c index 192bcddc44..cf2fd4bcd7 100644 --- a/src/wm/tree.c +++ b/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); diff --git a/src/wm/wm.c b/src/wm/wm.c index c741c87b1c..1dca6ada0f 100644 --- a/src/wm/wm.c +++ b/src/wm/wm.c @@ -1,6 +1,7 @@ // SPDX-License-Identifier: MPL-2.0 // Copyright (c) Yuxuan Shui +#include #include #include @@ -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,8 +241,37 @@ 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) { @@ -215,10 +279,22 @@ void wm_set_has_wm_state(struct wm *wm, struct wm_ref *cursor, bool 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 = { diff --git a/src/wm/wm.h b/src/wm/wm.h index 7947fb1b1a..6f112f2234 100644 --- a/src/wm/wm.h +++ b/src/wm/wm.h @@ -88,7 +88,8 @@ struct wm_change { } client; }; -/// Reference to a window in the `struct wm`. Most of wm operations operate on wm_refs. +/// Reference to a window in the `struct wm`. Most of wm operations operate on wm_refs. If +/// the referenced window is managed, a `struct window` can be retrieved by `wm_ref_deref`. struct wm_ref; struct atom; @@ -149,7 +150,7 @@ bool attr_pure wm_ref_is_zombie(const struct wm_ref *cursor); void wm_destroy(struct wm *wm, xcb_window_t wid); /// Remove a zombie window from the window tree. void wm_reap_zombie(struct wm_ref *zombie); -void wm_reparent(struct wm *wm, struct wm_ref *cursor, struct wm_ref *new_parent); +void wm_reparent(struct wm *wm, xcb_window_t wid, xcb_window_t parent); void wm_set_has_wm_state(struct wm *wm, struct wm_ref *cursor, bool has_wm_state); /// Create a tree node for `wid`, with `parent` as its parent. The parent must already @@ -158,6 +159,8 @@ void wm_set_has_wm_state(struct wm *wm, struct wm_ref *cursor, bool has_wm_state /// expectation is that the caller will later call `wm_complete_import` inside the /// X critical section to complete the import. /// +/// ## NOTE +/// /// The reason for this complicated dance is because we want to catch all windows ever /// created on X server's side. For a newly created windows, we will setup a /// SubstructureNotify event mask to catch any new windows created under it. But between @@ -165,10 +168,24 @@ void wm_set_has_wm_state(struct wm *wm, struct wm_ref *cursor, bool has_wm_state /// windows were created under the new window, we will miss them. Therefore we have to /// scan the new windows in X critical section so they won't change as we scan. /// -/// On the other hand, we can't push everything to the X critical section, because there -/// can be subsequent events that relate to the new windows, if we don't create something -/// in the tree immediately, we can't handle those events. That's why this has to be -/// broken into two steps. +/// On the other hand, we can't push everything to the X critical section, because updating +/// the window stack requires knowledge of all windows in the stack. Think ConfigureNotify, +/// if we don't know about the `above_sibling` window, we don't know where to put the window. +/// So we need to create an incomplete import first. +/// +/// But wait, this is actually way more involved. Because we choose to not set up event masks +/// for incomplete imports (we can also choose otherwise, but that also has its own set of +/// problems), there is a whole subtree of windows we don't know about. And those windows +/// might involve in reparent events. To handle that, we essentially put "fog of war" under +/// any incomplete imports, anything reparented into the fog is lost, and will be rediscovered +/// later during subtree scans. If a window is reparented out of the fog, then it's treated as +/// if a brand new window was created. +/// +/// But wait again, there's more. We can delete "lost" windows on our side and unset event masks, +/// but again because this is racy, we might still receive some events for those windows. So we +/// have to keep a list of "lost" windows, and correctly ignore events sent for them. (And since +/// we are actively ignoring events from these windows, we might as well not unset their event +/// masks, saves us a trip to the X server). /// /// (Now you have a glimpse of how much X11 sucks.) void wm_import_incomplete(struct wm *wm, xcb_window_t wid, xcb_window_t parent); @@ -176,6 +193,9 @@ void wm_import_incomplete(struct wm *wm, xcb_window_t wid, xcb_window_t parent); /// Check if there are any incomplete imports in the window tree. bool wm_has_incomplete_imports(const struct wm *wm); +/// Check if there are tree change events +bool wm_has_tree_changes(const struct wm *wm); + /// Complete the previous incomplete imports by querying the X server. This function will /// recursively import all children of previously created placeholders, and add them to /// the window tree. This function must be called from the X critical section. This @@ -183,4 +203,7 @@ bool wm_has_incomplete_imports(const struct wm *wm); /// as well as for the (now completed) placeholder windows. void wm_complete_import(struct wm *wm, struct x_connection *c, struct atom *atoms); + +bool wm_is_wid_masked(struct wm *wm, xcb_window_t wid); + struct wm_change wm_dequeue_change(struct wm *wm);