From bc21fb2cb2b51273ef4deda4c0715f8faeb7a215 Mon Sep 17 00:00:00 2001 From: Andreas Streichardt Date: Sat, 20 Dec 2025 07:28:17 +0100 Subject: [PATCH 1/8] =?UTF-8?q?=E2=9C=A8=20collect=20scene=20nodes=20recur?= =?UTF-8?q?sively?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- crates/renderling/src/gltf.rs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/crates/renderling/src/gltf.rs b/crates/renderling/src/gltf.rs index 6e108e55..479446c4 100644 --- a/crates/renderling/src/gltf.rs +++ b/crates/renderling/src/gltf.rs @@ -1237,14 +1237,21 @@ where self.primitives.iter().flat_map(|(_, rs)| rs.iter()) } + fn nodes_in_scene_recursive<'a>(&'a self, node_index: usize, nodes: &mut Vec<&'a GltfNode>) { + if let Some(node) = self.nodes.get(node_index) { + nodes.push(node); + for child_index in node.children.iter() { + self.nodes_in_scene_recursive(*child_index, nodes); + } + } + } + pub fn nodes_in_scene(&self, scene_index: usize) -> impl Iterator { let scene = self.scenes.get(scene_index); let mut nodes = vec![]; if let Some(indices) = scene { for node_index in indices { - if let Some(node) = self.nodes.get(*node_index) { - nodes.push(node); - } + self.nodes_in_scene_recursive(*node_index, &mut nodes); } } nodes.into_iter() From 79c3d964b5c81e84a098e834b3843526bf628e17 Mon Sep 17 00:00:00 2001 From: Schell Carl Scivally Date: Wed, 18 Mar 2026 17:18:07 +1300 Subject: [PATCH 2/8] Rename nodes_in_scene to root_nodes_in_scene, add recursive_nodes_in_scene Split the scene node query API into two explicit methods: - root_nodes_in_scene: returns only the top-level nodes directly referenced by the scene (the original behavior) - recursive_nodes_in_scene: returns all nodes including descendants in depth-first order This fixes a bug where animations targeting child (parented) nodes were silently skipped because only root nodes were collected. Update call sites: - Animator test now uses recursive_nodes_in_scene for correctness - Example crate simplified by removing manual get_children helper Co-authored-by: Andreas Streichardt --- crates/example/src/lib.rs | 23 +------------------ crates/renderling/src/gltf.rs | 34 +++++++++++++++++++++++++---- crates/renderling/src/gltf/anime.rs | 2 +- 3 files changed, 32 insertions(+), 27 deletions(-) diff --git a/crates/example/src/lib.rs b/crates/example/src/lib.rs index 7f6867c3..36d4ebf6 100644 --- a/crates/example/src/lib.rs +++ b/crates/example/src/lib.rs @@ -286,29 +286,8 @@ impl App { let scene = doc.default_scene.unwrap_or(0); log::info!("Displaying scene {scene}"); - fn get_children(doc: &GltfDocument, n: usize) -> Vec { - let mut children = vec![]; - if let Some(parent) = doc.nodes.get(n) { - children.extend(parent.children.iter().copied()); - let descendants = parent - .children - .iter() - .copied() - .flat_map(|n| get_children(doc, n)); - children.extend(descendants); - } - children - } - let nodes = doc.nodes_in_scene(scene).flat_map(|n| { - let mut all_nodes = vec![n]; - for child_index in get_children(&doc, n.index) { - if let Some(child_node) = doc.nodes.get(child_index) { - all_nodes.push(child_node); - } - } - all_nodes - }); + let nodes = doc.recursive_nodes_in_scene(scene); log::trace!(" nodes:"); for node in nodes { let tfrm = Mat4::from(node.global_transform()); diff --git a/crates/renderling/src/gltf.rs b/crates/renderling/src/gltf.rs index 479446c4..2d9706ee 100644 --- a/crates/renderling/src/gltf.rs +++ b/crates/renderling/src/gltf.rs @@ -1237,21 +1237,47 @@ where self.primitives.iter().flat_map(|(_, rs)| rs.iter()) } - fn nodes_in_scene_recursive<'a>(&'a self, node_index: usize, nodes: &mut Vec<&'a GltfNode>) { + fn collect_nodes_recursive<'a>(&'a self, node_index: usize, nodes: &mut Vec<&'a GltfNode>) { if let Some(node) = self.nodes.get(node_index) { nodes.push(node); for child_index in node.children.iter() { - self.nodes_in_scene_recursive(*child_index, nodes); + self.collect_nodes_recursive(*child_index, nodes); } } } - pub fn nodes_in_scene(&self, scene_index: usize) -> impl Iterator { + /// Returns the root (top-level) nodes in the given scene. + /// + /// This roughly follows [`gltf::Scene::nodes`](https://docs.rs/gltf/latest/gltf/scene/struct.Scene.html#method.nodes), + /// returning only the nodes directly referenced by the scene — not + /// their children. + /// + /// Use [`recursive_nodes_in_scene`](Self::recursive_nodes_in_scene) + /// if you need all nodes (including descendants). + pub fn root_nodes_in_scene(&self, scene_index: usize) -> impl Iterator { + let scene = self.scenes.get(scene_index); + let mut nodes = vec![]; + if let Some(indices) = scene { + for node_index in indices { + if let Some(node) = self.nodes.get(*node_index) { + nodes.push(node); + } + } + } + nodes.into_iter() + } + + /// Returns all nodes in the given scene, recursively including + /// children. + /// + /// Root nodes are visited first, followed by their descendants in + /// depth-first order. + pub fn recursive_nodes_in_scene(&self, scene_index: usize) -> impl Iterator { let scene = self.scenes.get(scene_index); let mut nodes = vec![]; if let Some(indices) = scene { for node_index in indices { - self.nodes_in_scene_recursive(*node_index, &mut nodes); + self.collect_nodes_recursive(*node_index, &mut nodes); } } nodes.into_iter() diff --git a/crates/renderling/src/gltf/anime.rs b/crates/renderling/src/gltf/anime.rs index ddaf90e4..939f8688 100644 --- a/crates/renderling/src/gltf/anime.rs +++ b/crates/renderling/src/gltf/anime.rs @@ -787,7 +787,7 @@ mod test { .unwrap(); let nodes = doc - .nodes_in_scene(doc.default_scene.unwrap_or_default()) + .recursive_nodes_in_scene(doc.default_scene.unwrap_or_default()) .collect::>(); let mut animator = Animator::new(nodes, doc.animations.first().unwrap().clone()); From 65eb48d12de1f106e549ed6afa0398a0b9aab018 Mon Sep 17 00:00:00 2001 From: Schell Carl Scivally Date: Sat, 21 Mar 2026 10:57:14 +1300 Subject: [PATCH 3/8] fix cache --- .github/workflows/push.yaml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/workflows/push.yaml b/.github/workflows/push.yaml index dbcc05f2..7f9f31ea 100644 --- a/.github/workflows/push.yaml +++ b/.github/workflows/push.yaml @@ -65,6 +65,10 @@ jobs: RUST_LOG: debug steps: - uses: actions/checkout@v2 + # Run moonrepo/setup-rust BEFORE restoring ~/.cargo cache, because + # moonrepo restores its own ~/.cargo cache which would overwrite + # the cargo-gpu binary installed by the install-cargo-gpu job. + - uses: moonrepo/setup-rust@v1 - uses: actions/cache@v4 with: path: ~/.cargo @@ -77,7 +81,6 @@ jobs: ${{ needs.install-cargo-gpu.outputs.cachepath-Linux }} ${{ needs.install-cargo-gpu.outputs.cachepath-Windows }} key: rust-gpu-cache-0-${{ runner.os }} - - uses: moonrepo/setup-rust@v1 - run: rustup install nightly - run: rustup component add --toolchain nightly rustfmt - run: cargo gpu show commitsh From b78c0705521b4c04d1600a794be8096203b7311a Mon Sep 17 00:00:00 2001 From: Schell Carl Scivally Date: Sat, 21 Mar 2026 13:05:31 +1300 Subject: [PATCH 4/8] use matrix.os instead of runner.os --- .github/workflows/push.yaml | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/push.yaml b/.github/workflows/push.yaml index 7f9f31ea..ff180947 100644 --- a/.github/workflows/push.yaml +++ b/.github/workflows/push.yaml @@ -35,7 +35,7 @@ jobs: with: path: ~/.cargo # THIS KEY MUST MATCH BELOW - key: cargo-cache-${{ env.CARGO_GPU_COMMITSH }}-${{ runner.os }} + key: cargo-cache-${{ env.CARGO_GPU_COMMITSH }}-${{ matrix.os }} - uses: moonrepo/setup-rust@v1 - run: rustup default stable - run: rustup update @@ -73,7 +73,7 @@ jobs: with: path: ~/.cargo # THIS KEY MUST MATCH ABOVE - key: cargo-cache-${{ env.CARGO_GPU_COMMITSH }}-${{ runner.os }} + key: cargo-cache-${{ env.CARGO_GPU_COMMITSH }}-${{ matrix.os }} - uses: actions/cache@v4 with: path: | @@ -99,7 +99,7 @@ jobs: with: path: ~/.cargo # THIS KEY MUST MATCH ABOVE - key: cargo-cache-${{ env.CARGO_GPU_COMMITSH }}-${{ runner.os }} + key: cargo-cache-${{ env.CARGO_GPU_COMMITSH }}-${{ matrix.os }} - uses: moonrepo/setup-rust@v1 - run: rustup install nightly - run: rustup component add --toolchain nightly rustfmt @@ -133,8 +133,8 @@ jobs: - uses: actions/cache@v4 with: path: ~/.cargo - key: ${{ runner.os }}-test-cargo-${{ hashFiles('**/Cargo.lock') }} - restore-keys: ${{ runner.os }}-cargo- + key: ${{ matrix.os }}-test-cargo-${{ hashFiles('**/Cargo.lock') }} + restore-keys: ${{ matrix.os }}-cargo- - name: Install linux deps if: runner.os == 'Linux' @@ -153,7 +153,7 @@ jobs: - uses: actions/upload-artifact@v4 if: always() with: - name: test-output-${{ runner.os }} + name: test-output-${{ matrix.os }} path: test_output ## WASM tests, commented out until we can get a proper headless browser on CI From 8aba87028a396c210fd7c950164ca3cee1521967 Mon Sep 17 00:00:00 2001 From: Schell Carl Scivally Date: Sat, 21 Mar 2026 14:28:37 +1300 Subject: [PATCH 5/8] ensure .cargo exists on CI --- .github/workflows/push.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/push.yaml b/.github/workflows/push.yaml index ff180947..dc348418 100644 --- a/.github/workflows/push.yaml +++ b/.github/workflows/push.yaml @@ -100,6 +100,7 @@ jobs: path: ~/.cargo # THIS KEY MUST MATCH ABOVE key: cargo-cache-${{ env.CARGO_GPU_COMMITSH }}-${{ matrix.os }} + - run: mkdir -p ~/.cargo/bin - uses: moonrepo/setup-rust@v1 - run: rustup install nightly - run: rustup component add --toolchain nightly rustfmt From 6704fa04fbe55b9998b88c0f1df99e0a070995c9 Mon Sep 17 00:00:00 2001 From: Schell Carl Scivally Date: Sat, 21 Mar 2026 15:09:46 +1300 Subject: [PATCH 6/8] ensure .cargo exists on CI --- .github/workflows/push.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/push.yaml b/.github/workflows/push.yaml index dc348418..a9fdc26f 100644 --- a/.github/workflows/push.yaml +++ b/.github/workflows/push.yaml @@ -100,7 +100,7 @@ jobs: path: ~/.cargo # THIS KEY MUST MATCH ABOVE key: cargo-cache-${{ env.CARGO_GPU_COMMITSH }}-${{ matrix.os }} - - run: mkdir -p ~/.cargo/bin + - run: mkdir -p $HOME/.cargo/bin - uses: moonrepo/setup-rust@v1 - run: rustup install nightly - run: rustup component add --toolchain nightly rustfmt From 072c6c5959299120097594150d5b3c76844c55ef Mon Sep 17 00:00:00 2001 From: Schell Carl Scivally Date: Sat, 21 Mar 2026 15:27:06 +1300 Subject: [PATCH 7/8] ensure .cargo exists on CI --- .github/workflows/push.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/push.yaml b/.github/workflows/push.yaml index a9fdc26f..00be12b2 100644 --- a/.github/workflows/push.yaml +++ b/.github/workflows/push.yaml @@ -80,7 +80,7 @@ jobs: ${{ needs.install-cargo-gpu.outputs.cachepath-macOS }} ${{ needs.install-cargo-gpu.outputs.cachepath-Linux }} ${{ needs.install-cargo-gpu.outputs.cachepath-Windows }} - key: rust-gpu-cache-0-${{ runner.os }} + key: rust-gpu-cache-0-${{ matrix.os }} - run: rustup install nightly - run: rustup component add --toolchain nightly rustfmt - run: cargo gpu show commitsh From 9a177ecc644e6c8f139ce36d7c8bbf031fae04de Mon Sep 17 00:00:00 2001 From: Schell Carl Scivally Date: Sat, 21 Mar 2026 15:30:02 +1300 Subject: [PATCH 8/8] ensure .cargo exists on CI --- .github/workflows/push.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/push.yaml b/.github/workflows/push.yaml index 00be12b2..494f9daa 100644 --- a/.github/workflows/push.yaml +++ b/.github/workflows/push.yaml @@ -99,7 +99,7 @@ jobs: with: path: ~/.cargo # THIS KEY MUST MATCH ABOVE - key: cargo-cache-${{ env.CARGO_GPU_COMMITSH }}-${{ matrix.os }} + key: cargo-cache-${{ env.CARGO_GPU_COMMITSH }}-ubuntu-latest - run: mkdir -p $HOME/.cargo/bin - uses: moonrepo/setup-rust@v1 - run: rustup install nightly