Add example to test normals and tangents (and fix WebGL) #5924

erikdubbelboer · 2023-12-29T14:57:01Z

This fixes normals and tangents in the WebGL2 shader. WebGPU is still broken, see: #5735

See also https://github.com/KhronosGroup/glTF-Sample-Models/tree/master/2.0/NormalTangentTest

I confirm I have read the contributing guidelines and signed the Contributor License Agreement.

examples/src/examples/graphics/normals-and-tangents.mjs

erikdubbelboer · 2023-12-29T14:58:28Z

I tried to find a way to fix these issues but I'm not sure where to start. Should this be fixed in the GLB parser or in somewhere else? If anyone can point me in the right direction I can try and fix the issues in this pull request as well.

slimbuck · 2024-01-03T09:18:15Z

Oh that's interesting. I never realised our lighting basis is wrong on the back faces. (We were aware of WebGPU issues though). Thanks for pointing this out!

erikdubbelboer · 2024-01-03T12:25:10Z

@slimbuck if you point me in the right direction, I can try and fix it. Are the normals and tangents calculated at a specific place? I couldn't easily find it as I'm not familiar enough with the source yet.

slimbuck · 2024-01-03T12:43:48Z

I would have expected backfaces to be handled by this bit of code here: https://github.com/playcanvas/engine/blob/main/src/scene/shader-lib/programs/lit-shader.js#L989

The actual tangents and binormals are calculated in this shader chunk:
https://github.com/playcanvas/engine/blob/main/src/scene/shader-lib/chunks/lit/frag/TBNderivative.js

erikdubbelboer · 2024-01-07T06:45:30Z

I have fixed the calculations for both WebGL and WebGPU. I have also fixed the WebGPU version looking so weird, this was caused by setting mipmaps: true on the envAtlas.

slimbuck · 2024-01-08T11:03:10Z

src/scene/shader-lib/chunks/lit/frag/TBNderivative.js

+    #ifdef TWO_SIDED_LIGHTING
+        #ifdef WEBGPU
+            dTBN = mat3(T * invmax, -B * invmax, gl_FrontFacing ? -normal : normal);
+        #else
+            dTBN = mat3(T * invmax, -B * invmax, gl_FrontFacing ? normal : -normal);
+        #endif
+    #else
+        #ifdef WEBGPU
+            dTBN = mat3(T * invmax, -B * invmax, -normal);
+        #else
+            dTBN = mat3(T * invmax, -B * invmax, normal);
+        #endif
+    #endif


Webgpu lighting space should not be different to webgl, so this normal negation is terribly confusing.

Are textures inverted (upside down) on webgpu vs webgl @mvaligursky? (If so then tangents and binormals as calculated would be inverted).

I think we need to get to the bottom of why normals needs flipping here and fix it at the source. My suspicion is we have an inverted transform somewhere else in the graphics pipeline under webgpu.

I agree, I also noticed that my solution causes other problems with the lighting. I haven't been able to find a good solution. If at any other place I modify the normals it always messes up other things. My guess right now is that it's not the normals being flipped but the result in dTBN being used incorrectly somewhere.

For now I have removed this code. I'm not sure if you want to merge this as a fix to the WebGL backfaces and to introduce an example that shows what is wrong in the WebGPU pipeline.

Actually I just noticed, looking in lit-shader.js, it seems we are already attempting to handle the normal for double-sided lighting case at https://github.com/playcanvas/engine/blob/main/src/scene/shader-lib/programs/lit-shader.js#L989. This calc must have a 🐛...

You are right, it seems the issue is that it was handled in the wrong place. I have added a negatively scaled mesh to the example and I have gone though all variations of the shader to maker sure they all work correctly now.

mvaligursky · 2024-01-22T10:52:16Z

I'd be definitely great to have this example as a test for normals / tangents.

src/scene/shader-lib/chunks/lit/frag/TBNfast.js

erikdubbelboer · 2024-02-24T04:04:07Z

I have rebased the example on main with the new examples refactor. What needs to happen to get this merged now?

willeastcott · 2024-02-24T07:49:11Z

Thanks for doing that, @erikdubbelboer - we'll take another look ASAP! 😄

mvaligursky · 2024-02-26T09:55:36Z

examples/src/examples/graphics/normals-and-tangents/config.mjs

+ * @type {import('../../../../types.mjs').ExampleConfig}
+ */
+export default {
+    WEBGPU_ENABLED: true,


Suggested change

WEBGPU_ENABLED: true,

HIDDEN: true,

WEBGPU_ENABLED: true

I'm suggesting to hide it from public .. this is more of an internal example for us to validate how things work, and not an example to show the public how to use the engine.

mvaligursky · 2024-02-26T09:59:37Z

It seems the description only shows 'before' state, but not 'after' state - could you please update it.

slimbuck · 2024-03-09T21:30:28Z

Hi @erikdubbelboer,

Looking at the changes, I think it makes sense to handle twoSidedLighting in just one place - after dTBN has been calculated.

This is probably best done in a new shader chunk. So I went ahead and implemented this approach in main...slimbuck:erik-normals-and-tangents.

Could you take a look and let me know what you think? This approach also means we probably fix the case where TBNObjectSpace.js is used.

If you're happy with this I can push directly to your branch?

Thanks!

This shows that normals and tangents are not used correctly in both WebGL2 and WebGPU.

erikdubbelboer · 2024-04-02T08:34:30Z

@slimbuck great, I have merged your changes.

@mvaligursky I think this pull is ready now.

mvaligursky

This is much simpler now, thanks!

erikdubbelboer commented Dec 29, 2023

View reviewed changes

examples/src/examples/graphics/normals-and-tangents.mjs Outdated Show resolved Hide resolved

willeastcott requested review from slimbuck and mvaligursky January 7, 2024 09:59

willeastcott added bug area: graphics Graphics related issue area: examples labels Jan 7, 2024

slimbuck reviewed Jan 8, 2024

View reviewed changes

mvaligursky mentioned this pull request Jan 19, 2024

WebGPU has incorrect tangents #5735

Open

erikdubbelboer force-pushed the normals-and-tangents branch 2 times, most recently from 2aa2bbd to b1b8c14 Compare January 22, 2024 02:26

erikdubbelboer force-pushed the normals-and-tangents branch from b1b8c14 to 96dddd5 Compare January 27, 2024 05:51

slimbuck reviewed Jan 29, 2024

View reviewed changes

src/scene/shader-lib/chunks/lit/frag/TBNfast.js Outdated Show resolved Hide resolved

erikdubbelboer force-pushed the normals-and-tangents branch from 96dddd5 to e65d942 Compare February 11, 2024 06:19

erikdubbelboer force-pushed the normals-and-tangents branch from e65d942 to 5f7b0bf Compare February 24, 2024 03:55

mvaligursky reviewed Feb 26, 2024

View reviewed changes

erikdubbelboer added 5 commits March 31, 2024 11:48

Add example to show and test normals and tangents

2a8c339

This shows that normals and tangents are not used correctly in both WebGL2 and WebGPU.

Fix backfaces in both WebGL and WebGPU

2e25965

Remove bad WebGPU fix

90bacdd

Move attribution to separate file

af12ff4

Fix models with negative scale

f20f445

erikdubbelboer and others added 6 commits March 31, 2024 11:48

Fix suggestion

a793cd2

Rebased on master

10c1174

add chunk

15a55bd

small

61d074d

revert change

5a7d583

revert change

8aefda5

erikdubbelboer force-pushed the normals-and-tangents branch from 76b87fd to aa3707e Compare March 31, 2024 09:51

erikdubbelboer changed the title ~~Add example to show and test normals and tangents~~ Add example to test normals and tangents (and fix WebGL) Mar 31, 2024

Hide it until WebGPU is fixed

b711709

erikdubbelboer force-pushed the normals-and-tangents branch from aa3707e to b711709 Compare March 31, 2024 10:03

mvaligursky requested a review from slimbuck April 2, 2024 09:50

slimbuck approved these changes Apr 2, 2024

View reviewed changes

mvaligursky approved these changes Apr 2, 2024

View reviewed changes

mvaligursky merged commit f065c8f into playcanvas:main Apr 2, 2024
7 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add example to test normals and tangents (and fix WebGL) #5924

Add example to test normals and tangents (and fix WebGL) #5924

erikdubbelboer commented Dec 29, 2023 •

edited

Loading

erikdubbelboer commented Dec 29, 2023

slimbuck commented Jan 3, 2024

erikdubbelboer commented Jan 3, 2024

slimbuck commented Jan 3, 2024

erikdubbelboer commented Jan 7, 2024

slimbuck Jan 8, 2024

erikdubbelboer Jan 22, 2024

slimbuck Jan 22, 2024

erikdubbelboer Jan 27, 2024

mvaligursky commented Jan 22, 2024

erikdubbelboer commented Feb 24, 2024

willeastcott commented Feb 24, 2024

mvaligursky Feb 26, 2024

mvaligursky Feb 26, 2024

mvaligursky commented Feb 26, 2024

slimbuck commented Mar 9, 2024

erikdubbelboer commented Apr 2, 2024

mvaligursky left a comment

Add example to test normals and tangents (and fix WebGL) #5924

Add example to test normals and tangents (and fix WebGL) #5924

Conversation

erikdubbelboer commented Dec 29, 2023 • edited Loading

erikdubbelboer commented Dec 29, 2023

slimbuck commented Jan 3, 2024

erikdubbelboer commented Jan 3, 2024

slimbuck commented Jan 3, 2024

erikdubbelboer commented Jan 7, 2024

slimbuck Jan 8, 2024

Choose a reason for hiding this comment

erikdubbelboer Jan 22, 2024

Choose a reason for hiding this comment

slimbuck Jan 22, 2024

Choose a reason for hiding this comment

erikdubbelboer Jan 27, 2024

Choose a reason for hiding this comment

mvaligursky commented Jan 22, 2024

erikdubbelboer commented Feb 24, 2024

willeastcott commented Feb 24, 2024

mvaligursky Feb 26, 2024

Choose a reason for hiding this comment

mvaligursky Feb 26, 2024

Choose a reason for hiding this comment

mvaligursky commented Feb 26, 2024

slimbuck commented Mar 9, 2024

erikdubbelboer commented Apr 2, 2024

mvaligursky left a comment

Choose a reason for hiding this comment

erikdubbelboer commented Dec 29, 2023 •

edited

Loading