Skip to content

Commit 76ccf48

Browse files
schellm0ppers
andcommitted
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 <andreas@mop.koeln>
1 parent 956f53d commit 76ccf48

4 files changed

Lines changed: 52 additions & 36 deletions

File tree

crates/example/src/lib.rs

Lines changed: 1 addition & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -286,29 +286,8 @@ impl App {
286286

287287
let scene = doc.default_scene.unwrap_or(0);
288288
log::info!("Displaying scene {scene}");
289-
fn get_children(doc: &GltfDocument, n: usize) -> Vec<usize> {
290-
let mut children = vec![];
291-
if let Some(parent) = doc.nodes.get(n) {
292-
children.extend(parent.children.iter().copied());
293-
let descendants = parent
294-
.children
295-
.iter()
296-
.copied()
297-
.flat_map(|n| get_children(doc, n));
298-
children.extend(descendants);
299-
}
300-
children
301-
}
302289

303-
let nodes = doc.nodes_in_scene(scene).flat_map(|n| {
304-
let mut all_nodes = vec![n];
305-
for child_index in get_children(&doc, n.index) {
306-
if let Some(child_node) = doc.nodes.get(child_index) {
307-
all_nodes.push(child_node);
308-
}
309-
}
310-
all_nodes
311-
});
290+
let nodes = doc.recursive_nodes_in_scene(scene);
312291
log::trace!(" nodes:");
313292
for node in nodes {
314293
let tfrm = Mat4::from(node.global_transform());

crates/renderling/src/gltf.rs

Lines changed: 38 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@
22
//!
33
//! # Loading GLTF files
44
//!
5-
//! Loading GLTF files is accomplished through [`Stage::load_gltf_document_from_path`]
5+
//! Loading GLTF files is accomplished through
6+
//! [`Stage::load_gltf_document_from_path`]
67
//! and [`Stage::load_gltf_document_from_bytes`].
78
use std::{collections::HashMap, sync::Arc};
89

@@ -458,8 +459,8 @@ impl GltfPrimitive {
458459
// https://registry.khronos.org/glTF/specs/2.0/glTF-2.0.html#morph-targets
459460
//
460461
// TODO: Generate morph target normals and tangents if absent.
461-
// Although the spec says we have to generate normals or tangents if not specified,
462-
// we are explicitly *not* doing that here.
462+
// Although the spec says we have to generate normals or tangents if not
463+
// specified, we are explicitly *not* doing that here.
463464
let morph_targets: Vec<Vec<MorphTarget>> = reader
464465
.read_morph_targets()
465466
.map(|(may_ps, may_ns, may_ts)| {
@@ -1236,29 +1237,56 @@ where
12361237
self.primitives.iter().flat_map(|(_, rs)| rs.iter())
12371238
}
12381239

1239-
fn nodes_in_scene_recursive<'a>(&'a self, node_index: usize, nodes: &mut Vec<&'a GltfNode>) {
1240+
fn collect_nodes_recursive<'a>(&'a self, node_index: usize, nodes: &mut Vec<&'a GltfNode>) {
12401241
if let Some(node) = self.nodes.get(node_index) {
12411242
nodes.push(node);
12421243
for child_index in node.children.iter() {
1243-
self.nodes_in_scene_recursive(*child_index, nodes);
1244+
self.collect_nodes_recursive(*child_index, nodes);
12441245
}
12451246
}
12461247
}
12471248

1248-
pub fn nodes_in_scene(&self, scene_index: usize) -> impl Iterator<Item = &GltfNode> {
1249+
/// Returns the root (top-level) nodes in the given scene.
1250+
///
1251+
/// This roughly follows [`gltf::Scene::nodes`](https://docs.rs/gltf/latest/gltf/scene/struct.Scene.html#method.nodes),
1252+
/// returning only the nodes directly referenced by the scene — not
1253+
/// their children.
1254+
///
1255+
/// Use [`recursive_nodes_in_scene`](Self::recursive_nodes_in_scene)
1256+
/// if you need all nodes (including descendants).
1257+
pub fn root_nodes_in_scene(&self, scene_index: usize) -> impl Iterator<Item = &GltfNode> {
1258+
let scene = self.scenes.get(scene_index);
1259+
let mut nodes = vec![];
1260+
if let Some(indices) = scene {
1261+
for node_index in indices {
1262+
if let Some(node) = self.nodes.get(*node_index) {
1263+
nodes.push(node);
1264+
}
1265+
}
1266+
}
1267+
nodes.into_iter()
1268+
}
1269+
1270+
/// Returns all nodes in the given scene, recursively including
1271+
/// children.
1272+
///
1273+
/// Root nodes are visited first, followed by their descendants in
1274+
/// depth-first order.
1275+
pub fn recursive_nodes_in_scene(&self, scene_index: usize) -> impl Iterator<Item = &GltfNode> {
12491276
let scene = self.scenes.get(scene_index);
12501277
let mut nodes = vec![];
12511278
if let Some(indices) = scene {
12521279
for node_index in indices {
1253-
self.nodes_in_scene_recursive(*node_index, &mut nodes);
1280+
self.collect_nodes_recursive(*node_index, &mut nodes);
12541281
}
12551282
}
12561283
nodes.into_iter()
12571284
}
12581285

12591286
/// Returns the bounding volume of this document, if possible.
12601287
///
1261-
/// This function will return `None` if this document does not contain meshes.
1288+
/// This function will return `None` if this document does not contain
1289+
/// meshes.
12621290
pub fn bounding_volume(&self) -> Option<Aabb> {
12631291
let mut aabbs = vec![];
12641292
for node in self.nodes.iter() {
@@ -1512,8 +1540,8 @@ mod test {
15121540
// .get(0)
15131541
// .unwrap()
15141542
// .clone()
1515-
// .into_animator(doc.nodes.iter().map(|n| (n.index, n.transform.clone())));
1516-
// animator.progress(0.0).unwrap();
1543+
// .into_animator(doc.nodes.iter().map(|n| (n.index,
1544+
// n.transform.clone()))); animator.progress(0.0).unwrap();
15171545
// let frame = ctx.get_next_frame().unwrap();
15181546
// stage.render(&frame.view());
15191547
// let img = frame.read_image().unwrap();

crates/renderling/src/gltf/anime.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -746,7 +746,10 @@ impl Animator {
746746
}
747747
TweenProperty::MorphTargetWeights(new_weights) => {
748748
if node.morph_weights.array().is_empty() {
749-
log::error!("animation is applied to morph targets but node {node_index} is missing weights");
749+
log::error!(
750+
"animation is applied to morph targets but node {node_index} is \
751+
missing weights"
752+
);
750753
} else {
751754
for (i, w) in new_weights.into_iter().enumerate() {
752755
node.morph_weights.set_item(i, w);
@@ -785,7 +788,7 @@ mod test {
785788
.unwrap();
786789

787790
let nodes = doc
788-
.nodes_in_scene(doc.default_scene.unwrap_or_default())
791+
.recursive_nodes_in_scene(doc.default_scene.unwrap_or_default())
789792
.collect::<Vec<_>>();
790793

791794
let mut animator = Animator::new(nodes, doc.animations.first().unwrap().clone());

crates/renderling/src/linkage/light_tiling_compute_tile_min_and_max_depth_multisampled.rs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,10 @@ mod target {
66
pub const ENTRY_POINT: &str =
77
"light::shader::light_tiling_compute_tile_min_and_max_depth_multisampled";
88
pub fn descriptor() -> wgpu::ShaderModuleDescriptor<'static> {
9-
wgpu :: include_spirv ! ("../../shaders/light-shader-light_tiling_compute_tile_min_and_max_depth_multisampled.spv")
9+
wgpu::include_spirv!(
10+
"../../shaders/light-shader-light_tiling_compute_tile_min_and_max_depth_multisampled.\
11+
spv"
12+
)
1013
}
1114
pub fn linkage(device: &wgpu::Device) -> super::ShaderLinkage {
1215
log::debug!(
@@ -24,7 +27,10 @@ mod target {
2427
pub const ENTRY_POINT: &str =
2528
"lightshaderlight_tiling_compute_tile_min_and_max_depth_multisampled";
2629
pub fn descriptor() -> wgpu::ShaderModuleDescriptor<'static> {
27-
wgpu :: include_wgsl ! ("../../shaders/light-shader-light_tiling_compute_tile_min_and_max_depth_multisampled.wgsl")
30+
wgpu::include_wgsl!(
31+
"../../shaders/light-shader-light_tiling_compute_tile_min_and_max_depth_multisampled.\
32+
wgsl"
33+
)
2834
}
2935
pub fn linkage(device: &wgpu::Device) -> super::ShaderLinkage {
3036
log::debug!(

0 commit comments

Comments
 (0)