From 5077d566765476da7f60f78d61931034993c9ea1 Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Thu, 4 Apr 2019 09:11:27 +0100 Subject: [PATCH] Refactor linked-list operations into a header Signed-off-by: Yuxuan Shui --- src/common.h | 5 +-- src/compton.c | 82 ++++++++++++++++++------------------------ src/dbus.c | 8 +++-- src/list.h | 99 +++++++++++++++++++++++++++++++++++++++++++++++++++ src/opengl.c | 2 +- src/win.c | 37 +++++++------------ src/win.h | 9 ++--- 7 files changed, 155 insertions(+), 87 deletions(-) create mode 100644 src/list.h diff --git a/src/common.h b/src/common.h index 6a69e36..fa3bc4b 100644 --- a/src/common.h +++ b/src/common.h @@ -393,10 +393,7 @@ typedef struct session { /// A hash table of all windows. win *windows; /// Windows in their stacking order - win *window_stack; - /// Pointer to the `next` field of the bottom most window, - /// or a pointer to `window_stack` when there is no window - win **window_stack_bottom; + struct list_node window_stack; /// Pointer to win of current active window. Used by /// EWMH _NET_ACTIVE_WINDOW focus detection. In theory, /// it's more reliable to store the window ID directly here, just in diff --git a/src/compton.c b/src/compton.c index 9fc4a77..5983544 100644 --- a/src/compton.c +++ b/src/compton.c @@ -52,6 +52,7 @@ #include "event.h" #include "options.h" #include "uthash_extra.h" +#include "list.h" /// Get session_t pointer from a pointer to a member of session_t #define session_ptr(ptr, member) \ @@ -437,7 +438,7 @@ static win *paint_preprocess(session_t *ps, bool *fade_running) { ps->fade_time += steps * ps->o.fade_delta; // First, let's process fading - WIN_STACK_ITER(ps, w) { + list_foreach_safe(win, w, &ps->window_stack, stack_neighbour) { const winmode_t mode_old = w->mode; const bool was_painted = w->to_paint; const double opacity_old = w->opacity; @@ -490,7 +491,7 @@ static win *paint_preprocess(session_t *ps, bool *fade_running) { // Track whether it's the highest window to paint bool is_highest = true; bool reg_ignore_valid = true; - WIN_STACK_ITER(ps, w) { + list_foreach(win, w, &ps->window_stack, stack_neighbour) { __label__ skip_window; bool to_paint = true; // w->to_paint remembers whether this window is painted last time @@ -650,12 +651,10 @@ static void rebuild_shadow_exclude_reg(session_t *ps) { static void restack_win(session_t *ps, win *w, xcb_window_t new_above) { xcb_window_t old_above; - if (w->next) { - old_above = w->next->id; - assert(&w->next != ps->window_stack_bottom); + if (!list_node_is_last(&ps->window_stack, &w->stack_neighbour)) { + old_above = list_next_entry(w, stack_neighbour)->id; } else { old_above = XCB_NONE; - assert(&w->next == ps->window_stack_bottom); } log_debug("Restack %#010x (%s), old_above: %#010x, new_above: %#010x", w->id, w->name, old_above, new_above); @@ -663,45 +662,31 @@ static void restack_win(session_t *ps, win *w, xcb_window_t new_above) { if (old_above != new_above) { w->reg_ignore_valid = false; rc_region_unref(&w->reg_ignore); - if (w->next) { - w->next->reg_ignore_valid = false; - rc_region_unref(&w->next->reg_ignore); - } - - win **prev = NULL, *tmp_w; - HASH_FIND_INT(ps->windows, &new_above, tmp_w); - - if (new_above && !tmp_w) { - log_error("(%#010x, %#010x): Failed to found new above window.", - w->id, new_above); - return; - } - - // Unlink from old position - *w->prev = w->next; - if (w->next) { - w->next->prev = w->prev; - } - if (ps->window_stack_bottom == &w->next) { - ps->window_stack_bottom = w->prev; + if (!list_node_is_last(&ps->window_stack, &w->stack_neighbour)) { + auto next_w = list_next_entry(w, stack_neighbour); + next_w->reg_ignore_valid = false; + rc_region_unref(&next_w->reg_ignore); } + struct list_node *new_next; if (!new_above) { - *ps->window_stack_bottom = w; - w->prev = ps->window_stack_bottom; - ps->window_stack_bottom = &w->next; - w->next = NULL; + new_next = &ps->window_stack; } else { - prev = tmp_w->prev; - // Link to new position - w->next = *prev; - if (w->next) { - w->next->prev = &w->next; + win *tmp_w = NULL; + HASH_FIND_INT(ps->windows, &new_above, tmp_w); + + if (!tmp_w) { + log_error("(%#010x, %#010x): Failed to found new above " + "window.", + w->id, new_above); + return; } - w->prev = prev; - *w->prev = w; + + new_next = &tmp_w->stack_neighbour; } + list_move_before(&w->stack_neighbour, new_next); + // add damage for this window add_damage_from_win(ps, w); @@ -720,7 +705,7 @@ static void restack_win(session_t *ps, win *w, xcb_window_t new_above) { /// Free up all the images and deinit the backend static void destroy_backend(session_t *ps) { - WIN_STACK_ITER(ps, w) { + list_foreach_safe(win, w, &ps->window_stack, stack_neighbour) { // Wrapping up fading in progress win_skip_fading(ps, &w); @@ -808,8 +793,11 @@ void configure_root(session_t *ps, int width, int height) { ps->damage = ps->damage_ring + ps->ndamage - 1; // Invalidate reg_ignore from the top - rc_region_unref(&ps->window_stack->reg_ignore); - ps->window_stack->reg_ignore_valid = false; + if (!list_is_empty(&ps->window_stack)) { + auto top_w = list_entry(ps->window_stack.next, win, stack_neighbour); + rc_region_unref(&top_w->reg_ignore); + top_w->reg_ignore_valid = false; + } #ifdef CONFIG_OPENGL // GLX root change callback @@ -904,7 +892,7 @@ void circulate_win(session_t *ps, xcb_circulate_notify_event_t *ce) { return; if (ce->place == PlaceOnTop) { - new_above = ps->window_stack->id; + new_above = list_entry(ps->window_stack.next, win, stack_neighbour)->id; } else { new_above = XCB_NONE; } @@ -1752,7 +1740,6 @@ static session_t *session_init(int argc, char **argv, Display *dpy, #ifdef CONFIG_DBUS .dbus_data = NULL, #endif - .window_stack = NULL, }; auto stderr_logger = stderr_logger_new(); @@ -1765,7 +1752,7 @@ static session_t *session_init(int argc, char **argv, Display *dpy, // Allocate a session and copy default values into it session_t *ps = cmalloc(session_t); *ps = s_def; - ps->window_stack_bottom = &ps->window_stack; + list_init_head(&ps->window_stack); ps->loop = EV_DEFAULT; pixman_region32_init(&ps->screen_reg); @@ -2144,7 +2131,7 @@ static session_t *session_init(int argc, char **argv, Display *dpy, free(reply); log_trace("Initial stack:"); - WIN_STACK_ITER(ps, w) { + list_foreach(win, w, &ps->window_stack, stack_neighbour) { log_trace("%#010x \"%s\"", w->id, w->name); } } @@ -2195,7 +2182,7 @@ static void session_destroy(session_t *ps) { // Free window linked list - WIN_STACK_ITER(ps, w) { + list_foreach_safe(win, w, &ps->window_stack, stack_neighbour) { if (w->state != WSTATE_DESTROYING) { win_ev_stop(ps, w); HASH_DEL(ps->windows, w); @@ -2204,8 +2191,7 @@ static void session_destroy(session_t *ps) { free_win_res(ps, w); free(w); } - ps->window_stack = NULL; - ps->window_stack_bottom = &ps->window_stack; + list_init_head(&ps->window_stack); // Free blacklists free_wincondlst(&ps->o.shadow_blacklist); diff --git a/src/dbus.c b/src/dbus.c index 18f74cb..b60934f 100644 --- a/src/dbus.c +++ b/src/dbus.c @@ -24,9 +24,10 @@ #include "log.h" #include "string_utils.h" #include "types.h" +#include "uthash_extra.h" #include "utils.h" #include "win.h" -#include "uthash_extra.h" +#include "list.h" #include "dbus.h" @@ -760,7 +761,10 @@ static bool cdbus_process_win_get(session_t *ps, DBusMessage *msg) { // next if (!strcmp("next", target)) { - cdbus_reply_wid(ps, msg, (w->next ? w->next->id : 0)); + cdbus_reply_wid(ps, msg, + (list_node_is_last(&ps->window_stack, &w->stack_neighbour) + ? 0 + : list_next_entry(w, stack_neighbour)->id)); return true; } diff --git a/src/list.h b/src/list.h new file mode 100644 index 0000000..69426ac --- /dev/null +++ b/src/list.h @@ -0,0 +1,99 @@ +#pragma once +#include +#include + +/** + * container_of - cast a member of a structure out to the containing structure + * @ptr: the pointer to the member. + * @type: the type of the container struct this is embedded in. + * @member: the name of the member within the struct. + * + */ +#define container_of(ptr, type, member) \ + ({ \ + const __typeof__(((type *)0)->member) *__mptr = (ptr); \ + (type *)((char *)__mptr - offsetof(type, member)); \ + }) + +struct list_node { + struct list_node *next, *prev; +}; + +#define list_entry(ptr, type, node) container_of(ptr, type, node) +#define list_next_entry(ptr, node) list_entry((ptr)->node.next, __typeof__(*(ptr)), node) +#define list_prev_entry(ptr, node) list_entry((ptr)->node.prev, __typeof__(*(ptr)), node) + +/// Insert a new node between two adjacent nodes in the list +static inline void +__list_insert_between(struct list_node *restrict prev, struct list_node *restrict next, + struct list_node *restrict new_) { + new_->prev = prev; + new_->next = next; + next->prev = new_; + prev->next = new_; +} + +/// Insert a new node after `curr` +static inline void +list_insert_after(struct list_node *restrict curr, struct list_node *restrict new_) { + __list_insert_between(curr, curr->next, new_); +} + +/// Insert a new node before `curr` +static inline void +list_insert_before(struct list_node *restrict curr, struct list_node *restrict new_) { + __list_insert_between(curr->prev, curr, new_); +} + +/// Link two nodes in the list, so `next` becomes the successor node of `prev` +static inline void __list_link(struct list_node *prev, struct list_node *next) { + next->prev = prev; + prev->next = next; +} + +/// Remove a node from the list +static inline void list_remove(struct list_node *to_remove) { + __list_link(to_remove->prev, to_remove->next); + to_remove->prev = (void *)-1; + to_remove->next = (void *)-2; +} + +/// Move `to_move` so that it's before `new_next` +static inline void +list_move_before(struct list_node *restrict to_move, struct list_node *restrict new_next) { + list_remove(to_move); + list_insert_before(new_next, to_move); +} + +/// Initialize a list node that's intended to be the head node +static inline void list_init_head(struct list_node *head) { + head->next = head->prev = head; +} + +/// Return true if head is the only node in the list. Under usual circumstances this means +/// the list is empty +static inline bool list_is_empty(const struct list_node *head) { + return head->prev == head; +} + +/// Return true if `to_check` is the first node in list headed by `head` +static inline bool +list_node_is_first(const struct list_node *head, const struct list_node *to_check) { + return head->next == to_check; +} + +/// Return true if `to_check` is the last node in list headed by `head` +static inline bool +list_node_is_last(const struct list_node *head, const struct list_node *to_check) { + return head->prev == to_check; +} + +#define list_foreach(type, i, head, member) \ + for (type *i = list_entry((head)->next, type, member); &i->member != (head); \ + i = list_next_entry(i, member)) + +/// Like list_for_each, but it's safe to remove the current list node from the list +#define list_foreach_safe(type, i, head, member) \ + for (type *i = list_entry((head)->next, type, member), \ + *__tmp = list_next_entry(i, member); \ + &i->member != (head); i = __tmp, __tmp = list_next_entry(i, member)) diff --git a/src/opengl.c b/src/opengl.c index 08563a3..8683827 100644 --- a/src/opengl.c +++ b/src/opengl.c @@ -221,7 +221,7 @@ void glx_destroy(session_t *ps) { return; // Free all GLX resources of windows - WIN_STACK_ITER(ps, w) { + list_foreach(win, w, &ps->window_stack, stack_neighbour) { free_win_res_glx(ps, w); } diff --git a/src/win.c b/src/win.c index 0536d75..2de78c8 100644 --- a/src/win.c +++ b/src/win.c @@ -28,6 +28,7 @@ #include "uthash_extra.h" #include "utils.h" #include "x.h" +#include "list.h" #ifdef CONFIG_DBUS #include "dbus.h" @@ -57,7 +58,7 @@ * Clear leader cache of all windows. */ static inline void clear_cache_win_leaders(session_t *ps) { - WIN_STACK_ITER(ps, w) { + list_foreach(win, w, &ps->window_stack, stack_neighbour) { w->cache_leader = XCB_NONE; } } @@ -918,7 +919,6 @@ void add_win(session_t *ps, xcb_window_t id, xcb_window_t prev) { .invert_color_force = UNSET, // Initialized in this function - .next = NULL, .id = XCB_NONE, .a = {0}, .pictfmt = NULL, @@ -1025,24 +1025,16 @@ void add_win(session_t *ps, xcb_window_t id, xcb_window_t prev) { } // Find window insertion point - win **p = NULL; + struct list_node *p = NULL; if (prev) { win *w = NULL; HASH_FIND_INT(ps->windows, &prev, w); assert(w); - p = w->prev; + p = w->stack_neighbour.prev; } else { p = &ps->window_stack; } - new->next = *p; - if (new->next) { - new->next->prev = &new->next; - } - new->prev = p; - *p = new; - if (p == ps->window_stack_bottom) { - ps->window_stack_bottom = &new->next; - } + list_insert_after(p, &new->stack_neighbour); HASH_ADD_INT(ps->windows, id, new); #ifdef CONFIG_DBUS @@ -1427,7 +1419,7 @@ void win_update_frame_extents(session_t *ps, win *w, xcb_window_t client) { } bool win_is_region_ignore_valid(session_t *ps, const win *w) { - WIN_STACK_ITER(ps, i) { + list_foreach(win, i, &ps->window_stack, stack_neighbour) { if (i == w) break; if (!i->reg_ignore_valid) @@ -1487,19 +1479,14 @@ static void finish_destroy_win(session_t *ps, win **_w) { // 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 (w->next) { - rc_region_unref(&w->next->reg_ignore); - w->next->reg_ignore_valid = false; + if (!list_node_is_last(&ps->window_stack, &w->stack_neighbour)) { + auto next_w = list_next_entry(w, stack_neighbour); + rc_region_unref(&next_w->reg_ignore); + next_w->reg_ignore_valid = false; } log_trace("Trying to destroy (%#010x)", w->id); - *w->prev = w->next; - if (w->next) { - w->next->prev = w->prev; - } - if (&w->next == ps->window_stack_bottom) { - ps->window_stack_bottom = w->prev; - } + list_remove(&w->stack_neighbour); if (w == ps->active_win) { ps->active_win = NULL; @@ -1510,7 +1497,7 @@ static void finish_destroy_win(session_t *ps, win **_w) { // 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_ITER(ps, w2) { + list_foreach(win, w2, &ps->window_stack, stack_neighbour) { if (w == w2->prev_trans) { w2->prev_trans = NULL; } diff --git a/src/win.h b/src/win.h index 3f74f07..8494c3a 100644 --- a/src/win.h +++ b/src/win.h @@ -22,9 +22,7 @@ #include "types.h" #include "utils.h" #include "x.h" - -#define WIN_STACK_ITER(ps, w) \ - for (win *w = ps->window_stack, *next; w ? (next = w->next, true) : false; w = next) +#include "list.h" typedef struct session session_t; typedef struct _glx_texture glx_texture_t; @@ -131,14 +129,11 @@ typedef enum win_flags { /// Structure representing a top-level window compton manages. typedef struct win win; struct win { + struct list_node stack_neighbour; /// backend data attached to this window. Only available when /// `state` is not UNMAPPED void *win_image; void *shadow_image; - /// Pointer to the next lower window in window stack. - win *next; - /// Pointer to a linked-list pointer that points to this window. - win **prev; /// Pointer to the next higher window to paint. win *prev_trans; // TODO rethink reg_ignore