[Data/GltfLoad] The nodes' transforms' application is now recursive

- This effectively fixes the issue mentioned before, where a node loaded first could have its transform still in its local space
Razakhel · Jan 13, 2024 · bc919a3 · bc919a3
1 parent f689f2c
commit bc919a3
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 50 deletions.
diff --git a/src/RaZ/Data/GltfLoad.cpp b/src/RaZ/Data/GltfLoad.cpp
@@ -26,58 +26,41 @@ Transform loadTransform(const fastgltf::Node& node) {
 
 }
 
-std::vector<std::optional<Transform>> loadTransforms(const std::vector<fastgltf::Node>& nodes, std::size_t meshNodeCount) {
-  std::vector<std::optional<Transform>> transforms;
-  transforms.resize(meshNodeCount);
-
-  for (std::size_t transformIndex = 0; transformIndex < meshNodeCount; ++transformIndex) {
-    const fastgltf::Node& node = nodes[transformIndex];
-
-    if (!node.meshIndex.has_value())
-      continue;
-
-    const std::size_t nodeMeshIndex = *node.meshIndex;
+void computeNodeTransform(const fastgltf::Node& currentNode,
+                          const std::optional<Transform>& parentTransform,
+                          const std::vector<fastgltf::Node>& nodes,
+                          std::vector<std::optional<Transform>>& transforms) {
+  if (!currentNode.meshIndex.has_value())
+    return;
 
-    if (nodeMeshIndex >= meshNodeCount) {
-      Logger::error("[GltfLoad] Unexpected node mesh index.");
-      continue;
-    }
+  const std::size_t currentMeshIndex = *currentNode.meshIndex;
 
-    std::optional<Transform>& nodeTransform = transforms[nodeMeshIndex];
+  if (currentMeshIndex >= transforms.size()) {
+    Logger::error("[GltfLoad] Unexpected node mesh index.");
+    return;
+  }
 
-    if (!nodeTransform.has_value())
-      nodeTransform = loadTransform(node);
+  std::optional<Transform>& currentTransform = transforms[currentMeshIndex];
 
-    for (const std::size_t childIndex : node.children) {
-      const fastgltf::Node& childNode = nodes[childIndex];
+  if (!currentTransform.has_value())
+    currentTransform = loadTransform(currentNode);
 
-      if (!childNode.meshIndex.has_value())
-        continue;
+  if (parentTransform.has_value()) {
+    currentTransform->setPosition(parentTransform->getPosition() + parentTransform->getRotation() * (currentTransform->getPosition() * parentTransform->getScale()));
+    currentTransform->setRotation((parentTransform->getRotation() * currentTransform->getRotation()).normalize());
+    currentTransform->scale(parentTransform->getScale());
+  }
 
-      const std::size_t childNodeMeshIndex = *childNode.meshIndex;
+  for (const std::size_t childIndex : currentNode.children)
+    computeNodeTransform(nodes[childIndex], currentTransform, nodes, transforms);
+}
 
-      if (childNodeMeshIndex >= meshNodeCount) {
-        Logger::error("[GltfLoad] Unexpected child node mesh index.");
-        continue;
-      }
+std::vector<std::optional<Transform>> loadTransforms(const std::vector<fastgltf::Node>& nodes, std::size_t meshCount) {
+  std::vector<std::optional<Transform>> transforms;
+  transforms.resize(meshCount);
 
-      std::optional<Transform>& childTransform = transforms[childNodeMeshIndex];
-
-      if (!childTransform.has_value())
-        childTransform = loadTransform(childNode);
-
-      // TODO: this will not be enough for configurations (perhaps a bit esoteric) that have children nodes referenced before their parents
-      //   For example, take the following configuration, where the nodes are taken from the file in the order { A, B, C }:
-      //   B -> A -> C
-      //   We first load A's transform matrix which is relative to B's, its parent, then apply it to its children (here C), themselves in their
-      //     own local transform (that we load at this moment, although it's not relevant here). Then we load B, the root, apply its transform to
-      //     its *direct* children (here A), but *not recursively*. C ends up in a local space made up from its own transform and its *direct* parent's,
-      //     not from those all the way up to the root
-      childTransform->setPosition(nodeTransform->getPosition() + nodeTransform->getRotation() * (childTransform->getPosition() * nodeTransform->getScale()));
-      childTransform->setRotation(nodeTransform->getRotation() * childTransform->getRotation());
-      childTransform->scale(nodeTransform->getScale());
-    }
-  }
+  for (const fastgltf::Node& node : nodes)
+    computeNodeTransform(node, std::nullopt, nodes, transforms);
 
   return transforms;
 }

diff --git a/tests/src/RaZ/Data/GltfFormat.cpp b/tests/src/RaZ/Data/GltfFormat.cpp
@@ -19,12 +19,10 @@ TEST_CASE("GltfFormat load glTF") {
 
   // Meshes are chained in the order 2 -> 1 -> 0, hence applying each of its parents' transforms before its own
 
-  // TODO: this first submesh is wrong, as it does not take the global transform (the root's, here the third submesh's) into account. See the
-  //   related comment in the glTF's transforms loading function
-  CHECK(mesh.getSubmeshes()[0].getVertices()[0]  == Raz::Vertex{ Raz::Vec3f(10.15f, 10.15f, 9.85f), Raz::Vec2f(0.f, 0.f), -Raz::Axis::Z, -Raz::Axis::Y });
-  CHECK(mesh.getSubmeshes()[0].getVertices()[1]  == Raz::Vertex{ Raz::Vec3f(10.05f, 10.05f, 9.85f), Raz::Vec2f(0.f, 1.f), -Raz::Axis::Z, -Raz::Axis::Y });
-  CHECK(mesh.getSubmeshes()[0].getVertices()[17] == Raz::Vertex{ Raz::Vec3f(10.05f, 10.15f, 9.95f), Raz::Vec2f(0.f, 1.f), -Raz::Axis::X, -Raz::Axis::Y });
-  CHECK(mesh.getSubmeshes()[0].getVertices()[35] == Raz::Vertex{ Raz::Vec3f(10.05f, 10.05f, 9.95f), Raz::Vec2f(0.f, 1.f), -Raz::Axis::X, -Raz::Axis::Y });
+  CHECK(mesh.getSubmeshes()[0].getVertices()[0]  == Raz::Vertex{ Raz::Vec3f(121.97f, 82.03f, 122.03f), Raz::Vec2f(0.f, 0.f), -Raz::Axis::X, -Raz::Axis::Z });
+  CHECK(mesh.getSubmeshes()[0].getVertices()[1]  == Raz::Vertex{ Raz::Vec3f(121.97f, 82.01f, 122.01f), Raz::Vec2f(0.f, 1.f), -Raz::Axis::X, -Raz::Axis::Z });
+  CHECK(mesh.getSubmeshes()[0].getVertices()[17] == Raz::Vertex{ Raz::Vec3f(121.99f, 82.01f, 122.03f), Raz::Vec2f(0.f, 1.f), -Raz::Axis::Y, -Raz::Axis::Z });
+  CHECK(mesh.getSubmeshes()[0].getVertices()[35] == Raz::Vertex{ Raz::Vec3f(121.99f, 82.01f, 122.01f), Raz::Vec2f(0.f, 1.f), -Raz::Axis::Y, -Raz::Axis::Z });
 
   CHECK(mesh.getSubmeshes()[1].getVertices()[0]  == Raz::Vertex{ Raz::Vec3f(120.2f, 80.2f, 119.8f), Raz::Vec2f(0.f, 0.f), -Raz::Axis::Z, -Raz::Axis::Y });
   CHECK(mesh.getSubmeshes()[1].getVertices()[1]  == Raz::Vertex{ Raz::Vec3f(119.8f, 79.8f, 119.8f), Raz::Vec2f(0.f, 1.f), -Raz::Axis::Z, -Raz::Axis::Y });