Added HDR (arbitrary encoding) support to CubeTexturePass

[DanielSturk]

I also added the MeshCubeMaterial to the WebGLBackground so that it wouldn't have to do that weird map injection (https://github.com/mrdoob/three.js/blob/dev/src/renderers/webgl/WebGLBackground.js#L90)

Examples: webgl_postprocessing_backgrounds.html (for HDR CubeTexturePass) webgl_materials_envmaps_hdr.html (for WebGLBackground)

[Mugen87]

I guess this closes #9163?

[DanielSturk]

I guess this closes #9163?

Yes, thank you for linking that

[WestLangley]

I guess this closes #9163?

After spending almost an hour studying this PR, I was wondering how you came to have the skills to do this, and why you were making so many proposed changes in a single PR.

You used code #9163? What is different here?

[bhouston]

@WestLangley btw to clear things up, @DanielSturk is working with me to porting back some of our private ThreeJS changes into ThreeJS mainline. Thus yeah, some of this is my code.

[DanielSturk]

In addition to webgl_postprocessing_backgrounds.html I tested it with all of -webgl_envmap_* -webgl_materials_variations_standard.html -webgl_materials_standard.html And it seems to work fine

[DanielSturk]

You used code #9163? What is different here?

It's mostly a revive of #9163 (I didn't know that existed, #9163 has now been closed), plus I integrated it into WebGLBackground

[bhouston]

We have checked and it does not appear to break anything, where as the previous PR did break things.

[Mugen87]

@DanielSturk If you resolve the merge conflicts, I'd like to approve this change. Introducing MeshCubeMaterial is a good idea.

[WestLangley]

@WestLangley btw to clear things up, @DanielSturk is working with me to porting back some of our private ThreeJS changes into ThreeJS mainline

I see.

This PR was filed with little explanation. Is the objective to be able to blur backgrounds? If so, I am generally supportive of this.

I'll add some comments inline.

[WestLangley]

I also added the MeshCubeMaterial to the WebGLBackground

I would request that you remove that for now since it changes the background behavoir, and the current background code works fine as-is.

Perhaps you can use your new material in WebGLBackground in a follow-up PR if there is agreement.

[WestLangley]

@mrdoob, an alternative to adding THREE.SkyboxMaterial is to add THREE.Skybox, instead.

It may be more handy as it could automate things such as:

skybox.onBeforeRender = function ( renderer, scene, camera ) {

	this.matrixWorld.copyPosition( camera.matrixWorld );
													
};

If we went with THREE.Skybox, it would still use the shader proposed in this PR -- it just would not be a separate material; the shader would be inlined.

[DanielSturk]

Thanks for all the feedback @WestLangley, I'll look into making these changes soon

[DanielSturk]

This PR was filed with little explanation. Is the objective to be able to blur backgrounds? If so, I am generally supportive of this.

The primary object is to add HDR support to CubeTexturePass

[WestLangley]

Should the dat.gui provide a roughness control? Maybe in a later PR?

In the mean time, this is what is rendered for me. Do you get the same result?

[DanielSturk]

In the mean time, this is what is rendered for me. Do you get the same result?

That looks correct to me. A better demonstration would be turning the exposure way down to really illustrate the HDR

[WestLangley]

That looks correct to me

Sorry, but there is no way that is correct.

Compare with http://threejs.org/examples/webgl_materials_envmaps_hdr.html.

[DanielSturk]

That looks correct to me

Sorry, but there is no way that is correct.

Compare with http://threejs.org/examples/webgl_materials_envmaps_hdr.html.

It is correct. The other demo you compared with (http://threejs.org/examples/webgl_materials_envmaps_hdr.html) had renderer.gammaOutput=true. I added renderer.gammaOutput=true to this demo so that you can compare them more easily.

[WestLangley]

I added renderer.gammaOutput=true to this demo so that you can compare them more easily.

You don't add that so @WestLangley can "compare them more easily". You add it because it is required. These are concepts you need to understand.

With the last commit, you have broken the LDR case.

After generously investing several hours helping you with this PR, I think fixing that is the last change required.

[WestLangley]

@DanielSturk This is almost ready if you can fix the LDR case... :-)

[DanielSturk]

@WestLangley Correct me if I'm mistaken, but this demo was never actually broken The color only appeared off because the newly added 'hdr' box was checked by default. If you check out my new commit which reverts the renderer.gammaOutput=true change, and ensure the box isn't checked, then the demo should appear the same as on master. webgl_materials_envmaps_hdr.html should be correct on both commits

[WestLangley]

@DanielSturk I will continue help you fix your code...

First, revert the last commit. You need to covert from linear space to gamma-corrected color space.

Second, properly set the encoding for the loaded LDR texture in the loader callback function.

texture.encoding = THREE.sRGBEncoding;

The LDR and HDR use cases should now look similar to each other and similar to the other examples.

I think at that point this can be merged. :-)

[DanielSturk]

@WestLangley Thanks for sticking around... I tried applying your 2 changes to webgl_postprocessing_backgrounds.html, and I agree, the demo does appear very similar to the master version. However, applying neither of these 2 changes, still appears the same as master. Neither of these changes exist in master, and I believe they only appear correct when applied together because they are canceling each-other out.

I examined the fragment shaders for the skybox/cube background in both master and in my branch, and found that without the changes, the encoding/decoding is the same. With the 2 changes, the encoding is different, but visually, the 2 changes to the encoding cancel each other out.

master: Linear

without changes: Linear -> Linear -> Linear

with both changes: sRGB -> Linear -> Gamma

Of course it's no coincidence that sRGB->Linear->Gamma appear very similar to just Linear. The sRGBToLinear() function raises the color to the power of 2.4, and the linearToGamma() function raises the color to the power of 1/2.0

[WestLangley]

@bhouston See if you can provide Daniel some references about rendering in linear colorspace so he understands the issue and we can get this PR merged. I have done my best to help.

An alternate solution -- an probably more appropriate solution -- is to add GammaCorrectionPass in place of copyPass as the last EffectComposer pass. You will then remove renderer.gammaOutput and post-process in linear space. That is probably the most appropriate workflow, but it is up to you.

@DanielSturk Good luck! :-)

[bhouston]

If we want to be fully correct, we should be using "sRGB -> Linear -> Gamma" via:

• envMapToTexelToLinaer
• linearToOutputTexel

Then it will work no matter the input and output encoding.

[bhouston]

An alternate solution -- an probably more appropriate solution -- is to add GammaCorrectionPass in place of copyPass as the last EffectComposer pass. You will then remove renderer.gammaOutput and post-process in linear space. That is probably the most appropriate workflow, but it is up to you.

Be aware that GammaCorrectionPass is generally a bad idea and should never be used if it can be avoided.

The reason is that there is a rounding from the main linear space render results when it writes to the intermediate RGBA8 buffer and then you read in the GammaCorrectionPass and then do another quite subtle transform and it rounds again when it writes to the final RGBA8 buffer. Because of the nature of the gamma correction transform, it will reduce the possible color intensites from the normal 256 per channel to just 184.

Here is a spreadsheet that shows the rounding that occurs and all the intensities that are no longer achievable:

https://docs.google.com/spreadsheets/d/1sEhW8QeilNtsjcgd0fX1dcoO3L1p4Ow-U9v96I0aYiM/edit#gid=0

Instead of 16,777,216 (256256256) possible colors, you can only get 6,229,504 (184184184) if you use the GammaCorrectionPAss -- you've reduced the number of total colors that Three.JS can render by 64%. It will exacerbate banding in gradient areas especially.

Basically no one should ever use GammaCorrectionPass if they can avoid it.

[bhouston]

@DanielSturk please consult/ping me if @WestLangley or another veteran Three.JS contributor gives you feedback before responding. I should properly take the load off of them when possible.

[WestLangley]

@bhouston Thanks for that info!

[DanielSturk]

Thanks @WestLangley. After speaking with @bhouston I now understand the situation. I've add back changes now.

[WestLangley]

@DanielSturk Thanks for this!

[WestLangley]

@mrdoob Let's merge this if you approve.

[DanielSturk]

@jvestolas Sure... removed