From 3203e1985e8a7d0e9d56fba619e94cda0dce0305 Mon Sep 17 00:00:00 2001 From: Thomas Gamper Date: Thu, 23 Nov 2023 15:14:46 +0100 Subject: [PATCH] Fix #464 tinygltf.h - serialize empty scenes as empty json objects; tester.cc - ad respective test case, bring test cases in correct order, tag test case for light index with correct issue number and fix it to compare to deserialozed model --- tests/tester.cc | 154 ++++++++++++++++++++++++++++-------------------- tiny_gltf.h | 9 +++ 2 files changed, 100 insertions(+), 63 deletions(-) diff --git a/tests/tester.cc b/tests/tester.cc index 5803cfd..409df1c 100644 --- a/tests/tester.cc +++ b/tests/tester.cc @@ -756,34 +756,83 @@ TEST_CASE("load-issue-416-model", "[issue-416]") { REQUIRE(true == ret); } -TEST_CASE("serialize-light-index", "[issue-457]") { - tinygltf::Model model; - tinygltf::Scene scene; +TEST_CASE("serialize-empty-node", "[issue-457]") { + tinygltf::Model m; + // Add default constructed node to model + m.nodes.push_back({}); + // Add scene to model + m.scenes.push_back({}); + // The scene's only node is the empty node + m.scenes.front().nodes.push_back(0); + + // Serialize model to output stream + std::stringstream os; + tinygltf::TinyGLTF ctx; + bool ret = ctx.WriteGltfSceneToStream(&m, os, false, false); + REQUIRE(true == ret); + + // Parse serialized model + nlohmann::json j = nlohmann::json::parse(os.str()); + + // Serialized nodes shall hold an empty object that + // represents the default constructed node + REQUIRE(j.find("nodes") != j.end()); + REQUIRE(j["nodes"].is_array()); + REQUIRE(1 == j["nodes"].size()); + CHECK(j["nodes"][0].is_object()); + CHECK(j["nodes"][0].empty()); + + // We also want to make sure that the serialized scene + // is referencing the empty node. + + // There shall be a single serialized scene + auto scenes = j.find("scenes"); + REQUIRE(scenes != j.end()); + REQUIRE(scenes->is_array()); + REQUIRE(1 == scenes->size()); + auto scene = scenes->at(0); + REQUIRE(scene.is_object()); + // The scene's nodes array shall hold a reference + // to the single node + auto nodes = scene.find("nodes"); + REQUIRE(nodes != scene.end()); + REQUIRE(nodes->is_array()); + REQUIRE(1 == nodes->size()); + auto node = nodes->at(0); + CHECK(node.is_number_integer()); + int idx = -1; + node.get_to(idx); + CHECK(0 == idx); +} + +TEST_CASE("serialize-light-index", "[issue-458]") { // Create the light tinygltf::Light light; light.type = "point"; light.intensity = 0.75; light.color = std::vector{1.0, 0.8, 0.95}; - // Add the light to the model - model.lights.push_back(light); - // Create a node that uses the light - tinygltf::Node node; - node.light = 0; - // Add the node to the model - model.nodes.push_back(node); - // Add the node to the scene - scene.nodes.push_back(0); - // Add the scene to the model - model.scenes.push_back(scene); // Stream to serialize to std::stringstream os; { + tinygltf::Model m; + tinygltf::Scene scene; + // Add the light to the model + m.lights.push_back(light); + // Create a node that uses the light + tinygltf::Node node; + node.light = 0; + // Add the node to the model + m.nodes.push_back(node); + // Add the node to the scene + scene.nodes.push_back(0); + // Add the scene to the model + m.scenes.push_back(scene); // Serialize model to output stream tinygltf::TinyGLTF ctx; - bool ret = ctx.WriteGltfSceneToStream(&model, os, false, false); + bool ret = ctx.WriteGltfSceneToStream(&m, os, false, false); REQUIRE(true == ret); } @@ -794,11 +843,11 @@ TEST_CASE("serialize-light-index", "[issue-457]") { bool ok = ctx.LoadASCIIFromString(&m, nullptr, nullptr, os.str().c_str(), os.str().size(), ""); REQUIRE(true == ok); // Check if the light was correctly serialized - REQUIRE(1 == model.lights.size()); - CHECK(model.lights[0] == light); + REQUIRE(1 == m.lights.size()); + CHECK(m.lights[0] == light); // Check that the node properly references the light - REQUIRE(1 == model.nodes.size()); - CHECK(model.nodes[0].light == 0); + REQUIRE(1 == m.nodes.size()); + CHECK(m.nodes[0].light == 0); } } @@ -826,51 +875,30 @@ TEST_CASE("default-material", "[issue-459]") { CHECK(mat.emissiveTexture.index == -1); } -TEST_CASE("serialize-empty-node", "[issue-457]") { - tinygltf::Model m; - // Add default constructed node to model - m.nodes.push_back({}); - // Add scene to model - m.scenes.push_back({}); - // The scene's only node is the empty node - m.scenes.front().nodes.push_back(0); - - // Serialize model to output stream +TEST_CASE("serialize-empty-scene", "[issue-464]") { + // Stream to serialize to std::stringstream os; - tinygltf::TinyGLTF ctx; - bool ret = ctx.WriteGltfSceneToStream(&m, os, false, false); - REQUIRE(true == ret); - // Parse serialized model - nlohmann::json j = nlohmann::json::parse(os.str()); - - // Serialized nodes shall hold an empty object that - // represents the default constructed node - REQUIRE(j.find("nodes") != j.end()); - REQUIRE(j["nodes"].is_array()); - REQUIRE(1 == j["nodes"].size()); - CHECK(j["nodes"][0].is_object()); - CHECK(j["nodes"][0].empty()); - - // We also want to make sure that the serialized scene - // is referencing the empty node. + { + tinygltf::Model m; + // Add empty scene to the model + m.scenes.push_back({}); + // Serialize model to output stream + tinygltf::TinyGLTF ctx; + bool ret = ctx.WriteGltfSceneToStream(&m, os, false, false); + REQUIRE(true == ret); + } - // There shall be a single serialized scene - auto scenes = j.find("scenes"); - REQUIRE(scenes != j.end()); - REQUIRE(scenes->is_array()); - REQUIRE(1 == scenes->size()); - auto scene = scenes->at(0); - REQUIRE(scene.is_object()); - // The scene's nodes array shall hold a reference - // to the single node - auto nodes = scene.find("nodes"); - REQUIRE(nodes != scene.end()); - REQUIRE(nodes->is_array()); - REQUIRE(1 == nodes->size()); - auto node = nodes->at(0); - CHECK(node.is_number_integer()); - int idx = -1; - node.get_to(idx); - CHECK(0 == idx); + { + tinygltf::Model m; + tinygltf::TinyGLTF ctx; + // Parse the serialized model + bool ok = ctx.LoadASCIIFromString(&m, nullptr, nullptr, os.str().c_str(), os.str().size(), ""); + REQUIRE(true == ok); + // Make sure the empty scene is there + REQUIRE(1 == m.scenes.size()); + tinygltf::Scene scene{}; + // Check that the scene is empty + CHECK(m.scenes[0] == scene); + } } diff --git a/tiny_gltf.h b/tiny_gltf.h index f957968..5f64f17 100644 --- a/tiny_gltf.h +++ b/tiny_gltf.h @@ -8063,6 +8063,15 @@ static void SerializeGltfModel(const Model *model, detail::json &o) { for (unsigned int i = 0; i < model->scenes.size(); ++i) { detail::json currentScene; SerializeGltfScene(model->scenes[i], currentScene); + if (detail::JsonIsNull(currentScene)) { + // Issue 464. + // `scene` does not have any required parameters, + // so the result may be null(unmodified) when all scene parameters + // have default value. + // + // null is not allowed thus we create an empty JSON object. + detail::JsonSetObject(currentScene); + } detail::JsonPushBack(scenes, std::move(currentScene)); } detail::JsonAddMember(o, "scenes", std::move(scenes));