On 07/12/16 10:55, Eric Engestrom wrote:
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 <[email protected]>
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`.
Ah thanks for the hint. I'm not quite sure about this particular case
because right after the wl_display_dispatch_queue_pending() we make sure
to allocate a dri_image and reset age to 0.
Now I could see other cases where we could have a stale value (as you've
mentioned), for example with resize scenarios.
I'll send another patch to reset the age to 0 upon destruction.
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 :)
I'm no expert either :)
Thanks again!
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
[email protected]
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
