GLTFLoader: Invert normalScale.y, not normalScale.x.

[donmccurdy]

Followup from #13716. Can't say I have great confidence in why this is correct, but if we're planning to release r92 soon, this should probably be patched...

example	image
boombox_invertx
boombox_none
boombox_inverty
normaltangenttest_invertx
normaltangenttest_none
normaltangenttest_inverty
normaltangentmirrortest_invertx
normaltangentmirrortest_none
normaltangentmirrortest_inverty

[WestLangley]

I would not flip the normal scale in the loader. The proper value for normal scale depends on the tangent space coordinate system assumed when the normal map was authored. It is the user's responsibility to set it.

[mrdoob]

@WestLangley GLTF is supposed to hide all that. I think it's our responsibility to make sure the GLTF model displays as it should (and as it does in other engines and applications).

For reference: https://github.com/vorg/pbr-compare

[mrdoob]

Thanks!

[WestLangley]

Please allow me to rephrase my statement, then.

If I use GLTFExporter to export a model with a normal map, and then use GLTFLoaderto load it, I don't get what I started with because the normal scale is now inverted.

[WestLangley]

According to the spec:

https://github.com/KhronosGroup/glTF/tree/master/specification/2.0#materialnormaltexture

A tangent space normal map. The texture contains RGB components in linear space. Each texel represents the XYZ components of a normal vector in tangent space. Red [0 to 255] maps to X [-1 to 1]. Green [0 to 255] maps to Y [-1 to 1]. Blue [128 to 255] maps to Z [1/255 to 1]. The normal vectors use OpenGL conventions where +X is right and +Y is up. +Z points toward the viewer. In GLSL, this vector would be unpacked like so: vec3 normalVector = tex2D(normalMap, texCoord) * 2 - 1.

glTF adheres to the OpenGL convention. So does three.js. Therefore, there should be no flipping required.

If there IS flipping required, then the normal map used in the model was authored using a different convention, such as the DirectX convention. But according to the spec, other conventions are not supported by glTF.

This is why the normal map scale factor in glTF (normalTextureInfo.scale) is only a scalar -- not a 2-component vector.

See the spec:

https://github.com/KhronosGroup/glTF/tree/master/specification/2.0#normaltextureinfoscale

[donmccurdy]

If there IS flipping required, then the normal map used in the model was authored using a different convention, such as the DirectX convention. But according to the spec, other conventions are not supported by glTF.

If the samples are using another convention, they are badly inconsistent with the spec, yes.

But I am fairly confident these models are following the OpenGL convention. You can compare NormalTangentTest and NormalTangentMirrorTest models in various engines here, and see that BabylonJS, CesiumJS, ClayGL, the Khronos sample renderer, and others are rendering them correctly. If three.js is following the OpenGL convention, then it seems more likely there is another bug in our implementation being hacked around with this flipping.

/cc @emackey

[emackey]

The NormalTangentTest models were authored with the OpenGL convention. You can see in the normal map image that it looks as if a green light comes from above, not below. Locally I do have DirectX versions of these maps for testing, and they have the look of green light coming from below.

[emackey]

If I use GLTFExporter to export a model with a normal map, and then use GLTFLoader to load it, I don't get what I started with because the normal scale is now inverted.

Can someone export a model with a convex (outward) bump, for comparison?

[WestLangley]

For reference, there is an OpenGL-style normal map texture available here.

[emackey]

@WestLangley Thanks, actually I meant is there any way to export one from ThreeJS, to understand how or why a flip happens during a round-trip export/import? We're very confident that the sample model uses OpenGL maps, and that is further confirmed by the reference link you just provided.

I don't think Don or I have ever understood why the ThreeJS glTF loader needs this manual Y flip in order to be able to load an OpenGL-style normal map. I'm not a ThreeJS expert, but I do have a stand-alone sample of ThreeJS loading an OpenGL normal map without a flip. Yet inside the glTF loader, it seems we always need to specify a Y flip, to be compatible with the very same map.

[donmccurdy]

About GLTFExporter, @WestLangley is very likely correct that round-trip will get this wrong, but I haven't tested with any clear cases like NormalTangentTest yet. But let's address the exporter once we are satisfied with what GLTFLoader is doing.

Yet inside the glTF loader, it seems we always need to specify a Y flip, to be compatible with the very same map.

Flipping normalScale.y is new, actually — prior to a recent change we were flipping normalScale.x instead. 😕 See #13716 for discussion. But agreed, I have never understood why this was needed.

I don't think it's related but as a sanity check, note that we also set .flipY = false on all textures in GLTFLoader.

[WestLangley]

I don't think it's related but as a sanity check, note that we also set .flipY = false on all textures in GLTFLoader.

Oh. Well, that is definitely an issue. .flipY is by default true for us, so the first byte pushed to the GPU is the lower left, not the upper left.

So that begs the question: "How could any of your glTF models have rendered correctly?" Your uv's must be flipped, too.

And sure enough, the spec says uv ( 0, 0 ) is the upper-left.

https://github.com/KhronosGroup/glTF/tree/master/specification/2.0#images

In three.js, uv ( 0, 0 ) is the lower-left -- or maybe it is more correct to say the uv ( 0, 0 ) corresponds to the first byte in the data buffer.

So, you can flip all your uvs, too. I expect things will start to make more sense, then...

Now, I expect you don't want to do that, so I think you can leave things as they are.

To compensate, you will have to flip normalScale.y when you load the model. When you export the model, you flip it back, so it is a positive value.

[donmccurdy]

Ok thanks — that makes sense. I'll add clearer comments in GLTFLoader.js and fix GLTFExporter for this case then.

[WestLangley]

@donmccurdy I think this and this are the reasons, then, that the loader must flip (only) normalScale.y in the loader.

I think the Loader comments should be corrected, and the code can simply be

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

if ( materialDef.normalTexture.scale !== undefined ) {

	materialParams.normalScale.multiplyScalar( materialDef.normalTexture.scale );

}

Also, the Exporter simplified:

if ( material.normalScale.x !== 1 ) {

	gltfMaterial.normalTexture.scale = material.normalScale.x;

}

Then, on export, you need to also know if the uv-convention in the model is three.js-based or glTF-based. If the former, then you have a problem due to the inflexibility of the glTF spec.

In any event, this highlights the importance of both import-export and export-import test cases.

[emackey]

If the former, then you have a problem due to the inflexibility of the glTF spec.

My hope is that this won't be such a problem. exportedV = 1.0 - v can convert lower-left based UV coordinates to upper-left, and has the consequence of flipping the handedness of the UVs, such that setting normalScale.y to -1 on reload from glTF is expected.