From 9a29e465a3ebc9c3113a0a6618232b79eee51f65 Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Fri, 22 Mar 2024 02:41:48 +0000 Subject: [PATCH] win: restructure window states Remove transitional window states, such as UNMAPPING, MAPPING, etc. We are trying to generialize animation, keeping all those states for animation/fading will only make things more complicated. Instead the window state now exactly reflect how the window is to the X server, and we check the animatable states to determine the animation progress. Signed-off-by: Yuxuan Shui --- src/dbus.c | 3 +- src/event.c | 62 +++++--- src/picom.c | 60 ++++--- src/win.c | 415 +++++++++++++++++++++---------------------------- src/win.h | 15 +- src/win_defs.h | 46 ++---- 6 files changed, 269 insertions(+), 332 deletions(-) diff --git a/src/dbus.c b/src/dbus.c index 7dc83547f8..f9553723f4 100644 --- a/src/dbus.c +++ b/src/dbus.c @@ -29,6 +29,7 @@ #include "uthash_extra.h" #include "utils.h" #include "win.h" +#include "win_defs.h" #include "dbus.h" @@ -877,7 +878,7 @@ cdbus_process_window_property_get(session_t *ps, DBusMessage *msg, cdbus_window_ if (!strcmp("Mapped", target)) { cdbus_reply(ps, msg, cdbus_append_bool_variant, - (bool[]){win_is_mapped_in_x(w)}); + (bool[]){w->state == WSTATE_MAPPED}); return true; } diff --git a/src/event.c b/src/event.c index 9640fa5041..f76d693c7f 100644 --- a/src/event.c +++ b/src/event.c @@ -21,6 +21,7 @@ #include "region.h" #include "utils.h" #include "win.h" +#include "win_defs.h" #include "x.h" /// Event handling with X is complicated. Handling events with other events possibly @@ -273,18 +274,27 @@ static inline void ev_destroy_notify(session_t *ps, xcb_destroy_notify_event_t * assert(&mw->base == w); mw = NULL; } + + // A window can't be a client window and a top-level window at the same time, + // so only one of `w` and `mw` can be non-NULL assert(w == NULL || mw == NULL); if (w != NULL) { - auto _ attr_unused = destroy_win_start(ps, w); - } else if (mw != NULL) { + destroy_win_start(ps, w); + if (!w->managed || !((struct managed_win *)w)->to_paint) { + // If the window wasn't managed, or was already not rendered, + // we don't need to fade it out. + destroy_win_finish(ps, w); + } + return; + } + if (mw != NULL) { win_unmark_client(ps, mw); win_set_flags(mw, WIN_FLAGS_CLIENT_STALE); ps->pending_updates = true; - } else { - log_debug("Received a destroy notify from an unknown window, %#010x", - ev->window); + return; } + log_debug("Received a destroy notify from an unknown window, %#010x", ev->window); } static inline void ev_map_notify(session_t *ps, xcb_map_notify_event_t *ev) { @@ -308,6 +318,13 @@ static inline void ev_map_notify(session_t *ps, xcb_map_notify_event_t *ev) { } win_set_flags(w, WIN_FLAGS_MAPPED); + // We set `ever_damaged` to false here, instead of in `map_win_start`, + // because we might receive damage events before that function is called + // (which is called when we handle the `WIN_FLAGS_MAPPED` flag), in + // which case `repair_win` will be called, which uses `ever_damaged` so + // it needs to be correct. This also covers the case where the window is + // unmapped before `map_win_start` is called. + w->ever_damaged = false; // FocusIn/Out may be ignored when the window is unmapped, so we must // recheck focus here @@ -349,10 +366,8 @@ static inline void ev_reparent_notify(session_t *ps, xcb_reparent_notify_event_t { auto w = find_win(ps, ev->window); if (w) { - auto ret = destroy_win_start(ps, w); - if (!ret && w->managed) { + if (w->managed) { auto mw = (struct managed_win *)w; - CHECK(win_skip_fading(ps, mw)); // Usually, damage for unmapped windows // are added in `paint_preprocess`, when // a window was painted before and isn't @@ -363,7 +378,17 @@ static inline void ev_reparent_notify(session_t *ps, xcb_reparent_notify_event_t if (mw->to_paint) { add_damage_from_win(ps, mw); } + // Emulating what X server does: a destroyed + // window is always unmapped first. + if (mw->state == WSTATE_MAPPED) { + unmap_win_start(ps, mw); + } } + // Window reparenting is unlike normal window destruction, + // This window is going to be rendered under another + // parent, so we don't fade here. + destroy_win_start(ps, w); + destroy_win_finish(ps, w); } } @@ -378,25 +403,14 @@ static inline void ev_reparent_notify(session_t *ps, xcb_reparent_notify_event_t evmask |= XCB_EVENT_MASK_PROPERTY_CHANGE; } else { auto w_real_top = find_managed_window_or_parent(ps, ev->parent); - if (w_real_top && w_real_top->state != WSTATE_UNMAPPED && - w_real_top->state != WSTATE_UNMAPPING) { + if (w_real_top) { log_debug("Mark window %#010x (%s) as having a stale " "client", w_real_top->base.id, w_real_top->name); win_set_flags(w_real_top, WIN_FLAGS_CLIENT_STALE); ps->pending_updates = true; } else { - if (!w_real_top) { - log_debug("parent %#010x not found", ev->parent); - } else { - // Window is not currently mapped, unmark its - // client to trigger a client recheck when it is - // mapped later. - win_unmark_client(ps, w_real_top); - log_debug("parent %#010x (%s) is in state %d", - w_real_top->base.id, w_real_top->name, - w_real_top->state); - } + log_debug("parent %#010x not found", ev->parent); } } XCB_AWAIT_VOID(xcb_change_window_attributes, ps->c.c, ev->window, @@ -605,7 +619,7 @@ static inline void ev_property_notify(session_t *ps, xcb_property_notify_event_t static inline void repair_win(session_t *ps, struct managed_win *w) { // Only mapped window can receive damages - assert(win_is_mapped_in_x(w)); + assert(w->state == WSTATE_MAPPED || win_check_flags_all(w, WIN_FLAGS_MAPPED)); region_t parts; pixman_region32_init(&parts); @@ -628,6 +642,10 @@ static inline void repair_win(session_t *ps, struct managed_win *w) { free(e); } win_extents(w, &parts); + + // We only binds the window pixmap once the window is damaged. + win_set_flags(w, WIN_FLAGS_PIXMAP_STALE); + ps->pending_updates = true; } else { auto cookie = xcb_damage_subtract(ps->c.c, w->damage, XCB_NONE, ps->damage_ring.x_region); diff --git a/src/picom.c b/src/picom.c index b9afaa02a0..ec26cfe992 100644 --- a/src/picom.c +++ b/src/picom.c @@ -496,22 +496,10 @@ static double fade_timeout(session_t *ps) { * @param steps steps of fading * @return whether we are still in fading mode */ -static bool run_fade(session_t *ps, struct managed_win **_w, unsigned int steps) { +static bool run_fade(struct managed_win **_w, unsigned int steps) { auto w = *_w; log_trace("Process fading for window %s (%#010x), steps: %u", w->name, w->base.id, steps); - if (w->state == WSTATE_MAPPED || w->state == WSTATE_UNMAPPED) { - // We are not fading - assert(w->number_of_animations == 0); - log_trace("|- not animated"); - return false; - } - - if (!win_should_fade(ps, w)) { - log_trace("|- in transition but doesn't need fading"); - animatable_early_stop(&w->opacity); - animatable_early_stop(&w->blur_opacity); - } if (w->number_of_animations == 0) { // We have reached target opacity. // We don't call win_check_fade_finished here because that could destroy @@ -634,10 +622,7 @@ static void rebuild_shadow_exclude_reg(session_t *ps) { static void destroy_backend(session_t *ps) { win_stack_foreach_managed_safe(w, &ps->window_stack) { // Wrapping up fading in progress - if (win_skip_fading(ps, w)) { - // `w` is freed by win_skip_fading - continue; - } + win_skip_fading(w); if (ps->backend_data) { // Unmapped windows could still have shadow images, but not pixmap @@ -653,6 +638,10 @@ static void destroy_backend(session_t *ps) { win_release_images(ps->backend_data, w); } free_paint(ps, &w->paint); + + if (w->state == WSTATE_DESTROYED) { + destroy_win_finish(ps, &w->base); + } } HASH_ITER2(ps->shaders, shader) { @@ -954,12 +943,14 @@ static bool paint_preprocess(session_t *ps, bool *fade_running, bool *animation, } // Run fading - if (run_fade(ps, &w, steps)) { + if (run_fade(&w, steps)) { *fade_running = true; } - if (win_maybe_finalize_fading(ps, w)) { - // the window has been destroyed because fading finished + if (w->state == WSTATE_DESTROYED && w->number_of_animations == 0) { + // the window should be destroyed because it was destroyed + // by X server and now its animations are finished + destroy_win_finish(ps, &w->base); continue; } @@ -992,6 +983,7 @@ static bool paint_preprocess(session_t *ps, bool *fade_running, bool *animation, // w->to_paint remembers whether this window is painted last time const bool was_painted = w->to_paint; const double window_opacity = animatable_get(&w->opacity); + const double blur_opacity = animatable_get(&w->blur_opacity); // Destroy reg_ignore if some window above us invalidated it if (!reg_ignore_valid) { @@ -1005,7 +997,12 @@ static bool paint_preprocess(session_t *ps, bool *fade_running, bool *animation, // Give up if it's not damaged or invisible, or it's unmapped and its // pixmap is gone (for example due to a ConfigureNotify), or when it's // excluded - if (w->state == WSTATE_UNMAPPED) { + if (w->state == WSTATE_UNMAPPED && w->number_of_animations == 0) { + if (window_opacity != 0 || blur_opacity != 0) { + log_warn("Window %#010x (%s) is unmapped but still has " + "opacity", + w->base.id, w->name); + } log_trace("|- is unmapped"); to_paint = false; } else if (unlikely(ps->debug_window != XCB_NONE) && @@ -1013,22 +1010,18 @@ static bool paint_preprocess(session_t *ps, bool *fade_running, bool *animation, w->client_win == ps->debug_window)) { log_trace("|- is the debug window"); to_paint = false; - } else if (!w->ever_damaged && w->state != WSTATE_UNMAPPING && - w->state != WSTATE_DESTROYING) { - // Unmapping clears w->ever_damaged, but the fact that the window - // is fading out means it must have been damaged when it was still - // mapped (because unmap_win_start will skip fading if it wasn't), - // so we still need to paint it. + } else if (!w->ever_damaged) { log_trace("|- has not received any damages"); to_paint = false; } else if (unlikely(w->g.x + w->g.width < 1 || w->g.y + w->g.height < 1 || w->g.x >= ps->root_width || w->g.y >= ps->root_height)) { log_trace("|- is positioned outside of the screen"); to_paint = false; - } else if (unlikely(window_opacity * MAX_ALPHA < 1 && !w->blur_background)) { - /* TODO(yshui) for consistency, even a window has 0 opacity, we - * still probably need to blur its background, so to_paint - * shouldn't be false for them. */ + } else if (unlikely(window_opacity * MAX_ALPHA < 1 && + (!w->blur_background || blur_opacity * MAX_ALPHA < 1))) { + // For consistency, even a window has 0 opacity, we would still + // blur its background. (unless it's background is not blurred, or + // the blur opacity is 0) log_trace("|- has 0 opacity"); to_paint = false; } else if (w->paint_excluded) { @@ -1821,7 +1814,10 @@ static void draw_callback_impl(EV_P_ session_t *ps, int revents attr_unused) { // Using foreach_safe here since skipping fading can cause window to be // freed if it's destroyed. win_stack_foreach_managed_safe(w, &ps->window_stack) { - auto _ attr_unused = win_skip_fading(ps, w); + win_skip_fading(w); + if (w->state == WSTATE_DESTROYED) { + destroy_win_finish(ps, &w->base); + } } } diff --git a/src/win.c b/src/win.c index 0956d18144..09e6a7641c 100644 --- a/src/win.c +++ b/src/win.c @@ -31,6 +31,7 @@ #include "types.h" #include "uthash_extra.h" #include "utils.h" +#include "win_defs.h" #include "x.h" #ifdef CONFIG_DBUS @@ -118,17 +119,6 @@ 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. */ @@ -437,6 +427,9 @@ 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 static void win_update_properties(session_t *ps, struct managed_win *w) { + // we cannot receive property change when window has been destroyed + assert(w->state != WSTATE_DESTROYED); + if (win_fetch_and_unset_property_stale(w, ps->atoms->a_NET_WM_WINDOW_TYPE)) { if (win_update_wintype(&ps->c, ps->atoms, w)) { win_set_flags(w, WIN_FLAGS_FACTOR_CHANGED); @@ -445,8 +438,6 @@ 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_OPACITY)) { win_update_opacity_prop(&ps->c, ps->atoms, w, ps->o.detect_client_opacity); - // we cannot receive OPACITY change when window has been destroyed - assert(w->state != WSTATE_DESTROYING); win_update_opacity_target(ps, w); } @@ -503,33 +494,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) { - // The window can have 3 different states of visibility: - // 1. The window is mapped and visible. - // 2. The window is unmapped and not visible. - // 3. The window is unmapped or destroyed, but still visible (fading out). - // (note the window cannot be in destroyed state in this function) - // - // `was_visible` captures (1) and (3) before the MAPPED flag is processed. - // `win_is_real_visible` captures (1) only. Only window with (1) has a - // pixmap. - // - // It is important that `was_visible` tracks (1) and (3). Because we do not - // process window flags for unmapped windows, so an window fading out won't - // move even if the underlying unmapped window is moved. When the window is - // mapped again when it's still fading out, it should have the same effect - // as a mapped window being moved, meaning we have to add both the previous - // and the new window extents to damage. - bool was_visible = win_is_real_visible(w) || w->state == WSTATE_UNMAPPING; - log_trace("Processing flags for window %#010x (%s), was visible: %d, flags: " + log_trace("Processing flags for window %#010x (%s), was rendered: %d, flags: " "%#" PRIx64, - w->base.id, w->name, was_visible, w->flags); + w->base.id, w->name, w->to_paint, w->flags); if (win_check_flags_all(w, WIN_FLAGS_MAPPED)) { map_win_start(ps, w); win_clear_flags(w, WIN_FLAGS_MAPPED); } - if (!win_is_real_visible(w)) { + if (w->state != WSTATE_MAPPED) { // Window is not mapped, so we ignore all its changes until it's mapped // again. return; @@ -548,7 +522,21 @@ 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) { + // For damage calculation purposes, we don't care if the window + // is mapped in X server, we only care if we rendered it last + // frame. + // + // We do not process window flags for unmapped windows even when + // it was rendered, so an window fading out won't move even if the + // underlying unmapped window is moved. When the window is + // mapped again when it's still fading out, it should have the + // same effect as a mapped window being moved, meaning we have + // to add both the previous and the new window extents to + // damage. + // + // All that is basically me saying what really matters is if the + // window was rendered last frame, not if it's mapped in X server. + if (w->to_paint) { // Mark the old extents of this window as damaged. The new // extents will be marked damaged below, after the window // extents are updated. @@ -608,10 +596,10 @@ void win_process_update_flags(session_t *ps, struct managed_win *w) { } void win_process_image_flags(session_t *ps, struct managed_win *w) { + // Assert that the MAPPED flag is already handled. assert(!win_check_flags_all(w, WIN_FLAGS_MAPPED)); - if (w->state == WSTATE_UNMAPPED || w->state == WSTATE_DESTROYING || - w->state == WSTATE_UNMAPPING) { + if (w->state != WSTATE_MAPPED) { // Flags of invisible windows are processed when they are mapped return; } @@ -630,7 +618,6 @@ void win_process_image_flags(session_t *ps, struct managed_win *w) { // otherwise we won't be able to rebind pixmap after // releasing it, yet we might still need the pixmap for // rendering. - assert(w->state != WSTATE_UNMAPPING && w->state != WSTATE_DESTROYING); if (!win_check_flags_all(w, WIN_FLAGS_PIXMAP_NONE)) { // Must release images first, otherwise breaks // NVIDIA driver @@ -859,13 +846,10 @@ winmode_t win_calc_mode(const struct managed_win *w) { static double win_calc_opacity_target(session_t *ps, const struct managed_win *w) { double opacity = 1; - if (w->state == WSTATE_UNMAPPED) { + if (w->state == WSTATE_UNMAPPED || w->state == WSTATE_DESTROYED) { // be consistent return 0; } - if (w->state == WSTATE_UNMAPPING || w->state == WSTATE_DESTROYING) { - return 0; - } // Try obeying opacity property and window type opacity firstly if (w->has_opacity_prop) { opacity = ((double)w->opacity_prop) / OPAQUE; @@ -892,26 +876,123 @@ static double win_calc_opacity_target(session_t *ps, const struct managed_win *w return opacity; } -static void win_transition_callback(enum transition_event event attr_unused, void *data) { - auto w = (struct managed_win *)data; +/// Finish the unmapping of a window (e.g. after fading has finished). +/// Doesn't free `w` +static void unmap_win_finish(session_t *ps, struct managed_win *w) { + w->reg_ignore_valid = false; + w->state = WSTATE_UNMAPPED; + + // We are in unmap_win, this window definitely was viewable + if (ps->backend_data) { + // Only the pixmap needs to be freed and reacquired when mapping. + // Shadow image can be preserved. + if (!win_check_flags_all(w, WIN_FLAGS_PIXMAP_NONE)) { + win_release_pixmap(ps->backend_data, w); + } + } else { + assert(!w->win_image); + assert(!w->shadow_image); + } + + free_paint(ps, &w->paint); + free_paint(ps, &w->shadow_paint); + + // Try again at binding images when the window is mapped next time + win_clear_flags(w, WIN_FLAGS_IMAGE_ERROR); +} + +struct window_transition_data { + struct managed_win *w; + session_t *ps; + // TODO(yshui) switch to only pass backend_data after the legacy backend removal + // struct backend_base *backend_data; + uint64_t refcount; +}; + +static void win_transition_callback(enum transition_event event attr_unused, void *data_) { + auto data = (struct window_transition_data *)data_; + auto w = data->w; w->number_of_animations--; + if (w->number_of_animations == 0) { + if (w->state == WSTATE_DESTROYED || w->state == WSTATE_UNMAPPED) { + if (animatable_get(&w->opacity) != 0) { + log_warn("Window %#010x (%s) has finished fading out but " + "its opacity is not 0", + w->base.id, w->name); + } + } + if (w->state == WSTATE_UNMAPPED) { + unmap_win_finish(data->ps, data->w); + } + // Destroyed windows are freed in paint_preprocess, this makes managing + // the lifetime of windows easier. + w->in_openclose = false; + } + data->refcount--; + if (data->refcount == 0) { + free(data); + } +} + +/** + * Determine if a window should fade on opacity change. + */ +bool win_should_fade(session_t *ps, const struct managed_win *w) { + // To prevent it from being overwritten by last-paint value if the window + // is + if (w->fade_force != UNSET) { + return w->fade_force; + } + if (ps->o.no_fading_openclose && w->in_openclose) { + return false; + } + if (ps->o.no_fading_destroyed_argb && w->state == WSTATE_DESTROYED && + win_has_alpha(w) && w->client_win && w->client_win != w->base.id) { + // deprecated + return false; + } + if (w->fade_excluded) { + return false; + } + return ps->o.wintype_option[w->window_type].fade; } /// Call `animatable_set_target` on the opacity of a window, with appropriate /// target opacity and duration. -/// -/// @return duration of the fade -static inline unsigned int win_start_fade(session_t *ps, struct managed_win *w) { +static inline void +win_start_fade(session_t *ps, struct managed_win *w, double target_blur_opacity) { double current_opacity = animatable_get(&w->opacity), target_opacity = win_calc_opacity_target(ps, w); double step_size = target_opacity > current_opacity ? ps->o.fade_in_step : ps->o.fade_out_step; unsigned int duration = (unsigned int)(fabs(target_opacity - current_opacity) / step_size); + if (!win_should_fade(ps, w)) { + duration = 0; + } + auto data = ccalloc(1, struct window_transition_data); + data->ps = ps; + data->w = w; + data->refcount = 1; + + animatable_cancel(&w->blur_opacity); // Cancel any ongoing blur animation + // so we can check its current value + + // We want to set the correct `number_of_animations` before calling + // `animatable_set_target`, because it might trigger our callback which will + // decrement `number_of_animations` w->number_of_animations++; + if (target_blur_opacity != w->blur_opacity.start) { + w->number_of_animations++; + data->refcount++; + } + animatable_set_target(&w->opacity, target_opacity, duration, - win_transition_callback, w); - return duration; + win_transition_callback, data); + if (target_blur_opacity != w->blur_opacity.start) { + animatable_set_target(&w->blur_opacity, target_blur_opacity, duration, + win_transition_callback, data); + } } /** @@ -929,29 +1010,6 @@ bool win_should_dim(session_t *ps, const struct managed_win *w) { return false; } -/** - * Determine if a window should fade on opacity change. - */ -bool win_should_fade(session_t *ps, const struct managed_win *w) { - // To prevent it from being overwritten by last-paint value if the window - // is - if (w->fade_force != UNSET) { - return w->fade_force; - } - if (ps->o.no_fading_openclose && w->in_openclose) { - return false; - } - if (ps->o.no_fading_destroyed_argb && w->state == WSTATE_DESTROYING && - win_has_alpha(w) && w->client_win && w->client_win != w->base.id) { - // deprecated - return false; - } - if (w->fade_excluded) { - return false; - } - return ps->o.wintype_option[w->window_type].fade; -} - /** * Reread _COMPTON_SHADOW property from a window. * @@ -981,8 +1039,7 @@ static void win_set_shadow(session_t *ps, struct managed_win *w, bool shadow_new // We don't handle property updates of non-visible windows until they are // mapped. - assert(w->state != WSTATE_UNMAPPED && w->state != WSTATE_DESTROYING && - w->state != WSTATE_UNMAPPING); + assert(w->state == WSTATE_MAPPED); // Keep a copy of window extent before the shadow change. Will be used for // calculation of damaged region @@ -1340,8 +1397,7 @@ void win_on_win_size_change(struct managed_win *w, int shadow_offset_x, // We don't handle property updates of non-visible windows until they are // mapped. - assert(w->state != WSTATE_UNMAPPED && w->state != WSTATE_DESTROYING && - w->state != WSTATE_UNMAPPING); + assert(w->state == WSTATE_MAPPED); } /** @@ -1968,8 +2024,7 @@ void win_update_bounding_shape(struct x_connection *c, struct managed_win *w, bool shape_exists, bool detect_rounded_corners) { // We don't handle property updates of non-visible windows until they are // mapped. - assert(w->state != WSTATE_UNMAPPED && w->state != WSTATE_DESTROYING && - w->state != WSTATE_UNMAPPING); + assert(w->state == WSTATE_MAPPED); pixman_region32_clear(&w->bounding_shape); // Start with the window rectangular region @@ -2127,51 +2182,17 @@ void win_ev_stop(session_t *ps, const struct win *w) { } } -/// Finish the unmapping of a window (e.g. after fading has finished). -/// Doesn't free `w` -static void unmap_win_finish(session_t *ps, struct managed_win *w) { - w->reg_ignore_valid = false; - w->state = WSTATE_UNMAPPED; - - // We are in unmap_win, this window definitely was viewable - if (ps->backend_data) { - // Only the pixmap needs to be freed and reacquired when mapping. - // Shadow image can be preserved. - if (!win_check_flags_all(w, WIN_FLAGS_PIXMAP_NONE)) { - win_release_pixmap(ps->backend_data, w); - } - } else { - assert(!w->win_image); - assert(!w->shadow_image); - } - - free_paint(ps, &w->paint); - free_paint(ps, &w->shadow_paint); - - // Try again at binding images when the window is mapped next time - win_clear_flags(w, WIN_FLAGS_IMAGE_ERROR); -} - /// Finish the destruction of a window (e.g. after fading has finished). /// Frees `w` -static void destroy_win_finish(session_t *ps, struct win *w) { - log_trace("Trying to finish destroying (%#010x)", w->id); +void destroy_win_finish(session_t *ps, struct win *w) { + log_verbose("Trying to finish destroying (%#010x)", w->id); auto next_w = win_stack_find_next_managed(ps, &w->stack_neighbour); list_remove(&w->stack_neighbour); if (w->managed) { auto mw = (struct managed_win *)w; - - if (mw->state != WSTATE_UNMAPPED) { - // Only UNMAPPED state has window resources freed, - // otherwise we need to call unmap_win_finish to free - // them. - // XXX actually we unmap_win_finish only frees the - // rendering resources, we still need to call free_win_res. - // will fix later. - unmap_win_finish(ps, mw); - } + unmap_win_finish(ps, mw); // Unmapping might preserve the shadow image, so free it here win_release_shadow(ps->backend_data, mw); @@ -2215,11 +2236,6 @@ static void destroy_win_finish(session_t *ps, struct win *w) { free(w); } -static void map_win_finish(struct managed_win *w) { - w->in_openclose = false; - w->state = WSTATE_MAPPED; -} - /// Move window `w` so it's before `next` in the list static inline void restack_win(session_t *ps, struct win *w, struct list_node *next) { struct managed_win *mw = NULL; @@ -2307,9 +2323,7 @@ void restack_top(session_t *ps, struct win *w) { /// Start destroying a window. Windows cannot always be destroyed immediately /// because of fading and such. -/// -/// @return whether the window has finished destroying and is freed -bool destroy_win_start(session_t *ps, struct win *w) { +void destroy_win_start(session_t *ps, struct win *w) { auto mw = (struct managed_win *)w; assert(w); @@ -2324,14 +2338,15 @@ bool destroy_win_start(session_t *ps, struct win *w) { // finishes destroying. HASH_DEL(ps->windows, w); - if (!w->managed || mw->state == WSTATE_UNMAPPED) { - // Window is already unmapped, or is an unmanaged window, just - // destroy it - destroy_win_finish(ps, w); - return true; - } - if (w->managed) { + if (mw->state != WSTATE_UNMAPPED) { + // Only UNMAPPED state has window resources freed, + // otherwise we need to call unmap_win_finish to free + // them. + log_warn("Did X server not unmap window %#010x before destroying " + "it?", + w->id); + } // Clear IMAGES_STALE flags since the window is destroyed: Clear // PIXMAP_STALE as there is no pixmap available anymore, so STALE // doesn't make sense. @@ -2363,9 +2378,17 @@ bool destroy_win_start(session_t *ps, struct win *w) { WIN_FLAGS_FACTOR_CHANGED | WIN_FLAGS_CLIENT_STALE); // Update state flags of a managed window - mw->state = WSTATE_DESTROYING; + mw->state = WSTATE_DESTROYED; mw->a.map_state = XCB_MAP_STATE_UNMAPPED; mw->in_openclose = true; + + // We don't initiate animation here, because it should already have been + // started by unmap_win_start, because X automatically unmaps windows + // before destroying them. But we do need to stop animation if + // no_fading_destroyed_windows, or no_fading_openclose is enabled. + if (!win_should_fade(ps, mw)) { + win_skip_fading(mw); + } } // don't need win_ev_stop because the window is gone anyway @@ -2376,12 +2399,10 @@ bool destroy_win_start(session_t *ps, struct win *w) { } #endif - if (!ps->redirected) { + if (!ps->redirected && w->managed) { // Skip transition if we are not rendering - return win_skip_fading(ps, mw); + win_skip_fading(mw); } - - return false; } void unmap_win_start(session_t *ps, struct managed_win *w) { @@ -2391,25 +2412,13 @@ void unmap_win_start(session_t *ps, struct managed_win *w) { log_debug("Unmapping %#010x \"%s\"", w->base.id, w->name); - if (unlikely(w->state == WSTATE_DESTROYING)) { - log_warn("Trying to undestroy a window?"); - assert(false); - } - - bool was_damaged = w->ever_damaged; - w->ever_damaged = false; + assert(w->state != WSTATE_DESTROYED); - if (unlikely(w->state == WSTATE_UNMAPPING || w->state == WSTATE_UNMAPPED)) { - if (win_check_flags_all(w, WIN_FLAGS_MAPPED)) { - // Clear the pending map as this window is now unmapped - win_clear_flags(w, WIN_FLAGS_MAPPED); - } else { - log_warn("Trying to unmapping an already unmapped window " - "%#010x " - "\"%s\"", - w->base.id, w->name); - assert(false); - } + if (unlikely(w->state == WSTATE_UNMAPPED)) { + assert(win_check_flags_all(w, WIN_FLAGS_MAPPED)); + // Window is mapped, but we hadn't had a chance to handle the MAPPED flag. + // Clear the pending map as this window is now unmapped + win_clear_flags(w, WIN_FLAGS_MAPPED); return; } @@ -2417,10 +2426,8 @@ void unmap_win_start(session_t *ps, struct managed_win *w) { // triggered by subsequence Focus{In, Out} event, or by recheck_focus w->a.map_state = XCB_MAP_STATE_UNMAPPED; - w->state = WSTATE_UNMAPPING; - auto duration = win_start_fade(ps, w); - w->number_of_animations++; - animatable_set_target(&w->blur_opacity, 0, duration, win_transition_callback, w); + w->state = WSTATE_UNMAPPED; + win_start_fade(ps, w, 0); #ifdef CONFIG_DBUS // Send D-Bus signal @@ -2429,52 +2436,24 @@ void unmap_win_start(session_t *ps, struct managed_win *w) { } #endif - if (!ps->redirected || !was_damaged) { + if (!ps->redirected || !w->ever_damaged) { // If we are not redirected, we skip fading because we aren't // rendering anything anyway. If the window wasn't ever damaged, // it shouldn't be painted either. But a fading out window is // always painted, so we have to skip fading here. - CHECK(!win_skip_fading(ps, w)); + win_skip_fading(w); } } -/** - * Execute fade callback of a window if fading finished. - * - * @return whether the window is destroyed and freed - */ -bool win_maybe_finalize_fading(session_t *ps, struct managed_win *w) { - if (w->state == WSTATE_MAPPED || w->state == WSTATE_UNMAPPED) { - // No fading in progress - assert(w->number_of_animations == 0); - return false; - } - if (w->number_of_animations == 0) { - switch (w->state) { - case WSTATE_UNMAPPING: unmap_win_finish(ps, w); return false; - case WSTATE_DESTROYING: destroy_win_finish(ps, &w->base); return true; - case WSTATE_MAPPING: map_win_finish(w); return false; - case WSTATE_FADING: w->state = WSTATE_MAPPED; break; - default: unreachable(); - } - } - - return false; -} - /// Skip the current in progress fading of window, /// transition the window straight to its end state -/// -/// @return whether the window is destroyed and freed -bool win_skip_fading(session_t *ps, struct managed_win *w) { - if (w->state == WSTATE_MAPPED || w->state == WSTATE_UNMAPPED) { - assert(w->number_of_animations == 0); - return false; +void win_skip_fading(struct managed_win *w) { + if (w->number_of_animations == 0) { + return; } log_debug("Skipping fading process of window %#010x (%s)", w->base.id, w->name); animatable_early_stop(&w->opacity); animatable_early_stop(&w->blur_opacity); - return win_maybe_finalize_fading(ps, w); } // TODO(absolutelynothelix): rename to x_update_win_(randr_?)monitor and move to @@ -2510,9 +2489,10 @@ void map_win_start(session_t *ps, struct managed_win *w) { log_debug("Mapping (%#010x \"%s\")", w->base.id, w->name); - assert(w->state != WSTATE_DESTROYING); - if (w->state != WSTATE_UNMAPPED && w->state != WSTATE_UNMAPPING) { - log_warn("Mapping an already mapped window"); + assert(w->state != WSTATE_DESTROYED); + if (w->state == WSTATE_MAPPED) { + log_error("Mapping an already mapped window"); + assert(false); return; } @@ -2531,26 +2511,12 @@ void map_win_start(session_t *ps, struct managed_win *w) { log_debug("Window (%#010x) has type %s", w->base.id, WINTYPES[w->window_type].name); - // XXX We need to make sure that win_data is available - // iff `state` is MAPPED - w->state = WSTATE_MAPPING; - auto duration = win_start_fade(ps, w); - w->number_of_animations++; - animatable_set_target(&w->blur_opacity, 1, duration, win_transition_callback, w); + w->state = WSTATE_MAPPED; + win_start_fade(ps, w, 1); log_debug("Window %#010x has opacity %f, opacity target is %f", w->base.id, animatable_get(&w->opacity), w->opacity.target); - // Cannot set w->ever_damaged = false here, since window mapping could be - // delayed, so a damage event might have already arrived before this - // function is called. But this should be unnecessary in the first place, - // since ever_damaged is set to false in unmap_win_finish anyway. - - // Sets the WIN_FLAGS_IMAGES_STALE flag so later in the critical section - // the window's image will be bound - - win_set_flags(w, WIN_FLAGS_PIXMAP_STALE); - #ifdef CONFIG_DBUS // Send D-Bus signal if (ps->o.dbus) { @@ -2559,7 +2525,7 @@ void map_win_start(session_t *ps, struct managed_win *w) { #endif if (!ps->redirected) { - CHECK(!win_skip_fading(ps, w)); + win_skip_fading(w); } } @@ -2567,7 +2533,8 @@ void map_win_start(session_t *ps, struct managed_win *w) { * Update target window opacity depending on the current state. */ void win_update_opacity_target(session_t *ps, struct managed_win *w) { - auto duration = win_start_fade(ps, w); + win_start_fade(ps, w, w->blur_opacity.target); // We don't want to change + // blur_opacity target if (w->number_of_animations == 0) { return; @@ -2575,23 +2542,9 @@ void win_update_opacity_target(session_t *ps, struct managed_win *w) { log_debug("Window %#010x (%s) opacity %f, opacity target %f, start %f", w->base.id, w->name, animatable_get(&w->opacity), w->opacity.target, w->opacity.start); - if (w->state == WSTATE_MAPPED) { - // Opacity target changed while MAPPED. Transition to FADING. - w->state = WSTATE_FADING; - } else if (w->state == WSTATE_MAPPING) { - // Opacity target changed while fading in, keep the blur_opacity - // in lock step with the opacity - w->number_of_animations++; - animatable_set_target(&w->blur_opacity, w->blur_opacity.target, duration, - win_transition_callback, w); - log_debug("Opacity changed while fading in"); - } else if (w->state == WSTATE_FADING) { - // Opacity target changed while FADING. - log_debug("Opacity changed while already fading"); - } if (!ps->redirected) { - CHECK(!win_skip_fading(ps, w)); + win_skip_fading(w); } } @@ -2619,7 +2572,7 @@ struct managed_win *find_managed_win(session_t *ps, xcb_window_t id) { } auto mw = (struct managed_win *)w; - assert(mw->state != WSTATE_DESTROYING); + assert(mw->state != WSTATE_DESTROYED); return mw; } @@ -2689,7 +2642,7 @@ struct managed_win *find_managed_window_or_parent(session_t *ps, xcb_window_t wi /// Set flags on a window. Some sanity checks are performed void win_set_flags(struct managed_win *w, uint64_t flags) { log_debug("Set flags %" PRIu64 " to window %#010x (%s)", flags, w->base.id, w->name); - if (unlikely(w->state == WSTATE_DESTROYING)) { + if (unlikely(w->state == WSTATE_DESTROYED)) { log_error("Flags set on a destroyed window %#010x (%s)", w->base.id, w->name); return; } @@ -2701,7 +2654,7 @@ void win_set_flags(struct managed_win *w, uint64_t flags) { void win_clear_flags(struct managed_win *w, uint64_t flags) { log_debug("Clear flags %" PRIu64 " from window %#010x (%s)", flags, w->base.id, w->name); - if (unlikely(w->state == WSTATE_DESTROYING)) { + if (unlikely(w->state == WSTATE_DESTROYED)) { log_warn("Flags cleared on a destroyed window %#010x (%s)", w->base.id, w->name); return; @@ -2820,9 +2773,3 @@ win_stack_find_next_managed(const session_t *ps, const struct list_node *w) { } return NULL; } - -/// Return whether this window is mapped on the X server side -bool win_is_mapped_in_x(const struct managed_win *w) { - return w->state == WSTATE_MAPPING || w->state == WSTATE_FADING || - w->state == WSTATE_MAPPED || (w->flags & WIN_FLAGS_MAPPED); -} diff --git a/src/win.h b/src/win.h index 414b96ba80..8198343f0c 100644 --- a/src/win.h +++ b/src/win.h @@ -128,7 +128,10 @@ struct managed_win { const xcb_render_pictforminfo_t *client_pictfmt; /// Window painting mode. winmode_t mode; - /// Whether the window has been damaged at least once. + /// Whether the window has been damaged at least once since it + /// was mapped. Unmapped windows that were previously mapped + /// retain their `ever_damaged` state. Mapping a window resets + /// this. bool ever_damaged; /// Whether the window was damaged after last paint. bool pixmap_damaged; @@ -314,7 +317,7 @@ void map_win_start(struct session *, struct managed_win *); /// Start the destroying of a window. Windows cannot always be destroyed immediately /// because of fading and such. -bool must_use destroy_win_start(session_t *ps, struct win *w); +void destroy_win_start(session_t *ps, struct win *w); /// Release images bound with a window, set the *_NONE flags on the window. Only to be /// used when de-initializing the backend outside of win.c @@ -386,18 +389,16 @@ void restack_bottom(session_t *ps, struct win *w); void restack_top(session_t *ps, struct win *w); /** - * Execute fade callback of a window if fading finished. + * Release a destroyed window that is no longer needed. */ -bool must_use win_maybe_finalize_fading(session_t *ps, struct managed_win *w); +void destroy_win_finish(session_t *ps, struct win *w); // Stop receiving events (except ConfigureNotify, XXX why?) from a window void win_ev_stop(session_t *ps, const struct win *w); /// Skip the current in progress fading of window, /// transition the window straight to its end state -/// -/// @return whether the window is destroyed and freed -bool must_use win_skip_fading(session_t *ps, struct managed_win *w); +void win_skip_fading(struct managed_win *w); /** * Find a managed window from window id in window linked list of the session. */ diff --git a/src/win_defs.h b/src/win_defs.h index dc96a0c5bb..a9d3b5fd4d 100644 --- a/src/win_defs.h +++ b/src/win_defs.h @@ -27,44 +27,18 @@ typedef enum { WMODE_SOLID, // The window is opaque including the frame } winmode_t; -/// Transition table: -/// (DESTROYED is when the win struct is destroyed and freed) -/// ('o' means in all other cases) -/// (Window is created in the UNMAPPED state) -/// +-------------+---------+----------+-------+-------+--------+--------+---------+ -/// | |UNMAPPING|DESTROYING|MAPPING|FADING |UNMAPPED| MAPPED |DESTROYED| -/// +-------------+---------+----------+-------+-------+--------+--------+---------+ -/// | UNMAPPING | o | Window |Window | - | Fading | - | - | -/// | | |destroyed |mapped | |finished| | | -/// +-------------+---------+----------+-------+-------+--------+--------+---------+ -/// | DESTROYING | - | o | - | - | - | - | Fading | -/// | | | | | | | |finished | -/// +-------------+---------+----------+-------+-------+--------+--------+---------+ -/// | MAPPING | Window | Window | o |Opacity| - | Fading | - | -/// | |unmapped |destroyed | |change | |finished| | -/// +-------------+---------+----------+-------+-------+--------+--------+---------+ -/// | FADING | Window | Window | - | o | - | Fading | - | -/// | |unmapped |destroyed | | | |finished| | -/// +-------------+---------+----------+-------+-------+--------+--------+---------+ -/// | UNMAPPED | - | - |Window | - | o | - | Window | -/// | | | |mapped | | | |destroyed| -/// +-------------+---------+----------+-------+-------+--------+--------+---------+ -/// | MAPPED | Window | Window | - |Opacity| - | o | - | -/// | |unmapped |destroyed | |change | | | | -/// +-------------+---------+----------+-------+-------+--------+--------+---------+ +/// The state of a window from Xserver's perspective typedef enum { - // The window is being faded out because it's unmapped. - WSTATE_UNMAPPING, - // The window is being faded out because it's destroyed, - WSTATE_DESTROYING, - // The window is being faded in - WSTATE_MAPPING, - // Window opacity is not at the target level - WSTATE_FADING, - // The window is mapped, no fading is in progress. - WSTATE_MAPPED, - // The window is unmapped, no fading is in progress. + /// The window is unmapped. Equivalent to map-state == XCB_MAP_STATE_UNMAPPED WSTATE_UNMAPPED, + /// The window no longer exists on the X server. + WSTATE_DESTROYED, + /// The window is mapped and viewable. Equivalent to map-state == + /// XCB_MAP_STATE_VIEWABLE + WSTATE_MAPPED, + + // XCB_MAP_STATE_UNVIEWABLE is not represented here because it should not be + // possible for top-level windows. } winstate_t; enum win_flags {