From fd6c7855e750cbcc7d81c30cef54cb6e6d7e1570 Mon Sep 17 00:00:00 2001 From: Thomas Gamper Date: Wed, 22 Nov 2023 14:17:46 +0100 Subject: [PATCH 1/2] fix #459 tiny_gltf.h - properly initialise emissiveFactor; tests/tester.cc - add test case --- tests/tester.cc | 46 ++++++++++++++++++++++++++++++++++------------ tiny_gltf.h | 4 +++- 2 files changed, 37 insertions(+), 13 deletions(-) diff --git a/tests/tester.cc b/tests/tester.cc index 8ebeeae..daf088f 100644 --- a/tests/tester.cc +++ b/tests/tester.cc @@ -494,25 +494,23 @@ TEST_CASE("image-uri-spaces", "[issue-236]") { } TEST_CASE("serialize-empty-material", "[issue-294]") { - tinygltf::Model m; - - tinygltf::Material mat; - mat.pbrMetallicRoughness.baseColorFactor = {1.0f, 1.0f, 1.0f, 1.0f}; // default baseColorFactor - m.materials.push_back(mat); - + // Add default constructed material to model + m.materials.push_back({}); + // Serialize model to output stream std::stringstream os; - tinygltf::TinyGLTF ctx; bool ret = ctx.WriteGltfSceneToStream(&m, os, false, false); REQUIRE(true == ret); - - // use nlohmann json + // Parse serialized model nlohmann::json j = nlohmann::json::parse(os.str()); - + // Serialized materials shall hold an empty object that + // represents the default constructed material + REQUIRE(j.find("materials") != j.end()); + REQUIRE(j["materials"].is_array()); REQUIRE(1 == j["materials"].size()); - REQUIRE(j["materials"][0].is_object()); - + CHECK(j["materials"][0].is_object()); + CHECK(j["materials"][0].empty()); } TEST_CASE("empty-skeleton-id", "[issue-321]") { @@ -757,3 +755,27 @@ TEST_CASE("load-issue-416-model", "[issue-416]") { // external file load fails, but reading glTF itself is ok. REQUIRE(true == ret); } + +TEST_CASE("default-material", "[issue-459]") { + const std::vector default_emissive_factor{ 0.0, 0.0, 0.0 }; + const std::vector default_base_color_factor{ 1.0, 1.0, 1.0, 1.0 }; + const std::string default_alpha_mode = "OPAQUE"; + const double default_alpha_cutoff = 0.5; + const bool default_double_sided = false; + const double default_metallic_factor = 1.0; + const double default_roughness_factor = 1.0; + // Check that default constructed material + // holds actual default GLTF material properties + tinygltf::Material mat; + CHECK(mat.alphaMode == default_alpha_mode); + CHECK(mat.alphaCutoff == default_alpha_cutoff); + CHECK(mat.doubleSided == default_double_sided); + CHECK(mat.emissiveFactor == default_emissive_factor); + CHECK(mat.pbrMetallicRoughness.baseColorFactor == default_base_color_factor); + CHECK(mat.pbrMetallicRoughness.metallicFactor == default_metallic_factor); + CHECK(mat.pbrMetallicRoughness.roughnessFactor == default_roughness_factor); + // None of the textures should be set + CHECK(mat.normalTexture.index == -1); + CHECK(mat.occlusionTexture.index == -1); + CHECK(mat.emissiveTexture.index == -1); +} diff --git a/tiny_gltf.h b/tiny_gltf.h index 0b4a1d9..6b0b75c 100644 --- a/tiny_gltf.h +++ b/tiny_gltf.h @@ -783,7 +783,9 @@ struct Material { std::string extras_json_string; std::string extensions_json_string; - Material() : alphaMode("OPAQUE") {} + Material() + : emissiveFactor(std::vector{0.0, 0.0, 0.0}), alphaMode("OPAQUE") + {} DEFAULT_METHODS(Material) bool operator==(const Material &) const; From 1f42c963e620d958e11b450ffa925fd061034936 Mon Sep 17 00:00:00 2001 From: Thomas Gamper Date: Wed, 22 Nov 2023 15:59:13 +0100 Subject: [PATCH 2/2] fix #459 tiny_gltf.h - use member initialization --- tiny_gltf.h | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/tiny_gltf.h b/tiny_gltf.h index 6b0b75c..7991303 100644 --- a/tiny_gltf.h +++ b/tiny_gltf.h @@ -735,7 +735,7 @@ struct OcclusionTextureInfo { // pbrMetallicRoughness class defined in glTF 2.0 spec. struct PbrMetallicRoughness { - std::vector baseColorFactor; // len = 4. default [1,1,1,1] + std::vector baseColorFactor{1.0, 1.0, 1.0, 1.0}; // len = 4. default [1,1,1,1] TextureInfo baseColorTexture; double metallicFactor{1.0}; // default 1 double roughnessFactor{1.0}; // default 1 @@ -748,9 +748,9 @@ struct PbrMetallicRoughness { std::string extras_json_string; std::string extensions_json_string; - PbrMetallicRoughness() - : baseColorFactor(std::vector{1.0, 1.0, 1.0, 1.0}) {} + PbrMetallicRoughness() = default; DEFAULT_METHODS(PbrMetallicRoughness) + bool operator==(const PbrMetallicRoughness &) const; }; @@ -760,10 +760,10 @@ struct PbrMetallicRoughness { struct Material { std::string name; - std::vector emissiveFactor; // length 3. default [0, 0, 0] - std::string alphaMode; // default "OPAQUE" - double alphaCutoff{0.5}; // default 0.5 - bool doubleSided{false}; // default false; + std::vector emissiveFactor{0.0, 0.0, 0.0}; // length 3. default [0, 0, 0] + std::string alphaMode{"OPAQUE"}; // default "OPAQUE" + double alphaCutoff{0.5}; // default 0.5 + bool doubleSided{false}; // default false PbrMetallicRoughness pbrMetallicRoughness; @@ -783,9 +783,7 @@ struct Material { std::string extras_json_string; std::string extensions_json_string; - Material() - : emissiveFactor(std::vector{0.0, 0.0, 0.0}), alphaMode("OPAQUE") - {} + Material() = default; DEFAULT_METHODS(Material) bool operator==(const Material &) const;