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

Normal issue with new pmremGenerator and StandardNodeMaterial #18360

Closed
1 task done
njarraud opened this issue Jan 10, 2020 · 11 comments
Closed
1 task done

Normal issue with new pmremGenerator and StandardNodeMaterial #18360

njarraud opened this issue Jan 10, 2020 · 11 comments

Comments

@njarraud
Copy link
Contributor

njarraud commented Jan 10, 2020

Description of the problem

In the process of moving my app to the latest version of Threejs to have access to the new pmremGenerator, I found an issue with StandardNodeMaterial which wasn't existing before when I was using r108.

When an environment map is used, the normal input is sort of washed away/faded.
When no environment map is used and other types of lights are in the scene, it is back to normal.

I also made a comparison between StandardNodeMaterial and MeshStandardMaterial.
The object with MeshStandardMaterial is correctly rendered with the new pmremGenerator but not the one with StandardNodeMaterial.

I made two jsfiddle. First one is using an environment map - left is MeshStandardMaterial, right is StandardNodeMaterial. Second one is using point lights only:

env

point

Three.js version
  • Dev

/ping @sunag

@njarraud
Copy link
Contributor Author

I am really looking forward to finding a solution to this issue. If anybody has even a rough idea of which part of the code I shall look into I am really keen.

/ping @sunag

@WestLangley
Copy link
Collaborator

@njarraud Thanks for this, and for your other excellent Node test cases!

I do not know where the problem is, but I do know that it has been a challenge to keep Node up-to-date with all the changes in the core library. @sunag has done a great job making those change whenever he is able, but if you'd like to have a go at it, comparing glsl chunks is one reasonable place to start.

BTW, the differences you refer to are subtile enough not to readily be apparent -- at least to me. One thing you could do is create two scenes, each with one light, and then make two render passes. That way, each model is identically-lit. Describing what you see as different is helpful.

@sunag
Copy link
Collaborator

sunag commented Jan 14, 2020

I think that this issue already had detect in PREM Update #18161 (comment) ( Unsolved issues )

@njarraud Thanks for this, and for your other excellent Node test cases!

Yes. Thanks, too.

@sunag has done a great job making those change whenever he is able

Thank you @WestLangley. I hope soon I can work it out in full time for it will be faster...
You help aways been instrumental in doing a great job.

if you'd like to have a go at it, comparing glsl chunks is one reasonable place to start.

@njarraud This is exactly what you would do too.

@njarraud
Copy link
Contributor Author

njarraud commented Jan 14, 2020

@sunag has created an awesome system and unlocked new possibilities. He is my man. I cannot thank him enough for what he created :-)

However this issue makes everything fall down for my use case. The new pmremGenerator is so much better, faster and not buggy compared to the old one but on the other hand the whole system that I developed for the past 6 months and the wow factor resulting is kinda gone because of it.

I do not expect sunag to solve it for me and your pointer is very welcome. I'll investigate.

I made a couple of examples to get a better idea of the visual differences between both material systems (both used to look identical).

Environment texture:
envmap
Here are the individual images:
envmap_MeshStandardMaterial
envmap_StandardNodeMaterial

The whole benefit of baking round edges on a normal map is gone. It is even more obvious for brighter colours where the normal map is pretty much invisible.
envmap_StandardNodeMaterial_lines

Point Light - No difference at all. They are identical to the pixel
pointLight_MeshStandardMaterial

@njarraud
Copy link
Contributor Author

njarraud commented Jan 14, 2020

So I spent hours trying to understand more about how the shader system works and how to compare both. It looks like the issue comes from the irradiance calculation. The radiance seems to be correct.

Radiance only (irradiance = vec3(0.0)) - Both are exactly identical:
radiance

Irradiance only (radiance = vec3(0.0)):
irradiance

The mistake probably lies somewhere here 😁

nodeT23 = vWNormal;
nodeT26 = roughnessToMip( 1.0 );
nodeT25 = clamp( nodeT26, m0, cubeUV_maxMipLevel );
nodeT24 = floor( nodeT25 );
nodeT22 = bilinearCubeUV( nodeU6, nodeT23, nodeT24 );
nodeT28 = ( nodeT24 + 1.0 );
nodeT27 = bilinearCubeUV( nodeU6, nodeT23, nodeT28 );
nodeT31 = ( RGBEToLinear( ( nodeT22.tl ) ) );
nodeT32 = ( RGBEToLinear( ( nodeT22.tr ) ) );
nodeT33 = ( RGBEToLinear( ( nodeT22.bl ) ) );
nodeT34 = ( RGBEToLinear( ( nodeT22.br ) ) );
nodeT30 = ( mix( mix( nodeT31, nodeT32, nodeT22.f.x ), mix( nodeT33, nodeT34, nodeT22.f.x ), nodeT22.f.y ) );
nodeT36 = ( RGBEToLinear( ( nodeT27.tl ) ) );
nodeT37 = ( RGBEToLinear( ( nodeT27.tr ) ) );
nodeT38 = ( RGBEToLinear( ( nodeT27.bl ) ) );
nodeT39 = ( RGBEToLinear( ( nodeT27.br ) ) );
nodeT35 = ( mix( mix( nodeT36, nodeT37, nodeT27.f.x ), mix( nodeT38, nodeT39, nodeT27.f.x ), nodeT27.f.y ) );
nodeT40 = fract( nodeT25 );
nodeT29 = mix( nodeT30, nodeT35, nodeT40 );

iblIrradiance += PI * nodeT29.xyz;

Could it be vWNormal that is incorrect?

@njarraud
Copy link
Contributor Author

I finally managed to get the same result with MeshStandardMaterial and StandardNodeMaterial.
I replaced:
nodeT23 = vWNormal;
by
nodeT23 = inverseTransformDirection( geometry.normal, viewMatrix );
for the calculation of the irradiance.

This is the calculation used by the standard material system as found in the file envmap_physical_pars_fragment.glsl.js at line 10 in the getLightProbeIndirectIrradiance function:
vec3 worldNormal = inverseTransformDirection( geometry.normal, viewMatrix );

Does it mean that the normal node with the world scope shall be updated or simply that the irradiance calculation requires something else?

@WestLangley
Copy link
Collaborator

@njarraud Nice detective work!

/ping @elalish
/ping @sunag

@elalish
Copy link
Contributor

elalish commented Jan 14, 2020

@njarraud I think you're on the right track. It looks like the reflection is being passed the output of NormalNode (which is only the vertices' contribution) rather than NormalMapNode, which adds the effect of the normal map. In the core shaders I use the first for anti-aliasing and the second for building the reflection vector. That's a likely source of confusion, but I'm not familiar enough with the Node system to fix it myself. @sunag Does that make sense to you?

@sunag
Copy link
Collaborator

sunag commented Jan 15, 2020

I finally managed to get the same result with MeshStandardMaterial and StandardNodeMaterial.

@njarraud Wow, you did things get easy now... I should publish a PR in the next few days.

@sunag has created an awesome system and unlocked new possibilities...

@njarraud I don't know how to thanks for this. You are contributing a lot too.
I am very happy with this, thank you :-)

@sunag Does that make sense to you?

@elalish Yeah, It makes a lot of sense.

@sunag
Copy link
Collaborator

sunag commented Jan 15, 2020

@elalish Wherein I can find the anti-aliasing code in shader?

@elalish
Copy link
Contributor

elalish commented Jan 15, 2020

@sunag It's here

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

4 participants