Three.js: WebGLRenderer.render() should respect previously set render target

[maccesch]

A little while ago clearRenderTarget() was (rightly) deprecated because it internally changed the render target. Now you have to do it explicitely:

renderer.setRenderTarget(target);
renderer.clear();

This makes the internally used concept of a state machine more clear.
But by that logic a call to render() should use the current render target as well:

renderer.setRenderTarget(target);
renderer.render(scene, camera);      // should render to target

But that doesn't work because of this line:
https://github.com/mrdoob/three.js/blob/3013895bf68ad0c27fb0f9d3ca0beea2c0e7a87a/src/renderers/WebGLRenderer.js#L1106

Right now it only works like this:

renderer.setRenderTarget(target);
renderer.render(scene, camera, target);

Please let me know what you think. I'd be happy to submit a PR for this.

[pailhead]

I just ran into this.

I have to do:

renderer.setRenderTarget( target )
renderer.clear()
renderer.render( scene1, camera, target, false)
renderer.render( scene2, camera, target, false)

I'd expect:

renderer.setRenderTarget( target )
renderer.clear()
renderer.render( scene1, camera )
renderer.render( scene2, camera )

[Mugen87]

Yeah, the current API is somewhat confusing. From my point of view the problem with this issue is the following: Just removing the following line is no good solution.

https://github.com/mrdoob/three.js/blob/3013895bf68ad0c27fb0f9d3ca0beea2c0e7a87a/src/renderers/WebGLRenderer.js#L1106

The problem is that the entire signature of WebGLRenderer.render() needs to be changed since the renderTarget parameter does not fit into the "state machine" concept.

https://github.com/mrdoob/three.js/blob/3013895bf68ad0c27fb0f9d3ca0beea2c0e7a87a/src/renderers/WebGLRenderer.js#L1027

[maccesch]

@Mugen87 Yes just removing the line would probably break a lot of existing code. But couldn't we deprecate the use of the renderTarget parameter and change the code so if you don't provide one then the current active one is used instead of setting it to null?

Here's a quick code draft:

this.render = function ( scene, camera, renderTarget, forceClear ) {
  if ( renderTarget === true ) {
    forceClear = true;
  }
  ...
  if ( renderTarget instanceof RenderTarget ) {
    console.warn('Deprecated ...');
    this.setRenderTarget( renderTarget );
  }
}

[Mugen87]

Let's see what @mrdoob and @WestLangley thinks about this. I remember we have recently done similar ctor/method signature changes e.g. #14790

[WestLangley]

The WebGL API is a state machine, but the three.js API is not.

The suggestion proposed in this issue would require the render target to be set prior to every render call:

renderer.setRenderTarget( null );
renderer.render( scene, camera );

because there is no guarantee the render target had not been changed by some other method.

[maccesch]

@WestLangley I'm a little confused now. In deprecating clearRenderTarget() (#14842) you made basically the opposite argument regarding statefulness. And the WebGLRenderer.clear() method definitely follows a state machine concept now as the initial examples in this issue demonstrate.

On the other hand I can see your point. Next to WebGLRenderer
CubeCamera and WebGLShadowMap are changing the render target behind the scenes. Although CubeCamera sets it to null after updating. So when using those some special considerations would be necessary.

But in any case it would good when these changing render target occurrences would be pointed out in the docs, right?

[Mugen87]

But in any case it would good when these changing render target occurrences would be pointed out in the docs, right?

Definitely!

[WestLangley]

Right now it only works like this:

renderer.setRenderTarget(target);
renderer.render(scene, camera, target);

Why are you calling setRenderTarget() in that case?

So when using those some special considerations would be necessary.

But in any case it would good when these changing render target occurrences would be pointed out in the docs, right?

I do not understand what you are trying to say in those two sentences.

Although CubeCamera sets it to null after updating.

Apparently, the cube camera code needs to be corrected to restore the previous render target, instead.

If the user changes the render target, the user should restore the target to the previous value. Shadow mapping is implemented by the renderer, so that is the renderer's business, I guess.

[maccesch]

Right now it only works like this:

renderer.setRenderTarget(target);
renderer.render(scene, camera, target);

Why are you calling setRenderTarget() in that case?

You're absolutely right, in that case it wouldn't make sense. I forgot to add that I need to clear the render target before. So really I use it like this:

renderer.setRenderTarget(target);
renderer.clear(true, false, false);
renderer.render(scene, camera, target);
renderer.render(otherScene, otherCamera, target);

---

I do not understand what you are trying to say in those two sentences.

When the render target isn't left changed in any function then of course we don't need to point anything out in the docs.

---

If the user changes the render target, the user should restore the target to the previous value.

I totally agree. But that means it is ok if the user has to reset the render target to null if he changed it before:

renderer.setRenderTarget(null);
renderer.render(scene, camera);

[pailhead]

I somewhat expect:

renderer.setRenderTarget( target )
renderer.clear()
renderer.render( scene1, camera, target, false)
renderer.render( scene2, camera, target, false)

To be the same as

renderer.render( scene1, camera, target, true )
renderer.render( scene2, camera, target, false )

But:

• it doesnt seem to be (?)
• im not sure how to clear just the depth for example, or just the color or stencil. It seems only possible with renderer.clear()?

[mrdoob]

I agree with the OP, I think we should deprecate the renderTarget argument (and the forceClear argument too while we are at it.

this.render = function ( scene, camera ) {

    if ( arguments[ 2 ] !== undefined ) {
        console.warn( 'THREE.WebGLRenderer.render(): the renderTarget argument has been removed. Use .setRenderTarget() instead.' );
    }
    if ( arguments[ 3 ] !== undefined ) {
        console.warn( 'THREE.WebGLRenderer.render(): the forceClear argument has been removed.' );
    }