From 6129cc912c87c63edb7756ed204dbe91ac62ee56 Mon Sep 17 00:00:00 2001 From: Maksym Pavlenko Date: Tue, 7 Apr 2026 11:59:10 -0700 Subject: [PATCH 1/5] Move composition logic from Stage to compose/ Signed-off-by: Maksym Pavlenko --- src/compose/mod.rs | 9 +- src/compose/prim_index.rs | 291 +++++++++++++++++++++++++++++++- src/sdf/mod.rs | 3 + src/stage.rs | 343 +------------------------------------- 4 files changed, 305 insertions(+), 341 deletions(-) diff --git a/src/compose/mod.rs b/src/compose/mod.rs index 825f3c1..e17684a 100644 --- a/src/compose/mod.rs +++ b/src/compose/mod.rs @@ -6,7 +6,6 @@ //! is a [`Vec`] of [`Layer`]s, each wrapping a parsed [`AbstractData`] with its resolved //! identity. Cycles are detected and skipped automatically. -#[allow(dead_code)] // TODO: Remove once all arc types are implemented. pub(crate) mod prim_index; use std::collections::{HashMap, HashSet}; @@ -17,7 +16,7 @@ use anyhow::{bail, Context, Result}; use crate::ar::{self, Resolver}; use crate::expr; use crate::sdf::schema::{ChildrenKey, FieldKey}; -use crate::sdf::{AbstractData, Path, Value}; +use crate::sdf::{AbstractData, LayerData, Path, Value}; use crate::{usda, usdc}; /// The kind of layer dependency that triggered a composition error. @@ -92,11 +91,11 @@ pub struct Layer { /// Resolved, canonical identifier for this layer. pub identifier: String, /// The parsed scene description data. - pub data: Box, + pub data: LayerData, } impl Layer { - fn new(identifier: impl Into, data: Box) -> Self { + fn new(identifier: impl Into, data: LayerData) -> Self { Self { identifier: identifier.into(), data, @@ -317,7 +316,7 @@ fn collect_prim_paths(data: &dyn AbstractData) -> Vec { /// Supports `.usda` (text), `.usdc` (binary), `.usd` (auto-detected via magic /// bytes), and `.usdz` (archive — reads the first layer). Returns the parsed /// data as a boxed [`AbstractData`]. -pub fn open_layer(resolver: &impl Resolver, resolved: &ar::ResolvedPath) -> Result> { +pub fn open_layer(resolver: &impl Resolver, resolved: &ar::ResolvedPath) -> Result { let ext = resolved.extension().and_then(|e| e.to_str()).unwrap_or_default(); if ext == "usdz" { diff --git a/src/compose/prim_index.rs b/src/compose/prim_index.rs index a9a9133..654fcfc 100644 --- a/src/compose/prim_index.rs +++ b/src/compose/prim_index.rs @@ -13,7 +13,12 @@ //! //! See -use crate::sdf::Path; +use std::collections::HashMap; + +use anyhow::Result; + +use crate::sdf::schema::FieldKey; +use crate::sdf::{AbstractData, LayerData, ListOp, Path, Payload, Reference, Value}; /// The type of composition arc that introduced a [`Node`]. #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -44,6 +49,7 @@ pub struct Node { /// The path within that layer (may differ from composed path due to remapping). pub path: Path, /// The composition arc that introduced this node. + #[allow(dead_code)] // Part of the public data model; read by tests and downstream users. pub arc: ArcType, } @@ -62,4 +68,287 @@ impl PrimIndex { pub fn is_empty(&self) -> bool { self.nodes.is_empty() } + + /// Builds a prim index for the given path across the layer stack. + /// + /// Follows LIVERPS ordering: + /// Local (sublayers) > Inherits > Variants > References > Payloads > Specializes. + pub(crate) fn build(path: &Path, layers: &[LayerData], identifiers: &[String]) -> Self { + let mut nodes = Vec::new(); + + // L — Root / sublayer opinions: check each layer in strength order. + for (i, layer) in layers.iter().enumerate() { + if layer.has_spec(path) { + nodes.push(Node { + layer_index: i, + path: path.clone(), + arc: ArcType::Root, + }); + } + } + + // I — Inherits: compose PathListOp across root nodes, then add nodes + // from the inherited prims within the same layer stack. + let inherits = compose_arc_list::(&nodes, FieldKey::InheritPaths, layers); + for inherit_path in &inherits { + for (i, layer) in layers.iter().enumerate() { + add_remapped_nodes(layer.as_ref(), i, path, inherit_path, ArcType::Inherit, &mut nodes); + } + } + + // V — Variants: resolve variant selection (first opinion wins), then + // add specs from the selected variant path in each layer. + let selections = resolve_variant_selections(&nodes, layers); + for (set_name, selection) in &selections { + let variant_path = path.append_variant_selection(set_name, selection); + for (i, layer) in layers.iter().enumerate() { + if layer.has_spec(&variant_path) { + nodes.push(Node { + layer_index: i, + path: variant_path.clone(), + arc: ArcType::Variant, + }); + } + } + } + + // R — References: compose ReferenceListOp across root nodes, then add + // nodes from referenced layers with namespace remapping. + let references = compose_arc_list::(&nodes, FieldKey::References, layers); + for reference in &references { + add_reference_nodes(path, reference, ArcType::Reference, &mut nodes, layers, identifiers); + } + + // P — Payloads: same as references but weaker. + let payloads = collect_payloads(&nodes, layers); + for payload in &payloads { + let reference = Reference { + asset_path: payload.asset_path.clone(), + prim_path: payload.prim_path.clone(), + ..Default::default() + }; + add_reference_nodes(path, &reference, ArcType::Payload, &mut nodes, layers, identifiers); + } + + // S — Specializes: same as inherits but weakest in LIVERPS. + let specializes = compose_arc_list::(&nodes, FieldKey::Specializes, layers); + for specialize_path in &specializes { + for (i, layer) in layers.iter().enumerate() { + add_remapped_nodes( + layer.as_ref(), + i, + path, + specialize_path, + ArcType::Specialize, + &mut nodes, + ); + } + } + + PrimIndex { nodes } + } + + /// Resolves a field by walking nodes from strongest to weakest, returning the first opinion. + /// + /// `make_path` maps each node to the path to query in that layer. + /// A [`Value::ValueBlock`] explicitly blocks opinions from weaker layers. + pub(crate) fn resolve_field( + &self, + field: &str, + layers: &[LayerData], + make_path: impl Fn(&Node) -> Result, + ) -> Result> { + for node in &self.nodes { + let query_path = make_path(node)?; + let data = &layers[node.layer_index]; + if !data.has_field(&query_path, field) { + continue; + } + + let value = data.get(&query_path, field)?; + if matches!(value.as_ref(), Value::ValueBlock) { + return Ok(None); + } + return Ok(Some(value.into_owned())); + } + + Ok(None) + } +} + +/// Resolves variant selections by walking root nodes strongest-to-weakest. +/// +/// For each variant set, the first opinion wins. +fn resolve_variant_selections(root_nodes: &[Node], layers: &[LayerData]) -> Vec<(String, String)> { + let mut selections: HashMap = HashMap::new(); + + for node in root_nodes { + let data = &layers[node.layer_index]; + let Ok(value) = data.get(&node.path, FieldKey::VariantSelection.as_str()) else { + continue; + }; + + if let Value::VariantSelectionMap(map) = value.into_owned() { + for (set_name, selection) in map { + // First opinion wins. + selections.entry(set_name).or_insert(selection); + } + } + } + + selections.into_iter().collect() +} + +/// Composes a list-op field across root nodes, returning the flattened list. +fn compose_arc_list( + root_nodes: &[Node], + field: FieldKey, + layers: &[LayerData], +) -> Vec +where + Value: TryInto>, +{ + let field = field.as_str(); + let mut result: Vec = Vec::new(); + + // Walk from weakest to strongest, composing each ListOp on top. + for node in root_nodes.iter().rev() { + let data = &layers[node.layer_index]; + let Ok(value) = data.get(&node.path, field) else { + continue; + }; + + let Ok(list_op) = value.into_owned().try_into() else { + continue; + }; + + result = list_op.compose_over(&result); + } + + result +} + +/// Collects payloads from root nodes, handling both single `Payload` and `PayloadListOp`. +fn collect_payloads(root_nodes: &[Node], layers: &[LayerData]) -> Vec { + let mut result: Vec = Vec::new(); + + for node in root_nodes.iter().rev() { + let data = &layers[node.layer_index]; + let Ok(value) = data.get(&node.path, FieldKey::Payload.as_str()) else { + continue; + }; + + match value.into_owned() { + Value::Payload(p) => { + if !result.contains(&p) { + result.push(p); + } + } + Value::PayloadListOp(list_op) => { + result = list_op.compose_over(&result); + } + _ => {} + } + } + + result +} + +/// Adds nodes from a referenced layer for a given prim path. +/// +/// If the reference has an `asset_path`, looks up the target layer by +/// identifier. If empty, the reference is internal (same layer stack). +/// The source `prim_path` is used for namespace remapping; if empty, +/// the target layer's `defaultPrim` is used. +fn add_reference_nodes( + composed_path: &Path, + reference: &Reference, + arc: ArcType, + nodes: &mut Vec, + layers: &[LayerData], + identifiers: &[String], +) { + if reference.asset_path.is_empty() { + // Internal reference — target is within the same layer stack. + let source = &reference.prim_path; + if source.is_empty() { + return; + } + for (i, layer) in layers.iter().enumerate() { + add_remapped_nodes(layer.as_ref(), i, composed_path, source, arc, nodes); + } + } else { + // External reference — find the target layer by identifier. + let Some(layer_index) = find_layer(&reference.asset_path, identifiers) else { + return; + }; + let layer = layers[layer_index].as_ref(); + + let source = if reference.prim_path.is_empty() { + // Use the target layer's defaultPrim. + let root = Path::abs_root(); + let Ok(value) = layer.get(&root, FieldKey::DefaultPrim.as_str()) else { + return; + }; + match value.into_owned() { + Value::Token(name) | Value::String(name) => Path::new(&format!("/{name}")).unwrap_or_default(), + _ => return, + } + } else { + reference.prim_path.clone() + }; + + add_remapped_nodes(layer, layer_index, composed_path, &source, arc, nodes); + } +} + +/// Adds nodes from a single layer with namespace remapping. +/// +/// Maps `composed_path` and its children from `source_path` in the layer. +fn add_remapped_nodes( + layer: &dyn AbstractData, + layer_index: usize, + composed_path: &Path, + source_path: &Path, + arc: ArcType, + nodes: &mut Vec, +) { + // Remap: the composed_path corresponds to source_path in the target layer. + // For the prim itself, just check if source_path exists. + let query_path = composed_path.replace_prefix(composed_path, source_path); + let Some(query_path) = query_path else { + return; + }; + + if layer.has_spec(&query_path) { + nodes.push(Node { + layer_index, + path: query_path, + arc, + }); + } +} + +/// Finds a layer index whose identifier matches `asset_path`. +/// +/// Tries an exact match first, then falls back to suffix matching at a +/// path separator boundary (so `_stage.usda` matches `/abs/path/_stage.usda` +/// but not `/abs/path/not_stage.usda`). +fn find_layer(asset_path: &str, identifiers: &[String]) -> Option { + let sep = std::path::MAIN_SEPARATOR as u8; + + for (i, id) in identifiers.iter().enumerate() { + if *id == asset_path { + return Some(i); + } + + if id.ends_with(asset_path) { + let prefix_len = id.len() - asset_path.len(); + if id.as_bytes()[prefix_len - 1] == sep { + return Some(i); + } + } + } + + None } diff --git a/src/sdf/mod.rs b/src/sdf/mod.rs index 9b0cc3b..9aa92a0 100644 --- a/src/sdf/mod.rs +++ b/src/sdf/mod.rs @@ -256,6 +256,9 @@ pub trait AbstractData { fn list(&self, path: &Path) -> Option>; } +/// A boxed layer data source, used throughout the layer stack. +pub type LayerData = Box; + /// A single spec in a scene description layer, consisting of a type and a set of fields. /// /// See [SdfSpec](https://openusd.org/dev/api/class_sdf_spec.html) in the USD documentation. diff --git a/src/stage.rs b/src/stage.rs index 06cb179..f27033c 100644 --- a/src/stage.rs +++ b/src/stage.rs @@ -30,10 +30,10 @@ use anyhow::Result; use crate::ar::{DefaultResolver, Resolver}; use crate::compose; -use crate::compose::prim_index::{ArcType, Node, PrimIndex}; +use crate::compose::prim_index::PrimIndex; use crate::compose::CompositionError; use crate::sdf::schema::{ChildrenKey, FieldKey}; -use crate::sdf::{AbstractData, ListOp, Path, Payload, Reference, SpecType, Value}; +use crate::sdf::{LayerData, Path, SpecType, Value}; /// A composed USD stage. /// @@ -41,7 +41,7 @@ use crate::sdf::{AbstractData, ListOp, Path, Payload, Reference, SpecType, Value /// properties, and metadata. pub struct Stage { /// All layers, root (strongest) first. - layers: Vec>, + layers: Vec, /// Layer identifiers, parallel to `layers`. identifiers: Vec, /// Cached prim indices, built lazily per prim. @@ -187,7 +187,7 @@ impl Stage { /// Walks the prim index for a prim path, returning the first opinion for `field`. fn resolve_field(&self, path: &Path, field: &str) -> Result> { let index = self.prim_index(path); - self.resolve_field_in(&index, field, |node| Ok(node.path.clone())) + index.resolve_field(field, &self.layers, |node| Ok(node.path.clone())) } /// Resolves a field on a property spec (attribute or relationship). @@ -198,33 +198,9 @@ impl Stage { let prim_path = prop_path.prim_path(); let prop_suffix = &prop_path.as_str()[prim_path.as_str().len()..]; let index = self.prim_index(&prim_path); - self.resolve_field_in(&index, field, |node| Path::new(&format!("{}{prop_suffix}", node.path))) - } - - /// Walks a prim index from strongest to weakest, returning the first opinion. - /// - /// `make_path` maps each node to the path to query in that layer. - fn resolve_field_in( - &self, - index: &PrimIndex, - field: &str, - make_path: impl Fn(&Node) -> Result, - ) -> Result> { - for node in &index.nodes { - let query_path = make_path(node)?; - let data = &self.layers[node.layer_index]; - if !data.has_field(&query_path, field) { - continue; - } - - let value = data.get(&query_path, field)?; - if matches!(value.as_ref(), Value::ValueBlock) { - return Ok(None); - } - return Ok(Some(value.into_owned())); - } - - Ok(None) + index.resolve_field(field, &self.layers, |node| { + Path::new(&format!("{}{prop_suffix}", node.path)) + }) } /// Traverses all composed prims depth-first, calling `visitor` for each. @@ -257,7 +233,7 @@ impl Stage { return cached.clone(); } - let index = self.build_prim_index(path); + let index = PrimIndex::build(path, &self.layers, &self.identifiers); self.prim_indices.borrow_mut().insert(path.clone(), index.clone()); index } @@ -283,252 +259,6 @@ impl Stage { Ok(result) } - - /// Builds a prim index for the given path. - /// - /// Follows LIVERPS ordering: - /// Local (sublayers) > Inherits > Variants > References > Payloads > Specializes. - fn build_prim_index(&self, path: &Path) -> PrimIndex { - let mut nodes = Vec::new(); - - // L — Root / sublayer opinions: check each layer in strength order. - for (i, layer) in self.layers.iter().enumerate() { - if layer.has_spec(path) { - nodes.push(Node { - layer_index: i, - path: path.clone(), - arc: ArcType::Root, - }); - } - } - - // I — Inherits: compose PathListOp across root nodes, then add nodes - // from the inherited prims within the same layer stack. - let inherits = self.compose_arc_list::(&nodes, FieldKey::InheritPaths); - for inherit_path in &inherits { - for (i, layer) in self.layers.iter().enumerate() { - self.add_remapped_nodes(layer.as_ref(), i, path, inherit_path, ArcType::Inherit, &mut nodes); - } - } - - // V — Variants: resolve variant selection (first opinion wins), then - // add specs from the selected variant path in each layer. - let selections = self.resolve_variant_selections(&nodes); - for (set_name, selection) in &selections { - let variant_path = path.append_variant_selection(set_name, selection); - for (i, layer) in self.layers.iter().enumerate() { - if layer.has_spec(&variant_path) { - nodes.push(Node { - layer_index: i, - path: variant_path.clone(), - arc: ArcType::Variant, - }); - } - } - } - - // R — References: compose ReferenceListOp across root nodes, then add - // nodes from referenced layers with namespace remapping. - let references = self.compose_arc_list::(&nodes, FieldKey::References); - for reference in &references { - self.add_reference_nodes(path, reference, ArcType::Reference, &mut nodes); - } - - // P — Payloads: same as references but weaker. - let payloads = self.collect_payloads(&nodes); - for payload in &payloads { - let reference = Reference { - asset_path: payload.asset_path.clone(), - prim_path: payload.prim_path.clone(), - ..Default::default() - }; - self.add_reference_nodes(path, &reference, ArcType::Payload, &mut nodes); - } - - // S — Specializes: same as inherits but weakest in LIVERPS. - let specializes = self.compose_arc_list::(&nodes, FieldKey::Specializes); - for specialize_path in &specializes { - for (i, layer) in self.layers.iter().enumerate() { - self.add_remapped_nodes( - layer.as_ref(), - i, - path, - specialize_path, - ArcType::Specialize, - &mut nodes, - ); - } - } - - PrimIndex { nodes } - } - - /// Resolves variant selections by walking root nodes strongest-to-weakest. - /// - /// For each variant set, the first opinion wins. Returns a list of - /// `(set_name, selection)` pairs. - fn resolve_variant_selections(&self, root_nodes: &[Node]) -> Vec<(String, String)> { - let mut selections: HashMap = HashMap::new(); - - for node in root_nodes { - let data = &self.layers[node.layer_index]; - let Ok(value) = data.get(&node.path, FieldKey::VariantSelection.as_str()) else { - continue; - }; - - if let Value::VariantSelectionMap(map) = value.into_owned() { - for (set_name, selection) in map { - // First opinion wins. - selections.entry(set_name).or_insert(selection); - } - } - } - - selections.into_iter().collect() - } - - /// Composes a list-op field across root nodes, returning the flattened list. - fn compose_arc_list(&self, root_nodes: &[Node], field: FieldKey) -> Vec - where - Value: TryInto>, - { - let field = field.as_str(); - let mut result: Vec = Vec::new(); - - // Walk from weakest to strongest, composing each ListOp on top. - for node in root_nodes.iter().rev() { - let data = &self.layers[node.layer_index]; - let Ok(value) = data.get(&node.path, field) else { - continue; - }; - - let Ok(list_op) = value.into_owned().try_into() else { - continue; - }; - - result = list_op.compose_over(&result); - } - - result - } - - /// Collects payloads from root nodes, handling both single `Payload` and `PayloadListOp`. - fn collect_payloads(&self, root_nodes: &[Node]) -> Vec { - let mut result: Vec = Vec::new(); - - for node in root_nodes.iter().rev() { - let data = &self.layers[node.layer_index]; - let Ok(value) = data.get(&node.path, FieldKey::Payload.as_str()) else { - continue; - }; - - match value.into_owned() { - Value::Payload(p) => { - if !result.contains(&p) { - result.push(p); - } - } - Value::PayloadListOp(list_op) => { - result = list_op.compose_over(&result); - } - _ => {} - } - } - - result - } - - /// Adds nodes from a referenced layer for a given prim path. - /// - /// If the reference has an `asset_path`, looks up the target layer by - /// identifier. If empty, the reference is internal (same layer stack). - /// The source `prim_path` is used for namespace remapping; if empty, - /// the target layer's `defaultPrim` is used. - fn add_reference_nodes(&self, composed_path: &Path, reference: &Reference, arc: ArcType, nodes: &mut Vec) { - if reference.asset_path.is_empty() { - // Internal reference — target is within the same layer stack. - let source = &reference.prim_path; - if source.is_empty() { - return; - } - for (i, layer) in self.layers.iter().enumerate() { - self.add_remapped_nodes(layer.as_ref(), i, composed_path, source, arc, nodes); - } - } else { - // External reference — find the target layer by identifier. - let Some((layer_index, layer)) = self.find_layer(&reference.asset_path) else { - return; - }; - - let source = if reference.prim_path.is_empty() { - // Use the target layer's defaultPrim. - let root = Path::abs_root(); - let Ok(value) = layer.get(&root, FieldKey::DefaultPrim.as_str()) else { - return; - }; - match value.into_owned() { - Value::Token(name) | Value::String(name) => Path::new(&format!("/{name}")).unwrap_or_default(), - _ => return, - } - } else { - reference.prim_path.clone() - }; - - self.add_remapped_nodes(layer, layer_index, composed_path, &source, arc, nodes); - } - } - - /// Adds nodes from a single layer with namespace remapping. - /// - /// Maps `composed_path` and its children from `source_path` in the layer. - fn add_remapped_nodes( - &self, - layer: &dyn AbstractData, - layer_index: usize, - composed_path: &Path, - source_path: &Path, - arc: ArcType, - nodes: &mut Vec, - ) { - // Remap: the composed_path corresponds to source_path in the target layer. - // For the prim itself, just check if source_path exists. - let query_path = composed_path.replace_prefix(composed_path, source_path); - let Some(query_path) = query_path else { - return; - }; - - if layer.has_spec(&query_path) { - nodes.push(Node { - layer_index, - path: query_path, - arc, - }); - } - } - - /// Finds a layer whose identifier matches `asset_path`. - /// - /// Tries an exact match first, then falls back to suffix matching at a - /// path separator boundary (so `_stage.usda` matches `/abs/path/_stage.usda` - /// but not `/abs/path/not_stage.usda`). - fn find_layer(&self, asset_path: &str) -> Option<(usize, &dyn AbstractData)> { - let sep = std::path::MAIN_SEPARATOR as u8; - - for (i, id) in self.identifiers.iter().enumerate() { - if *id == asset_path { - return Some((i, self.layers[i].as_ref())); - } - - if id.ends_with(asset_path) { - let prefix_len = id.len() - asset_path.len(); - if id.as_bytes()[prefix_len - 1] == sep { - return Some((i, self.layers[i].as_ref())); - } - } - } - - None - } } /// Default composition error handler that treats all errors as fatal. @@ -605,63 +335,6 @@ mod tests { format!("{}/fixtures/{relative}", manifest_dir()) } - // --- find_layer --- - - /// Exact identifier match should return the layer. - #[test] - fn find_layer_exact_match() -> Result<()> { - let path = fixture_path("ref_external.usda"); - let stage = Stage::open(&path)?; - - // The full identifier of the root layer should match exactly. - assert!( - stage.find_layer(&stage.identifiers[0]).is_some(), - "exact match should succeed" - ); - - Ok(()) - } - - /// Suffix match at a path separator boundary should work - /// (e.g. "ref_target.usda" matches "/full/path/ref_target.usda"). - #[test] - fn find_layer_suffix_match() -> Result<()> { - let path = fixture_path("ref_external.usda"); - let stage = Stage::open(&path)?; - - // ref_external.usda references ref_target.usda, which should be loaded. - let result = stage.find_layer("ref_target.usda"); - assert!(result.is_some(), "suffix match at separator boundary should succeed"); - - Ok(()) - } - - /// A partial filename overlap without a separator boundary must not match - /// (e.g. "target.usda" should not match "ref_target.usda"). - #[test] - fn find_layer_no_partial_name_match() -> Result<()> { - let path = fixture_path("ref_external.usda"); - let stage = Stage::open(&path)?; - - assert!( - stage.find_layer("target.usda").is_none(), - "partial name should not match" - ); - - Ok(()) - } - - /// A completely unknown path should return None. - #[test] - fn find_layer_not_found() -> Result<()> { - let path = fixture_path("ref_external.usda"); - let stage = Stage::open(&path)?; - - assert!(stage.find_layer("nonexistent.usda").is_none()); - - Ok(()) - } - // --- PrimIndex internals --- /// A prim in a single-layer stage should produce a PrimIndex with exactly From bc196ec6ec48710e928833c8f50d752dd83f06cf Mon Sep 17 00:00:00 2001 From: Maksym Pavlenko Date: Tue, 7 Apr 2026 12:19:03 -0700 Subject: [PATCH 2/5] Move composition logic from Stage to PrimIndex Extract prim index building, field resolution, and LIVERPS arc handling from Stage into PrimIndex::build() and PrimIndex::resolve_field(). Stage now delegates to these methods, keeping only caching, the query API, and traversal. Add LayerData type alias for Box. Reduce allocations: use Cow for prim path in arc resolution, remove identity replace_prefix call in add_remapped_nodes, unify reference and payload handling via add_arc_nodes to avoid intermediate clones, return HashMap from resolve_variant_selections, and replace cloning prim index cache with update_index + borrow pattern. --- src/compose/prim_index.rs | 85 ++++++++++++++++++--------------------- src/stage.rs | 64 ++++++++++++++++------------- 2 files changed, 77 insertions(+), 72 deletions(-) diff --git a/src/compose/prim_index.rs b/src/compose/prim_index.rs index 654fcfc..7365b02 100644 --- a/src/compose/prim_index.rs +++ b/src/compose/prim_index.rs @@ -13,6 +13,7 @@ //! //! See +use std::borrow::Cow; use std::collections::HashMap; use anyhow::Result; @@ -92,7 +93,7 @@ impl PrimIndex { let inherits = compose_arc_list::(&nodes, FieldKey::InheritPaths, layers); for inherit_path in &inherits { for (i, layer) in layers.iter().enumerate() { - add_remapped_nodes(layer.as_ref(), i, path, inherit_path, ArcType::Inherit, &mut nodes); + add_remapped_nodes(layer.as_ref(), i, inherit_path, ArcType::Inherit, &mut nodes); } } @@ -116,32 +117,34 @@ impl PrimIndex { // nodes from referenced layers with namespace remapping. let references = compose_arc_list::(&nodes, FieldKey::References, layers); for reference in &references { - add_reference_nodes(path, reference, ArcType::Reference, &mut nodes, layers, identifiers); + add_arc_nodes( + &reference.asset_path, + &reference.prim_path, + ArcType::Reference, + &mut nodes, + layers, + identifiers, + ); } // P — Payloads: same as references but weaker. let payloads = collect_payloads(&nodes, layers); for payload in &payloads { - let reference = Reference { - asset_path: payload.asset_path.clone(), - prim_path: payload.prim_path.clone(), - ..Default::default() - }; - add_reference_nodes(path, &reference, ArcType::Payload, &mut nodes, layers, identifiers); + add_arc_nodes( + &payload.asset_path, + &payload.prim_path, + ArcType::Payload, + &mut nodes, + layers, + identifiers, + ); } // S — Specializes: same as inherits but weakest in LIVERPS. let specializes = compose_arc_list::(&nodes, FieldKey::Specializes, layers); for specialize_path in &specializes { for (i, layer) in layers.iter().enumerate() { - add_remapped_nodes( - layer.as_ref(), - i, - path, - specialize_path, - ArcType::Specialize, - &mut nodes, - ); + add_remapped_nodes(layer.as_ref(), i, specialize_path, ArcType::Specialize, &mut nodes); } } @@ -179,7 +182,7 @@ impl PrimIndex { /// Resolves variant selections by walking root nodes strongest-to-weakest. /// /// For each variant set, the first opinion wins. -fn resolve_variant_selections(root_nodes: &[Node], layers: &[LayerData]) -> Vec<(String, String)> { +fn resolve_variant_selections(root_nodes: &[Node], layers: &[LayerData]) -> HashMap { let mut selections: HashMap = HashMap::new(); for node in root_nodes { @@ -196,7 +199,7 @@ fn resolve_variant_selections(root_nodes: &[Node], layers: &[LayerData]) -> Vec< } } - selections.into_iter().collect() + selections } /// Composes a list-op field across root nodes, returning the flattened list. @@ -254,51 +257,51 @@ fn collect_payloads(root_nodes: &[Node], layers: &[LayerData]) -> Vec { result } -/// Adds nodes from a referenced layer for a given prim path. +/// Adds nodes from a referenced or payloaded layer for a given prim path. /// -/// If the reference has an `asset_path`, looks up the target layer by -/// identifier. If empty, the reference is internal (same layer stack). -/// The source `prim_path` is used for namespace remapping; if empty, -/// the target layer's `defaultPrim` is used. -fn add_reference_nodes( - composed_path: &Path, - reference: &Reference, +/// If `asset_path` is empty, the target is internal (same layer stack). +/// `prim_path` is used for namespace remapping; if empty, the target +/// layer's `defaultPrim` is used. +fn add_arc_nodes( + asset_path: &str, + prim_path: &Path, arc: ArcType, nodes: &mut Vec, layers: &[LayerData], identifiers: &[String], ) { - if reference.asset_path.is_empty() { + if asset_path.is_empty() { // Internal reference — target is within the same layer stack. - let source = &reference.prim_path; - if source.is_empty() { + if prim_path.is_empty() { return; } for (i, layer) in layers.iter().enumerate() { - add_remapped_nodes(layer.as_ref(), i, composed_path, source, arc, nodes); + add_remapped_nodes(layer.as_ref(), i, prim_path, arc, nodes); } } else { // External reference — find the target layer by identifier. - let Some(layer_index) = find_layer(&reference.asset_path, identifiers) else { + let Some(layer_index) = find_layer(asset_path, identifiers) else { return; }; let layer = layers[layer_index].as_ref(); - let source = if reference.prim_path.is_empty() { + let source = if prim_path.is_empty() { // Use the target layer's defaultPrim. let root = Path::abs_root(); let Ok(value) = layer.get(&root, FieldKey::DefaultPrim.as_str()) else { return; }; match value.into_owned() { - Value::Token(name) | Value::String(name) => Path::new(&format!("/{name}")).unwrap_or_default(), + Value::Token(name) | Value::String(name) => { + Cow::Owned(Path::new(&format!("/{name}")).unwrap_or_default()) + } _ => return, } } else { - reference.prim_path.clone() + Cow::Borrowed(prim_path) }; - add_remapped_nodes(layer, layer_index, composed_path, &source, arc, nodes); + add_remapped_nodes(layer, layer_index, &source, arc, nodes); } } @@ -308,22 +311,14 @@ fn add_reference_nodes( fn add_remapped_nodes( layer: &dyn AbstractData, layer_index: usize, - composed_path: &Path, source_path: &Path, arc: ArcType, nodes: &mut Vec, ) { - // Remap: the composed_path corresponds to source_path in the target layer. - // For the prim itself, just check if source_path exists. - let query_path = composed_path.replace_prefix(composed_path, source_path); - let Some(query_path) = query_path else { - return; - }; - - if layer.has_spec(&query_path) { + if layer.has_spec(source_path) { nodes.push(Node { layer_index, - path: query_path, + path: source_path.clone(), arc, }); } diff --git a/src/stage.rs b/src/stage.rs index f27033c..11f8798 100644 --- a/src/stage.rs +++ b/src/stage.rs @@ -129,13 +129,18 @@ impl Stage { /// Returns `true` if any layer has a spec at the given composed path. pub fn has_spec(&self, path: impl Into) -> bool { - !self.prim_index(&path.into()).is_empty() + let path = path.into(); + self.update_index(&path); + let cache = self.prim_indices.borrow(); + !cache[&path].is_empty() } /// Returns the spec type at a composed path from the strongest contributing layer. pub fn spec_type(&self, path: impl Into) -> Option { - let index = self.prim_index(&path.into()); - for node in &index.nodes { + let path = path.into(); + self.update_index(&path); + let cache = self.prim_indices.borrow(); + for node in &cache[&path].nodes { if let Some(ty) = self.layers[node.layer_index].spec_type(&node.path) { return Some(ty); } @@ -186,8 +191,9 @@ impl Stage { /// Walks the prim index for a prim path, returning the first opinion for `field`. fn resolve_field(&self, path: &Path, field: &str) -> Result> { - let index = self.prim_index(path); - index.resolve_field(field, &self.layers, |node| Ok(node.path.clone())) + self.update_index(path); + let cache = self.prim_indices.borrow(); + cache[path].resolve_field(field, &self.layers, |node| Ok(node.path.clone())) } /// Resolves a field on a property spec (attribute or relationship). @@ -197,8 +203,9 @@ impl Stage { fn property_field(&self, prop_path: &Path, field: &str) -> Result> { let prim_path = prop_path.prim_path(); let prop_suffix = &prop_path.as_str()[prim_path.as_str().len()..]; - let index = self.prim_index(&prim_path); - index.resolve_field(field, &self.layers, |node| { + self.update_index(&prim_path); + let cache = self.prim_indices.borrow(); + cache[&prim_path].resolve_field(field, &self.layers, |node| { Path::new(&format!("{}{prop_suffix}", node.path)) }) } @@ -226,26 +233,24 @@ impl Stage { Ok(()) } - /// Returns the prim index for a path, building and caching it if needed. - fn prim_index(&self, path: &Path) -> PrimIndex { - // Check cache first. - if let Some(cached) = self.prim_indices.borrow().get(path) { - return cached.clone(); + /// Ensures the prim index for `path` is built and cached. + fn update_index(&self, path: &Path) { + if self.prim_indices.borrow().contains_key(path) { + return; } - let index = PrimIndex::build(path, &self.layers, &self.identifiers); - self.prim_indices.borrow_mut().insert(path.clone(), index.clone()); - index + self.prim_indices.borrow_mut().insert(path.clone(), index); } /// Merges a children field (e.g. `primChildren`, `properties`) across all /// nodes in the prim index, returning the union with strongest-first ordering. fn composed_children(&self, path: &Path, children_field: impl AsRef) -> Result> { let children_field: &str = children_field.as_ref(); - let index = self.prim_index(path); + self.update_index(path); + let cache = self.prim_indices.borrow(); let mut result: Vec = Vec::new(); - for node in &index.nodes { + for node in &cache[path].nodes { if let Ok(value) = self.layers[node.layer_index].get(&node.path, children_field) { if let Value::TokenVec(names) = value.into_owned() { for name in names { @@ -335,6 +340,11 @@ mod tests { format!("{}/fixtures/{relative}", manifest_dir()) } + /// Builds a prim index for the given path using the stage's layers. + fn prim_index(stage: &Stage, path: &str) -> PrimIndex { + PrimIndex::build(&Path::from(path), &stage.layers, &stage.identifiers) + } + // --- PrimIndex internals --- /// A prim in a single-layer stage should produce a PrimIndex with exactly @@ -344,7 +354,7 @@ mod tests { let path = composition_path("active.usda"); let stage = Stage::open(&path)?; - let index = stage.prim_index(&Path::new("/World")?); + let index = prim_index(&stage, "/World"); assert_eq!(index.nodes.len(), 1); assert_eq!(index.nodes[0].layer_index, 0); assert_eq!(index.nodes[0].arc, ArcType::Root); @@ -360,7 +370,7 @@ mod tests { let path = fixture_path("sublayer_override.usda"); let stage = Stage::open(&path)?; - let index = stage.prim_index(&Path::new("/World")?); + let index = prim_index(&stage, "/World"); assert_eq!(index.nodes.len(), 2, "both layers should have /World"); assert_eq!(index.nodes[0].layer_index, 0, "stronger layer first"); assert_eq!(index.nodes[1].layer_index, 1, "weaker layer second"); @@ -375,7 +385,7 @@ mod tests { let stage = Stage::open(&path)?; // /World/Sphere is only defined in the override layer. - let index = stage.prim_index(&Path::new("/World/Sphere")?); + let index = prim_index(&stage, "/World/Sphere"); assert_eq!(index.nodes.len(), 1); assert_eq!(index.nodes[0].layer_index, 0); @@ -388,7 +398,7 @@ mod tests { let path = composition_path("active.usda"); let stage = Stage::open(&path)?; - let index = stage.prim_index(&Path::new("/DoesNotExist")?); + let index = prim_index(&stage, "/DoesNotExist"); assert!(index.is_empty()); Ok(()) @@ -544,7 +554,7 @@ mod tests { assert!(stage.has_spec("/World/MyPrim")); // The prim index should have a Reference arc node. - let index = stage.prim_index(&Path::new("/World/MyPrim")?); + let index = prim_index(&stage, "/World/MyPrim"); assert!( index.nodes.iter().any(|n| n.arc == ArcType::Reference), "prim index should contain a Reference node" @@ -589,7 +599,7 @@ mod tests { let stage = Stage::open(&path)?; // /World/RefPrim should exist with a Reference arc. - let index = stage.prim_index(&Path::new("/World/RefPrim")?); + let index = prim_index(&stage, "/World/RefPrim"); assert!( index.nodes.iter().any(|n| n.arc == ArcType::Reference), "should have a Reference arc" @@ -616,7 +626,7 @@ mod tests { // The prim index for cubeWithoutSetColor should include an Inherit node // pointing at /_myClass. - let index = stage.prim_index(&Path::new("/World/cubeWithoutSetColor")?); + let index = prim_index(&stage, "/World/cubeWithoutSetColor"); assert!( index.nodes.iter().any(|n| n.arc == ArcType::Inherit), "should have an Inherit arc" @@ -642,7 +652,7 @@ mod tests { // cubeWithSetColor has both a local and inherited displayColor. // The prim index should have Root first, then Inherit. - let index = stage.prim_index(&Path::new("/World/cubeWithSetColor")?); + let index = prim_index(&stage, "/World/cubeWithSetColor"); let arcs: Vec<_> = index.nodes.iter().map(|n| n.arc).collect(); assert_eq!(arcs[0], ArcType::Root, "Root should be strongest"); assert!(arcs.contains(&ArcType::Inherit), "should also have Inherit"); @@ -673,7 +683,7 @@ mod tests { let stage = Stage::open(&path)?; // The prim index should contain a Variant arc node. - let index = stage.prim_index(&Path::new("/World/Sphere")?); + let index = prim_index(&stage, "/World/Sphere"); assert!( index.nodes.iter().any(|n| n.arc == ArcType::Variant), "should have a Variant arc for the selected variant" @@ -733,7 +743,7 @@ mod tests { let path = composition_path("inherit_and_specialize.usda"); let stage = Stage::open(&path)?; - let index = stage.prim_index(&Path::new("/World/cubeScene/specializes")?); + let index = prim_index(&stage, "/World/cubeScene/specializes"); assert!( index.nodes.iter().any(|n| n.arc == ArcType::Specialize), "should have a Specialize arc" From 052a3294c034cb84d7d8ea391fc3de44a6937e8b Mon Sep 17 00:00:00 2001 From: Maksym Pavlenko Date: Tue, 7 Apr 2026 12:22:19 -0700 Subject: [PATCH 3/5] Move composition tests to compose/prim_index Move prim index, arc type, and find_layer tests from stage to compose/prim_index where the implementation now lives. Stage tests retain only the public API assertions (field resolution, children, traversal). --- src/compose/prim_index.rs | 165 ++++++++++++++++++++++++++++++++++++++ src/stage.rs | 139 +------------------------------- 2 files changed, 166 insertions(+), 138 deletions(-) diff --git a/src/compose/prim_index.rs b/src/compose/prim_index.rs index 7365b02..bf6087b 100644 --- a/src/compose/prim_index.rs +++ b/src/compose/prim_index.rs @@ -347,3 +347,168 @@ fn find_layer(asset_path: &str, identifiers: &[String]) -> Option { None } + +#[cfg(test)] +mod tests { + use super::*; + use crate::ar::DefaultResolver; + use crate::compose::collect_layers; + use crate::sdf::LayerData; + + use anyhow::Result; + + const VENDOR_COMPOSITION: &str = "vendor/usd-wg-assets/test_assets/foundation/stage_composition"; + + fn manifest_dir() -> String { + std::env::var("CARGO_MANIFEST_DIR").unwrap() + } + + fn composition_path(relative: &str) -> String { + format!("{}/{VENDOR_COMPOSITION}/{relative}", manifest_dir()) + } + + fn fixture_path(relative: &str) -> String { + format!("{}/fixtures/{relative}", manifest_dir()) + } + + /// Loads layers and splits into parallel vecs for PrimIndex::build. + fn load_layers(path: &str) -> Result<(Vec, Vec)> { + let resolver = DefaultResolver::new(); + let collected = collect_layers(&resolver, path)?; + let mut layers = Vec::new(); + let mut identifiers = Vec::new(); + for layer in collected { + identifiers.push(layer.identifier); + layers.push(layer.data); + } + Ok((layers, identifiers)) + } + + /// Builds a prim index for a given path string. + fn build(layers: &[LayerData], identifiers: &[String], prim: &str) -> PrimIndex { + PrimIndex::build(&Path::from(prim), layers, identifiers) + } + + #[test] + fn single_layer_root_node() -> Result<()> { + let (layers, ids) = load_layers(&composition_path("active.usda"))?; + let index = build(&layers, &ids, "/World"); + + assert_eq!(index.nodes.len(), 1); + assert_eq!(index.nodes[0].layer_index, 0); + assert_eq!(index.nodes[0].arc, ArcType::Root); + Ok(()) + } + + #[test] + fn sublayer_two_root_nodes() -> Result<()> { + let (layers, ids) = load_layers(&fixture_path("sublayer_override.usda"))?; + let index = build(&layers, &ids, "/World"); + + assert_eq!(index.nodes.len(), 2, "both layers should have /World"); + assert_eq!(index.nodes[0].layer_index, 0, "stronger layer first"); + assert_eq!(index.nodes[1].layer_index, 1, "weaker layer second"); + Ok(()) + } + + #[test] + fn prim_only_in_stronger_layer() -> Result<()> { + let (layers, ids) = load_layers(&fixture_path("sublayer_override.usda"))?; + let index = build(&layers, &ids, "/World/Sphere"); + + assert_eq!(index.nodes.len(), 1); + assert_eq!(index.nodes[0].layer_index, 0); + Ok(()) + } + + #[test] + fn nonexistent_prim_empty_index() -> Result<()> { + let (layers, ids) = load_layers(&composition_path("active.usda"))?; + let index = build(&layers, &ids, "/DoesNotExist"); + + assert!(index.is_empty()); + Ok(()) + } + + #[test] + fn reference_arc_present() -> Result<()> { + let (layers, ids) = load_layers(&fixture_path("ref_external.usda"))?; + let index = build(&layers, &ids, "/World/MyPrim"); + + assert!(index.nodes.iter().any(|n| n.arc == ArcType::Reference)); + Ok(()) + } + + #[test] + fn inherit_arc_present() -> Result<()> { + let (layers, ids) = load_layers(&composition_path("class_inherit.usda"))?; + let index = build(&layers, &ids, "/World/cubeWithoutSetColor"); + + assert!(index.nodes.iter().any(|n| n.arc == ArcType::Inherit)); + Ok(()) + } + + #[test] + fn inherit_root_is_strongest() -> Result<()> { + let (layers, ids) = load_layers(&composition_path("class_inherit.usda"))?; + let index = build(&layers, &ids, "/World/cubeWithSetColor"); + let arcs: Vec<_> = index.nodes.iter().map(|n| n.arc).collect(); + + assert_eq!(arcs[0], ArcType::Root); + assert!(arcs.contains(&ArcType::Inherit)); + Ok(()) + } + + #[test] + fn variant_arc_with_selection() -> Result<()> { + let path = format!( + "{}/vendor/usd-wg-assets/docs/CompositionPuzzles/VariantSetAndLocal1/puzzle_1.usda", + manifest_dir() + ); + let (layers, ids) = load_layers(&path)?; + let index = build(&layers, &ids, "/World/Sphere"); + + assert!(index.nodes.iter().any(|n| n.arc == ArcType::Variant)); + + let variant_node = index.nodes.iter().find(|n| n.arc == ArcType::Variant).unwrap(); + assert_eq!(variant_node.path.as_str(), "/World/Sphere{size=small}"); + Ok(()) + } + + #[test] + fn specialize_arc_present() -> Result<()> { + let (layers, ids) = load_layers(&composition_path("inherit_and_specialize.usda"))?; + let index = build(&layers, &ids, "/World/cubeScene/specializes"); + + assert!(index.nodes.iter().any(|n| n.arc == ArcType::Specialize)); + Ok(()) + } + + #[test] + fn find_layer_exact_match() -> Result<()> { + let (_, ids) = load_layers(&fixture_path("ref_external.usda"))?; + assert!(find_layer(&ids[0], &ids).is_some()); + Ok(()) + } + + #[test] + fn find_layer_suffix_match() -> Result<()> { + let (_, ids) = load_layers(&fixture_path("ref_external.usda"))?; + assert!(find_layer("ref_target.usda", &ids).is_some()); + Ok(()) + } + + #[test] + fn find_layer_no_partial_name_match() -> Result<()> { + let (_, ids) = load_layers(&fixture_path("ref_external.usda"))?; + assert!(find_layer("target.usda", &ids).is_none()); + Ok(()) + } + + #[test] + fn find_layer_not_found() -> Result<()> { + let (_, ids) = load_layers(&fixture_path("ref_external.usda"))?; + assert!(find_layer("nonexistent.usda", &ids).is_none()); + Ok(()) + } +} diff --git a/src/stage.rs b/src/stage.rs index 11f8798..b6eeaaf 100644 --- a/src/stage.rs +++ b/src/stage.rs @@ -324,7 +324,6 @@ impl Result<()>> StageBuilder { #[cfg(test)] mod tests { use super::*; - use crate::compose::prim_index::ArcType; const VENDOR_COMPOSITION: &str = "vendor/usd-wg-assets/test_assets/foundation/stage_composition"; @@ -340,70 +339,6 @@ mod tests { format!("{}/fixtures/{relative}", manifest_dir()) } - /// Builds a prim index for the given path using the stage's layers. - fn prim_index(stage: &Stage, path: &str) -> PrimIndex { - PrimIndex::build(&Path::from(path), &stage.layers, &stage.identifiers) - } - - // --- PrimIndex internals --- - - /// A prim in a single-layer stage should produce a PrimIndex with exactly - /// one Root node pointing at layer 0. - #[test] - fn prim_index_single_layer() -> Result<()> { - let path = composition_path("active.usda"); - let stage = Stage::open(&path)?; - - let index = prim_index(&stage, "/World"); - assert_eq!(index.nodes.len(), 1); - assert_eq!(index.nodes[0].layer_index, 0); - assert_eq!(index.nodes[0].arc, ArcType::Root); - - Ok(()) - } - - /// When a prim exists in both layers of a sublayer composition, the index - /// should contain two Root nodes with the stronger layer (index 0) first. - #[test] - fn prim_index_sublayer_two_layers() -> Result<()> { - // sublayer_override.usda sublayers sublayer_base.usda; both have /World. - let path = fixture_path("sublayer_override.usda"); - let stage = Stage::open(&path)?; - - let index = prim_index(&stage, "/World"); - assert_eq!(index.nodes.len(), 2, "both layers should have /World"); - assert_eq!(index.nodes[0].layer_index, 0, "stronger layer first"); - assert_eq!(index.nodes[1].layer_index, 1, "weaker layer second"); - - Ok(()) - } - - /// A prim that only exists in the stronger layer should have a single node. - #[test] - fn prim_index_prim_only_in_stronger_layer() -> Result<()> { - let path = fixture_path("sublayer_override.usda"); - let stage = Stage::open(&path)?; - - // /World/Sphere is only defined in the override layer. - let index = prim_index(&stage, "/World/Sphere"); - assert_eq!(index.nodes.len(), 1); - assert_eq!(index.nodes[0].layer_index, 0); - - Ok(()) - } - - /// A path that doesn't exist in any layer should produce an empty PrimIndex. - #[test] - fn prim_index_nonexistent() -> Result<()> { - let path = composition_path("active.usda"); - let stage = Stage::open(&path)?; - - let index = prim_index(&stage, "/DoesNotExist"); - assert!(index.is_empty()); - - Ok(()) - } - // --- Basic stage opening (vendor/usd-wg-assets) --- /// A single-layer .usda file should load with correct defaultPrim and @@ -553,15 +488,7 @@ mod tests { // /World/MyPrim should exist via the reference. assert!(stage.has_spec("/World/MyPrim")); - // The prim index should have a Reference arc node. - let index = prim_index(&stage, "/World/MyPrim"); - assert!( - index.nodes.iter().any(|n| n.arc == ArcType::Reference), - "prim index should contain a Reference node" - ); - - // /World/MyPrim/Child should be reachable via namespace remapping - // (maps /Source/Child from the target layer to /World/MyPrim/Child). + // /World/MyPrim/Child should be reachable via namespace remapping. let children = stage.prim_children("/World/MyPrim")?; assert!( children.contains(&"Child".to_string()), @@ -598,13 +525,6 @@ mod tests { let path = fixture_path("ref_prim.usda"); let stage = Stage::open(&path)?; - // /World/RefPrim should exist with a Reference arc. - let index = prim_index(&stage, "/World/RefPrim"); - assert!( - index.nodes.iter().any(|n| n.arc == ArcType::Reference), - "should have a Reference arc" - ); - // /Source/Child in ref_target.usda should appear as /World/RefPrim/Child. let children = stage.prim_children("/World/RefPrim")?; assert!( @@ -624,14 +544,6 @@ mod tests { let path = composition_path("class_inherit.usda"); let stage = Stage::open(&path)?; - // The prim index for cubeWithoutSetColor should include an Inherit node - // pointing at /_myClass. - let index = prim_index(&stage, "/World/cubeWithoutSetColor"); - assert!( - index.nodes.iter().any(|n| n.arc == ArcType::Inherit), - "should have an Inherit arc" - ); - // The inherited property should be visible. let props = stage.prim_properties("/World/cubeWithoutSetColor")?; assert!( @@ -650,13 +562,6 @@ mod tests { let path = composition_path("class_inherit.usda"); let stage = Stage::open(&path)?; - // cubeWithSetColor has both a local and inherited displayColor. - // The prim index should have Root first, then Inherit. - let index = prim_index(&stage, "/World/cubeWithSetColor"); - let arcs: Vec<_> = index.nodes.iter().map(|n| n.arc).collect(); - assert_eq!(arcs[0], ArcType::Root, "Root should be strongest"); - assert!(arcs.contains(&ArcType::Inherit), "should also have Inherit"); - // The local displayColor (red) should win over inherited (green). let prop = Path::new("/World/cubeWithSetColor")?.append_property("primvars:displayColor")?; let value: Option = stage.field(&prop, FieldKey::Default)?; @@ -671,31 +576,6 @@ mod tests { // --- Variant selection --- - /// puzzle_1.usda: /World/Sphere has variantSet "size" with selection "small". - /// The selected variant sets radius=2, while the local opinion sets radius=1. - /// Local should win (L > V in LIVERPS), but the variant node should exist. - #[test] - fn variant_selection_resolves() -> Result<()> { - let path = format!( - "{}/vendor/usd-wg-assets/docs/CompositionPuzzles/VariantSetAndLocal1/puzzle_1.usda", - manifest_dir() - ); - let stage = Stage::open(&path)?; - - // The prim index should contain a Variant arc node. - let index = prim_index(&stage, "/World/Sphere"); - assert!( - index.nodes.iter().any(|n| n.arc == ArcType::Variant), - "should have a Variant arc for the selected variant" - ); - - // The variant node's path should be /World/Sphere{size=small}. - let variant_node = index.nodes.iter().find(|n| n.arc == ArcType::Variant).unwrap(); - assert_eq!(variant_node.path.as_str(), "/World/Sphere{size=small}"); - - Ok(()) - } - /// The local opinion on radius (1) should be stronger than the variant's (2). #[test] fn variant_local_opinion_wins() -> Result<()> { @@ -735,23 +615,6 @@ mod tests { // --- Specialize composition --- - /// inherit_and_specialize.usda: /World/cubeScene/specializes specializes - /// . The specialize arc should appear in the - /// prim index as the weakest arc. - #[test] - fn specialize_arc_present() -> Result<()> { - let path = composition_path("inherit_and_specialize.usda"); - let stage = Stage::open(&path)?; - - let index = prim_index(&stage, "/World/cubeScene/specializes"); - assert!( - index.nodes.iter().any(|n| n.arc == ArcType::Specialize), - "should have a Specialize arc" - ); - - Ok(()) - } - /// The local opinion on displayColor (yellow) should win over the /// specialized source's displayColor (red). #[test] From 887979b8573c06023860130e3c260327e1168b7b Mon Sep 17 00:00:00 2001 From: Maksym Pavlenko Date: Tue, 7 Apr 2026 12:28:04 -0700 Subject: [PATCH 4/5] Replace resolve_field closure with prop_suffix parameter Use Cow to avoid allocations in the common prim field resolution path. Property fields pass a suffix that gets appended per-node; prim fields pass None and borrow the node path directly. --- src/compose/prim_index.rs | 11 ++++++++--- src/stage.rs | 6 ++---- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/compose/prim_index.rs b/src/compose/prim_index.rs index bf6087b..2fdea49 100644 --- a/src/compose/prim_index.rs +++ b/src/compose/prim_index.rs @@ -153,16 +153,21 @@ impl PrimIndex { /// Resolves a field by walking nodes from strongest to weakest, returning the first opinion. /// - /// `make_path` maps each node to the path to query in that layer. + /// When `prop_suffix` is `None`, queries use the node's path directly (zero-copy). + /// When `Some`, appends the suffix to form a property path for each node. /// A [`Value::ValueBlock`] explicitly blocks opinions from weaker layers. pub(crate) fn resolve_field( &self, field: &str, layers: &[LayerData], - make_path: impl Fn(&Node) -> Result, + prop_suffix: Option<&str>, ) -> Result> { for node in &self.nodes { - let query_path = make_path(node)?; + let query_path = match prop_suffix { + Some(suffix) => Cow::Owned(Path::new(&format!("{}{suffix}", node.path))?), + None => Cow::Borrowed(&node.path), + }; + let data = &layers[node.layer_index]; if !data.has_field(&query_path, field) { continue; diff --git a/src/stage.rs b/src/stage.rs index b6eeaaf..a9cd7ee 100644 --- a/src/stage.rs +++ b/src/stage.rs @@ -193,7 +193,7 @@ impl Stage { fn resolve_field(&self, path: &Path, field: &str) -> Result> { self.update_index(path); let cache = self.prim_indices.borrow(); - cache[path].resolve_field(field, &self.layers, |node| Ok(node.path.clone())) + cache[path].resolve_field(field, &self.layers, None) } /// Resolves a field on a property spec (attribute or relationship). @@ -205,9 +205,7 @@ impl Stage { let prop_suffix = &prop_path.as_str()[prim_path.as_str().len()..]; self.update_index(&prim_path); let cache = self.prim_indices.borrow(); - cache[&prim_path].resolve_field(field, &self.layers, |node| { - Path::new(&format!("{}{prop_suffix}", node.path)) - }) + cache[&prim_path].resolve_field(field, &self.layers, Some(prop_suffix)) } /// Traverses all composed prims depth-first, calling `visitor` for each. From 1b173f4b98c90c53424c469a03e744ca4d80e346 Mon Sep 17 00:00:00 2001 From: Maksym Pavlenko Date: Tue, 7 Apr 2026 12:30:32 -0700 Subject: [PATCH 5/5] Rename compose/prim_index to compose/index --- src/compose/{prim_index.rs => index.rs} | 0 src/compose/mod.rs | 2 +- src/stage.rs | 2 +- 3 files changed, 2 insertions(+), 2 deletions(-) rename src/compose/{prim_index.rs => index.rs} (100%) diff --git a/src/compose/prim_index.rs b/src/compose/index.rs similarity index 100% rename from src/compose/prim_index.rs rename to src/compose/index.rs diff --git a/src/compose/mod.rs b/src/compose/mod.rs index e17684a..6d6fd94 100644 --- a/src/compose/mod.rs +++ b/src/compose/mod.rs @@ -6,7 +6,7 @@ //! is a [`Vec`] of [`Layer`]s, each wrapping a parsed [`AbstractData`] with its resolved //! identity. Cycles are detected and skipped automatically. -pub(crate) mod prim_index; +pub(crate) mod index; use std::collections::{HashMap, HashSet}; use std::io::Cursor; diff --git a/src/stage.rs b/src/stage.rs index a9cd7ee..28ead0c 100644 --- a/src/stage.rs +++ b/src/stage.rs @@ -30,7 +30,7 @@ use anyhow::Result; use crate::ar::{DefaultResolver, Resolver}; use crate::compose; -use crate::compose::prim_index::PrimIndex; +use crate::compose::index::PrimIndex; use crate::compose::CompositionError; use crate::sdf::schema::{ChildrenKey, FieldKey}; use crate::sdf::{LayerData, Path, SpecType, Value};