From 6b016d58d23d16eaae9908a92ed90547d1926317 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michel=20D=C3=A4nzer?= Date: Thu, 1 Nov 2018 18:44:24 +0100 Subject: [PATCH 1/4] xwayland: Replace xwl_window::present_window with ::present_flipped There's no need to keep track of the window which last performed a Present flip. This fixes crashes due to the assertion in xwl_present_flips_stop failing. Fixes issue #10. The damage generated by a flip only needs to be ignored once, then xwl_window::present_flipped can be cleared. This may fix freezing in the (hypothetical) scenario where Present flips are performed on a window, followed by other drawing requests using the window as the destination, but nothing triggering xwl_present_flips_stop. The damage from the latter drawing requests would continue being ignored. --- hw/xwayland/xwayland-present.c | 56 ++++++++-------------------------- hw/xwayland/xwayland.c | 17 ++++++++--- hw/xwayland/xwayland.h | 4 ++- 3 files changed, 27 insertions(+), 50 deletions(-) diff --git a/hw/xwayland/xwayland-present.c b/hw/xwayland/xwayland-present.c index 792eaa69c..e37ab5229 100644 --- a/hw/xwayland/xwayland-present.c +++ b/hw/xwayland/xwayland-present.c @@ -90,24 +90,19 @@ xwl_present_has_events(struct xwl_present_window *xwl_present_window) !xorg_list_is_empty(&xwl_present_window->release_queue); } -static inline Bool -xwl_present_is_flipping(WindowPtr window, struct xwl_window *xwl_window) -{ - return xwl_window && xwl_window->present_window == window; -} - static void xwl_present_reset_timer(struct xwl_present_window *xwl_present_window) { if (xwl_present_has_events(xwl_present_window)) { - WindowPtr present_window = xwl_present_window->window; - Bool is_flipping = xwl_present_is_flipping(present_window, - xwl_window_from_window(present_window)); + CARD32 timeout; + + if (xwl_present_window->frame_callback) + timeout = TIMER_LEN_FLIP; + else + timeout = TIMER_LEN_COPY; xwl_present_window->frame_timer = TimerSet(xwl_present_window->frame_timer, - 0, - is_flipping ? TIMER_LEN_FLIP : - TIMER_LEN_COPY, + 0, timeout, &xwl_present_timer_callback, xwl_present_window); } else { @@ -118,16 +113,12 @@ xwl_present_reset_timer(struct xwl_present_window *xwl_present_window) void xwl_present_cleanup(WindowPtr window) { - struct xwl_window *xwl_window = xwl_window_from_window(window); struct xwl_present_window *xwl_present_window = xwl_present_window_priv(window); struct xwl_present_event *event, *tmp; if (!xwl_present_window) return; - if (xwl_window && xwl_window->present_window == window) - xwl_window->present_window = NULL; - if (xwl_present_window->frame_callback) { wl_callback_destroy(xwl_present_window->frame_callback); xwl_present_window->frame_callback = NULL; @@ -366,10 +357,6 @@ xwl_present_queue_vblank(WindowPtr present_window, if (!xwl_window) return BadMatch; - if (xwl_window->present_window && - xwl_window->present_window != present_window) - return BadMatch; - event = malloc(sizeof *event); if (!event) return BadAlloc; @@ -438,13 +425,6 @@ xwl_present_check_flip2(RRCrtcPtr crtc, if (!xwl_window) return FALSE; - /* - * Do not flip if there is already another child window doing flips. - */ - if (xwl_window->present_window && - xwl_window->present_window != present_window) - return FALSE; - /* * We currently only allow flips of windows, that have the same * dimensions as their xwl_window parent window. For the case of @@ -481,8 +461,6 @@ xwl_present_flip(WindowPtr present_window, if (!event) return FALSE; - xwl_window->present_window = present_window; - buffer = xwl_glamor_pixmap_get_wl_buffer(pixmap, &buffer_created); event->event_id = event_id; @@ -507,13 +485,6 @@ xwl_present_flip(WindowPtr present_window, /* We can flip directly to the main surface (full screen window without clips) */ wl_surface_attach(xwl_window->surface, buffer, 0, 0); - if (!xwl_present_window->frame_timer || - xwl_present_window->frame_timer_firing) { - /* Realign timer */ - xwl_present_window->frame_timer_firing = FALSE; - xwl_present_reset_timer(xwl_present_window); - } - if (!xwl_present_window->frame_callback) { xwl_present_window->frame_callback = wl_surface_frame(xwl_window->surface); wl_callback_add_listener(xwl_present_window->frame_callback, @@ -521,6 +492,10 @@ xwl_present_flip(WindowPtr present_window, xwl_present_window); } + /* Realign timer */ + xwl_present_window->frame_timer_firing = FALSE; + xwl_present_reset_timer(xwl_present_window); + wl_surface_damage(xwl_window->surface, 0, 0, damage_box->x2 - damage_box->x1, damage_box->y2 - damage_box->y1); @@ -536,22 +511,15 @@ xwl_present_flip(WindowPtr present_window, } wl_display_flush(xwl_window->xwl_screen->display); + xwl_window->present_flipped = TRUE; return TRUE; } static void xwl_present_flips_stop(WindowPtr window) { - struct xwl_window *xwl_window = xwl_window_from_window(window); struct xwl_present_window *xwl_present_window = xwl_present_window_priv(window); - if (!xwl_window) - return; - - assert(xwl_window->present_window == window); - - xwl_window->present_window = NULL; - /* Change back to the fast refresh rate */ xwl_present_reset_timer(xwl_present_window); } diff --git a/hw/xwayland/xwayland.c b/hw/xwayland/xwayland.c index 2b2274d09..a05f69b92 100644 --- a/hw/xwayland/xwayland.c +++ b/hw/xwayland/xwayland.c @@ -377,6 +377,18 @@ damage_report(DamagePtr pDamage, RegionPtr pRegion, void *data) struct xwl_window *xwl_window = data; struct xwl_screen *xwl_screen = xwl_window->xwl_screen; +#ifdef GLAMOR_HAS_GBM + if (xwl_window->present_flipped) { + /* This damage is from a Present flip, which already committed a new + * buffer for the surface, so we don't need to do anything in response + */ + RegionEmpty(DamageRegion(pDamage)); + xorg_list_del(&xwl_window->link_damage); + xwl_window->present_flipped = FALSE; + return; + } +#endif + xorg_list_add(&xwl_window->link_damage, &xwl_screen->damage_window_list); } @@ -722,11 +734,6 @@ xwl_screen_post_damage(struct xwl_screen *xwl_screen) xorg_list_for_each_entry_safe(xwl_window, next_xwl_window, &xwl_screen->damage_window_list, link_damage) { -#ifdef GLAMOR_HAS_GBM - /* Present on the main surface. So don't commit here as well. */ - if (xwl_window->present_window) - continue; -#endif /* If we're waiting on a frame callback from the server, * don't attach a new buffer. */ if (xwl_window->frame_callback) diff --git a/hw/xwayland/xwayland.h b/hw/xwayland/xwayland.h index 3c240c1b0..e5dd86b67 100644 --- a/hw/xwayland/xwayland.h +++ b/hw/xwayland/xwayland.h @@ -179,7 +179,9 @@ struct xwl_window { struct xorg_list link_damage; struct wl_callback *frame_callback; Bool allow_commits; - WindowPtr present_window; +#ifdef GLAMOR_HAS_GBM + Bool present_flipped; +#endif }; #ifdef GLAMOR_HAS_GBM -- GitLab From 8c9538573cb9a342897eb3fb4b0c1e4ed917bd0e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michel=20D=C3=A4nzer?= Date: Thu, 1 Nov 2018 18:24:28 +0100 Subject: [PATCH 2/4] xwayland: Add xwl_present_unrealize_window When a window is unrealized, a pending frame callback may never be called, which could result in repeatedly freezing until the frame timer fires after a second. Fixes these symptoms when switching from fullscreen to windowed mode in sauerbraten. --- hw/xwayland/xwayland-present.c | 16 ++++++++++++++++ hw/xwayland/xwayland.c | 5 +++++ hw/xwayland/xwayland.h | 1 + 3 files changed, 22 insertions(+) diff --git a/hw/xwayland/xwayland-present.c b/hw/xwayland/xwayland-present.c index e37ab5229..01ba2ab90 100644 --- a/hw/xwayland/xwayland-present.c +++ b/hw/xwayland/xwayland-present.c @@ -524,6 +524,22 @@ xwl_present_flips_stop(WindowPtr window) xwl_present_reset_timer(xwl_present_window); } +void +xwl_present_unrealize_window(WindowPtr window) +{ + struct xwl_present_window *xwl_present_window = xwl_present_window_priv(window); + + if (!xwl_present_window || !xwl_present_window->frame_callback) + return; + + /* The pending frame callback may never be called, so drop it and shorten + * the frame timer interval. + */ + wl_callback_destroy(xwl_present_window->frame_callback); + xwl_present_window->frame_callback = NULL; + xwl_present_reset_timer(xwl_present_window); +} + static present_wnmd_info_rec xwl_present_info = { .version = PRESENT_SCREEN_INFO_VERSION, .get_crtc = xwl_present_get_crtc, diff --git a/hw/xwayland/xwayland.c b/hw/xwayland/xwayland.c index a05f69b92..cc815d511 100644 --- a/hw/xwayland/xwayland.c +++ b/hw/xwayland/xwayland.c @@ -616,6 +616,11 @@ xwl_unrealize_window(WindowPtr window) xwl_screen->UnrealizeWindow = screen->UnrealizeWindow; screen->UnrealizeWindow = xwl_unrealize_window; +#ifdef GLAMOR_HAS_GBM + if (xwl_screen->present) + xwl_present_unrealize_window(window); +#endif + xwl_window = xwl_window_get(window); if (!xwl_window) return ret; diff --git a/hw/xwayland/xwayland.h b/hw/xwayland/xwayland.h index e5dd86b67..92664e812 100644 --- a/hw/xwayland/xwayland.h +++ b/hw/xwayland/xwayland.h @@ -451,6 +451,7 @@ void xwl_glamor_egl_make_current(struct xwl_screen *xwl_screen); #ifdef GLAMOR_HAS_GBM Bool xwl_present_init(ScreenPtr screen); void xwl_present_cleanup(WindowPtr window); +void xwl_present_unrealize_window(WindowPtr window); #endif /* GLAMOR_HAS_GBM */ #ifdef XV -- GitLab From f541615342ce6bfb0e6d4e68deb3a924a87e8ba9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michel=20D=C3=A4nzer?= Date: Fri, 9 Nov 2018 17:18:53 +0100 Subject: [PATCH 3/4] xwayland: Don't need xwl_window anymore in xwl_present_queue_vblank Fixes issue #12. Presumably the problem was that Present operations on unmapped windows were executed immediately instead of only when reaching the target MSC. --- hw/xwayland/xwayland-present.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/hw/xwayland/xwayland-present.c b/hw/xwayland/xwayland-present.c index 01ba2ab90..943d7290f 100644 --- a/hw/xwayland/xwayland-present.c +++ b/hw/xwayland/xwayland-present.c @@ -350,13 +350,9 @@ xwl_present_queue_vblank(WindowPtr present_window, uint64_t event_id, uint64_t msc) { - struct xwl_window *xwl_window = xwl_window_from_window(present_window); struct xwl_present_window *xwl_present_window = xwl_present_window_get_priv(present_window); struct xwl_present_event *event; - if (!xwl_window) - return BadMatch; - event = malloc(sizeof *event); if (!event) return BadAlloc; -- GitLab From e6cd1c9bdefe83e7d99b703a68d26eebb451f889 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michel=20D=C3=A4nzer?= Date: Thu, 15 Nov 2018 17:16:59 +0100 Subject: [PATCH 4/4] xwayland: Don't take buffer release queue into account for frame timer The buffer release queue has two kinds of entries: * Pending async flips. * Completed flips waiting for their buffer to be released by the Wayland compositor. xwl_present_timer_callback neither completes async flips nor releases buffers, so the timer isn't needed for the buffer release queue. --- hw/xwayland/xwayland-present.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/hw/xwayland/xwayland-present.c b/hw/xwayland/xwayland-present.c index 943d7290f..2d5597bc4 100644 --- a/hw/xwayland/xwayland-present.c +++ b/hw/xwayland/xwayland-present.c @@ -86,8 +86,7 @@ static inline Bool xwl_present_has_events(struct xwl_present_window *xwl_present_window) { return !!xwl_present_window->sync_flip || - !xorg_list_is_empty(&xwl_present_window->event_list) || - !xorg_list_is_empty(&xwl_present_window->release_queue); + !xorg_list_is_empty(&xwl_present_window->event_list); } static void -- GitLab