Add inverted normals when flipped model scale

[nonunknown]

Solves #29317

When the user scales a 3d model by -1, the model lighting gets messed, thanks to @lyuma I could solve this problem!
In the image examples below, its a multimeshinstance with half side of the kart flipped to another!

PS: Must be cherrypicked for 3.x

Before:

After:

[Calinou]

This may have a performance cost for complex meshes, so it should probably only be done if Ensure Correct Normals is enabled in the SpatialMaterial.

[clayjohn]

Can you confirm that this fix works when cull mode is set to cull front or cull back and not just cull disabled?

[nonunknown]

@Calinou I was thinking, instead of checking add it as a option in parameter called: "Invert Normals"

@clayjohn Will test and return

[Calinou]

I was thinking, instead of checking add it as a option in parameter called: "Invert Normals"

I don't think we need such a property. The goal is to have something that works automatically, without requiring the user to adjust the material by hand if they flip the MeshInstance3D node.

[Ansraer]

Might just be me, but I think I can still see the seam. Imo the material still behaves differently on the two sides. You will probably need to also flip some other properties like tangents and bitangents.

[nonunknown]

@Ansraer you're right, @lyuma and me was trying to find a solution for that, I've tried tweaking a lot of parameters:

NORMAL = -NORMAL;
BINORMAL = -BINORMAL;
TANGENT = -TANGENT;

but none of the two last ones changed anything sadly, this is just a partial solution to the problem!

[lyuma]

I was writing a long answer for this, but I reached the conclusion that three different answers are reasonable here.

0. Do nothing and leave these cases broken.

1. Use glFrontFace(GL_CCW) instead of flipping front face culling. I have looked into it, and this approach is actually commonly used: Both three.js and Unity use this approach for negatively scaled objects. This is not what is in this PR... but do note something: both Unity and Three.js have the disadvantage that they have existing custom shaders which they want to retain compatibility with: perhaps that is one reason that they opted for an approach which did not require editing the shader.

glFrontFace has the advantage that the shader doesn't need to change as it sees the correct value for glFrontFace. But it has the disadvantage of not allowing a single MultiMeshInstance to contain both types of meshes as a state change would be necessary.
(Note that negatively scaled meshes in MultiMeshInstance without cull_disabled can't be fixed without a state change, afaik, but that's a different issue... since the gpu will discard polygons due to winding order)

2. This PR: Use the sign of determinant in the vertex shader to flip NORMAL.
   (Side note: From my understanding, flipping TANGENT or BINORMAL is not desirable. The reason we needed it is actually a consequence of flipping all 3 axes instead of just 1, but that's a shading issue, not something godot can feasibly fix automatically, afaik. You really shouldn't be lining up inverted meshes side by side and expect them to just work without correcting UVs)

3. Add a uniform, or repurpose the sign bit of some existing uniform indicating that the mesh is negative scale (to save a determinant call). However, I think this is a mistake, and the overhead of a single 3x3 determinant is not sufficiently high to warrant adding more hidden complexity here (and definitely not enough to warrant adding an extra uniform).

Here's a strong argument in favor of fixing negative scale: these combinations are likely supported and specified in glTF 2.0. For this reason, fixing this issue might be necessary to make sure that glTF files with negative scale work. Here is a testcase for negative scale:
https://github.com/KhronosGroup/glTF-Asset-Generator/blob/master/Output/Positive/Node_NegativeScale/README.md
We should possibly repurpose one of those testcases to also include cull_front and cull_disabled objects, since those can exist in glTF.

For reference, here is the glTF issue where negative scale is brought up:
KhronosGroup/glTF#1697

In fact, donmccurdy makes a good comment here summarizing the issue with the multimesh usecase (EXT_mesh_gpu_instancing glTF 2.0 extension) and why it requires splitting them:
KhronosGroup/glTF#1691 (comment)

[lyuma]

Oh, I omitted an important point and part of my explanation.

This PR must be changed to wrap the determinant with

		code += "#if defined(DO_SIDE_CHECK)"
		code += "	if (determinant(mat3(WORLD_MATRIX)) < 0.0) {";
		code += "		NORMAL = -NORMAL;";
		code += "	}";
		code += "#endif"

Also, your code has a mistake that it was added inside the "triplanar" case. That is wrong: it should be done regardless of triplanar.

So, let's step back a bit and understand why the normal needs to be flipped in the first place. Well, the answer is actually, quite simply, that it does not need to be flipped. In fact, this whole PR is trying to undo a normal flip which happens in the fragment program. So why does it need that macro check? It's because that Godot's scene shader does the following in the fragment main function:

#if defined(DO_SIDE_CHECK)
        if (!gl_FrontFacing) {
                normal = -normal;
        }
#endif

Let's go back and see which cases are currently broken in MeshInstance:

cull_back + negative scale: Works perfectly, because no change was made to the normal due to DO_SIDE_CHECK being unset.

cull_front + negative scale: Breaks. It works differently than cull_back because of this code (gles3):

./shader_compiler_gles3.cpp:    actions[VS::SHADER_SPATIAL].render_mode_defines["cull_front"] = "#define DO_SIDE_CHECK\n";
./shader_compiler_gles3.cpp:    actions[VS::SHADER_SPATIAL].render_mode_defines["cull_disabled"] = "#define DO_SIDE_CHECK\n";

cull_disabled + negative scale: Breaks, because gl_IsFrontFace is backwards.

So yeah, we only want to be fixing cull_front and cull_disabled here. This also explains why the check can be more obscure and hidden behind the "Ensure Correct Normals" setting as noted by @Calinou .... though I wish Godot would strive to be correct first and use settings for performance optimization, rather than the other way around.

So I would suggest actually adding it inside of the current ENSURE_CORRECT_NORMALS case which has the inverse.

[fire]

There were some unfinished review comments. Can this be updated? It's great!