Re: [Mesa-dev] [PATCH] egl: fix _eglQuerySurface in EGL_BUFFER_AGE_EXT case
On 06/08/2017 06:01 PM, Emil Velikov wrote:
On 8 June 2017 at 11:10, Tapani Pälli 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-sta...@lists.freedesktop.org
Signed-off-by: Tapani Pälli
---
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
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
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] egl: fix _eglQuerySurface in EGL_BUFFER_AGE_EXT case
On Thu 08 Jun 2017, Tapani Pälli 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 > > Signed-off-by: Tapani Pälli > --- > 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(-) Reviewed-by: Chad Versace ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] egl: fix _eglQuerySurface in EGL_BUFFER_AGE_EXT case
On 8 June 2017 at 11:10, Tapani Pälli 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-sta...@lists.freedesktop.org
> Signed-off-by: Tapani Pälli
> ---
> 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
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
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] egl: fix _eglQuerySurface in EGL_BUFFER_AGE_EXT case
On Thursday, 2017-06-08 13:10:58 +0300, Tapani Pälli 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
>
> Signed-off-by: Tapani Pälli
> ---
> 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) {
>_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;
Nit: this could be a separate commit.
Regardless, as one or two commits it all gets:
Reviewed-by: Eric Engestrom
Cc: mesa-sta...@lists.freedesktop.org
> }
>
> 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;
> }
>
> return dri2_surf->back->age;
> diff --git a/src/egl/main/eglsurface.c b/src/egl/main/eglsurface.c
> index e935c83..5b3e83e 100644
> --- a/src/egl/main/eglsurface.c
> +++ b/src/egl/main/eglsurface.c
> @@ -409,7 +409,11 @@ _eglQuerySurface(_EGLDriver *drv, _EGLDisplay *dpy,
> _EGLSurface *surface,
> _eglError(EGL_BAD_ATTRIBUTE, "eglQuerySurface");
> return EGL_FALSE;
>}
> - *value = drv->API.QueryBufferAge(drv, dpy, surface);
> + EGLint result = drv->API.QueryBufferAge(drv, dpy, surface);
> + /* error happened */
> + if (result < 0)
> + return EGL_FALSE;
> + *value = result;
>break;
> default:
>_eglError(EGL_BAD_ATTRIBUTE, "eglQuerySurface");
> --
> 2.9.4
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] egl: fix _eglQuerySurface in EGL_BUFFER_AGE_EXT case
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
Signed-off-by: Tapani Pälli
---
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) {
_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;
}
return dri2_surf->back->age;
diff --git a/src/egl/main/eglsurface.c b/src/egl/main/eglsurface.c
index e935c83..5b3e83e 100644
--- a/src/egl/main/eglsurface.c
+++ b/src/egl/main/eglsurface.c
@@ -409,7 +409,11 @@ _eglQuerySurface(_EGLDriver *drv, _EGLDisplay *dpy,
_EGLSurface *surface,
_eglError(EGL_BAD_ATTRIBUTE, "eglQuerySurface");
return EGL_FALSE;
}
- *value = drv->API.QueryBufferAge(drv, dpy, surface);
+ EGLint result = drv->API.QueryBufferAge(drv, dpy, surface);
+ /* error happened */
+ if (result < 0)
+ return EGL_FALSE;
+ *value = result;
break;
default:
_eglError(EGL_BAD_ATTRIBUTE, "eglQuerySurface");
--
2.9.4
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
