Skip to content

Conversation

@daxpedda
Copy link
Member

@daxpedda daxpedda commented Feb 28, 2023

When I tried actually using Softbuffer with #76 in a multi-threaded Wasm environment using transferControlToOffscreen() I found that Softbuffer actually fails on ImageData::new_with_u8_clamped_array(wasm_bindgen::Clamped(&bitmap), width.into()).

Apparently it is not allowed to create a ImageData from a SharedArrayBuffer. I only found a couple of Stackoverflow issues facing this (#1, #2) but no actual documentation on MDN or WHATWG.

This fixes it by copying the data to a Uint8Array first and then creating the ImageData from that. Which is an extra copy that can't be avoided when using multi-threaded Wasm. I hid it all behind #[cfg(target_feature = "atomics")] to make sure not to affect people not needing this.

This can be further optimized by keeping the Uint8Array and not having to recreate it from scratch again every time. I thought maybe we should wait for #65 before that, happy to add it now if desired.
This could be further optimized if we add an additional function for Web that uses a copy API instead of the currently owned buffer one.

To make this happen I had to do an import by hand because web-sys doesn't actually provide the constructors we need for ImageData.

I also added -Ctarget-feature=+atomics to the CI separately to the regular Wasm build.

See the wasm-bindgen documentation for more information on multi-threaded Wasm.

This can be run locally with RUSTFLAGS=-Ctarget-feature=+atomics,+bulk-memory cargo +nightly run-wasm --example winit -Zbuild-std=panic_abort,std.

@daxpedda
Copy link
Member Author

I am assuming that the CI test I added is not executed because of some security settings on the repo.

@daxpedda daxpedda marked this pull request as ready for review March 1, 2023 15:47
@daxpedda daxpedda requested a review from john01dav as a code owner March 1, 2023 15:47
@daxpedda
Copy link
Member Author

daxpedda commented Mar 1, 2023

Now that we decided that #76 should not be merged until wasm-bindgen releases a new version with the imports we needed (#76 (comment)), I rebased this PR onto master.

To address any potential concerns (considering @madsmtm didn't want to use custom imports but wait for wasm-bindgen/web-sys), the imports I introduced here won't make it into the wasm-bindgen ecosystem. This is because they already have their own custom variant of Uint8ClampedArray which really doesn't work the same way other wasm-bindgen imports work. This would require a breaking change on their side, not really something that will happen anytime soon.

@daxpedda
Copy link
Member Author

CI fails because the new version of raw-window-handle now requires Rust v1.64, but our CI runs on v1.60:
https://docs.rs/crate/raw-window-handle/0.5.1/source/Cargo.toml

Copy link
Member

@notgull notgull left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If that's the blessed way of doing it, then I suppose everything else looks good enough.

@notgull
Copy link
Member

notgull commented Mar 18, 2023

Try rebasing on top of the new master

@daxpedda
Copy link
Member Author

Done!

@daxpedda daxpedda force-pushed the wasm-atomics branch 2 times, most recently from 3a21c8f to f3e1435 Compare April 6, 2023 15:32
@daxpedda
Copy link
Member Author

daxpedda commented Apr 6, 2023

Rebased after #65.

@notgull
Copy link
Member

notgull commented Apr 6, 2023

This works on my machine under run-wasm, so I'll just merge it.

@notgull notgull merged commit 379910c into rust-windowing:master Apr 6, 2023
This was referenced Apr 6, 2023
@notgull notgull mentioned this pull request Jun 4, 2023
notgull added a commit that referenced this pull request Jun 4, 2023
* On MacOS, the contents scale is updated when set_buffer() is called, to adapt when the window is on a new screen (#68).
* **Breaking:** Split the `GraphicsContext` type into `Context` and `Surface` (#64).
* On Web, cache the document in the `Context` type (#66).
* **Breaking:** Introduce a new "owned buffer" for no-copy presentation (#65).
* Enable support for multi-threaded WASM (#77).
* Fix buffer resizing on X11 (#69).
* Add a set of functions for handling buffer damage (#99).
* Add a `fetch()` function for getting the window contents (#104).
* Bump MSRV to 1.64 (#81).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants