Skip to content
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

Fix emscripten 3.1.51 breaking change about *glGetProcAddress() #87981

Conversation

adamscott
Copy link
Member

Since emscripten 3.1.51, a link flag is now required in order to use *glGetProcAddress() family of functions.

This PR fixes that breaking change.

We use *glGetProcAddress() (relating to the web platform) in:

  • drivers/gles3/rasterizer_gles3.cpp
  • drivers/gles3/storage/config.cpp

A check is done to apply the link flag -s GL_ENABLE_GET_PROC_ADDRESS only if it detects emscripten ≥ 3.1.51.

@adamscott adamscott force-pushed the add-emscripten--sgl_enable_get_proc_address-linker-flag branch from c484aec to 5922ac0 Compare February 5, 2024 16:55
@akien-mga akien-mga added cherrypick:3.x Considered for cherry-picking into a future 3.x release cherrypick:3.5 Considered for cherry-picking into a future 3.5.x release cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release labels Feb 5, 2024
@akien-mga akien-mga modified the milestones: 4.x, 4.3 Feb 5, 2024
@akien-mga akien-mga merged commit d335281 into godotengine:master Feb 5, 2024
15 checks passed
@akien-mga
Copy link
Member

Thanks!

@dsnopek
Copy link
Contributor

dsnopek commented Feb 9, 2024

Hm, I don't think we should be relying on glGetProcAddress() in our web builds? If we're not compiling without this, it probably means we're missing an #ifdef WEB_ENABLED somewhere

@akien-mga
Copy link
Member

Cherry-picked for 4.2.2.
Cherry-picked for 4.1.4.

@akien-mga akien-mga removed cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release labels Mar 11, 2024
@akien-mga
Copy link
Member

Cherry-picked for 3.6.
Cherry-picked for 3.5.4.

@akien-mga akien-mga removed cherrypick:3.x Considered for cherry-picking into a future 3.x release cherrypick:3.5 Considered for cherry-picking into a future 3.5.x release labels Apr 25, 2024
@dsnopek
Copy link
Contributor

dsnopek commented Jun 22, 2024

I came across this one again, because I'm debugging an unrelated issue concerning Emscripten 3.1.51.

I really think this PR was the wrong way to go. As I said previously, we really shouldn't need glGetProcAddress() for the web, and if we are using it, I would consider that a bug since it's very inefficient:

Selection_166

If we chose not to use the -s GL_ENABLE_GET_PROC_ADDRESS flag, then any time code tried to use it on the web, it would be a compiler error, which is a nice way to prevent the "bug" of using glGetProcAddress() on the web.

@dsnopek
Copy link
Contributor

dsnopek commented Jun 22, 2024

Another note from the Emscripten docs about how -s GL_ENABLE_GET_PROC_ADDRESS=1 should be avoided:

Selection_167

@dsnopek
Copy link
Contributor

dsnopek commented Jun 22, 2024

Rather than just complaining, I've started draft PR #93489 :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants