On Tuesday, 2016-12-06 12:13:32 +0000, Lionel Landwerlin wrote: > On 06/12/16 11:36, Eric Engestrom wrote: > > On Saturday, 2016-12-03 22:47:17 +0000, Lionel Landwerlin wrote: > > > Seeing gtk+ application lockup when they query the buffer age of a > > > surface. > > > > > > Since we update the buffer age field only when creating buffers & swaping > > > them on the client side, there shouldn't be any need for requesting a new > > > back buffer if there is already one available. > > > > > > Signed-off-by: Lionel Landwerlin <lionel.g.landwer...@intel.com> > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=84016 > > > --- > > > src/egl/drivers/dri2/platform_wayland.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/src/egl/drivers/dri2/platform_wayland.c > > > b/src/egl/drivers/dri2/platform_wayland.c > > > index 395f1e1..84d9ddd 100644 > > > --- a/src/egl/drivers/dri2/platform_wayland.c > > > +++ b/src/egl/drivers/dri2/platform_wayland.c > > > @@ -798,7 +798,7 @@ dri2_wl_query_buffer_age(_EGLDriver *drv, > > > { > > > struct dri2_egl_surface *dri2_surf = dri2_egl_surface(surface); > > > - if (get_back_bo(dri2_surf) < 0) { > > > + if (dri2_surf->back == NULL && get_back_bo(dri2_surf) < 0) { > > I'm not convinced this is the right fix: this change the behaviour in > > two ways: > > 1. the backing dri_image might not be allocated, which we don't care > > about here because we only want to read dri2_surf->back->age. > > I don't see anything in the specification that requires allocating the image > when the buffer age is queried. > So this doesn't seem to be an issue right?
I should've been more explicit: this change is fine :) > > > 2. wl_display_dispatch_queue_pending() not being called, ie. we might > > operate on stale data. > > I don't really agree with this argument : > > My understanding (correct me if I'm wrong) is that the age value of the back > buffers is entirely managed by the client. > It is to be updated when a swap buffer request is issued (also in > destructions and cases that don't really concern us here, like memory > pressure events etc...). > The only information that the display server gives us is that a buffer is > not used anymore and can therefore be reused as a back buffer, it does not > change the buffer age value a any buffer. Not directly, but if I'm reading the code right, a buffer could be released during wl_display_dispatch_queue_pending(), the new one not have a backing dri_image (dri2_surf->back->dri_image == NULL), resulting in a createImage() and `age = 0`. In this situation, the buffer_age returned with your patch would not match the back buffer that would actually be used. Is there anything that makes this not happen in the real world? Like I said, I'm not an expert in this area, I'm just reading the code, which I could be misunderstanding :) Cheers, Eric > > Reading the dri3 backend for example, there is no communication between the > client & server upon buffer age query. > > > Thanks a lot for looking this patch. > > Cheers, > > - > Lionel > > > > > If I understand (2) correctly, it means after your patch, we might > > return the buffer age of a buffer about to be released, instead of > > processing the queue, letting it get released and getting a new one. > > This might fix your lockup, but I think this results in potentially > > invalid data being returned to the client. > > > > Cheers, > > Eric > > _______________________________________________ > > mesa-dev mailing list > > mesa-dev@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev > > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
