WebGLRenderTargets shouldn't be treated like special Textures

[vanruesc]

Preface

Following up on #8417.

This issue's purpose is to keep track of a problem that was identified during an attempt to refactor the WebGLRenderer in order to move away from treating render targets like special textures.

The goal

WebGLRenderTargets should not be used like Textures. Their texture property should be used instead.

material.map = renderTarget.texture; // not material.map = renderTarget;

The first attempt

WebGLRenderer: e255f01
WebGLPrograms: ae22bc1
WebGLRenderTargetCube: dcfcd4d

Description of the problem

By dropping the special treatment for WebGLRenderTargets, this flipEnvMap check and these cube render target checks: 1 | 2 become problematic.

When texture properties of materials are always Texture instances, it's no longer possible to rely on instanceof WebGLRenderTargetCube. The only way to know whether the texture at hand belongs to a WebGLRenderTargetCube is an ugly flag. Furthermore, the WebGLRenderTargetCube has a Texture instance because it extends WebGLRenderTarget. CubeTexture is therefore not involved in these checks, so instanceof THREE.CubeTexture is not an option.

Shader texture uniforms are another problem, because they are updated every frame.

Providing the user with warnings after checking for instanceof THREE.WebGLRenderTarget shouldn't be done every frame, but I couldn't find a better place for this.

Any suggestions would be much appreciated. Don't hesitate to make your own PR if you find a solid solution to the problem.

Three.js version

• Dev
• r75

[tschw]

Just moved that code around in #8606 . Now we already know whether a texture is 2D or Cube (because WebGL gives us that info querying the uniforms of the program). The entry points (from WebGLUniforms) into the renderer are now setTexture2D (formerly setTexture) and setTextureCube.

Furthermore, the WebGLRenderTargetCube has a Texture instance because it extends WebGLRenderTarget

A quite elegant solution (possible in JavaScript, but one that would break in C++) is to defer the creation of the value that is used for .texture to a method (e.g. createTexture). The constructor of WebGLRenderTarget would call that method and WebGLRenderTargetCube would override it to get a proper CubeTexture.

I think, mipmapping should be switchable, and off by default for render targets, and the renderer-internal function setCubeTexture could really use a bit of a cleanup.

Shader texture uniforms are another problem, because they are updated every frame.

Could force .needsUpdate to true on the render target texture by setting a property.

Providing the user with warnings after checking for instanceof THREE.WebGLRenderTarget shouldn't be done every frame, but I couldn't find a better place for this.

See here.

HTH to pull your PR up (bet I broke it).

[vanruesc]

Thanks for the heads-up! These changes might really help, will look into it as soon as I can.

[verteAzur]

What does it means from a user perspective when one sees this warning:

three.webglrenderer.setTexture2D: don't use render targets as textures. Use their .texture property instead.

[mrdoob]

@verteAzur

Instead of doing this...

var renderTarget = new THREE.RenderTarget( 256, 256 );
var material = new THREE.MeshBasicMaterial( {
    map: renderTarget
} );

You should do this:

var renderTarget = new THREE.RenderTarget( 256, 256 );
var material = new THREE.MeshBasicMaterial( {
    map: renderTarget.texture
} );

What code is triggering this?

[verteAzur]

Thank you for the info. It's the only warning/error I have seen from r77 to r78 migration.

[WestLangley]

@mrdoob You mean renderTarget and THREE.WebGLRenderTarget.

[verteAzur]

@mrdoob

What code is triggering this?

I have to pin point the exact spot. It is in the last part of the large loading process though. I have updated the mirror and water shaders, thinking it could be coming from that piece of code, but in vain. I'll get back to you when I find the line...

[mrdoob]

@mrdoob You mean renderTarget and THREE.WebGLRenderTarget.

Ops! Thanks! It was early in the morning... 😇
Fixed!

[verteAzur]

The code triggering the warning is vast. It comes from the dynamic terrain example prior v75. It could be in the long list of integrated shaders: maybe the blooming or the blurring one. Do you have any idea?

[mrdoob]

No... Shouldn't you be able to check the callstack?

[verteAzur]

re mrdoob: I have found the line that needed the "renderTarget.texture" update. It was located in the render member function of the THREE.ShaderPass.prototype. I did make the change, but the warning was still triggering. Then, I've decided to update all postprocessing classes and shaders (Convolution, Copy,Effectcomposer, Shaderpass, Maskpass, RenderPass, Bloompass, FilmPass) and it did work like a charm. Everything is working without any warning nor error.

We did notice a few things while going through the update process that might help others: #9267

Thank you all for all!