SDL icon indicating copy to clipboard operation
SDL copied to clipboard

Allow customizing canvas on Emscripten

Open RReverser opened this issue 3 years ago • 9 comments

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

However, in SDL the canvas ID is currently hardcoded: https://github.com/libsdl-org/SDL/blob/71e06a536a6fb9eebf15881e9a1ddaf2f463a5f1/src/video/emscripten/SDL_emscriptenvideo.c#L204

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

RReverser avatar Jan 26 '22 19:01 RReverser

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: https://github.com/emscripten-ports/SDL2/pull/75)

Daft-Freak avatar Jan 26 '22 20:01 Daft-Freak

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 avatar Jan 26 '22 20:01 RReverser

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

RReverser avatar Jan 26 '22 20:01 RReverser

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?

connorjclark avatar Apr 11 '22 01:04 connorjclark

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...

Daft-Freak avatar Apr 11 '22 10:04 Daft-Freak

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!

connorjclark avatar Apr 12 '22 06:04 connorjclark

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 ...)

Daft-Freak avatar Apr 12 '22 10:04 Daft-Freak

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

sherpya avatar Aug 25 '22 10:08 sherpya

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

slouken avatar Aug 25 '22 13:08 slouken