OffscreenCanvas Support for WebGL Backend

[haraldreingruber-dedalus]

Connections
Ideas discussed in #1686, Thanks @zicklag for helping me to get started 🙏

Description
The goal is to expose create_surface_from_canvas() and create_surface_from_offscreen_canvas() also for the WebGL Backend.

Testing
Will be tested in Firefox and Chrome on Windows 10.
The skybox_offscreen example can be used for testing.

[haraldreingruber-dedalus]

I think this PR is ready for review.

Looking forward to improvement ideas!
Also, I am not sure if it is a good idea, to remove the unsafe prefixes from the functions in lib.rs. It looks like they are not needed for the WebGPU-/WebGL-specific functions.

[kvark]

@zicklag would you be able to review?

[haraldreingruber-dedalus]

The new skybox_offscreen is a copy of the skybox example but contains only the Wasm code.
I thought for the sake of nice examples, a copy is better than avoiding code duplication... But I am happy to follow along with a different approach if concerns about maintainability have higher priority.

[kvark]

It would definitely be unfortunate to duplicate any examples for WebGL + OffscreenCanvas specifically. If you can make it by just modifying the example (conditionally on the target platform, potentially), that would be great!

[haraldreingruber-dedalus]

If you can make it by just modifying the example (conditionally on the target platform, potentially), that would be great!

Sure. Does it make sense to add an offscreen-example feature for compiling the examples conditionally?

[kvark]

Sure. Does it make sense to add an offscreen-example feature for compiling the examples conditionally?

Does it have to be a feature? Can it be just a run-time switch for the skybox example?

[haraldreingruber-dedalus]

I like the idea, I guess the only question is how to activate it in the wasm environment? Any ideas?

Depending on the approach, maybe it's an advantage to provide it for all examples. It has to be implemented in the shared framework.rs anyway.

[zicklag]

I've done runtime flags for WASM before by reading the query string. So you could access the example with http://localhost:1234?offscreen_canvas=true. It's easy to get the query string with web_sys::window()?.location().search()?, then a naive parser for the args that just uses String::split can get the key-value pairs out of it so you can check for any flags you want to add.

---

Not sure when I'll get to being able to do a review, but I'll try to get to it soon if I have time.

[haraldreingruber-dedalus]

I've done runtime flags for WASM before by reading the query string.

Cool, I will try that in the next couple of days or on the weekend latest. Maybe this way it could be allowed for all examples. Which is probably nice for testing.
In the meanwhile, please ignore the current changes in the example folder.

[haraldreingruber-dedalus]

@zicklag I was able to make OffscreenCanvas usage configurable in all examples via http://localhost:8000/?offscreen_canvas=true

[cwfitzgerald]

What's the current status of this PR? Any way I can help drive it forward?

[haraldreingruber-dedalus]

What's the current status of this PR? Any way I can help drive it forward?

Thanks for running the CI pipeline. I've fixed the fmt errors. Looks like the pipeline doesn't allow build warnings. Will look into that as well and let you know when fixed.

Other than that the PR is ready to be reviewed.
The OffscreenCanvas approach can be tested with cargo run-wasm --example cube --features webgl
and opening http://localhost:8000/?offscreen_canvas=true

[haraldreingruber-dedalus]

Okay, I was able to check locally at least the Windows and WebAssembly warnings.
If I was able to run with the same configuration, it should be fixed now.

@cwfitzgerald could you approve again the workflow run for this PR?

Would you also review the PR?
I just realized that many but not all examples use the common framework.rs module, so the OffscreenCanvas functionality can not be tested (yet) with the hello-triangle example.

skybox or cube can be tested as mentioned (updated) above.

The OffscreenCanvas approach can be tested with cargo run-wasm --example cube --features webgl
and opening http://localhost:8000/?offscreen_canvas=true

[zicklag]

I just did a quick review and it looks fine to me, but it wasn't a very thorough review, more like a quick sanity check to see if anything sticks out to me.

I tested the cube and skybox examples and they work in my browser: Brave Browser 97.1.34.80 unknown ( a chrome derivative ).

I noticed the shadow example is broken for me, but it's like that on master, so not because of this PR.

My first thought is that it's probably fine just to use the offscreen canvas example in framework.rs and leave the hello-triangle example simpler, but I'm not a maintainer so that's not my call. :)

Good job, this looks neat!

[haraldreingruber-dedalus]

Thanks a lot for the feedback @zicklag!

Do you remember if the shadow example was working for webgl in the past? If yes, I would try to look into that (for a separate PR) in the next couple of weeks....

[zicklag]

Do you remember if the shadow example was working for webgl in the past? If yes, I would try to look into that (for a separate PR) in the next couple of weeks....

Yeah it was working back when we first got the WebGL stuff working again, but I think I noticed the demo on the website not working maybe ~3 months ago or more ( I hadn't really kept track ). I wasn't sure if it was my browser or something, but the fancy Bevy PBR & glTF demos still work, so it might be just a problem with the example and not with the backend. Or it's a small backend workaround that needs to be implemented to avoid a problem that only shows up in the example.

I'm getting this error every frame:

[.WebGL-0x3c540933d400] GL_INVALID_OPERATION: It is undefined behaviour to use a uniform buffer that is too small.

( But I should probably open a new issue, I just hadn't gotten the chance yet. )

[cwfitzgerald]

could you approve again the workflow run for this PR?

Really wish there was an "approve running for this whole pr". I get why but it's exceedingly annoying when you have CI that can't really be hijacked.

My first thought is that it's probably fine just to use the offscreen canvas example in framework.rs and leave the hello-triangle example simpler, but I'm not a maintainer so that's not my call. :)

Agree with @zicklag here, hello-triangle needn't do everything, it's job is to be simple.

Other than that the PR is ready to be reviewed.
The OffscreenCanvas approach can be tested with

Great, will review in the next couple days

[haraldreingruber-dedalus]

Really wish there was an "approve running for this whole pr". I get why but it's exceedingly annoying when you have CI that can't really be hijacked.

Is workflow run approval only required for the first-time contribution? If yes, a workaround could probably be a "fake" contribution after the initial PR.

[haraldreingruber-dedalus]

The remaining build issues are because of the missing functions in the "webgl+emscripten" flavor.
Not sure if it makes sense to have something similar there as well. Probably it's better to extend the wasm+webgl compile flags by not(emscripten)...

Does anyone know how the emscripten version is typically used? Would it be an interesting use case to pass a Canvas or OffscreenCanvas created externally?

[grovesNL]

How should web workers fit into this PR (if at all)? Or are we focused on the use case of targeting multiple canvas on the same UI thread for now?

Does anyone know how the emscripten version is typically used? Would it be an interesting use case to pass a Canvas or OffscreenCanvas created externally?

It would probably be useful to expose offscreen canvas support to Emscripten eventually, but for simplicity we could skip it for now. As far as I know, a lot of applications using Emscripten use a default canvas created by Emscripten.

[haraldreingruber-dedalus]

How should web workers fit into this PR (if at all)? Or are we focused on the use case of targeting multiple canvas on the same UI thread for now?

To me, the main use case is updating multiple canvases with the same OffscreenCanvas as a workaround to share resources between them. But if the changes required for web-worker support would be small, we could consider it as well. Otherwise, a separate PR might be better. @grovesNL Do you have experience with that?

It would probably be useful to expose offscreen canvas support to Emscripten eventually, but for simplicity we could skip it for now. As far as I know, a lot of applications using Emscripten use a default canvas created by Emscripten.

I agree, it's probably better to keep things simple at least until somebody shows interest.

[cwfitzgerald]

Is workflow run approval only required for the first-time contribution? If yes, a workaround could probably be a "fake" contribution after the initial PR.

Yeah, it's just first time. It's not enough of a hassle to bother doing "intro" PRs, but enough that I want to complain about it 😆

[cwfitzgerald]

Looks good, minus some small nits!

[haraldreingruber-dedalus]

@cwfitzgerald please also approve the workflow run. If I checked the right things, the builds should pass now. 😉

[haraldreingruber-dedalus]

Cool, the build was successful.
Thanks for your feedback @cwfitzgerald

Let me know if there are more improvement ideas. Otherwise, I'm looking forward to getting this merged 😊

[cwfitzgerald]

LGTM! Thank you!