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

[Emil Velikov]

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.
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.

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