NME: fix wrong perturbed normals when using pre-existing tangents

[Popov72]

Fix #13182

Note that regarding tangents in the bump context we don't take into account the w component of the vector.

In the regular bump code, we use this component as a factor to negate the bitangent (the w component is normally either 1 or -1):

From bumpVertex.fx:

#if defined(BUMP) || defined(PARALLAX) || defined(CLEARCOAT_BUMP) || defined(ANISOTROPIC)
	#if defined(TANGENT) && defined(NORMAL)
		vec3 tbnNormal = normalize(normalUpdated);
		vec3 tbnTangent = normalize(tangentUpdated.xyz);
		vec3 tbnBitangent = cross(tbnNormal, tbnTangent) * tangentUpdated.w;
		vTBN = mat3(finalWorld) * mat3(tbnTangent, tbnBitangent, tbnNormal);
	#endif
#endif

We don't do it in the NME: should we? If yes, we need to discuss it, I don't see a way to do it without a breaking change...

[azure-pipelines]

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

[azure-pipelines]

Snapshot stored with reference name:
refs/pull/13186/merge

Test environment:
https://babylonsnapshots.z22.web.core.windows.net/refs/pull/13186/merge/index.html

To test a playground add it to the URL, for example:

https://babylonsnapshots.z22.web.core.windows.net/refs/pull/13186/merge/index.html#WGZLGJ#4600

Links to test babylon tools with this snapshot:

https://playground.babylonjs.com/?snapshot=refs/pull/13186/merge
https://sandbox.babylonjs.com/?snapshot=refs/pull/13186/merge
https://gui.babylonjs.com/?snapshot=refs/pull/13186/merge
https://nme.babylonjs.com/?snapshot=refs/pull/13186/merge

To test the snapshot in the playground with a playground ID add it after the snapshot query string:

https://playground.babylonjs.com/?snapshot=refs/pull/13186/merge#BCU1XR#0

[sebavan]

Should this not also apply to anisotropy and clear coat block ???

[sebavan]

Cool thanks a lot, I am not sure to understand the impact of the breaking change you are mentioning ?

Isnt it more like a bug fix as the materials won t be broken, just lit properly ?

[Popov72]

It's not a bug fix if tangent.w=+1 for all vertices, so for a number of people what we currently do is ok: I don't know if tangent.w=-1 is used a lot or not in models...

The breaking change I'm speaking about is in the implementation on our side: the existing node materials using perturbed normals + pre-existing tangents would break if we implement the change. The users will have to fix their node materials to account for the change. At least it's what I think would happen but I didn't think about it too thoroughly, so maybe we should discuss about that issue.

In any case, I think it's a different issue and we can merge this PR if it is ok.

[sebavan]

Makes sense !!! Let s chat on wednesday