Skip to content

Parameterize test_webgl_offscreen_framebuffer_state_restoration. NFC #23064

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

Conversation

sbc100
Copy link
Collaborator

@sbc100 sbc100 commented Dec 3, 2024

No description provided.

@sbc100
Copy link
Collaborator Author

sbc100 commented Dec 3, 2024

Split out from #23056

@sbc100 sbc100 requested review from kainino0x and kripken December 3, 2024 22:10
Copy link
Collaborator

@kainino0x kainino0x left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't realize that this whole mess was fully my doing (#8037). I checked there and the TEST_VAO vs TEST_REQUIRE_VAO thing was just wrong from the beginning. Probably I tried to refactor something during the PR and did it incompletely.

However from looking at that PR I think I have figured out what the intent was: not to enable the extension, but to verify that it was supported so we could be sure that OffscreenFramebuffer would be using it.

Yesterday I was confused about what would actually be enabling it for OffscreenFramebuffer to use, but that was because I thought emscripten_webgl_create_context wouldn't enable it. But it does, because GL_SUPPORT_AUTOMATIC_ENABLE_EXTENSIONS=1 by default.

Back when I originally wrote this I'm not sure there was such an option, or at least it was phrased differently. But I think what this means is if GL_SUPPORT_AUTOMATIC_ENABLE_EXTENSIONS=0 then the VAO path will never be used on webgl1, because the extension won't be enabled early enough to hit it.

Also fix typo in test code: TEST_VAO -> TEST_REQUIRE_VAO.
@sbc100 sbc100 force-pushed the test_webgl_offscreen_framebuffer_state_restoration branch from be48e42 to 60b2d1c Compare December 4, 2024 00:13
@sbc100
Copy link
Collaborator Author

sbc100 commented Dec 4, 2024

Thanks @kainino0x !

@sbc100 sbc100 requested a review from kainino0x December 4, 2024 00:14
@sbc100 sbc100 enabled auto-merge (squash) December 4, 2024 00:35
@sbc100 sbc100 disabled auto-merge December 4, 2024 03:52
@sbc100 sbc100 merged commit da54ee3 into emscripten-core:main Dec 4, 2024
27 of 28 checks passed
@sbc100 sbc100 deleted the test_webgl_offscreen_framebuffer_state_restoration branch December 4, 2024 03:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants