BitHeaven
/
picom
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

core: delayed handling of root ConfigureNotify 

Previously, root ConfigureNotify is handled immediately, by resetting
the backend, which in turn releases all the window images. This puts all
the windows into a state where they don't have images attached, which
they really should be in when the screen is redirected.

(To expand a little, a window without images should only exist if:
    * It's an unmanaged window.
    * Screen is unredirected.)

Normally, this kind of window could be fine, as the next render phase
will re-acquire images for them. However, if a window in this state is
destroyed with fading enabled, then the render phase won't try to
acquire images for it, causing it to go into the main rendering function
without images attached, and trigger an assertion.

This commit delays the handling of root ConfigureNotify until the render
phase. This way, the images will be immediately re-acquired after they
are released, thus prevent this problem from happening.

Also adds a testcase for this.

Fixes #357

Signed-off-by: Yuxuan Shui <yshuiv7@gmail.com>
This commit is contained in:
Yuxuan Shui 2020-03-31 05:01:14 +01:00 committed by yshui
parent 5a22fb0828
commit 9332cba8df
6 changed files with 272 additions and 218 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/event.c
View File

@ -250,7 +250,7 @@ static inline void ev_configure_notify(session_t *ps, xcb_configure_notify_event
log_debug("{ send_event: %d, id: %#010x, above: %#010x, override_redirect: %d }",
ev->event, ev->window, ev->above_sibling, ev->override_redirect);
if (ev->window == ps->root) {
configure_root(ps, ev->width, ev->height);
set_root_flags(ps, ROOT_FLAGS_CONFIGURED);
} else {
configure_win(ps, ev);
}

441
src/picom.c
View File

@ -98,6 +98,7 @@ const char *const BACKEND_STRS[] = {[BKEND_XRENDER] = "xrender",
session_t *ps_g = NULL;
void set_root_flags(session_t *ps, uint64_t flags) {
log_debug("Setting root flags: %lu", flags);
ps->root_flags |= flags;
ps->pending_updates = true;
}
@ -400,6 +401,222 @@ xcb_window_t find_client_win(session_t *ps, xcb_window_t w) {
return ret;
}
/**
* Rebuild cached <code>screen_reg</code>.
*/
static void rebuild_screen_reg(session_t *ps) {
get_screen_region(ps, &ps->screen_reg);
}
/**
* Rebuild <code>shadow_exclude_reg</code>.
*/
static void rebuild_shadow_exclude_reg(session_t *ps) {
bool ret = parse_geometry(ps, ps->o.shadow_exclude_reg_str, &ps->shadow_exclude_reg);
if (!ret)
exit(1);
}
/// Free up all the images and deinit the backend
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;
}
if (ps->backend_data) {
if (w->state == WSTATE_MAPPED) {
win_release_images(ps->backend_data, w);
} else {
assert(!w->win_image);
assert(!w->shadow_image);
}
}
free_paint(ps, &w->paint);
}
if (ps->backend_data && ps->root_image) {
ps->backend_data->ops->release_image(ps->backend_data, ps->root_image);
ps->root_image = NULL;
}
if (ps->backend_data) {
// deinit backend
if (ps->backend_blur_context) {
ps->backend_data->ops->destroy_blur_context(
ps->backend_data, ps->backend_blur_context);
ps->backend_blur_context = NULL;
}
ps->backend_data->ops->deinit(ps->backend_data);
ps->backend_data = NULL;
}
}
static bool initialize_blur(session_t *ps) {
struct kernel_blur_args kargs;
struct gaussian_blur_args gargs;
struct box_blur_args bargs;
void *args = NULL;
switch (ps->o.blur_method) {
case BLUR_METHOD_BOX:
bargs.size = ps->o.blur_radius;
args = (void *)&bargs;
break;
case BLUR_METHOD_KERNEL:
kargs.kernel_count = ps->o.blur_kernel_count;
kargs.kernels = ps->o.blur_kerns;
args = (void *)&kargs;
break;
case BLUR_METHOD_GAUSSIAN:
gargs.size = ps->o.blur_radius;
gargs.deviation = ps->o.blur_deviation;
args = (void *)&gargs;
break;
default: return true;
}
ps->backend_blur_context = ps->backend_data->ops->create_blur_context(
ps->backend_data, ps->o.blur_method, args);
return ps->backend_blur_context != NULL;
}
/// Init the backend and bind all the window pixmap to backend images
static bool initialize_backend(session_t *ps) {
if (ps->o.experimental_backends) {
assert(!ps->backend_data);
// Reinitialize win_data
assert(backend_list[ps->o.backend]);
ps->backend_data = backend_list[ps->o.backend]->init(ps);
if (!ps->backend_data) {
log_fatal("Failed to initialize backend, aborting...");
quit(ps);
return false;
}
ps->backend_data->ops = backend_list[ps->o.backend];
if (!initialize_blur(ps)) {
log_fatal("Failed to prepare for background blur, aborting...");
ps->backend_data->ops->deinit(ps->backend_data);
ps->backend_data = NULL;
quit(ps);
return false;
}
// window_stack shouldn't include window that's
// not in the hash table at this point. Since
// there cannot be any fading windows.
HASH_ITER2(ps->windows, _w) {
if (!_w->managed) {
continue;
}
auto w = (struct managed_win *)_w;
assert(w->state == WSTATE_MAPPED || w->state == WSTATE_UNMAPPED);
if (w->state == WSTATE_MAPPED) {
// We need to reacquire image
log_debug("Marking window %#010x (%s) for update after "
"redirection",
w->base.id, w->name);
if (w->shadow) {
struct color c = {
.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, c,
ps->gaussian_map);
}
w->flags |= WIN_FLAGS_PIXMAP_STALE;
ps->pending_updates = true;
}
}
}
// The old backends binds pixmap lazily, nothing to do here
return true;
}
/// Handle configure event of the root window
static void configure_root(session_t *ps) {
auto r = XCB_AWAIT(xcb_get_geometry, ps->c, ps->root);
if (!r) {
log_fatal("Failed to fetch root geometry");
abort();
}
log_info("Root configuration changed, new geometry: %dx%d", r->width, r->height);
bool has_root_change = false;
if (ps->redirected) {
// On root window changes
if (ps->o.experimental_backends) {
assert(ps->backend_data);
has_root_change = ps->backend_data->ops->root_change != NULL;
} else {
// Old backend can handle root change
has_root_change = true;
}
if (!has_root_change) {
// deinit/reinit backend and free up resources if the backend
// cannot handle root change
destroy_backend(ps);
}
free_paint(ps, &ps->tgt_buffer);
}
ps->root_width = r->width;
ps->root_height = r->height;
rebuild_screen_reg(ps);
rebuild_shadow_exclude_reg(ps);
// Invalidate reg_ignore from the top
auto top_w = win_stack_find_next_managed(ps, &ps->window_stack);
if (top_w) {
rc_region_unref(&top_w->reg_ignore);
top_w->reg_ignore_valid = false;
}
if (ps->redirected) {
for (int i = 0; i < ps->ndamage; i++) {
pixman_region32_clear(&ps->damage_ring[i]);
}
ps->damage = ps->damage_ring + ps->ndamage - 1;
#ifdef CONFIG_OPENGL
// GLX root change callback
if (BKEND_GLX == ps->o.backend && !ps->o.experimental_backends) {
glx_on_root_change(ps);
}
#endif
if (has_root_change) {
if (ps->backend_data != NULL) {
ps->backend_data->ops->root_change(ps->backend_data, ps);
}
// Old backend's root_change is not a specific function
} else {
if (!initialize_backend(ps)) {
log_fatal("Failed to re-initialize backend after root "
"change, aborting...");
ps->quit = true;
// TODO only event handlers should request ev_break,
// otherwise it's too hard to keep track of what can break
// the event loop
ev_break(ps->loop, EVBREAK_ALL);
return;
}
// Re-acquire the root pixmap.
root_damaged(ps);
}
force_repaint(ps);
}
return;
}
static void handle_root_flags(session_t *ps) {
if ((ps->root_flags & ROOT_FLAGS_SCREEN_CHANGE) != 0) {
if (ps->o.xinerama_shadow_crop) {
@ -415,6 +632,11 @@ static void handle_root_flags(session_t *ps) {
}
ps->root_flags &= ~(uint64_t)ROOT_FLAGS_SCREEN_CHANGE;
}
if ((ps->root_flags & ROOT_FLAGS_CONFIGURED) != 0) {
configure_root(ps);
ps->root_flags &= ~(uint64_t)ROOT_FLAGS_CONFIGURED;
}
}
static struct managed_win *paint_preprocess(session_t *ps, bool *fade_running) {
@ -646,216 +868,6 @@ static struct managed_win *paint_preprocess(session_t *ps, bool *fade_running) {
return bottom;
}
/**
* Rebuild cached <code>screen_reg</code>.
*/
static void rebuild_screen_reg(session_t *ps) {
get_screen_region(ps, &ps->screen_reg);
}
/**
* Rebuild <code>shadow_exclude_reg</code>.
*/
static void rebuild_shadow_exclude_reg(session_t *ps) {
bool ret = parse_geometry(ps, ps->o.shadow_exclude_reg_str, &ps->shadow_exclude_reg);
if (!ret)
exit(1);
}
/// Free up all the images and deinit the backend
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;
}
if (ps->backend_data) {
if (w->state == WSTATE_MAPPED) {
win_release_images(ps->backend_data, w);
} else {
assert(!w->win_image);
assert(!w->shadow_image);
}
}
free_paint(ps, &w->paint);
}
if (ps->backend_data && ps->root_image) {
ps->backend_data->ops->release_image(ps->backend_data, ps->root_image);
ps->root_image = NULL;
}
if (ps->backend_data) {
// deinit backend
if (ps->backend_blur_context) {
ps->backend_data->ops->destroy_blur_context(
ps->backend_data, ps->backend_blur_context);
ps->backend_blur_context = NULL;
}
ps->backend_data->ops->deinit(ps->backend_data);
ps->backend_data = NULL;
}
}
static bool initialize_blur(session_t *ps) {
struct kernel_blur_args kargs;
struct gaussian_blur_args gargs;
struct box_blur_args bargs;
void *args = NULL;
switch (ps->o.blur_method) {
case BLUR_METHOD_BOX:
bargs.size = ps->o.blur_radius;
args = (void *)&bargs;
break;
case BLUR_METHOD_KERNEL:
kargs.kernel_count = ps->o.blur_kernel_count;
kargs.kernels = ps->o.blur_kerns;
args = (void *)&kargs;
break;
case BLUR_METHOD_GAUSSIAN:
gargs.size = ps->o.blur_radius;
gargs.deviation = ps->o.blur_deviation;
args = (void *)&gargs;
break;
default: return true;
}
ps->backend_blur_context = ps->backend_data->ops->create_blur_context(
ps->backend_data, ps->o.blur_method, args);
return ps->backend_blur_context != NULL;
}
/// Init the backend and bind all the window pixmap to backend images
static bool initialize_backend(session_t *ps) {
if (ps->o.experimental_backends) {
assert(!ps->backend_data);
// Reinitialize win_data
assert(backend_list[ps->o.backend]);
ps->backend_data = backend_list[ps->o.backend]->init(ps);
if (!ps->backend_data) {
log_fatal("Failed to initialize backend, aborting...");
quit(ps);
return false;
}
ps->backend_data->ops = backend_list[ps->o.backend];
if (!initialize_blur(ps)) {
log_fatal("Failed to prepare for background blur, aborting...");
ps->backend_data->ops->deinit(ps->backend_data);
ps->backend_data = NULL;
quit(ps);
return false;
}
// window_stack shouldn't include window that's
// not in the hash table at this point. Since
// there cannot be any fading windows.
HASH_ITER2(ps->windows, _w) {
if (!_w->managed) {
continue;
}
auto w = (struct managed_win *)_w;
assert(w->state == WSTATE_MAPPED || w->state == WSTATE_UNMAPPED);
if (w->state == WSTATE_MAPPED) {
// We need to reacquire image
log_debug("Marking window %#010x (%s) for update after "
"redirection",
w->base.id, w->name);
if (w->shadow) {
struct color c = {
.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, c,
ps->gaussian_map);
}
w->flags |= WIN_FLAGS_PIXMAP_STALE;
ps->pending_updates = true;
}
}
}
// The old backends binds pixmap lazily, nothing to do here
return true;
}
/// Handle configure event of a root window
void configure_root(session_t *ps, int width, int height) {
log_info("Root configuration changed, new geometry: %dx%d", width, height);
bool has_root_change = false;
if (ps->redirected) {
// On root window changes
if (ps->o.experimental_backends) {
assert(ps->backend_data);
has_root_change = ps->backend_data->ops->root_change != NULL;
} else {
// Old backend can handle root change
has_root_change = true;
}
if (!has_root_change) {
// deinit/reinit backend and free up resources if the backend
// cannot handle root change
destroy_backend(ps);
}
free_paint(ps, &ps->tgt_buffer);
}
ps->root_width = width;
ps->root_height = height;
rebuild_screen_reg(ps);
rebuild_shadow_exclude_reg(ps);
// Invalidate reg_ignore from the top
auto top_w = win_stack_find_next_managed(ps, &ps->window_stack);
if (top_w) {
rc_region_unref(&top_w->reg_ignore);
top_w->reg_ignore_valid = false;
}
if (ps->redirected) {
for (int i = 0; i < ps->ndamage; i++) {
pixman_region32_clear(&ps->damage_ring[i]);
}
ps->damage = ps->damage_ring + ps->ndamage - 1;
#ifdef CONFIG_OPENGL
// GLX root change callback
if (BKEND_GLX == ps->o.backend && !ps->o.experimental_backends) {
glx_on_root_change(ps);
}
#endif
if (has_root_change) {
if (ps->backend_data != NULL) {
ps->backend_data->ops->root_change(ps->backend_data, ps);
}
// Old backend's root_change is not a specific function
} else {
if (!initialize_backend(ps)) {
log_fatal("Failed to re-initialize backend after root "
"change, aborting...");
ps->quit = true;
// TODO only event handlers should request ev_break,
// otherwise it's too hard to keep track of what can break
// the event loop
ev_break(ps->loop, EVBREAK_ALL);
return;
}
// Re-acquire the root pixmap.
root_damaged(ps);
}
force_repaint(ps);
}
return;
}
void root_damaged(session_t *ps) {
if (ps->root_tile_paint.pixmap) {
free_root_tile(ps);
@ -1378,12 +1390,15 @@ 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);
// Handle screen changes
handle_root_flags(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");

9
src/picom.h
View File

@ -25,7 +25,11 @@
#include "win.h"
#include "x.h"
enum root_flags { ROOT_FLAGS_SCREEN_CHANGE = 1 };
enum root_flags {
ROOT_FLAGS_SCREEN_CHANGE = 1, // Received RandR screen change notify, we
// use this to track refresh rate changes
ROOT_FLAGS_CONFIGURED = 2 // Received configure notify on the root window
};
// == Functions ==
// TODO move static inline functions that are only used in picom.c, into
@ -40,9 +44,6 @@ uint32_t determine_evmask(session_t *ps, xcb_window_t wid, win_evmode_t mode);
xcb_window_t find_client_win(session_t *ps, xcb_window_t w);
/// Handle configure event of a root window
void configure_root(session_t *ps, int width, int height);
void circulate_win(session_t *ps, xcb_circulate_notify_event_t *ce);
void update_refresh_rate(session_t *ps);

3
tests/configs/issue357.conf Normal file
View File

@ -0,0 +1,3 @@
fading = true;
fade-in-step = 1;
fade-out-step = 0.01;

1
tests/run_tests.sh
View File

@ -4,6 +4,7 @@ exe=$(realpath $1)
cd $(dirname $0)
./run_one_test.sh $exe configs/empty.conf testcases/basic.py
./run_one_test.sh $exe configs/issue357.conf testcases/issue357.py
./run_one_test.sh $exe configs/issue239.conf testcases/issue239.py
./run_one_test.sh $exe configs/issue239_2.conf testcases/issue239_2.py
./run_one_test.sh $exe configs/issue239_3.conf testcases/issue239_3.py

34
tests/testcases/issue357.py Executable file
View File

@ -0,0 +1,34 @@
#!/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
# issue 239 is caused by a window gaining a shadow during its fade-out transition
wid = conn.generate_id()
print("Window 1: ", hex(wid))
# Create a window
conn.core.CreateWindowChecked(depth, wid, root, 0, 0, 100, 100, 0, xproto.WindowClass.InputOutput, visual, 0, []).check()
# Set Window name so it doesn't get a shadow
set_window_name(conn, wid, "Test window 1")
# Map the window, causing picom to unredirect
print("mapping 1")
conn.core.MapWindowChecked(wid).check()
time.sleep(0.5)
trigger_root_configure(conn)
# Destroy the windows
conn.core.DestroyWindowChecked(wid).check()
time.sleep(1)