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

Fix CubeCamera breaking in WebXR #19439

Merged
merged 4 commits into from
May 25, 2020
Merged

Fix CubeCamera breaking in WebXR #19439

merged 4 commits into from
May 25, 2020

Conversation

michaelybecker
Copy link
Contributor

@michaelybecker michaelybecker commented May 24, 2020

implements @Mugen87's fix to #19428 by disabling WebXR before updating the cube render target and reenabling afterwards.

Tested locally on an existing WebXR project and confirmed working (after slight refactoring to accommodate the new WebGLCubeRenderTarget injection method per #19137).

Fixed #19428.

@Mugen87 Mugen87 added this to the r117 milestone May 24, 2020
@mrdoob mrdoob merged commit 00fd213 into mrdoob:dev May 25, 2020
@mrdoob
Copy link
Owner

mrdoob commented May 25, 2020

Thanks!

@mrdoob
Copy link
Owner

mrdoob commented May 25, 2020

@Mugen87 What do you think about using this pattern?

diff --git a/src/cameras/CubeCamera.js b/src/cameras/CubeCamera.js
index d2fa6ee17..156862d65 100644
--- a/src/cameras/CubeCamera.js
+++ b/src/cameras/CubeCamera.js
@@ -27,36 +27,42 @@ function CubeCamera( near, far, renderTarget ) {
 	this.renderTarget = renderTarget;
 
 	var cameraPX = new PerspectiveCamera( fov, aspect, near, far );
+	cameraPX.isInternal = true;
 	cameraPX.layers = this.layers;
 	cameraPX.up.set( 0, - 1, 0 );
 	cameraPX.lookAt( new Vector3( 1, 0, 0 ) );
 	this.add( cameraPX );
 
 	var cameraNX = new PerspectiveCamera( fov, aspect, near, far );
+	cameraNX.isInternal = true;
 	cameraNX.layers = this.layers;
 	cameraNX.up.set( 0, - 1, 0 );
 	cameraNX.lookAt( new Vector3( - 1, 0, 0 ) );
 	this.add( cameraNX );
 
 	var cameraPY = new PerspectiveCamera( fov, aspect, near, far );
+	cameraPY.isInternal = true;
 	cameraPY.layers = this.layers;
 	cameraPY.up.set( 0, 0, 1 );
 	cameraPY.lookAt( new Vector3( 0, 1, 0 ) );
 	this.add( cameraPY );
 
 	var cameraNY = new PerspectiveCamera( fov, aspect, near, far );
+	cameraNY.isInternal = true;
 	cameraNY.layers = this.layers;
 	cameraNY.up.set( 0, 0, - 1 );
 	cameraNY.lookAt( new Vector3( 0, - 1, 0 ) );
 	this.add( cameraNY );
 
 	var cameraPZ = new PerspectiveCamera( fov, aspect, near, far );
+	cameraPZ.isInternal = true;
 	cameraPZ.layers = this.layers;
 	cameraPZ.up.set( 0, - 1, 0 );
 	cameraPZ.lookAt( new Vector3( 0, 0, 1 ) );
 	this.add( cameraPZ );
 
 	var cameraNZ = new PerspectiveCamera( fov, aspect, near, far );
+	cameraNZ.isInternal = true;
 	cameraNZ.layers = this.layers;
 	cameraNZ.up.set( 0, - 1, 0 );
 	cameraNZ.lookAt( new Vector3( 0, 0, - 1 ) );
diff --git a/src/renderers/WebGLRenderer.js b/src/renderers/WebGLRenderer.js
index cd5578850..5178a8ae2 100644
--- a/src/renderers/WebGLRenderer.js
+++ b/src/renderers/WebGLRenderer.js
@@ -1161,7 +1161,7 @@ function WebGLRenderer( parameters ) {
 
 		if ( camera.parent === null ) camera.updateMatrixWorld();
 
-		if ( xr.enabled && xr.isPresenting ) {
+		if ( camera.isInternal !== true && xr.enabled && xr.isPresenting ) {
 
 			camera = xr.getCamera( camera );
 

Do you think this would work with Reflector, Fire, Refractor and Water?

@Mugen87
Copy link
Collaborator

Mugen87 commented May 25, 2020

Um, not sure. Enabling/disabling XR is related to the cases when code renders to a texture. This can also happen with the same camera that is used to render the scene. If so, you would have to toggle isInternal in the same way like xr.enabled.

@mrdoob
Copy link
Owner

mrdoob commented May 25, 2020

Ah, right. Okay 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

scene.background and/or THREE.CubeCamera broken in WebXR
3 participants