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

Not update renderer property for ArrayCamera render #11051

Merged
merged 1 commit into from
Apr 4, 2017

Conversation

takahirox
Copy link
Collaborator

@takahirox takahirox commented Mar 24, 2017

This PR provides proper property update for rendering with ArrayCamera and
enables Post Processing with VR.

If I'm right, we shouldn't update WebGLRenderer properties for rendering with ArrayCamera
because it affects next render call. Updating state is good enough.

For example, assume we render with EffectComposer,
first pass is render pass with WebVRCamera(ArrayCamera) and
second pass is shader pass with OrthographicCamera.

The WebGLRenderer viewport/scissor property update of first render pass affects
the second shader pass so it results in rendering left/right into right side.

Current
image

With this change
image

@mrdoob
Copy link
Owner

mrdoob commented Apr 4, 2017

Hmm... I think this will have side effects. I'll merge and fix if I find anything.

@mrdoob mrdoob merged commit 61ec445 into mrdoob:dev Apr 4, 2017
@mrdoob
Copy link
Owner

mrdoob commented Apr 4, 2017

Thanks!

@mrdoob
Copy link
Owner

mrdoob commented Apr 4, 2017

I reverted this... I'll have a proper look tomorrow.
Create a PR with the example whenever you have a chance though.

@takahirox
Copy link
Collaborator Author

takahirox commented Apr 4, 2017

Let me explain my proposal again.

The current code's problem is rendering with ArrayCamera has viewport/scissor side effects
and they're propagated to the following rendering with other cameras.

In the following example, renderer.render( scene, arrayCamera )
updates viewport/scissor and doesn't reset them in .render()
then they unexpectedly affect renderer.render( scene, camera )

var arrayCamera = new THREE.ArrayCamera( [
    new THREE.PerspectiveCamera(),
    new THREE.PerspectiveCamera()
] );
var camera = new THREE.PerspectiveCamera();

function render() {

    renderer.render( scene, arrayCamera ); // has viewport/scissor side effect
    renderer.render( scene, camera ); // unexpectedly using updated viewport/scissor

}

because of this code in WebGLRenderer

if ( camera.isArrayCamera && camera.enabled ) {

    var cameras = camera.cameras;

    for ( var j = 0, jl = cameras.length; j < jl; j ++ ) {

        var camera2 = cameras[ j ];
        var bounds = camera2.bounds;

        _this.setViewport(
            bounds.x * _width * _pixelRatio, bounds.y * _height * _pixelRatio,
            bounds.z * _width * _pixelRatio, bounds.w * _height * _pixelRatio
        );
        _this.setScissor(
            bounds.x * _width * _pixelRatio, bounds.y * _height * _pixelRatio,
            bounds.z * _width * _pixelRatio, bounds.w * _height * _pixelRatio
        );
        _this.setScissorTest( true );

        renderObject( object, scene, camera2, geometry, material, group );

    }

} else {

_this.setViewport(
bounds.x * _width * _pixelRatio, bounds.y * _height * _pixelRatio,
bounds.z * _width * _pixelRatio, bounds.w * _height * _pixelRatio
);
_this.setScissor(
bounds.x * _width * _pixelRatio, bounds.y * _height * _pixelRatio,
bounds.z * _width * _pixelRatio, bounds.w * _height * _pixelRatio
);
_this.setScissorTest( true );

My idea is to set viewport/scissor with state not to have side effect.
If I'm right, state is dynamically set from renderer's property (and something else)
in the beginning of .render() so it doesn't have side effect.

Even if my idea wouldn't work, I think we should reset viewport/scissor in the end of .render().

@mrdoob
Copy link
Owner

mrdoob commented Apr 4, 2017

I think I understand. It'll be better to have the example so I can make sure the code is robust.

@takahirox
Copy link
Collaborator Author

Yup, I'll make.
(Sadly I've been too busy to work on that since I got back to Japan... but I will make!)

@mrdoob
Copy link
Owner

mrdoob commented Apr 4, 2017

No rush!

@takahirox
Copy link
Collaborator Author

I made jsfiddle. http://jsfiddle.net/o5rvfg9x/2/

renderer.autoClear = true;
renderer.render( scene, arrayCamera ); // should render left and right
renderer.autoClear = false;
renderer.render( scene, camera ); // should render center

It should render three spheres (left, right, and center) but
it renders the two(left and right) because
renderer.render( scene, arrayCamera ); has side effect of
renderer viewport/scissor and it's propagated to renderer.render( scene, camera );
then the second .render() unexpectedly renders right overlay.

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.

2 participants