On Tue, 16 May 2017 11:02:22 +0100 Daniel Stone <dani...@collabora.com> wrote:
> Commit 9ca6711faa03 changed the Wayland winsys to only block for the > frame callback inside SwapBuffers, rather than get_back_bo. get_back_bo > would perform a single non-blocking Wayland event dispatch, to try to > find any release events which we had pulled off the wire but not > actually processed. The blocking dispatch was moved to SwapBuffers. > > This removed a guarantee that we would've processed all events inside > get_back_bo(), and introduced a failure whereby the server could've sent > a buffer release event, but we wouldn't have read it. In clients > unconstrained by SwapInterval (rendering ~as fast as possible), which > were being displayed on a hardware overlay (buffer release delayed), Hi, hardware overlay, or as in the bug report, on the primary plane, i.e. the only composite-bypass case in pre-atomic Weston available without hacks. > this could lead to get_back_bo() failing because there were no free > buffers available to it. > > The drawing rightly failed, but this was papered over because of the > path in eglSwapBuffers() which attempts to guarantee a BO, in order to > support calling SwapBuffers twice in a row with no rendering actually > having been performed. > > Since eglSwapBuffers will perform a blocking dispatch of Wayland > events, a buffer release would have arrived by that point, and we > could then choose a buffer to post to the server. The effect was that > frames were displayed out-of-order, since we grabbed a frame with random > past content to display to the compositor. > > Ideally get_back_bo() failing should store a failure flag inside the > surface and cause the next SwapBuffers to fail, but for the meantime, > restore the correct behaviour such that get_back_bo() no longer fails. > > Signed-off-by: Daniel Stone <dani...@collabora.com> > Reported-by: Eero Tamminen <eero.t.tammi...@intel.com> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98833 > Fixes: 9ca6711faa03 ("Revert "wayland: Block for the frame callback in > get_back_bo not dri2_swap_buffers"") > --- > src/egl/drivers/dri2/platform_wayland.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/src/egl/drivers/dri2/platform_wayland.c > b/src/egl/drivers/dri2/platform_wayland.c > index 52ab9f7579..7837790bdb 100644 > --- a/src/egl/drivers/dri2/platform_wayland.c > +++ b/src/egl/drivers/dri2/platform_wayland.c > @@ -353,7 +353,7 @@ get_back_bo(struct dri2_egl_surface *dri2_surf) > /* There might be a buffer release already queued that wasn't processed */ > wl_display_dispatch_queue_pending(dri2_dpy->wl_dpy, dri2_surf->wl_queue); > > - if (dri2_surf->back == NULL) { > + while (dri2_surf->back == NULL) { > for (i = 0; i < ARRAY_SIZE(dri2_surf->color_buffers); i++) { > /* Get an unlocked buffer, preferrably one with a dri_buffer > * already allocated. */ > @@ -364,6 +364,14 @@ get_back_bo(struct dri2_egl_surface *dri2_surf) > else if (dri2_surf->back->dri_image == NULL) > dri2_surf->back = &dri2_surf->color_buffers[i]; > } > + > + if (dri2_surf->back) > + continue; Would 'break' be more readable? > + > + /* If we don't have a buffer, then block on the server to release one > for > + * us, and try again. */ > + if (wl_display_dispatch_queue(dri2_dpy->wl_dpy, dri2_surf->wl_queue) < > 0) > + return -1; > } > > if (dri2_surf->back == NULL) The swrast path does not need fixing, because there buffer release can never be delayed due to composite bypass, right? Nothing looks suspicious to me, so: Acked-by: Pekka Paalanen <pekka.paala...@collabora.co.uk> Thanks, pq
pgpQ7NLV1j8uw.pgp
Description: OpenPGP digital signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
