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

Different environment with ENVMAP_TYPE_CUBE_UV - NodeMaterial #17102

Closed
3 tasks done
sunag opened this issue Jul 25, 2019 · 22 comments
Closed
3 tasks done

Different environment with ENVMAP_TYPE_CUBE_UV - NodeMaterial #17102

sunag opened this issue Jul 25, 2019 · 22 comments

Comments

@sunag
Copy link
Collaborator

sunag commented Jul 25, 2019

Description of the problem

Adding ENVMAP_TYPE_CUBE_UV in physical-based material or the property envMap is a PREM affect significantly the light calculation in material, e.g:

mesh.material = new THREE.StandardNodeMaterial();
//...
mesh.material.defines['ENVMAP_TYPE_CUBE_UV'] = true;

It probably is related to this:

// Both indirect specular and diffuse light accumulate here
// if energy preservation enabled, and PMREM provided.
#if defined( ENVMAP_TYPE_CUBE_UV )

I think that none ENVMAP... defines should be in lights_physical_pars_fragment chuck.

/ping @WestLangley

Three.js version
  • Dev
Browser
  • All of them
OS
  • All of them
@WestLangley
Copy link
Collaborator

It is to be expected that PMREM and non-PMREM will produce different lighting.

I think that no ENVMAP... defines should be in lights_physical_pars_fragment chuck.

I do not have a strong opinion on that. Feel free to propose a refactoring.

if energy preservation enabled...

@jsantell will have to help with that issue.

/ping @jsantell

@sunag You do need to add support for envMapIntensity and include it in the Node examples.

@sunag
Copy link
Collaborator Author

sunag commented Jul 26, 2019

It is to be expected that PMREM and non-PMREM will produce different lighting.

My main issue is when I should use ENVMAP_TYPE_CUBE_UV or also some ENERGY_PRESERVATION if I have differents envMap in the same environment using NodeMaterial, e.g:

material.environment = mix( premCube,  traditionalCube, .5 );

In this case, ENVMAP_TYPE_CUBE_UV is enabled or not? and even if it is enabled It can affect traditionalCube, if it is disabled affect premCube lighting, this approach seems incompatible with NodeMaterial

Maybe it should be ever enabled in material with a property: material.energyPreservation = true?

@sunag You do need to add support for envMapIntensity and include it in the Node examples.

Ok I will add 👍

@sunag
Copy link
Collaborator Author

sunag commented Aug 1, 2019

Does @bhouston have any thoughts about this?

@bhouston
Copy link
Contributor

bhouston commented Aug 1, 2019

One only needs a single PMREM map per object. It is much more accurate than a standard envMap.
It should look identical to a non-node material PMREM envMap result.

Having envMapIntensity would be useful in my opinion as unfortunately cubeMaps are usually unitless -- although I wish there was a HDR/light map standard that had units. envMapIntensity is basically the intensity of the light.

material.environment = mix( premCube, traditionalCube, .5 );

The above case is a bit weird. Generally one does not interpolate cubemaps used for specular reflections as the reflections do not line up between the two positionally different cubemaps. Usually one just picks between a series of different cube maps and assigns one to a particular object, per: https://seblagarde.wordpress.com/2012/09/29/image-based-lighting-approaches-and-parallax-corrected-cubemap/

https://seblagarde.wordpress.com/2012/09/29/image-based-lighting-approaches-and-parallax-corrected-cubemap/
The above says that if you do a bunch of corrections based on knowing the positions of the original cubemaps you can correct the cube maps to make them align when interpolating. We should do that in the future, but one cubemap per object for the near term I believe is more than sufficient.

@sunag
Copy link
Collaborator Author

sunag commented Aug 1, 2019

Thanks @bhouston.

The above case is a bit weird. Generally one does not interpolate cubemaps used for specular reflections as the reflections do not line up between the two positionally different cubemaps.

The example of mix two type of cubemaps is only a paradigm to illustrate that we have a compatibility issue.

To be more specific, if add material.defines['ENVMAP_TYPE_CUBE_UV'] = true this affect even direct light, almost all light of threejs is affected not only image based, same without use any envMap. Using ENVMAP_TYPE_CUBE_UV define can be noted a additional brighness in material only adding DirectionalLight.

It would not be possible move this code to envmap_physical_pars_fragment?

// Both indirect specular and diffuse light accumulate here
// if energy preservation enabled, and PMREM provided.
#if defined( ENVMAP_TYPE_CUBE_UV )

Code Example

var material = new Nodes.StandardNodeMaterial();
// affect global light not only envMap (PREM)
material.defines['ENVMAP_TYPE_CUBE_UV'] = true;

@sunag
Copy link
Collaborator Author

sunag commented Aug 2, 2019

As I understand it, energy preservation or Energy conservation ( https://learnopengl.com/PBR/Theory, https://marmoset.co/posts/basic-theory-of-physically-based-rendering/ ) is a characteristic of the material and not of the type of envMap, in this implementation seems wrong use energy preservation only to PREM -> ENVMAP_TYPE_CUBE_UV once this is correctly implemented as lights_physical for any type of light but in pratice this is not happening because of this define ENVMAP_TYPE_CUBE_UV.

I do not have a strong opinion on that. Feel free to propose a refactoring.

@WestLangley I would like to make a refactoring but first I would like to know if you support I implement this as default in Physically-Based Material or add a property in material as material.energyPreservation = true?

@sunag
Copy link
Collaborator Author

sunag commented Aug 2, 2019

Energy Compensation in #16977 Enterprise PBR

@WestLangley
Copy link
Collaborator

@sunag

I do not blame you for being confused. There have been a lot of changes, and it is confusing to me, too.

An environment map is treated as a source of light. Some of that light is reflected specularly (indirect specular), and some is reflected diffusely (indirect diffuse).

When material.envMap is a traditional cube map, the Standard and Physical shaders ignore the indirect diffuse light from the env map. This is not correct, and the rendered material is too dark.

As a result, PMREM was supported.

When material.envMap is a PMREM map, the Standard and Physical shaders correctly model the indirect diffuse light from the env map.

There are some other minor differences, but those are the main ones.

// if energy preservation enabled, and PMREM provided.

I am not aware of a flag that enables energy preservation. I think it is automatic if PMREM is provided. Unless I am missing something, I see no reason for such a flag.

Maybe @jsantell or @bhouston can confirm.

@sunag
Copy link
Collaborator Author

sunag commented Aug 2, 2019

@WestLangley Thank you very much for the explanation. I will create a PR with this refactoring and try to justify with the base on PBR articles and NodeMaterial.

@njarraud
Copy link
Contributor

njarraud commented Aug 3, 2019

@sunag You do need to add support for envMapIntensity and include it in the Node examples.

Really looking forward to this one. For now, it is not possible to properly decouple the environment intensity and the light object strengths. That is something critical to produce great custom scene lightings.

@sunag
Copy link
Collaborator Author

sunag commented Aug 3, 2019

I think that envMapIntensity only makes sense in MeshStandardNodeMaterial for the base of the new API like StandardNodeMaterial for example, it can be done with nodes:

var intensity = new Nodes.FloatNode( 1 );
var envMap = new Nodes.CubeTextureNode( cubemap );
var envMapIntensity = new Nodes.OperatorNode( envMap, intensity, Nodes.OperatorNode.MUL );

material.environment = envMapIntensity;

@njarraud
Copy link
Contributor

njarraud commented Aug 3, 2019

@sunag Thank you for the explanations. It does make sense.

However I tried this method but the result is not as expected -
I made an example with an intensity of 0 which shall produce a black mesh but does not.
intensity

@sunag
Copy link
Collaborator Author

sunag commented Aug 3, 2019

I did not have time to test but I imagine that this is a bug related with this:

builder.context.extra.irradiance = irradiance;

output.push( "irradiance += PI * " + environment.extra.irradiance + ";" );

Apparently only the indirect light remained here...

Maybe I was a bit naive implementation of this extra.irradiance but we will consider as the way to something better.

@njarraud thanks too for the test.

@njarraud
Copy link
Contributor

njarraud commented Aug 3, 2019

Apparently only the indirect light remained here...

If you tweak the metalness value from 0 to 1 in my example, it will slowly get darker until it is all black for 1.

@sunag
Copy link
Collaborator Author

sunag commented Aug 3, 2019

Fixed! I modify an example to support intensity.

@jsantell
Copy link
Contributor

jsantell commented Aug 4, 2019

I am not aware of a flag that enables energy preservation. I think it is automatic if PMREM is provided. Unless I am missing something, I see no reason for such a flag.

That’s correct. I'm not terribly familiar with how the node system interacts with the traditional shaders. The materials support two types of environment maps: a cube map that provides indirect specular (ENVMAP_TYPE_CUBE) and a texture with several mipmaps packed via the PMREM components interpreted as HDR, providing indirect diffuse and specular (ENVMAP_TYPE_CUBE_UV). The indirect diffuse provided by the PMREM maps is mostly the energy preservation, leaving us with two map types referred to by related but independent properties: cubemap/LDR and cubemapUV/pmrem/HDR/energy preservation.

That being said, with the context of supporting node materials, it makes sense to me to decouple sampling logic (cubemap vs PMREM-specific UV packing) from whether or not indirect diffuse is used (energy preservation), assuming that there is another way to provide the data necessary to generate the indirect diffuse via the node material (via LUT? SH?). That being said, what is the scenario in which ENVMAP_TYPE_CUBE_UV is true when a non-PMREM-processed envmap is used?

@sunag
Copy link
Collaborator Author

sunag commented Aug 4, 2019

That being said, what is the scenario in which ENVMAP_TYPE_CUBE_UV is true when a non-PMREM-processed envmap is used?

ENVMAP_TYPE_CUBE has implemented in lights_physical_pars_fragment affecting global light, not only IBL PREM, the problem this, is that the indirect/direct diffuse and specular of PREM blend with others lights, including the analytical AmbientLight, HemisphereLight because they all too generate irradiance diffuse that can be used in energy preservation calculation.

I tried to know more about through some references cited here and found it energy preservation is a characteristic of BRDF class, thus being is a correct the implementation used today in global light. The issue is that there are other lights in threejs besides IBL PREM that also generate indirect diffuse and specular, and in the specific case of NodeMaterial it cannot get stuck just in the PREM approach although it is the best threejs approach to IBL today.

This example can be reproduced as follows:

  1. Add some lights DirectionalLight and indirect light like AmbientLight
  2. Add ENVMAP_TYPE_CUBE_UV define in material even without PREM or any other envMap
  3. With define global light will be affected positively because of use the preservation calculation
var material = new Nodes.StandardNodeMaterial();
material.defines['ENVMAP_TYPE_CUBE_UV'] = true;

For this reason I see the need to become energy preservation a feature independent of PREM.

@WestLangley
Copy link
Collaborator

I think that envMapIntensity only makes sense in MeshStandardNodeMaterial

envMapIntensity is required as a built-in property, even though a user can implement it using Node.

A JPG texture can be an environment map, but it does not have units. envMapIntensity is where the units come from.

It is true that some environment maps can have implied physical units (radiance or luminance), however, they are also sometimes normalized, in which case the envMapIntensity property is required.

@WestLangley
Copy link
Collaborator

WestLangley commented Aug 5, 2019

ENVMAP_TYPE_CUBE has implemented in lights_physical_pars_fragment affecting global light, not only IBL PREM, the problem this, is that the indirect/direct diffuse and specular of PREM blend with others lights, including the analytical AmbientLight, HemisphereLight because they all too generate irradiance diffuse that can be used in energy preservation calculation.

@jsantell Yes, @sunag is correct. The irradiance you pass into your multiple-scattering calculation is from all indirect light sources, not just from the env map. I am pretty sure that is not correct. The referenced article is only about energy conservation when using IBLs.

@WestLangley
Copy link
Collaborator

the PMREM components interpreted as HDR

PMREM can be used with LDR. It is not HDR-specific.

@sunag
Copy link
Collaborator Author

sunag commented Aug 5, 2019

A JPG texture can be an environment map, but it does not have units. envMapIntensity is where the units come from.

I agree with envMapIntensity only in MeshStandardNodeMaterial to keep backwards compatibility and not in the extended API because the concept of nodes in NodeMaterial should be different, so material.environment in StandardNodeMaterial is not approached to use only map like material.envMap but as any light source, IBL or not. This is also a simplification of the code so much for the glsl as for javascript.

In case the user needs adjust intensity he can add this:

material.environment = new OperatorNode( envMap, new FloatNode( 1 ), OperatorNode.MUL );

Like this example:
https://rawgit.com/sunag/three.js/dev-fix-nodes/examples/webgl_materials_envmaps_pmrem_nodes.html

@WestLangley
Copy link
Collaborator

I agree with envMapIntensity only in MeshStandardNodeMaterial ... and not in the extended API

Yes, we are in agreement.

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

No branches or pull requests

5 participants