From 32754b02625d7c19407af8285348e88fe4763be0 Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Mon, 6 Apr 2020 20:52:32 +0100 Subject: [PATCH 1/5] win: merge win::pending_updates and win::flags These two flags are intended for subtly different things. I can probably justify the distinction, but it's definitely difficult to explain. And there is no obvious benefits to keep them separate. Signed-off-by: Yuxuan Shui --- src/event.c | 2 +- src/picom.c | 23 +++------- src/win.c | 121 ++++++++++++++++++++++++------------------------- src/win.h | 14 ++++-- src/win_defs.h | 9 ++-- 5 files changed, 79 insertions(+), 90 deletions(-) diff --git a/src/event.c b/src/event.c index 87de238..675261d 100644 --- a/src/event.c +++ b/src/event.c @@ -287,7 +287,7 @@ static inline void ev_map_notify(session_t *ps, xcb_map_notify_event_t *ev) { return; } - win_queue_update(w, WIN_UPDATE_MAP); + win_set_flags(w, WIN_FLAGS_MAPPED); // FocusIn/Out may be ignored when the window is unmapped, so we must // recheck focus here diff --git a/src/picom.c b/src/picom.c index b4a4d49..bbade20 100644 --- a/src/picom.c +++ b/src/picom.c @@ -1309,12 +1309,6 @@ static void handle_new_windows(session_t *ps) { } static void refresh_windows(session_t *ps) { - win_stack_foreach_managed_safe(w, &ps->window_stack) { - win_process_updates(ps, w); - } -} - -static void refresh_stale_images(session_t *ps) { win_stack_foreach_managed(w, &ps->window_stack) { win_process_flags(ps, w); } @@ -1351,7 +1345,13 @@ static void handle_pending_updates(EV_P_ struct session *ps) { // Call fill_win on new windows handle_new_windows(ps); - // Process window updates + // Handle screen changes + // This HAS TO be called before refresh_windows, as handle_root_flags + // could call configure_root, which will release images and mark them + // stale. + handle_root_flags(ps); + + // Process window flags refresh_windows(ps); { @@ -1363,15 +1363,6 @@ static void handle_pending_updates(EV_P_ struct session *ps) { free(r); } - // Handle screen changes - // This HAS TO be called before refresh_stale_images, as handle_root_flags - // could call configure_root, which will release images and mark them - // stale. - handle_root_flags(ps); - - // Refresh pixmaps and shadows - refresh_stale_images(ps); - e = xcb_request_check(ps->c, xcb_ungrab_server_checked(ps->c)); if (e) { log_fatal_x_error(e, "failed to ungrab x server"); diff --git a/src/win.c b/src/win.c index a27643c..c999bbd 100644 --- a/src/win.c +++ b/src/win.c @@ -46,9 +46,6 @@ // TODO Make more window states internal struct managed_win_internal { struct managed_win base; - - /// A bit mask of unhandled window updates - uint_fast32_t pending_updates; }; #define OPAQUE (0xffffffff) @@ -247,6 +244,7 @@ static inline void win_release_pixmap(backend_t *base, struct managed_win *w) { if (w->win_image) { base->ops->release_image(base, w->win_image); w->win_image = NULL; + // Bypassing win_set_flags, because `w` might have been destroyed w->flags |= WIN_FLAGS_PIXMAP_NONE; } } @@ -256,6 +254,7 @@ static inline void win_release_shadow(backend_t *base, struct managed_win *w) { if (w->shadow_image) { base->ops->release_image(base, w->shadow_image); w->shadow_image = NULL; + // Bypassing win_set_flags, because `w` might have been destroyed w->flags |= WIN_FLAGS_SHADOW_NONE; } } @@ -276,11 +275,11 @@ static inline bool win_bind_pixmap(struct backend_base *b, struct managed_win *w b->ops->bind_pixmap(b, pixmap, x_get_visual_info(b->c, w->a.visual), true); if (!w->win_image) { log_error("Failed to bind pixmap"); - w->flags |= WIN_FLAGS_IMAGE_ERROR; + win_set_flags(w, WIN_FLAGS_IMAGE_ERROR); return false; } - w->flags &= ~WIN_FLAGS_PIXMAP_NONE; + win_clear_flags(w, WIN_FLAGS_PIXMAP_NONE); return true; } @@ -294,13 +293,13 @@ bool win_bind_shadow(struct backend_base *b, struct managed_win *w, struct color log_error("Failed to bind shadow image, shadow will be disabled for " "%#010x (%s)", w->base.id, w->name); - w->flags |= WIN_FLAGS_SHADOW_NONE; + win_set_flags(w, WIN_FLAGS_SHADOW_NONE); w->shadow = false; return false; } log_debug("New shadow for %#010x (%s)", w->base.id, w->name); - w->flags &= ~WIN_FLAGS_SHADOW_NONE; + win_clear_flags(w, WIN_FLAGS_SHADOW_NONE); return true; } @@ -310,40 +309,38 @@ void win_release_images(struct backend_base *backend, struct managed_win *w) { // But if we are not releasing any images anyway, we don't care about the stale // flags. - if ((w->flags & WIN_FLAGS_PIXMAP_NONE) == 0) { - assert((w->flags & WIN_FLAGS_PIXMAP_STALE) == 0); + if (!win_check_flags_all(w, WIN_FLAGS_PIXMAP_NONE)) { + assert(!win_check_flags_all(w, WIN_FLAGS_PIXMAP_STALE)); win_release_pixmap(backend, w); } - if ((w->flags & WIN_FLAGS_SHADOW_NONE) == 0) { - assert((w->flags & WIN_FLAGS_SHADOW_STALE) == 0); + if (!win_check_flags_all(w, WIN_FLAGS_SHADOW_NONE)) { + assert(!win_check_flags_all(w, WIN_FLAGS_SHADOW_STALE)); win_release_shadow(backend, w); } } void win_process_flags(session_t *ps, struct managed_win *w) { - // Make sure all pending window updates are processed before this. Making this - // assumption simplifies some checks (e.g. whether window is mapped) - assert(((struct managed_win_internal *)w)->pending_updates == 0); - - if (!w->flags || (w->flags & WIN_FLAGS_IMAGE_ERROR) != 0) { - return; + if (win_check_flags_all(w, WIN_FLAGS_MAPPED)) { + map_win_start(ps, w); } + win_clear_flags(w, WIN_FLAGS_MAPPED); // Not a loop - while ((w->flags & WIN_FLAGS_IMAGES_STALE) != 0) { + while (win_check_flags_any(w, WIN_FLAGS_IMAGES_STALE) && + !win_check_flags_all(w, WIN_FLAGS_IMAGE_ERROR)) { // Image needs to be updated, update it. if (!ps->backend_data) { // We are using legacy backend, nothing to do here. break; } - if ((w->flags & WIN_FLAGS_PIXMAP_STALE) != 0) { + if (win_check_flags_all(w, WIN_FLAGS_PIXMAP_STALE)) { // Check to make sure the window is still mapped, 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 ((w->flags & WIN_FLAGS_PIXMAP_NONE) == 0) { + if (!win_check_flags_all(w, WIN_FLAGS_PIXMAP_NONE)) { // Must release images first, otherwise breaks // NVIDIA driver win_release_pixmap(ps->backend_data, w); @@ -351,8 +348,8 @@ void win_process_flags(session_t *ps, struct managed_win *w) { win_bind_pixmap(ps->backend_data, w); } - if ((w->flags & WIN_FLAGS_SHADOW_STALE) != 0) { - if ((w->flags & WIN_FLAGS_SHADOW_NONE) == 0) { + if (win_check_flags_all(w, WIN_FLAGS_SHADOW_STALE)) { + if (!win_check_flags_all(w, WIN_FLAGS_SHADOW_NONE)) { win_release_shadow(ps->backend_data, w); } if (w->shadow) { @@ -370,7 +367,7 @@ void win_process_flags(session_t *ps, struct managed_win *w) { } // Clear stale image flags - w->flags &= ~WIN_FLAGS_IMAGES_STALE; + win_clear_flags(w, WIN_FLAGS_IMAGES_STALE); } /** @@ -694,7 +691,7 @@ static void win_set_shadow(session_t *ps, struct managed_win *w, bool shadow_new w->shadow = shadow_new; assert(!w->shadow_image); assert(!w->win_image); - assert(w->flags & WIN_FLAGS_IMAGES_NONE); + assert(win_check_flags_all(w, WIN_FLAGS_IMAGES_NONE)); return; } @@ -720,14 +717,14 @@ static void win_set_shadow(session_t *ps, struct managed_win *w, bool shadow_new // asserting the existence of the shadow image. if (w->shadow) { // Mark the new extents as damaged if the shadow is added - assert(!w->shadow_image || (w->flags & WIN_FLAGS_SHADOW_STALE) || + assert(!w->shadow_image || win_check_flags_all(w, WIN_FLAGS_SHADOW_STALE) || !ps->o.experimental_backends); pixman_region32_clear(&extents); win_extents(w, &extents); add_damage_from_win(ps, w); } else { // Mark the old extents as damaged if the shadow is removed - assert(w->shadow_image || (w->flags & WIN_FLAGS_SHADOW_STALE) || + assert(w->shadow_image || win_check_flags_all(w, WIN_FLAGS_SHADOW_STALE) || !ps->o.experimental_backends); add_damage(ps, &extents); } @@ -737,7 +734,7 @@ static void win_set_shadow(session_t *ps, struct managed_win *w, bool shadow_new // Delayed update of shadow image // By setting WIN_FLAGS_SHADOW_STALE, we ask win_process_flags to re-create or // release the shaodw in based on whether w->shadow is set. - w->flags |= WIN_FLAGS_SHADOW_STALE; + win_set_flags(w, WIN_FLAGS_SHADOW_STALE); ps->pending_updates = true; } @@ -945,7 +942,7 @@ void win_on_win_size_change(session_t *ps, struct managed_win *w) { // Invalidate the shadow we built if (w->state == WSTATE_MAPPED || w->state == WSTATE_MAPPING || w->state == WSTATE_FADING) { - w->flags |= WIN_FLAGS_IMAGES_STALE; + win_set_flags(w, WIN_FLAGS_IMAGES_STALE); ps->pending_updates = true; } else { assert(w->state == WSTATE_UNMAPPED); @@ -1301,7 +1298,6 @@ struct win *fill_win(session_t *ps, struct win *w) { // Allocate and initialize the new win structure auto new_internal = cmalloc(struct managed_win_internal); auto new = (struct managed_win *)new_internal; - new_internal->pending_updates = 0; // Fill structure // We only need to initialize the part that are not initialized @@ -1599,7 +1595,7 @@ void win_update_bounding_shape(session_t *ps, struct managed_win *w) { // Note we only do this when screen is redirected, because // otherwise win_data is not valid assert(w->state != WSTATE_UNMAPPING && w->state != WSTATE_DESTROYING); - w->flags |= WIN_FLAGS_IMAGES_STALE; + win_set_flags(w, WIN_FLAGS_IMAGES_STALE); ps->pending_updates = true; } free_paint(ps, &w->paint); @@ -1713,7 +1709,7 @@ static void unmap_win_finish(session_t *ps, struct managed_win *w) { free_paint(ps, &w->shadow_paint); // Try again at binding images when the window is mapped next time - w->flags &= ~WIN_FLAGS_IMAGE_ERROR; + win_clear_flags(w, WIN_FLAGS_IMAGE_ERROR); } /// Finish the destruction of a window (e.g. after fading has finished). @@ -1886,14 +1882,15 @@ bool destroy_win_start(session_t *ps, struct win *w) { } if (w->managed) { + // Clear PIXMAP_STALE flag, since the window is destroyed there is no + // pixmap available so STALE doesn't make sense. + // Do this before changing the window state to destroying + win_clear_flags(mw, WIN_FLAGS_PIXMAP_STALE); + // Update state flags of a managed window mw->state = WSTATE_DESTROYING; mw->a.map_state = XCB_MAP_STATE_UNMAPPED; mw->in_openclose = true; - - // Clear PIXMAP_STALE flag, since the window is destroyed there is no - // pixmap available so STALE doesn't make sense. - mw->flags &= ~WIN_FLAGS_PIXMAP_STALE; } // don't need win_ev_stop because the window is gone anyway @@ -1913,7 +1910,6 @@ bool destroy_win_start(session_t *ps, struct win *w) { } void unmap_win_start(session_t *ps, struct managed_win *w) { - auto internal_w = (struct managed_win_internal *)w; assert(w); assert(w->base.managed); assert(w->a._class != XCB_WINDOW_CLASS_INPUT_ONLY); @@ -1926,8 +1922,9 @@ void unmap_win_start(session_t *ps, struct managed_win *w) { } if (unlikely(w->state == WSTATE_UNMAPPING || w->state == WSTATE_UNMAPPED)) { - if (internal_w->pending_updates & WIN_UPDATE_MAP) { - internal_w->pending_updates &= ~(unsigned long)WIN_UPDATE_MAP; + 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\"", @@ -1946,7 +1943,7 @@ void unmap_win_start(session_t *ps, struct managed_win *w) { // Clear PIXMAP_STALE flag, since the window is unmapped there is no pixmap // available so STALE doesn't make sense. - w->flags &= ~WIN_FLAGS_PIXMAP_STALE; + win_clear_flags(w, WIN_FLAGS_PIXMAP_STALE); // don't care about properties anymore win_ev_stop(ps, &w->base); @@ -2053,8 +2050,7 @@ void map_win_start(session_t *ps, struct managed_win *w) { } assert(w->state == WSTATE_UNMAPPED); - assert((w->flags & WIN_FLAGS_IMAGES_NONE) == WIN_FLAGS_IMAGES_NONE || - !ps->o.experimental_backends); + assert(win_check_flags_all(w, WIN_FLAGS_IMAGES_NONE) || !ps->o.experimental_backends); // We stopped processing window size change when we were unmapped, refresh the // size of the window @@ -2148,7 +2144,7 @@ void map_win_start(session_t *ps, struct managed_win *w) { // the window's image will be bound win_update_bounding_shape(ps, w); - assert((w->flags & WIN_FLAGS_IMAGES_STALE) == WIN_FLAGS_IMAGES_STALE); + assert(win_check_flags_all(w, WIN_FLAGS_IMAGES_STALE)); #ifdef CONFIG_DBUS // Send D-Bus signal @@ -2284,31 +2280,33 @@ win_is_fullscreen_xcb(xcb_connection_t *c, const struct atom *a, const xcb_windo return false; } -/// Queue an update on a window. A series of sanity checks are performed -void win_queue_update(struct managed_win *_w, enum win_update update) { - auto w = (struct managed_win_internal *)_w; - assert(popcount(update) == 1); - assert(update == WIN_UPDATE_MAP); // Currently the only supported update - - if (unlikely(_w->state == WSTATE_DESTROYING)) { - log_error("Updates queued on a destroyed window %#010x (%s)", _w->base.id, - _w->name); +/// Set flags on a window. Some sanity checks are performed +void win_set_flags(struct managed_win *w, uint64_t flags) { + if (unlikely(w->state == WSTATE_DESTROYING)) { + log_error("Flags set on a destroyed window %#010x (%s)", w->base.id, w->name); return; } - w->pending_updates |= update; + w->flags |= flags; } -/// Process pending updates on a window. Has to be called in X critical section -void win_process_updates(struct session *ps, struct managed_win *_w) { - assert(ps->server_grabbed); - auto w = (struct managed_win_internal *)_w; - - if (w->pending_updates & WIN_UPDATE_MAP) { - map_win_start(ps, _w); +/// Clear flags on a window. Some sanity checks are performed +void win_clear_flags(struct managed_win *w, uint64_t flags) { + if (unlikely(w->state == WSTATE_DESTROYING)) { + log_error("Flags cleared on a destroyed window %#010x (%s)", w->base.id, + w->name); + return; } - w->pending_updates = 0; + w->flags = w->flags & (~flags); +} + +bool win_check_flags_any(struct managed_win *w, uint64_t flags) { + return (w->flags & flags) != 0; +} + +bool win_check_flags_all(struct managed_win *w, uint64_t flags) { + return (w->flags & flags) == flags; } /** @@ -2364,7 +2362,6 @@ win_stack_find_next_managed(const session_t *ps, const struct list_node *i) { /// Return whether this window is mapped on the X server side bool win_is_mapped_in_x(const struct managed_win *w) { - auto iw = (const struct managed_win_internal *)w; return w->state == WSTATE_MAPPING || w->state == WSTATE_FADING || - w->state == WSTATE_MAPPED || (iw->pending_updates & WIN_UPDATE_MAP); + w->state == WSTATE_MAPPED || (w->flags & WIN_FLAGS_MAPPED); } diff --git a/src/win.h b/src/win.h index d3d9071..5ee4891 100644 --- a/src/win.h +++ b/src/win.h @@ -131,7 +131,7 @@ struct managed_win { /// See above about coordinate systems. region_t bounding_shape; /// Window flags. Definitions above. - int_fast16_t flags; + uint64_t flags; /// The region of screen that will be obscured when windows above is painted, /// in global coordinates. /// We use this to reduce the pixels that needed to be paint when painting @@ -252,12 +252,8 @@ struct managed_win { #endif }; -/// Process pending updates on a window. Has to be called in X critical section -void win_process_updates(struct session *ps, struct managed_win *_w); /// Process pending images flags on a window. Has to be called in X critical section void win_process_flags(session_t *ps, struct managed_win *w); -/// Queue an update on a window. A series of sanity checks are performed -void win_queue_update(struct managed_win *_w, enum win_update update); /// Bind a shadow to the window, with color `c` and shadow kernel `kernel` bool win_bind_shadow(struct backend_base *b, struct managed_win *w, struct color c, struct conv *kernel); @@ -434,6 +430,14 @@ bool win_is_mapped_in_x(const struct managed_win *w); // Find the managed window immediately below `w` in the window stack struct managed_win *attr_pure win_stack_find_next_managed(const session_t *ps, const struct list_node *w); +/// Set flags on a window. Some sanity checks are performed +void win_set_flags(struct managed_win *w, uint64_t flags); +/// Clear flags on a window. Some sanity checks are performed +void win_clear_flags(struct managed_win *w, uint64_t flags); +/// Returns true if any of the flags in `flags` is set +bool win_check_flags_any(struct managed_win *w, uint64_t flags); +/// Returns true if all of the flags in `flags` are set +bool win_check_flags_all(struct managed_win *w, uint64_t flags); /// Free all resources in a struct win void free_win_res(session_t *ps, struct managed_win *w); diff --git a/src/win_defs.h b/src/win_defs.h index 2a18e4b..f458b4b 100644 --- a/src/win_defs.h +++ b/src/win_defs.h @@ -27,11 +27,6 @@ typedef enum { WMODE_SOLID, // The window is opaque including the frame } winmode_t; -/// Pending window updates -enum win_update { - WIN_UPDATE_MAP = 1, -}; - /// Transition table: /// (DESTROYED is when the win struct is destroyed and freed) /// ('o' means in all other cases) @@ -86,9 +81,11 @@ enum win_flags { WIN_FLAGS_SHADOW_STALE = 8, /// shadow has not been generated WIN_FLAGS_SHADOW_NONE = 16, + /// the window is mapped by X, we need to call map_win_start for it + WIN_FLAGS_MAPPED = 64, }; -static const int_fast16_t WIN_FLAGS_IMAGES_STALE = +static const uint64_t WIN_FLAGS_IMAGES_STALE = WIN_FLAGS_PIXMAP_STALE | WIN_FLAGS_SHADOW_STALE; #define WIN_FLAGS_IMAGES_NONE (WIN_FLAGS_PIXMAP_NONE | WIN_FLAGS_SHADOW_NONE) From ad8632b01758ba5a92246892733120a0523979c6 Mon Sep 17 00:00:00 2001 From: Bernd Busse Date: Fri, 20 Dec 2019 20:17:38 +0100 Subject: [PATCH 2/5] New backends: smoothly fade blur-texture on fade-in/-out * Add new field `opacity_target_old` to `struct managed_win` for tracking relevant `opacity_target` changes. * Smoothly fade blur-texture opacity on window opacity changes (based on window opacity), when the window was or will be fully transparent (`w->opacity ~< 0.004`). * Fixed alpha-clipping of the blur-texture when using `inactive-opacity` or repeatedly setting window opacity with large fade intervals (should fix #314). --- src/backend/backend.c | 24 +++++++++++++-- src/event.c | 9 +----- src/win.c | 69 +++++++++++++++++++++++++++++++++++-------- src/win.h | 3 ++ src/win_defs.h | 4 +-- 5 files changed, 84 insertions(+), 25 deletions(-) diff --git a/src/backend/backend.c b/src/backend/backend.c index bded608..910c49c 100644 --- a/src/backend/backend.c +++ b/src/backend/backend.c @@ -204,16 +204,34 @@ void paint_all_new(session_t *ps, struct managed_win *t, bool ignore_damage) { // itself is not opaque, only the frame is. double blur_opacity = 1; - if (w->state == WSTATE_MAPPING) { + if (w->opacity < (1.0 / MAX_ALPHA)) { + // Hide blur for fully transparent windows. + blur_opacity = 0; + } else if (w->state == WSTATE_MAPPING) { // Gradually increase the blur intensity during // fading in. + assert(w->opacity <= w->opacity_target); blur_opacity = w->opacity / w->opacity_target; } else if (w->state == WSTATE_UNMAPPING || w->state == WSTATE_DESTROYING) { // Gradually decrease the blur intensity during // fading out. - blur_opacity = - w->opacity / win_calc_opacity_target(ps, w, true); + assert(w->opacity <= w->opacity_target_old); + blur_opacity = w->opacity / w->opacity_target_old; + } else if (w->state == WSTATE_FADING) { + if (w->opacity < w->opacity_target && + w->opacity_target_old < (1.0 / MAX_ALPHA)) { + // Gradually increase the blur intensity during + // fading in. + assert(w->opacity <= w->opacity_target); + blur_opacity = w->opacity / w->opacity_target; + } else if (w->opacity > w->opacity_target && + w->opacity_target < (1.0 / MAX_ALPHA)) { + // Gradually decrease the blur intensity during + // fading out. + assert(w->opacity <= w->opacity_target_old); + blur_opacity = w->opacity / w->opacity_target_old; + } } assert(blur_opacity >= 0 && blur_opacity <= 1); diff --git a/src/event.c b/src/event.c index 675261d..c6e1179 100644 --- a/src/event.c +++ b/src/event.c @@ -482,14 +482,7 @@ static inline void ev_property_notify(session_t *ps, xcb_property_notify_event_t win_update_opacity_prop(ps, w); // we cannot receive OPACITY change when window is destroyed assert(w->state != WSTATE_DESTROYING); - w->opacity_target = win_calc_opacity_target(ps, w, false); - if (w->state == WSTATE_MAPPED) { - // See the winstate_t transition table - w->state = WSTATE_FADING; - } - if (!ps->redirected) { - CHECK(!win_skip_fading(ps, w)); - } + win_update_opacity_target(ps, w); } } diff --git a/src/win.c b/src/win.c index c999bbd..9b89886 100644 --- a/src/win.c +++ b/src/win.c @@ -912,18 +912,7 @@ void win_on_factor_change(session_t *ps, struct managed_win *w) { w->unredir_if_possible_excluded = c2_match(ps, w, ps->o.unredir_if_possible_blacklist, NULL); - auto opacity_target_old = w->opacity_target; - w->opacity_target = win_calc_opacity_target(ps, w, false); - if (opacity_target_old != w->opacity_target && w->state == WSTATE_MAPPED) { - // Only MAPPED can transition to FADING - assert(w->opacity == opacity_target_old); - w->state = WSTATE_FADING; - log_debug("Window %#010x (%s) opactiy %f, opacity target %f", w->base.id, - w->name, w->opacity, w->opacity_target); - if (!ps->redirected) { - CHECK(!win_skip_fading(ps, w)); - } - } + win_update_opacity_target(ps, w); w->reg_ignore_valid = false; } @@ -1939,6 +1928,7 @@ void unmap_win_start(session_t *ps, struct managed_win *w) { w->a.map_state = XCB_MAP_STATE_UNMAPPED; w->state = WSTATE_UNMAPPING; + w->opacity_target_old = fmax(w->opacity_target, w->opacity_target_old); w->opacity_target = win_calc_opacity_target(ps, w, false); // Clear PIXMAP_STALE flag, since the window is unmapped there is no pixmap @@ -2123,6 +2113,7 @@ void map_win_start(session_t *ps, struct managed_win *w) { // XXX We need to make sure that win_data is available // iff `state` is MAPPED w->state = WSTATE_MAPPING; + w->opacity_target_old = 0; w->opacity_target = win_calc_opacity_target(ps, w, false); log_debug("Window %#010x has opacity %f, opacity target is %f", w->base.id, @@ -2158,6 +2149,60 @@ 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 opacity_target_old = w->opacity_target; + w->opacity_target = win_calc_opacity_target(ps, w, false); + + if (opacity_target_old == w->opacity_target) { + return; + } + + if (w->state == WSTATE_MAPPED) { + // Opacity target changed while MAPPED. Transition to FADING. + assert(w->opacity == opacity_target_old); + w->opacity_target_old = opacity_target_old; + w->state = WSTATE_FADING; + log_debug("Window %#010x (%s) opacity %f, opacity target %f, set " + "old target %f", + w->base.id, w->name, w->opacity, w->opacity_target, + w->opacity_target_old); + } else if (w->state == WSTATE_MAPPING) { + // Opacity target changed while fading in. + if (w->opacity >= w->opacity_target) { + // Already reached new target opacity. Transition to + // FADING. + map_win_finish(w); + w->opacity_target_old = fmax(opacity_target_old, w->opacity); + w->state = WSTATE_FADING; + log_debug("Window %#010x (%s) opacity %f already reached " + "new opacity target %f while mapping, set old " + "target %f", + w->base.id, w->name, w->opacity, w->opacity_target, + w->opacity_target_old); + } + } else if (w->state == WSTATE_FADING) { + // Opacity target changed while FADING. + if ((w->opacity < opacity_target_old && w->opacity > w->opacity_target) || + (w->opacity > opacity_target_old && w->opacity < w->opacity_target)) { + // Changed while fading in and will fade out or while + // fading out and will fade in. + w->opacity_target_old = opacity_target_old; + log_debug("Window %#010x (%s) opacity %f already reached " + "new opacity target %f while fading, set " + "old target %f", + w->base.id, w->name, w->opacity, w->opacity_target, + w->opacity_target_old); + } + } + + if (!ps->redirected) { + CHECK(!win_skip_fading(ps, w)); + } +} + /** * Find a managed window from window id in window linked list of the session. */ diff --git a/src/win.h b/src/win.h index 5ee4891..f5ac8bc 100644 --- a/src/win.h +++ b/src/win.h @@ -192,6 +192,8 @@ struct managed_win { double opacity; /// Target window opacity. double opacity_target; + /// Previous window opacity. + double opacity_target_old; /// true if window (or client window, for broken window managers /// not transferring client window's _NET_WM_OPACITY value) has opacity prop bool has_opacity_prop; @@ -287,6 +289,7 @@ void win_set_focused(session_t *ps, struct managed_win *w); bool attr_pure win_should_fade(session_t *ps, const struct managed_win *w); void win_update_prop_shadow_raw(session_t *ps, struct managed_win *w); void win_update_prop_shadow(session_t *ps, struct managed_win *w); +void win_update_opacity_target(session_t *ps, struct managed_win *w); void win_on_factor_change(session_t *ps, struct managed_win *w); /** * Update cache data in struct _win that depends on window size. diff --git a/src/win_defs.h b/src/win_defs.h index f458b4b..f8494bb 100644 --- a/src/win_defs.h +++ b/src/win_defs.h @@ -40,8 +40,8 @@ typedef enum { /// | DESTROYING | - | o | - | - | - | - | Fading | /// | | | | | | | |finished | /// +-------------+---------+----------+-------+-------+--------+--------+---------+ -/// | MAPPING | Window | Window | o | - | - | Fading | - | -/// | |unmapped |destroyed | | | |finished| | +/// | MAPPING | Window | Window | o |Opacity| - | Fading | - | +/// | |unmapped |destroyed | |change | |finished| | /// +-------------+---------+----------+-------+-------+--------+--------+---------+ /// | FADING | Window | Window | - | o | - | Fading | - | /// | |unmapped |destroyed | | | |finished| | From 132332c483493834883d4b6370c55953b93f9e5f Mon Sep 17 00:00:00 2001 From: Bernd Busse Date: Sun, 5 Apr 2020 14:07:06 +0200 Subject: [PATCH 3/5] win: remove unused parameter `ignore_state` remove unrequired parameter `ignore_state` of `win_calc_opacity_target()` that was always `false` now. --- src/win.c | 13 ++++++------- src/win.h | 4 +--- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/src/win.c b/src/win.c index 9b89886..28d84ed 100644 --- a/src/win.c +++ b/src/win.c @@ -579,18 +579,17 @@ winmode_t win_calc_mode(const struct managed_win *w) { * * @param ps current session * @param w struct _win object representing the window - * @param ignore_state whether window state should be ignored in opacity calculation * * @return target opacity */ -double win_calc_opacity_target(session_t *ps, const struct managed_win *w, bool ignore_state) { +double win_calc_opacity_target(session_t *ps, const struct managed_win *w) { double opacity = 1; - if (w->state == WSTATE_UNMAPPED && !ignore_state) { + if (w->state == WSTATE_UNMAPPED) { // be consistent return 0; } - if ((w->state == WSTATE_UNMAPPING || w->state == WSTATE_DESTROYING) && !ignore_state) { + if (w->state == WSTATE_UNMAPPING || w->state == WSTATE_DESTROYING) { return 0; } // Try obeying opacity property and window type opacity firstly @@ -1929,7 +1928,7 @@ void unmap_win_start(session_t *ps, struct managed_win *w) { w->a.map_state = XCB_MAP_STATE_UNMAPPED; w->state = WSTATE_UNMAPPING; w->opacity_target_old = fmax(w->opacity_target, w->opacity_target_old); - w->opacity_target = win_calc_opacity_target(ps, w, false); + w->opacity_target = win_calc_opacity_target(ps, w); // Clear PIXMAP_STALE flag, since the window is unmapped there is no pixmap // available so STALE doesn't make sense. @@ -2114,7 +2113,7 @@ void map_win_start(session_t *ps, struct managed_win *w) { // iff `state` is MAPPED w->state = WSTATE_MAPPING; w->opacity_target_old = 0; - w->opacity_target = win_calc_opacity_target(ps, w, false); + w->opacity_target = win_calc_opacity_target(ps, w); log_debug("Window %#010x has opacity %f, opacity target is %f", w->base.id, w->opacity, w->opacity_target); @@ -2154,7 +2153,7 @@ void map_win_start(session_t *ps, struct managed_win *w) { */ void win_update_opacity_target(session_t *ps, struct managed_win *w) { auto opacity_target_old = w->opacity_target; - w->opacity_target = win_calc_opacity_target(ps, w, false); + w->opacity_target = win_calc_opacity_target(ps, w); if (opacity_target_old == w->opacity_target) { return; diff --git a/src/win.h b/src/win.h index f5ac8bc..d6778b6 100644 --- a/src/win.h +++ b/src/win.h @@ -310,12 +310,10 @@ bool win_get_class(session_t *ps, struct managed_win *w); * * @param ps current session * @param w struct _win object representing the window - * @param ignore_state whether window state should be ignored in opacity calculation * * @return target opacity */ -double attr_pure win_calc_opacity_target(session_t *ps, const struct managed_win *w, - bool ignore_state); +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 *); /** From cd4913876fa9c6b260460a9b3a09dfb8b5340af8 Mon Sep 17 00:00:00 2001 From: Bernd Busse Date: Tue, 7 Apr 2020 13:34:50 +0200 Subject: [PATCH 4/5] gitignore: exclude python cache for testcases --- .gitignore | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.gitignore b/.gitignore index 7edc5b7..63ff3ad 100644 --- a/.gitignore +++ b/.gitignore @@ -52,3 +52,5 @@ doxygen/ /src/backtrace-symbols.[ch] /compton*.trace *.orig +/tests/log +/tests/testcases/__pycache__/ From c1d985eb7c0371991d29a7aa638d221da7e06e3a Mon Sep 17 00:00:00 2001 From: Bernd Busse Date: Tue, 7 Apr 2020 13:35:06 +0200 Subject: [PATCH 5/5] tests: update testcase for #314 Handle more (hopefully all) edge-cases when updating `opacity_target_old`: * `MAPPING` transitions to `FADING` if new target is reached. Otherwise no update. * `FADING` clamps to current `opacity` if old target is too small. * `UNMAPPING`/`DESTROYING` clamps to current `opacity` if old target is too small. --- tests/configs/issue314.conf | 2 +- tests/run_tests.sh | 2 ++ tests/testcases/issue314.py | 13 ++++---- tests/testcases/issue314_2.py | 46 +++++++++++++++++++++++++ tests/testcases/issue314_3.py | 63 +++++++++++++++++++++++++++++++++++ 5 files changed, 118 insertions(+), 8 deletions(-) create mode 100755 tests/testcases/issue314_2.py create mode 100755 tests/testcases/issue314_3.py diff --git a/tests/configs/issue314.conf b/tests/configs/issue314.conf index ce71a3e..bb3c4cb 100644 --- a/tests/configs/issue314.conf +++ b/tests/configs/issue314.conf @@ -1,5 +1,5 @@ fading = true -fade-in-step = 1 +fade-in-step = 0.01 fade-out-step = 0.01 inactive-opacity = 0 blur-background = true diff --git a/tests/run_tests.sh b/tests/run_tests.sh index 53292ca..9f3c9e4 100755 --- a/tests/run_tests.sh +++ b/tests/run_tests.sh @@ -10,3 +10,5 @@ cd $(dirname $0) ./run_one_test.sh $exe configs/issue239_3.conf testcases/issue239_3.py ./run_one_test.sh $exe configs/issue239_3.conf testcases/issue239_3_norefresh.py ./run_one_test.sh $exe configs/issue314.conf testcases/issue314.py +./run_one_test.sh $exe configs/issue314.conf testcases/issue314_2.py +./run_one_test.sh $exe configs/issue314.conf testcases/issue314_3.py diff --git a/tests/testcases/issue314.py b/tests/testcases/issue314.py index de7226b..ee29fc4 100755 --- a/tests/testcases/issue314.py +++ b/tests/testcases/issue314.py @@ -12,11 +12,11 @@ visual = setup.roots[0].root_visual depth = setup.roots[0].root_depth x = xproto.xprotoExtension(conn) -# issue 239 is caused by a window gaining a shadow during its fade-out transition +# issue 314 is caused by changing a windows target opacity during its fade-in/-out transition wid1 = conn.generate_id() print("Window 1: ", hex(wid1)) wid2 = conn.generate_id() -print("Window 1: ", hex(wid2)) +print("Window 2: ", hex(wid2)) # Create windows conn.core.CreateWindowChecked(depth, wid1, root, 0, 0, 100, 100, 0, xproto.WindowClass.InputOutput, visual, 0, []).check() @@ -26,20 +26,19 @@ conn.core.CreateWindowChecked(depth, wid2, root, 0, 0, 100, 100, 0, xproto.Windo set_window_name(conn, wid1, "Test window 1") set_window_name(conn, wid2, "Test window 2") -print("mapping 1") +# Check updating opacity while UNMAPPING/DESTROYING windows +print("Mapping 1") conn.core.MapWindowChecked(wid1).check() -print("mapping 2") +print("Mapping 2") conn.core.MapWindowChecked(wid2).check() - time.sleep(0.5) x.SetInputFocusChecked(0, wid1, xproto.Time.CurrentTime).check() - time.sleep(0.5) # Destroy the windows +print("Destroy 1 while fading out") conn.core.DestroyWindowChecked(wid1).check() - x.SetInputFocusChecked(0, wid2, xproto.Time.CurrentTime).check() time.sleep(1) conn.core.DestroyWindowChecked(wid2).check() diff --git a/tests/testcases/issue314_2.py b/tests/testcases/issue314_2.py new file mode 100755 index 0000000..01192de --- /dev/null +++ b/tests/testcases/issue314_2.py @@ -0,0 +1,46 @@ +#!/usr/bin/env python + +import xcffib.xproto as xproto +import xcffib +import time +from common import set_window_name, trigger_root_configure + +conn = xcffib.connect() +setup = conn.get_setup() +root = setup.roots[0].root +visual = setup.roots[0].root_visual +depth = setup.roots[0].root_depth +x = xproto.xprotoExtension(conn) + +opacity_80 = [int(0xffffffff * 0.8), ] +opacity_single = [int(0xffffffff * 0.002), ] + +# issue 314 is caused by changing a windows target opacity during its fade-in/-out transition +wid1 = conn.generate_id() +print("Window 1: ", hex(wid1)) + +atom = "_NET_WM_WINDOW_OPACITY" +opacity_atom = conn.core.InternAtom(False, len(atom), atom).reply().atom + +# Create windows +conn.core.CreateWindowChecked(depth, wid1, root, 0, 0, 100, 100, 0, xproto.WindowClass.InputOutput, visual, 0, []).check() + +# Set Window names +set_window_name(conn, wid1, "Test window 1") + +# Check updating opacity while MAPPING windows +print("Mapping window") +conn.core.MapWindowChecked(wid1).check() +time.sleep(0.5) + +print("Update opacity while fading in") +conn.core.ChangePropertyChecked(xproto.PropMode.Replace, wid1, opacity_atom, xproto.Atom.CARDINAL, 32, 1, opacity_80).check() +time.sleep(0.2) +conn.core.ChangePropertyChecked(xproto.PropMode.Replace, wid1, opacity_atom, xproto.Atom.CARDINAL, 32, 1, opacity_single).check() +time.sleep(1) + +conn.core.DeletePropertyChecked(wid1, opacity_atom).check() +time.sleep(0.5) + +# Destroy the windows +conn.core.DestroyWindowChecked(wid1).check() diff --git a/tests/testcases/issue314_3.py b/tests/testcases/issue314_3.py new file mode 100755 index 0000000..8e5d62d --- /dev/null +++ b/tests/testcases/issue314_3.py @@ -0,0 +1,63 @@ +#!/usr/bin/env python + +import xcffib.xproto as xproto +import xcffib +import time +from common import set_window_name, trigger_root_configure + +conn = xcffib.connect() +setup = conn.get_setup() +root = setup.roots[0].root +visual = setup.roots[0].root_visual +depth = setup.roots[0].root_depth +x = xproto.xprotoExtension(conn) + +opacity_100 = [0xffffffff, ] +opacity_80 = [int(0xffffffff * 0.8), ] +opacity_single = [int(0xffffffff * 0.002), ] +opacity_0 = [0, ] + +# issue 314 is caused by changing a windows target opacity during its fade-in/-out transition +wid1 = conn.generate_id() +print("Window 1: ", hex(wid1)) + +atom = "_NET_WM_WINDOW_OPACITY" +opacity_atom = conn.core.InternAtom(False, len(atom), atom).reply().atom + +# Create windows +conn.core.CreateWindowChecked(depth, wid1, root, 0, 0, 100, 100, 0, xproto.WindowClass.InputOutput, visual, 0, []).check() + +# Set Window names +set_window_name(conn, wid1, "Test window 1") + +# Check updating opacity while FADING windows +print("Mapping window") +conn.core.MapWindowChecked(wid1).check() +time.sleep(1.2) + +print("Update opacity while fading out") +conn.core.ChangePropertyChecked(xproto.PropMode.Replace, wid1, opacity_atom, xproto.Atom.CARDINAL, 32, 1, opacity_single).check() +time.sleep(0.2) +conn.core.ChangePropertyChecked(xproto.PropMode.Replace, wid1, opacity_atom, xproto.Atom.CARDINAL, 32, 1, opacity_0).check() +time.sleep(1) + +print("Change from fading in to fading out") +conn.core.ChangePropertyChecked(xproto.PropMode.Replace, wid1, opacity_atom, xproto.Atom.CARDINAL, 32, 1, opacity_80).check() +time.sleep(0.5) +conn.core.ChangePropertyChecked(xproto.PropMode.Replace, wid1, opacity_atom, xproto.Atom.CARDINAL, 32, 1, opacity_0).check() +time.sleep(1) + +print("Update opacity while fading in") +conn.core.ChangePropertyChecked(xproto.PropMode.Replace, wid1, opacity_atom, xproto.Atom.CARDINAL, 32, 1, opacity_80).check() +time.sleep(0.2) +conn.core.ChangePropertyChecked(xproto.PropMode.Replace, wid1, opacity_atom, xproto.Atom.CARDINAL, 32, 1, opacity_100).check() +time.sleep(1) + +print("Change from fading out to fading in") +conn.core.ChangePropertyChecked(xproto.PropMode.Replace, wid1, opacity_atom, xproto.Atom.CARDINAL, 32, 1, opacity_0).check() +time.sleep(0.5) +conn.core.ChangePropertyChecked(xproto.PropMode.Replace, wid1, opacity_atom, xproto.Atom.CARDINAL, 32, 1, opacity_80).check() +time.sleep(1) + +# Destroy the windows +conn.core.DestroyWindowChecked(wid1).check()