Skip to content

Commit

Permalink
Merge pull request #1202 from yshui/remove-shadow-flags
Browse files Browse the repository at this point in the history
  • Loading branch information
@yshui
yshui authored Feb 17, 2024
2 parents c23dbe6 + 8640933 commit 664f8b8
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 72 deletions.
9 changes: 8 additions & 1 deletion src/backend/backend.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,14 @@ bool paint_all_new(session_t *ps, struct managed_win *const t) {

// Draw shadow on target
if (w->shadow) {
assert(!(w->flags & WIN_FLAGS_SHADOW_NONE));
if (w->shadow_image == NULL) {
struct color shadow_color = {.red = ps->o.shadow_red,
.green = ps->o.shadow_green,
.blue = ps->o.shadow_blue,
.alpha = ps->o.shadow_opacity};
win_bind_shadow(ps->backend_data, w, shadow_color,
ps->shadow_context);
}
// Clip region for the shadow
// reg_shadow \in reg_paint
auto reg_shadow = win_extents_by_val(w);
Expand Down
6 changes: 3 additions & 3 deletions src/picom.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -628,7 +628,7 @@ static void destroy_backend(session_t *ps) {
// Unmapped windows could still have shadow images, but not pixmap
// images
assert(!w->win_image || w->state != WSTATE_UNMAPPED);
if (win_check_flags_any(w, WIN_FLAGS_IMAGES_STALE) &&
if (win_check_flags_any(w, WIN_FLAGS_PIXMAP_STALE) &&
w->state == WSTATE_MAPPED) {
log_warn("Stale flags set for mapped window %#010x "
"during backend destruction",
Expand All @@ -637,7 +637,7 @@ static void destroy_backend(session_t *ps) {
}
// Unmapped windows can still have stale flags set, because their
// stale flags aren't handled until they are mapped.
win_clear_flags(w, WIN_FLAGS_IMAGES_STALE);
win_clear_flags(w, WIN_FLAGS_PIXMAP_STALE);
win_release_images(ps->backend_data, w);
}
free_paint(ps, &w->paint);
Expand Down Expand Up @@ -769,7 +769,7 @@ static bool initialize_backend(session_t *ps) {
log_debug("Marking window %#010x (%s) for update after "
"redirection",
w->base.id, w->name);
win_set_flags(w, WIN_FLAGS_IMAGES_STALE);
win_set_flags(w, WIN_FLAGS_PIXMAP_STALE);
ps->pending_updates = true;
}
}
Expand Down
77 changes: 18 additions & 59 deletions src/win.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -309,12 +309,10 @@ static inline void win_release_pixmap(backend_t *base, struct managed_win *w) {
}
static inline void win_release_shadow(backend_t *base, struct managed_win *w) {
log_debug("Releasing shadow of window %#010x (%s)", w->base.id, w->name);
assert(w->shadow_image);
if (w->shadow_image) {
assert(w->shadow);
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;
}
}

Expand Down Expand Up @@ -385,13 +383,11 @@ bool win_bind_shadow(struct backend_base *b, struct managed_win *w, struct color
"for "
"%#010x (%s)",
w->base.id, w->name);
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);
win_clear_flags(w, WIN_FLAGS_SHADOW_NONE);
return true;
}

Expand All @@ -405,11 +401,7 @@ void win_release_images(struct backend_base *backend, struct managed_win *w) {
win_release_pixmap(backend, w);
}

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);
}

win_release_shadow(backend, w);
win_release_mask(backend, w);
}

Expand Down Expand Up @@ -567,7 +559,7 @@ void win_process_image_flags(session_t *ps, struct managed_win *w) {
}

// Not a loop
while (win_check_flags_any(w, WIN_FLAGS_IMAGES_STALE) &&
while (win_check_flags_any(w, WIN_FLAGS_PIXMAP_STALE) &&
!win_check_flags_all(w, WIN_FLAGS_IMAGE_ERROR)) {
// Image needs to be updated, update it.
if (!ps->backend_data) {
Expand All @@ -589,27 +581,13 @@ void win_process_image_flags(session_t *ps, struct managed_win *w) {
win_bind_pixmap(ps->backend_data, w);
}

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) {
win_bind_shadow(ps->backend_data, w,
(struct color){.red = ps->o.shadow_red,
.green = ps->o.shadow_green,
.blue = ps->o.shadow_blue,
.alpha = ps->o.shadow_opacity},
ps->shadow_context);
}
}

// break here, loop always run only once
break;
}

// Clear stale image flags
if (win_check_flags_any(w, WIN_FLAGS_IMAGES_STALE)) {
win_clear_flags(w, WIN_FLAGS_IMAGES_STALE);
if (win_check_flags_any(w, WIN_FLAGS_PIXMAP_STALE)) {
win_clear_flags(w, WIN_FLAGS_PIXMAP_STALE);
}
}

Expand Down Expand Up @@ -937,50 +915,32 @@ static void win_set_shadow(session_t *ps, struct managed_win *w, bool shadow_new
pixman_region32_init(&extents);
win_extents(w, &extents);

// Apply the shadow change
w->shadow = shadow_new;

if (ps->redirected) {
// Add damage for shadow change

// Window extents need update on shadow state change
// Shadow geometry currently doesn't change on shadow state change
// calc_shadow_geometry(ps, w);

// Note: because the release and creation of the shadow images are
// delayed. When multiple shadow changes happen in a row, without
// rendering phase between them, there could be a stale shadow
// image attached to the window even if w->shadow was previously
// false. And vice versa. So we check the STALE flag before
// asserting the existence of the shadow image.
if (w->shadow) {
if (shadow_new) {
// Mark the new extents as damaged if the shadow is added
assert(!w->shadow_image ||
win_check_flags_all(w, WIN_FLAGS_SHADOW_STALE) ||
ps->o.legacy_backends);
assert(!w->shadow_image);
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 ||
win_check_flags_all(w, WIN_FLAGS_SHADOW_STALE) ||
ps->o.legacy_backends);
add_damage(ps, &extents);
win_release_shadow(ps->backend_data, w);
}

// Delayed update of shadow image
// By setting WIN_FLAGS_SHADOW_STALE, we ask win_process_flags to
// re-create or release the shadow in based on whether w->shadow
// is set.
win_set_flags(w, WIN_FLAGS_SHADOW_STALE);

// Only set pending_updates if we are redirected. Otherwise change
// of a shadow won't have influence on whether we should redirect.
ps->pending_updates = true;
}

w->shadow = shadow_new;

pixman_region32_fini(&extents);
}

Expand Down Expand Up @@ -1313,8 +1273,9 @@ void win_on_win_size_change(session_t *ps, struct managed_win *w) {
w->state != WSTATE_UNMAPPING);

// Invalidate the shadow we built
win_set_flags(w, WIN_FLAGS_IMAGES_STALE);
win_set_flags(w, WIN_FLAGS_PIXMAP_STALE);
win_release_mask(ps->backend_data, w);
win_release_shadow(ps->backend_data, w);
ps->pending_updates = true;
free_paint(ps, &w->shadow_paint);
}
Expand Down Expand Up @@ -1578,7 +1539,7 @@ struct win *fill_win(session_t *ps, struct win *w) {
.in_openclose = true, // set to false after first map is done,
// true here because window is just created
.reg_ignore_valid = false, // set to true when damaged
.flags = WIN_FLAGS_IMAGES_NONE, // updated by
.flags = WIN_FLAGS_PIXMAP_NONE, // updated by
// property/attributes/etc
// change
.stale_props = NULL,
Expand Down Expand Up @@ -2053,8 +2014,9 @@ void win_update_bounding_shape(session_t *ps, struct managed_win *w) {

// Window shape changed, we should free old wpaint and shadow pict
// log_trace("free out dated pict");
win_set_flags(w, WIN_FLAGS_IMAGES_STALE | WIN_FLAGS_FACTOR_CHANGED);
win_set_flags(w, WIN_FLAGS_PIXMAP_STALE | WIN_FLAGS_FACTOR_CHANGED);
win_release_mask(ps->backend_data, w);
win_release_shadow(ps->backend_data, w);
ps->pending_updates = true;

free_paint(ps, &w->paint);
Expand Down Expand Up @@ -2205,11 +2167,8 @@ static void destroy_win_finish(session_t *ps, struct win *w) {
unmap_win_finish(ps, mw);
}

// Unmapping preserves the shadow image, so free it here
if (!win_check_flags_all(mw, WIN_FLAGS_SHADOW_NONE)) {
assert(mw->shadow_image != NULL);
win_release_shadow(ps->backend_data, mw);
}
// Unmapping might preserve the shadow image, so free it here
win_release_shadow(ps->backend_data, mw);
win_release_mask(ps->backend_data, mw);

// Invalidate reg_ignore of windows below this one
Expand Down Expand Up @@ -2374,7 +2333,7 @@ bool destroy_win_start(session_t *ps, struct win *w) {
// window doesn't work leading to an inconsistent state where the
// shadow is refreshed but the flags are stuck in STALE. Do this
// before changing the window state to destroying
win_clear_flags(mw, WIN_FLAGS_IMAGES_STALE);
win_clear_flags(mw, WIN_FLAGS_PIXMAP_STALE);

// If size/shape/position information is stale,
// win_process_update_flags will update them and add the new
Expand Down
9 changes: 0 additions & 9 deletions src/win_defs.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,6 @@ enum win_flags {
WIN_FLAGS_PIXMAP_NONE = 2,
/// there was an error trying to bind the images
WIN_FLAGS_IMAGE_ERROR = 4,
/// shadow is out of date, will be updated in win_process_flags
WIN_FLAGS_SHADOW_STALE = 8,
/// shadow has not been generated
WIN_FLAGS_SHADOW_NONE = 16,
/// the client window needs to be updated
WIN_FLAGS_CLIENT_STALE = 32,
/// the window is mapped by X, we need to call map_win_start for it
Expand All @@ -95,8 +91,3 @@ enum win_flags {
/// need better name for this, is set when some aspects of the window changed
WIN_FLAGS_FACTOR_CHANGED = 1024,
};

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)

0 comments on commit 664f8b8

Please sign in to comment.