From 95a64acf5a03c7c22a42d40ebcafff33e924792a Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Thu, 3 Dec 2020 23:53:10 +0000 Subject: [PATCH] event, win: fix damage when window is moved while fading out When a window is moved during fade-out, configure_win won't add the old window extent to damage (because it's unmapping), but the new window extent will be added to damage when the window is rendered. And the window will be rendered in its new location. Thus the damage for the old window extent is missing. We could fix this by adding damage in configure_win for unmapping window too. But in general we don't want to move a window while it's fading out. So we shadow the window geometry. The shadow geometry is updated in configure_win, and the update will only propagate to real geometry in win_process_update_flags. This also fix the case where a window is unmapped, moved, then mapped all in the same render cycle. map_win_start is supposed to add the old window extent to damage in that case, but the old window extent is already overwritten when we reach map_win_start. The shadow geometry fixes that. Signed-off-by: Yuxuan Shui --- src/event.c | 43 +++++++++++++++---------------------------- src/win.c | 50 ++++++++++++++++++++++++++++++++++++++++++++------ src/win.h | 16 +++++++++++++--- 3 files changed, 72 insertions(+), 37 deletions(-) diff --git a/src/event.c b/src/event.c index 88bcf31..16baa0c 100644 --- a/src/event.c +++ b/src/event.c @@ -203,26 +203,14 @@ static void configure_win(session_t *ps, xcb_configure_notify_event_t *ce) { restack_above(ps, w, ce->above_sibling); - // 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) { - // 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)) { - if (mw->state != WSTATE_UNMAPPED && mw->state != WSTATE_UNMAPPING && - mw->state != WSTATE_DESTROYING) { - // Mark the old extents as damaged. The new extents will - // be marked damaged when processing window flags. - // If the window is not mapped, we don't care - region_t damage; - pixman_region32_init(&damage); - win_extents(mw, &damage); - add_damage(ps, &damage); - pixman_region32_fini(&damage); - } - } - + // We check against pending_g here, because there might have been multiple + // configure notifies in this cycle, or the window could receive multiple updates + // while it's unmapped. + bool position_changed = mw->pending_g.x != ce->x || mw->pending_g.y != ce->y; + bool size_changed = mw->pending_g.width != ce->width || + mw->pending_g.height != ce->height || + mw->pending_g.border_width != ce->border_width; + if (position_changed || size_changed) { // Queue pending updates win_set_flags(mw, WIN_FLAGS_FACTOR_CHANGED); // TODO(yshui) don't set pending_updates if the window is not @@ -230,21 +218,20 @@ static void configure_win(session_t *ps, xcb_configure_notify_event_t *ce) { ps->pending_updates = true; // At least one of the following if's is true - if (mw->g.x != ce->x || mw->g.y != ce->y) { + if (position_changed) { 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; + mw->pending_g.x = ce->x; + mw->pending_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) { + if (size_changed) { 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; + mw->pending_g.width = ce->width; + mw->pending_g.height = ce->height; + mw->pending_g.border_width = ce->border_width; win_set_flags(mw, WIN_FLAGS_SIZE_STALE); } diff --git a/src/win.c b/src/win.c index 13cc71d..d223a7f 100644 --- a/src/win.c +++ b/src/win.c @@ -120,6 +120,17 @@ static inline xcb_window_t win_get_leader(session_t *ps, struct managed_win *w) return win_get_leader_raw(ps, w, 0); } +/** + * Whether the real content of the window is visible. + * + * A window is not considered "real" visible if it's fading out. Because in that case a + * cached version of the window is displayed. + */ +static inline bool attr_pure win_is_real_visible(const struct managed_win *w) { + return w->state == WSTATE_UNMAPPED || w->state == WSTATE_DESTROYING || + w->state == WSTATE_UNMAPPING; +} + /** * Update focused state of a window. */ @@ -154,8 +165,9 @@ static void win_update_focused(session_t *ps, struct managed_win *w) { * @param leader leader window ID */ static inline void group_on_factor_change(session_t *ps, xcb_window_t leader) { - if (!leader) + if (!leader) { return; + } HASH_ITER2(ps->windows, w) { assert(!w->destroyed); @@ -167,8 +179,6 @@ static inline void group_on_factor_change(session_t *ps, xcb_window_t leader) { win_on_factor_change(ps, mw); } } - - return; } static inline const char *win_get_name_if_managed(const struct win *w) { @@ -421,13 +431,16 @@ static void win_update_properties(session_t *ps, struct managed_win *w) { /// Handle non-image flags. This phase might set IMAGES_STALE flags void win_process_update_flags(session_t *ps, struct managed_win *w) { + // Whether the window was visible before we process the mapped flag. i.e. is the + // window just mapped. + bool was_visible = win_is_real_visible(w); + if (win_check_flags_all(w, WIN_FLAGS_MAPPED)) { map_win_start(ps, w); win_clear_flags(w, WIN_FLAGS_MAPPED); } - if (w->state == WSTATE_UNMAPPED || w->state == WSTATE_DESTROYING || - w->state == WSTATE_UNMAPPING) { + if (win_is_real_visible(w)) { // Flags of invisible windows are processed when they are mapped return; } @@ -440,6 +453,24 @@ void win_process_update_flags(session_t *ps, struct managed_win *w) { } bool damaged = false; + if (win_check_flags_any(w, WIN_FLAGS_SIZE_STALE | WIN_FLAGS_POSITION_STALE)) { + if (was_visible) { + // Mark the old extents of this window as damaged. The new extents + // will be marked damaged below, after the window extents are + // updated. + // + // If the window is just mapped, we don't need to mark the old + // extent as damaged. (It's possible that the window was in fading + // and is interrupted by being mapped. In that case, the fading + // window will be added to damage by map_win_start, so we don't + // need to do it here) + add_damage_from_win(ps, w); + } + + // Update window geometry + w->g = w->pending_g; + } + if (win_check_flags_all(w, WIN_FLAGS_SIZE_STALE)) { win_on_win_size_change(ps, w); win_update_bounding_shape(ps, w); @@ -1513,7 +1544,14 @@ struct win *fill_win(session_t *ps, struct win *w) { free(new); return w; } - new->g = *g; + new->pending_g = (struct win_geometry){ + .x = g->x, + .y = g->y, + .width = g->width, + .height = g->height, + .border_width = g->border_width, + }; + free(g); win_update_screen(ps->xinerama_nscrs, ps->xinerama_scr_regs, new); diff --git a/src/win.h b/src/win.h index 12dc063..acb22e1 100644 --- a/src/win.h +++ b/src/win.h @@ -89,6 +89,15 @@ struct win { /// Always false if `is_new` is true. bool managed : 1; }; + +struct win_geometry { + int16_t x; + int16_t y; + uint16_t width; + uint16_t height; + uint16_t border_width; +}; + struct managed_win { struct win base; /// backend data attached to this window. Only available when @@ -107,9 +116,10 @@ struct managed_win { winstate_t state; /// Window attributes. xcb_get_window_attributes_reply_t a; - /// Reply of xcb_get_geometry, which returns the geometry of the window body, - /// excluding the window border. - xcb_get_geometry_reply_t g; + /// The geometry of the window body, excluding the window border region. + struct win_geometry g; + /// Updated geometry received in events + struct win_geometry pending_g; /// Xinerama screen this window is on. int xinerama_scr; /// Window visual pict format