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

Support input tracking loss and selection events in XR #24362

Merged
merged 6 commits into from Oct 4, 2019

Conversation

@Manishearth
Copy link
Member

Manishearth commented Oct 3, 2019

Fixes #24192

Requires servo/webxr#65

r? @jdm @asajeffrey


This change is Reviewable

@highfive
Copy link

highfive commented Oct 3, 2019

Heads up! This PR modifies the following files:

  • @asajeffrey: components/script/dom/webidls/XRSession.webidl, components/script/dom/xrinputsourceevent.rs, components/script/dom/webidls/XRInputSourceEvent.webidl, components/script/dom/mod.rs, components/script/dom/xrspace.rs and 2 more
  • @KiChjang: components/script/dom/webidls/XRSession.webidl, components/script/dom/xrinputsourceevent.rs, components/script/dom/webidls/XRInputSourceEvent.webidl, components/script/dom/mod.rs, components/script/dom/xrspace.rs and 2 more
@highfive
Copy link

highfive commented Oct 3, 2019

warning Warning warning

  • These commits modify script code, but no tests are modified. Please consider adding a test!
@Manishearth Manishearth force-pushed the Manishearth:input-selections branch from 9a8992b to 09d978d Oct 3, 2019
@Manishearth Manishearth force-pushed the Manishearth:input-selections branch from 09d978d to d6470c4 Oct 3, 2019
@Manishearth Manishearth changed the title WIP support input tracking loss and selection events in XR Support input tracking loss and selection events in XR Oct 3, 2019
Copy link
Member

asajeffrey left a comment

Just a minor question. otherwise LGTM. You can r=me. I'm a bit surprised there's no tests that now pass.

.find(|s| s.id() == input)
.map(|x| DomRoot::from_ref(&**x));
if let Some(source) = source {
let frame = XRFrame::new(&self.global(), self, frame);

This comment has been minimized.

@asajeffrey

asajeffrey Oct 3, 2019

Member

Do you have a spec link for this? Generating a new frame here seems odd, since we're not in an rAF.

This comment has been minimized.

@Manishearth

Manishearth Oct 3, 2019

Author Member

https://immersive-web.github.io/webxr/#primary-action , each event comes with a frame

There's a concept of a non-animation frame "frame", which is basically for updating input poses.

@Manishearth
Copy link
Member Author

Manishearth commented Oct 3, 2019

Tests won't pass since we've not added headless support for this yet.

@Manishearth Manishearth force-pushed the Manishearth:input-selections branch from d6470c4 to cf60776 Oct 3, 2019
@Manishearth Manishearth force-pushed the Manishearth:input-selections branch from cf60776 to 931f0b1 Oct 3, 2019
@Manishearth
Copy link
Member Author

Manishearth commented Oct 3, 2019

@bors-servo r=asajeffrey

@bors-servo
Copy link
Contributor

bors-servo commented Oct 3, 2019

📌 Commit 931f0b1 has been approved by asajeffrey

@highfive highfive assigned asajeffrey and unassigned jdm Oct 3, 2019
@bors-servo
Copy link
Contributor

bors-servo commented Oct 4, 2019

Testing commit 931f0b1 with merge ea4e3ae...

bors-servo added a commit that referenced this pull request Oct 4, 2019
Support input tracking loss and selection events in XR

Fixes #24192

Requires servo/webxr#65

r? @jdm @asajeffrey

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/24362)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Oct 4, 2019

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

@bors-servo bors-servo merged commit 931f0b1 into servo:master Oct 4, 2019
3 checks passed
3 checks passed
Taskcluster (pull_request) TaskGroup: success
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
homu Test successful
Details
bors-servo added a commit to servo/webxr that referenced this pull request Oct 4, 2019
Input support for OpenXR

Requires #65

This treats the right hand as an input source. While I've tested the input code, I haven't yet tested this hooked up to the Servo DOM (servo/servo#24362), so I'd like to hold off on merging this until I get a chance

Still, feel free to review the last few commits that have "openxr" in their bodies
bors-servo added a commit to servo/webxr that referenced this pull request Oct 4, 2019
Input support for OpenXR

Requires #65

This treats the right hand as an input source. While I've tested the input code, I haven't yet tested this hooked up to the Servo DOM (servo/servo#24362), so I'd like to hold off on merging this until I get a chance

Still, feel free to review the last few commits that have "openxr" in their bodies
@Manishearth Manishearth deleted the Manishearth:input-selections branch Oct 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

5 participants
You can’t perform that action at this time.