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

WebGPU: Fix FrontFace #23965

Merged
merged 1 commit into from
Apr 29, 2022
Merged

WebGPU: Fix FrontFace #23965

merged 1 commit into from
Apr 29, 2022

Conversation

sunag
Copy link
Collaborator

@sunag sunag commented Apr 29, 2022

Description

In some comparative tests, I notice that built-in front_facing, it is NormalMap was inverted because of the direction of frontFace.

This contribution is funded by Google via Igalia

@sunag sunag added this to the r140 milestone Apr 29, 2022
@Mugen87 Mugen87 merged commit c654ba7 into mrdoob:dev Apr 29, 2022
@sunag sunag deleted the dev-fix-frontFace branch April 29, 2022 14:53
@WestLangley
Copy link
Collaborator

Oh, no.

Front faces have counter-clockwise winding order in three.js. With this PR, when material.side is front-sided, then so-called "front faces" are culled.

We can't do this. This is going to cause incredible confusion as we try to get normal mapping correct inside the shader.

Furthermore, back-faces with normal maps are still not correct.

Normal mapping has to be corrected by other methods -- via normalScale, and WebGPU-specific shader mods.

@sunag
Copy link
Collaborator Author

sunag commented Aug 1, 2022

I can't see any visual difference after this PR between WebGL and WebGPU referring to normal direction.

This is the test of AB:

WebGPU WebGL Side
image image Front
image image Back

WebGPU - A
WebGL - B

The idea is not to use hacks or shaders mods, I think this should be an approach to be abolished if possible.
NodeMaterial should work on any backend using the same shader graph.

About the PR, do you think this might be related to the NDC?

@WestLangley
Copy link
Collaborator

I think there will be issues related to the different viewport transform in WebGPU.

WebGPU has different pipeline conventions than WebGL, so I expect the normal/bumpmap shader code may have to be modified.

Can you test viewing a back face in WebGPU when material.side = THREE.BackSide and when material.side = THREE.DoubleSide. The back faces should appear the same in those two test cases.

Also, AFAIK, it has not been specified whether WebGPU will support the DirectX normal map convention. I assume that will be the case. three.js uses the OpenGL convention.

abernier pushed a commit to abernier/three.js that referenced this pull request Sep 16, 2022
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

Successfully merging this pull request may close these issues.

3 participants