GLTFLoader: Normal-Tangent Test model result is incorrect

[cx20]

Description of the problem

I tried to display the Normal-Tangent Test model.
However, the displayed result seems to be different from Khronos' sample.

Three.js + Normal-Tangent Test model result:

Khronos sample loader + Normal-Tangent Test model result:

I think that this sample model should have the same left and right results.

Related : https://emackey.github.io/testing-pbr/normal-tangent-readme.html

/cc @emackey

Three.js version

• Dev
• r85
• ...

Browser

• All of them
• Chrome
• Firefox
• Internet Explorer

OS

• All of them
• Windows
• macOS
• Linux
• Android
• iOS

Hardware Requirements (graphics card, VR Device, ...)

ThinkPad X260 + Windows 10 + Intel HD Graphics 520

[emackey]

Thanks for writing this up @cx20.

Just for more context, here are some related info & issues:

• I reported a similar issue on @donmccurdy's ThreeJS glTF viewer, Normal maps don't play nice donmccurdy/three-gltf-viewer#10. But I now think this is a bug in ThreeJS's glTF loader, not in the viewer. So, this new issue is better placed than my old issue.

• In the above older issue, the model used to face the sky, but later I rotated it to face the horizon. This makes it easier to see when the reflections aren't right, because the horizon rotates around crazily, as shown above.

• The model's own use of a normal map was confirmed correct by discussion in Direction of Y (green) channel in normal maps KhronosGroup/glTF#952.

• ThreeJS's handling of normal maps was questioned in What is three.js's normal map handedness convention? #11315.

• BabylonJS recently had a similar problem, reported here, and fixed here. Here's a live demo of BabylonJS's glTF 2.0 loader with the fix applied.

[donmccurdy]

Good writeup, thanks!

The model appears to render correctly with the addition of this line:

materialParams.normalScale = new THREE.Vector2(-1, 1);

But I'm not sure I understand the issue well. Understanding #11315 might help here.

[cx20]

@donmccurdy Thanks for your advice.
I understood that improving by adjusting normalScale.
However, if glTF model is correct, I think that it is better for glTF Loader to handle it.

[donmccurdy]

@cx20 agreed, this fix is now merged into THREE.GLTF2Loader.

[cx20]

I have confirmed that it is being fixed.

Three.js + Normal-Tangent Test model result:

[elalish]

This has regressed, sometime between r101 and r104:

[donmccurdy]

See #15749 — the regression is intentional, and can be avoided by including tangents in the model.

Ideally we would have a JS implementation for generating mikktspace tangents, to fully solve this, but that is fairly complex.

[emackey]

I wasn't aware of #15749 until now. I'm caught off guard by this, I had thought we did a good enough job defining the tangents in glTF that they could be at least approximated at runtime.

Note that the Blender exporter won't export any glTF tangents by default, as it helps keep the file size down, and the major implementations of glTF were all passing this test without tangents. I suspect this change may have broken normal mapping for a majority of glTF models in ThreeJS.

I'll need some time to read through all the linked issues to get a deeper understanding of what's happened and why. But I think the glTF community should consider this high priority to get models without tangents rendering correctly again, as I believe most models are in that category by default.

[mrdoob]

Reopening 😅

[donmccurdy]

I suspect this change may have broken normal mapping for a majority of glTF models in ThreeJS.

I don't believe it's anything that severe – we've always generated tangents realtime in the shader with derivatives, and we still do. We previously included a hack (normalScale.y *= -1) that happened to fix this specific test model, but also happened to break some other examples. I have no explanation for when that helped, or didn't, so we removed the hack once we supported stored tangents – in which case it would certainly have been wrong. Now the models that relied on the hack (and don't include tangents) are broken, and models that were broken by the hack (and don't include tangents) are fixed.

But I think the glTF community should consider this high priority to get models without tangents rendering correctly again, as I believe most models are in that category by default.

See above. In general, I believe we do render models without tangents adequately. We do not, however, generate mikktspace tangents as required by the glTF spec. To my knowledge, no JS implementation of that exists, and our derivatives-based shader implementation is simply a "mostly good enough" approximation. This sample model is an intentionally extreme case that demonstrates the limits of that approximation.

We'd be glad to have a JS mikktspace tangent generation implementation; that would be good addition to THREE.BufferGeometryUtils. But the official (native) mikktspace code is fairly long, and I haven't dug in enough yet to see how much of that is required for generating tangents.

[emackey]

We previously included a hack (normalScale.y *= -1) that happened to fix this specific test model, but also happened to break some other examples

Did it break other glTF models specifically, or just examples in general?

There are two different types of tangent-space normal maps out in the wild. Substance Painter calls these "DirectX Normals" and "OpenGL Normals", which is not the greatest name for it. The difference is specifically the y channel is inverted, meaning all of the green channel values in the texture are inverted. Multiplying y *= -1 is the correct way to convert one to the other. The so-called "DirectX Normals" use a left-handed coordinate system, and glTF defines a right-handed coordinate system for normal/tangent/bitangent.

What I suspect is happening is that when ThreeJS auto-calculates tangents, it expects the normal map to have been authored with the flipped Y (DirectX) style, and gets that channel backwards for glTF, so the flip is needed. However, when the tangents are supplied, no such flip is needed.

The question of mikktspace I think is separate from this. It's unfortunate that the spec calls for mikktspace and most implementations approximate that with screen-space derivatives. I don't know how similar the two are, but, normal maps generated in mikktspace appear to work reasonably well when shown with the approximation, so long as the left/right-handedness of the map is done correctly.

(There's also some discussion of this from last year in KhronosGroup/glTF-Sample-Models#174)

[emackey]

I'd like to call out one sentence from the glTF spec on normal maps in particular:

The normal vectors use OpenGL conventions where +X is right and +Y is up. +Z points toward the viewer.

This sentence is what allows models to be shipped without tangent vectors, saving space.

Let's test it. Here I've made a lousy height map (bump map) out of a splotch in a paint program:

Let's define this height field as an outward bump, where white pixels are closer to the viewer, and black pixels are further away.

Using an online converter (of questionable quality, but we'll examine the result in a moment), I've converted this from a height map to a normal map. Keep in mind there's no 3D model here, no UV coordinates or mikktspace calculations or any geometry. Just a height map converted to a normal map. I had to manually configure the online converter per glTF's instructions, such that X is right, Y is up, and Z faces the viewer. This is the result:

Let's bring that back into a paint program and bust out the color channels to see where these vectors point. Below, each color channel has been separated into a grayscale image of its own. Remember that these will be interpreted such that black means -1.0, middle gray means 0.0, and white means +1.0.

So I think the online converter did what glTF asked, at least after configuring it correctly. In the Red (X) image, we can see the slope on the right has white pixels (+1.0), pointing the X vector at the right edge of the image. On the left side of the Red image, black pixels (-1.0) point the X vector at the left side of the image. In the Green (Y) image, white pixels along the top slope of the bump point the Y vector at the top of the image. The Z values are the least intuitive, but remember that the tip of the bump and the back plate itself both point at the viewer, and the slopes on all sides point away, so are all evenly darker.

What if we load this into Blender Eevee, which (just like glTF) accepts OpenGL-style normal maps? What happens if the UV map is rotated, or even scaled to be inverted?

Turns out, this works just fine. Indeed, the whole point of defining the tangent space this way is not to enable software to go crazy with the vectors, it's to allow texture artists some sanity by ensuring that their normal maps will be right-side up regardless of the geometry.

But, not all software uses the OpenGL convention. Some uses a different convention (sometimes called the DirectX convention), where the Y vectors point at the bottom of the image instead of the top. Here's the decomposed Y channel of my image in this form. The lighter pixels are the ones facing the bottom of the image.

If I load one of these DirectX-style normal maps into Blender Eevee, can I still expect it to work?

No. Blender was expecting +Y up. The math is wrong, and the reflected horizon line spins all around.
The same thing happens if you load an OpenGL-style normal map into an engine that was expecting +Y down.

This is what the NormalTangentTest model is attempting to test. Each row spins the UV coordinates into a different orientation, trying to make sure that the reflections remain right-side up in these different orientations.

[Themaister]

There still needs to be a concrete formula in the specification for how to compute a tangent for a lone primitive, given a primitive and its UV coordinates, and which W sign to use for bitangent. "OpenGL normals" and "DX normals" is not precise enough to derive the formula. They might refer to conventions, but I have no idea as an implementer what to do with that.

What I currently do is emit flipped TangentW from MikkTSpace to match up with this particular sample, but that was just what happened to work.

[donmccurdy]

Did it break other glTF models specifically, or just examples in general?

Reported bugs were related to glTF models, specifically. That said, I doubt there's enough confidence in material export via FBX or COLLADA to say those normal map conventions were ever thoroughly understood and tested either.

The difference is specifically the y channel is inverted, meaning all of the green channel values in the texture are inverted. Multiplying y *= -1 is the correct way to convert one to the other.

Thanks, this is a much better justification of our "hack" than we had when we implemented it. 😇

It's unfortunate that the spec calls for mikktspace and most implementations approximate that with screen-space derivatives.

The spec is right that MikkTSpace is the most robust way to generate tangents, I think, it's just not universally the right choice to do this automatically at runtime. If cheaper alternatives look correct for a particular model, there's no reason to do something more expensive that doesn't look any better. The spec language could be loosened to allow for approximations but I don't feel strongly about this.

There still needs to be a concrete formula in the specification for how to compute a tangent for a lone primitive, given a primitive and its UV coordinates, and which W sign to use for bitangent.

I'm not sure the MikkTSpace algorithm is so easy to represent as a discrete formula... are you asking for an alternative to the canonical MikkTSpace code? Or some additional information beyond the instruction to use MikkTSpace? @Themaister

---

For the original issue, it sounds like we ought to restore the normal.y *= -1 multiplier somewhere. There are three possible places to do this:

• (a) in GLTFLoader, for meshes that do not have tangents
• (b) in GLTFLoader, for all meshes
• (c) in WebGLRenderer, for all meshes

If threejs is really using the DirectX convention and e.g. Blender is not, I could see a case for (c). In the interest of a quick and safe solution, though, I am inclined to go with (a).

[WestLangley]

What I suspect is happening is that when ThreeJS auto-calculates tangents, it expects the normal map to have been authored with the flipped Y (DirectX) style.

No, three.js does not assume that...

three.js assumes +Y is "up" for tangent space, and increasing-V is "up" for uv space.

That is, three.js assumes uv ( 0, 0 ) is in the lower-left corner of the texture, while the glTF spec assumes the upper-left. This is why GLTFLoader sets texture.flipY to false. (three.js sets texture.flipY to true by default.)

When tangents are not present, three.js uses screen-space derivatives to estimate tangents. It does so using the chain rule. An assumption in that calculation is that tangent space and uv space have the same orientation.

For properly-authored glTF models, that assumption is not correct. However, you should be able to compensate by setting normalScale.y = - 1 for any model that properly adheres to the glTF spec.

It would also seem to me we could fix this automatically by honoring the flipY flag in the shader.

If setting normalScale.y is not working, then something else is going on.

[emackey]

Thanks for this clarification. I think we have a path forward here.

It would also seem to me we could fix this automatically by honoring the flipY flag in the shader.

Yes, with the exception of cases where the glTF supplies its own tangent vectors, right? I would expect only the auto-computed tangents to need the flipY test and corresponding negation of y.

If that's not the case... that would mean that my NormalTangentMirrorTest model has incorrect tangents encoded into it, meaning the Blender glTF exporter itself is putting the wrong tangents into glTF models. Blender is using a lower-left origin, and off the top of my head I don't know if that means the tangent vectors themselves must be edited or not before copying into the model (and whether the exporter performs such edits).

Edit: I believe I've confirmed the correctness of the exported tangent vectors, and they do not need any Y flipping. I can post more detail on that if needed.

But it seems like the correct action is to flip normalScale.y only when auto-generated tangents (not supplied tangents) are being used with the flipY flag. Thoughts?

[WestLangley]

In my previous post, I explained how to manually compensate for inverted normals when attribute tangents are not present. There should be nothing to compensate for when tangents are present because screen-space derivatives are not being used.

I also suggested we may be able to fix this automatically by honoring the flipY flag in the shader. I did not say we would fix this automatically be flipping normalScale.y, however. I do not think we should alter the user's settings.

In any event, before we go down that path, I think it is imperative that we verify this hypothesis:

[Y]ou should be able to compensate by setting normalScale.y = - 1 for any model that properly adheres to the glTF spec.

We must have an explanation for every model that is not being rendered correctly.

[emackey]

I do not think we should alter the user's settings.

Yes, apologies, I did not intend to be dictating that sort of implementation detail. I'm just trying to make sure there's not a misunderstanding of what honoring the flipY flag means mathematically, for the purpose of screen-space tangent generation.

We must have an explanation for every model that is not being rendered correctly.

That sounds like that could potentially be a large set. If there are indeed models out there that are doing something radically different than the official test models, and yet could still be considered valid uses of normal maps in glTF, that would be important to discover. The test models are intended to cover valid uses of normal maps on static (non-transformed) geometry. There shouldn't be a way, particularly when relying on viewer-generated tangent vectors, to construct the model in some other coordinate system and yet claim that it's valid glTF.

[elalish]

I assume this is closely related to this issue? KhronosGroup/glTF-Sample-Models#174

[elalish]

@mrdoob @Mugen87 @donmccurdy This issue has resurfaced in r125, I would bet due to #21076. I still think the benefits of that change far outweigh this regression, but it would be nice if we could fix it all.

[donmccurdy]

It would be interesting to know if #21076 is really the cause. Have you tested after the corrections for material -> cachedMaterial?

I'll comment further in #20997 (comment) – tl;dr i think we should probably revert #21076 for several reasons, but am not sure yet what else should be done for the various issues.

[Mugen87]

Instead of reverting #21076 I vote to just remove the following code section in GLTFLoader if no other solution is found:

three.js/examples/js/loaders/GLTFLoader.js

Lines 3242 to 3255 in a159495

	// Fix double sided rendered models on certain mobile devices, see https://github.com/mrdoob/three.js/issues/20997#issuecomment-756082184
	if ( material.isMeshStandardMaterial === true &&
	material.side === THREE.DoubleSide &&
	geometry.getIndex() !== null &&
	geometry.hasAttribute( 'position' ) === true &&
	geometry.hasAttribute( 'normal' ) === true &&
	geometry.hasAttribute( 'uv' ) === true &&
	geometry.hasAttribute( 'tangent' ) === false ) {
	geometry.computeTangents();
	material.vertexTangents = true;
	}

Having computeTangents() in BufferGeometry is something I would like to keep.

[Mugen87]

What is actually broken? The current screenshots (made in the editor) look like so:

[cx20]

There are two similar samples in the Khronos glTF Sample model:
https://github.com/KhronosGroup/glTF-Sample-Models/blob/master/2.0/NormalTangentTest/README.md
https://github.com/KhronosGroup/glTF-Sample-Models/blob/master/2.0/NormalTangentMirrorTest/README.md
Of these, it seems that the NormalTangentMirrorTest is causing degradation.

Three.js r124 + NormalTangentMirrorTest.gltf result:

Three.js r125 + NormalTangentMirrorTest.gltf result:

[Mugen87]

@cx20 It seems the r125 version is not based on the latest patch level. Can you please try it with r125.2? It seems this PR #21168 is missing.

[cx20]

@Mugen87 Thanks. I updated from r125 to r125.2 as you suggested and confirmed that the NormalTangentMirrorTest is resolved.

Three.js r125.2 + NormalTangentMirrorTest.gltf result:

[Mugen87]

That's great! Can you please double check NormalTangentTest with this release? For some reasons, it seems this asset is now broken again when validating in the editor^^.

[cx20]

@Mugen87 I checked NormalTangentTest again with r125.2. Certainly the problem will be reproduced.

Three.js r124 + NormalTangentTest.gltf result:

Three.js r125.2 + NormalTangentTest.gltf result:

[WestLangley]

@cx20 Would you be willing to fix the Normal Tangent Test demo?

The normal map has tangents largely parallel to the plane and reflections are unreasonable. (This is your model in my testbed.)

You will need to "flatten" the hemispheres and bake new normal maps.

I achieve the same effect by hacking the loader callback:

mesh.scale.z = 0.25;
mesh.material.normalScale.multiplyScalar( 0.25 );

In your demo, it also helps if your environment map doesn't look practically the same in every direction. Maybe you can use a different one.

There is also no need for a spinning scene. OrbitControls alone is sufficient.

Also, it would be helpful to fix the near-plane clipping so the scene is visible when zooming.

Thanks.

[emackey]

@WestLangley That's an intentional effect. The model's README mentions that the test is only valid when viewed face-on. It's intended to test the broadest range of normal vectors, not viewing angles.

[WestLangley]

@emackey Thank you for your reply.

Without changes, however, I would have to remind the three.js devs to be suspect when using that model to test three.js code.

[mrdoob]

After #21202, #21203 and #21205 this should now be fixed.

[cx20]

@WestLangley Thank you for the improvement plan.
If you have any problems specific to gltf-test, please post them at this URL: https://github.com/cx20/gltf-test/issues
I just fixed the clipping issue.