Skip to content
This repository has been archived by the owner on Dec 22, 2023. It is now read-only.

Normal-Tangent Mirror Test #149

Merged
merged 3 commits into from
Mar 9, 2018
Merged

Normal-Tangent Mirror Test #149

merged 3 commits into from
Mar 9, 2018

Conversation

emackey
Copy link
Member

@emackey emackey commented Mar 2, 2018

The existing Normal-Tangent test model tests an engine's ability to automatically generate the correct tangent vectors given only a set of normal vectors and a tangent-space normal map.

This new model tests an engine's ability to use the supplied tangent vectors, and in particular includes tests for when geometry has been mirrored. Mirrored geometry relies on the supplied tangent vectors to indicate the correct direction, and if they are replaced with auto-generated vectors there will be a visible difference.

The README files for both NormalTangentTest and NormalTangentMirrorTest have been overhauled with instructions on how to perform the test and read the test results, along with screenshots of what the tests look like when they pass, and what they look like in the most common failure patterns that have been observed with these models.

@bghgary
Copy link
Contributor

bghgary commented Mar 2, 2018

Related tests in the asset generator here:
https://github.com/bghgary/glTF-Asset-Generator/tree/master/Output/Node_NegativeScale

I think we might want to explain exactly what "mirrored geometry" means. I don't see any negative scales in the glTF file for this PR, so I'm assuming the geometry was mirrored in the DCC tool?

@donmccurdy
Copy link
Contributor

Great test model, thanks!

For my own ignorance, what motivation would an artist usually have in supplying tangents? Is this done because they're not confident the engine will calculate tangents correctly? Or do supplied tangents improve visual quality... enable UV mirroring... something else?

@bghgary
Copy link
Contributor

bghgary commented Mar 2, 2018

For my own ignorance, what motivation would an artist usually have in supplying tangents? Is this done because they're not confident the engine will calculate tangents correctly? Or do supplied tangents improve visual quality... enable UV mirroring... something else?

I can think of a few reasons for supplying tangents:

  • The supplied normal map is in a specific tangent space that is not MikkTSpace.
  • Trading the cost of download size for the cost of computing tangents at runtime.
  • Not depend on an engine implementation of tangent calculation (which for the web is not very consistent yet). If the tangent space is wrong, the normal map will be rendered wrong (e.g. have seams or completely incorrect results)

@emackey
Copy link
Member Author

emackey commented Mar 2, 2018

So I created this model after reading some discussion of a problem with DamagedHelmet (somewhere... I think in KhronosGroup/glTF-Sample-Viewer#65). The helmet model has two halves, and one side is a mirror of the other side (in the DCC tool, not with a scale command). The corresponding UV maps then have only half of the pixels they would otherwise have, since the UV atlas need only cover one side of the helmet.

But there's a problem: On the "mirrored" side of the helmet, all of the normal-mapped indentations are portrayed as bumps, and bumps as indentations. It was suggested this is because the current helmet model does not supply tangent vectors, and relies on the engine to generate them. The generated tangents don't take the mirroring into account, and the +/- sense of the normal map gets flipped.

Thus, a revelation: Supplying tangent vectors can actually save space in the form of texturemap data that need not be repeated for mirrored geometry.

So I made this model to test the theory, that simply taking the old NormalTangentTest into Blender, creating mirror copies of the normal-mapped quads, would result in geometry that didn't auto-generate tangents correctly, and that Blender would supply the correct tangents. This is quickly and easily confirmed with this model (by removing the reference to the tangents in BabylonJS, or just by viewing in ThreeJS where tangents are always auto-generated and supplied tangents are ignored). It also uncovered a number of issues in Cesium, which wasn't ingesting supplied tangents correctly at all.

In any case, when exporting normal-mapped geometry that may have been mirrored, it would appear that supplying tangents is the safest thing to do.

@emackey
Copy link
Member Author

emackey commented Mar 2, 2018

Think of it this way: For mirrored geometry, it's as if the normal map calculation had taken place on the back faces of all the polygons, instead of the front faces.

@bghgary
Copy link
Contributor

bghgary commented Mar 2, 2018

Thus, a revelation: Supplying tangent vectors can actually save space in the form of texturemap data that need not be repeated for mirrored geometry.

I think the reason why removing the tangents don't work in three or Babylon is because neither implementation compute tangents use MikkTSpace algorithms as per spec. Assuming using a proper algorithm will result in correct tangents (even for mirrored geometry), I don't see why supplying tangents will result in a smaller download since the texturemap data will not need to be repeated.

@emackey
Copy link
Member Author

emackey commented Mar 2, 2018

I don't know the details of MikkTSpace calculation, but there's a more fundamental problem here, and I'm having trouble describing it. Please bear with me as I make another attempt.

supplied-tangents-ignored

In the "normal" columns above, tangents are calculated the normal way, and work as expected whether you use the supplied tangents or you calculate them in the fragment shader.

In the "mirror" columns above, two steps were taken in Blender:

  1. The mirror geometry command was used. This resulted in the vertex winding order going the other way, the new polygons facing the wrong way, etc.
  2. The "flip normals" command was used. This flipped the vertex normals back to face front, and flipped the winding order back to front-facing as well.

By the end of step 2, a single-sided material will show this polygon only from the front and cull it from the back, and the vertex normals face front too. The only remaining change here is the mapping of the UVs. Here's a screenshot of what happened to the normal map image as a result of UV mirroring:

gltf-normal-map

And there it is. The actual image of the normal map itself has been mirrored, first vertically, then horizontally. No matter how good your math is, you can't "know" that this has happened, unless you have the tangent vectors supplied. There are no clues to be found in the winding order or normal vector. If you don't know these images apply to the back face, you're going to get inverted normals, and it will look like the back faces. The supplied tangent vectors are required to fix this.

@emackey
Copy link
Member Author

emackey commented Mar 2, 2018

I think this problem is exclusive to when a normal map is "baked" using forwards geometry, and then applied to a mirrored-and-flipped version of that same geometry.

@bghgary
Copy link
Contributor

bghgary commented Mar 2, 2018

I see what you mean now. I wonder how often this kind of thing happens. I'll ask tech art.

@emackey
Copy link
Member Author

emackey commented Mar 3, 2018

Even if it turns out to be not all that common, I'll postulate that this is still a valuable addition to our test model suite, because:

  • It shows a specific feature that is available to tangent-space users that is not available to auto-tangent generators.
  • It has already cropped up "in the wild" in the form of our beloved DamagedHelmet model (which I haven't fixed yet, but that's beside the point for this PR).
  • It exposed a bug in Cesium which was subsequently fixed.
  • It will allow testing of ThreeJS if and when they decide to implement glTF-supplied tangent vectors.

@bghgary
Copy link
Contributor

bghgary commented Mar 3, 2018

Yeah, I'm not arguing that at all. I'm just curious.

@donmccurdy
Copy link
Contributor

donmccurdy commented Mar 3, 2018

Thanks for the detailed explanation, I'm also a +1 for including the sample, just trying to get a better idea of if/when three.js should consider implementing.

@cx20
Copy link
Contributor

cx20 commented Mar 3, 2018

I tried this model with gltf-test. The current status is as follows.

https://github.com/cx20/gltf-test#agi-sample-models
image

(It is not a complete test because some WebGL Engine can not display the environment map. I need to check how to use the environment map for each WebGL engine, but it seems that it will take time.)

@emackey
Copy link
Member Author

emackey commented Mar 3, 2018

Thanks for your awesome tests @cx20. Cesium will be fixed with the 1.44 release on April 2nd. CesiumGS/cesium#6302

@emackey
Copy link
Member Author

emackey commented Mar 7, 2018

/cc KhronosGroup/glTF#1252

@emackey emackey merged commit c589f37 into master Mar 9, 2018
@emackey emackey deleted the normal-tangent-mirror-test branch March 9, 2018 19:09
@donmccurdy
Copy link
Contributor

Thanks for this thread, by the way — just confirmed that adding tangents in three.js and updating to a newer version of the DamagedHelmet model fixes the seam:

without tangents with tangents
screen shot 2018-03-28 at 8 58 28 am screen shot 2018-03-28 at 8 58 52 am

Special shout-out to this sample model and SpectorJS for making the three.js implementation dramatically easier to debug. 😅

@bhouston
Copy link

This issue can be quickly fixed in the calculation of the normal map tangent space because it is possible to detected mirrored faces by comparing the specified normal with the tangent space normal.

Just update Three.JS's normalmap_pars_fragments.glsl with this code and it probably will be fixed - it is a four line fix I think:

vec3 perturbNormal2Arb( vec3 eye_pos, vec3 surf_norm ) {

		#if defined( TEXTURE_SLOTS )
			vec2 normalUv = normalMapUV();
		#else
			vec2 normalUv = vUv;
		#endif

		vec3 q0 = dFdx( eye_pos.xyz );
		vec3 q1 = dFdy( eye_pos.xyz );
		vec2 st0 = dFdx( normalUv.st );
		vec2 st1 = dFdy( normalUv.st );

		vec3 S = normalize( q0 * st1.t - q1 * st0.t );
		vec3 T = normalize( -q0 * st1.s + q1 * st0.s );		
		vec3 N = normalize( surf_norm );

		// invert S, T when the UV direction is backwards (from mirrored faces),
		// otherwise it will do the normal mapping backwards.
		vec3 NfromST = cross( S, T );
		if( dot( NfromST, N ) < 0.0 ) {
			S *= -1.0;
			T *= -1.0;
		}

		vec3 mapN = texture2D( normalMap, normalUv ).xyz * 2.0 - 1.0;
		mapN.xy = normalScale * mapN.xy;
		mat3 tsn = mat3( S, T, N );
		return normalize( tsn * mapN );

	}

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants