-
Notifications
You must be signed in to change notification settings - Fork 671
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
Shipping extensions with GLES names can break existing content #3604
Comments
Emscripten should be allowlisting extensions, and only adding them to its extension string if they are recognized and implemented. It is a defect in emscripten that it hacks together an extension string like this. |
Why do they start crashing? For example, in Firefox, we only try to dlsym pfns for e.g. glClipControlEXT when we see EXT_clip_control in the extension string, but we soft-assert and mark-as-disabled if e.g. EXT_clip_control is in EXTENSIONS but |
I agree, but realistically there is a bunch of content out there that has this bug and is not going to get rebuilt with a new version of Emscripten. And unfortunately I don't think we could really do browser telemetry to determine what sites depend on this behavior in order to do a proper "deprecate and remove" process.
Filament, at least, is not, and that is a pretty well battle-tested engine (on Android mostly), so I guess they have not had issues with it. AFAIK the presence of the string is supposed to guarantee the presence of the function(s), so this would only be needed if there's a driver bug? |
Problem: - Emscripten exposes the GL_EXTENSIONS directly from WebGL. - Emscripten does not implement glClipControlEXT or glPolygonOffsetClampEXT entry points. - Chrome ships EXT_clip_control or EXT_polygon_offset_clamp. - Existing pages on the Web that were compiled using Emscripten, from C/C++ code that supported these extensions (like Filament), start crashing (entry points are NULL). Partially reverts https://crrev.com/c/5021828 Fixed: 1502022 Bug: 1501471 Bug: emscripten-core/emscripten#20798 Bug: KhronosGroup/WebGL#3604 Change-Id: If672d9ed825eaa6f50ccf0a937dd12ce72c36b8f Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5068523 Reviewed-by: Gregg Tavares <[email protected]> Auto-Submit: Kai Ninomiya <[email protected]> Commit-Queue: Kai Ninomiya <[email protected]> Cr-Commit-Position: refs/heads/main@{#1230453}
If I understand the issue // Original native C/C++ code. Runs on native OpenGL ES
bool supportsEXTClipControl = !!strstr(glGetString(GL_EXTENSIONS), "GL_EXT_clip_control");
...
if (supportsEXTClipControl) {
glClipControlEXT(...); // works in native, crashes on emscripten since
// glClipControlEXT is not connected to anything.
// Or, if it's connected to a no-op then it's not
// actually doing the thing advertised as supported.
} |
Correct, I have some sample code in the Emscripten bug too: |
Driver bugs are a reality, and we've hit this class of 'em before. That's why Firefox is paranoid about it. Before moving forward with any changes to our process, I must insist that Emscripten fix its defect here before it causes further harm. |
Jukka has a fix up: emscripten-core/emscripten#20802 |
That PR does not fix the problem. It disables GetProcAddress thus preventing people from encountering the problem but in the process breaks any app that really needs to use GetProcAddress. A new compile option has to be specified to have GetProcAddress. There is a new PR #20948 <emscripten-core/emscripten#20948> to flip the flag so GetProcAddress is available by default and the flag disables it. I do not understand why they are reluctant to fix the extension list they advertise. There is mention of some people wanting to see the unfiltered WebGL list but I don’t truly understand why. Possibly something to do with extensions that have no functions. But a filtered list could still include those extensions.
Regards
-Mark
… On Nov 30, 2023, at 10:25, Kai Ninomiya ***@***.***> wrote:
Jukka has a fix up: emscripten-core/emscripten#20802 <emscripten-core/emscripten#20802>
—
Reply to this email directly, view it on GitHub <#3604 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAREASYVUKGCNV6LSKDZY53YG7OCLAVCNFSM6AAAAAA76VEXICVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMZSHE2TONZVGU>.
You are receiving this because you are subscribed to this thread.
|
Please see this bug for details: emscripten-core/emscripten#20798
tl;dr:
I have a few ideas about this but my suggestion on that bug is for the WebGL group to simply not ship new
EXT_
extensions that add new functions, and instead rebrand them asWEBGL_
.The text was updated successfully, but these errors were encountered: