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

Webxrdevice #2000

Merged
merged 11 commits into from
Mar 17, 2020
Merged

Webxrdevice #2000

merged 11 commits into from
Mar 17, 2020

Conversation

kevthecoder
Copy link
Contributor

@kevthecoder kevthecoder commented Feb 13, 2020

Adds WebIDL for "Candidate Recommendation" version of WebXR (https://www.w3.org/TR/2019/WD-webxr-20191010/).

Supported without flags in Chrome 80, but not supported in Firefox (as of 13 Feb 2020).

To update WebIDL bindings:

cd crates/web-sys/
RUST_LOG=wasm_bindgen_webidl __WASM_BINDGEN_DUMP_FEATURES=webxr cargo build -vv
# (had to run the command twice for some reason)

Linked issues:
#1950
#1774
#1948

This was referenced Feb 13, 2020
@Pauan
Copy link
Contributor

Pauan commented Feb 13, 2020

@kevthecoder Thanks, though that isn't a Candidate Recommendation, it's a Draft, so it's not standardized yet.

So this will need to wait for #1997 to be implemented first.

@kevthecoder
Copy link
Contributor Author

Thanks @Pauan , sounds like a plan.

@kevthecoder
Copy link
Contributor Author

kevthecoder commented Feb 16, 2020

I've had to disable some properties in the WebIDL for XRWebGLLayer to get the bindings to work, otherwise the build command throws an error (with no further error message). The constructor for XRWebGLLayer needed to be moved to the top level to produce bindings for XRWebGLLayer::new().

I've also had to apply a workaround for attributes marked with FrozenArray, see https://bugzilla.mozilla.org/show_bug.cgi?id=1236777 for more details.

All alterations are commented with a TODO to keep track of them.

@Manishearth
Copy link

Note that it's a draft, but also it's been shipped in multiple browsers now.

@Pauan
Copy link
Contributor

Pauan commented Feb 21, 2020

@Manishearth Do you have any evidence for that? As far as I know, only one engine supports it, and even then it requires the user to enable an experimental flag, it is not enabled by default. I have verified this by testing in both Chrome and Firefox.

I'm very confused why everybody seems to think this is a widely implemented API.

@Manishearth
Copy link

@Pauan You don't need a flag to use it in Chrome (but it only works for some devices). It's shipped on Oculus Browser and Chrome, and Firefox Reality is shipping the same API very soon. Firefox planned to ship in December along with Chrome, but some issues cropped up on the Firefox side delaying it a bit.

Either way, the editors consider the API surface done (and we're gunning for CR soon, but the process takes time), and it would be extremely unlikely for any further changes to be breaking given that there is real world content using this API now. New APIs may of course be tacked on, but that's true of all web specs.

@Manishearth
Copy link

Basically: if the main concern is breaking changes, I wouldn't be too concerned. If the main concern is just not having anything that's not gone through the full w3c standards pipeline, that's understandable.

@Pauan
Copy link
Contributor

Pauan commented Feb 21, 2020

@Manishearth Ah, I see, that explains why it wasn't working for me. In that case we should consider stabilizing XR.

@alexcrichton
Copy link
Contributor

Want to rebase this on #1997 and we should be able to merge then?

@kevthecoder
Copy link
Contributor Author

Want to rebase this on #1997 and we should be able to merge then?

No problem, but might be a few days before I get the time. I've been trying to improve the example code too.

@Pauan
Copy link
Contributor

Pauan commented Feb 28, 2020

Just a note that the process of generating WebIDL will change after #2012 lands.

@Pauan
Copy link
Contributor

Pauan commented Mar 3, 2020

Since #2012 has landed now, the process for generating WebIDL is:

  1. Put any unstable WebIDL files into the crates/web-sys/webidls/unstable folder.

  2. Run this command:

    cargo run --release --package wasm-bindgen-webidl crates/web-sys/webidls crates/web-sys/src/features
    
  3. Copy the crates/web-sys/features file into the crates/web-sys/Cargo.toml file (right underneath [features]).

  4. Use git add crates/web-sys to add all the generated files into git.

@kevthecoder
Copy link
Contributor Author

kevthecoder commented Mar 14, 2020

@Pauan @alexcrichton the code is updated to compatible with #2012 now. I like the new system, found it much easier to debug a webidl issue I was having with XrWebGlLayer.

I've tested the new bindings on a WebXr game engine project and they seem to work fine. I tried to make a simplified example application in examples/webxr, but there's a small issue I can't seem to fix at the moment (something to do with calling request_animation_frame). I'll try to take another look at it with a fresh pair of eyes, but at least the bindings seem to be working ok.

For testing I've been using Chrome with the Mozilla WebXr emulator plugin - https://blog.mozvr.com/webxr-emulator-extension.

So in summary, the bindings work and the example 90% works.

@alexcrichton
Copy link
Contributor

Nice, thanks for this! This all looks great to me, so any further iteration can happen in-repo!

@alexcrichton alexcrichton merged commit 2b29650 into rustwasm:master Mar 17, 2020
@kevthecoder
Copy link
Contributor Author

Fantastic, thanks @alexcrichton.

@kevthecoder kevthecoder deleted the webxrdevice branch March 17, 2020 14:46
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.

None yet

4 participants