Fix re-creating webgl context for offscreen canvas #23518

JoeOsborn · 2025-01-28T20:26:47Z

This adds a test and a one-line fix to resolve a bug. After a canvas has been transferred offscreen, it can't be used again to create a webgl context because the result of findCanvasEventTarget is a {canvas:...} map instead of an actual canvas object. This patch grabs the underlying offscreencanvas if it exists.

sbc100

Are there other places in the codebase that do the same unpacking? It seems a little odd to have findCanvasEventTarget return such disparate types.. but I guess that is the status quo.

test/browser/webgl_create_context_swapcontrol.c

sbc100 · 2025-01-28T20:33:37Z

test/browser/webgl_create_context_swapcontrol.c

+  // Test that creating a context for a canvas that's already been used once succeeds
+  context = emscripten_webgl_create_context("#canvas", &attrs);
+  assert(context > 0); // Must have received a valid context.
+  res = emscripten_webgl_make_context_current(context);


Which variant of this test fails without the fix? Is there some kind of thread involved here or are we just rendering to and offscreen buffer on the main thread?

This error shows up even without threads, so I thought using the existing webgl explicit swap control test would be simplest.

Presumably the issue only shows up when USE_OFFSCREEN_FRAMEBUFFE is used above?

It's broken in the condition without the offscreen framebuffer, which shows that the problem is with the offscreencanvas support rather than an offscreen framebuffer.

I see, but where in this test is offscreencanvas used then?

It's used because OFFSCREENCANVAS_SUPPORT is 1 in the build settings and explicitSwapControl is true in the context.

This patch eliminates the need for asyncify and uses modern filesystem APIs instead of the deprecated, unmaintained BrowserFS. This is a WIP patch because it won't fully work until these two Emscripten PRs land and are released: emscripten-core/emscripten#23518 emscripten-core/emscripten#23021 The former fixes an offscreen canvas context recreation bug, and the latter adds an equivalent to BrowserFS's XHR filesystem (but without the hazardous running-XHR-on-the-main-thread problem). The biggest issue is that local storage of users who were using the old version of the webplayer will be gone when they switch to the new webplayer. I don't have a good story for converting the old BrowserFS IDBFS contents into the new OPFS filesystem (the move is worth doing because OPFS supports seeking and reading only bits of a file, and because BrowserFS is dead). I've kept around the old libretro webplayer under pkg/emscripten/libretro-classic, and with these make flags you can build a non-workerized RA that uses asyncify to sleep as before: make -f Makefile.emscripten libretro=$CORE HAVE_WORKER=0 HAVE_WASMFS=0 PTHREAD=0 HAVE_AL=1 I also moved the default directory for core content on emscripten to not be a subdirectory of the local filesystem mount, because it's confusing to have a subdirectory that's lazily fetched and not mirrored to the local storage. I think it won't impact existing users of the classic web player because they already have a retroarch.cfg in place.

sbc100 · 2025-01-28T23:02:55Z

src/lib/libhtml5_webgl.js

@@ -109,7 +109,7 @@ var LibraryHtml5WebGL = {
 #endif

    var canvas = findCanvasEventTarget(target);
-
+    if (canvas.canvas) { canvas = canvas.canvas; }


Looks like you canvas can be null here? How about canvas?.canvas?

Yes, I think the condition in the if also must be guarded. if (canvas && canvas.canvas) { canvas = canvas.canvas }?

sbc100 · 2025-01-28T23:29:10Z

src/lib/libhtml5_webgl.js

@@ -109,7 +109,7 @@ var LibraryHtml5WebGL = {
 #endif

    var canvas = findCanvasEventTarget(target);
-    if (canvas.canvas) { canvas = canvas.canvas; }
+    if (canvas && canvas.canvas) { canvas = canvas.canvas; }


I think you can write canvas?.canvas instead of canvas && canvas.canvas?

Yes, that's true. I'll fix it.

This patch eliminates the need for asyncify and uses modern filesystem APIs instead of the deprecated, unmaintained BrowserFS. This is a WIP patch because it won't fully work until these two Emscripten PRs land and are released: emscripten-core/emscripten#23518 emscripten-core/emscripten#23021 The former fixes an offscreen canvas context recreation bug, and the latter adds an equivalent to BrowserFS's XHR filesystem (but without the hazardous running-XHR-on-the-main-thread problem). The biggest issue is that local storage of users who were using the old version of the webplayer will be gone when they switch to the new webplayer. I don't have a good story for converting the old BrowserFS IDBFS contents into the new OPFS filesystem (the move is worth doing because OPFS supports seeking and reading only bits of a file, and because BrowserFS is dead). I've kept around the old libretro webplayer under pkg/emscripten/libretro-classic, and with these make flags you can build a non-workerized RA that uses asyncify to sleep as before: make -f Makefile.emscripten libretro=$CORE HAVE_WORKER=0 HAVE_WASMFS=0 PTHREAD=0 HAVE_AL=1 I also moved the default directory for core content on emscripten to not be a subdirectory of the local filesystem mount, because it's confusing to have a subdirectory that's lazily fetched and not mirrored to the local storage. I think it won't impact existing users of the classic web player because they already have a retroarch.cfg in place.

* workerized RA * Workerized (non-async) web player, using OPFS This patch eliminates the need for asyncify and uses modern filesystem APIs instead of the deprecated, unmaintained BrowserFS. This is a WIP patch because it won't fully work until these two Emscripten PRs land and are released: emscripten-core/emscripten#23518 emscripten-core/emscripten#23021 The former fixes an offscreen canvas context recreation bug, and the latter adds an equivalent to BrowserFS's XHR filesystem (but without the hazardous running-XHR-on-the-main-thread problem). The biggest issue is that local storage of users who were using the old version of the webplayer will be gone when they switch to the new webplayer. I don't have a good story for converting the old BrowserFS IDBFS contents into the new OPFS filesystem (the move is worth doing because OPFS supports seeking and reading only bits of a file, and because BrowserFS is dead). I've kept around the old libretro webplayer under pkg/emscripten/libretro-classic, and with these make flags you can build a non-workerized RA that uses asyncify to sleep as before: make -f Makefile.emscripten libretro=$CORE HAVE_WORKER=0 HAVE_WASMFS=0 PTHREAD=0 HAVE_AL=1 I also moved the default directory for core content on emscripten to not be a subdirectory of the local filesystem mount, because it's confusing to have a subdirectory that's lazily fetched and not mirrored to the local storage. I think it won't impact existing users of the classic web player because they already have a retroarch.cfg in place. * Get fetchfs working without manifest support * makefile fixes

fix re-creating a webgl context on an offscreen canvas

JoeOsborn · 2025-02-10T05:49:49Z

I think this CI issue is just a code size change?

kripken · 2025-02-13T20:51:17Z

If this PR changes code size, you can update those locally by running the tests with --rebaseline (just add that in the test/runner.py command). Then commit and upload the changes for review.

src/lib/libhtml5_webgl.js

* workerized RA * Workerized (non-async) web player, using OPFS This patch eliminates the need for asyncify and uses modern filesystem APIs instead of the deprecated, unmaintained BrowserFS. This is a WIP patch because it won't fully work until these two Emscripten PRs land and are released: emscripten-core/emscripten#23518 emscripten-core/emscripten#23021 The former fixes an offscreen canvas context recreation bug, and the latter adds an equivalent to BrowserFS's XHR filesystem (but without the hazardous running-XHR-on-the-main-thread problem). The biggest issue is that local storage of users who were using the old version of the webplayer will be gone when they switch to the new webplayer. I don't have a good story for converting the old BrowserFS IDBFS contents into the new OPFS filesystem (the move is worth doing because OPFS supports seeking and reading only bits of a file, and because BrowserFS is dead). I've kept around the old libretro webplayer under pkg/emscripten/libretro-classic, and with these make flags you can build a non-workerized RA that uses asyncify to sleep as before: make -f Makefile.emscripten libretro=$CORE HAVE_WORKER=0 HAVE_WASMFS=0 PTHREAD=0 HAVE_AL=1 I also moved the default directory for core content on emscripten to not be a subdirectory of the local filesystem mount, because it's confusing to have a subdirectory that's lazily fetched and not mirrored to the local storage. I think it won't impact existing users of the classic web player because they already have a retroarch.cfg in place. * Get fetchfs working without manifest support * makefile fixes * fix scaling, remove zip dependency * Support asset/cheats/etc downloaders for emscripten - Add http transfer support for emscripten - At the task_http level, not the net_http level --- so no netplay or webdav. - Change default paths to be more like other platforms - Gives us smaller bundles and a faster boot time - Had to work around a task queue bug on Emscripten - I made the smallest possible change to do it, but it may be better to fix in rthread.c * Load an emscripten file_packager package on first run If no ozone assets are present, load a libretro_minimal package created using Emscripten's built-in file packager. * updated readme, removed indexer from wasmfs libretro-web * Put back zip dependency, load asset bundle into opfs on first run * fix upload path * Remove unused function * easy testing setup for two multithreaded conditions 1. make PROXY_TO_PTHREAD=1 (slower) 2. make PROXY_TO_PTHREAD=0 (bad audio, because doesn't sleep in openal.c) * Remove condition on sleep in openal also make input_driver check existence of drv->axis, drv->button before calling them. * Fix resizing under EGL * Don't force config file path on emscripten * Add time.h include to netplay, default HAVE_NETPLAYDISCOVERY to 0 * Remove nearly all proxied joypad calls under emscripten * Fix file uploads under firefox * Fix safari API uses, but Safari still hangs in OPFS filesystem mount I think this can be fixed by moving the backend creation off the main thread. * Move filesystem init into emscripten C entry point * Setup filesystems off of main thread * re-set default player to async Also improve Safari compatibility under proxy-to-pthread condition * Safari upload file fixes * Remove some excess prints * Fix typo

sbc100 approved these changes Jan 28, 2025

View reviewed changes

JoeOsborn force-pushed the offscreen-canvas-recreate-context branch from 9581535 to 7b00b36 Compare January 28, 2025 20:58

JoeOsborn mentioned this pull request Jan 28, 2025

Workerized emscripten retroarch (WIP) libretro/RetroArch#17484

Merged

sbc100 reviewed Jan 28, 2025

View reviewed changes

JoeOsborn force-pushed the offscreen-canvas-recreate-context branch 2 times, most recently from 0de1a93 to 8c13b2a Compare January 29, 2025 16:58

JoeOsborn added 2 commits February 9, 2025 19:45

add failing test for context recreation

878fff9

fix re-creating a webgl context on an offscreen canvas

Guard canvas access in case no canvas was found

7039dd8

JoeOsborn force-pushed the offscreen-canvas-recreate-context branch from 8c13b2a to 7039dd8 Compare February 10, 2025 03:45

rebaseline webgl code sizes

bb03600

kripken reviewed Feb 14, 2025

View reviewed changes

src/lib/libhtml5_webgl.js Show resolved Hide resolved

JoeOsborn added 2 commits February 14, 2025 16:58

add comment and guard canvas extraction on offscreencanvas_support

abb79ce

rebaseline code size tests

9731e84

JoeOsborn mentioned this pull request Feb 17, 2025

Enable pthreads on Emscripten libretro/RetroArch#17586

Merged

kripken approved these changes Feb 18, 2025

View reviewed changes

kripken merged commit cb88605 into emscripten-core:main Feb 18, 2025
29 checks passed

dschuff mentioned this pull request Mar 14, 2025

Temporarily disable some tests #23933

Merged

dschuff mentioned this pull request Mar 29, 2025

Revert "Temporarily disable some tests (#23933) #24011

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix re-creating webgl context for offscreen canvas #23518

Fix re-creating webgl context for offscreen canvas #23518

JoeOsborn commented Jan 28, 2025

sbc100 left a comment

sbc100 Jan 28, 2025

JoeOsborn Jan 28, 2025

sbc100 Jan 28, 2025

JoeOsborn Jan 28, 2025 •

edited

Loading

sbc100 Jan 28, 2025 •

edited

Loading

JoeOsborn Jan 28, 2025

sbc100 Jan 28, 2025

JoeOsborn Jan 28, 2025

sbc100 Jan 28, 2025

JoeOsborn Jan 28, 2025

JoeOsborn commented Feb 10, 2025

kripken commented Feb 13, 2025

Fix re-creating webgl context for offscreen canvas #23518

Fix re-creating webgl context for offscreen canvas #23518

Conversation

JoeOsborn commented Jan 28, 2025

sbc100 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JoeOsborn Jan 28, 2025 • edited Loading

Choose a reason for hiding this comment

sbc100 Jan 28, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JoeOsborn commented Feb 10, 2025

kripken commented Feb 13, 2025

JoeOsborn Jan 28, 2025 •

edited

Loading

sbc100 Jan 28, 2025 •

edited

Loading