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

Don't use system emcc #76

Merged
merged 1 commit into from
Aug 14, 2024
Merged

Conversation

luchak
Copy link
Contributor

@luchak luchak commented Aug 14, 2024

Mixing system emcc with the versioned SDK from this package can cause mismatches in memory layouts, resulting in errors and crashes.

I discovered this because none of the demos here were working for me. They all saw GL errors when initializing sokol-gfx. It looked like the context attributes were corrupted between where they were written in _sapp_emsc_webgl_init and where they were read in _emscripten_webgl_do_create_context - the requested context major version was 0. It looked like an issue with struct layout: on the JS side, renderViaOffscreenBackBuffer looked like it was read as 2, which was certainly suspicious.

Syncing my system Emscripten version with the package version seemed to solve the issue, although it was always hard to be 100% sure I've cleared out all relevant build caches correctly. (Hardcoding the GL major version to 2 in JS also worked, but was a lot less informative.)

This change removes the most obvious source of cross-contamination I could find: it avoids using system emcc entirely. I seem to be getting successful builds now, but it might be worth testing this yourself.

Mixing system emcc with the versioned SDK from this package can cause
mismatches in memory layouts, resulting in errors and crashes.
@floooh
Copy link
Owner

floooh commented Aug 14, 2024

Ooof, yeah, that wasn't actually intended. @kassane maybe also something to fix in the D bindings.

@floooh floooh merged commit 91a6e03 into floooh:master Aug 14, 2024
3 checks passed
@floooh
Copy link
Owner

floooh commented Aug 14, 2024

Thanks for the fix @luchak!

kassane added a commit to kassane/sokol-d that referenced this pull request Aug 14, 2024
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.

2 participants