[Mesa-dev] [PATCH EGL/MESA] EGL/mesa: Initial writeup for MESA_query_renderer
Adam Jackson
ajax at redhat.com
Wed Nov 7 16:17:35 UTC 2018
On Mon, 2018-11-05 at 10:04 +0530, Veluri Mithun wrote:
> Signed-off-by: Veluri Mithun <velurimithun38 at gmail.com>
Thanks for looking into this! Many of these comments apply equally to
the GLX extension I think; if you wanted to write a patch for the
equivalent text for that extension too, that'd be awesome.
> + EGL 1.4 is required.
I'm not sure this is true, from the EGL types you're (currently) using
it looks like even an EGL 1.0 implementation could support this.
Probably better to say "EGL 1.0 is required; this extension is written
against the text of the EGL 1.4 specification."
> + EGL_ARB_create_context and EGL_ARB_create_context_profile are required.
> +
> + This extension interacts with EGL_EXT_create_context_es2_profile and
> + EGL_EXT_create_context_es_profile.
There are GLX extensions with almost these names, but not EGL:
https://www.khronos.org/registry/EGL/
These references should all be deleted.
> +New Procedures and Functions
EGL functions start with 'egl', lowercase.
> + Bool EGLQueryRendererIntegerMESA(EGLDisplay *dpy, int screen,
> + int renderer, int attribute,
> + unsigned int *value);
EGL displays don't have screens, that's an X11-ism. The screen
parameter should be deleted.
> + const char *EGLQueryRendererStringMESA(EGLDisplay *dpy, int screen,
> + int renderer, int attribute);
Same comment about 'screen' here.
> +New Tokens
> +
> + Accepted as an <attribute> in EGLQueryRendererIntegerMESA and
> + EGLQueryCurrentRendererIntegerMESA:
> +
> + EGL_RENDERER_VENDOR_ID_MESA 0xXXXX
> + EGL_RENDERER_DEVICE_ID_MESA 0xXXXX
> + EGL_RENDERER_VERSION_MESA 0xXXXX
> + EGL_RENDERER_ACCELERATED_MESA 0xXXXX
> + EGL_RENDERER_VIDEO_MEMORY_MESA 0xXXXX
> + EGL_RENDERER_UNIFIED_MEMORY_ARCHITECTURE_MESA 0xXXXX
> + EGL_RENDERER_PREFERRED_PROFILE_MESA 0xXXXX
> + EGL_RENDERER_OPENGL_CORE_PROFILE_VERSION_MESA 0xXXXX
> + EGL_RENDERER_OPENGL_COMPATIBILITY_PROFILE_VERSION_MESA 0xXXXX
> + EGL_RENDERER_OPENGL_ES_PROFILE_VERSION_MESA 0xXXXX
> + EGL_RENDERER_OPENGL_ES2_PROFILE_VERSION_MESA 0xXXXX
> +
> + Accepted as an <attribute> in EGLQueryRendererStringMESA and
> + EGLQueryCurrentRendererStringMESA:
> +
> + EGL_RENDERER_VENDOR_ID_MESA
> + EGL_RENDERER_DEVICE_ID_MESA
> +
> + Accepted as an attribute name in <*attrib_list> in
> + EGLCreateContextAttribsARB:
> +
> + EGL_RENDERER_ID_MESA 0xXXXX
Normally these are reserved by creating a request against the EGL
registry on github. However we happen to have a block of 16 enum values
already reserved:
> <enums namespace="EGL" start="0x3290" end="0x329F" vendor="MESA" comment="Reserved for John Kåre Alsaker (Public bug 757)">
> <unused start="0x3290" end="0x329F"/>
> </enums>
That "public bug 757" is for the abandoned EGL_MESA_image_sRGB
extension:
https://www.khronos.org/bugzilla/show_bug.cgi?id=757
So I think it's safe to use enums from that range, and we'll still have
two free.
> +Additions to the EGL 1.4 Specification
> +
> + [Add the following to Section X.Y.Z of the EGL Specification]
Should be Section 3.3 "EGL Versioning". If this were against the EGL
1.5 text it'd be the more-obviously-correct Section 3.3 "EGL Queries".
> + To obtain information about the available renderers for a particular
> + display and screen,
> +
> + Bool EGLQueryRendererIntegerMESA(EGLDisplay *dpy, int screen, int renderer,
> + int attribute, unsigned int *value);
The corresponding eglQueryCurrentRendererIntegerMESA is not documented.
I'm not entirely sure it should even exist, to be honest; I'd prefer if
these attributes were instead newly legal values to pass to
eglQueryContext. Too late to make that change for GLX I suppose.
> + String versions of some attributes may also be queried using
> +
> + const char *EGLQueryRendererStringMESA(EGLDisplay *dpy, int screen,
> + int renderer, int attribute);
Likewise eglQueryCurrentRendererStringMESA is not documented. This
could not be queried with eglQueryContext since that can only return
integers not pointers.
> + [Add to section section 3.3.7 "Rendering Contexts"]
> +
> + The attribute name EGL_RENDERER_ID_MESA specified the index of the render
> + against which the context should be created. The default value of
> + EGL_RENDERER_ID_MESA is 0.
This startled me to read, I didn't think GLX_MESA_query_renderer had
this. Turns out it does have this text in the extension spec, but the
functionality is not actually implemented. Worse, there's no way to
enumerate how many renderers a display (or display/screen in GLX) has.
To address this, I would:
1) Remove this text from both GLX and EGL extension specs
(Both here and above where the enums are listed)
2) Update both specs with an explicit way to enumerate renderers
(probably something like "if renderer is -1 and the attribute is
XXX_RENDERER_COUNT_MESA, return one integer")
3) Add a layered extension to add this functionality to CreateContext
#2 and #3 are optional, I suppose. Probably it's better to have only
one way to do that, and in EGL that way is probably using the EGL
device extensions instead. But GLX doesn't have anything like that, so
this functionality might make sense there.
> + [Add to list of errors for EGLCreateContextAttribsARB in section section
> + 3.3.7 "Rendering Contexts"]
> +
> + * If the value of EGL_RENDERER_ID_MESA specifies a non-existent
> + renderer, BadMatch is generated.
This should be deleted too.
> +Dependencies on EGL_EXT_create_context_es_profile and
> +EGL_EXT_create_context_es2_profile
> +
> + If neither extension is supported, remove all mention of
> + EGL_RENDERER_OPENGL_ES2_PROFILE_VERSION_MESA from the spec.
> +
> + If EGL_EXT_create_context_es_profile is not supported, remove all mention of
> + EGL_RENDERER_OPENGL_ES_PROFILE_VERSION_MESA from the spec.
Neither of these extensions exist, and the text already says what
happens if those APIs aren't supported, so this can be deleted as well.
- ajax
More information about the mesa-dev
mailing list