Skip to content

Commit

Permalink
wm/tree: return less information from tree changes
Browse files Browse the repository at this point in the history 
Turns out we don't really need all that.

Signed-off-by: Yuxuan Shui <yshuiv7@gmail.com>
  • Loading branch information
@yshui
yshui committed Jun 6, 2024
1 parent 042b271 commit 6d4b4cf
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 102 deletions.
9 changes: 2 additions & 7 deletions src/picom.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1603,20 +1603,15 @@ static void handle_new_windows(session_t *ps) {
log_debug("Client window for window %#010x changed from "
"%#010x to %#010x",
wm_ref_win_id(wm_change.toplevel),
wm_change.client.old != NULL
? wm_ref_win_id(wm_change.client.old)
: 0,
wm_change.client.new_ != NULL
? wm_ref_win_id(wm_change.client.new_)
: 0);
wm_change.client.old.x, wm_change.client.new_.x);
w = wm_ref_deref(wm_change.toplevel);
if (w != NULL) {
win_on_client_update(ps, w);
} else {
log_debug("An unmanaged window %#010x has a new client "
"%#010x",
wm_ref_win_id(wm_change.toplevel),
wm_ref_win_id(wm_change.client.new_));
wm_change.client.new_.x);
}
break;
case WM_TREE_CHANGE_TOPLEVEL_RESTACKED: invalidate_reg_ignore(ps); break;
Expand Down
89 changes: 40 additions & 49 deletions src/wm/tree.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,11 @@ static void wm_tree_enqueue_change(struct wm_tree *tree, struct wm_tree_change c
bool found = false;
list_foreach_safe(struct wm_tree_change_list, i, &tree->changes, siblings) {
if (i->item.type == WM_TREE_CHANGE_TOPLEVEL_NEW &&
wm_treeid_eq(i->item.toplevel, change.toplevel)) {
i->item.toplevel == change.toplevel) {
list_remove(&i->siblings);
list_insert_after(&tree->free_changes, &i->siblings);
found = true;
} else if (wm_treeid_eq(i->item.toplevel, change.toplevel) && found) {
} else if (i->item.toplevel == change.toplevel && found) {
// We also need to delete all other changes related to
// this toplevel in between the new and gone changes.
list_remove(&i->siblings);
Expand All @@ -59,26 +59,21 @@ static void wm_tree_enqueue_change(struct wm_tree *tree, struct wm_tree_change c
} else if (change.type == WM_TREE_CHANGE_CLIENT) {
// A client change can coalesce with a previous client change.
list_foreach_safe(struct wm_tree_change_list, i, &tree->changes, siblings) {
if (!wm_treeid_eq(i->item.toplevel, change.toplevel) ||
if (i->item.toplevel != change.toplevel ||
i->item.type != WM_TREE_CHANGE_CLIENT) {
continue;
}

auto old_new_id =
i->item.client.new_ ? i->item.client.new_->id : WM_TREEID_NONE;
if (!wm_treeid_eq(old_new_id, change.client.old_id)) {
if (!wm_treeid_eq(i->item.client.new_, change.client.old)) {
log_warn("Inconsistent client change for toplevel "
"%#010x. Missing changes from %#010x to %#010x. "
"Possible bug.",
change.toplevel.x, old_new_id.x,
change.client.old->id.x);
change.toplevel->id.x, i->item.client.new_.x,
change.client.old.x);
}

i->item.client.new_ = change.client.new_;

auto new_new_id =
change.client.new_ ? change.client.new_->id : WM_TREEID_NONE;
if (wm_treeid_eq(i->item.client.old_id, new_new_id)) {
if (wm_treeid_eq(i->item.client.old, change.client.new_)) {
list_remove(&i->siblings);
list_insert_after(&tree->free_changes, &i->siblings);
}
Expand Down Expand Up @@ -191,8 +186,7 @@ struct wm_tree_node *wm_tree_find_toplevel_for(struct wm_tree_node *node) {
/// Change whether a tree node has the `WM_STATE` property set.
/// `destroyed` indicate whether `node` is about to be destroyed, in which case, the `old`
/// field of the change event will be set to NULL.
static void wm_tree_set_wm_state_impl(struct wm_tree *tree, struct wm_tree_node *node,
bool has_wm_state, bool destroyed) {
void wm_tree_set_wm_state(struct wm_tree *tree, struct wm_tree_node *node, bool has_wm_state) {
BUG_ON(node == NULL);

if (node->has_wm_state == has_wm_state) {
Expand All @@ -217,12 +211,15 @@ static void wm_tree_set_wm_state_impl(struct wm_tree *tree, struct wm_tree_node
if (!has_wm_state && toplevel->client_window == node) {
auto new_client = wm_tree_find_client(toplevel);
toplevel->client_window = new_client;

auto new_id = new_client != NULL ? new_client->id : WM_TREEID_NONE;
wm_tree_enqueue_change(tree, (struct wm_tree_change){
.toplevel = toplevel->id,
.client = {.old = destroyed ? NULL : node,
.old_id = node->id,
.new_ = new_client,
.toplevel = toplevel},
.toplevel = toplevel,
.client =
{
.old = node->id,
.new_ = new_id,
},
.type = WM_TREE_CHANGE_CLIENT,
});
} else if (has_wm_state) {
Expand All @@ -231,11 +228,12 @@ static void wm_tree_set_wm_state_impl(struct wm_tree *tree, struct wm_tree_node
// try to usurp it.
toplevel->client_window = node;
wm_tree_enqueue_change(tree, (struct wm_tree_change){
.toplevel = toplevel->id,
.client = {.old = NULL,
.old_id = WM_TREEID_NONE,
.new_ = node,
.toplevel = toplevel},
.toplevel = toplevel,
.client =
{
.old = WM_TREEID_NONE,
.new_ = node->id,
},
.type = WM_TREE_CHANGE_CLIENT,
});
} else {
Expand All @@ -247,10 +245,6 @@ static void wm_tree_set_wm_state_impl(struct wm_tree *tree, struct wm_tree_node
}
}

void wm_tree_set_wm_state(struct wm_tree *tree, struct wm_tree_node *node, bool has_wm_state) {
wm_tree_set_wm_state_impl(tree, node, has_wm_state, false);
}

struct wm_tree_node *
wm_tree_new_window(struct wm_tree *tree, xcb_window_t id, struct wm_tree_node *parent) {
auto node = ccalloc(1, struct wm_tree_node);
Expand All @@ -269,9 +263,8 @@ wm_tree_new_window(struct wm_tree *tree, xcb_window_t id, struct wm_tree_node *p
if (parent->parent == NULL) {
// Parent is root, this is a new toplevel window
wm_tree_enqueue_change(tree, (struct wm_tree_change){
.toplevel = node->id,
.toplevel = node,
.type = WM_TREE_CHANGE_TOPLEVEL_NEW,
.new_ = node,
});
}
}
Expand All @@ -283,7 +276,7 @@ void wm_tree_destroy_window(struct wm_tree *tree, struct wm_tree_node *node) {
BUG_ON(node->parent == NULL); // Trying to destroy the root window?!

if (node->has_wm_state) {
wm_tree_set_wm_state_impl(tree, node, false, true);
wm_tree_set_wm_state(tree, node, false);
}

HASH_DEL(tree->nodes, node);
Expand All @@ -301,7 +294,7 @@ void wm_tree_destroy_window(struct wm_tree *tree, struct wm_tree_node *node) {
if (node->parent->parent == NULL) {
node->is_zombie = true;
wm_tree_enqueue_change(tree, (struct wm_tree_change){
.toplevel = node->id,
.toplevel = node,
.type = WM_TREE_CHANGE_TOPLEVEL_KILLED,
.killed = node,
});
Expand Down Expand Up @@ -385,7 +378,7 @@ void wm_tree_reparent(struct wm_tree *tree, struct wm_tree_node *node,
list_replace(&node->siblings, &zombie->siblings);

wm_tree_enqueue_change(tree, (struct wm_tree_change){
.toplevel = node->id,
.toplevel = node,
.type = WM_TREE_CHANGE_TOPLEVEL_KILLED,
.killed = zombie,
});
Expand All @@ -400,9 +393,8 @@ void wm_tree_reparent(struct wm_tree *tree, struct wm_tree_node *node,

if (new_parent->parent == NULL) {
wm_tree_enqueue_change(tree, (struct wm_tree_change){
.toplevel = node->id,
.toplevel = node,
.type = WM_TREE_CHANGE_TOPLEVEL_NEW,
.new_ = node,
});
}

Expand Down Expand Up @@ -445,16 +437,16 @@ TEST_CASE(tree_manipulation) {
assert(node2->parent == root);

change = wm_tree_dequeue_change(&tree);
assert(change.toplevel.x == 2);
assert(change.toplevel->id.x == 2);
assert(change.type == WM_TREE_CHANGE_TOPLEVEL_NEW);
assert(node2 == change.new_);
assert(node2 == change.toplevel);

wm_tree_new_window(&tree, 3, root);
auto node3 = wm_tree_find(&tree, 3);
assert(node3 != NULL);

change = wm_tree_dequeue_change(&tree);
assert(change.toplevel.x == 3);
assert(change.toplevel->id.x == 3);
assert(change.type == WM_TREE_CHANGE_TOPLEVEL_NEW);

wm_tree_reparent(&tree, node2, node3);
Expand All @@ -463,16 +455,16 @@ TEST_CASE(tree_manipulation) {

// node2 is now a child of node3, so it's no longer a toplevel
change = wm_tree_dequeue_change(&tree);
assert(change.toplevel.x == 2);
assert(change.toplevel->id.x == 2);
assert(change.type == WM_TREE_CHANGE_TOPLEVEL_KILLED);
wm_tree_reap_zombie(change.killed);

wm_tree_set_wm_state(&tree, node2, true);
change = wm_tree_dequeue_change(&tree);
assert(change.toplevel.x == 3);
assert(change.toplevel->id.x == 3);
assert(change.type == WM_TREE_CHANGE_CLIENT);
assert(change.client.old == NULL);
assert(change.client.new_->id.x == 2);
assert(wm_treeid_eq(change.client.old, WM_TREEID_NONE));
assert(change.client.new_.x == 2);

wm_tree_new_window(&tree, 4, node3);
auto node4 = wm_tree_find(&tree, 4);
Expand All @@ -486,22 +478,21 @@ TEST_CASE(tree_manipulation) {

wm_tree_destroy_window(&tree, node2);
change = wm_tree_dequeue_change(&tree);
assert(change.toplevel.x == 3);
assert(change.toplevel->id.x == 3);
assert(change.type == WM_TREE_CHANGE_CLIENT);
assert(change.client.old_id.x == 2);
assert(change.client.new_->id.x == 4);
assert(change.client.old.x == 2);
assert(change.client.new_.x == 4);

// Test window ID reuse
wm_tree_destroy_window(&tree, node4);
node4 = wm_tree_new_window(&tree, 4, node3);
wm_tree_set_wm_state(&tree, node4, true);

change = wm_tree_dequeue_change(&tree);
assert(change.toplevel.x == 3);
assert(change.toplevel->id.x == 3);
assert(change.type == WM_TREE_CHANGE_CLIENT);
assert(change.client.old_id.x == 4);
assert(change.client.old == NULL);
assert(change.client.new_ == node4);
assert(change.client.old.x == 4);
assert(change.client.new_.x == 4);

auto node5 = wm_tree_new_window(&tree, 5, root);
wm_tree_destroy_window(&tree, node5);
Expand Down
26 changes: 7 additions & 19 deletions src/wm/wm.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -300,29 +300,17 @@ bool wm_has_incomplete_imports(const struct wm *wm) {

struct wm_change wm_dequeue_change(struct wm *wm) {
auto tree_change = wm_tree_dequeue_change(&wm->tree);

struct wm_change ret = {
.type = tree_change.type,
.toplevel = tree_change.toplevel != NULL
? (struct wm_ref *)&tree_change.toplevel->siblings
: NULL,
};
switch (tree_change.type) {
case WM_TREE_CHANGE_CLIENT:
ret.toplevel = (struct wm_ref *)&tree_change.client.toplevel->siblings;
ret.client.old = tree_change.client.old
? (struct wm_ref *)&tree_change.client.old->siblings
: NULL;
ret.client.new_ = tree_change.client.new_
? (struct wm_ref *)&tree_change.client.new_->siblings
: NULL;
break;
case WM_TREE_CHANGE_TOPLEVEL_KILLED:
if (tree_change.type == WM_TREE_CHANGE_CLIENT) {
ret.client.old = tree_change.client.old;
ret.client.new_ = tree_change.client.new_;
} else if (tree_change.type == WM_TREE_CHANGE_TOPLEVEL_KILLED) {
ret.toplevel = (struct wm_ref *)&tree_change.killed->siblings;
break;
case WM_TREE_CHANGE_TOPLEVEL_NEW:
ret.toplevel = (struct wm_ref *)&tree_change.new_->siblings;
break;
case WM_TREE_CHANGE_TOPLEVEL_RESTACKED: ret.toplevel = NULL; break;
case WM_TREE_CHANGE_NONE: break;
default: unreachable();
}
return ret;
}
30 changes: 16 additions & 14 deletions src/wm/wm.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,20 +60,6 @@ enum wm_tree_change_type {
WM_TREE_CHANGE_NONE,
};

struct wm_change {
enum wm_tree_change_type type;
struct wm_ref *toplevel;
union {
struct {
struct wm_ref *old, *new_;
} client;
};
};

/// Reference to a window in the `struct wm`. Most of wm operations operate on wm_refs.
struct wm_ref;
struct atom;

typedef struct wm_treeid {
/// The generation of the window ID. This is used to detect if the window ID is
/// reused. Inherited from the wm_tree at cr
Expand All @@ -90,6 +76,22 @@ static const wm_treeid WM_TREEID_NONE = {.gen = 0, .x = XCB_NONE};
static_assert(sizeof(wm_treeid) == 16, "wm_treeid size is not 16 bytes");
static_assert(alignof(wm_treeid) == 8, "wm_treeid alignment is not 8 bytes");

struct wm_change {
enum wm_tree_change_type type;
/// The toplevel window this change is about. For
/// `WM_TREE_CHANGE_TOPLEVEL_KILLED`, this is the zombie window left in place of
/// the killed toplevel. For `WM_TREE_CHANGE_TOPLEVEL_RESTACKED`, this is NULL.
struct wm_ref *toplevel;
struct {
wm_treeid old;
wm_treeid new_;
} client;
};

/// Reference to a window in the `struct wm`. Most of wm operations operate on wm_refs.
struct wm_ref;
struct atom;

static inline bool wm_treeid_eq(wm_treeid a, wm_treeid b) {
return a.gen == b.gen && a.x == b.x;
}
Expand Down
17 changes: 4 additions & 13 deletions src/wm/wm_internal.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,24 +56,15 @@ struct wm_tree_node {
/// window before the change, and if `new_client` is `XCB_NONE`, it means the toplevel
/// window lost its client window after the change.
struct wm_tree_change {
wm_treeid toplevel;
struct wm_tree_node *toplevel;
union {
/// Information for `WM_TREE_CHANGE_CLIENT`.
struct {
/// The toplevel whose client changed.
struct wm_tree_node *toplevel;
/// The old and new client windows. If the old window is destroyed
/// `old` will be NULL.
struct wm_tree_node *old, *new_;
/// The window ID of the old client window. This is set regardless
/// if the old client window is destroyed.
wm_treeid old_id;
/// The old and new client windows.
wm_treeid old, new_;
} client;
/// Information for `WM_TREE_CHANGE_TOPLEVEL_NEW`.
/// The new toplevel window.
struct wm_tree_node *new_;
/// Information for `WM_TREE_CHANGE_TOPLEVEL_KILLED`.
/// The zombie toplevel window.
/// The zombie window left in place of the killed toplevel.
struct wm_tree_node *killed;
};

Expand Down

0 comments on commit 6d4b4cf

Please sign in to comment.