Add event support, use for disconnection and inputs #20

Manishearth · 2019-07-19T21:34:42Z

Allow the device to send events back to script
r? @asajeffrey

Manishearth · 2019-07-20T04:19:07Z

Okay, session ending is a total mess. We need:

The ability for JS to request the session to end, and get an event when the session is fully shut down
The ability for the device to decide it needs to end, and send the same event.
The ability for mock APIs to ask the device to shut down and notice when it's done

Currently, there are a bunch of things preventing this from happening:

Mock events are only processed if there is an rAF. There usually isn't in the tests
The device has no way to end the loop since it can't access it. Simply signalling disconnectedness as done here doesn't work since something needs to wake up the loop
There's no way for the headless device to "return" its receiver to the headless device

I think this will need a bunch of changes to work right.

I think we should just land this PR and servo/servo#23814 first as it improves some of the tests and makes them less flaky, then overhaul how we do mocking. The changes we need:

Don't have multiple mock discovery objects, just have a single one
Provide session objects with "quitter"s that loop back to their own event loop
route mock events through the main device event loop, perhaps as a core part of the API (Session.send_mock())

This will change a lot of things, thus the preference for landing the progress we have so far as-is to deal with intermittents.

asajeffrey

OK, I agree that this isn't perfect, but can land. I had a bunch of niggles but none of them are show-stoppers.

asajeffrey · 2019-07-22T14:29:10Z

webxr-api/device.rs

+    fn connected(&mut self) -> bool;
+
+    /// Quit the session
+    fn quit(&mut self);


Can we just make this the Drop code for the device?

Then XRSession would have to store an Option<Session> for no good reason.

asajeffrey · 2019-07-22T14:29:10Z

webxr-api/device.rs

+    fn set_event_callback(&mut self, callback: Box<dyn EventCallback>);
+
+    /// Whether the device is still connected
+    fn connected(&mut self) -> bool;


Should this be an event rather than a flag that is polled?

This is the communication issue I need to fix

asajeffrey · 2019-07-22T14:29:10Z

webxr-api/events.rs

+    /// Input source disconnected
+    RemoveInput(InputId),
+    /// Session ended by device
+    SessionEnd,


Should there be an event for the device being disconnected too? Blur / focus events?

Same thing as SessionEnd from the POV of the device IMO.

Blur/Focus yes, I just haven't gotten to it yet.

We should probably have VisibilityStateChange instead

asajeffrey · 2019-07-22T14:29:10Z

webxr/glwindow/mod.rs

+
+    fn quit(&mut self) {
+        self.connected = false;
+        self.events.callback(Event::SessionEnd);


Only send this event if the device is currently connected?

asajeffrey · 2019-07-22T14:29:10Z

webxr/headless/mod.rs

 struct HeadlessDevice {
    gl: Rc<dyn Gl>,
    floor_transform: TypedRigidTransform3D<f32, Native, Floor>,
    viewer_origin: TypedRigidTransform3D<f32, Viewer, Native>,
    views: Views,
    receiver: Receiver<MockDeviceMsg>,
+    events: EventBuffer,
+    inputs: Vec<InputInfo>,
+    disconnect_callbacks: Vec<Sender<()>>,


Why is disconnect treated differently from the other events?

The mock API requires we signal disconnection to have succeeded.

asajeffrey · 2019-07-22T14:29:10Z

webxr/headless/mod.rs

+        } else {
+            for callback in self.disconnect_callbacks.drain(..) {
+                let _ = callback.send(());
+            }


Calling this method has a side-effect? I wasn't expecting that!

Yeah, this isn't ideal, I want to change this in a followup.

Manishearth · 2019-07-22T14:36:32Z

@bors-servo r=asajeffrey

bors-servo · 2019-07-22T14:36:33Z

📌 Commit 4acb8fb has been approved by asajeffrey

bors-servo · 2019-07-22T14:36:36Z

⌛ Testing commit 4acb8fb with merge 944bc16...


        Auto merge of #20 - servo:events, r=asajeffrey

@asajeffrey

Add event support, use for disconnection and inputs Allow the device to send events back to script r? @asajeffrey

Manishearth · 2019-07-22T14:43:27Z

@bors-servo r=asajeffrey

bors-servo · 2019-07-22T14:43:28Z

📌 Commit c3f0aa2 has been approved by asajeffrey

bors-servo · 2019-07-22T14:43:32Z

⌛ Testing commit c3f0aa2 with merge 57f8ab8...


        Auto merge of #20 - servo:events, r=asajeffrey

@asajeffrey

Add event support, use for disconnection and inputs Allow the device to send events back to script r? @asajeffrey

bors-servo · 2019-07-22T14:48:17Z

💔 Test failed - checks-travis


        Add support for events


        Allow disconnecting the headless device


        Add support for mocking inputs


        Address comments

Manishearth · 2019-07-22T14:52:06Z

@bors-servo r=asajeffrey

bors-servo · 2019-07-22T14:52:06Z

📌 Commit bd38449 has been approved by asajeffrey

bors-servo · 2019-07-22T14:52:10Z

⌛ Testing commit 0b1ec62 with merge 7c06970...


        Auto merge of #20 - servo:events, r=

@asajeffrey

Add event support, use for disconnection and inputs Allow the device to send events back to script r? @asajeffrey

bors-servo · 2019-07-22T14:57:20Z

☀️ Test successful - checks-travis
State: approved= try=False

Manishearth · 2019-07-22T15:04:36Z

@bors-servo r=asajeffrey

bors-servo · 2019-07-22T15:04:37Z

📌 Commit 0b1ec62 has been approved by asajeffrey

bors-servo · 2019-07-22T15:04:40Z

⌛ Testing commit 0b1ec62 with merge 6603553...


        Auto merge of #20 - servo:events, r=asajeffrey

@asajeffrey

Add event support, use for disconnection and inputs Allow the device to send events back to script r? @asajeffrey

bors-servo · 2019-07-22T15:09:46Z

☀️ Test successful - checks-travis
Approved by: asajeffrey
Pushing 6603553 to master...


        Auto merge of #23814 - Manishearth:events, r=asajeffrey

@asajeffrey

Partial support for events in WebXR Needs servo/webxr#20 r? @asajeffrey  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/23814)


        Auto merge of #23814 - Manishearth:events, r=asajeffrey

@asajeffrey

Partial support for events in WebXR Needs servo/webxr#20 r? @asajeffrey  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/23814)

Manishearth force-pushed the events branch 5 times, most recently from 834d906 to b7d5165 Jul 19, 2019

Manishearth changed the title ~~Add event support, use for disconnection~~ Add event support, use for disconnection and inputs Jul 19, 2019

Manishearth mentioned this pull request Jul 20, 2019

Partial support for events in WebXR servo/servo#23814

Merged

asajeffrey approved these changes Jul 22, 2019

View changes

Manishearth force-pushed the events branch from 4acb8fb to c3f0aa2 Jul 22, 2019

Manishearth added 2 commits Jul 19, 2019

Add support for events

88bf546

Allow disconnecting the headless device

736557f

Manishearth force-pushed the events branch from c3f0aa2 to bd38449 Jul 22, 2019

Manishearth added 2 commits Jul 19, 2019

Add support for mocking inputs

35e7dc7

Address comments

Loading status checks…

0b1ec62

Manishearth force-pushed the events branch from bd38449 to 0b1ec62 Jul 22, 2019

Manishearth deleted the events branch Jul 24, 2019

servo / webxr

Add event support, use for disconnection and inputs #20

Add event support, use for disconnection and inputs #20

Manishearth commented Jul 19, 2019

Manishearth commented Jul 20, 2019 •

edited

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.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Manishearth commented Jul 22, 2019

bors-servo commented Jul 22, 2019

bors-servo commented Jul 22, 2019

Manishearth commented Jul 22, 2019

bors-servo commented Jul 22, 2019

bors-servo commented Jul 22, 2019

bors-servo commented Jul 22, 2019

Manishearth commented Jul 22, 2019

bors-servo commented Jul 22, 2019

bors-servo commented Jul 22, 2019

bors-servo commented Jul 22, 2019

Manishearth commented Jul 22, 2019

bors-servo commented Jul 22, 2019

bors-servo commented Jul 22, 2019

bors-servo commented Jul 22, 2019

servo / webxr

Join GitHub today

Add event support, use for disconnection and inputs #20

Add event support, use for disconnection and inputs #20

Conversation

Manishearth commented Jul 19, 2019

Manishearth commented Jul 20, 2019 • edited

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.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Manishearth commented Jul 22, 2019

bors-servo commented Jul 22, 2019

bors-servo commented Jul 22, 2019

Manishearth commented Jul 22, 2019

bors-servo commented Jul 22, 2019

bors-servo commented Jul 22, 2019

bors-servo commented Jul 22, 2019

Manishearth commented Jul 22, 2019

bors-servo commented Jul 22, 2019

bors-servo commented Jul 22, 2019

bors-servo commented Jul 22, 2019

Manishearth commented Jul 22, 2019

bors-servo commented Jul 22, 2019

bors-servo commented Jul 22, 2019

bors-servo commented Jul 22, 2019

Manishearth commented Jul 20, 2019 •

edited