Validate defined tangent space (#224)

KhronosGroup · Jun 25, 2024 · bc2159d · bc2159d
1 parent 0cdee26
commit bc2159d
Show file tree

Hide file tree

Showing 18 changed files with 304 additions and 11 deletions.
diff --git a/ISSUES.md b/ISSUES.md
@@ -101,11 +101,13 @@
 |MESH_PRIMITIVE_ACCESSOR_WITHOUT_BYTESTRIDE|bufferView.byteStride must be defined when two or more accessors use the same buffer view.|Error|
 |MESH_PRIMITIVE_ATTRIBUTES_ACCESSOR_INVALID_FORMAT|Invalid accessor format '`%1`' for this attribute semantic. Must be one of ('`%a`', '`%b`', '`%c`').|Error|
 |MESH_PRIMITIVE_ATTRIBUTES_ACCESSOR_UNSIGNED_INT|Mesh attributes cannot use UNSIGNED_INT component type.|Error|
+|MESH_PRIMITIVE_GENERATED_TANGENT_SPACE|Material requires a tangent space but the mesh primitive does not provide it. Runtime-generated tangent space may be non-portable across implementations.|Warning|
 |MESH_PRIMITIVE_INCOMPATIBLE_MODE|Number of vertices or indices (`%1`) is not compatible with used drawing mode ('`%2`').|Warning|
 |MESH_PRIMITIVE_INDICES_ACCESSOR_INVALID_FORMAT|Invalid indices accessor format '`%1`'. Must be one of ('`%a`', '`%b`', '`%c`'). |Error|
 |MESH_PRIMITIVE_INDICES_ACCESSOR_WITH_BYTESTRIDE|bufferView.byteStride must not be defined for indices accessor.|Error|
 |MESH_PRIMITIVE_MORPH_TARGET_INVALID_ATTRIBUTE_COUNT|Base accessor has different count.|Error|
 |MESH_PRIMITIVE_MORPH_TARGET_NO_BASE_ACCESSOR|The mesh primitive does not define this attribute semantic.|Error|
+|MESH_PRIMITIVE_NO_TANGENT_SPACE|Material requires a tangent space but the mesh primitive does not provide it and the material does not contain a normal map to generate it.|Error|
 |MESH_PRIMITIVE_POSITION_ACCESSOR_WITHOUT_BOUNDS|accessor.min and accessor.max must be defined for POSITION attribute accessor.|Error|
 |MESH_PRIMITIVE_TOO_FEW_TEXCOORDS|Material is incompatible with mesh primitive: Texture binding '`%1`' needs 'TEXCOORD_`%2`' attribute.|Error|
 |MESH_PRIMITIVE_UNEQUAL_ACCESSOR_COUNT|All accessors of the same primitive must have the same count.|Error|

diff --git a/lib/src/base/material.dart b/lib/src/base/material.dart
@@ -28,9 +28,9 @@ class Material extends GltfChildOfRootProperty {
   final double alphaCutoff;
   final bool doubleSided;
 
-  bool _needsTangent = false;
+  bool needsTangent = false;
 
-  bool get needsTangent => _needsTangent;
+  bool get canProvideTangent => normalTexture != null;
 
   final Map<String, int> texCoordIndices = <String, int>{};
 
@@ -250,7 +250,7 @@ class NormalTextureInfo extends TextureInfo {
     while (o != null) {
       o = context.owners[o];
       if (o is Material) {
-        o._needsTangent = true;
+        o.needsTangent = true;
         break;
       }
     }

diff --git a/lib/src/base/mesh.dart b/lib/src/base/mesh.dart
@@ -526,6 +526,14 @@ class MeshPrimitive extends GltfProperty {
         } else {
           _material.markAsUsed();
 
+          if (!(hasNormal && hasTangent) && _material.needsTangent) {
+            context.addIssue(
+                _material.canProvideTangent
+                    ? LinkError.meshPrimitiveGeneratedTangentSpace
+                    : LinkError.meshPrimitiveNoTangentSpace,
+                name: MATERIAL);
+          }
+
           _material.texCoordIndices.forEach((pointer, texCoord) {
             if (texCoord != -1) {
               if (texCoord + 1 > texCoordCount) {

diff --git a/lib/src/errors.dart b/lib/src/errors.dart
@@ -694,6 +694,19 @@ class LinkError extends IssueType {
           'Texture binding ${_q(args[0])} '
           "needs 'TEXCOORD_${args[1]}' attribute.");
 
+  static final LinkError meshPrimitiveNoTangentSpace = LinkError._(
+      'MESH_PRIMITIVE_NO_TANGENT_SPACE',
+      (args) => 'Material requires a tangent space but the mesh '
+          'primitive does not provide it and the material does not contain '
+          'a normal map to generate it.');
+
+  static final LinkError meshPrimitiveGeneratedTangentSpace = LinkError._(
+      'MESH_PRIMITIVE_GENERATED_TANGENT_SPACE',
+      (args) => 'Material requires a tangent space but the mesh '
+          'primitive does not provide it. Runtime-generated tangent space '
+          'may be non-portable across implementations.',
+      Severity.Warning);
+
   static final LinkError meshPrimitiveUnequalAccessorsCount = LinkError._(
       'MESH_PRIMITIVE_UNEQUAL_ACCESSOR_COUNT',
       (args) =>

diff --git a/test/base/assets.json b/test/base/assets.json
@@ -222,6 +222,7 @@
             "morph_weights_wrong_count.gltf": "Morph weights wrong count",
             "position_accessor_invalid_format.gltf": "Position accessor invalid format",
             "position_accessor_no_bounds.gltf": "Position accessor no bounds",
+            "primitive_generated_tangent_space.gltf": "Primitive generated tangent space",
             "primitive_incompatible_mode.gltf": "Primitive incompatible mode",
             "primitive_no_position.gltf": "Primitive no position",
             "unequal_joints.gltf": "Unequal joints per primitive",

diff --git a/test/base/data/mesh/primitive_generated_tangent_space.gltf b/test/base/data/mesh/primitive_generated_tangent_space.gltf
@@ -0,0 +1,78 @@
+{
+    "asset": {
+        "version": "2.0"
+    },
+    "meshes": [
+        {
+            "primitives": [
+                {
+                    "attributes": {
+                        "POSITION": 0,
+                        "TEXCOORD_0": 3
+                    },
+                    "material": 0
+                },
+                {
+                    "attributes": {
+                        "POSITION": 0,
+                        "NORMAL": 1,
+                        "TEXCOORD_0": 3
+                    },
+                    "material": 0
+                },
+                {
+                    "attributes": {
+                        "POSITION": 0,
+                        "TANGENT": 2,
+                        "TEXCOORD_0": 3
+                    },
+                    "material": 0
+                },
+                {
+                    "attributes": {
+                        "POSITION": 0,
+                        "NORMAL": 1,
+                        "TANGENT": 2,
+                        "TEXCOORD_0": 3
+                    },
+                    "material": 0
+                }
+            ]
+        }
+    ],
+    "accessors": [
+        {
+            "type": "VEC3",
+            "componentType": 5126,
+            "count": 3,
+            "min": [0, 0, 0],
+            "max": [1, 1, 1]
+        },
+        {
+            "type": "VEC3",
+            "componentType": 5126,
+            "count": 3
+        },
+        {
+            "type": "VEC4",
+            "componentType": 5126,
+            "count": 3
+        },
+        {
+            "type": "VEC2",
+            "componentType": 5126,
+            "count": 3
+        }
+    ],
+    "materials": [
+        {
+            "normalTexture": {
+                "index": 0
+            }
+        }
+    ],
+    "textures": [
+        {
+        }
+    ]
+}
diff --git a/test/base/data/mesh/primitive_generated_tangent_space.gltf.report.json b/test/base/data/mesh/primitive_generated_tangent_space.gltf.report.json
@@ -0,0 +1,59 @@
+{
+    "uri": "test/base/data/mesh/primitive_generated_tangent_space.gltf",
+    "mimeType": "model/gltf+json",
+    "validatorVersion": "2.0.0-dev.3.10",
+    "issues": {
+        "numErrors": 0,
+        "numWarnings": 4,
+        "numInfos": 1,
+        "numHints": 0,
+        "messages": [
+            {
+                "code": "MESH_PRIMITIVE_TANGENT_WITHOUT_NORMAL",
+                "message": "TANGENT attribute without NORMAL found.",
+                "severity": 1,
+                "pointer": "/meshes/0/primitives/2/attributes/TANGENT"
+            },
+            {
+                "code": "MESH_PRIMITIVE_GENERATED_TANGENT_SPACE",
+                "message": "Material requires a tangent space but the mesh primitive does not provide it. Runtime-generated tangent space may be non-portable across implementations.",
+                "severity": 1,
+                "pointer": "/meshes/0/primitives/0/material"
+            },
+            {
+                "code": "MESH_PRIMITIVE_GENERATED_TANGENT_SPACE",
+                "message": "Material requires a tangent space but the mesh primitive does not provide it. Runtime-generated tangent space may be non-portable across implementations.",
+                "severity": 1,
+                "pointer": "/meshes/0/primitives/1/material"
+            },
+            {
+                "code": "MESH_PRIMITIVE_GENERATED_TANGENT_SPACE",
+                "message": "Material requires a tangent space but the mesh primitive does not provide it. Runtime-generated tangent space may be non-portable across implementations.",
+                "severity": 1,
+                "pointer": "/meshes/0/primitives/2/material"
+            },
+            {
+                "code": "UNUSED_OBJECT",
+                "message": "This object may be unused.",
+                "severity": 2,
+                "pointer": "/meshes/0"
+            }
+        ],
+        "truncated": false
+    },
+    "info": {
+        "version": "2.0",
+        "animationCount": 0,
+        "materialCount": 1,
+        "hasMorphTargets": false,
+        "hasSkins": false,
+        "hasTextures": true,
+        "hasDefaultScene": false,
+        "drawCallCount": 4,
+        "totalVertexCount": 12,
+        "totalTriangleCount": 4,
+        "maxUVs": 1,
+        "maxInfluences": 0,
+        "maxAttributes": 4
+    }
+}
diff --git a/test/base/data/mesh/uv.gltf b/test/base/data/mesh/uv.gltf
@@ -12,13 +12,17 @@
                     "material": 0,
                     "attributes": {
                         "POSITION": 0,
+                        "NORMAL": 2,
+                        "TANGENT": 3,
                         "TEXCOORD_0": 1
                     }
                 },
                 {
                     "material": 0,
                     "attributes": {
                         "POSITION": 0,
+                        "NORMAL": 2,
+                        "TANGENT": 3,
                         "TEXCOORD_0": 1,
                         "TEXCOORD_1": 1
                     }
@@ -98,6 +102,16 @@
                 1,
                 1
             ]
+        },
+        {
+            "type": "VEC3",
+            "componentType": 5126,
+            "count": 3
+        },
+        {
+            "type": "VEC4",
+            "componentType": 5126,
+            "count": 3
         }
     ],
     "textures": [

diff --git a/test/base/data/mesh/uv.gltf.report.json b/test/base/data/mesh/uv.gltf.report.json
@@ -1,7 +1,7 @@
 {
     "uri": "test/base/data/mesh/uv.gltf",
     "mimeType": "model/gltf+json",
-    "validatorVersion": "2.0.0-dev.3.0",
+    "validatorVersion": "2.0.0-dev.3.10",
     "issues": {
         "numErrors": 7,
         "numWarnings": 0,
@@ -93,6 +93,6 @@
         "totalTriangleCount": 3,
         "maxUVs": 2,
         "maxInfluences": 0,
-        "maxAttributes": 3
+        "maxAttributes": 5
     }
 }
diff --git a/test/base/data/root/named_objects.gltf b/test/base/data/root/named_objects.gltf
@@ -27,6 +27,7 @@
         {
           "attributes": {
             "NORMAL": 1,
+            "TANGENT": 8,
             "POSITION": 2,
             "TEXCOORD_0": 3,
             "JOINTS_0": 6,
@@ -113,6 +114,11 @@
       "count": 24,
       "type": "VEC4"
     },
+    {
+      "componentType": 5126,
+      "count": 24,
+      "type": "VEC4"
+    },
     {
       "componentType": 5126,
       "count": 24,

diff --git a/test/base/data/root/named_objects.gltf.report.json b/test/base/data/root/named_objects.gltf.report.json
@@ -1,7 +1,7 @@
 {
     "uri": "test/base/data/root/named_objects.gltf",
     "mimeType": "model/gltf+json",
-    "validatorVersion": "2.0.0-dev.3.0",
+    "validatorVersion": "2.0.0-dev.3.10",
     "issues": {
         "numErrors": 0,
         "numWarnings": 0,
@@ -42,6 +42,6 @@
         "totalTriangleCount": 12,
         "maxUVs": 1,
         "maxInfluences": 4,
-        "maxAttributes": 5
+        "maxAttributes": 6
     }
 }
diff --git a/test/base/data/root/unused_objects.gltf b/test/base/data/root/unused_objects.gltf
@@ -24,6 +24,7 @@
         {
           "attributes": {
             "NORMAL": 1,
+            "TANGENT": 7,
             "POSITION": 2,
             "TEXCOORD_0": 3
           },
@@ -115,6 +116,11 @@
       "componentType": 5126,
       "count": 1,
       "type": "VEC3"
+    },
+    {
+      "componentType": 5126,
+      "count": 24,
+      "type": "VEC4"
     }
   ],
   "materials": [

diff --git a/test/base/data/root/unused_objects.gltf.report.json b/test/base/data/root/unused_objects.gltf.report.json
@@ -1,7 +1,7 @@
 {
     "uri": "test/base/data/root/unused_objects.gltf",
     "mimeType": "model/gltf+json",
-    "validatorVersion": "2.0.0-dev.3.0",
+    "validatorVersion": "2.0.0-dev.3.10",
     "issues": {
         "numErrors": 0,
         "numWarnings": 0,
@@ -132,6 +132,6 @@
         "totalTriangleCount": 20,
         "maxUVs": 1,
         "maxInfluences": 0,
-        "maxAttributes": 3
+        "maxAttributes": 4
     }
 }
diff --git a/test/ext/KHR_materials_clearcoat/assets.json b/test/ext/KHR_materials_clearcoat/assets.json
@@ -3,6 +3,7 @@
         "name": "material.KHR_materials_clearcoat",
         "tests": {
             "custom_property.gltf": "Custom property",
+            "no_tangent_space.gltf": "No tangent space",
             "unexpected_extension.gltf": "Unexpected extension object location",
             "valid.gltf": "Valid",
             "valid_with_tangent.gltf": "Valid with TANGENT"