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

scene.background and/or THREE.CubeCamera broken in WebXR #19428

Closed
3 of 13 tasks
michaelybecker opened this issue May 22, 2020 · 4 comments · Fixed by #19439
Closed
3 of 13 tasks

scene.background and/or THREE.CubeCamera broken in WebXR #19428

michaelybecker opened this issue May 22, 2020 · 4 comments · Fixed by #19439
Labels

Comments

@michaelybecker
Copy link
Contributor

michaelybecker commented May 22, 2020

Description of the problem

Hello,

What it says on the tin. Got a nice day/night cycle based on the ocean shader example. Relevant code is:

const cubeCamera = new CubeCamera(0.1, 1, 512);
cubeCamera.renderTarget.texture.generateMipmaps = true;
cubeCamera.renderTarget.texture.minFilter = LinearMipmapLinearFilter;
scene.background = cubeCamera.renderTarget;

and in my render loop:

cubeCamera.update(renderer, sky);

looks great on screen (right click+WASDQE to travel around), very broken in WebXR (tested on the Quest and in SteamVR via Virtual Desktop). Coordinate frame is all wonky, scale is all over the place, hard to even tell what's happening - guessing scene.background needs special treatment in the separate XR render loop?

Full code here. Feedback / thoughts appreciated! Happy to take a stab at fixing it myself but not sure where to start...

Three.js version
  • Dev
  • r116.1
  • ...
Browser
  • All of them
  • Chrome
  • Firefox
  • Internet Explorer
OS
  • All of them
  • Windows
  • macOS
  • Linux
  • Android
  • iOS
Hardware Requirements (graphics card, VR Device, ...)

Presumably any VR device; I'm testing on the quest via oculus browser and via Virtual Desktop / SteamVR.

@michaelybecker michaelybecker changed the title scene.background and/or three.cubecamera broken in WebXR scene.background and/or THREE.CubeCamera broken in WebXR May 22, 2020
@Mugen87
Copy link
Collaborator

Mugen87 commented May 23, 2020

I could imagine that CubeCamera.update() needs to disable WebXR when updating the cube render target and enable it again if necessary. Can you please test with the following modified version of CameraCamera.update():

this.update = function ( renderer, scene ) {

	if ( this.parent === null ) this.updateMatrixWorld();

	var currentXrEnabled = renderer.xr.enabled;
	var currentRenderTarget = renderer.getRenderTarget();

	renderer.xr.enabled = false;

	var generateMipmaps = renderTarget.texture.generateMipmaps;

	renderTarget.texture.generateMipmaps = false;

	renderer.setRenderTarget( renderTarget, 0 );
	renderer.render( scene, cameraPX );

	renderer.setRenderTarget( renderTarget, 1 );
	renderer.render( scene, cameraNX );

	renderer.setRenderTarget( renderTarget, 2 );
	renderer.render( scene, cameraPY );

	renderer.setRenderTarget( renderTarget, 3 );
	renderer.render( scene, cameraNY );

	renderer.setRenderTarget( renderTarget, 4 );
	renderer.render( scene, cameraPZ );

	renderTarget.texture.generateMipmaps = generateMipmaps;

	renderer.setRenderTarget( renderTarget, 5 );
	renderer.render( scene, cameraNZ );

	renderer.setRenderTarget( currentRenderTarget );
	renderer.xr.enabled = currentXrEnabled;

};

This updated method requires to use the dev version of three.js. Notice that CubeCamera has been slightly refactored. You now have to pass in an instance of WebGLCubeRenderTarget to the ctor (see webgl_materials_cubemap_dynamic for more details).

@michaelybecker
Copy link
Contributor Author

Lovely - thanks a bunch! Solves it right out of the box (minus adding this to the renderTarget references, which tells me you prob did this from memory, which is even more impressive somehow:)

Before closing - while I'm content hoarding this nice fix to myself, would this merit a quick PR? Happy to do it if so.

@Mugen87
Copy link
Collaborator

Mugen87 commented May 23, 2020

Yeah, feel free to make a PR with the fix 👍

@Mugen87 Mugen87 added the Bug label May 23, 2020
@Mugen87
Copy link
Collaborator

Mugen87 commented May 23, 2020

(minus adding this to the renderTarget references,

And yes, it's necessary to do this when using the current prod version. On dev it should be exactly like in the code listing.

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

Successfully merging a pull request may close this issue.

2 participants