From 643382dfd364a193686201e1c82b6fe7f351f068 Mon Sep 17 00:00:00 2001
From: 3gg <3gg@shellblade.net>
Date: Sat, 25 Oct 2025 15:38:47 -0700
Subject: Simplify node construction

---
 include/gfx/scene/node.h |  15 -----
 src/asset/model.c        | 161 ++++++++++++++++++++++++-----------------------
 src/scene/node.c         |  47 --------------
 3 files changed, 81 insertions(+), 142 deletions(-)

diff --git a/include/gfx/scene/node.h b/include/gfx/scene/node.h
index 193eb25..20b97e2 100644
--- a/include/gfx/scene/node.h
+++ b/include/gfx/scene/node.h
@@ -50,21 +50,6 @@ SceneNode* gfx_make_model_node(Model*);
 /// Create a new object node.
 SceneNode* gfx_make_object_node(SceneObject*);
 
-/// Make the node an anima node.
-void gfx_construct_anima_node(SceneNode*, Anima*);
-
-/// Make the node a camera node.
-void gfx_construct_camera_node(SceneNode*, SceneCamera*);
-
-/// Make the node a light node.
-void gfx_construct_light_node(SceneNode*, Light*);
-
-/// Make the node a model node.
-void gfx_construct_model_node(SceneNode*, Model*);
-
-/// Make the node an object node.
-void gfx_construct_object_node(SceneNode*, SceneObject*);
-
 /// Recursively destroy the scene node and its children.
 ///
 /// The scene node and its children are removed from the scene graph.
diff --git a/src/asset/model.c b/src/asset/model.c
index 1e05a85..ccda27d 100644
--- a/src/asset/model.c
+++ b/src/asset/model.c
@@ -1489,6 +1489,66 @@ static void load_animations(
   }
 }
 
+/// Remove joint nodes from the Gfx Scene.
+///
+/// Joint nodes are not needed because joints are packed into the Anima.
+static void remove_joint_nodes(
+    const cgltf_data* data, SceneNode** scene_nodes) {
+  assert(data);
+  assert(scene_nodes);
+
+  // This works assuming the joint nodes are contiguous. Contiguity is checked
+  // when loading skins. See load_skins().
+  size_t min_joint_index = (size_t)-1;
+  size_t max_joint_index = 0;
+
+  // First get the minimum and maximum indices of all joint nodes.
+  for (cgltf_size s = 0; s < data->skins_count; ++s) {
+    const cgltf_skin* skin = &data->skins[s];
+
+    for (cgltf_size j = 0; j < skin->joints_count; ++j) {
+      // Joint is an index/pointer into the nodes array.
+      const cgltf_size joint_index = skin->joints[j] - data->nodes;
+      assert(joint_index < data->nodes_count);
+
+      if (joint_index < min_joint_index) {
+        min_joint_index = joint_index;
+      }
+      if (joint_index > max_joint_index) {
+        max_joint_index = joint_index;
+      }
+    }
+  }
+
+  assert(min_joint_index < data->nodes_count);
+  assert(max_joint_index < data->nodes_count);
+
+  // Now walk over the joint nodes. If a joint's parent is itself not a joint
+  // node, then that joint is a root of a joint hierarchy (skins in glTF may
+  // have multiple roots). In such case, delete the root joint recursively.
+  for (cgltf_size s = 0; s < data->skins_count; ++s) {
+    const cgltf_skin* skin = &data->skins[s];
+
+    for (cgltf_size j = 0; j < skin->joints_count; ++j) {
+      // Joint is an index/pointer into the nodes array.
+      const cgltf_size joint_index = skin->joints[j] - data->nodes;
+      assert(joint_index < data->nodes_count);
+
+      const cgltf_node* joint = &data->nodes[joint_index];
+
+      // Parent node index.
+      const cgltf_size parent_index = joint->parent - data->nodes;
+      assert(parent_index < data->nodes_count);
+
+      // If the parent is not a joint node, recursively delete this joint node.
+      if ((parent_index < min_joint_index) ||
+          (parent_index > max_joint_index)) {
+        gfx_destroy_node(&scene_nodes[joint_index]);
+      }
+    }
+  }
+}
+
 /// Load all nodes from the glTF scene.
 ///
 /// This function ignores the many scenes and default scene of the glTF spec
@@ -1497,7 +1557,7 @@ static void load_nodes(
     const cgltf_data* data, SceneNode* root_node, SceneObject** objects,
     SceneCamera** cameras, const Anima* anima, SceneNode** nodes) {
   // Note that with glTF 2.0, nodes do not form a DAG / scene graph but a
-  // disjount union of strict trees:
+  // disjoint union of strict trees:
   //
   // "For Version 2.0 conformance, the glTF node hierarchy is not a directed
   // acyclic graph (DAG) or scene graph, but a disjoint union of strict trees.
@@ -1515,20 +1575,22 @@ static void load_nodes(
 
   cgltf_size next_camera = 0;
 
+  // First pass: create the nodes.
   for (cgltf_size n = 0; n < data->nodes_count; ++n) {
     const cgltf_node* node = &data->nodes[n];
 
     // Add SceneObject, SceneCamera or Lights.
     // TODO: Handle lights once they are implemented in the gfx library.
+    assert(!nodes[n]);
     if (node->mesh) {
       const cgltf_size mesh_index = node->mesh - data->meshes;
       assert(mesh_index < data->meshes_count);
       SceneObject* object = objects[mesh_index];
-      gfx_construct_object_node(nodes[n], object);
+
+      nodes[n] = gfx_make_object_node(object);
 
       if (node->skin) {
         assert(anima);
-
         const cgltf_size skin_index = node->skin - data->skins;
         assert(skin_index < data->skins_count);
         const Skeleton* skeleton = gfx_get_anima_skeleton(anima, skin_index);
@@ -1559,11 +1621,12 @@ static void load_nodes(
       }
 
       gfx_set_camera_camera(cameras[next_camera], &camera);
-      gfx_construct_camera_node(nodes[n], cameras[next_camera]);
+      nodes[n] = gfx_make_camera_node(cameras[next_camera]);
       ++next_camera;
     } else {
       // TODO: implementation for missing node types.
       // These nodes currently default to logical nodes.
+      nodes[n] = gfx_make_node();
     }
     assert(nodes[n]);
 
@@ -1589,6 +1652,11 @@ static void load_nodes(
       }
     }
     gfx_set_node_transform(nodes[n], &transform);
+  }
+
+  // Second pass: wire up the node hierarchy.
+  for (cgltf_size n = 0; n < data->nodes_count; ++n) {
+    const cgltf_node* node = &data->nodes[n];
 
     // If this is a top-level node in the glTF scene, set its parent to the
     // given root node.
@@ -1601,66 +1669,12 @@ static void load_nodes(
       assert(parent);
       gfx_set_node_parent(nodes[n], parent);
     }
-  } // SceneNode.
-}
-
-/// Remove joint nodes from the Gfx Scene.
-///
-/// Joint nodes are not needed because joints are packed into the Anima.
-static void remove_joint_nodes(
-    const cgltf_data* data, SceneNode** scene_nodes) {
-  assert(data);
-  assert(scene_nodes);
-
-  // This works assuming the joint nodes are contiguous. Contiguity is checked
-  // when loading skins. See load_skins().
-  size_t min_joint_index = (size_t)-1;
-  size_t max_joint_index = 0;
-
-  // First get the minimum and maximum indices of all joint nodes.
-  for (cgltf_size s = 0; s < data->skins_count; ++s) {
-    const cgltf_skin* skin = &data->skins[s];
-
-    for (cgltf_size j = 0; j < skin->joints_count; ++j) {
-      // Joint is an index/pointer into the nodes array.
-      const cgltf_size joint_index = skin->joints[j] - data->nodes;
-      assert(joint_index < data->nodes_count);
-
-      if (joint_index < min_joint_index) {
-        min_joint_index = joint_index;
-      }
-      if (joint_index > max_joint_index) {
-        max_joint_index = joint_index;
-      }
-    }
   }
 
-  assert(min_joint_index < data->nodes_count);
-  assert(max_joint_index < data->nodes_count);
-
-  // Now walk over the joint nodes. If a joint's parent is itself not a joint
-  // node, then that joint is a root of a joint hierarchy (skins in glTF may
-  // have multiple roots). In such case, delete the root joint recursively.
-  for (cgltf_size s = 0; s < data->skins_count; ++s) {
-    const cgltf_skin* skin = &data->skins[s];
-
-    for (cgltf_size j = 0; j < skin->joints_count; ++j) {
-      // Joint is an index/pointer into the nodes array.
-      const cgltf_size joint_index = skin->joints[j] - data->nodes;
-      assert(joint_index < data->nodes_count);
-
-      const cgltf_node* joint = &data->nodes[joint_index];
-
-      // Parent node index.
-      const cgltf_size parent_index = joint->parent - data->nodes;
-      assert(parent_index < data->nodes_count);
-
-      // If the parent is not a joint node, recursively delete this joint node.
-      if ((parent_index < min_joint_index) ||
-          (parent_index > max_joint_index)) {
-        gfx_destroy_node(&scene_nodes[joint_index]);
-      }
-    }
+  // Clean up scene nodes that correspond to joints in the glTF. These are not
+  // needed anymore.
+  if (data->skins_count > 0) {
+    remove_joint_nodes(data, nodes);
   }
 }
 
@@ -1768,19 +1782,9 @@ static Model* load_scene(
     goto cleanup;
   }
 
-  // Skins refer to nodes, and nodes may refer to skins. To break this circular
-  // dependency, glTF defines skins in terms of node indices. We could do the
-  // same if Gfx allowed allocating nodes contiguously in memory. For now,
-  // create the nodes up front and use the indices of the array to map to the
-  // node_idx.
-  for (cgltf_size i = 0; i < data->nodes_count; ++i) {
-    scene_nodes[i] = gfx_make_node();
-  }
-
   // Create the scene's root node.
   // This is an anima node if the scene has skins; otherwise it is a logical
   // node.
-  root_node = gfx_make_node();
   if (data->skins_count > 0) {
     anima_desc = calloc(1, sizeof(AnimaDesc));
     if (!anima_desc) {
@@ -1797,19 +1801,16 @@ static Model* load_scene(
     compute_joint_bounding_boxes(
         data, anima_desc->num_joints, anima_desc->joints);
 
-    anima = gfx_make_anima(anima_desc);
-    gfx_construct_anima_node(root_node, anima);
+    anima     = gfx_make_anima(anima_desc);
+    root_node = gfx_make_anima_node(anima);
+  } else {
+    root_node = gfx_make_node();
   }
+  assert(root_node);
 
   // The root node becomes the root of all scene nodes.
   load_nodes(data, root_node, scene_objects, scene_cameras, anima, scene_nodes);
 
-  // Clean up scene nodes that correspond to joints in the glTF. These are
-  // not needed anymore.
-  if (data->skins_count > 0) {
-    remove_joint_nodes(data, scene_nodes);
-  }
-
   model = gfx_make_model(root_node);
 
   success = true;
diff --git a/src/scene/node.c b/src/scene/node.c
index c5c8d94..7cc315c 100644
--- a/src/scene/node.c
+++ b/src/scene/node.c
@@ -110,53 +110,6 @@ static void free_node_resource(SceneNode* node) {
   FAIL("unhandled node type");
 }
 
-void gfx_construct_anima_node(SceneNode* node, Anima* anima) {
-  assert(node);
-  assert(anima);
-  free_node_resource(node);
-  node->type    = AnimaNode;
-  node->anima   = mem_get_anima_index(anima);
-  anima->parent = mem_get_node_index(node);
-}
-
-void gfx_construct_camera_node(SceneNode* node, SceneCamera* camera) {
-  assert(node);
-  assert(camera);
-  free_node_resource(node);
-  node->type     = CameraNode;
-  node->camera   = mem_get_camera_index(camera);
-  camera->parent = mem_get_node_index(node);
-}
-
-// TODO: Add a common helper function between each gfx_make_xyz_node() and
-//  gfx_construct_xyz_node() pair.
-void gfx_construct_light_node(SceneNode* node, Light* light) {
-  assert(node);
-  assert(light);
-  free_node_resource(node);
-  node->type    = LightNode;
-  node->light   = mem_get_light_index(light);
-  light->parent = mem_get_node_index(node);
-}
-
-void gfx_construct_model_node(SceneNode* node, Model* model) {
-  assert(node);
-  assert(model);
-  free_node_resource(node);
-  node->type    = ModelNode;
-  node->model   = mem_get_model_index(model);
-  model->parent = mem_get_node_index(node);
-}
-
-void gfx_construct_object_node(SceneNode* node, SceneObject* object) {
-  assert(node);
-  assert(object);
-  free_node_resource(node);
-  node->type     = ObjectNode;
-  node->object   = mem_get_object_index(object);
-  object->parent = mem_get_node_index(node);
-}
-
 static void destroy_node_rec(SceneNode* node) {
   assert(node);
 
-- 
cgit v1.2.3