Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement inline conversion to/from JS values #918

Closed
wants to merge 2 commits into from

Conversation

alexcrichton
Copy link
Contributor

These commits were originally spawned from #811. When working with web-sys not all values are well typed, but rather some values are simply JsValue and they're worked with elsewhere. Those values, however, sometimes need to be manufactured or interpreted as specific Rust types without handy conversions available.

This commit adds two new methods, JsValue::{from_wasm_abi,into_wasm_abi}, which can convert to/from a JsValue using the {From,Into}WasmAbi traits.

The implementation here is a bit heinous as it relies on the same closure rewriting logic we already have. I'm also starting to get a little worried abou the large slew of conversions we're accruing without necessarily a lot of direction about which conversion should be used when. Some thoughts on this would definitely be appreciated!

Closes #811

This commit implements a generic function on `JsValue` which can convert
*any* Rust value that implements `IntoWasmAbi` into the corresponding
`JsValue`. This can be handy when binding APIs and the intermediate JS
value is needed is needed over a few function calls or is otherwise
needed when calling APIs that take `JsValue` but otherwise work with
Rust primitives.

One of the primary intended use cases for this will be interoperation
with the various `JsValue`-taking functions (or `Object`) in `web-sys`.

cc rustwasm#811
This commit implements the inverse of the previous commit, converting
any arbitrary `JsValue` into the Rust-typed equivalent so long as it
implements `IntoWasmAbi`.
Copy link
Member

@fitzgen fitzgen left a comment

Choose a reason for hiding this comment

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

I'm a bit confused and having a hard time seeing how these new conversions fit in with our existing conversion story.

It seems there are certain conversions where intrinsic rewriting is the only way to do the conversion:

  • Closure<F> to JsValue
  • &[u8] to JsValue (although this is also possible via &[u8] -> Uint8Array -> JsValue I think?)

but then these intrinsic rewrite conversions are also implemented for every WasmAbi type. When would I (the user) use which conversion?

};

// Actual interpretation loop! We keep track of our stack's length to
// recover it as part of the `Return` instruction, and otherwise this is
// a pretty straightforward interpretation loop.
for instr in body.code().elements() {
match instr {
// Ignore loads/stores of locals which are f32/f64, as we're
// ignoring all of these values.
SetLocal(i) if local_f32.contains(i) => {}
Copy link
Member

Choose a reason for hiding this comment

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

:-|

@alexcrichton
Copy link
Contributor Author

Ah sorry I've let this languish a bit! I agree that this is probably adding too much surface area for conversions to be worth it, and it makes it a bit confusing as to what's being converted where and when. I haven't had a chance to sit down and try to think about the fundamental value this is adding (if there's anything other than the convenient slice <-> jsvalue conversions), but in the meantime I think it's best that we defer this to a later date

@dakom

This comment was marked as abuse.

@dakom

This comment was marked as abuse.

@alexcrichton
Copy link
Contributor Author

Good questions! For now the easiest way is to use wasm_bindgen::memory and create an offset directly into wasm's own memory and then hand that off, that way there's no copying/allocations!

@dakom

This comment was marked as abuse.

@chinedufn
Copy link
Contributor

chinedufn commented Dec 8, 2018

@dakom here's a quick example of creating Float32Array and Uint16Array in case it helps!

let memory_buffer = wasm_bindgen::memory()
  .dyn_into::<WebAssembly::Memory>()
  .unwrap()
  .buffer();

let vertices: [f32; 12] = [
   0., 0., 0., // Bottom Left
    1., 0., 0., // Bottom Right
    1., 0., -1., // Top Right
    0., 0., -1., // Top Left
];

let vertices_location = vertices.as_ptr() as u32 / 4;

let vert_array = js_sys::Float32Array::new(&memory_buffer)
  .subarray(vertices_location, vertices_location + vertices.len() as u32);

let indices: [u16; 6] = [0, 1, 2, 0, 2, 3];

let indices_location = indices.as_ptr() as u32 / 2;

let indices_array = js_sys::Uint16Array::new(&memory_buffer)
  .subarray(indices_location, indices_location + indices.len() as u32);

For f64 you'd just need to divide by 8 (I haven't used f64 myself but I'd imagine this would work fine)

@dakom

This comment was marked as abuse.

@chinedufn
Copy link
Contributor

Sorry just saw this! - not entirely sure what you're asking

@dakom

This comment was marked as abuse.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support converting js_sys typed arrays to Vec
4 participants