Type error in emscripten_webgl_do_create_context

[JoeOsborn]

If client code asks for explicit swap control, a transferred canvas record is obtained from GL.offscreenCanvases, but then that record (and not the underlying OffscreenCanvas) is passed through to GL.createContext, which expects an actual offscreen canvas. The fix is to add ....offscreenCanvas || ....canvas on the end of line 174, or else to change the code of emscripten_webgl_do_create_context to unpack the canvas.

emscripten/src/library_html5_webgl.js

Line 174 in 5203d08

canvas = GL.offscreenCanvases[canvas.id];

[JoeOsborn]

findCanvasEventTarget has a similar bug, in that it will sometimes return a key from GL.offscreenCanvases, sometimes return an element from GL.offscreenCanvases, and sometimes return an arbitrary HTML element. It should always return something of a reasonable type, whether or not offscreen canvas support is enabled. (In particular it does not make sense for it to sometimes return an element of GL.offscreenCanvases and sometimes an HTML element.)

[sbc100]

Hmm, so in that case it seems like that GL.createContext call will always fail if explicitSwapControl is requested and transferControlToOffscreen exists (i.e. offscreen canvas is supported).

I guess that means we don't have any testing of this configuration?

Would you have time to send a PR for that? It seems like the fix would be to add .canvas to the end of line 174 (e.g. canvas = GL.offscreenCanvases[canvas.id].canvas?

[JoeOsborn]

I tried adding .canvas there, but sometimes the result is later accessed by its canvasSharedPtr or offscreenCanvas fields. I think this function’s callers should be checked to see which ones want shared pointers to offscreen/transferred canvases and should use the GL.offscreenCanvases map only, and which ones want “an element of type canvas or offscreencanvas” and need to return the element’s offscreen canvas or canvas fields if it has been transferred, or the result of findEventTarget otherwise.

I’m planning to look into this Monday in case no one with more expertise has a chance to look by then.

[sbc100]

The function (emscripten_webgl_do_create_context) is returning the result of GL.createContext though right? Not the canvas.

The canvas variable on line 174 is only ever used in the call to GL.createContext isn't it? I don't see it used anywhere else.

[JoeOsborn]

Sorry, by “this function” I meant findCanvasEventTarget. I think you’re right that adding .canvas in do_create_context is the right fix, but other similar issues arise from the incompatible types coming out of findCanvasEventTarget.

[sbc100]

OK that sounds right. I guess we can/should fix do_create_context separately, and we should try to add some testing (since I guess there cannot be any testing today since the GL.createContext will fail in this code path won't it?

[JoeOsborn]

I believe changing just do_create_context is enough. I can make PRs tomorrow for this and for #22942 as two separate patches.

[JoeOsborn]

This was resolved by #22958 .