Fix: PBRData shouldn't be optional

spnda · Jul 20, 2023 · 92ecd09 · 92ecd09
1 parent e53e894
commit 92ecd09
Show file tree

Hide file tree

Showing 4 changed files with 24 additions and 36 deletions.
diff --git a/include/fastgltf/types.hpp b/include/fastgltf/types.hpp
@@ -1118,19 +1118,19 @@ namespace fastgltf {
 
     struct PBRData {
         /**
-         * The factors for the base color of then material. Defaults to 1,1,1,1
+         * The factors for the base color of then material.
          */
-        std::array<float, 4> baseColorFactor;
+        std::array<float, 4> baseColorFactor = {{ 1, 1, 1, 1 }};
 
         /**
-         * The factor for the metalness of the material. Defaults to 1
+         * The factor for the metalness of the material.
          */
-        float metallicFactor;
+        float metallicFactor = 1.0f;
 
         /**
-         * The factor for the roughness of the material. Defaults to 1
+         * The factor for the roughness of the material.
          */
-        float roughnessFactor;
+        float roughnessFactor = 1.0f;
 
         std::optional<TextureInfo> baseColorTexture;
         std::optional<TextureInfo> metallicRoughnessTexture;
@@ -1191,10 +1191,9 @@ namespace fastgltf {
     struct Material {
         /**
          * A set of parameter values that are used to define the metallic-roughness material model
-         * from Physically Based Rendering (PBR) methodology. When undefined, all the default
-         * values of pbrMetallicRoughness MUST apply.
+         * from Physically Based Rendering (PBR) methodology.
          */
-        std::optional<PBRData> pbrData;
+        PBRData pbrData;
 
         /**
          * The tangent space normal texture.

diff --git a/src/fastgltf.cpp b/src/fastgltf.cpp
@@ -802,14 +802,12 @@ fg::Error fg::Parser::validate(const fastgltf::Asset& asset) const {
 			return Error::InvalidGltf;
 		if (material.occlusionTexture.has_value() && isInvalidTexture(material.occlusionTexture->textureIndex))
 			return Error::InvalidGltf;
-		if (material.pbrData.has_value()) {
-			if (material.pbrData->baseColorTexture.has_value() &&
-			    isInvalidTexture(material.pbrData->baseColorTexture->textureIndex))
-				return Error::InvalidGltf;
-			if (material.pbrData->metallicRoughnessTexture.has_value() &&
-			    isInvalidTexture(material.pbrData->metallicRoughnessTexture->textureIndex))
-				return Error::InvalidGltf;
-		}
+		if (material.pbrData.baseColorTexture.has_value() &&
+		    isInvalidTexture(material.pbrData.baseColorTexture->textureIndex))
+			return Error::InvalidGltf;
+		if (material.pbrData.metallicRoughnessTexture.has_value() &&
+		    isInvalidTexture(material.pbrData.metallicRoughnessTexture->textureIndex))
+			return Error::InvalidGltf;
 	}
 
 	for (const auto& mesh : asset.meshes) {
@@ -2007,20 +2005,14 @@ fg::Error fg::Parser::parseMaterials(simdjson::dom::array& materials, Asset& ass
                     }
                     pbr.baseColorFactor[i] = static_cast<float>(val);
                 }
-            } else {
-                pbr.baseColorFactor = {{ 1, 1, 1, 1 }};
             }
 
             double factor;
             if (pbrMetallicRoughness["metallicFactor"].get_double().get(factor) == SUCCESS) {
                 pbr.metallicFactor = static_cast<float>(factor);
-            } else {
-                pbr.metallicFactor = 1.0F;
             }
             if (pbrMetallicRoughness["roughnessFactor"].get_double().get(factor) == SUCCESS) {
                 pbr.roughnessFactor = static_cast<float>(factor);
-            } else {
-                pbr.roughnessFactor = 1.0F;
             }
 
             if (auto error = parseTextureObject(pbrMetallicRoughness, "baseColorTexture", &textureObject, config.extensions); error == Error::None) {

diff --git a/tests/basic_test.cpp b/tests/basic_test.cpp
@@ -166,11 +166,10 @@ TEST_CASE("Loading some basic glTF", "[gltf-loader]") {
         REQUIRE(cube->materials.size() == 1);
         auto& material = cube->materials.front();
         REQUIRE(material.name == "Cube");
-        REQUIRE(material.pbrData.has_value());
-        REQUIRE(material.pbrData->baseColorTexture.has_value());
-        REQUIRE(material.pbrData->baseColorTexture->textureIndex == 0);
-        REQUIRE(material.pbrData->metallicRoughnessTexture.has_value());
-        REQUIRE(material.pbrData->metallicRoughnessTexture->textureIndex == 1);
+        REQUIRE(material.pbrData.baseColorTexture.has_value());
+        REQUIRE(material.pbrData.baseColorTexture->textureIndex == 0);
+        REQUIRE(material.pbrData.metallicRoughnessTexture.has_value());
+        REQUIRE(material.pbrData.metallicRoughnessTexture->textureIndex == 1);
         REQUIRE(!material.normalTexture.has_value());
         REQUIRE(!material.emissiveTexture.has_value());
         REQUIRE(!material.occlusionTexture.has_value());
@@ -197,9 +196,8 @@ TEST_CASE("Loading some basic glTF", "[gltf-loader]") {
 
         REQUIRE(box->materials.size() == 1);
         REQUIRE(box->materials[0].name == "Red");
-        REQUIRE(box->materials[0].pbrData.has_value());
-        REQUIRE(box->materials[0].pbrData->baseColorFactor[3] == 1.0f);
-        REQUIRE(box->materials[0].pbrData->metallicFactor == 0.0f);
+        REQUIRE(box->materials[0].pbrData.baseColorFactor[3] == 1.0f);
+        REQUIRE(box->materials[0].pbrData.metallicFactor == 0.0f);
     }
 }
 

diff --git a/tests/extension_tests.cpp b/tests/extension_tests.cpp
@@ -59,11 +59,10 @@ TEST_CASE("Loading KHR_texture_transform glTF files", "[gltf-loader]") {
     REQUIRE(!asset->materials.empty());
 
     auto& material = asset->materials.front();
-    REQUIRE(material.pbrData.has_value());
-    REQUIRE(material.pbrData->baseColorTexture.has_value());
-    REQUIRE(material.pbrData->baseColorTexture->transform != nullptr);
-    REQUIRE(material.pbrData->baseColorTexture->transform->uvOffset[0] == 0.705f);
-    REQUIRE(material.pbrData->baseColorTexture->transform->rotation == Catch::Approx(1.5707963705062866f));
+    REQUIRE(material.pbrData.baseColorTexture.has_value());
+    REQUIRE(material.pbrData.baseColorTexture->transform != nullptr);
+    REQUIRE(material.pbrData.baseColorTexture->transform->uvOffset[0] == 0.705f);
+    REQUIRE(material.pbrData.baseColorTexture->transform->rotation == Catch::Approx(1.5707963705062866f));
 }
 
 TEST_CASE("Test KHR_lights_punctual", "[gltf-loader]") {