Discussion:
[PATCH] egl: remove unneeded _eglGetNativePlatform check
Add Reply
Emil Velikov
2017-11-13 14:04:59 UTC
Reply
Permalink
Raw Message
From: Emil Velikov <***@collabora.com>

There's little point in calling _eglGetNativePlatform() in
eglCopyBuffers. The platform return is identical to the one already
stored in our _EGLDisplay.

Modulo subtle memory corruption of course. But in these cases returning
EGL_BAD_NATIVE_PIXMAP doesn't sound right.

Signed-off-by: Emil Velikov <***@collabora.com>
---
src/egl/main/eglapi.c | 2 --
1 file changed, 2 deletions(-)

diff --git a/src/egl/main/eglapi.c b/src/egl/main/eglapi.c
index c1bf5bbfe19..4aa93db829b 100644
--- a/src/egl/main/eglapi.c
+++ b/src/egl/main/eglapi.c
@@ -1396,8 +1396,6 @@ eglCopyBuffers(EGLDisplay dpy, EGLSurface surface, EGLNativePixmapType target)
native_pixmap_ptr = (void*) target;

_EGL_CHECK_SURFACE(disp, surf, EGL_FALSE, drv);
- if (disp->Platform != _eglGetNativePlatform(disp->PlatformDisplay))
- RETURN_EGL_ERROR(disp, EGL_BAD_NATIVE_PIXMAP, EGL_FALSE);
ret = drv->API.CopyBuffers(drv, disp, surf, native_pixmap_ptr);

RETURN_EGL_EVAL(disp, ret);
--
2.15.0
Emil Velikov
2017-12-06 17:32:56 UTC
Reply
Permalink
Raw Message
Post by Emil Velikov
There's little point in calling _eglGetNativePlatform() in
eglCopyBuffers. The platform return is identical to the one already
stored in our _EGLDisplay.
Modulo subtle memory corruption of course. But in these cases returning
EGL_BAD_NATIVE_PIXMAP doesn't sound right.
---
src/egl/main/eglapi.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/src/egl/main/eglapi.c b/src/egl/main/eglapi.c
index c1bf5bbfe19..4aa93db829b 100644
--- a/src/egl/main/eglapi.c
+++ b/src/egl/main/eglapi.c
@@ -1396,8 +1396,6 @@ eglCopyBuffers(EGLDisplay dpy, EGLSurface surface, EGLNativePixmapType target)
native_pixmap_ptr = (void*) target;
_EGL_CHECK_SURFACE(disp, surf, EGL_FALSE, drv);
- if (disp->Platform != _eglGetNativePlatform(disp->PlatformDisplay))
- RETURN_EGL_ERROR(disp, EGL_BAD_NATIVE_PIXMAP, EGL_FALSE);
ret = drv->API.CopyBuffers(drv, disp, surf, native_pixmap_ptr);
RETURN_EGL_EVAL(disp, ret);
--
Humble ping anyone?

-Emil
Ian Romanick
2017-12-06 20:36:41 UTC
Reply
Permalink
Raw Message
Do we have any tests at all that exercise this path? I don't know this
code well enough to feel comfortable reviewing this (says everyone). :(
Post by Emil Velikov
There's little point in calling _eglGetNativePlatform() in
eglCopyBuffers. The platform return is identical to the one already
stored in our _EGLDisplay.
Modulo subtle memory corruption of course. But in these cases returning
EGL_BAD_NATIVE_PIXMAP doesn't sound right.
---
src/egl/main/eglapi.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/src/egl/main/eglapi.c b/src/egl/main/eglapi.c
index c1bf5bbfe19..4aa93db829b 100644
--- a/src/egl/main/eglapi.c
+++ b/src/egl/main/eglapi.c
@@ -1396,8 +1396,6 @@ eglCopyBuffers(EGLDisplay dpy, EGLSurface surface, EGLNativePixmapType target)
native_pixmap_ptr = (void*) target;
_EGL_CHECK_SURFACE(disp, surf, EGL_FALSE, drv);
- if (disp->Platform != _eglGetNativePlatform(disp->PlatformDisplay))
- RETURN_EGL_ERROR(disp, EGL_BAD_NATIVE_PIXMAP, EGL_FALSE);
ret = drv->API.CopyBuffers(drv, disp, surf, native_pixmap_ptr);
RETURN_EGL_EVAL(disp, ret);
Emil Velikov
2017-12-11 20:12:05 UTC
Reply
Permalink
Raw Message
Post by Ian Romanick
Do we have any tests at all that exercise this path? I don't know this
code well enough to feel comfortable reviewing this (says everyone). :(
At first I was thinking that the error "can never happen", although it
seems like it can...
Although that's a bug in Mesa - it shouldn't flag up ;-)

Thanks for the prod Ian! A simple piglit test, have uncovered a bug in
the shmfence/DRI3 code.
DRI2 works like a charm - I'll send the patches in a second.

Emil

Loading...