Skip to content

Commit

Permalink
win: split WIN_FLAGS_IMAGE_STALE
Browse files Browse the repository at this point in the history
Split it into PIXMAP_STALE and SHADOW_STALE, this allows us to update
pixmaps and shadow images separately.

Also added PIXMAP_NONE and SHADOW_NONE, as redundancy to detect logic
errors.

Convenient constants and functions are provided for updating pixmap and
shadow together.

Signed-off-by: Yuxuan Shui <[email protected]>
  • Loading branch information
yshui committed Sep 20, 2019
1 parent 1549997 commit eecee13
Show file tree
Hide file tree
Showing 5 changed files with 143 additions and 66 deletions.
3 changes: 3 additions & 0 deletions src/backend/backend.c
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,8 @@ void paint_all_new(session_t *ps, struct managed_win *t, bool ignore_damage) {
for (auto w = t; w; w = w->prev_trans) {
pixman_region32_subtract(&reg_visible, &ps->screen_reg, w->reg_ignore);
assert(!(w->flags & WIN_FLAGS_IMAGE_ERROR));
assert(!(w->flags & WIN_FLAGS_PIXMAP_STALE));
assert(!(w->flags & WIN_FLAGS_PIXMAP_NONE));

// The bounding shape of the window, in global/target coordinates
// reminder: bounding shape contains the WM frame
Expand Down Expand Up @@ -201,6 +203,7 @@ void paint_all_new(session_t *ps, struct managed_win *t, bool ignore_damage) {

// Draw shadow on target
if (w->shadow) {
assert(!(w->flags & WIN_FLAGS_SHADOW_NONE));
// Clip region for the shadow
// reg_shadow \in reg_paint
auto reg_shadow = win_extents_by_val(w);
Expand Down
11 changes: 7 additions & 4 deletions src/compton.c
Original file line number Diff line number Diff line change
Expand Up @@ -662,7 +662,7 @@ static void destroy_backend(session_t *ps) {

if (ps->backend_data) {
if (w->state == WSTATE_MAPPED) {
win_release_image(ps->backend_data, w);
win_release_images(ps->backend_data, w);
} else {
assert(!w->win_image);
assert(!w->shadow_image);
Expand Down Expand Up @@ -753,9 +753,12 @@ static bool initialize_backend(session_t *ps) {
}
auto w = (struct managed_win *)_w;
if (w->a.map_state == XCB_MAP_STATE_VIEWABLE) {
if (!win_bind_image(ps, w)) {
w->flags |= WIN_FLAGS_IMAGE_ERROR;
}
win_bind_image(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->gaussian_map);
}
}
}
Expand Down
164 changes: 110 additions & 54 deletions src/win.c
Original file line number Diff line number Diff line change
Expand Up @@ -241,78 +241,127 @@ void add_damage_from_win(session_t *ps, const struct managed_win *w) {
}

/// Release the images attached to this window
void win_release_image(backend_t *base, struct managed_win *w) {
log_debug("Releasing image of window %#010x (%s)", w->base.id, w->name);
assert(w->win_image || (w->flags & WIN_FLAGS_IMAGE_ERROR));
static inline void win_release_pixmap(backend_t *base, struct managed_win *w) {
log_debug("Releasing pixmap of window %#010x (%s)", w->base.id, w->name);
assert(w->win_image);
if (w->win_image) {
base->ops->release_image(base, w->win_image);
w->win_image = NULL;
w->flags |= WIN_FLAGS_PIXMAP_NONE;
}
if (w->shadow) {
assert(w->shadow_image || (w->flags & WIN_FLAGS_IMAGE_ERROR));
if (w->shadow_image) {
base->ops->release_image(base, w->shadow_image);
w->shadow_image = NULL;
}
}
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) {
base->ops->release_image(base, w->shadow_image);
w->shadow_image = NULL;
w->flags |= WIN_FLAGS_SHADOW_NONE;
}
}

static inline bool
_win_bind_image(session_t *ps, struct managed_win *w, void **win_image, void **shadow_image) {
auto pixmap = x_new_id(ps->c);
static inline bool win_bind_pixmap(struct backend_base *b, struct managed_win *w) {
assert(!w->win_image);
auto pixmap = x_new_id(b->c);
auto e = xcb_request_check(
ps->c, xcb_composite_name_window_pixmap_checked(ps->c, w->base.id, pixmap));
b->c, xcb_composite_name_window_pixmap_checked(b->c, w->base.id, pixmap));
if (e) {
log_error("Failed to get named pixmap for window %#010x(%s)", w->base.id,
w->name);
free(e);
return false;
}
log_trace("New named pixmap for %#010x (%s) : %#010x", w->base.id, w->name, pixmap);
*win_image = ps->backend_data->ops->bind_pixmap(
ps->backend_data, pixmap, x_get_visual_info(ps->c, w->a.visual), true);
if (!*win_image) {
log_debug("New named pixmap for %#010x (%s) : %#010x", w->base.id, w->name, pixmap);
w->win_image =
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;
return false;
}
if (w->shadow) {
*shadow_image = ps->backend_data->ops->render_shadow(
ps->backend_data, w->widthb, w->heightb, ps->gaussian_map, ps->o.shadow_red,
ps->o.shadow_green, ps->o.shadow_blue, ps->o.shadow_opacity);
if (!*shadow_image) {
log_error("Failed to bind shadow image");
ps->backend_data->ops->release_image(ps->backend_data, *win_image);
*win_image = NULL;
return false;
}
}

w->flags &= ~WIN_FLAGS_PIXMAP_NONE;
return true;
}

bool win_bind_image(session_t *ps, struct managed_win *w) {
assert(!w->win_image && !w->shadow_image);
return _win_bind_image(ps, w, &w->win_image, &w->shadow_image);
static inline bool win_bind_shadow(struct backend_base *b, struct managed_win *w,
struct color c, struct conv *kernel) {
assert(!w->shadow_image);
assert(w->shadow);
w->shadow_image = b->ops->render_shadow(b, w->widthb, w->heightb, kernel, c.red,
c.green, c.blue, c.alpha);
if (!w->shadow_image) {
log_error("Failed to bind shadow image");
w->flags |= WIN_FLAGS_IMAGE_ERROR;
return false;
}

log_debug("New shadow for %#010x (%s)", w->base.id, w->name);
w->flags &= ~WIN_FLAGS_SHADOW_NONE;
return true;
}

bool win_try_rebind_image(session_t *ps, struct managed_win *w) {
log_trace("Freeing old window image");
// Must release first, otherwise breaks NVIDIA driver
win_release_image(ps->backend_data, w);
void win_release_images(struct backend_base *backend, struct managed_win *w) {
// we don't want to consider what we should do if the image we want to release is
// stale (do we clear the stale flags or not?)
assert((w->flags & WIN_FLAGS_IMAGES_STALE) == 0);

return win_bind_image(ps, w);
if ((w->flags & WIN_FLAGS_PIXMAP_NONE) == 0) {
win_release_pixmap(backend, w);
}

if ((w->flags & WIN_FLAGS_SHADOW_NONE) == 0) {
win_release_shadow(backend, w);
}
}

void win_process_flags(struct session *ps, struct managed_win *w) {
if ((w->flags & WIN_FLAGS_IMAGE_STALE) != 0 && (w->flags & WIN_FLAGS_IMAGE_ERROR) == 0) {
void win_bind_image(struct backend_base *backend, struct managed_win *w, struct color c,
struct conv *kernel) {
win_bind_pixmap(backend, w);
if (w->shadow) {
win_bind_shadow(backend, w, c, kernel);
}
}

void win_process_flags(session_t *ps, struct managed_win *w) {
if (!w->flags || (w->flags & WIN_FLAGS_IMAGE_ERROR) != 0) {
return;
}

// Not a loop
while ((w->flags & WIN_FLAGS_IMAGES_STALE) != 0) {
// Image needs to be updated, update it.
w->flags &= ~WIN_FLAGS_IMAGE_STALE;
if (w->state != WSTATE_UNMAPPING && w->state != WSTATE_DESTROYING &&
ps->backend_data) {
// Rebind image only when the window does have an image
// available
if (!win_try_rebind_image(ps, w)) {
w->flags |= WIN_FLAGS_IMAGE_ERROR;
if (w->state == WSTATE_UNMAPPING || w->state == WSTATE_DESTROYING ||
!ps->backend_data) {
// Window is already gone, or we are using the legacy backend
// we cannot rebind image
break;
}

// Must release images first, otherwise breaks NVIDIA driver
if ((w->flags & WIN_FLAGS_PIXMAP_STALE) != 0) {
if ((w->flags & WIN_FLAGS_PIXMAP_NONE) == 0) {
win_release_pixmap(ps->backend_data, w);
}
win_bind_pixmap(ps->backend_data, w);
}

if ((w->flags & WIN_FLAGS_SHADOW_STALE) != 0) {
if ((w->flags & WIN_FLAGS_SHADOW_NONE) == 0) {
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->gaussian_map);
}
}

// Flags are cleared here, loop always run only once
w->flags &= ~WIN_FLAGS_IMAGES_STALE;
}
}

Expand Down Expand Up @@ -865,7 +914,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_IMAGE_STALE;
w->flags |= WIN_FLAGS_IMAGES_STALE;
ps->pending_updates = true;
} else {
assert(w->state == WSTATE_UNMAPPED);
Expand Down Expand Up @@ -1097,7 +1146,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 = 0, // updated by property/attributes/etc change
.flags = WIN_FLAGS_IMAGES_NONE, // updated by property/attributes/etc change

// Runtime variables, updated by dbus
.fade_force = UNSET,
Expand Down Expand Up @@ -1504,7 +1553,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_IMAGE_STALE;
w->flags |= WIN_FLAGS_IMAGES_STALE;
ps->pending_updates = true;
}
free_paint(ps, &w->paint);
Expand Down Expand Up @@ -1608,12 +1657,13 @@ static void finish_unmap_win(session_t *ps, struct managed_win *w) {

// We are in unmap_win, this window definitely was viewable
if (ps->backend_data) {
win_release_image(ps->backend_data, w);
win_release_images(ps->backend_data, w);
}
free_paint(ps, &w->paint);
free_paint(ps, &w->shadow_paint);

w->flags = 0;
// Try again at binding images when the window is mapped next time
w->flags &= ~WIN_FLAGS_IMAGE_ERROR;
}

/// Finish the destruction of a window (e.g. after fading has finished).
Expand Down Expand Up @@ -1929,6 +1979,9 @@ void map_win(session_t *ps, struct managed_win *w) {
assert(w);
}

assert((w->flags & WIN_FLAGS_IMAGES_NONE) == WIN_FLAGS_IMAGES_NONE ||
!ps->o.experimental_backends);

// We stopped processing window size change when we were unmapped, refresh the
// size of the window
xcb_get_geometry_cookie_t gcookie = xcb_get_geometry(ps->c, w->base.id);
Expand Down Expand Up @@ -2020,13 +2073,16 @@ void map_win(session_t *ps, struct managed_win *w) {
//
// Also because NVIDIA driver doesn't like seeing the same pixmap under different
// ids, so avoid naming the pixmap again when it didn't actually change.
w->flags &= ~WIN_FLAGS_IMAGE_STALE;
w->flags &= ~WIN_FLAGS_IMAGES_STALE;

// Bind image after update_bounding_shape, so the shadow has the correct size.
if (ps->backend_data) {
if (!win_bind_image(ps, w)) {
w->flags |= WIN_FLAGS_IMAGE_ERROR;
}
win_bind_image(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->gaussian_map);
}

#ifdef CONFIG_DBUS
Expand Down
10 changes: 5 additions & 5 deletions src/win.h
Original file line number Diff line number Diff line change
Expand Up @@ -250,14 +250,14 @@ struct managed_win {
#endif
};

void win_release_image(struct backend_base *base, struct managed_win *w);
bool must_use win_bind_image(session_t *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);

/// Attempt a rebind of window's images. If that failed, the original images are kept.
bool must_use win_try_rebind_image(session_t *ps, struct managed_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
void win_release_images(struct backend_base *base, struct managed_win *w);
void win_bind_image(struct backend_base *backend, struct managed_win *w, struct color c,
struct conv *kernel);
int win_get_name(session_t *ps, struct managed_win *w);
int win_get_role(session_t *ps, struct managed_win *w);
winmode_t attr_pure win_calc_mode(const struct managed_win *w);
Expand Down
21 changes: 18 additions & 3 deletions src/win_defs.h
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#pragma once
#include <stdint.h>

typedef enum {
WINTYPE_UNKNOWN,
Expand Down Expand Up @@ -67,8 +68,22 @@ typedef enum {
} winstate_t;

enum win_flags {
/// win_image/shadow_image is out of date
WIN_FLAGS_IMAGE_STALE = 1,
// Note: *_NONE flags are mostly redudant and meant for detecting logical errors
// in the code

/// pixmap is out of date, will be update in win_process_flags
WIN_FLAGS_PIXMAP_STALE = 1,
/// window does not have pixmap bound
WIN_FLAGS_PIXMAP_NONE = 2,
/// there was an error trying to bind the images
WIN_FLAGS_IMAGE_ERROR = 2,
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,
};

static const int_fast16_t WIN_FLAGS_IMAGES_STALE =
WIN_FLAGS_PIXMAP_STALE | WIN_FLAGS_SHADOW_STALE;

static const int_fast16_t WIN_FLAGS_IMAGES_NONE = WIN_FLAGS_PIXMAP_NONE | WIN_FLAGS_SHADOW_NONE;

0 comments on commit eecee13

Please sign in to comment.