On 8 December 2017 at 10:10, Harish Krupo <harish.krupo....@intel.com> wrote: > Hi Emil, > > Emil Velikov <emil.l.veli...@gmail.com> writes: > >> Hi Harish, >> >> On 7 December 2017 at 13:34, Harish Krupo <harish.krupo....@intel.com> wrote: >>> From android cts 8.0_r4, a new test case checks if all the required egl >>> extensions are exposed. In the current implementation we expose KHR_image >>> if KHR_image_base and KHR_image_pixmap are supported but KHR_image spec >>> does not mandate the existence of both the extensions. >>> This patch preserves the current check and also provides the backend >>> with an option to expose the KHR_image extension. >>> >>> Test: run cts -m CtsOpenGLTestCases -t \ >>> android.opengl.cts.OpenGlEsVersionTest#testRequiredEglExtensions >>> >> >> A couple of things that come to mind. Hope that I'm not loosing my >> marbles ... too badly. >> >> The KHR_image_pixmap extension lists the following as dependency: >> >> The EGL implementation must define an EGLNativePixmapType (although it >> is not required either to export any EGLConfigs supporting rendering to >> native pixmaps, or to support eglCreatePixmapSurface). >> >> At the same time 'implementations' define the type even ones that lack >> native pixmaps - Wayland, GBM, ... >> >> (A) If one is to ignore that 'detail' we could simply toggle all of >> KHR_image_pixmap across the board and simplify things a bit. >> (B) if native pixmaps is a must - we have a bug in GBM as it >> advertises KHR_image_pixmap. >> >> >>> Signed-off-by: Harish Krupo <harish.krupo....@intel.com> >>> --- >>> src/egl/drivers/dri2/platform_android.c | 1 + >>> src/egl/main/eglapi.c | 3 ++- >>> src/egl/main/egldisplay.h | 1 + >>> 3 files changed, 4 insertions(+), 1 deletion(-) >>> >>> diff --git a/src/egl/drivers/dri2/platform_android.c >>> b/src/egl/drivers/dri2/platform_android.c >>> index 63223e9a69..0459cc8be2 100644 >>> --- a/src/egl/drivers/dri2/platform_android.c >>> +++ b/src/egl/drivers/dri2/platform_android.c >>> @@ -1212,6 +1212,7 @@ dri2_initialize_android(_EGLDriver *drv, _EGLDisplay >>> *dpy) >>> #if ANDROID_API_LEVEL >= 23 >>> dpy->Extensions.KHR_partial_update = EGL_TRUE; >>> #endif >>> + dpy->Extensions.KHR_image = EGL_TRUE; >> >> A: all of KHR_image* can will be a single boot >> B: add _EGLDisplay::has_native_pixmap (or alike) and toggle KHR_image >> and KHR_image_pixmap as applicable in egl_dri2.c >> One can go without ::has_native_pixmap - we have platform type in >> ::Platform so we can deduce it locally. >> >> Change the guards in _eglCreate/DestroyImageCommon as both KHR_image >> and not KHR_image_base provide the entry points. >> Personally I'd add an assert(...KHR_image) since it's close to >> impossible to trigger. >> >>> >>> /* Fill vtbl last to prevent accidentally calling virtual function >>> during >>> * initialization. >>> diff --git a/src/egl/main/eglapi.c b/src/egl/main/eglapi.c >>> index cec67425e1..5110688f2d 100644 >>> --- a/src/egl/main/eglapi.c >>> +++ b/src/egl/main/eglapi.c >>> @@ -504,7 +504,8 @@ _eglCreateExtensionsString(_EGLDisplay *dpy) >>> _EGL_CHECK_EXTENSION(KHR_gl_texture_3D_image); >>> _EGL_CHECK_EXTENSION(KHR_gl_texture_cubemap_image); >>> if (dpy->Extensions.KHR_image_base && dpy->Extensions.KHR_image_pixmap) >>> - _eglAppendExtension(&exts, "EGL_KHR_image"); >>> + dpy->Extensions.KHR_image = EGL_TRUE; >> It's very misleading to set the extension bool in a function called >> CreateExtensionString :-\ >> >> Depending on how A/B goes we can move that to dri2_setup_screen or the >> specific platform*.c. >> >> With the assert() (rest can be done as follow-up) the patch is > > Sorry, I didn't understand, wouldn't checking against KHR_image break > wayland as it doesn't advertise KHR_image (nor KHR_image_pixmap)? > If such a check is required then we will have to add KHR_image=EGL_TRUE > to the relevent places. > Grr you're right - please ignore.
-Emil _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
