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

Update SDL2 port to 2.0.22 #16916

Merged
merged 4 commits into from
Jun 9, 2022
Merged

Update SDL2 port to 2.0.22 #16916

merged 4 commits into from
Jun 9, 2022

Conversation

Daft-Freak
Copy link
Collaborator

Update to latest release, includes some threading/proxying improvements.

@Daft-Freak
Copy link
Collaborator Author

Well, that went badly: error: undefined symbol: emscripten_sync_run_in_main_runtime_thread_. From libsdl-org/SDL#5365 i think

@kripken
Copy link
Member

kripken commented May 9, 2022

That error might be an internal problem, as I don't think that function is called directly by user code. Bisecting on emscripten might be a fast way to find where things changed, if its not obvious to you already.

cc @tlively who made some recent proxying changes, and might have an idea.

@tlively
Copy link
Member

tlively commented May 9, 2022

We reimplemented emscripten_sync_run_in_main_runtime_thread_ in terms of the new Emscripten proxying API, but it is still defined in library_pthread.c, so I'm not sure why the linker would be giving you trouble about it.

And that function is called by user SDL code (via a wrapper macro) here:
https://github.com/libsdl-org/SDL/pull/5365/files#diff-edb4aebb55db5736cc47e163e4a8ff5ca56b86a24b75031c9e4d6860fd1c0756R169.

@ericoporto
Copy link
Contributor

ericoporto commented May 14, 2022

This PR will fix #16646

@Daft-Freak
Copy link
Collaborator Author

I'm guessing library_pthread.c isn't built if you're not using threads? So

if (emscripten_is_main_runtime_thread())
    do_thing();
else
    emscripten_sync_run_in_main_runtime_thread(do_thing);

maybe isn't a great idea.

(I haven't done much with threads/proxying myself, so don't really know what the right thing to do here is...)

@tlively
Copy link
Member

tlively commented May 15, 2022

Probably the simplest thing to do is hide that code behind #ifdef _REENTRANT, which is always defined if threads are enabled.

@sbc100
Copy link
Collaborator

sbc100 commented Jun 8, 2022

Are we still trying to land this?

@Daft-Freak
Copy link
Collaborator Author

Ah, thanks for the reminder... I think I've fixed this by using MAIN_THREAD_EM_ASM for those two uses of emscripten_sync_run_in_main_runtime_thread (which everywhere else was already doing). libsdl-org/SDL#5773

This does mean we still can't use a release tag, which was part of the reason for this PR... hmm

@sbc100
Copy link
Collaborator

sbc100 commented Jun 8, 2022

It seems reasonable to continue to use non-released revisions.. at least until we are able to.

Partly undo the previous "fix"
@Daft-Freak
Copy link
Collaborator Author

Now passing after a few test adjustments (mouse events again, version number is 2.23.0 instead of 2.0.23)

@sbc100
Copy link
Collaborator

sbc100 commented Jun 9, 2022

Maybe worth adding this to the ChangeLog?

@sbc100 sbc100 merged commit 85bdabf into emscripten-core:main Jun 9, 2022
@Daft-Freak Daft-Freak deleted the patch-3 branch August 27, 2022 11:16
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.

5 participants