Set WebXR's fixedFoveation property based on foveationLevel (fixes #5108)#5110
Set WebXR's fixedFoveation property based on foveationLevel (fixes #5108)#5110dmarcos merged 1 commit intoaframevr:masterfrom
Conversation
|
Thank you! This is awesome. Congrats on your first contribution 👏 |
| vrManager.layersEnabled = xrInit.requiredFeatures.indexOf('layers') !== -1; | ||
| vrManager.setSession(xrSession); | ||
| vrManager.setSession(xrSession).then(function () { | ||
| vrManager.setFoveation(rendererSystem.foveationLevel); |
There was a problem hiding this comment.
This broke Firefox, we need to check if vrManager.setFoveation is defined before calling it. On Firefox vrManager is WebVRManager that we still maintain in the super-three fork, not WebXRManager.
There was a problem hiding this comment.
Instead of adding a condition in aframe code, we could maybe add a dummy setFoveation function in WebVRManager to fix that @dmarcos?
There was a problem hiding this comment.
I don't think it's this invocation that causes issues, since it's inside the this.hasWebXR clause. It's probably the one added in renderer.js#update, as that one is called unconditionally. Good catch, in Firefox I always have dom.vr.webxr.enabled set to true, so it didn't fail for me during testing.
In any case, I also think a (dummy) setFoveation in WebVRManager is the cleanest solution. The less difference there is between the WebVRManager and WebXRManager in terms of interface, the less likely bugs like these are.
There was a problem hiding this comment.
You're right. We need to do some other fixes in WebVRManager, see #5102 (comment) so I guess it's one more. :)
Description:
These changes make
foveationLevelproperty have an effect on WebXR by setting thefoveationproperty through Three.js. In contrast to thefoveationLevelof WebVR, this can be changed on the fly.One thing I noticed is that the async
WebXRManager.setSessionis not awaited. Setting thefoveationbecomes a no-op unless the base layer is initialized, which happens duringsetSession. I now simply chain the setting of thefoveationto it, but it feels like the promise returned byenterVRshould not resolve before setSession is done in the first place. But I might be missing something and didn't want to change more than is needed.Changes proposed:
foveationLevelset thefoveationof WebXR (and no longer for WebVR)foveationLevelto1(which it effectively already was in case of WebXR)