Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
GL_ENABLE_GET_PROC_ADDRESS #20802
GL_ENABLE_GET_PROC_ADDRESS #20802
Changes from all commits
fa94c64
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about if the user calls
glGetString(GL_EXTENSIONS)
but they don't setGL_ENABLE_GET_PROC_ADDRESS
? Is it OK that they see the full unfiltered list in this case?Similarly
emscripten_webgl_get_supported_extensions
.. ifGL_ENABLE_GET_PROC_ADDRESS
is not set what do we expect it to return?How about GL_SUPPORT_AUTOMATIC_ENABLE_EXTENSIONS which is enabled by default.. do we want to see the full unfiltered list by default?
How about the existing wrapping of getSupportedExtensions by GL_DISABLE_HALF_FLOAT_EXTENSION_IF_BROKEN? Does that wrapper simply layer in top of this one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it should be OK to expose the unfiltered list in all cases when GetProcAddress is not enabled, because any code that uses an entry point Emscripten doesn't implement will cause a linker error. (And any extension that doesn't have entry points is OK.) It is a little unfortunate that they will have to
#ifdef __EMSCRIPTEN__
though instead of#ifdef GL_EXT_clip_control
which many codebases will already have.Come to think of it, it must be spec-valid in OpenGL for an implementation to list extensions that aren't available in the header - because you can use an older header with a newer driver. It's less guaranteed that applications would be robust to this though.
That wrapper shadows
WebGLRenderingContext.prototype.getSupportedExtensions
withctx.getSupportedExtensions
so I think it will apply first (at the JS level) and then this patch will apply on top.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Er, my point is, Emscripten could remove them from the header instead of filtering them out, and that would be a less robust but technically valid fix (that would work for Filament at least in this case).
Actually it would be more robust than I thought - if we did this, we'd be disabling not just the static entrypoint
glClipControlEXT
, but alsoPFNGLCLIPCONTROLEXTPROC
which should be used by any program that's using GetProcAddress on that function. Example: https://github.com/google/filament/blob/b007e9137eb029e256675eceb5fd52e6b873cecd/filament/backend/src/opengl/gl_headers.h#L132There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes that is ok. That is just the common case as if user had hand-written native JavaScript code and called
ctx.getSupportedExtensions()
in it.Same as above. All extensions that the WebGL implementation advertises, unfiltered.
Yes. The root problem is in the use of the
glGetProcAddress()
function, because it is unable to truly enumerate extensions.Yeah, the two things should cascade well together.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this outer
#if
required?