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

Allow customizing canvas on Emscripten #5260

Open
RReverser opened this issue Jan 26, 2022 · 9 comments
Open

Allow customizing canvas on Emscripten #5260

RReverser opened this issue Jan 26, 2022 · 9 comments
Milestone

Comments

@RReverser
Copy link

Emscripten allows customizing the canvas element used for graphics via the Module.canvas property.

However, in SDL the canvas ID is currently hardcoded:

wdata->canvas_id = SDL_strdup("#canvas");

This results in undecipherable errors during start-up when #canvas element does not exist on the page:
image

Looking further through Emscripten docs, I see that #canvas used to mean "whatever element is currently set as the default canvas" in the past, and I'm guessing SDL's hardcoded string was added before that behaviour has changed:

image

Indeed, compiling with -s DISABLE_DEPRECATED_FIND_EVENT_TARGET_BEHAVIOR=0 works around the issue and draws to the canvas set in Module.canvas instead of trying to find #canvas element on the page.

However, this is just a workaround and, I believe SDL should support the non-deprecated behaviour and respect canvas customizations set in Module.canvas or via emscripten_webgl_make_context_current.

cc @Daft-Freak @juj @kripken

@Daft-Freak
Copy link
Collaborator

I had patches a while ago to allow specifying the canvas selector using SDL_CreateWindowFrom (and use multiple canvases), the problem was that EGL always uses Module.canvas... so GL doesn't work.

I think I suggested somewhere that there could he a hint for the default canvas selector, but that would have the same problems unless you also set Module.canvas (which is a bit deprecated?)... or EGL gets some improvements (I had patches for that are probably completely outdated now)... or we start using the emscripten_webgl_* APIs directly.

(The default canvas "id" is #canvas as that worked for the default shell with both the old and new behaviour: emscripten-ports/SDL2#75)

@RReverser
Copy link
Author

which is a bit deprecated?

Oh, it is? I'm only digging into those APIs and having hard time figuring out which methods are new and which are deprecated, or how Module.canvas, #canvas (old and new behaviour), and emscripten_webgl_* APIs all relate to each other...

(The default canvas "id" is #canvas as that worked for the default shell with both the old and new behaviour: emscripten-ports/SDL2#75)

Yeah it works with the default shell, but not when you want to ship a reusable and isolated JS module - that's when Module.canvas or similar override is preferable.

Do you have plans to revive those patches? They seem promising.

@RReverser
Copy link
Author

Btw, I've noticed that SDL doesn't work with OffscreenCanvas mode either (emscripten-core/emscripten#13652). I wonder if that's for the same reasons - not supporting custom WebGL contexts - or a separate issue?

@connorjclark
Copy link
Contributor

Btw, I've noticed that SDL doesn't work with OffscreenCanvas mode either

I see the same, here's the error:

VM425:69 Uncaught DOMException: Failed to execute 'getContext' on 'HTMLCanvasElement': Cannot get context from a canvas that has transferred its control to offscreen.

@Daft-Freak is that branch that you linked to the most up to date, or do you happen to have something newer I could tinker with?

@Daft-Freak
Copy link
Collaborator

Newer branch: https://github.com/Daft-Freak/SDL/commits/create-window-from-2 (commits up to adding the hint should be useful here, the rest is WIP multi-window stuff)

Has some issues in cursor/framebuffer handling...

@connorjclark
Copy link
Contributor

Wow, thanks, this works! I was able to compile my SDL application which previously was failing w/ PROXY_TO_PTHREAD/offscreen canvas when it tried to configure GL options. I only had to make a single tweak to your branch:

https://github.com/Daft-Freak/SDL/blob/a93fc312a87cc2a94ce19bedec8ba846a6ee50e4/src/audio/emscripten/SDL_emscriptenaudio.c#L259 should use MAIN_THREAD_EM_ASM_INT to proxy to the main thread.

Without that change, SDL would fail at init (unless you excluded the audio subsystem).

Has some issues in cursor/framebuffer handling...

Didn't see any issue with custom cursor or mouse lock FWIW. this pr recently fixed some cursor issues.

I'm gonna point my local SDL fork to this for now and keep playing with it. But so far... this is beautiful. So good to get off that main thread!

@Daft-Freak
Copy link
Collaborator

Ah nice, skipping the EGL layer also fixes thread/proxying issues.

(The cursor issues are that everything else is going through the html5 API/selectors and cursors are still using Module['canvas'].style ...)

@sherpya
Copy link

sherpya commented Aug 25, 2022

there is no need to add another sdl call, you can pass the canvas id in ENV (and calling getenv), use an hint like with keyboard, or fille specialHTMLTargets in preInit

@slouken
Copy link
Collaborator

slouken commented Aug 25, 2022

Yes, adding a hint sounds like a good idea. Feel free to create a PR with the relevant changes.

@slouken slouken added this to the 2.26.0 milestone Aug 25, 2022
@slouken slouken modified the milestones: 2.26.0, 3.0 Oct 25, 2022
jrguzman-ms pushed a commit to msft-mirror-aosp/platform.external.webp that referenced this issue Apr 1, 2024
after:
24d7f9c Switch code to SDL2.

fixes:
webp_wasm.js:1 Uncaught TypeError: Cannot read properties of null
    (reading 'addEventListener')
    at Object.registerOrRemoveHandler (webp_wasm.js:1:101330)
    at registerMouseEventCallback (webp_wasm.js:1:154227)
    at _emscripten_set_mousemove_callback_on_thread (webp_wasm.js:1:155015)
    ...

The SDL2 port forces the canvas id to '#canvas':
https://github.com/emscripten-ports/SDL2/blob/324df6865ae3c7d194ed233a86867c48ec96c6a3/src/video/emscripten/SDL_emscriptenvideo.c#L210

This change maps '#output_canvas' to this entry in specialHTMLTargets[]:
https://emscripten.org/docs/api_reference/html5.h.html

libsdl-org/SDL#5260 &
emscripten-ports/SDL2#130 may also be related.

Change-Id: I26f4aa22b9d68b0fc45b83edfe6fe074b59a82a7
Test: emscripten 3.1.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 a pull request may close this issue.

5 participants