From a099678664c17d2ac58fdd05c1939813e74c452a Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Fri, 23 Oct 2020 01:27:55 +0100 Subject: [PATCH 1/5] win: delayed handling of configure notify Part of the configure notify handling which requires querying the X server, has been moved into the X critical section. Signed-off-by: Yuxuan Shui --- src/event.c | 57 ++++++++++---------- src/win.c | 137 +++++++++++++++++++++++++++---------------------- src/win.h | 2 +- src/win_defs.h | 4 ++ 4 files changed, 110 insertions(+), 90 deletions(-) diff --git a/src/event.c b/src/event.c index d12d76ee..8e885428 100644 --- a/src/event.c +++ b/src/event.c @@ -189,8 +189,6 @@ static inline void ev_create_notify(session_t *ps, xcb_create_notify_event_t *ev /// Handle configure event of a regular window static void configure_win(session_t *ps, xcb_configure_notify_event_t *ce) { auto w = find_win(ps, ce->window); - region_t damage; - pixman_region32_init(&damage); if (!w) { return; @@ -210,44 +208,47 @@ static void configure_win(session_t *ps, xcb_configure_notify_event_t *ce) { restack_above(ps, w, ce->above_sibling); } else { restack_above(ps, w, ce->above_sibling); - bool factor_change = false; - win_extents(mw, &damage); // If window geometry change, free old extents if (mw->g.x != ce->x || mw->g.y != ce->y || mw->g.width != ce->width || mw->g.height != ce->height || mw->g.border_width != ce->border_width) { - factor_change = true; - } + region_t damage, new_extents; + pixman_region32_init(&damage); + pixman_region32_init(&new_extents); - mw->g.x = ce->x; - mw->g.y = ce->y; + // Get the old window extents + win_extents(mw, &damage); - if (mw->g.width != ce->width || mw->g.height != ce->height || - mw->g.border_width != ce->border_width) { - log_trace("Window size changed, %dx%d -> %dx%d", mw->g.width, - mw->g.height, ce->width, ce->height); - mw->g.width = ce->width; - mw->g.height = ce->height; - mw->g.border_width = ce->border_width; - win_on_win_size_change(ps, mw); - win_update_bounding_shape(ps, mw); - } + // Queue pending updates + win_set_flags(mw, WIN_FLAGS_FACTOR_CHANGED); + ps->pending_updates = true; - region_t new_extents; - pixman_region32_init(&new_extents); - win_extents(mw, &new_extents); - pixman_region32_union(&damage, &damage, &new_extents); - pixman_region32_fini(&new_extents); + mw->g.x = ce->x; + mw->g.y = ce->y; - if (factor_change) { - win_on_factor_change(ps, mw); + if (mw->g.width != ce->width || mw->g.height != ce->height || + mw->g.border_width != ce->border_width) { + log_trace("Window size changed, %dx%d -> %dx%d", + mw->g.width, mw->g.height, ce->width, ce->height); + mw->g.width = ce->width; + mw->g.height = ce->height; + mw->g.border_width = ce->border_width; + win_set_flags(mw, WIN_FLAGS_SIZE_STALE); + } + + win_extents(mw, &new_extents); + // Mark the union of the old and new extents as damaged + pixman_region32_union(&damage, &damage, &new_extents); add_damage(ps, &damage); - win_update_screen(ps, mw); + + pixman_region32_fini(&damage); + pixman_region32_fini(&new_extents); + + // Recalculate which screen this window is on + win_update_screen(ps->xinerama_nscrs, ps->xinerama_scr_regs, mw); } } - pixman_region32_fini(&damage); - // override_redirect flag cannot be changed after window creation, as far // as I know, so there's no point to re-match windows here. mw->a.override_redirect = ce->override_redirect; diff --git a/src/win.c b/src/win.c index 15fb6023..0d4e08d8 100644 --- a/src/win.c +++ b/src/win.c @@ -350,6 +350,60 @@ static bool win_fetch_and_unset_property_stale(struct managed_win *w, xcb_atom_t /// Returns true if any of the properties are stale, as well as clear all the stale flags. static bool win_check_and_clear_all_properties_stale(struct managed_win *w); +/// Fetch new window properties from the X server, and run appropriate updates. Might set +/// WIN_FLAGS_FACTOR_CHANGED +static void win_update_properties(session_t *ps, struct managed_win *w) { + if (win_fetch_and_unset_property_stale(w, ps->atoms->a_NET_WM_WINDOW_TYPE)) { + win_update_wintype(ps, w); + } + + if (win_fetch_and_unset_property_stale(w, ps->atoms->a_NET_WM_WINDOW_OPACITY)) { + win_update_opacity_prop(ps, w); + // we cannot receive OPACITY change when window has been destroyed + assert(w->state != WSTATE_DESTROYING); + win_update_opacity_target(ps, w); + } + + if (win_fetch_and_unset_property_stale(w, ps->atoms->a_NET_FRAME_EXTENTS)) { + win_update_frame_extents(ps, w, w->client_win); + add_damage_from_win(ps, w); + } + + if (win_fetch_and_unset_property_stale(w, ps->atoms->aWM_NAME) || + win_fetch_and_unset_property_stale(w, ps->atoms->a_NET_WM_NAME)) { + if (win_update_name(ps, w) == 1) { + win_set_flags(w, WIN_FLAGS_FACTOR_CHANGED); + } + } + + if (win_fetch_and_unset_property_stale(w, ps->atoms->aWM_CLASS)) { + if (win_update_class(ps, w)) { + win_set_flags(w, WIN_FLAGS_FACTOR_CHANGED); + } + } + + if (win_fetch_and_unset_property_stale(w, ps->atoms->aWM_WINDOW_ROLE)) { + if (win_update_role(ps, w) == 1) { + win_set_flags(w, WIN_FLAGS_FACTOR_CHANGED); + } + } + + if (win_fetch_and_unset_property_stale(w, ps->atoms->a_COMPTON_SHADOW)) { + win_update_prop_shadow(ps, w); + } + + if (win_fetch_and_unset_property_stale(w, ps->atoms->aWM_CLIENT_LEADER) || + win_fetch_and_unset_property_stale(w, ps->atoms->aWM_TRANSIENT_FOR)) { + win_update_leader(ps, w); + } + + if (win_check_and_clear_all_properties_stale(w)) { + // Some other flags we didn't explicitly check has changed, must + // have been a tracked atom for the custom rules + win_set_flags(w, WIN_FLAGS_FACTOR_CHANGED); + } +} + void win_process_update_flags(session_t *ps, struct managed_win *w) { if (win_check_flags_all(w, WIN_FLAGS_MAPPED)) { map_win_start(ps, w); @@ -363,62 +417,21 @@ void win_process_update_flags(session_t *ps, struct managed_win *w) { win_clear_flags(w, WIN_FLAGS_CLIENT_STALE); } + if (win_check_flags_all(w, WIN_FLAGS_SIZE_STALE)) { + win_on_win_size_change(ps, w); + win_update_bounding_shape(ps, w); + win_clear_flags(w, WIN_FLAGS_SIZE_STALE); + } + if (win_check_flags_all(w, WIN_FLAGS_PROPERTY_STALE)) { - bool factor_change = false; + win_update_properties(ps, w); + win_clear_flags(w, WIN_FLAGS_PROPERTY_STALE); + } - if (win_fetch_and_unset_property_stale(w, ps->atoms->a_NET_WM_WINDOW_TYPE)) { - win_update_wintype(ps, w); - } - - if (win_fetch_and_unset_property_stale(w, ps->atoms->a_NET_WM_WINDOW_OPACITY)) { - win_update_opacity_prop(ps, w); - // we cannot receive OPACITY change when window has been destroyed - assert(w->state != WSTATE_DESTROYING); - win_update_opacity_target(ps, w); - } - - if (win_fetch_and_unset_property_stale(w, ps->atoms->a_NET_FRAME_EXTENTS)) { - win_update_frame_extents(ps, w, w->client_win); - add_damage_from_win(ps, w); - } - - if (win_fetch_and_unset_property_stale(w, ps->atoms->aWM_NAME) || - win_fetch_and_unset_property_stale(w, ps->atoms->a_NET_WM_NAME)) { - if (win_update_name(ps, w) == 1) { - factor_change = true; - } - } - - if (win_fetch_and_unset_property_stale(w, ps->atoms->aWM_CLASS)) { - if (win_update_class(ps, w)) { - factor_change = true; - } - } - - if (win_fetch_and_unset_property_stale(w, ps->atoms->aWM_WINDOW_ROLE)) { - if (win_update_role(ps, w) == 1) { - factor_change = true; - } - } - - if (win_fetch_and_unset_property_stale(w, ps->atoms->a_COMPTON_SHADOW)) { - win_update_prop_shadow(ps, w); - } - - if (win_fetch_and_unset_property_stale(w, ps->atoms->aWM_CLIENT_LEADER) || - win_fetch_and_unset_property_stale(w, ps->atoms->aWM_TRANSIENT_FOR)) { - win_update_leader(ps, w); - } - - if (win_check_and_clear_all_properties_stale(w)) { - // Some other flags we didn't explicitly check has changed, must - // have been a tracked atom for the custom rules - factor_change = true; - } - - if (factor_change) { - win_on_factor_change(ps, w); - } + // Factor change flags could be set by previous stages, so must be handled last + if (win_check_flags_all(w, WIN_FLAGS_FACTOR_CHANGED)) { + win_on_factor_change(ps, w); + win_clear_flags(w, WIN_FLAGS_FACTOR_CHANGED); } } @@ -884,8 +897,9 @@ void win_update_prop_shadow(session_t *ps, struct managed_win *w) { } static void win_set_invert_color(session_t *ps, struct managed_win *w, bool invert_color_new) { - if (w->invert_color == invert_color_new) + if (w->invert_color == invert_color_new) { return; + } w->invert_color = invert_color_new; @@ -898,10 +912,11 @@ static void win_set_invert_color(session_t *ps, struct managed_win *w, bool inve static void win_determine_invert_color(session_t *ps, struct managed_win *w) { bool invert_color_new = w->invert_color; - if (UNSET != w->invert_color_force) + if (UNSET != w->invert_color_force) { invert_color_new = w->invert_color_force; - else if (w->a.map_state == XCB_MAP_STATE_VIEWABLE) + } else if (w->a.map_state == XCB_MAP_STATE_VIEWABLE) { invert_color_new = c2_match(ps, w, ps->o.invert_color_list, NULL); + } win_set_invert_color(ps, w, invert_color_new); } @@ -2118,11 +2133,11 @@ bool win_skip_fading(session_t *ps, struct managed_win *w) { * TODO(yshui) move to x.c * TODO(yshui) use xrandr */ -void win_update_screen(session_t *ps, struct managed_win *w) { +void win_update_screen(int nscreens, region_t *screens, struct managed_win *w) { w->xinerama_scr = -1; - for (int i = 0; i < ps->xinerama_nscrs; i++) { - auto e = pixman_region32_extents(&ps->xinerama_scr_regs[i]); + for (int i = 0; i < nscreens; i++) { + auto e = pixman_region32_extents(&screens[i]); if (e->x1 <= w->g.x && e->y1 <= w->g.y && e->x2 >= w->g.x + w->widthb && e->y2 >= w->g.y + w->heightb) { w->xinerama_scr = i; @@ -2190,7 +2205,7 @@ void map_win_start(session_t *ps, struct managed_win *w) { // XXX Can we assume map_state is always viewable? w->a.map_state = XCB_MAP_STATE_VIEWABLE; - win_update_screen(ps, w); + win_update_screen(ps->xinerama_nscrs, ps->xinerama_scr_regs, w); // Set window event mask before reading properties so that no property // changes are lost diff --git a/src/win.h b/src/win.h index 0b70c4ce..6d94573d 100644 --- a/src/win.h +++ b/src/win.h @@ -314,7 +314,7 @@ void win_recheck_client(session_t *ps, struct managed_win *w); */ double attr_pure win_calc_opacity_target(session_t *ps, const struct managed_win *w); bool attr_pure win_should_dim(session_t *ps, const struct managed_win *w); -void win_update_screen(session_t *, struct managed_win *); +void win_update_screen(int nscreens, region_t *screens, struct managed_win *w); /** * Retrieve the bounding shape of a window. */ diff --git a/src/win_defs.h b/src/win_defs.h index 278ccc08..48ed6512 100644 --- a/src/win_defs.h +++ b/src/win_defs.h @@ -87,6 +87,10 @@ enum win_flags { WIN_FLAGS_MAPPED = 64, /// this window has properties which needs to be updated WIN_FLAGS_PROPERTY_STALE = 128, + /// this window has an unhandled size/shape change + WIN_FLAGS_SIZE_STALE = 256, + /// need better name for this, is set when some aspects of the window changed + WIN_FLAGS_FACTOR_CHANGED = 512, }; static const uint64_t WIN_FLAGS_IMAGES_STALE = From 21e9aab6b695862b5193828ba2d226baab0c4bd2 Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Fri, 23 Oct 2020 01:51:28 +0100 Subject: [PATCH 2/5] event: set WIN_FLAGS_FACTOR_CHANGED on tracked property change Previously, when some explicitly checked property (e.g. _NET_WM_WINDOW_OPACITY) changed, it won't trigger a win_on_factor_change, and the rules will not be re-evaluated. Because that property stale flag will be cleared after we explicitly check the property, so it won't be detected as a tracked property change. Here, we instead set WIN_FLAGS_FACTOR_CHANGED directly when a tracked property changed. Fixes: f3ff7eff8c90 Signed-off-by: Yuxuan Shui --- src/event.c | 6 +++++- src/win.c | 17 ++++------------- 2 files changed, 9 insertions(+), 14 deletions(-) diff --git a/src/event.c b/src/event.c index 8e885428..70021837 100644 --- a/src/event.c +++ b/src/event.c @@ -570,7 +570,11 @@ static inline void ev_property_notify(session_t *ps, xcb_property_notify_event_t w = find_toplevel(ps, ev->window); } if (w) { - win_set_property_stale(w, ev->atom); + // Set FACTOR_CHANGED so rules based on properties will be + // re-evaluated. + // Don't need to set property stale here, since that only + // concerns properties we explicitly check. + win_set_flags(w, WIN_FLAGS_FACTOR_CHANGED); } break; } diff --git a/src/win.c b/src/win.c index 0d4e08d8..9034516b 100644 --- a/src/win.c +++ b/src/win.c @@ -348,7 +348,7 @@ void win_release_images(struct backend_base *backend, struct managed_win *w) { /// Returns true if the `prop` property is stale, as well as clears the stale flag. static bool win_fetch_and_unset_property_stale(struct managed_win *w, xcb_atom_t prop); /// Returns true if any of the properties are stale, as well as clear all the stale flags. -static bool win_check_and_clear_all_properties_stale(struct managed_win *w); +static void win_clear_all_properties_stale(struct managed_win *w); /// Fetch new window properties from the X server, and run appropriate updates. Might set /// WIN_FLAGS_FACTOR_CHANGED @@ -397,11 +397,7 @@ static void win_update_properties(session_t *ps, struct managed_win *w) { win_update_leader(ps, w); } - if (win_check_and_clear_all_properties_stale(w)) { - // Some other flags we didn't explicitly check has changed, must - // have been a tracked atom for the custom rules - win_set_flags(w, WIN_FLAGS_FACTOR_CHANGED); - } + win_clear_all_properties_stale(w); } void win_process_update_flags(session_t *ps, struct managed_win *w) { @@ -2502,14 +2498,9 @@ void win_set_property_stale(struct managed_win *w, xcb_atom_t prop) { win_set_flags(w, WIN_FLAGS_PROPERTY_STALE); } -static bool win_check_and_clear_all_properties_stale(struct managed_win *w) { - bool ret = false; - for (size_t i = 0; i < w->stale_props_capacity; i++) { - ret = ret || (w->stale_props[i] != 0); - w->stale_props[i] = 0; - } +static void win_clear_all_properties_stale(struct managed_win *w) { + memset(w->stale_props, 0, w->stale_props_capacity * sizeof(*w->stale_props)); win_clear_flags(w, WIN_FLAGS_PROPERTY_STALE); - return ret; } static bool win_fetch_and_unset_property_stale(struct managed_win *w, xcb_atom_t prop) { From 35b4e82085d47c0764eb347cf330be0fbcbe999a Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Fri, 23 Oct 2020 02:08:34 +0100 Subject: [PATCH 3/5] win: cache the result of fade-exclude rules So we don't need to call c2_match every frame something is fading. Also saves us from some out-of-critical-section X server queries. Signed-off-by: Yuxuan Shui --- src/win.c | 60 +++++++++++++++++++++++++++++++++++-------------------- src/win.h | 2 ++ 2 files changed, 40 insertions(+), 22 deletions(-) diff --git a/src/win.c b/src/win.c index 9034516b..e78e2d3a 100644 --- a/src/win.c +++ b/src/win.c @@ -758,11 +758,7 @@ bool win_should_fade(session_t *ps, const struct managed_win *w) { // deprecated return false; } - // Ignore other possible causes of fading state changes after window - // gets unmapped - // if (w->a.map_state != XCB_MAP_STATE_VIEWABLE) { - //} - if (c2_match(ps, w, ps->o.fade_blacklist, NULL)) { + if (w->fade_excluded) { return false; } return ps->o.wintype_option[w->window_type].fade; @@ -1030,11 +1026,15 @@ void win_on_factor_change(session_t *ps, struct managed_win *w) { w->mode = win_calc_mode(w); log_debug("Window mode changed to %d", w->mode); win_update_opacity_rule(ps, w); - if (w->a.map_state == XCB_MAP_STATE_VIEWABLE) + if (w->a.map_state == XCB_MAP_STATE_VIEWABLE) { w->paint_excluded = c2_match(ps, w, ps->o.paint_blacklist, NULL); - if (w->a.map_state == XCB_MAP_STATE_VIEWABLE) + } + if (w->a.map_state == XCB_MAP_STATE_VIEWABLE) { w->unredir_if_possible_excluded = c2_match(ps, w, ps->o.unredir_if_possible_blacklist, NULL); + } + + w->fade_excluded = c2_match(ps, w, ps->o.fade_blacklist, NULL); win_update_opacity_target(ps, w); @@ -1083,8 +1083,9 @@ void win_update_wintype(session_t *ps, struct managed_win *w) { w->window_type = WINTYPE_DIALOG; } - if (w->window_type != wtype_old) + if (w->window_type != wtype_old) { win_on_factor_change(ps, w); + } } /** @@ -1099,8 +1100,9 @@ void win_mark_client(session_t *ps, struct managed_win *w, xcb_window_t client) // If the window isn't mapped yet, stop here, as the function will be // called in map_win() - if (w->a.map_state != XCB_MAP_STATE_VIEWABLE) + if (w->a.map_state != XCB_MAP_STATE_VIEWABLE) { return; + } auto e = xcb_request_check( ps->c, xcb_change_window_attributes( @@ -1117,8 +1119,9 @@ void win_mark_client(session_t *ps, struct managed_win *w, xcb_window_t client) win_update_frame_extents(ps, w, client); // Get window group - if (ps->o.track_leader) + if (ps->o.track_leader) { win_update_leader(ps, w); + } // Get window name and class if we are tracking them win_update_name(ps, w); @@ -1366,6 +1369,7 @@ struct win *fill_win(session_t *ps, struct win *w) { .bounding_shape = {0}, .rounded_corners = false, .paint_excluded = false, + .fade_excluded = false, .unredir_if_possible_excluded = false, .prop_shadow = -1, // following 4 are set in win_mark_client @@ -1497,11 +1501,13 @@ void win_update_leader(session_t *ps, struct managed_win *w) { xcb_window_t leader = XCB_NONE; // Read the leader properties - if (ps->o.detect_transient && !leader) + if (ps->o.detect_transient && !leader) { leader = wid_get_prop_window(ps, w->client_win, ps->atoms->aWM_TRANSIENT_FOR); + } - if (ps->o.detect_client_leader && !leader) + if (ps->o.detect_client_leader && !leader) { leader = wid_get_prop_window(ps, w->client_win, ps->atoms->aWM_CLIENT_LEADER); + } win_set_leader(ps, w, leader); @@ -1603,10 +1609,11 @@ static void win_on_focus_change(session_t *ps, struct managed_win *w) { #ifdef CONFIG_DBUS // Send D-Bus signal if (ps->o.dbus) { - if (win_is_focused_raw(ps, w)) + if (win_is_focused_raw(ps, w)) { cdbus_ev_win_focusin(ps, &w->base); - else + } else { cdbus_ev_win_focusout(ps, &w->base); + } } #endif } @@ -1733,13 +1740,15 @@ void win_update_opacity_prop(session_t *ps, struct managed_win *w) { // get frame opacity first w->has_opacity_prop = wid_get_opacity_prop(ps, w->base.id, OPAQUE, &w->opacity_prop); - if (w->has_opacity_prop) + if (w->has_opacity_prop) { // opacity found return; + } - if (ps->o.detect_client_opacity && w->client_win && w->base.id == w->client_win) + if (ps->o.detect_client_opacity && w->client_win && w->base.id == w->client_win) { // checking client opacity not allowed return; + } // get client opacity w->has_opacity_prop = @@ -1771,8 +1780,9 @@ void win_update_frame_extents(session_t *ps, struct managed_win *w, xcb_window_t // If frame_opacity != 1, then frame of this window // is not included in reg_ignore of underneath windows - if (ps->o.frame_opacity == 1 && changed) + if (ps->o.frame_opacity == 1 && changed) { w->reg_ignore_valid = false; + } } log_trace("(%#010x): %d, %d, %d, %d", w->base.id, w->frame_extents.left, @@ -1783,10 +1793,12 @@ void win_update_frame_extents(session_t *ps, struct managed_win *w, xcb_window_t bool win_is_region_ignore_valid(session_t *ps, const struct managed_win *w) { win_stack_foreach_managed(i, &ps->window_stack) { - if (i == w) + if (i == w) { break; - if (!i->reg_ignore_valid) + } + if (!i->reg_ignore_valid) { return false; + } } return true; } @@ -2443,14 +2455,16 @@ win_is_fullscreen_xcb(xcb_connection_t *c, const struct atom *a, const xcb_windo xcb_get_property_cookie_t prop = xcb_get_property(c, 0, w, a->a_NET_WM_STATE, XCB_ATOM_ATOM, 0, 12); xcb_get_property_reply_t *reply = xcb_get_property_reply(c, prop, NULL); - if (!reply) + if (!reply) { return false; + } if (reply->length) { xcb_atom_t *val = xcb_get_property_value(reply); for (uint32_t i = 0; i < reply->length; i++) { - if (val[i] != a->a_NET_WM_STATE_FULLSCREEN) + if (val[i] != a->a_NET_WM_STATE_FULLSCREEN) { continue; + } free(reply); return true; } @@ -2529,8 +2543,10 @@ bool win_check_flags_all(struct managed_win *w, uint64_t flags) { * It's not using w->border_size for performance measures. */ bool win_is_fullscreen(const session_t *ps, const struct managed_win *w) { - if (!ps->o.no_ewmh_fullscreen && win_is_fullscreen_xcb(ps->c, ps->atoms, w->client_win)) + if (!ps->o.no_ewmh_fullscreen && + win_is_fullscreen_xcb(ps->c, ps->atoms, w->client_win)) { return true; + } return rect_is_fullscreen(ps, w->g.x, w->g.y, w->widthb, w->heightb) && (!w->bounding_shaped || w->rounded_corners); } diff --git a/src/win.h b/src/win.h index 6d94573d..eb8b54ae 100644 --- a/src/win.h +++ b/src/win.h @@ -211,6 +211,8 @@ struct managed_win { // Fading-related members /// Override value of window fade state. Set by D-Bus method calls. switch_t fade_force; + /// Whether fading is excluded by the rules. Calculated. + bool fade_excluded; // Frame-opacity-related members /// Current window frame opacity. Affected by window opacity. From f476a78ad1860d65bbabfd686af9f0d988f50a8a Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Fri, 23 Oct 2020 04:11:52 +0100 Subject: [PATCH 4/5] event: don't eagerly add damage for configure notify If a window receives multiple configure notifies in between 2 frames, we add all the in between positions into damage, when we really just need the starting and the end position. Signed-off-by: Yuxuan Shui --- src/event.c | 41 ++++++++++++++++++++++++----------------- src/win.c | 14 ++++++++++++++ src/win_defs.h | 4 +++- 3 files changed, 41 insertions(+), 18 deletions(-) diff --git a/src/event.c b/src/event.c index 70021837..f4defed1 100644 --- a/src/event.c +++ b/src/event.c @@ -212,19 +212,33 @@ static void configure_win(session_t *ps, xcb_configure_notify_event_t *ce) { // If window geometry change, free old extents if (mw->g.x != ce->x || mw->g.y != ce->y || mw->g.width != ce->width || mw->g.height != ce->height || mw->g.border_width != ce->border_width) { - region_t damage, new_extents; - pixman_region32_init(&damage); - pixman_region32_init(&new_extents); - - // Get the old window extents - win_extents(mw, &damage); + // We don't mark the old region as damaged if we have stale + // shape/size/position information. The old region should have + // already been add to damage when the information became stale. + if (!win_check_flags_any( + mw, WIN_FLAGS_SIZE_STALE | WIN_FLAGS_POSITION_STALE)) { + // Mark the old extents as damaged. + // The new extents will be marked damaged when processing + // window flags. + region_t damage; + pixman_region32_init(&damage); + win_extents(mw, &damage); + add_damage(ps, &damage); + pixman_region32_fini(&damage); + } // Queue pending updates win_set_flags(mw, WIN_FLAGS_FACTOR_CHANGED); ps->pending_updates = true; - mw->g.x = ce->x; - mw->g.y = ce->y; + // At least one of the following if's is true + if (mw->g.x != ce->x || mw->g.y != ce->y) { + log_trace("Window position changed, %dx%d -> %dx%d", + mw->g.x, mw->g.y, ce->x, ce->y); + mw->g.x = ce->x; + mw->g.y = ce->y; + win_set_flags(mw, WIN_FLAGS_POSITION_STALE); + } if (mw->g.width != ce->width || mw->g.height != ce->height || mw->g.border_width != ce->border_width) { @@ -236,14 +250,6 @@ static void configure_win(session_t *ps, xcb_configure_notify_event_t *ce) { win_set_flags(mw, WIN_FLAGS_SIZE_STALE); } - win_extents(mw, &new_extents); - // Mark the union of the old and new extents as damaged - pixman_region32_union(&damage, &damage, &new_extents); - add_damage(ps, &damage); - - pixman_region32_fini(&damage); - pixman_region32_fini(&new_extents); - // Recalculate which screen this window is on win_update_screen(ps->xinerama_nscrs, ps->xinerama_scr_regs, mw); } @@ -611,8 +617,9 @@ static inline void repair_win(session_t *ps, struct managed_win *w) { } // Remove the part in the damage area that could be ignored - if (w->reg_ignore && win_is_region_ignore_valid(ps, w)) + if (w->reg_ignore && win_is_region_ignore_valid(ps, w)) { pixman_region32_subtract(&parts, &parts, w->reg_ignore); + } add_damage(ps, &parts); pixman_region32_fini(&parts); diff --git a/src/win.c b/src/win.c index e78e2d3a..a2908054 100644 --- a/src/win.c +++ b/src/win.c @@ -400,6 +400,7 @@ static void win_update_properties(session_t *ps, struct managed_win *w) { win_clear_all_properties_stale(w); } +/// Handle non-image flags. This phase might set IMAGES_STALE flags void win_process_update_flags(session_t *ps, struct managed_win *w) { if (win_check_flags_all(w, WIN_FLAGS_MAPPED)) { map_win_start(ps, w); @@ -413,12 +414,19 @@ void win_process_update_flags(session_t *ps, struct managed_win *w) { win_clear_flags(w, WIN_FLAGS_CLIENT_STALE); } + bool damaged = false; if (win_check_flags_all(w, WIN_FLAGS_SIZE_STALE)) { win_on_win_size_change(ps, w); win_update_bounding_shape(ps, w); + damaged = true; win_clear_flags(w, WIN_FLAGS_SIZE_STALE); } + if (win_check_flags_all(w, WIN_FLAGS_POSITION_STALE)) { + damaged = true; + win_clear_flags(w, WIN_FLAGS_POSITION_STALE); + } + if (win_check_flags_all(w, WIN_FLAGS_PROPERTY_STALE)) { win_update_properties(ps, w); win_clear_flags(w, WIN_FLAGS_PROPERTY_STALE); @@ -429,6 +437,12 @@ void win_process_update_flags(session_t *ps, struct managed_win *w) { win_on_factor_change(ps, w); win_clear_flags(w, WIN_FLAGS_FACTOR_CHANGED); } + + // Add damage, has to be done last so the window has the latest geometry + // information. + if (damaged) { + add_damage_from_win(ps, w); + } } void win_process_image_flags(session_t *ps, struct managed_win *w) { diff --git a/src/win_defs.h b/src/win_defs.h index 48ed6512..6a95a664 100644 --- a/src/win_defs.h +++ b/src/win_defs.h @@ -89,8 +89,10 @@ enum win_flags { WIN_FLAGS_PROPERTY_STALE = 128, /// this window has an unhandled size/shape change WIN_FLAGS_SIZE_STALE = 256, + /// this window has an unhandled position (i.e. x and y) change + WIN_FLAGS_POSITION_STALE = 512, /// need better name for this, is set when some aspects of the window changed - WIN_FLAGS_FACTOR_CHANGED = 512, + WIN_FLAGS_FACTOR_CHANGED = 1024, }; static const uint64_t WIN_FLAGS_IMAGES_STALE = From e53ac7a6f9bac28c9028ae2ddf3dbfb24abfc772 Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Fri, 23 Oct 2020 04:34:05 +0100 Subject: [PATCH 5/5] event, win: delayed handling of shape notify Signed-off-by: Yuxuan Shui --- src/event.c | 21 +++++++++------------ src/win.c | 9 +++++++-- src/win.h | 6 +++--- src/win_defs.h | 1 + 4 files changed, 20 insertions(+), 17 deletions(-) diff --git a/src/event.c b/src/event.c index f4defed1..7442c0e2 100644 --- a/src/event.c +++ b/src/event.c @@ -653,19 +653,16 @@ static inline void ev_shape_notify(session_t *ps, xcb_shape_notify_event_t *ev) * seemingly BadRegion errors would be triggered * if we attempt to rebuild border_size */ - // Mark the old border_size as damaged - region_t tmp = win_get_bounding_shape_global_by_val(w); - add_damage(ps, &tmp); - pixman_region32_fini(&tmp); - - win_update_bounding_shape(ps, w); - - // Mark the new border_size as damaged - tmp = win_get_bounding_shape_global_by_val(w); - add_damage(ps, &tmp); - pixman_region32_fini(&tmp); - + // Mark the old bounding shape as damaged + if (!win_check_flags_any(w, WIN_FLAGS_SIZE_STALE | WIN_FLAGS_POSITION_STALE)) { + region_t tmp = win_get_bounding_shape_global_by_val(w); + add_damage(ps, &tmp); + pixman_region32_fini(&tmp); + } w->reg_ignore_valid = false; + + win_set_flags(w, WIN_FLAGS_SIZE_STALE); + ps->pending_updates = true; } static inline void diff --git a/src/win.c b/src/win.c index a2908054..99e79679 100644 --- a/src/win.c +++ b/src/win.c @@ -255,6 +255,9 @@ gen_by_val(win_get_region_frame_local); void add_damage_from_win(session_t *ps, const struct managed_win *w) { // XXX there was a cached extents region, investigate // if that's better + + // TODO(yshui) use the bounding shape when the window is shaped, otherwise the + // damage would be excessive region_t extents; pixman_region32_init(&extents); win_extents(w, &extents); @@ -1681,8 +1684,9 @@ gen_by_val(win_extents); * Mark the window shape as updated */ void win_update_bounding_shape(session_t *ps, struct managed_win *w) { - if (ps->shape_exists) + if (ps->shape_exists) { w->bounding_shaped = win_bounding_shaped(ps, w->base.id); + } pixman_region32_clear(&w->bounding_shape); // Start with the window rectangular region @@ -1700,8 +1704,9 @@ void win_update_bounding_shape(session_t *ps, struct managed_win *w) { ps->c, xcb_shape_get_rectangles(ps->c, w->base.id, XCB_SHAPE_SK_BOUNDING), NULL); - if (!r) + if (!r) { break; + } xcb_rectangle_t *xrects = xcb_shape_get_rectangles_rectangles(r); int nrects = xcb_shape_get_rectangles_rectangles_length(r); diff --git a/src/win.h b/src/win.h index eb8b54ae..3439acf2 100644 --- a/src/win.h +++ b/src/win.h @@ -434,7 +434,7 @@ void win_set_property_stale(struct managed_win *w, xcb_atom_t prop); /// Free all resources in a struct win void free_win_res(session_t *ps, struct managed_win *w); -static inline region_t win_get_bounding_shape_global_by_val(struct managed_win *w) { +static inline region_t attr_unused win_get_bounding_shape_global_by_val(struct managed_win *w) { region_t ret; pixman_region32_init(&ret); pixman_region32_copy(&ret, &w->bounding_shape); @@ -446,7 +446,7 @@ static inline region_t win_get_bounding_shape_global_by_val(struct managed_win * * Calculate the extents of the frame of the given window based on EWMH * _NET_FRAME_EXTENTS and the X window border width. */ -static inline margin_t attr_pure win_calc_frame_extents(const struct managed_win *w) { +static inline margin_t attr_pure attr_unused win_calc_frame_extents(const struct managed_win *w) { margin_t result = w->frame_extents; result.top = max2(result.top, w->g.border_width); result.left = max2(result.left, w->g.border_width); @@ -458,7 +458,7 @@ static inline margin_t attr_pure win_calc_frame_extents(const struct managed_win /** * Check whether a window has WM frames. */ -static inline bool attr_pure win_has_frame(const struct managed_win *w) { +static inline bool attr_pure attr_unused win_has_frame(const struct managed_win *w) { return w->g.border_width || w->frame_extents.top || w->frame_extents.left || w->frame_extents.right || w->frame_extents.bottom; } diff --git a/src/win_defs.h b/src/win_defs.h index 6a95a664..e032bc74 100644 --- a/src/win_defs.h +++ b/src/win_defs.h @@ -87,6 +87,7 @@ enum win_flags { WIN_FLAGS_MAPPED = 64, /// this window has properties which needs to be updated WIN_FLAGS_PROPERTY_STALE = 128, + // TODO(yshui) _maybe_ split SIZE_STALE into SIZE_STALE and SHAPE_STALE /// this window has an unhandled size/shape change WIN_FLAGS_SIZE_STALE = 256, /// this window has an unhandled position (i.e. x and y) change