[Mesa-dev] [PATCH] egl: fix _eglQuerySurface in EGL_BUFFER_AGE_EXT case

[Tapani Pälli]

On 06/08/2017 06:01 PM, Emil Velikov wrote:
> On 8 June 2017 at 11:10, Tapani Pälli <tapani.palli at intel.com> wrote:
>> Specification states that in case of error, value should not be
>> written, patch changes buffer age queries to return -1 in case of
>> error so that we can skip changing the value.
>>
>> In addition, small change to droid_query_buffer_age to return 0
>> in case buffer does not have a back buffer available.
>>
>> Fixes:
>>     dEQP-EGL.functional.negative_partial_update.not_postable_surface
>>
> Great find Tapani. Please don't forget the stable tag.
> 
> Cc: mesa-stable at lists.freedesktop.org
> 
>> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
>> ---
>>   src/egl/drivers/dri2/platform_android.c | 4 ++--
>>   src/egl/drivers/dri2/platform_drm.c     | 2 +-
>>   src/egl/main/eglsurface.c               | 6 +++++-
>>   3 files changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/egl/drivers/dri2/platform_android.c b/src/egl/drivers/dri2/platform_android.c
>> index 48ecb9f..4c97935 100644
>> --- a/src/egl/drivers/dri2/platform_android.c
>> +++ b/src/egl/drivers/dri2/platform_android.c
>> @@ -614,10 +614,10 @@ droid_query_buffer_age(_EGLDriver *drv,
>>
>>      if (update_buffers(dri2_surf) < 0) {
> Completely unrelated:
> 
> This combined with the following discrepancy might be the reason why
> Android is special wrt get_back_bo()
> On wayland/drm we have
> - getBuffers{WithFormat,} (dri2/image respectively) -> misc -> get_back_bo
> - swap_buffers -> age buffers -> get_back_bo -> pick unlocked (and
> oldest for gbm) buffer -> create image -> init. age
> - query_buffer_age -> get_back_bo -> return age
> 
> while on Android
> - getBuffers{WithFormat,} (dri2/image respectively) -> misc ->
> _missing_ get_back_bo from update_buffers()
> - swap_buffers -> age buffers -> if we have back bo -> init. age
> - query_buffer_age -> get_back_bo -> return age
> 
> Hence, things won't fly on Android if a query_buffer_age is not called
> before swapbuffers.
> 
>>         _eglError(EGL_BAD_ALLOC, "droid_query_buffer_age");
>> -      return 0;
>> +      return -1;
>>      }
>>
>> -   return dri2_surf->back->age;
>> +   return dri2_surf->back ? dri2_surf->back->age : 0;
>>   }
>>
>>   static EGLBoolean
>> diff --git a/src/egl/drivers/dri2/platform_drm.c b/src/egl/drivers/dri2/platform_drm.c
>> index 2f04589..36c89fc 100644
>> --- a/src/egl/drivers/dri2/platform_drm.c
>> +++ b/src/egl/drivers/dri2/platform_drm.c
>> @@ -464,7 +464,7 @@ dri2_drm_query_buffer_age(_EGLDriver *drv,
>>
>>      if (get_back_bo(dri2_surf) < 0) {
>>         _eglError(EGL_BAD_ALLOC, "dri2_query_buffer_age");
>> -      return 0;
>> +      return -1;
> dri2_wl_query_buffer_age() could use an identical fix.
> 
> Please either squash a wayland fix in (adding a note that x11 needs
> similar fix) or split them up.

Oopsie yes, I'll add dri2_wl_query_buffer_age in the mix too, x11 will 
be a separate thing, I took a look at it and it is .. different.

> In either case
> Reviewed-by: Emil Velikov <emil.velikov at collabora.com>
> 
> Warning slight diversion incoming :-\
> 
> On platform_x11.c side similar fix is needed, but let's keep that separate.
> Mostly since we need to update loader_dri3_query_buffer_age() as well
> as libGLX :-\
> ... Not to mention there's some fun points around glXQueryDrawable.

Agreed, will take a look separately.

> Spec states that the function returns "void", same as the XML file and
> headers (Mesa, Khronos and Nvidia shipped ones). Yet nearly all
> documentation pages [1] [2] [3] claim that "int" is returned.
> An older man page [4] seems to be more accurate, yet the only correct
> instance that I can find.
> 
> Might be worth checking with other vendors/Khronos and following from there.
> 
> Checking python-opengl - seems to be doing the right thing. So we
> should be fine - barring that the manuals that need fixing ;-)
> 
> -Emil
> 
> [1] https://www.khronos.org/registry/OpenGL-Refpages/gl2.1/xhtml/glXQueryDrawable.xml
> [2] http://pyopengl.sourceforge.net/documentation/manual-3.0/glXQueryDrawable.html
> [3] http://code.nabla.net/doc/OpenGL/api/OpenGL/man/glXQueryDrawable.html
> [4] http://nixdoc.net/man-pages/IRIX/man3/glxquerydrawable.3.html
>