Feature for `web_sys::HtmlCanvasElement` or `web_sys::OffscreenCanvas`
parasyte opened this issue · 21 comments
Following from rust-windowing/winit#2259, the WebWindowHandle.id is a flaky way to share canvas references between crates. As discussed in that thread, synchronizing the IDs is challenging (leading to duplicates) and it isn't possible to query canvas elements in a Shadow DOM or offscreen canvases at all.
What consumers need is a way to pass the canvas reference through HasRawWindowHandle. AFAICT, that can be done with either an enum with variants for web_sys::HtmlCanvasElement and web_sys::OffscreenCanvas, or with wasm_bindgen::JsValue. It looks like another option is the FromWasmAbi and IntoWasmAbi traits provided by wasm_bindgen if the reference needs to cross the WASM ABI boundary for some reason.
I think the best way forward in the short-term is a feature to enable the use of one of these reference types in addition to the id: u32. It is preferable to replace the id entirely but may not be realistic without a deprecation period. The feature also mitigates a potential downside that it adds a dependency on web_sys and/or wasm_bindgen.
Open to discussion and other alternatives, but the current design appears to be a significant shortcoming in web environments.
The current design also doesn't allow writing independent components that are added to a web application, since there's no guarantee that it's the only component using raw-window-handle, and thus the data-raw-handle attribute could be in conflict.
The only quick fix I can think of right now is to always use a random number for the id and hope that you never get the same number for two canvases in the same DOM.
For example, bevy always uses "1" there, which means that if you had two canvases in the DOM that use bevy, both want to use the first one they can find (I just tested that).
One more difficulty in adding a platform-specific dependency is that we'll be reintroducing a cfg guard that we've otherwise tried quite hard to remove: #63.
For what it's worth, the reasoning for the current API is discussed in #26 (comment) mentions some of the downsides of exposing web_sys types directly
For what it's worth, the reasoning for the current API is discussed in #26 (comment) mentions some of the downsides of exposing web_sys types directly
That's from before OffscreenCanvas was usable. Firefox enabled their implementation by default just in the last update, that's why things have changed now.
Also, I'm not sure if anybody is still using stdweb. The last update to that crate was almost three years ago.
I linked to the same thread in the winit issue that spawned this one. While I can appreciate the spirit and intention of those decisions, I have a hard time accepting the current situation which pushes the dependencies and complexities to downstream crates. There is going to be little-to-no sharing of implementations between them and that additional surface area increases the likelihood of interoperability bugs.
Also, I'm not sure if anybody is still using stdweb.
This is something I also forgot to mention, thanks for the reminder. I agree it appears most of the ecosystem has settled on web-sys. I'm not sure I would entertain the thought of trying to support anything else.
Dropping stdweb sounds ok.
Using web-sys types directly causes a bit of an issue for anyone using canvases that aren't created from web-sys though (not just stdweb - this includes Emscripten, passing a canvas created manually in JavaScript, etc.). I think using the Abi trait from web-sys would be problematic for those use cases.
I think the point about tying raw-window-handle to a particular version of web-sys/wasm-bindgen is still relevant. I'm not sure of a good way around that, but maybe it's ok because major versions aren't expected to happen that often in practice.
I can see the issue with emscripten, but how does creating it in JavaScript cause issues? You can simply pass it to a wasm function call and wasm-bindgen will wrap it into its own type.
I see it as similar to the Emscripten use case. If you have a canvas somewhere in the DOM already, you can manually set up a raw window handle (using the data attribute) to have everything work fine today, without interacting with wasm-bindgen or web-sys in your own code.
It might not be in the DOM though, or even a DOM element in the first place (OffscreenCanvas).
A solution might be to make WebWindowHandle an enum with three variants:
- the existing
i32 - a
web_sys::HtmlCanvasElement web_sys::OffscreenCanvas
On the emscripten target, only the first one would be supported. wgpu has two different function calls for HtmlCanvasElement and OffscreenCanvas, so an enum would be needed anyways (or dynamic type checking, which I wouldn't like).
Yeah exactly - I agree that the current situation doesn't work for canvases that aren't in the DOM, so we should change something here.
I just wanted to mention some of the original use cases and why it avoided exposing web-sys types (e.g., major version pinning, external canvases not created through web-sys) so we don't forget about those if we change this API.
Well, you could rely on the IntoWasmAbi and FromWasmAbi traits to just store a u32 for the JsValue, but this requires some pretty unsanitary and unsafe code on the implementor side.
Are there any realistic alternatives to types that are known to be valid, without introducing SemVer hazards? Querying the DOM with a u32 is a hack that has reached the limits of its usefulness. And I think that casting i32 as JsValue has the same downsides with SemVer hazards but also introduces the possibility of Really Bad Things happening in the worst case.
The benefit of using the web-sys types is that it removes a lot of potential errors and it doesn't have any issues with querying the DOM or ID collisions mentioned elsewhere. The downside as mentioned in #26 (comment) is that wasm-bindgen maintainers do not want to have JsValue treated as one would use a raw pointer.
If there truly is no safe way to type-erase HtmlCanvasElement and OffscreenCanvas, then at least I am personally ok with the compromise of risking SemVer hazards and depending directly on the web-sys types. It would still be miles better than pretending that querying the DOM solves everything.
I also want to address this specific comment from that thread:
Setting ourselves up for either a breaking change or forcing our users to use legacy code would fundamentally undermine the stability and common ground that this library is supposed to provide, and as such is an unacceptable solution.
It sounds nice in theory that one can avoid breaking changes entirely, but we've already seen at least 5 minor release versions here with various breaking changes. In fact, winit 0.27 brought in both 0.4 and 0.5 to address breaking changes: rust-windowing/winit#2418 Given these facts, I'm entirely unconvinced that "avoiding breaking changes at all costs" is attainable or even wise. And I bet if you look at the state of the art, you'll find that the one thing that is proven to work is maintaining backward compatibility with some deprecation strategy.
In other words, when (or rather if) web-sys 2.0 releases, then raw-window-handle does need to follow suit to ensure compatibility. But it can be done in a backward-compatible way as necessary. As far as compromises go this sounds reasonable. I don't believe it's "all or nothing" as implied by the quoted comment.
I haven't had time to review the thread and try to read the links, but I can say this: Breaking changes are certainly to be avoided, but since we're a communication library, our hand really is forced any time the data we want to communicate changes.
Implementation wise: We could probably just add a new variant WebWindowHandle::Web2(Web2WindowHandle), deprecate the old one, and fix it next time breaking changes are required.
It should probably be noted that web-sys technically hasn't had a breaking release in years, so using its types in our public API would likely not force any breaking changes onto us for a while yet (although I'm not all that happy about this seemingly being the only solution for OffscreenCanvas).
There might be another solution for OffscreenCanvas, but I haven't proven it out. Rather than synchronizing both sides through the DOM, the synchronization can be done through the JavaScript global scope. The disadvantages that I know of are:
- Web workers cannot access
global(akawindow) without some help via message passing (postMessage) with another context that does have access to it. - It has the same ID collision issue as the DOM.
- It would "pollute" the global namespace with some new field containing
Map<u32, HtmlCanvasElement | OffscreenCanvas>.
The additional global "pollution" and not addressing the ID collision issue are minor annoyances, but without good support for web workers this would not support the Bevy use case. Looking at the example made by @anlumo it looks like the helper would have to be provided by the end user creating the Worker, ala https://github.com/anlumo/bevy-webworker-test/blob/44a4eef9942c6b65815ef7be2d26ac781b1d9b58/wasm/index.html#L18-L20
That said, I am by no means qualified to recommend something like this. It really smells like another kludge to avoid a shared dependency. So, I'll leave this here as a FWIW.
A collision-free id could be generated by self.crypto.randomUUID() (documentation), it just needs to be a string instead of a u32 (a u128 would also work technically, but then you'd have to convert the hex string back and forth).
One issue I have with this solution is that it requires the application developer to add some weird code to the codebase. Since the canvas is created outside of raw-window-handle and thus this Map doesn't exist yet, the developer would have to create this global object on their side with some specified name (so the crate using raw-window-handle can find it), generate the id and then insert the canvas.
It would look something like this:
const canvas = document.createElement('canvas');
document.body.appendChild(canvas);
const uuid = self.crypto.randomUUID();
if (window.rawWindowHandleCanvases) {
window.rawWindowHandleCanvases.set(uuid, canvas);
} else {
window.rawWindowHandleCanvases = new Map([[uuid, canvas]]);
}
// send uuid to Rust code hereThis is for the regular case without OffscreenCanvas and without Web Workers. This could also be done in Rust via web-sys of course.
I worked out an interesting workaround for the shadow dom issue at construction time.
https://github.com/Zageron/pixels-shadow-dom-example
I have not tried yet, but I imagine I could:
- instantiate a shadow component with a canvas
- get a ref to the canvas
- append child to the Dom root, hidden.
- remove child from shadow dom
- initialize library that uses raw handle
- when finished, remove child from root, add back to shadow dom.
Quite annoying compared to just passing the canvas in and having it work, but at least there is an option that will likely work.
For the sub-goal of avoiding ID collisions, this could be done by querying document.querySelectorAll("[data-raw-handle]") to find existing elements with a data-raw-handle value, then pick a value not already used. Of course, this is an O(N²) algorithm, but N is likely to be very small. (A fancier version could first try picking a weakly-random or sequential ID and check if it is occupied, which will succeed fast in the likely-common case of exactly one Rust canvas. Also, browsers limit the number of active GPU-using canvas contexts to a fairly small number, too.)
This does not solve problems like confusion over which Document is in use or difficulty accessing it (web workers, shadow DOM), so I personally would rather see some solution involving an actual DOM node reference, but the above algorithm can be implemented within winit and other crates implementing HasRawWindowHandle without any breaking changes or new dependencies.
One issue I have with this solution is that it requires the application developer to add some weird code to the codebase.
Not any weirder than the status quo. The RWH provider (like winit) is responsible for managing the mapping, just like it is responsible today for adding the data-raw-handle attribute to a canvas element that it may optionally create and append to the DOM. The consumer side also needs to be aware of this protocol, just as wgpu today queries the DOM for the right data-raw-handle attribute.
I think the only difference with using the global scope instead of the DOM is that it allows access to OffscreenCanvas (which cannot be stored in the DOM). Regardless, it still requires some consumer-side protocol awareness with Web Workers to share the reference between contexts.
Still not arguing for this as a solution. Just pointing out that it does have advantages over the DOM.