Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upPreliminary WebXR support #22528
Preliminary WebXR support #22528
Conversation
highfive
commented
Dec 22, 2018
|
Heads up! This PR modifies the following files:
|
highfive
commented
Dec 22, 2018
| [NoInterfaceObject] | ||
| interface VR { | ||
| [Pref="dom.webvr.enabled"] | ||
| Promise<sequence<VRDisplay>> getDisplays(); |
This comment has been minimized.
This comment has been minimized.
jdm
Dec 22, 2018
Member
It is not clear to me that eliminating WebVR from our implementation is the right choice right now. It's useful to test against the existing WebVR content.
This comment has been minimized.
This comment has been minimized.
Manishearth
Dec 22, 2018
Author
Member
I'm not! navigator.vr is old WebVR (it's not in the spec anymore). I still support navigator.getVRDisplays() which is the new thing.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Manishearth
Dec 22, 2018
Author
Member
I just found it convenient to repurpose the VR object as XR since it has the same purpose and I didn't want to duplicate the messaging/syncing logic
|
Oh, also, one departure from the spec: we don't support I might try to make it use |
|
Great work! Code looks good on first pass. |
|
|
||
| // https://immersive-web.github.io/webxr/#xrframe-interface | ||
|
|
||
| [SecureContext, Exposed=Window] interface XRFrame { |
This comment has been minimized.
This comment has been minimized.
MortimerGoro
Dec 22, 2018
Contributor
The new XR interfaces should be behind a pref [Pref="dom.webxr.enabled"]
This comment has been minimized.
This comment has been minimized.
|
|
||
| callback XRFrameRequestCallback = void (DOMHighResTimeStamp time, XRFrame frame); | ||
|
|
||
| [SecureContext, Exposed=Window] interface XRSession : EventTarget { |
This comment has been minimized.
This comment has been minimized.
| @@ -266,6 +268,7 @@ impl WebGLRenderingContext { | |||
| pub fn recreate(&self, size: Size2D<u32>) { | |||
| let (sender, receiver) = webgl_channel().unwrap(); | |||
| self.webgl_sender.send_resize(size, sender).unwrap(); | |||
| self.size.set(size); | |||
This comment has been minimized.
This comment has been minimized.
MortimerGoro
Dec 22, 2018
Contributor
Can we guarantee that rust-offscreen-rendering-context is going to respect this size? There may be some corner cases such as JS requesting a larger size than the maximum allowed by the GPU. We can check with the existing webgl tests.
If we can guarantee this size you can also optimize DrawingBufferWidth() and DrawingBufferHeigh()
This comment has been minimized.
This comment has been minimized.
Manishearth
Dec 23, 2018
•
Author
Member
I believe when that happens it just doesn't work (throwing a GL error). At least, when I was playing with this in Firefox with Pathfinder that was the case, there's a limit to the texture size.
This comment has been minimized.
This comment has been minimized.
jdm
Jan 3, 2019
Member
The webgl implementation is allowed to choose a smaller size than what is requested; Servo's implementation does not right now and panics, #21718 tracks implementing this correctly.
This comment has been minimized.
This comment has been minimized.
Manishearth
Jan 3, 2019
Author
Member
Is it enough to just drop a comment linking to that since that's not implemented yet?
This comment has been minimized.
This comment has been minimized.
3a6a1aa
to
f7eab6e
|
|
|
@bors-servo try- r=jdm,MortimerGoro |
|
|
Preliminary WebXR support This implements just enough WebXR to display to 3DOF devices in immersive mode only. Couple missing things: - [ ] Handling reference spaces (even if just supporting eye-level spaces) - [x] Spec links - [ ] We enter immersive mode when baseLayer is set, but it seems like we're supposed to do this when requestSession is called (immersive-web/webxr#453) - [ ] VR/XR should block less (#22505) - [x] More pref-gating - [x] `views` is a method instead of an attribute because we don't support FrozenArray <s>Once I add spec links and pref gating</s> this can be landed as-is for further experimentation. r? @jdm @MortimerGoro <!-- 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/22528) <!-- Reviewable:end -->
|
|
|
|
@bors-servo r=jdm,MortimerGoro |
|
|
Preliminary WebXR support This implements just enough WebXR to display to 3DOF devices in immersive mode only. Couple missing things: - [ ] Handling reference spaces (even if just supporting eye-level spaces) - [x] Spec links - [ ] We enter immersive mode when baseLayer is set, but it seems like we're supposed to do this when requestSession is called (immersive-web/webxr#453) - [ ] VR/XR should block less (#22505) - [x] More pref-gating - [x] `views` is a method instead of an attribute because we don't support FrozenArray <s>Once I add spec links and pref gating</s> this can be landed as-is for further experimentation. r? @jdm @MortimerGoro <!-- 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/22528) <!-- Reviewable:end -->
|
|
Manishearth commentedDec 22, 2018
•
edited
This implements just enough WebXR to display to 3DOF devices in
immersive mode only.
Couple missing things:
we're supposed to do this when requestSession is called (immersive-web/webxr#453)
viewsis a method instead of an attribute because we don't support FrozenArrayOnce I add spec links and pref gatingthis can be landed as-is for further experimentation.r? @jdm @MortimerGoro
This change is