Bring some GL ES 3.0 functions to GetProcAddress #5951
Conversation
There was a problem hiding this comment.
Thanks!
- Looks like there's a conflict that needs resolving.
- This looks right to me, but I don't remember if there isn't another necessary step here. We can verify that with adding a test, which we should do anyhow. We don't need to test them all, but at least one GL ES 3 method would be good. Can grep for related tests in
tests/, might be easy to add to an existing GL ES 3 test, or an existing GetProcAddress one.
| if (!strcmp(name, "glGetActiveUniformBlockiv")) return emscripten_glGetActiveUniformBlockiv; | ||
| if (!strcmp(name, "glGetActiveUniformBlockName")) return emscripten_glGetActiveUniformBlockName; | ||
| if (!strcmp(name, "glUniformBlockBinding")) return emscripten_glUniformBlockBinding; | ||
| // GLES 3.0 functions |
There was a problem hiding this comment.
I assume all the new lines are from here, and above this it was just removing the else? (which I agree is a nice change)
|
Uh oh, noticed the following comment explaining the issue (in docs/optimizing/Optimizing-WebGL.rst):
|
|
@kripken this behavior seems to contradict EGL specification:
:( |
|
I don't understand, what's the contradiction? |
|
@kripken if Emscripten is implementing context creation via EGL, it has to obey the EGL specification, which states that Also, I found a largely similar (but better) work - #4829 |
|
@juj would know best what to do here. Also, the status of that other PR. But I don't see a problem in the documentation paragraph - it just says the function pointers are slower, it doesn't say if they will or won't be supported? They should be supported, I think. Maybe we can improve the text there, but I'm not sure how? |
|
This PR looks good to me (will need rebasing to resolve merge conflict). We want to support both forms - querying via function pointers and static linking, and we encourage everyone to use static linking, because we have that opportunity. Static linking is faster in practice - invoking via dynamic pointers does show up in profiles as a few % unnecessary overhead, plus also Still, we do want to support |
|
@juj thanks for clarifying. I rebased the PR now. |
1706: More Emscripten support r=msiglreith a=kvark ## Intro This is a big follow-up to #1681 and #1685 After being blocked by rust-lang/rust#47023, I got stuff building in release, so continued porting the examples over. Note: there is a lot of shader code duplication, we need some (simple) templating/preprocessor there. ## Environment OS: Linux + Firefox Nightly, Emscripten: built from source with the following changes: - integrated emscripten-core/emscripten#5951 - `USE_WEBGL2=1` is forcefully set in "settings.js" ## Status 🎉 Work 🎉 : triangle, cube, flowmap, blend, gamma, terrain, ubo_tilemap Almost works: mipmap - see #1704 Not tested: deferred (too many shaders!), performance Blocked by the lack of buffer mapping: instancing Blocked by the lack of geometry/tessellation shader support: particle, render_target, terrain_tessellated Fail to run: shadow, skybox - see #1705 ## Proof 
|
Hi @kvark - sorry this PR got forgotten! I did some work to apply this over current incoming, here are two commits that should do that, first does the else removal, second adds your new functions here: I think you can start with current incoming, cherry-pick those two commits, and add yourself to AUTHORS, and then we can merge this. |
|
@kripken please take another look |
|
Great, thanks @kvark ! |
No description provided.