-
Notifications
You must be signed in to change notification settings - Fork 142
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
feat: Update to WebXR #285
Conversation
…w Device API is more likely the better thing to do though
WebVR to WebXR
[KJSL] Swap webvr ro webxr polyfill packages
[KJSL] Swap webvr ro webxr polyfill packages
[KJSL] Add WebXR immersive VR support
chore: Swap webvr ro webxr polyfill packages
…olyfill if webxr immersive is not available)
chore: Swap webvr ro webxr polyfill packages (continue to use webvr p…
… operation to play/pause and exit VR/AR session)
chore: Swap webvr for webxr polyfill packages (adding basic controller…
…k on entering 360)
chore: Swap webvr ro webxr polyfill packages
…k on entering 360)
chore: Swap webvr to webxr polyfill packages
Co-authored-by: Gary Katsevman <[email protected]>
Co-authored-by: Gary Katsevman <[email protected]>
d319e4d
to
4576704
Compare
Great stuff, will give this a proper go later but looks good already, I'll see if anyone wants to help with that too A bit of context and doco for anyone new https://docs.google.com/presentation/d/19jz0guudowBpcBlMg2INdD6PX5w9tdJfS54Q8Twvqkc/edit Credits: Don't forget to give StreamShark some major love next time seeing them around who sponsored this effort! Shout out to Nik Lever too for his Udemy course on WebXR which is particularly good - head here https://www.udemy.com/course/learn-webxr/?couponCode=DEC23_SALE |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for porting this over, @mister-ben!
I don't know this project well, so take my review with a grain of salt, but the changes look good to me.
src/plugin.js
Outdated
let displays = []; | ||
|
||
if (window.navigator.getVRDisplays) { | ||
this.log('is supported, getting vr displays'); | ||
window.navigator.getVRDisplays().then((displays) => { | ||
if (displays.length > 0) { | ||
this.log('Displays found', displays); | ||
this.vrDisplay = displays[0]; | ||
|
||
// Native WebVR Head Mounted Displays (HMDs) like the HTC Vive | ||
// also need the cardboard button to enter fully immersive mode | ||
// so, we want to add the button if we're not polyfilled. | ||
if (!this.vrDisplay.isPolyfilled) { | ||
this.log('Real HMD found using VRControls', this.vrDisplay); | ||
this.addCardboardButton_(); | ||
|
||
// We use VRControls here since we are working with an HMD | ||
// and we only want orientation controls. | ||
this.controls3d = new VRControls(this.camera); | ||
} | ||
} | ||
|
||
if (!this.controls3d) { | ||
this.log('no HMD found Using Orbit & Orientation Controls'); | ||
const options = { | ||
camera: this.camera, | ||
canvas: this.renderedCanvas, | ||
// check if its a half sphere view projection | ||
halfView: this.currentProjection_.indexOf('180') === 0, | ||
orientation: videojs.browser.IS_IOS || videojs.browser.IS_ANDROID || false | ||
}; | ||
|
||
if (this.options_.motionControls === false) { | ||
options.orientation = false; | ||
} | ||
window.navigator.getVRDisplays().then((displaysArray) => { | ||
displays = displaysArray; | ||
}); | ||
} | ||
|
||
this.controls3d = new OrbitOrientationContols(options); | ||
this.canvasPlayerControls = new CanvasPlayerControls(this.player_, this.renderedCanvas, this.options_); | ||
} | ||
// Detect WebXR is supported | ||
if (window.navigator.xr) { | ||
this.log('WebXR is supported'); | ||
window.navigator.xr.isSessionSupported('immersive-vr').then((supportsImmersiveVR) => { | ||
if (supportsImmersiveVR) { | ||
// We support WebXR show the enter VRButton | ||
this.vrButton = VRButton.createButton(this.renderer); | ||
document.body.appendChild(this.vrButton); | ||
this.initImmersiveVR(); | ||
this.initXRPolyfill(displays); | ||
|
||
// key or pointer click toogle playback | ||
document.body.addEventListener('keydown', () => { | ||
this.togglePlay_(); | ||
}); | ||
document.body.addEventListener('click', () => { | ||
this.togglePlay_(); | ||
}); | ||
|
||
this.animationFrameId_ = this.requestAnimationFrame(this.animate_); | ||
} else { | ||
// fallback to older WebVR if WebXR immersive session is not available | ||
this.initVRPolyfill(displays); | ||
} | ||
window.navigator.xr.setSession = (session) => { | ||
this.currentSession = session; | ||
this.renderer.xr.setSession(this.currentSession); | ||
}; | ||
}); | ||
} else { | ||
// fallback to older WebVR if WebXR Device API is not available | ||
this.initVRPolyfill(displays); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The variable displays
is set in a promise, I don't see how this could work but maybe I'm missing something.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to lead to an NPE on the property this.player_.vr().vrDisplay
in the following file: https://github.com/mister-ben/videojs-vr/blob/4576704f9c807b632a744607bfed8a084dedf509/src/cardboard-button.js#L42-L49
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm, and getVRDisplays
is deprecated and only implemented on android firefox
@mister-ben I have this behavior with all the examples on
|
I've removed the cardboard button in the control bar for now, and used three.js's VRButton as an overlay button. I don't love the UX, if we want the cardboard button we need to include what's going on inside the VRButton class, it's quite different from what worked before. This seems ok as an interim compromise. This works well on Android. iOS is a pain going in and out of immersive mode. It works sometimes, other times Safari doesn't understand the screen orientation and messes up the display. When exiting, the canvas is larger than the player. I don't know yet whether the issues are in three.js, the plugin, or Safari. Or a combination. |
@mister-ben on Android on Firefox, moving the phone to change orientation works well, but on Chrome it doesn't. Also, on VR mode, the back and settings buttons don't respond to touch, as well as when using a google cardboard the toggle play doesn't work. |
I see that the canvas is created here plugin.js#L783-L788, and then it's deleted somehow when entering immersive mode. When leaving immersive mode, the canvas is added to the player as the last child somehow, and this is the cause of #285 (comment). But I can't figure out how. |
This seems to break the EAC projections. Can anyone else confirm? |
(do we have some good examples, happy to dive in and check on the headsets I have)On 12 Feb 2024, at 5:42 am, Will Petersen ***@***.***> wrote:
This seems to break the EAC projections. Can anyone else confirm?
—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: ***@***.***>
|
Yes, there are examples for both EAC and EAC_LR in the examples/samples dirs. They should be listed when you run |
const obj = (index === 0) ? self.controllers[0] : self.controllers[1]; | ||
|
||
if (obj) { | ||
if (obj.controller) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
obj.controller
and obj.grip
are never set.
// Not using the cardboard button in favour of three.js's vrbutton. | ||
// if (!this.player_.controlBar.getChild('CardboardButton')) { | ||
// this.player_.controlBar.addChild('CardboardButton', {}); | ||
// } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the cardboard button is being removed in favor of the three.js VRButton, shouldn't all references to addCardboardButton_()
, the whole cardboard-button.js
file, and the forceCardboard
option be removed? Why go halfway?
return controllers; | ||
} | ||
|
||
createText(message, height) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does createText
ever get called?
Yeah controllers were working - I’ll try find some time to check if something changed On 20 Feb 2024, at 7:29 am, Will Petersen ***@***.***> wrote:
@WCPetersen commented on this pull request.
In src/plugin.js:
+ controller.userData.selectPressed = false;
+ controller.userData.index = i;
+ this.scene.add(controller);
+
+ controllers.push(controller);
+
+ const grip = this.renderer.xr.getControllerGrip(i);
+
+ grip.add(controllerModelFactory.createControllerModel(grip));
+ this.scene.add(grip);
+ }
+
+ return controllers;
+ }
+
+ createText(message, height) {
Does this ever get called?
—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: ***@***.***>
|
As an Apple Vision Pro user, I would love to see this implemented. Any chance it can worked on more soon? Thanks! |
We are actively working on it over here - once it stabilises we can have another go at merging upstream. Any feedback is welcome - right now we are focused on improving/making reliable the entering and exiting of immersive mode on HMDs, which is not working super reliably right now. |
Closing in favour of the other work |
Brings @kevleyski 's changes in #273 into main:
For devices that support WebXR, use that
Add iOS device orientation permission check on entering 360 (note ESLint workaround as this is iOS Safari specfic)
Fall back to original WebVR polyfill if navigator.xr not available after initialising WebXR polyfill
Includes fixes for projections other than equirectangular that needed updates after three.js version bump
Also:
Removes husky again, because the pre-commit lint needs a bump to at least Node 15 to accept some code in dependencies (nullish coalescing assignment), and upgrading that is a whole other package of work.
Prevents Browserstack CI from trying to run on Chromium, as the video playback will fail.