From 91e023971d06b441c5b086ffc49e148d9b0c601f Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Mon, 12 Dec 2022 08:51:42 +0000 Subject: [PATCH 01/13] utils: add rolling_max For tracking rolling max of a stream of integers. Signed-off-by: Yuxuan Shui --- src/utils.c | 120 +++++++++++++++++++++++++++++++++++++++++++++++++++- src/utils.h | 8 ++++ 2 files changed, 126 insertions(+), 2 deletions(-) diff --git a/src/utils.c b/src/utils.c index 8a27f39..8190226 100644 --- a/src/utils.c +++ b/src/utils.c @@ -4,6 +4,7 @@ #include "compiler.h" #include "string_utils.h" +#include "test.h" #include "utils.h" /// Report allocation failure without allocating memory @@ -36,8 +37,7 @@ void report_allocation_failure(const char *func, const char *file, unsigned int /// Calculates next closest power of two of 32bit integer n /// ref: https://graphics.stanford.edu/~seander/bithacks.html#RoundUpPowerOf2 /// -int next_power_of_two(int n) -{ +int next_power_of_two(int n) { n--; n |= n >> 1; n |= n >> 2; @@ -48,4 +48,120 @@ int next_power_of_two(int n) return n; } +/// Track the rolling maximum of a stream of integers. +struct rolling_max { + /// A priority queue holding the indices of the maximum element candidates. + /// The head of the queue is the index of the maximum element. + /// The indices in the queue are the "original" indices. + /// + /// There are only `capacity` elements in `elem`, all previous elements are + /// discarded. But the discarded elements' indices are not forgotten, that's why + /// it's called the "original" indices. + int *p; + int p_head, np; + + /// The elemets + int *elem; + int elem_head, nelem; + + int window_size; +}; + +void rolling_max_destroy(struct rolling_max *rm) { + free(rm->elem); + free(rm->p); + free(rm); +} + +struct rolling_max *rolling_max_new(int size) { + auto rm = ccalloc(1, struct rolling_max); + if (!rm) { + return NULL; + } + + rm->p = ccalloc(size, int); + rm->elem = ccalloc(size, int); + rm->window_size = size; + if (!rm->p || !rm->elem) { + goto err; + } + + return rm; + +err: + rolling_max_destroy(rm); + return NULL; +} + +void rolling_max_reset(struct rolling_max *rm) { + rm->p_head = 0; + rm->np = 0; + rm->nelem = 0; + rm->elem_head = 0; +} + +void rolling_max_push(struct rolling_max *rm, int val) { +#define IDX(n) ((n) % rm->window_size) + if (rm->nelem == rm->window_size) { + auto old_head = rm->elem_head; + // Discard the oldest element. + // rm->elem.pop_front(); + rm->nelem--; + rm->elem_head = IDX(rm->elem_head + 1); + + // Remove discarded element from the priority queue too. + assert(rm->np); + if (rm->p[rm->p_head] == old_head) { + // rm->p.pop_front() + rm->p_head = IDX(rm->p_head + 1); + rm->np--; + } + } + + // Add the new element to the queue. + // rm->elem.push_back(val) + rm->elem[IDX(rm->elem_head + rm->nelem)] = val; + rm->nelem++; + + // Update the prority queue. + // Remove all elements smaller than the new element from the queue. Because + // the new element will become the maximum element before them, and since they + // come b1efore the new element, they will have been popped before the new + // element, so they will never become the maximum element. + while (rm->np) { + int p_tail = IDX(rm->p_head + rm->np - 1); + if (rm->elem[rm->p[p_tail]] > val) { + break; + } + // rm->p.pop_back() + rm->np--; + } + // Add the new element to the end of the queue. + // rm->p.push_back(rm->start_index + rm->nelem - 1) + rm->p[IDX(rm->p_head + rm->np)] = IDX(rm->elem_head + rm->nelem - 1); + rm->np++; +#undef IDX +} + +int rolling_max_get_max(struct rolling_max *rm) { + if (rm->np == 0) { + return INT_MIN; + } + return rm->elem[rm->p[rm->p_head]]; +} + +TEST_CASE(rolling_max_test) { +#define NELEM 15 + auto rm = rolling_max_new(3); + const int data[NELEM] = {1, 2, 3, 1, 4, 5, 2, 3, 6, 5, 4, 3, 2, 0, 0}; + const int expected_max[NELEM] = {1, 2, 3, 3, 4, 5, 5, 5, 6, 6, 6, 5, 4, 3, 2}; + int max[NELEM] = {0}; + for (int i = 0; i < NELEM; i++) { + rolling_max_push(rm, data[i]); + max[i] = rolling_max_get_max(rm); + } + TEST_TRUE(memcmp(max, expected_max, sizeof(max)) == 0); +#undef NELEM +} + // vim: set noet sw=8 ts=8 : diff --git a/src/utils.h b/src/utils.h index a35bfa8..a1a3d34 100644 --- a/src/utils.h +++ b/src/utils.h @@ -289,6 +289,14 @@ static inline void free_charpp(char **str) { /// int next_power_of_two(int n); +struct rolling_max; + +struct rolling_max *rolling_max_new(int window_size); +void rolling_max_free(struct rolling_max *rm); +void rolling_max_reset(struct rolling_max *rm); +void rolling_max_push(struct rolling_max *rm, int val); +int rolling_max_get_max(struct rolling_max *rm); + // Some versions of the Android libc do not have timespec_get(), use // clock_gettime() instead. #ifdef __ANDROID__ From 459416894657b9791da1205f0da57362a5d61b1f Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Tue, 13 Dec 2022 08:39:11 +0000 Subject: [PATCH 02/13] utils: add rolling_avg Signed-off-by: Yuxuan Shui --- src/utils.c | 79 +++++++++++++++++++++++++++++++++++++++++++++++++++++ src/utils.h | 7 +++++ 2 files changed, 86 insertions(+) diff --git a/src/utils.c b/src/utils.c index 8190226..c080e53 100644 --- a/src/utils.c +++ b/src/utils.c @@ -164,4 +164,83 @@ TEST_CASE(rolling_max_test) { #undef NELEM } +/// A rolling average of a stream of integers. +struct rolling_avg { + /// The sum of the elements in the window. + int64_t sum; + + /// The elements in the window. + int *elem; + int head, nelem; + + int window_size; +}; + +struct rolling_avg *rolling_avg_new(int size) { + auto rm = ccalloc(1, struct rolling_avg); + if (!rm) { + return NULL; + } + + rm->elem = ccalloc(size, int); + rm->window_size = size; + if (!rm->elem) { + free(rm); + return NULL; + } + + return rm; +} + +void rolling_avg_destroy(struct rolling_avg *rm) { + free(rm->elem); + free(rm); +} + +void rolling_avg_reset(struct rolling_avg *ra) { + ra->sum = 0; + ra->nelem = 0; + ra->head = 0; +} + +void rolling_avg_push(struct rolling_avg *ra, int val) { + if (ra->nelem == ra->window_size) { + // Discard the oldest element. + // rm->elem.pop_front(); + ra->sum -= ra->elem[ra->head % ra->window_size]; + ra->nelem--; + ra->head = (ra->head + 1) % ra->window_size; + } + + // Add the new element to the queue. + // rm->elem.push_back(val) + ra->elem[(ra->head + ra->nelem) % ra->window_size] = val; + ra->sum += val; + ra->nelem++; +} + +double rolling_avg_get_avg(struct rolling_avg *ra) { + if (ra->nelem == 0) { + return 0; + } + return (double)ra->sum / (double)ra->nelem; +} + +TEST_CASE(rolling_avg_test) { +#define NELEM 15 + auto rm = rolling_avg_new(3); + const int data[NELEM] = {1, 2, 3, 1, 4, 5, 2, 3, 6, 5, 4, 3, 2, 0, 0}; + const double expected_avg[NELEM] = { + 1, 1.5, 2, 2, 8.0 / 3.0, 10.0 / 3.0, 11.0 / 3.0, 10.0 / 3.0, + 11.0 / 3.0, 14.0 / 3.0, 5, 4, 3, 5.0 / 3.0, 2.0 / 3.0}; + double avg[NELEM] = {0}; + for (int i = 0; i < NELEM; i++) { + rolling_avg_push(rm, data[i]); + avg[i] = rolling_avg_get_avg(rm); + } + for (int i = 0; i < NELEM; i++) { + TEST_EQUAL(avg[i], expected_avg[i]); + } +} + // vim: set noet sw=8 ts=8 : diff --git a/src/utils.h b/src/utils.h index a1a3d34..5fa9219 100644 --- a/src/utils.h +++ b/src/utils.h @@ -297,6 +297,13 @@ void rolling_max_reset(struct rolling_max *rm); void rolling_max_push(struct rolling_max *rm, int val); int rolling_max_get_max(struct rolling_max *rm); +struct rolling_avg; +struct rolling_avg *rolling_avg_new(int window_size); +void rolling_avg_free(struct rolling_avg *ra); +void rolling_avg_reset(struct rolling_avg *ra); +void rolling_avg_push(struct rolling_avg *ra, int val); +double rolling_avg_get_avg(struct rolling_avg *ra); + // Some versions of the Android libc do not have timespec_get(), use // clock_gettime() instead. #ifdef __ANDROID__ From 88608027b82d3777a6eed111018013640e1f3636 Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Wed, 14 Dec 2022 07:36:07 +0000 Subject: [PATCH 03/13] backend: add a comment Signed-off-by: Yuxuan Shui --- src/backend/backend.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/backend/backend.c b/src/backend/backend.c index 032c301..83af8fb 100644 --- a/src/backend/backend.c +++ b/src/backend/backend.c @@ -145,6 +145,8 @@ void paint_all_new(session_t *ps, struct managed_win *t, bool ignore_damage) { // region will be because of blur, we assume the worst case here. // That is, the damaged window is at the bottom of the stack, and // all other windows have semi-transparent background + // + // TODO(yshui): maybe we don't need to resize reg_damage, only reg_paint? int resize_factor = 1; if (t) { resize_factor = t->stacking_rank; From e407c5c7652da7e340e7c7797723b78be7ab1ab7 Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Wed, 14 Dec 2022 07:37:27 +0000 Subject: [PATCH 04/13] x: add x_{create,destroy}_region Signed-off-by: Yuxuan Shui --- src/x.c | 32 ++++++++++++++++++++++++++++++++ src/x.h | 6 ++++++ 2 files changed, 38 insertions(+) diff --git a/src/x.c b/src/x.c index 795211d..7faa1c9 100644 --- a/src/x.c +++ b/src/x.c @@ -388,6 +388,38 @@ bool x_fetch_region(xcb_connection_t *c, xcb_xfixes_region_t r, pixman_region32_ return ret; } +uint32_t x_create_region(xcb_connection_t *c, const region_t *reg) { + if (!reg) { + return XCB_NONE; + } + + int nrects; + auto rects = pixman_region32_rectangles(reg, &nrects); + auto xrects = ccalloc(nrects, xcb_rectangle_t); + for (int i = 0; i < nrects; i++) { + xrects[i] = + (xcb_rectangle_t){.x = to_i16_checked(rects[i].x1), + .y = to_i16_checked(rects[i].y1), + .width = to_u16_checked(rects[i].x2 - rects[i].x1), + .height = to_u16_checked(rects[i].y2 - rects[i].y1)}; + } + + xcb_xfixes_region_t ret = x_new_id(c); + bool success = + XCB_AWAIT_VOID(xcb_xfixes_create_region, c, ret, to_u32_checked(nrects), xrects); + free(xrects); + if (!success) { + return XCB_NONE; + } + return ret; +} + +void x_destroy_region(xcb_connection_t *c, xcb_xfixes_region_t r) { + if (r != XCB_NONE) { + xcb_xfixes_destroy_region(c, r); + } +} + void x_set_picture_clip_region(xcb_connection_t *c, xcb_render_picture_t pict, int16_t clip_x_origin, int16_t clip_y_origin, const region_t *reg) { diff --git a/src/x.h b/src/x.h index 3b8787c..a192886 100644 --- a/src/x.h +++ b/src/x.h @@ -216,6 +216,12 @@ x_create_picture_with_visual(xcb_connection_t *, xcb_drawable_t, int w, int h, /// Fetch a X region and store it in a pixman region bool x_fetch_region(xcb_connection_t *, xcb_xfixes_region_t r, region_t *res); +/// Create a X region from a pixman region +uint32_t x_create_region(xcb_connection_t *c, const region_t *reg); + +/// Destroy a X region +void x_destroy_region(xcb_connection_t *c, uint32_t region); + void x_set_picture_clip_region(xcb_connection_t *, xcb_render_picture_t, int16_t clip_x_origin, int16_t clip_y_origin, const region_t *); From 23a29470e50ef1d68b9c99186b069659a793184c Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Wed, 14 Dec 2022 07:40:42 +0000 Subject: [PATCH 05/13] backend: xrender: set update region for PresentPixmap request Signed-off-by: Yuxuan Shui --- src/backend/xrender/xrender.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/backend/xrender/xrender.c b/src/backend/xrender/xrender.c index e85a85f..19c2ebf 100644 --- a/src/backend/xrender/xrender.c +++ b/src/backend/xrender/xrender.c @@ -606,13 +606,16 @@ static void present(backend_t *base, const region_t *region) { XCB_NONE, xd->back[xd->curr_back], orig_x, orig_y, 0, 0, orig_x, orig_y, region_width, region_height); + auto xregion = x_create_region(base->c, region); + // Make sure we got reply from PresentPixmap before waiting for events, // to avoid deadlock auto e = xcb_request_check( base->c, xcb_present_pixmap_checked( xd->base.c, xd->target_win, - xd->back_pixmap[xd->curr_back], 0, XCB_NONE, XCB_NONE, 0, + xd->back_pixmap[xd->curr_back], 0, XCB_NONE, xregion, 0, 0, XCB_NONE, XCB_NONE, XCB_NONE, 0, 0, 0, 0, 0, NULL)); + x_destroy_region(base->c, xregion); if (e) { log_error("Failed to present pixmap"); free(e); From 8b189bc5ec1e1ed3aa601fc50eff1043307904a4 Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Wed, 14 Dec 2022 12:35:37 +0000 Subject: [PATCH 06/13] core: print error with full_sequence ev->sequence was just the lower 16 bits of the sequence number. Signed-off-by: Yuxuan Shui --- src/picom.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/picom.c b/src/picom.c index ecd0907..0e40d45 100644 --- a/src/picom.c +++ b/src/picom.c @@ -297,7 +297,7 @@ void discard_ignore(session_t *ps, unsigned long sequence) { } } -static int should_ignore(session_t *ps, unsigned long sequence) { +static int should_ignore(session_t *ps, uint32_t sequence) { if (ps == NULL) { // Do not ignore errors until the session has been initialized return false; @@ -964,7 +964,7 @@ void root_damaged(session_t *ps) { * Xlib error handler function. */ static int xerror(Display attr_unused *dpy, XErrorEvent *ev) { - if (!should_ignore(ps_g, ev->serial)) { + if (!should_ignore(ps_g, (uint32_t)ev->serial)) { x_print_error(ev->serial, ev->request_code, ev->minor_code, ev->error_code); } return 0; @@ -974,8 +974,9 @@ static int xerror(Display attr_unused *dpy, XErrorEvent *ev) { * XCB error handler function. */ void ev_xcb_error(session_t *ps, xcb_generic_error_t *err) { - if (!should_ignore(ps, err->sequence)) { - x_print_error(err->sequence, err->major_code, err->minor_code, err->error_code); + if (!should_ignore(ps, err->full_sequence)) { + x_print_error(err->full_sequence, err->major_code, err->minor_code, + err->error_code); } } From 5a5ea76006125711061a72f4f972c58b48c2ab1d Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Wed, 14 Dec 2022 13:35:11 +0000 Subject: [PATCH 07/13] event: restore event sequence number after passing it to Xlib handlers We set event sequence number to the last sequence xlib knows about to silence its complaint about missing sequence numbers, but we forgot to restore it back afterwards. This used to break error ignoring mechanism in `should_ignore`. In the last commit we updated it to use full_sequence which incidently fixed this problem. But let's restore the sequence number anyway for good measure. Signed-off-by: Yuxuan Shui --- src/event.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/event.c b/src/event.c index e6052f1..c4a62e8 100644 --- a/src/event.c +++ b/src/event.c @@ -121,11 +121,13 @@ static inline const char *ev_name(session_t *ps, xcb_generic_event_t *ev) { CASESTRRET(ClientMessage); } - if (ps->damage_event + XCB_DAMAGE_NOTIFY == ev->response_type) + if (ps->damage_event + XCB_DAMAGE_NOTIFY == ev->response_type) { return "Damage"; + } - if (ps->shape_exists && ev->response_type == ps->shape_event) + if (ps->shape_exists && ev->response_type == ps->shape_event) { return "ShapeNotify"; + } if (ps->xsync_exists) { int o = ev->response_type - ps->xsync_event; @@ -695,8 +697,11 @@ void ev_handle(session_t *ps, xcb_generic_event_t *ev) { // missing sequence numbers. // // We only need the low 16 bits + uint16_t seq = ev->sequence; ev->sequence = (uint16_t)(LastKnownRequestProcessed(ps->dpy) & 0xffff); proc(ps->dpy, &dummy, (xEvent *)ev); + // Restore the sequence number + ev->sequence = seq; } // XXX redraw needs to be more fine grained From 3b342afa953cbb8b615d85ae1fb174597eb49224 Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Wed, 14 Dec 2022 13:47:38 +0000 Subject: [PATCH 08/13] general: fix compiler warning about unused results Signed-off-by: Yuxuan Shui --- src/dbus.c | 5 ++++- src/log.c | 2 +- src/picom.c | 6 +++++- src/utils.c | 2 +- 4 files changed, 11 insertions(+), 4 deletions(-) diff --git a/src/dbus.c b/src/dbus.c index 8b17b30..2b17341 100644 --- a/src/dbus.c +++ b/src/dbus.c @@ -1435,7 +1435,10 @@ static bool cdbus_process_windows_root_introspect(session_t *ps, DBusMessage *ms continue; } char *tmp = NULL; - asprintf(&tmp, "\n", w->id); + if (asprintf(&tmp, "\n", w->id) < 0) { + log_fatal("Failed to allocate memory."); + abort(); + } mstrextend(&ret, tmp); free(tmp); } diff --git a/src/log.c b/src/log.c index 0b663e7..8a494b9 100644 --- a/src/log.c +++ b/src/log.c @@ -256,7 +256,7 @@ static void file_logger_write(struct log_target *tgt, const char *str, size_t le static void file_logger_writev(struct log_target *tgt, const struct iovec *vec, int vcnt) { auto f = (struct file_logger *)tgt; fflush(f->f); - writev(fileno(f->f), vec, vcnt); + ssize_t _ attr_unused = writev(fileno(f->f), vec, vcnt); } static void file_logger_destroy(struct log_target *tgt) { diff --git a/src/picom.c b/src/picom.c index 0e40d45..f922978 100644 --- a/src/picom.c +++ b/src/picom.c @@ -2560,7 +2560,11 @@ int main(int argc, char **argv) { // Notify the parent that we are done. This might cause the parent // to quit, so only do this after setsid() int tmp = 1; - write(pfds[1], &tmp, sizeof tmp); + if (write(pfds[1], &tmp, sizeof tmp) != sizeof tmp) { + log_fatal("Failed to notify parent process"); + ret_code = 1; + break; + } close(pfds[1]); // We only do this once need_fork = false; diff --git a/src/utils.c b/src/utils.c index c080e53..a1114a0 100644 --- a/src/utils.c +++ b/src/utils.c @@ -27,7 +27,7 @@ void report_allocation_failure(const char *func, const char *file, unsigned int {.iov_base = (void *)msg2, .iov_len = sizeof(msg2) - 1}, }; - writev(STDERR_FILENO, v, ARR_SIZE(v)); + ssize_t _ attr_unused = writev(STDERR_FILENO, v, ARR_SIZE(v)); abort(); unreachable; From 4ecb8093cf68aa5b38d417744fa9047a40c62511 Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Wed, 14 Dec 2022 13:57:09 +0000 Subject: [PATCH 09/13] x: fix CI build failure Signed-off-by: Yuxuan Shui --- src/x.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/x.c b/src/x.c index 7faa1c9..e71f1ee 100644 --- a/src/x.c +++ b/src/x.c @@ -394,7 +394,10 @@ uint32_t x_create_region(xcb_connection_t *c, const region_t *reg) { } int nrects; - auto rects = pixman_region32_rectangles(reg, &nrects); + // In older pixman versions, pixman_region32_rectangles doesn't take const + // region_t, instead of dealing with this version difference, just suppress the + // warning. + const pixman_box32_t *rects = pixman_region32_rectangles((region_t *)reg, &nrects); auto xrects = ccalloc(nrects, xcb_rectangle_t); for (int i = 0; i < nrects; i++) { xrects[i] = From aca3fdcef7bfcb1c3ce65cf87413fa6ab280d183 Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Wed, 14 Dec 2022 14:23:55 +0000 Subject: [PATCH 10/13] core: expand X error handling We used to have a list of X errors we should ignore in case they do occur. This commit expands that functionality to also allow us aborting on certain errors. Signed-off-by: Yuxuan Shui --- src/common.h | 44 ++++++++++++++++++------------ src/event.c | 2 +- src/picom.c | 76 ++++++++++++++++++++++++++++++++-------------------- src/picom.h | 2 +- src/x.c | 10 ++++++- src/x.h | 2 ++ 6 files changed, 87 insertions(+), 49 deletions(-) diff --git a/src/common.h b/src/common.h index c773187..863e14e 100644 --- a/src/common.h +++ b/src/common.h @@ -36,9 +36,9 @@ #include #include #include -#include #include #include +#include #include "uthash_extra.h" #ifdef CONFIG_OPENGL @@ -55,11 +55,11 @@ #include "backend/driver.h" #include "compiler.h" #include "config.h" +#include "list.h" #include "region.h" +#include "render.h" #include "types.h" #include "utils.h" -#include "list.h" -#include "render.h" #include "win_defs.h" #include "x.h" @@ -83,10 +83,17 @@ struct glx_session; struct atom; struct conv; -typedef struct _ignore { - struct _ignore *next; +enum pending_reply_action { + PENDING_REPLY_ACTION_IGNORE, + PENDING_REPLY_ACTION_ABORT, + PENDING_REPLY_ACTION_DEBUG_ABORT, +}; + +typedef struct pending_reply { + struct pending_reply *next; unsigned long sequence; -} ignore_t; + enum pending_reply_action action; +} pending_reply_t; #ifdef CONFIG_OPENGL #ifdef DEBUG_GLX_DEBUG_CONTEXT @@ -256,18 +263,18 @@ typedef struct session { /// Time of last fading. In milliseconds. long long fade_time; /// Head pointer of the error ignore linked list. - ignore_t *ignore_head; + pending_reply_t *pending_reply_head; /// Pointer to the next member of tail element of the error /// ignore linked list. - ignore_t **ignore_tail; + pending_reply_t **pending_reply_tail; // Cached blur convolution kernels. struct x_convolution_kernel **blur_kerns_cache; /// If we should quit - bool quit:1; + bool quit : 1; // TODO(yshui) use separate flags for dfferent kinds of updates so we don't // waste our time. /// Whether there are pending updates, like window creation, etc. - bool pending_updates:1; + bool pending_updates : 1; // === Expose event related === /// Pointer to an array of XRectangle-s of exposed region. @@ -468,18 +475,21 @@ static inline bool bkend_use_glx(session_t *ps) { return BKEND_GLX == ps->o.backend || BKEND_XR_GLX_HYBRID == ps->o.backend; } -static void set_ignore(session_t *ps, unsigned long sequence) { - if (ps->o.show_all_xerrors) +static void set_ignore(session_t *ps, uint32_t sequence) { + if (ps->o.show_all_xerrors) { return; + } - auto i = cmalloc(ignore_t); - if (!i) - return; + auto i = cmalloc(pending_reply_t); + if (!i) { + abort(); + } i->sequence = sequence; i->next = 0; - *ps->ignore_tail = i; - ps->ignore_tail = &i->next; + i->action = PENDING_REPLY_ACTION_IGNORE; + *ps->pending_reply_tail = i; + ps->pending_reply_tail = &i->next; } /** diff --git a/src/event.c b/src/event.c index c4a62e8..89f9624 100644 --- a/src/event.c +++ b/src/event.c @@ -665,7 +665,7 @@ ev_selection_clear(session_t *ps, xcb_selection_clear_event_t attr_unused *ev) { void ev_handle(session_t *ps, xcb_generic_event_t *ev) { if ((ev->response_type & 0x7f) != KeymapNotify) { - discard_ignore(ps, ev->full_sequence); + discard_pending(ps, ev->full_sequence); } xcb_window_t wid = ev_window(ps, ev); diff --git a/src/picom.c b/src/picom.c index f922978..ba97f4c 100644 --- a/src/picom.c +++ b/src/picom.c @@ -282,14 +282,14 @@ static bool run_fade(session_t *ps, struct managed_win **_w, long long steps) { // === Error handling === -void discard_ignore(session_t *ps, unsigned long sequence) { - while (ps->ignore_head) { - if (sequence > ps->ignore_head->sequence) { - ignore_t *next = ps->ignore_head->next; - free(ps->ignore_head); - ps->ignore_head = next; - if (!ps->ignore_head) { - ps->ignore_tail = &ps->ignore_head; +void discard_pending(session_t *ps, uint32_t sequence) { + while (ps->pending_reply_head) { + if (sequence > ps->pending_reply_head->sequence) { + auto next = ps->pending_reply_head->next; + free(ps->pending_reply_head); + ps->pending_reply_head = next; + if (!ps->pending_reply_head) { + ps->pending_reply_tail = &ps->pending_reply_head; } } else { break; @@ -297,13 +297,28 @@ void discard_ignore(session_t *ps, unsigned long sequence) { } } -static int should_ignore(session_t *ps, uint32_t sequence) { +static void handle_error(session_t *ps, xcb_generic_error_t *ev) { if (ps == NULL) { // Do not ignore errors until the session has been initialized - return false; + return; } - discard_ignore(ps, sequence); - return ps->ignore_head && ps->ignore_head->sequence == sequence; + discard_pending(ps, ev->full_sequence); + if (ps->pending_reply_head && ps->pending_reply_head->sequence == ev->full_sequence) { + if (ps->pending_reply_head->action != PENDING_REPLY_ACTION_IGNORE) { + x_log_error(LOG_LEVEL_ERROR, ev->full_sequence, ev->major_code, + ev->minor_code, ev->error_code); + } + switch (ps->pending_reply_head->action) { + case PENDING_REPLY_ACTION_ABORT: + log_fatal("An unrecoverable X error occurred, aborting..."); + abort(); + case PENDING_REPLY_ACTION_DEBUG_ABORT: assert(false); break; + case PENDING_REPLY_ACTION_IGNORE: break; + } + return; + } + x_log_error(LOG_LEVEL_WARN, ev->full_sequence, ev->major_code, ev->minor_code, + ev->error_code); } // === Windows === @@ -964,9 +979,13 @@ void root_damaged(session_t *ps) { * Xlib error handler function. */ static int xerror(Display attr_unused *dpy, XErrorEvent *ev) { - if (!should_ignore(ps_g, (uint32_t)ev->serial)) { - x_print_error(ev->serial, ev->request_code, ev->minor_code, ev->error_code); - } + // Fake a xcb error, fill in just enough information + xcb_generic_error_t xcb_err; + xcb_err.full_sequence = (uint32_t)ev->serial; + xcb_err.major_code = ev->request_code; + xcb_err.minor_code = ev->minor_code; + xcb_err.error_code = ev->error_code; + handle_error(ps_g, &xcb_err); return 0; } @@ -974,10 +993,7 @@ static int xerror(Display attr_unused *dpy, XErrorEvent *ev) { * XCB error handler function. */ void ev_xcb_error(session_t *ps, xcb_generic_error_t *err) { - if (!should_ignore(ps, err->full_sequence)) { - x_print_error(err->full_sequence, err->major_code, err->minor_code, - err->error_code); - } + handle_error(ps, err); } /** @@ -1678,8 +1694,8 @@ static session_t *session_init(int argc, char **argv, Display *dpy, .redirected = false, .alpha_picts = NULL, .fade_time = 0L, - .ignore_head = NULL, - .ignore_tail = NULL, + .pending_reply_head = NULL, + .pending_reply_tail = NULL, .quit = false, .expose_rects = NULL, @@ -1741,7 +1757,7 @@ static session_t *session_init(int argc, char **argv, Display *dpy, ps->loop = EV_DEFAULT; pixman_region32_init(&ps->screen_reg); - ps->ignore_tail = &ps->ignore_head; + ps->pending_reply_tail = &ps->pending_reply_head; ps->o.show_all_xerrors = all_xerrors; @@ -2330,26 +2346,28 @@ static void session_destroy(session_t *ps) { // Free ignore linked list { - ignore_t *next = NULL; - for (ignore_t *ign = ps->ignore_head; ign; ign = next) { + pending_reply_t *next = NULL; + for (auto ign = ps->pending_reply_head; ign; ign = next) { next = ign->next; free(ign); } // Reset head and tail - ps->ignore_head = NULL; - ps->ignore_tail = &ps->ignore_head; + ps->pending_reply_head = NULL; + ps->pending_reply_tail = &ps->pending_reply_head; } // Free tgt_{buffer,picture} and root_picture - if (ps->tgt_buffer.pict == ps->tgt_picture) + if (ps->tgt_buffer.pict == ps->tgt_picture) { ps->tgt_buffer.pict = XCB_NONE; + } - if (ps->tgt_picture == ps->root_picture) + if (ps->tgt_picture == ps->root_picture) { ps->tgt_picture = XCB_NONE; - else + } else { free_picture(ps->c, &ps->tgt_picture); + } free_picture(ps->c, &ps->root_picture); free_paint(ps, &ps->tgt_buffer); diff --git a/src/picom.h b/src/picom.h index 7ee289b..b5a1e8a 100644 --- a/src/picom.h +++ b/src/picom.h @@ -46,7 +46,7 @@ void cxinerama_upd_scrs(session_t *ps); void queue_redraw(session_t *ps); -void discard_ignore(session_t *ps, unsigned long sequence); +void discard_pending(session_t *ps, uint32_t sequence); void set_root_flags(session_t *ps, uint64_t flags); diff --git a/src/x.c b/src/x.c index e71f1ee..e598345 100644 --- a/src/x.c +++ b/src/x.c @@ -562,8 +562,16 @@ _x_strerror(unsigned long serial, uint8_t major, uint16_t minor, uint8_t error_c /** * Log a X11 error */ +void x_log_error(enum log_level level, unsigned long serial, uint8_t major, + uint16_t minor, uint8_t error_code) { + if (unlikely(level >= log_get_level_tls())) { + log_printf(tls_logger, level, __func__, "%s", + _x_strerror(serial, major, minor, error_code)); + } +} + void x_print_error(unsigned long serial, uint8_t major, uint16_t minor, uint8_t error_code) { - log_debug("%s", _x_strerror(serial, major, minor, error_code)); + x_log_error(LOG_LEVEL_DEBUG, serial, major, minor, error_code); } /* diff --git a/src/x.h b/src/x.h index a192886..78efea3 100644 --- a/src/x.h +++ b/src/x.h @@ -231,6 +231,8 @@ void x_clear_picture_clip_region(xcb_connection_t *, xcb_render_picture_t pict); * Log a X11 error */ void x_print_error(unsigned long serial, uint8_t major, uint16_t minor, uint8_t error_code); +void x_log_error(enum log_level level, unsigned long serial, uint8_t major, + uint16_t minor, uint8_t error_code); /* * Convert a xcb_generic_error_t to a string that describes the error From 17b34ce3909d7e847670d6c121e1f3e5f499bf40 Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Wed, 14 Dec 2022 14:28:42 +0000 Subject: [PATCH 11/13] core: add set_cant_fail_cookie Enables picom to abort when certain requests fail. Signed-off-by: Yuxuan Shui --- src/common.h | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/src/common.h b/src/common.h index 863e14e..542dc94 100644 --- a/src/common.h +++ b/src/common.h @@ -475,11 +475,8 @@ static inline bool bkend_use_glx(session_t *ps) { return BKEND_GLX == ps->o.backend || BKEND_XR_GLX_HYBRID == ps->o.backend; } -static void set_ignore(session_t *ps, uint32_t sequence) { - if (ps->o.show_all_xerrors) { - return; - } - +static void +set_reply_action(session_t *ps, uint32_t sequence, enum pending_reply_action action) { auto i = cmalloc(pending_reply_t); if (!i) { abort(); @@ -487,7 +484,7 @@ static void set_ignore(session_t *ps, uint32_t sequence) { i->sequence = sequence; i->next = 0; - i->action = PENDING_REPLY_ACTION_IGNORE; + i->action = action; *ps->pending_reply_tail = i; ps->pending_reply_tail = &i->next; } @@ -496,7 +493,15 @@ static void set_ignore(session_t *ps, uint32_t sequence) { * Ignore X errors caused by given X request. */ static inline void set_ignore_cookie(session_t *ps, xcb_void_cookie_t cookie) { - set_ignore(ps, cookie.sequence); + if (ps->o.show_all_xerrors) { + return; + } + + set_reply_action(ps, cookie.sequence, PENDING_REPLY_ACTION_IGNORE); +} + +static inline void set_cant_fail_cookie(session_t *ps, xcb_void_cookie_t cookie) { + set_reply_action(ps, cookie.sequence, PENDING_REPLY_ACTION_ABORT); } /** From 1ea3276bd135119a1f3da67a459f05151402958c Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Sun, 18 Dec 2022 19:23:06 +0000 Subject: [PATCH 12/13] x: fix missing _checked Signed-off-by: Yuxuan Shui --- src/x.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/x.c b/src/x.c index e598345..42bee3f 100644 --- a/src/x.c +++ b/src/x.c @@ -449,9 +449,10 @@ void x_set_picture_clip_region(xcb_connection_t *c, xcb_render_picture_t pict, } void x_clear_picture_clip_region(xcb_connection_t *c, xcb_render_picture_t pict) { + assert(pict != XCB_NONE); xcb_render_change_picture_value_list_t v = {.clipmask = XCB_NONE}; xcb_generic_error_t *e = xcb_request_check( - c, xcb_render_change_picture(c, pict, XCB_RENDER_CP_CLIP_MASK, &v)); + c, xcb_render_change_picture_checked(c, pict, XCB_RENDER_CP_CLIP_MASK, &v)); if (e) { log_error_x_error(e, "failed to clear clip region"); free(e); From c28462673e2b3332b355c33adbec1190572c5407 Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Sun, 18 Dec 2022 19:23:49 +0000 Subject: [PATCH 13/13] backend: xrender: fix using of invalid picture when vsync is disabled Fixes #974 Signed-off-by: Yuxuan Shui --- src/backend/xrender/xrender.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/backend/xrender/xrender.c b/src/backend/xrender/xrender.c index 19c2ebf..4381977 100644 --- a/src/backend/xrender/xrender.c +++ b/src/backend/xrender/xrender.c @@ -594,13 +594,13 @@ static void present(backend_t *base, const region_t *region) { uint16_t region_width = to_u16_checked(extent->x2 - extent->x1), region_height = to_u16_checked(extent->y2 - extent->y1); - // compose() sets clip region on the back buffer, so clear it first - x_clear_picture_clip_region(base->c, xd->back[xd->curr_back]); - // limit the region of update x_set_picture_clip_region(base->c, xd->back[2], 0, 0, region); if (xd->vsync) { + // compose() sets clip region on the back buffer, so clear it first + x_clear_picture_clip_region(base->c, xd->back[xd->curr_back]); + // Update the back buffer first, then present xcb_render_composite(base->c, XCB_RENDER_PICT_OP_SRC, xd->back[2], XCB_NONE, xd->back[xd->curr_back], orig_x, orig_y, 0,