WebXR: Allows near and far clipping to be set

[DougReeder]

Fixes #14829

[DougReeder]

Half of a fix for aframevr/aframe#4387
The other half is changing A-Frame to call setCameraClipping()

[Mugen87]

It seems this is a duplicate of #14854.

Also notice #14854 (comment). It seems your current implementation does not fulfill the mentioned requirements.

[DougReeder]

#14854 appears to be out of date and either incomplete, or for an older version of the WebVR spec - depthNear and depthFar must be set on the render state, not the session.

WebGLRenderer.render() is passed a single camera object (which might be an ArrayCamera) which it then always passes to WebXRManager.getCamera(camera):
https://github.com/mrdoob/three.js/blob/dev/src/renderers/WebGLRenderer.js#L1109
I don't see that that comment raises any actual issues.

I will update this PR to set depthNear and depthFar from the camera passed to getCamera().

[Mugen87]

Instead of always doing the API call, you could cache depthNear and depthFar and only call the method if these values are different to cameraVR.near and cameraVR.far.

This kind of state caching is also used in the renderer. I would call the cache variables: _currentDepthNear and _currentDepthFar and then do this:

if ( _currentDepthNear !== cameraVR.near || _currentDepthFar !== cameraVR.far ) {

    session.updateRenderState( {
        depthNear: cameraVR.near,
        depthFar: cameraVR.far
    } );

    _currentDepthNear = cameraVR.near;
    _currentDepthFar = cameraVR.far;

}

[DougReeder]

That’s certainly doable, but the render state is already a cache - see https://github.com/immersive-web/webxr/blob/master/explainer.md - the paragraph starting “The XRFrame also makes a copy of the XRSession’s renderState ... just proper to the requestAnimationFrame() callbacks ...” There’s no evidence that this API call is expensive.

…

Sent from my iPhone

On Jan 8, 2020, at 5:20 AM, Michael Herzog ***@***.***> wrote:  @Mugen87 commented on this pull request. In src/renderers/webxr/WebXRManager.js: > @@ -301,6 +301,13 @@ function WebXRManager( renderer, gl ) { this.getCamera = function ( camera ) { + cameraVR.near = cameraR.near = cameraL.near = camera.near; + cameraVR.far = cameraR.far = cameraL.far = camera.far; + session.updateRenderState( { Instead of always doing the API call, you could cache depthNear and depthFar and only call the method if these values are different to cameraVR.near and cameraVR.far. This state caching is also used in the renderer. I would call the cache variables: _currentDepthNear and _currentDepthFar and then do this: if ( _currentDepthNear !== cameraVR.near || _currentDepthFar !== cameraVR.far ) { session.updateRenderState( { depthNear: cameraVR.near, depthFar: cameraVR.far } ); _currentDepthNear = cameraVR.near; _currentDepthFar = cameraVR.far; } — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

[Mugen87]

There’s no evidence that this API call is expensive.

@toji Is this true? In this case, caching depthNear and depthFar on engine level would not be necessary.

[toji]

It'll depend on the browser implementation, but in Chrome at least this call wouldn't be too expensive.

I'd double-check the timing, though. Any values passed into session.updateRenderState() will wait until the start of the next session.requestAnimationFrame() callback to take effect (so that everything is synchronized and the native systems have time to take it into account.) As such if you call this in a session.requestAnimationFrame() it won't actually take effect until the next frame, and if you change the value per-frame it would always be a frame behind. (Which is silly for depth near/far, I'm just trying to illustrate the point.)

Given how Three.js is structured, though, it may be that's impossible to fully avoid, since you won't know what camera is in use till it's passed to the render() function and that's pretty much guaranteed to be in a frame callback. 🤷‍♀

[Mugen87]

It'll depend on the browser implementation, but in Chrome at least this call wouldn't be too expensive.

I think it can't hurt to implement the suggested caching. Checking to variables is not expensive and in this way, we are on the safe side.

@mrdoob What do you think?

[DougReeder]

So, how do we proceed from here? Who does the merge into dev? I don't see an r113 branch.

[Mugen87]

@DougReeder The PR will be reviewed by @mrdoob and then be merged (if everything is fine).

[mrdoob]

Thanks!