Use new WebXR API in script #23731

Manishearth · 2019-07-08T23:24:49Z

Todo:

Hook new Frame information into XRFrame
Make spaces use new transform info
Hook up session view metadata correctly
Get mocking working again
Get inputs working

Optional todos:

Add support for active and animationFrame bool on XRFrame
Correctly handle viewer and offset spaces instead of casting
Error on zero-length quaternions

Not really ready for review yet, but you can go ahead and review what I have so far. It doesn't do anything yet, aside from crash horribly. I'm opening this PR early so i have a place to track progress.

This change is

highfive · 2019-07-08T23:24:54Z

Heads up! This PR modifies the following files:

@asajeffrey: components/script/dom/xrrenderstate.rs, components/script/dom/webidls/XRSession.webidl, components/script/Cargo.toml, components/script/dom/xrview.rs, components/script/dom/bindings/trace.rs and 5 more
@KiChjang: components/script/dom/xrrenderstate.rs, components/script/dom/webidls/XRSession.webidl, components/script/Cargo.toml, components/script/dom/xrview.rs, components/script/dom/bindings/trace.rs and 5 more

highfive · 2019-07-08T23:24:55Z

Warning

These commits modify script code, but no tests are modified. Please consider adding a test!

Manishearth · 2019-07-09T01:46:29Z

Can start reviewing now

asajeffrey

OK, this looks good so far. There are a few sources of panic that we should probably get rid of before merging, and there's an annoying extra thread kept alive because script doesn't support serializable tasks. I see what you mean about the typed transforms, the types are just getting in the way because by the time the transforms get to JS they're untyped.

components/script/dom/bindings/trace.rs

components/script/dom/xr.rs

asajeffrey · 2019-07-09T15:05:27Z

components/script/dom/xrsession.rs

+
+#[allow(unsafe_code)]
+pub fn cast_transform<T, U, V, W>(transform: TypedRigidTransform3D<f32, T, U>) -> TypedRigidTransform3D<f32, V, W> {
+    unsafe { mem::transmute(transform) }


We can make this safe by casting to and from an untyped transform?

There's no cast function.

Er, to_untyped() and from_untyped()?

Ah, those are only on TypedTransform not TypedRigidTransform?

asajeffrey · 2019-07-09T15:05:27Z

components/script/dom/xrsession.rs

+            // XXXManishearth handle inline sessions and composition disabled flag
+            let layer = pending.GetBaseLayer();
+            if let Some(layer) = layer {
+                let layer = layer.downcast::<XRWebGLLayer>().unwrap();


Handle the case of a non-XRWebGLLayer without panicking?

asajeffrey · 2019-07-09T15:05:27Z

components/script/dom/xrsession.rs

+                .dom_manipulation_task_source_with_canceller();
+            let (sender, receiver) = ipc::channel(global.time_profiler_chan().clone()).unwrap();
+            *self.raf_sender.borrow_mut() = Some(sender);
+            ROUTER.add_route(


Ouch, we're using a ROUTER, which means a context switch every frame.

We can discuss this in #23733.

Ok so, if the goal is really removing the router from the equation for this specific workflow, here's an idea for your consideration:

At this very line, instead of using the ROUTER, queue a task that does the below:

let frame = raf_receiver.recv(); (or return if you're inactive)

self.raf_callback(frame);

If !self.raf_callback_list.is_empty()

request_animation_frame

queue a task to start at 1

Or, if there are no callbacks or the session has ended, skip 4 and 5

If you don't need to actually run tasks, or handle messages from the constellation, you could try an even more drastic approach where instead of running one callback in one task, you'd keep requesting frames and running callbacks for as long as new callbacks are added inside a callback, and for as long as the session remains active(and the pipeline isn't closed?). And that from within the same task, effectively blocking the script-thread in the meantime(but running the callbacks as "fast" as possible, I guess).

So you could just queue a task in the first requestAnimationFrame call, and after that just loop inside the same task, running a mini-event-loop inside the otherwise blocked window event-loop...

We could do something like that, but this would mean blocking the script thread while waiting for the raf_receiver.recv(), and IPC channels don't support recv_timeout(). Sigh. I think this would need quite a drastic re-think of how Servo approaches IPC.

Manishearth · 2019-07-09T16:55:14Z

Addressed.

Manishearth · 2019-07-09T17:00:58Z

There are no untyped functions on half the euclid types

https://docs.rs/euclid/0.19.7/euclid/struct.TypedRigidTransform3D.html

Manishearth · 2019-07-10T19:24:50Z

Mocking works! I'll still need to fix the test again

Manishearth · 2019-07-10T20:58:25Z

input's done, as well. ready for review.

I haven't yet tested locally (can't test on my laptop), will do after this meeting

asajeffrey · 2019-07-10T21:40:15Z

Argh, did you know github gets very confused if you force-push while there's a review being written?

asajeffrey

Yay, looks pretty good. Usual collection of nip-picks and bikesheds.

components/script/dom/fakexrdevice.rs

asajeffrey · 2019-07-10T21:55:37Z

components/script/dom/fakexrdevice.rs

 }

-pub fn get_views(views: &[FakeXRViewInit]) -> Fallible<(MockVRView, MockVRView)> {
+pub fn get_views(views: &[FakeXRViewInit]) -> Fallible<Views> {
    if views.len() != 2 {
        return Err(Error::NotSupported);


Might want to support mono at some point.

Yeah, inline support needs a bit more work

components/script/dom/webidls/XRTest.webidl

Cargo.toml

components/script/dom/fakexrdevice.rs

components/script/dom/xrrenderstate.rs


        Fill in updateRenderState() stubs


        Add basic rAF and cAF functions that do nothing with the callbacks


        Add actual frame request in rAF


        Apply pending render states in RAF


        Trigger callbacks in RAF


        Remove old RAF code


        Hook webxr data into XRFrame/XRView/XRSpace


        Clean up vrdisplay, set active immersive session correctly


        Add a viewer typed reference space


        Update test API, hook it up to webxr mocking


        Hook input code into new webxr crate


        Remove session deactivation


        Add active and animationFrame flags to XRFrame


        Error on invalid rotation values in XRRigidTransform constructor


        Use separate IPC-only locking mechanism when locking from webxr

asajeffrey · 2019-07-11T19:34:41Z

The failure with glwindow is probably OK, but we should fix the mock API. In particular, mirroring isn't working, possibly because we're not marking the canvas as dirty after an XR rAF?

Manishearth · 2019-07-11T20:05:07Z

The tests still work, which makes me kinda okay with the mock API being broken.

asajeffrey · 2019-07-11T20:59:17Z

Yes, I think it's display mirroring that's broken. I think it's OK to merge with that. You can r=me.

Can we enable some webxr tests now?

Manishearth · 2019-07-11T21:00:44Z

@bors-servo r=asajeffrey

not yet, Chrome is still fixing them

bors-servo · 2019-07-11T21:00:45Z

📌 Commit c5a0fc5 has been approved by asajeffrey

bors-servo · 2019-07-11T22:31:23Z

⌛ Testing commit c5a0fc5 with merge 5fdc7c0...


        Auto merge of #23731 - Manishearth:webxr-script, r=asajeffrey

Use new WebXR API in script Todo: - [x] Hook new `Frame` information into `XRFrame` - [x] Make spaces use new transform info - [x] Hook up session view metadata correctly - [x] Get mocking working again - [x] Get inputs working Optional todos: - [x] Add support for active and animationFrame bool on XRFrame - [x] Correctly handle viewer and offset spaces instead of casting - [x] Error on zero-length quaternions <s>Not really ready for review yet, but you can go ahead and review what I have so far. It doesn't do anything yet, aside from crash horribly. I'm opening this PR early so i have a place to track progress.</s>  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/23731)

bors-servo · 2019-07-12T00:28:20Z

☀️ Test successful - linux-rel-css, linux-rel-wpt, status-taskcluster
Approved by: asajeffrey
Pushing 5fdc7c0 to master...

Manishearth requested a review from asajeffrey Jul 8, 2019

highfive assigned jdm Jul 8, 2019

highfive added the S-awaiting-review label Jul 8, 2019

Manishearth force-pushed the Manishearth:webxr-script branch 9 times, most recently from 2dd8c3e to 59c8d85 Jul 9, 2019

Manishearth force-pushed the Manishearth:webxr-script branch from 59c8d85 to 65180a5 Jul 9, 2019

asajeffrey reviewed Jul 9, 2019

View changes

jdm assigned asajeffrey and unassigned jdm Jul 9, 2019

asajeffrey mentioned this pull request Jul 9, 2019

Support serializable tasks in script? #23733

Open

Manishearth force-pushed the Manishearth:webxr-script branch from d3a98f7 to 390a482 Jul 9, 2019

Manishearth force-pushed the Manishearth:webxr-script branch from 7353275 to afafc17 Jul 10, 2019

Manishearth force-pushed the Manishearth:webxr-script branch from 74e3d9e to 9d5252b Jul 10, 2019

asajeffrey reviewed Jul 10, 2019

View changes

Manishearth force-pushed the Manishearth:webxr-script branch from 9d5252b to 70688bc Jul 10, 2019

Manishearth added 15 commits Jul 8, 2019

Fill in updateRenderState() stubs

c7ed437

Add basic rAF and cAF functions that do nothing with the callbacks

84014ff

Add actual frame request in rAF

cf53cf6

Apply pending render states in RAF

33421e8

Trigger callbacks in RAF

877b6c6

Remove old RAF code

104a712

Hook webxr data into XRFrame/XRView/XRSpace

8780edb

Clean up vrdisplay, set active immersive session correctly

57c8536

Add a viewer typed reference space

b818af7

Update test API, hook it up to webxr mocking

0d5d1a3

Hook input code into new webxr crate

b654b60

Remove session deactivation

7a8640e

Add active and animationFrame flags to XRFrame

0b88c56

Error on invalid rotation values in XRRigidTransform constructor

036b495

Use separate IPC-only locking mechanism when locking from webxr

Loading status checks…

c5a0fc5

Manishearth force-pushed the Manishearth:webxr-script branch from b897b30 to c5a0fc5 Jul 11, 2019

highfive added S-awaiting-merge and removed S-awaiting-review labels Jul 11, 2019

bors-servo merged commit c5a0fc5 into servo:master Jul 12, 2019
2 of 3 checks passed

2 of 3 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details

Taskcluster (pull_request) TaskGroup: success
Details

homu Test successful
Details

highfive removed the S-awaiting-merge label Jul 12, 2019

This was referenced Jul 12, 2019

Move tinyfiledialog call from script to embedder #23651

Closed

Sync WebIDL.py with gecko #23748

Merged

servo / servo

Use new WebXR API in script #23731

Use new WebXR API in script #23731

Manishearth commented Jul 8, 2019 •

edited

highfive commented Jul 8, 2019

highfive commented Jul 8, 2019

Manishearth commented Jul 9, 2019

asajeffrey left a comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Manishearth commented Jul 9, 2019

Manishearth commented Jul 9, 2019

Manishearth commented Jul 10, 2019

Manishearth commented Jul 10, 2019

asajeffrey commented Jul 10, 2019

asajeffrey left a comment

This comment has been minimized.

This comment has been minimized.

asajeffrey commented Jul 11, 2019

Manishearth commented Jul 11, 2019

asajeffrey commented Jul 11, 2019

Manishearth commented Jul 11, 2019

bors-servo commented Jul 11, 2019

bors-servo commented Jul 11, 2019

bors-servo commented Jul 12, 2019

servo / servo

Join GitHub today

Use new WebXR API in script #23731

Use new WebXR API in script #23731

Conversation

Manishearth commented Jul 8, 2019 • edited

highfive commented Jul 8, 2019

highfive commented Jul 8, 2019

Manishearth commented Jul 9, 2019

asajeffrey left a comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

gterzian Jul 10, 2019 • edited

This comment has been minimized.

Manishearth commented Jul 9, 2019

Manishearth commented Jul 9, 2019

Manishearth commented Jul 10, 2019

Manishearth commented Jul 10, 2019

asajeffrey commented Jul 10, 2019

asajeffrey left a comment

This comment has been minimized.

This comment has been minimized.

asajeffrey commented Jul 11, 2019

Manishearth commented Jul 11, 2019

asajeffrey commented Jul 11, 2019

Manishearth commented Jul 11, 2019

bors-servo commented Jul 11, 2019

bors-servo commented Jul 11, 2019

bors-servo commented Jul 12, 2019

Manishearth commented Jul 8, 2019 •

edited

gterzian Jul 10, 2019 •

edited