Add wasm32-unknown-unknown support #92

grovesNL · 2019-03-01T06:42:15Z

This PR adds wasm32-unknown-unknown support for GLSL/ESSL compilation. Other compilers could easily be added to the Emscripten bundle, but it only includes the GLSL compiler for now.

This works by manually writing function bindings to redirect native Rust -> C++ calls to become Rust/wasm32-unknown-unknown -> C++/Emscripten calls. Rust and C++ have separate wasm memory, so we have to do some copying between both of these in order to pass arguments back and forth. There is some more information in gfx-rs/gfx#2554

There are a few parts to this PR:

A crate to generate an Emscripten bundle from the C wrapper of SPIRV-Cross
The generated Emscripten bundle which contains everything needed for compiling SPIR-V to GLSL/ESSL
- This bundle could easily include MSL and HLSL compilation too, but at the moment it's not clear how best to support all combinations (other than providing one large bundle), and we ideally don't want to require Emscripten binaries to be installed as a dependency. So for now to keep it straightforward we'll just keep the bundle limited to GLSL/ESSL, unless somebody needs HLSL/MSL support in wasm.
Some Emscripten and pointer utilities to unify access to pointers (to make Emscripten heap offsets look like pointers) so we can avoid changing the API. I think these could still be improved a lot, especially because it becomes confusing when accessing the wrong heap (Rust vs. Emscripten) but this seems to work ok for now. There is a bit of copying that could probably be made more efficient.

I've been testing this so far through an example so I can run native and wasm side-by-side to check the output of reflection/compilation:

In the future I want to expand the native test cases to improve coverage (there could still be a few calls omitted from my example) and figure out the best approach to run these tests in wasm too.

grovesNL · 2019-03-02T06:19:43Z

I still need to remove some leftover warnings, but at this point I think this is ready to review.

codecov · 2019-03-02T06:26:35Z

Codecov Report

Merging #92 into master will decrease coverage by 15.23%.
The diff coverage is 64.79%.

@@             Coverage Diff             @@
##           master      #92       +/-   ##
===========================================
- Coverage    68.6%   53.37%   -15.24%     
===========================================
  Files           5        7        +2     
  Lines         755     1036      +281     
===========================================
+ Hits          518      553       +35     
- Misses        237      483      +246

Impacted Files	Coverage Δ
spirv_cross/src/bindings_native.rs	`0.39% <ø> (ø)`
spirv_cross/src/compiler.rs	`76% <57.53%> (+0.92%)`	⬆️
spirv_cross/src/spirv.rs	`55.64% <75%> (+0.27%)`	⬆️
spirv_cross/src/glsl.rs	`85.22% <84.61%> (+11.15%)`	⬆️
spirv_cross/src/msl.rs	`58.57% <84.61%> (+0.29%)`	⬆️
spirv_cross/src/hlsl.rs	`73.84% <85.71%> (ø)`	⬆️
spirv_cross/src/ptr_util.rs	`92.3% <92.3%> (ø)`
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 14fd3d0...a5a299a. Read the comment docs.

kvark

This looks like magic to me. We might want to ask an extra review here from somebody who knows a bit of WASM interop.

kvark · 2019-03-12T02:08:11Z

spirv_cross/Cargo.toml

+crate-type = ["cdylib", "rlib"]
+
+[features]
+default = ["glsl", "hlsl", "msl"]


let's not have anything by default

kvark · 2019-03-12T02:10:11Z

spirv_cross/src/bindings_wasm.rs

+        #[repr(u32)]
+        #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
+        pub enum SourceLanguage {
+            SourceLanguageUnknown = 0,


can it be convinced to generate better enums?

Yeah it's probably possible to improve these. These are the same bindings generated by bindgen for bindings_native and are only used internally anyway.

The idea here is that all of the bindgen parts are the same for types. For functions in native, function bindings are generated as part of bindings_native. For functions in wasm, we implement the function bindings manually.

kvark · 2019-03-12T02:16:05Z

spirv_cross/src/emscripten.rs

+        loop {
+            let bytes_read = bytes.len();
+            let offset = &JsValue::from_f64((start_offset + bytes_read) as f64);
+            let byte = Reflect::get(heap, offset).unwrap().as_f64().unwrap() as u8;


what is as_f64() doing here?

The Reflect::get here returns a JsValue, and in order to read the value from JS we have to move through a f64 (https://rustwasm.github.io/wasm-bindgen/api/wasm_bindgen/struct.JsValue.html#method.as_f64).

In general the real issue here is that we have to go through f64 moving through the JS layer, because JS only knows Number. So even though we're trying to read from a Uint8Array, I don't see a way around this for now.

Although there is a slight improvement here for indexing. I asked about a better way to index into a typed array a while ago, and it appears the improvement has been merged since I originally wrote this: rustwasm/wasm-bindgen#1147

Now comes the dumb question! If we target WASM, why do we have to go through JsValue?

This is the part about dynamic linking mentioned briefly in my comment. I'm not sure how to dynamically two link wasm modules and make them use the same memory space. So without having a way to dynamically link, we have to copy things back and forth between wasm modules, and move everything through JS to accomplish it instead.

I searched around for resources about this, but I couldn't find much except some C++ conventions to achieve dynamic linking between two C++ modules with some help from Emscripten. But it would be amazing if we could merge the wasm modules together somehow to avoid the JS layer: it would mean we can drop the majority of the code in this PR and have one code path for both native/wasm.

I don't know if host bindings helps will help with this and unfortunately I couldn't find much about the plans for this, except some wasm design documents describing how it might eventually be accomplished.

Here's a reference document so we can follow the future progress of wasm dynamic linking: https://github.com/WebAssembly/tool-conventions/blob/master/DynamicLinking.md

kvark · 2019-03-12T02:16:30Z

spirv_cross/src/emscripten.rs

+
+    /// Clones all bytes from the heap into a `Vec<u8>` while `should_continue` returns `true`.
+    /// Optionally include the last byte (i.e. to support peeking the final byte for nul-terminated strings).
+    pub unsafe fn read_bytes_into_vec_while<F>(&self, pointer: Pointer, should_continue: F, include_last_byte: bool) -> Vec<u8>


can this be shaped as an iterator?

I think so, although it's only used in a couple places internally. I think one of them would require a collect anyway, so it might be better to leave it as a future optimization for now.

grovesNL · 2019-03-12T05:01:47Z

Thanks for the review! I'm happy to explain any of the magic parts to hopefully make it less magical. Unfortunately there is quite a bit going on here.

I think the Rust <-> JS interop is pretty reasonable on its own expect for things like moving through Number as you noticed already 😃 IMO it's the Rust <-> JS <-> C++ that really complicates everything.

To summarize the main ideas:

We have a C++ memory space (Emscripten)
We have a Rust memory space (wasm32)
We have a JavaScript layer which has access to both C++ and Rust memory spaces
native interop happens by going Rust -> C++ or C++ -> Rust
wasm interop happens by going Rust -> JS -> C++ or C++ -> JS -> Rust
Almost all of our interop is Rust -> C++ in the native target, and we do the same for the wasm target by going Rust -> JS -> C++
We want our Rust -> C++ calls to look the same between native and wasm, so we get bindgen to generate the same types
For Rust -> C++ calls in the native target:
- bindgen will generate all the function bindings directly, and dynamic/static linking will work at runtime
- pointers to repr(C) types can be accessed directly in the C++ code because memory is shared between Rust and C++
For Rust -> C++ calls in the wasm target:
- bindgen doesn't do anything with the functions
  - It can't, because we can't just write extern "C" and expect linking directly to work
  - We don't have a mechanism for dynamically linking two wasm modules yet or easily joining the memory spaces together, i.e. we need to copy between Rust and C++ memory spaces
- JavaScript functions (like _sc_internal_compiler_glsl_new) are exposed from the C++ wasm module
  - Emscripten knows which C++ functions to expose because the function names are passed as arguments to emcc
- We declare these functions and generate bindings with wasm-bindgen so we can call the JavaScript functions from Rust
- We write our own wrappers (like sc_internal_compiler_glsl_new, note the omitted _ prefix to match the native function name)
  - These wrappers accept Rust types (pointers/strings/whatever) which exist in the Rust memory space, then copy them into the C++ memory space, then call the bindings from wasm-bindgen
  - Pointer handling is really tricky here, because all Rust ptr types actually need to become u32 after we copy from Rust memory space to C++ memory space. u32 is just an offset into the C++ memory space. This is why there's lots of pointer casing to u32 and back.

grovesNL · 2019-03-12T18:49:03Z

I'm going to temporarily disable the Android build until Rust 2018 is supported by android-rs-glue (rust-mobile/android-rs-glue#202).

grovesNL · 2019-03-20T18:27:39Z

I'll follow-up with more tests after. At least the wasm build is working with gfx on wasm now so I'll continue testing there, and try to figure out the best way to share assets (the JS/wasm files here).

bors r=kvark

92: Add wasm32-unknown-unknown support r=kvark a=grovesNL This PR adds wasm32-unknown-unknown support for GLSL/ESSL compilation. Other compilers could easily be added to the Emscripten bundle, but it only includes the GLSL compiler for now. This works by manually writing function bindings to redirect native Rust -> C++ calls to become Rust/wasm32-unknown-unknown -> C++/Emscripten calls. Rust and C++ have separate wasm memory, so we have to do some copying between both of these in order to pass arguments back and forth. There is some more information in gfx-rs/gfx#2554 There are a few parts to this PR: - A crate to generate an Emscripten bundle from the C wrapper of SPIRV-Cross - The generated Emscripten bundle which contains everything needed for compiling SPIR-V to GLSL/ESSL - This bundle could easily include MSL and HLSL compilation too, but at the moment it's not clear how best to support all combinations (other than providing one large bundle), and we ideally don't want to require Emscripten binaries to be installed as a dependency. So for now to keep it straightforward we'll just keep the bundle limited to GLSL/ESSL, unless somebody needs HLSL/MSL support in wasm. - Some Emscripten and pointer utilities to unify access to pointers (to make Emscripten heap offsets look like pointers) so we can avoid changing the API. I think these could still be improved a lot, especially because it becomes confusing when accessing the wrong heap (Rust vs. Emscripten) but this seems to work ok for now. There is a bit of copying that could probably be made more efficient. I've been testing this so far through an example so I can run native and wasm side-by-side to check the output of reflection/compilation: ![image](https://user-images.githubusercontent.com/2113872/53678099-aa73c780-3c77-11e9-984f-749dcb42d2ec.png) In the future I want to expand the native test cases to improve coverage (there could still be a few calls omitted from my example) and figure out the best approach to run these tests in wasm too. Co-authored-by: Joshua Groves <[email protected]>

bors · 2019-03-20T18:40:53Z

Build failed

continuous-integration/appveyor/branch

grovesNL · 2019-03-20T18:42:35Z

bors retry

92: Add wasm32-unknown-unknown support r=kvark a=grovesNL This PR adds wasm32-unknown-unknown support for GLSL/ESSL compilation. Other compilers could easily be added to the Emscripten bundle, but it only includes the GLSL compiler for now. This works by manually writing function bindings to redirect native Rust -> C++ calls to become Rust/wasm32-unknown-unknown -> C++/Emscripten calls. Rust and C++ have separate wasm memory, so we have to do some copying between both of these in order to pass arguments back and forth. There is some more information in gfx-rs/gfx#2554 There are a few parts to this PR: - A crate to generate an Emscripten bundle from the C wrapper of SPIRV-Cross - The generated Emscripten bundle which contains everything needed for compiling SPIR-V to GLSL/ESSL - This bundle could easily include MSL and HLSL compilation too, but at the moment it's not clear how best to support all combinations (other than providing one large bundle), and we ideally don't want to require Emscripten binaries to be installed as a dependency. So for now to keep it straightforward we'll just keep the bundle limited to GLSL/ESSL, unless somebody needs HLSL/MSL support in wasm. - Some Emscripten and pointer utilities to unify access to pointers (to make Emscripten heap offsets look like pointers) so we can avoid changing the API. I think these could still be improved a lot, especially because it becomes confusing when accessing the wrong heap (Rust vs. Emscripten) but this seems to work ok for now. There is a bit of copying that could probably be made more efficient. I've been testing this so far through an example so I can run native and wasm side-by-side to check the output of reflection/compilation: ![image](https://user-images.githubusercontent.com/2113872/53678099-aa73c780-3c77-11e9-984f-749dcb42d2ec.png) In the future I want to expand the native test cases to improve coverage (there could still be a few calls omitted from my example) and figure out the best approach to run these tests in wasm too. Co-authored-by: Joshua Groves <[email protected]>

bors · 2019-03-20T19:00:18Z

Build succeeded

2554: Add wasm32-unknown-unknown/WebGL support r=kvark a=grovesNL Start to make a more maintainable version of my old branch from #1900 (comment). Heavily WIP but maybe somebody wants to start giving some feedback as I work through the remainder of issues. This PR replaces gl_generator with glow (https://github.com/grovesNL/glow), a experimental crate meant to unify WebGL/OpenGL types and function signatures. Currently unsupported functions will just panic, but I think it makes sense to move some of the version logic and extension fallbacks in there too (removing them from the gl backend). The WebGL types are now just keys (and `Copy`) which fixes some of the issues I mentioned previously. This is for the wasm32-unknown-unknown target and uses web-sys. TODO: - [X] ~~(major) spirv_cross wrapper on wasm32-unknown-unknown and C++ library through emscripten. For now the quad shaders are hardcoded.~~ See grovesNL/spirv_cross#92. (I'll probably leave the PR open until we work out how the integration between wasm bundles should work) - [X] ~~Consolidate render loop somehow (see WIP in glow example https://github.com/grovesNL/glow/blob/master/examples/hello/src/main.rs#L118). I need some more ideas on how to work around the `'static` bounds for wasm32 here. The basic idea is that wasm32 has to use callbacks and we use spinloops on the native target, so we would ideally like to unify these (at least for use in examples but also simple applications). For wasm32 with the quad example I just render once and quit instead.~~ Currently winit is working through the implementation of "Event Loop 2.0" and there's been [some effort to write a stdweb backend for this](rust-windowing/winit#797), which we could later help port to wasm32-unknown-unknown. I think for now we can just keep the wasm render loop path separate in gfx examples, and things will work naturally once winit wasm32-unknown-unknown support is ready. - [X] ~~Remove all remaining `#[cfg(not(target_arch = "wasm32"))]` wherever possible to make this more maintainable.~~ There are a few places remaining that can be removed when we move some logic into glow, like version parsing and extension checking. Some other parts like the runloop can't be addressed without winit support for wasm32-unknown-unknown or other workarounds in quad. - [X] ~~Decide where to put things like `index.html` for examples (I've excluded it for now) and guide about how to create wasm bundle with wasm-bindgen.~~ - [X] ~~Consider how logging should work – I don't see how to redirect stdout to `console.log` so it's a bit manually at the moment. It would be really cool if things like `info!` just work automatically with wasm32-unknown-unknown.~~ We should be able to use https://github.com/iamcodemaker/console_log for the wasm backend without any major challenges. - [X] ~~Publish glow~~ Published 0.2.0 - [x] ~~Publish updated spirv_cross~~ Published 0.14.0 - [X] ~~Add wasm-bindgen to CI~~ This has been added  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/gfx-rs/gfx/2554)  Co-authored-by: Joshua Groves <[email protected]>

grovesNL added 9 commits January 24, 2019 22:04

Upgrade to edition 2018

16c272f

Fix clippy warnings

081d7ea

Add features for GLSL, HLSL, and MSL

5148804

Conditionally include GLSL/HLSL/MSL in build step

d66609d

Start to add wasm target

fe31931

Manually create initial wasm function bindings

e7e33df

Remove ptr_at_offset

a8c20a7

Extend lifetime of new name

5d05d82

Rename Emscripten bundle

4def387

grovesNL marked this pull request as ready for review March 2, 2019 06:19

grovesNL mentioned this pull request Mar 2, 2019

Add wasm32-unknown-unknown/WebGL support gfx-rs/gfx#2554

Merged

8 tasks

Extend lifetime of resources

647d0c8

kvark approved these changes Mar 12, 2019

View reviewed changes

Temporarily disable Android build

222b09c

grovesNL force-pushed the features branch from 2354c09 to 222b09c Compare March 12, 2019 18:47

grovesNL added 5 commits March 12, 2019 12:58

Remove default features

a91b617

Fix Rust compiler warnings

9ed0090

Move namespace to functions

04aea65

Do not attempt to free combined image samplers

1dab068

Add test to rename combined image samplers

a5a299a

bors bot merged commit a5a299a into master Mar 20, 2019

grovesNL deleted the features branch March 21, 2019 05:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add wasm32-unknown-unknown support #92

Add wasm32-unknown-unknown support #92

grovesNL commented Mar 1, 2019 •

edited

Loading

grovesNL commented Mar 2, 2019

codecov bot commented Mar 2, 2019 •

edited

Loading

kvark left a comment

kvark Mar 12, 2019

kvark Mar 12, 2019

grovesNL Mar 12, 2019

kvark Mar 12, 2019

grovesNL Mar 12, 2019

kvark Mar 12, 2019

grovesNL Mar 12, 2019

grovesNL Mar 15, 2019

kvark Mar 12, 2019

grovesNL Mar 12, 2019

grovesNL commented Mar 12, 2019 •

edited

Loading

grovesNL commented Mar 12, 2019

grovesNL commented Mar 20, 2019

bors bot commented Mar 20, 2019

grovesNL commented Mar 20, 2019

bors bot commented Mar 20, 2019

Add wasm32-unknown-unknown support #92

Add wasm32-unknown-unknown support #92

Conversation

grovesNL commented Mar 1, 2019 • edited Loading

grovesNL commented Mar 2, 2019

codecov bot commented Mar 2, 2019 • edited Loading

Codecov Report

kvark 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

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

Choose a reason for hiding this comment

grovesNL commented Mar 12, 2019 • edited Loading

grovesNL commented Mar 12, 2019

grovesNL commented Mar 20, 2019

bors bot commented Mar 20, 2019

Build failed

grovesNL commented Mar 20, 2019

bors bot commented Mar 20, 2019

Build succeeded

grovesNL commented Mar 1, 2019 •

edited

Loading

codecov bot commented Mar 2, 2019 •

edited

Loading

grovesNL commented Mar 12, 2019 •

edited

Loading