[Merged by Bors] - Support for normal maps including from glTF models

[superdump]

Objective

• Support tangent vertex attributes, and normal maps
• Support loading these from glTF models

Solution

• Make two pipelines in both the shadow and pbr passes, one for without normal maps, one for with normal maps
• Select the correct pipeline to bind based on the presence of the normal map texture
• Share the vertex attribute layout between shadow and pbr passes
• Refactored pbr.wgsl to share a bunch of common code between the normal map and non-normal map entry points. I tried to do this in a way that will allow custom shader reuse.

[superdump]

Oh, I also noticed while refactoring that the shadow calculations were using the world normal from the interpolated vertex attribute and they should have been using the 'N' value I guess. So if there are normal maps, the shadow calculations will now use that normal. EDIT: This was incorrect. The normal is being used for depth biasing for shadow map comparisons so it makes sense to use the plain world normal, not the normal-mapped one.

[CptPotato]

Oh, I also noticed while refactoring that the shadow calculations were using the world normal from the interpolated vertex attribute and they should have been using the 'N' value I guess. So if there are normal maps, the shadow calculations will now use that normal.

It seems like the normal in fetch_directional_shadow() is used only for the normal bias, right? In that case I think using the interpolated one from the vertex attribute would cause less artifacts (for example, a "steep" angle in the normal map would throw off the bias).
Though, using the normal-mapped vector does have an interesting side effect of emphasizing the surface's structure a bit at the penumbra (but it doesn't look quite "correct").

[superdump]

Oh, I also noticed while refactoring that the shadow calculations were using the world normal from the interpolated vertex attribute and they should have been using the 'N' value I guess. So if there are normal maps, the shadow calculations will now use that normal.

It seems like the normal in fetch_directional_shadow() is used only for the normal bias, right? In that case I think using the interpolated one from the vertex attribute would cause less artifacts (for example, a "steep" angle in the normal map would throw off the bias).
Though, using the normal-mapped vector does have an interesting side effect of emphasizing the surface's structure a bit at the penumbra (but it doesn't look quite "correct").

Good points!

[superdump]

I need to select the pipeline based on the presence of the tangents in vertex attributes, and add a bit flag for the presence of the normal map texture, in case the vertex attribute is present but the texture is not, so stuff doesn't just break. We'll need to think about how to handle 'error' cases - should the presence of tangents but no normal map texture issue a warning? Or switch to an 'error state' shader like just showing unlit + pink or something?

[superdump]

I need to select the pipeline based on the presence of the tangents in vertex attributes, and add a bit flag for the presence of the normal map texture, in case the vertex attribute is present but the texture is not, so stuff doesn't just break. We'll need to think about how to handle 'error' cases - should the presence of tangents but no normal map texture issue a warning? Or switch to an 'error state' shader like just showing unlit + pink or something?

Done.

[superdump]

This is now on top of @cart 's 'pipeline specialisation' branch, and my MSAA implementation that I think @cart is going to implement differently.

[superdump]

This is on top of #3049

[cart]

Totally not your fault, but I'm getting pretty annoyed with the alphabetical offsets. I want a Postion-Normal-Uv-Tanget layout and I want to stop needing to leave GOTCHA comments everywhere :)

That would also enable less repetition here because you could just push the tanget attribute to the back of the vec. Not something to solve in this pr, but something to keep in mind.

[cart]

We could avoid this extra #ifdef entirely if we move the normal flip right below the var N declaration.

[superdump]

That is true, but then there would be more runtime logic. As such I chose to do it this way, prioritising runtime. Which way do you want?

[cart]

How is this:

        var N: vec3<f32> = normalize(in.world_normal);
        if ((material.flags & STANDARD_MATERIAL_FLAGS_DOUBLE_SIDED_BIT) != 0u) {
            if (!in.is_front) {
                N = -N;
            }
        }

#ifdef VERTEX_TANGENTS
#ifdef STANDARDMATERIAL_NORMAL_MAP
        var T: vec3<f32> = normalize(in.world_tangent.xyz);
        var B: vec3<f32> = cross(N, T) * in.world_tangent.w;
#endif
#endif

More runtime work than this?

        var N: vec3<f32> = normalize(in.world_normal);

#ifdef VERTEX_TANGENTS
#ifdef STANDARDMATERIAL_NORMAL_MAP
        var T: vec3<f32> = normalize(in.world_tangent.xyz);
        var B: vec3<f32> = cross(N, T) * in.world_tangent.w;
#endif
#endif

        if ((material.flags & STANDARD_MATERIAL_FLAGS_DOUBLE_SIDED_BIT) != 0u) {
            if (!in.is_front) {
                N = -N;
#ifdef VERTEX_TANGENTS
#ifdef STANDARDMATERIAL_NORMAL_MAP
                T = -T;
                B = -B;
#endif
#endif
            }
        }

To me this looks like less overall work.

[cart]

Ah we don't flip T properly in that case. Carry on

[cart]

Have you compared the cost of doing this here instead of in the vertex shader? This article points out an approach that seems like it could be much faster (because it does matrix multiplication per vertex instead of per fragment):
https://learnopengl.com/Advanced-Lighting/Normal-Mapping

[superdump]

I haven't and my focus has been on correctness to start with, and if any compromises are going to be made for performance for example then I would definitely want to understand and document them.

To respond to this I have been trying to understand if there is some inaccuracy or error in the way the learnopengl.com tutorial does it as I have seen and experienced things suggesting that while informative, it is not the best reference. I prefer Foundations of Game Engine Development 2: Rendering, and/or Real-Time Rendering 4th Edition as reference material. The former pays attention to and explains small inaccuracies and how to fix them and seems to be a particularly good reference.

So a couple of comments:

• I feel like for now at least, transforming the normal map per-fragment tangent space normal to world space in the fragment shader and doing all lighting calculations in world space as before is intuitive to work with. I don't think transforming the lighting vectors (view vector, light vector) to tangent space in the vertex shader and doing calculations in tangent space is intuitive to work with, even if it may improve performance. We can try it to see if it makes a difference but the stress test would be a complex worlds with a lot of normal maps. Maybe the NVIDIA Orca Emerald Square City scene is a good candidate.
• I am wary of constructing the TBN matrix in the vertex shader either not being interpolated (I wonder how a mat3 should be interpolated sensibly? Maybe it just works, I don't know) or the interpolation introducing issues and then having to do calculations in the fragment shader to fix it up.

Foundations of Game Engine Development 2: Rendering, which I do trust as reference material that really pays attention to these kinds of details, does the following in the fragment shader:

uniform Texture2D normalMap;

float3 FetchNormalVector(float2 texcoord) {
  float2 m = texture(normalMap, texcoord).xy; 
  return (float3(m, sqrt(1.0 − m.x * m.x − m.y * m.y))); 
}

float3 FetchObjectNormalVector(float2 texcoord, float3 normal, float3 tangent, float sigma) {
  float3 m = FetchNormalVector(texcoord); 
  float3 n = normalize(normal); 
  float3 t = normalize(tangent − n * dot(tangent, n)); 
  float3 b = cross(normal, tangent) * sigma; 
  return (t * m.x + b * m.y + n * m.z); 
}

and notes:

After interpolation, the normal and tangent vectors may not have unit length, and it’s possible that they are no longer perpendicular. To correct for this, we have to orthonormalize them in the pixel shader before we calculate the bitangent vector

So FGED2 recommends doing it basically the way I am doing it, and then additionally apply Gram-Schmidt to the world space tangent vector. I'll add that.

I think we could benchmark versus the calculate TBN in vertex shader and use it to transform the view and light vectors (and any others) there, etc in a separate step as I suspect it will have knock-on effects for other algorithms that work in world/view space. Is that ok?

[superdump]

I've also just checked the left side of https://github.com/KhronosGroup/glTF-Sample-Models/tree/master/2.0/NormalTangentMirrorTest and it looks correct to me, by the way.

[superdump]

I have continued following rabbit holes to understand what to do here and it looks like UE4 interpolates the world space normal and tangent, calculates the bitangent simply using the cross product (no normalisation yet), samples the tangent space normal from the normal map, on mobile it doesn't normalise that for performance, elsewhere it does. The TBN matrix is constructed in the fragment shader, the tangent space normal is transformed to world space, and the result is normalised.

Now, I thought it interesting that they don't use Gram-Schmidt or try to ensure orthonormality across the interpolation. When constructing the TBN matrix, there is this comment:

    // Will not be orthonormal after interpolation. This perfectly matches xNormal.
    // Any mismatch with xNormal will cause distortions for baked normal maps.

It looks like UE4 uses https://github.com/mmikk/MikkTSpace to calculate normals, and that is used in Blender and xNormal. There is a discussion of what they're all doing here: http://www.mikktspace.com/ tl;dr calculating tangents and normal maps is practically an encoding process and the decoding process needs to be precisely the inverse of the encoding process in order to obtain the desired results. But different software does different things, so Mikkelsen did a Masters thesis on it to try to design an approach that would be robust to things like the order of vertices supplied to process to generate tangents and normal maps, as well as the decoding process. This seems to be a possibly good pragmatic and practical solution that is worth investigating. Though perhaps it should be done separate to this PR?

[cart]

Thanks for digging in to this. It does sound like whats implemented right now is safe and has precedent in the industry. I agree that we can/should follow this up with tangent calculations, but honestly we might want that to wait until we have a real asset pipeline. I dont want to do a bunch of number-crunchy stuff at runtime when an asset loads.

[cart]

bors r+

[bors]

Pull request successfully merged into pipelined-rendering.

Build succeeded:

• build (nightly, ubuntu-latest)
• build (stable, macos-latest)
• build (stable, ubuntu-latest)
• build (stable, windows-latest)
• check-doc
• check-markdown-links
• check-missing-examples-in-docs
• check-unused-dependencies
• markdownlint
• run-examples