From a81df55106bcdf98f4b57d7ffbbd44f9fbe97b97 Mon Sep 17 00:00:00 2001 From: Trim Bresilla Date: Sun, 4 Oct 2026 18:38:45 +0200 Subject: [PATCH 1/9] perf(ar): hand over cursor buffer in read_all --- crates/openusd/src/ar.rs | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/crates/openusd/src/ar.rs b/crates/openusd/src/ar.rs index cbc726a9..4a8fbb4a 100644 --- a/crates/openusd/src/ar.rs +++ b/crates/openusd/src/ar.rs @@ -137,6 +137,17 @@ impl Asset for io::Cursor> { fn size(&self) -> io::Result { Ok(self.get_ref().len() as u64) } + + /// A cursor still at its start hands its buffer over instead of copying + /// it, leaving itself empty. + fn read_all(&mut self) -> io::Result> { + if self.position() != 0 { + let mut buf = Vec::new(); + self.read_to_end(&mut buf)?; + return Ok(buf); + } + Ok(std::mem::take(self.get_mut())) + } } /// Interface for resolving asset paths to physical locations. @@ -1057,6 +1068,21 @@ mod tests { assert_eq!(result, data); } + /// A cursor at its start hands its buffer over; one already read into + /// returns the rest. + #[test] + fn cursor_asset_read_all_moves_buffer() { + let data = b"hello world".to_vec(); + let pointer = data.as_ptr(); + let mut asset = io::Cursor::new(data); + let moved = asset.read_all().unwrap(); + assert_eq!(moved.as_ptr(), pointer); + + let mut asset = io::Cursor::new(b"hello world".to_vec()); + asset.seek(io::SeekFrom::Start(6)).unwrap(); + assert_eq!(asset.read_all().unwrap(), b"world"); + } + #[test] fn cursor_asset_seek() { let data = b"hello world".to_vec(); From c7606f355c34ad03e4a8c1ac185b4e2194835904 Mon Sep 17 00:00:00 2001 From: Trim Bresilla Date: Sun, 4 Oct 2026 18:40:33 +0200 Subject: [PATCH 2/9] perf(ar): share asset bytes with file formats --- crates/openusd/src/ar.rs | 8 +++ crates/openusd/src/sdf/file_format.rs | 13 +++- crates/openusd/src/sdf/layer_registry.rs | 24 +++++-- crates/openusd/src/usdc/mod.rs | 79 +++++++++++++++++++++++- 4 files changed, 117 insertions(+), 7 deletions(-) diff --git a/crates/openusd/src/ar.rs b/crates/openusd/src/ar.rs index 4a8fbb4a..3441d4d0 100644 --- a/crates/openusd/src/ar.rs +++ b/crates/openusd/src/ar.rs @@ -118,6 +118,14 @@ pub trait Asset: Read + Seek + Send { /// Returns the total size of the asset in bytes. fn size(&self) -> io::Result; + /// The complete asset as bytes shared with the caller, whatever the + /// cursor position, for an asset already held in memory (C++ + /// `ArAsset::GetBuffer`). A format that decodes in place keeps them + /// instead of copying; `None`, the default, has the asset read instead. + fn shared_bytes(&self) -> Option> { + None + } + /// Reads the entire asset into a byte buffer. fn read_all(&mut self) -> io::Result> { let size = self.size()? as usize; diff --git a/crates/openusd/src/sdf/file_format.rs b/crates/openusd/src/sdf/file_format.rs index 06339e0d..f5d6c786 100644 --- a/crates/openusd/src/sdf/file_format.rs +++ b/crates/openusd/src/sdf/file_format.rs @@ -163,6 +163,13 @@ pub trait FileFormat: Sync { /// decode without a copy while bytes just read off disk move in. fn read_bytes(&self, bytes: Cow<'static, [u8]>, source_name: &str) -> Result; + /// [`read_bytes`](Self::read_bytes) for bytes shared with an asset + /// ([`ar::Asset::shared_bytes`]). A format that decodes in place keeps + /// them; the default copies them into `read_bytes`. + fn read_shared_bytes(&self, bytes: std::sync::Arc<[u8]>, source_name: &str) -> Result { + self.read_bytes(bytes.as_ref().to_vec().into(), source_name) + } + /// Read a layer's data from `resolved`, opening the asset (and any /// sibling assets) through `resolver`. /// @@ -170,7 +177,11 @@ pub trait FileFormat: Sync { /// [`read_bytes`](Self::read_bytes); one that reaches for sibling assets /// overrides this. fn read(&self, resolver: &dyn ar::Resolver, resolved: &ar::ResolvedPath) -> Result { - let bytes = resolver.open_asset(resolved)?.read_all()?; + let mut asset = resolver.open_asset(resolved)?; + if let Some(bytes) = asset.shared_bytes() { + return self.read_shared_bytes(bytes, &resolved.to_string()); + } + let bytes = asset.read_all()?; self.read_bytes(bytes.into(), &resolved.to_string()) } diff --git a/crates/openusd/src/sdf/layer_registry.rs b/crates/openusd/src/sdf/layer_registry.rs index 4cbcd58c..22c62308 100644 --- a/crates/openusd/src/sdf/layer_registry.rs +++ b/crates/openusd/src/sdf/layer_registry.rs @@ -256,6 +256,20 @@ impl LayerRegistry { .read_bytes(bytes, source_name) } + /// [`read_bytes`](Self::read_bytes) for bytes shared with an asset, which + /// a format that decodes in place keeps rather than copies. + pub fn read_shared_bytes( + bytes: std::sync::Arc<[u8]>, + source_name: &str, + ) -> Result { + DEFAULT_FORMATS + .iter() + .copied() + .find(|format| format.matches_content(&bytes)) + .ok_or_else(|| sdf::FormatError::Unrecognized(source_name.into()))? + .read_shared_bytes(bytes, source_name) + } + /// Find the format claiming `ext` (without the leading dot, case-insensitive), /// e.g. `"usda"` or `"usd"`. C++ `SdfFileFormat::FindByExtension`. pub fn find_by_extension(ext: &str) -> Option<&'static dyn sdf::FileFormat> { @@ -454,11 +468,11 @@ impl LayerRegistry { fn read(&self, resolved: &ar::ResolvedPath) -> Result { let ext = resolved.extension(); if ext.eq_ignore_ascii_case("usd") { - let bytes = self - .resolver - .open_asset(resolved) - .and_then(|mut asset| asset.read_all()) - .map_err(sdf::FormatError::from)?; + let mut asset = self.resolver.open_asset(resolved).map_err(sdf::FormatError::from)?; + if let Some(bytes) = asset.shared_bytes() { + return Ok(Self::read_shared_bytes(bytes, &resolved.to_string())?); + } + let bytes = asset.read_all().map_err(sdf::FormatError::from)?; return Ok(Self::read_bytes(bytes.into(), &resolved.to_string())?); } Ok(Self::find_by_extension(&ext) diff --git a/crates/openusd/src/usdc/mod.rs b/crates/openusd/src/usdc/mod.rs index e501854b..45a0f0d6 100644 --- a/crates/openusd/src/usdc/mod.rs +++ b/crates/openusd/src/usdc/mod.rs @@ -381,6 +381,16 @@ impl sdf::FileFormat for UsdcFileFormat { Ok(Box::new(data)) } + fn read_shared_bytes( + &self, + bytes: std::sync::Arc<[u8]>, + _source_name: &str, + ) -> Result { + let data = + CrateData::open(io::Cursor::new(bytes), true).map_err(|error| sdf::FormatError::Decode(Box::new(error)))?; + Ok(Box::new(data)) + } + fn matches_content(&self, prefix: &[u8]) -> bool { prefix.starts_with(MAGIC) } @@ -399,7 +409,74 @@ const CRATE_PROPERTY_CHILDREN: &str = "properties"; #[cfg(test)] mod tests { use super::*; - use crate::Result; + use crate::sdf::FileFormat; + use crate::{Result, ar}; + use std::sync::Arc; + + /// An asset over shared bytes, as a resolver holding assets in memory + /// serves them. + struct SharedAsset(io::Cursor>); + + impl io::Read for SharedAsset { + fn read(&mut self, buf: &mut [u8]) -> io::Result { + self.0.read(buf) + } + } + + impl io::Seek for SharedAsset { + fn seek(&mut self, pos: io::SeekFrom) -> io::Result { + self.0.seek(pos) + } + } + + impl ar::Asset for SharedAsset { + fn size(&self) -> io::Result { + Ok(self.0.get_ref().len() as u64) + } + + fn shared_bytes(&self) -> Option> { + Some(self.0.get_ref().clone()) + } + } + + struct SharedResolver(Arc<[u8]>); + + impl ar::Resolver for SharedResolver { + fn create_identifier(&self, asset_path: &str, _anchor: Option<&ar::ResolvedPath>) -> String { + asset_path.to_string() + } + + fn resolve(&self, asset_path: &str) -> Option { + Some(ar::ResolvedPath::new(asset_path)) + } + + fn resolve_for_new_asset(&self, asset_path: &str) -> Option { + Some(ar::ResolvedPath::new(asset_path)) + } + + fn open_asset(&self, _resolved_path: &ar::ResolvedPath) -> io::Result> { + Ok(Box::new(SharedAsset(io::Cursor::new(self.0.clone())))) + } + } + + /// A crate layer read from an asset that shares its bytes decodes from + /// those bytes, holding a reference rather than a copy. + #[test] + fn shared_asset_bytes_are_kept() -> Result<()> { + let mut layer = sdf::Data::new(); + layer.create_spec(sdf::Path::abs_root(), sdf::SpecType::PseudoRoot); + let mut bytes = io::Cursor::new(Vec::new()); + CrateWriter::write(&layer, &mut bytes)?; + let bytes: Arc<[u8]> = bytes.into_inner().into(); + + let resolver = SharedResolver(bytes.clone()); + let data = UsdcFileFormat.read(&resolver, &ar::ResolvedPath::new("shared.usdc"))?; + assert!(data.has_spec(&sdf::Path::abs_root())); + assert_eq!(Arc::strong_count(&bytes), 3); + drop(data); + assert_eq!(Arc::strong_count(&bytes), 2); + Ok(()) + } use crate::gf; use crate::gf::f16; From 17361c0e34202a7d51cf50ce102b39111f6aad95 Mon Sep 17 00:00:00 2001 From: Trim Bresilla Date: Sun, 4 Oct 2026 18:41:26 +0200 Subject: [PATCH 3/9] perf(sdf): share a path's text between clones --- crates/openusd/src/sdf/path.rs | 98 +++++++++++++++++++++++++--------- 1 file changed, 72 insertions(+), 26 deletions(-) diff --git a/crates/openusd/src/sdf/path.rs b/crates/openusd/src/sdf/path.rs index 413e629b..9db10c65 100644 --- a/crates/openusd/src/sdf/path.rs +++ b/crates/openusd/src/sdf/path.rs @@ -40,9 +40,12 @@ pub fn try_into_path(path: impl IntoPath) -> Result { /// Parsing via [`Path::new`] (or [`FromStr`]) validates this grammar and /// rejects malformed text with a [`PathParseError`]. The empty path is not /// parseable; construct it with [`Path::default`]. +/// +/// Clones share the path's text rather than copying it, as C++ `SdfPath` +/// copies share one pooled path node. #[derive(Debug, Default, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct Path { - path: String, + path: std::sync::Arc, } impl fmt::Display for Path { @@ -54,7 +57,7 @@ impl fmt::Display for Path { #[cfg(feature = "serde")] impl serde::Serialize for Path { fn serialize(&self, serializer: S) -> Result { - self.path.serialize(serializer) + self.as_str().serialize(serializer) } } @@ -94,7 +97,9 @@ impl FromStr for Path { fn from_str(s: &str) -> Result { Path::validate(s)?; - Ok(Path { path: s.to_string() }) + Ok(Path { + path: s.to_owned().into(), + }) } } @@ -121,7 +126,9 @@ impl Path { path.is_empty() || Path::validate(path).is_ok(), "from_str_unchecked on invalid path {path:?}" ); - Path { path: path.to_string() } + Path { + path: path.to_owned().into(), + } } #[inline] @@ -132,7 +139,7 @@ impl Path { /// Returns `true` if this is the absolute root path `/` (pseudo-root). #[inline] pub fn is_abs_root(&self) -> bool { - self.path == "/" + self.as_str() == "/" } /// Whether this path's prim is a root prim — a direct child of the @@ -162,7 +169,7 @@ impl Path { // The pseudo-root, the empty path, and the `.`/`..` relative anchors // cannot own properties; appending to them would build an unparseable // path like `/.foo`. - if self.is_empty() || self.is_abs_root() || self.path == "." || self.path.ends_with("..") { + if self.is_empty() || self.is_abs_root() || self.as_str() == "." || self.path.ends_with("..") { return Err(fail(0, "path cannot own properties")); } if !Path::is_valid_namespace_identifier(property) { @@ -172,11 +179,11 @@ impl Path { )); } - let mut new_path = self.path.clone(); + let mut new_path = self.path.to_string(); new_path.push('.'); new_path.push_str(property); - Ok(Path { path: new_path }) + Ok(Path { path: new_path.into() }) } /// Appends `path` (parsed if given as a string) under this path with a `/` @@ -187,7 +194,7 @@ impl Path { if self.is_abs() && append.is_abs() { return Err(PathParseError { - input: append.path, + input: append.path.to_string(), offset: 0, reason: "cannot append an absolute path to an absolute path", }); @@ -195,7 +202,7 @@ impl Path { if self.is_property_path() { return Err(PathParseError { - input: self.path.clone(), + input: self.path.to_string(), offset: self.path.rfind('.').unwrap_or(0), reason: "cannot append a path to a property path", }); @@ -208,7 +215,7 @@ impl Path { // The reflexive base is the identity anchor: appending to `.` yields // the argument itself (C++ `SdfPath::AppendPath` on the reflexive // relative path). - if self.path == "." { + if self.as_str() == "." { return Ok(append); } @@ -217,7 +224,7 @@ impl Path { // a valid path. if append.as_str().starts_with('.') { return Err(PathParseError { - input: append.path, + input: append.path.to_string(), offset: 0, reason: "cannot append a `.`-anchored path under a prim path", }); @@ -225,7 +232,7 @@ impl Path { // If base is slash only. // "/" + "foo/bar" => "/foo/bar" - let combined = if self.path.as_str() == "/" { + let combined = if self.as_str() == "/" { format!("/{}", append.path) } else if self.is_prim_variant_selection_path() { // A prim child attaches directly to a variant selection with no @@ -235,7 +242,7 @@ impl Path { format!("{}/{}", self.path, append.path) }; - Ok(Path { path: combined }) + Ok(Path { path: combined.into() }) } pub fn is_property_path(&self) -> bool { @@ -278,13 +285,13 @@ impl Path { /// path. pub fn is_prim_path(&self) -> bool { // The relative anchors: `.`, and a `..(/..)*` run with nothing after it. - if self.path == "." || (!self.path.is_empty() && self.path.split('/').all(|seg| seg == "..")) { + if self.as_str() == "." || (!self.path.is_empty() && self.path.split('/').all(|seg| seg == "..")) { return true; } // Strip the anchor the grammar allows ahead of the prim chain, then ask // the prim-chain iterator what the last component was. A non-empty // remainder means a property tail the chain could not consume. - let mut rest = self.path.as_str(); + let mut rest = self.as_str(); if let Some(after) = rest.strip_prefix('/') { rest = after; } else { @@ -476,7 +483,7 @@ impl Path { /// "/A{set=sel}" -> Some(Variant { set: "set", selection: "sel" }) /// ``` pub fn last_element(&self) -> Option> { - if self.path.is_empty() || self.path == "/" { + if self.path.is_empty() || self.as_str() == "/" { return None; } // A property's element is the whole property name (everything after @@ -517,7 +524,7 @@ impl Path { /// "" -> None /// ``` pub fn parent(&self) -> Option { - if self.path.is_empty() || self.path == "/" { + if self.path.is_empty() || self.as_str() == "/" { return None; } // Drop a trailing `{set=sel}` variant selection. @@ -622,7 +629,7 @@ impl Path { /// "" -> None /// ``` pub fn name(&self) -> Option<&str> { - if self.path.is_empty() || self.path == "/" { + if self.path.is_empty() || self.as_str() == "/" { return None; } // The final prim name begins after the rightmost `/` or `}` (a child of @@ -633,7 +640,7 @@ impl Path { if start < self.path.len() { Some(&self.path[start..]) } else { - Some(self.path.rsplit_once('/').map_or(self.path.as_str(), |(_, name)| name)) + Some(self.path.rsplit_once('/').map_or(self.as_str(), |(_, name)| name)) } } @@ -702,7 +709,7 @@ impl Path { return self.clone(); } let mut out = String::with_capacity(self.path.len()); - let mut rest = self.path.as_str(); + let mut rest = self.as_str(); let mut depth = 0usize; let mut chars = rest.char_indices(); // Splice out each depth-0 `{…}` span; bracketed spans pass through. @@ -875,7 +882,7 @@ impl Path { if self.is_empty() || self.is_abs_root() || self.is_property_path() - || self.path == "." + || self.as_str() == "." || self.path.ends_with("..") || self.path.ends_with(']') { @@ -1335,10 +1342,10 @@ impl TryFrom<&str> for Path { impl TryFrom for Path { type Error = PathParseError; - /// Validates and reuses `value`'s allocation. + /// Validates `value` and shares its existing text allocation. fn try_from(value: String) -> Result { Path::validate(&value)?; - Ok(Path { path: value }) + Ok(Path { path: value.into() }) } } @@ -1356,6 +1363,39 @@ mod tests { use super::*; + #[test] + fn clones_share_text_and_derivations_preserve_original() { + let path = Path::new("/Root/Branch").unwrap(); + let clone = path.clone(); + assert!(std::sync::Arc::ptr_eq(&path.path, &clone.path)); + assert_eq!(clone.append_property("size").unwrap().as_str(), "/Root/Branch.size"); + assert_eq!(clone.append_path("Leaf").unwrap().as_str(), "/Root/Branch/Leaf"); + assert_eq!(path.as_str(), "/Root/Branch"); + let text = String::from("/Root/Branch"); + let allocation = text.as_ptr(); + let independent = Path::try_from(text).unwrap(); + assert_eq!(independent.as_str().as_ptr(), allocation); + assert_eq!(path, independent); + let mut paths = std::collections::HashSet::new(); + paths.insert(path); + assert!(paths.contains(&independent)); + } + + #[cfg(feature = "serde")] + #[test] + fn shared_paths_preserve_string_serialization() { + for path in [ + Path::default(), + Path::abs_root(), + Path::new("/Root.rel[/Target]").unwrap(), + ] { + let encoded = serde_json::to_string(&path).unwrap(); + assert_eq!(encoded, serde_json::to_string(path.as_str()).unwrap()); + assert_eq!(serde_json::from_str::(&encoded).unwrap(), path); + } + assert!(serde_json::from_str::("\"/invalid path\"").is_err()); + } + /// `is_prim_path` classifies by the path's final element, so every tail /// shape lands where C++ `SdfPath::IsPrimPath` puts it — including `.` and /// `..`, which C++ builds as prim nodes, and `/A.rel[/T]`, which the @@ -1384,7 +1424,9 @@ mod tests { /// Builds a `Path` directly from `path`, skipping validation — for /// exercising lenient read-side behavior on malformed input. fn raw(path: &str) -> Path { - Path { path: path.to_string() } + Path { + path: path.to_owned().into(), + } } #[test] @@ -1668,7 +1710,11 @@ mod tests { #[test] fn test_split_property() { - let split = |s: &str| raw(s).split_property().map(|(p, n)| (p.path, n.to_owned())); + let split = |s: &str| { + raw(s) + .split_property() + .map(|(p, n)| (p.as_str().to_owned(), n.to_owned())) + }; let owned = |p: &str, n: &str| Some((p.to_owned(), n.to_owned())); assert_eq!(split("/World/Mesh.points"), owned("/World/Mesh", "points")); From beba7b096cee48247352cc10efd5a99a4c9ae403 Mon Sep 17 00:00:00 2001 From: Trim Bresilla Date: Sun, 4 Oct 2026 18:44:21 +0200 Subject: [PATCH 4/9] perf(usd): read root layers once when opening --- crates/openusd/src/sdf/layer_registry.rs | 75 ++++++++++++++++++------ crates/openusd/src/usd/stage.rs | 53 ++++++++++++++--- crates/openusd/tests/stage.rs | 21 +++++++ 3 files changed, 124 insertions(+), 25 deletions(-) diff --git a/crates/openusd/src/sdf/layer_registry.rs b/crates/openusd/src/sdf/layer_registry.rs index 22c62308..01bb0f5f 100644 --- a/crates/openusd/src/sdf/layer_registry.rs +++ b/crates/openusd/src/sdf/layer_registry.rs @@ -147,6 +147,18 @@ pub struct LayerRegistry { resolver: Box, } +/// A stack's root layer, read ahead of its stack by +/// [`LayerRegistry::prepare_root`] and handed to +/// [`LayerRegistry::open_prepared_stack`] so it is not read twice. +pub(crate) struct PreparedLayer { + /// The layer's canonical identifier. + pub identifier: String, + /// Where the layer was found. + pub resolved: ar::ResolvedPath, + /// The layer's data. + pub data: sdf::LayerData, +} + impl Default for LayerRegistry { /// A registry over the filesystem [`DefaultResolver`](ar::DefaultResolver) /// and the built-in formats — what [`Stage::builder`](crate::usd::Stage) @@ -306,30 +318,32 @@ impl LayerRegistry { } } - /// The `expressionVariables` authored on the single layer at `asset_path` - /// (anchored against `anchor`), read without opening its sublayers — the shallow - /// read the stage root stack needs to compose its root and session layers' own - /// variables into one context before either region's sublayer subtree is - /// collected. An empty identifier yields an empty map; a resolve or read failure - /// propagates. - /// - /// TODO(perf): the layer read here is read again when its stack is collected; - /// the registry does not cache reads, so a root or session layer is parsed twice - /// at open. - pub(crate) fn own_expression_variables( + /// Reads the single layer at `asset_path` (anchored against `anchor`) + /// without opening its sublayers: the shallow read the stage root stack + /// needs to compose its root and session layers' own expression variables + /// into one context before either region's sublayer subtree is collected. + /// The read is then handed to + /// [`open_prepared_stack`](Self::open_prepared_stack), so the layer is + /// parsed once at open. An empty identifier yields `None`; a resolve or + /// read failure propagates. + pub(crate) fn prepare_root( &self, asset_path: &str, anchor: Option<&ar::ResolvedPath>, - ) -> Result, LoadError> { + ) -> Result, LoadError> { let identifier = self.create_identifier(asset_path, anchor); if identifier.is_empty() { - return Ok(HashMap::new()); + return Ok(None); } let resolved = self.resolve_layer(&identifier).ok_or_else(|| LoadError::Unresolved { asset_path: asset_path.to_owned(), })?; let data = self.read(&resolved)?; - Ok(expr::read_expression_variables(data.as_ref())?.into_owned()) + Ok(Some(PreparedLayer { + identifier, + resolved, + data, + })) } /// Opens the layer at `identifier` — a canonical identifier, as @@ -377,9 +391,6 @@ impl LayerRegistry { on_error: &dyn Fn(Error) -> Result<(), Error>, already_present: &dyn Fn(&str) -> bool, ) -> Result>, LoadError> { - let mut layers = Vec::new(); - let mut visited = HashSet::new(); - if identifier.is_empty() { return Ok(None); } @@ -393,6 +404,36 @@ impl LayerRegistry { return Ok(None); }; let data = self.read(&resolved)?; + self.open_prepared_stack( + PreparedLayer { + identifier, + resolved, + data, + }, + ancestor_expr_vars, + reload, + on_error, + already_present, + ) + } + + /// [`open_stack`](Self::open_stack) from a root layer already read by + /// [`prepare_root`](Self::prepare_root). + pub(crate) fn open_prepared_stack( + &self, + root: PreparedLayer, + ancestor_expr_vars: &HashMap, + reload: bool, + on_error: &dyn Fn(Error) -> Result<(), Error>, + already_present: &dyn Fn(&str) -> bool, + ) -> Result>, LoadError> { + let PreparedLayer { + identifier, + resolved, + data, + } = root; + let mut layers = Vec::new(); + let mut visited = HashSet::new(); visited.insert(identifier.clone()); // The whole stack resolves its `${VAR}` sublayers against one context (C++ diff --git a/crates/openusd/src/usd/stage.rs b/crates/openusd/src/usd/stage.rs index baef53ff..328523a9 100644 --- a/crates/openusd/src/usd/stage.rs +++ b/crates/openusd/src/usd/stage.rs @@ -3635,9 +3635,20 @@ impl StageBuilder { // a variable authored on the stage root layer (and a root sublayer one on the // session), and composition later resolves each `${VAR}` sublayer to the same // layer this collection opened. - let root_stack_vars = self.root_stack_expression_variables(root_path)?; - let session = self.collect_optional_session_layers(&root_stack_vars)?; - let root = self.collect_layers(root_path, &root_stack_vars)?; + let root_data = self.registry.prepare_root(root_path, None)?; + let session_data = self + .session_layer + .as_deref() + .map(|path| self.registry.prepare_root(path, None)) + .transpose()? + .flatten(); + let root_stack_vars = + self.root_stack_expression_variables(root_path, root_data.as_ref(), session_data.as_ref())?; + let session = match self.session_layer.as_deref() { + Some(path) => self.collect_prepared_layers(path, &root_stack_vars, session_data)?, + None => CollectedLayers::default(), + }; + let root = self.collect_prepared_layers(root_path, &root_stack_vars, root_data)?; let session_layer_count = session.layers.len(); let layers = session.layers.into_iter().chain(root.layers).collect(); let diagnostics = session.diagnostics.into_iter().chain(root.diagnostics).collect(); @@ -3686,14 +3697,27 @@ impl StageBuilder { /// later, once the muted-aware graph exists (see /// [`StageBuilder::make_stage`](Self::make_stage)). fn collect_layers(&self, path: &str, ancestor_expr_vars: &HashMap) -> Result { + self.collect_prepared_layers(path, ancestor_expr_vars, self.registry.prepare_root(path, None)?) + } + + /// [`collect_layers`](Self::collect_layers) from the root layer at `path` + /// already read by [`LayerRegistry::prepare_root`]. + fn collect_prepared_layers( + &self, + path: &str, + ancestor_expr_vars: &HashMap, + root: Option, + ) -> Result { let diagnostics = RefCell::new(pcp::Diagnostics::default()); // `ancestor_expr_vars` are the expression variables the enclosing context // contributes: the session layers' composed set for the root stack, empty // for the session stack itself (nothing sublayers it). let layers = self .registry - .open_stack( - &self.registry.create_identifier(path, None), + .open_prepared_stack( + root.ok_or_else(|| sdf::LoadError::Unresolved { + asset_path: path.to_owned(), + })?, ancestor_expr_vars, false, &|error| { @@ -3754,13 +3778,26 @@ impl StageBuilder { /// contributing none. Read shallowly from the two root layers — their sublayers /// contribute nothing — since it is the fixed context both the session region's /// and the root region's `${VAR}` sublayers resolve against. - fn root_stack_expression_variables(&self, root_path: &str) -> Result> { - let mut vars = self.registry.own_expression_variables(root_path, None)?; + fn root_stack_expression_variables( + &self, + root_path: &str, + root: Option<&sdf::layer_registry::PreparedLayer>, + session: Option<&sdf::layer_registry::PreparedLayer>, + ) -> Result> { + let mut vars = root + .map(|root| sdf::expr::read_expression_variables(root.data.as_ref()).map(|vars| vars.into_owned())) + .transpose()? + .unwrap_or_default(); if let Some(session_path) = self.session_layer.as_deref() { let session_id = self.registry.create_identifier(session_path, None); let muted = !self.muted.is_empty() && self.canonical_muted_set(root_path).contains(&session_id); if !muted { - let session_own = self.registry.own_expression_variables(session_path, None)?; + let session_own = session + .map(|session| { + sdf::expr::read_expression_variables(session.data.as_ref()).map(|vars| vars.into_owned()) + }) + .transpose()? + .unwrap_or_default(); sdf::expr::compose_over(&mut vars, &session_own); } } diff --git a/crates/openusd/tests/stage.rs b/crates/openusd/tests/stage.rs index bf2cb543..6a4828f5 100644 --- a/crates/openusd/tests/stage.rs +++ b/crates/openusd/tests/stage.rs @@ -3219,6 +3219,27 @@ fn lazy_reference_loads_on_demand() -> Result<()> { Ok(()) } +/// Opening a stage reads its root and session layers once each: the read that +/// composes their expression variables is the one their stacks are built from. +#[test] +fn root_and_session_layers_read_once() -> Result<()> { + let dir = tempfile::tempdir()?; + let root = dir.path().join("root.usda"); + let session = dir.path().join("session.usda"); + std::fs::write(&root, "#usda 1.0\ndef \"World\" {}\n")?; + std::fs::write(&session, "#usda 1.0\n")?; + let opened = Rc::new(RefCell::new(Vec::new())); + Stage::builder() + .resolver(RecordingResolver::new(opened.clone())) + .session_layer(session.to_str().unwrap()) + .open(root.to_str().unwrap())?; + + let opens = |name: &str| opened.borrow().iter().filter(|path| path.ends_with(name)).count(); + assert_eq!(opens("root.usda"), 1); + assert_eq!(opens("session.usda"), 1); + Ok(()) +} + /// A muted reference target contributes nothing and is never read from disk, /// even once composition reaches its arc, and surfaces a `MutedAssetPath` /// diagnostic. From b50760f58189e8f003d31ba6d8d46c4deb1e7878 Mon Sep 17 00:00:00 2001 From: Trim Bresilla Date: Sun, 4 Oct 2026 18:45:17 +0200 Subject: [PATCH 5/9] perf(usd): reuse the active check for loaded --- crates/openusd/src/usd/prim.rs | 8 +++++++- crates/openusd/src/usd/stage.rs | 7 ++++++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/crates/openusd/src/usd/prim.rs b/crates/openusd/src/usd/prim.rs index 4141c452..00f23dbf 100644 --- a/crates/openusd/src/usd/prim.rs +++ b/crates/openusd/src/usd/prim.rs @@ -769,7 +769,13 @@ impl Prim { /// or above it (per the stage's runtime load rules) is excluded. Mirrors /// C++ `UsdPrim::IsLoaded`. pub fn is_loaded(&self) -> Result { - if !self.is_active()? { + self.is_loaded_with_active(self.is_active()?) + } + + /// [`is_loaded`](Self::is_loaded) for a prim whose + /// [`is_active`](Self::is_active) is already known to be `active`. + pub(crate) fn is_loaded_with_active(&self, active: bool) -> Result { + if !active { return Ok(false); } // No rule anywhere means every path resolves loaded (`LoadRules`' diff --git a/crates/openusd/src/usd/stage.rs b/crates/openusd/src/usd/stage.rs index 328523a9..6da4b17b 100644 --- a/crates/openusd/src/usd/stage.rs +++ b/crates/openusd/src/usd/stage.rs @@ -3197,7 +3197,12 @@ impl Stage { status.set(PrimStatus::ACTIVE, prim.is_active()?); } if mask.contains(PrimStatus::LOADED) { - status.set(PrimStatus::LOADED, prim.is_loaded()?); + let loaded = if mask.contains(PrimStatus::ACTIVE) { + prim.is_loaded_with_active(status.contains(PrimStatus::ACTIVE))? + } else { + prim.is_loaded()? + }; + status.set(PrimStatus::LOADED, loaded); } if mask.contains(PrimStatus::DEFINED) { status.set(PrimStatus::DEFINED, prim.is_defined()?); From 68aca83e1ebe265216de345579aaa2a9c311fed0 Mon Sep 17 00:00:00 2001 From: Trim Bresilla Date: Sun, 4 Oct 2026 18:46:20 +0200 Subject: [PATCH 6/9] perf(usd): resolve abstract ancestry in one query --- crates/openusd/src/pcp/index_cache.rs | 24 ++++++++++++++++++++++++ crates/openusd/src/usd/prim.rs | 12 +++--------- 2 files changed, 27 insertions(+), 9 deletions(-) diff --git a/crates/openusd/src/pcp/index_cache.rs b/crates/openusd/src/pcp/index_cache.rs index 6a442d17..20122b55 100644 --- a/crates/openusd/src/pcp/index_cache.rs +++ b/crates/openusd/src/pcp/index_cache.rs @@ -2090,6 +2090,30 @@ impl IndexCache { Ok(true) } + /// Whether `path` or any ancestor below the pseudo-root resolves to + /// `class` (C++ `UsdPrim::IsAbstract`). A prim with no composed spec is + /// not abstract, and a prototype root is `def` whatever its source says, + /// as C++ `Usd_PrimData` sets it. Resolved like [`Self::is_defined`], + /// under one cache borrow for the whole ancestor chain. + pub(crate) fn is_abstract(&mut self, graph: &LayerGraph, path: &Path) -> Result { + if path.is_abs_root() || !self.has_spec(graph, path)? { + return Ok(false); + } + for ancestor in path.ancestors_below_root() { + if self.is_prototype(&ancestor) { + break; + } + let specifier = self + .resolve_field(graph, &ancestor, FieldKey::Specifier.as_str())? + .map(sdf::Specifier::try_from) + .transpose()?; + if specifier == Some(sdf::Specifier::Class) { + return Ok(true); + } + } + Ok(false) + } + /// This prim's own composed `active` opinion, defaulting to `true`. The /// per-prim read [`Self::is_active`] walks and [`Self::is_populated`] takes /// for the prim it is deciding, its ancestors having been decided already. diff --git a/crates/openusd/src/usd/prim.rs b/crates/openusd/src/usd/prim.rs index 00f23dbf..a010fcf6 100644 --- a/crates/openusd/src/usd/prim.rs +++ b/crates/openusd/src/usd/prim.rs @@ -820,15 +820,9 @@ impl Prim { /// `true` if the prim or any ancestor resolves to `class`. Mirrors C++ /// `UsdPrim::IsAbstract`. pub fn is_abstract(&self) -> Result { - if self.path == sdf::Path::abs_root() || !self.stage.has_spec(&self.path)? { - return Ok(false); - } - for path in self.path.ancestors_below_root() { - if self.stage.field::(&path, sdf::FieldKey::Specifier)? == Some(sdf::Specifier::Class) { - return Ok(true); - } - } - Ok(false) + Ok(self + .stage + .masked(&self.path, |g, cache| cache.is_abstract(g, &self.path))?) } /// `true` if the prim index contains at least one composition arc. From 660c39c9b01f345154cdc01bda75813e7ec6db4c Mon Sep 17 00:00:00 2001 From: Trim Bresilla Date: Sun, 4 Oct 2026 18:50:37 +0200 Subject: [PATCH 7/9] perf(usd): resolve defined and abstract in one walk --- crates/openusd/src/pcp/index_cache.rs | 27 ++++++++++++ crates/openusd/src/usd/stage.rs | 9 ++-- crates/openusd/tests/stage.rs | 62 +++++++++++++++++++++++++++ 3 files changed, 95 insertions(+), 3 deletions(-) diff --git a/crates/openusd/src/pcp/index_cache.rs b/crates/openusd/src/pcp/index_cache.rs index 20122b55..f17d897b 100644 --- a/crates/openusd/src/pcp/index_cache.rs +++ b/crates/openusd/src/pcp/index_cache.rs @@ -2114,6 +2114,33 @@ impl IndexCache { Ok(false) } + /// [`Self::is_defined`] and [`Self::is_abstract`] together, from one walk + /// up the ancestor chain, for a caller asking both. + pub(crate) fn specifier_status(&mut self, graph: &LayerGraph, path: &Path) -> Result<(bool, bool), QueryError> { + if path.is_abs_root() { + return Ok((true, false)); + } + if !self.has_spec(graph, path)? { + return Ok((false, false)); + } + let (mut defined, mut is_abstract) = (true, false); + for ancestor in path.ancestors_below_root() { + if self.is_prototype(&ancestor) { + break; + } + let specifier = self + .resolve_field(graph, &ancestor, FieldKey::Specifier.as_str())? + .map(sdf::Specifier::try_from) + .transpose()?; + defined &= matches!(specifier, Some(sdf::Specifier::Def | sdf::Specifier::Class)); + is_abstract |= specifier == Some(sdf::Specifier::Class); + if !defined && is_abstract { + break; + } + } + Ok((defined, is_abstract)) + } + /// This prim's own composed `active` opinion, defaulting to `true`. The /// per-prim read [`Self::is_active`] walks and [`Self::is_populated`] takes /// for the prim it is deciding, its ancestors having been decided already. diff --git a/crates/openusd/src/usd/stage.rs b/crates/openusd/src/usd/stage.rs index 6da4b17b..b8674510 100644 --- a/crates/openusd/src/usd/stage.rs +++ b/crates/openusd/src/usd/stage.rs @@ -3204,10 +3204,13 @@ impl Stage { }; status.set(PrimStatus::LOADED, loaded); } - if mask.contains(PrimStatus::DEFINED) { + if mask.contains(PrimStatus::DEFINED | PrimStatus::ABSTRACT) { + let (defined, is_abstract) = self.masked(prim.path(), |g, c| c.specifier_status(g, prim.path()))?; + status.set(PrimStatus::DEFINED, defined); + status.set(PrimStatus::ABSTRACT, is_abstract); + } else if mask.contains(PrimStatus::DEFINED) { status.set(PrimStatus::DEFINED, prim.is_defined()?); - } - if mask.contains(PrimStatus::ABSTRACT) { + } else if mask.contains(PrimStatus::ABSTRACT) { status.set(PrimStatus::ABSTRACT, prim.is_abstract()?); } if mask.contains(PrimStatus::INSTANCE) { diff --git a/crates/openusd/tests/stage.rs b/crates/openusd/tests/stage.rs index 6a4828f5..fd2a1121 100644 --- a/crates/openusd/tests/stage.rs +++ b/crates/openusd/tests/stage.rs @@ -4899,6 +4899,68 @@ fn prototype_root_active() -> Result<()> { Ok(()) } +/// The defined and abstract bits a prim status reads from one ancestor walk +/// agree with `is_defined` and `is_abstract` asked one at a time, across +/// `def`, `over` and `class` chains and an instance's prototype. +#[test] +fn prim_status_specifier_bits_match_queries() -> Result<()> { + let dir = tempfile::tempdir()?; + let root = dir.path().join("root.usda"); + fs::write( + &root, + r#"#usda 1.0 +class "Class" +{ + def "Child" + { + over "Over" + { + } + } +} + +over "Over" +{ + def "Child" + { + } +} + +def "World" +{ + class "Nested" + { + def "Leaf" + { + } + } +} + +def "A" ( + instanceable = true + references = +) +{ +} +"#, + )?; + let stage = Stage::open(root.to_str().expect("utf-8 temp path"))?; + let mut paths = Vec::new(); + stage.traverse(PrimPredicate::ALL, |path| paths.push(path.clone()))?; + for prototype in stage.prototypes()? { + paths.push(prototype.append_path("Child")?); + paths.push(prototype); + } + assert!(paths.len() > 10); + for path in paths { + let prim = stage.prim(path.clone())?; + let status = stage.prim_status(path.clone())?; + assert_eq!(status.contains(PrimStatus::DEFINED), prim.is_defined()?, "{path}"); + assert_eq!(status.contains(PrimStatus::ABSTRACT), prim.is_abstract()?, "{path}"); + } + Ok(()) +} + /// A prototype root is defined and not abstract whatever its source authors, /// as C++ `Usd_PrimData` sets it, and its children inherit that (spec /// 11.3.3). From a3752503077307cc2abf58471313da2e3af62380 Mon Sep 17 00:00:00 2001 From: Trim Bresilla Date: Sun, 4 Oct 2026 18:54:11 +0200 Subject: [PATCH 8/9] perf(usd): sort properties in one cache query --- crates/openusd/src/usd/prim.rs | 44 +++++++++++++++++++------------- crates/openusd/tests/stage.rs | 46 ++++++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 17 deletions(-) diff --git a/crates/openusd/src/usd/prim.rs b/crates/openusd/src/usd/prim.rs index a010fcf6..c53e309e 100644 --- a/crates/openusd/src/usd/prim.rs +++ b/crates/openusd/src/usd/prim.rs @@ -1172,7 +1172,7 @@ impl Prim { /// /// The namespace narrows the ordered names, so a property keeps the place /// the whole list gave it, and only the names it leaves are asked for - /// their spec type. + /// their spec type, all under one cache borrow. fn properties_of_type(&self, source: PropertySource, ty: sdf::SpecType, namespace: &str) -> Result> { let mut names = match source { PropertySource::Composed => self.property_names()?, @@ -1182,23 +1182,33 @@ impl Prim { let info = self.prim_type_info()?; let definition = info.prim_definition(); - let mut paths = Vec::new(); - for name in names { - let path = self.property_path(&name); - let spec_type = match (self.stage.spec_type(&path)?, source) { - (Some(spec_type), _) => Some(spec_type), - // A property the prim only inherits from its schema has no - // composed spec, so its kind comes from the declaration. - (None, PropertySource::Composed) => definition.property(&name).map(|property| property.spec_type()), - // Nothing a schema declares is authored, so a name with no - // composed spec belongs to neither kind. - (None, PropertySource::Authored) => None, - }; - if spec_type == Some(ty) { - paths.push(path); + Ok(self.stage.masked(&self.path, |graph, cache| { + // A property is one of the prim's opinions, so a prototype root + // has none. + let opinions = !cache.is_prototype(&self.path); + let mut paths = Vec::new(); + for name in &names { + let path = self.property_path(name); + let composed = if opinions && !path.is_empty() { + cache.spec_type(graph, &path)? + } else { + None + }; + let spec_type = match (composed, source) { + (Some(spec_type), _) => Some(spec_type), + // A property the prim only inherits from its schema has no + // composed spec, so its kind comes from the declaration. + (None, PropertySource::Composed) => definition.property(name).map(|property| property.spec_type()), + // Nothing a schema declares is authored, so a name with no + // composed spec belongs to neither kind. + (None, PropertySource::Authored) => None, + }; + if spec_type == Some(ty) { + paths.push(path); + } } - } - Ok(paths) + Ok(paths) + })?) } /// Property path for `name` under this prim. An invalid name yields the diff --git a/crates/openusd/tests/stage.rs b/crates/openusd/tests/stage.rs index fd2a1121..a4367657 100644 --- a/crates/openusd/tests/stage.rs +++ b/crates/openusd/tests/stage.rs @@ -4899,6 +4899,52 @@ fn prototype_root_active() -> Result<()> { Ok(()) } +/// A prim sorts its properties into attributes and relationships by their +/// composed spec, and a prototype root, which reads no opinions, has neither. +#[test] +fn properties_sorted_by_spec_type() -> Result<()> { + let dir = tempfile::tempdir()?; + let root = dir.path().join("root.usda"); + fs::write( + &root, + r#"#usda 1.0 +def "Source" +{ + double size = 1 + rel target = +} + +def "A" ( + instanceable = true + references = +) +{ +} +"#, + )?; + let stage = Stage::open(root.to_str().expect("utf-8 temp path"))?; + let names = |properties: Vec| -> Vec { + properties + .iter() + .map(|path| path.as_str().rsplit_once('.').unwrap_or_default().1.to_owned()) + .collect() + }; + let source = stage.prim("/Source")?; + assert_eq!( + names(source.attributes()?.iter().map(|a| a.path().clone()).collect()), + ["size"] + ); + assert_eq!( + names(source.relationships()?.iter().map(|r| r.path().clone()).collect()), + ["target"] + ); + + let prototype = stage.prim(stage.prim("/A")?.prototype()?.expect("A is an instance"))?; + assert!(prototype.attributes()?.is_empty()); + assert!(prototype.relationships()?.is_empty()); + Ok(()) +} + /// The defined and abstract bits a prim status reads from one ancestor walk /// agree with `is_defined` and `is_abstract` asked one at a time, across /// `def`, `over` and `class` chains and an instance's prototype. From 04b168619ee509da94125d6b60a6d7140ff755a4 Mon Sep 17 00:00:00 2001 From: Trim Bresilla Date: Sun, 4 Oct 2026 18:57:46 +0200 Subject: [PATCH 9/9] perf(usd): carry parent status through traversal --- crates/openusd/src/pcp/index_cache.rs | 24 ++++++ crates/openusd/src/usd/prim.rs | 9 +++ crates/openusd/src/usd/stage.rs | 59 +++++++++++--- crates/openusd/tests/stage.rs | 110 ++++++++++++++++++++++++++ 4 files changed, 189 insertions(+), 13 deletions(-) diff --git a/crates/openusd/src/pcp/index_cache.rs b/crates/openusd/src/pcp/index_cache.rs index f17d897b..6b271e9c 100644 --- a/crates/openusd/src/pcp/index_cache.rs +++ b/crates/openusd/src/pcp/index_cache.rs @@ -2141,6 +2141,30 @@ impl IndexCache { Ok((defined, is_abstract)) } + /// The active, defined and abstract state of `path` from its own opinions + /// alone, for a prim whose parent resolved active, defined and not + /// abstract: below such a parent, [`Self::is_active`], + /// [`Self::is_defined`] and [`Self::is_abstract`] are each decided at the + /// prim itself. + pub(crate) fn local_status(&mut self, graph: &LayerGraph, path: &Path) -> Result<(bool, bool, bool), QueryError> { + if !self.has_spec(graph, path)? { + return Ok((false, false, false)); + } + let active = self.active_locally(graph, path)?; + if self.is_prototype(path) { + return Ok((active, true, false)); + } + let specifier = self + .resolve_field(graph, path, FieldKey::Specifier.as_str())? + .map(sdf::Specifier::try_from) + .transpose()?; + Ok(( + active, + matches!(specifier, Some(sdf::Specifier::Def | sdf::Specifier::Class)), + specifier == Some(sdf::Specifier::Class), + )) + } + /// This prim's own composed `active` opinion, defaulting to `true`. The /// per-prim read [`Self::is_active`] walks and [`Self::is_populated`] takes /// for the prim it is deciding, its ancestors having been decided already. diff --git a/crates/openusd/src/usd/prim.rs b/crates/openusd/src/usd/prim.rs index c53e309e..baa6e765 100644 --- a/crates/openusd/src/usd/prim.rs +++ b/crates/openusd/src/usd/prim.rs @@ -791,6 +791,15 @@ impl Prim { Ok(true) } + /// [`is_loaded_with_active`](Self::is_loaded_with_active) for a prim + /// whose parent is loaded, so only its own payload can leave it unloaded. + pub(crate) fn is_loaded_below_loaded(&self, active: bool) -> Result { + if !active || self.stage.cache().load_rules().is_empty() { + return Ok(active); + } + Ok(!has_payload(&self.stage, &self.path)? || self.stage.is_path_loaded(&self.path)) + } + /// Loads this prim's payload, its ancestors', and — under /// [`LoadPolicy::WithDescendants`] — every descendant's (C++ /// `UsdPrim::Load`). diff --git a/crates/openusd/src/usd/stage.rs b/crates/openusd/src/usd/stage.rs index b8674510..5d3f0141 100644 --- a/crates/openusd/src/usd/stage.rs +++ b/crates/openusd/src/usd/stage.rs @@ -3213,6 +3213,31 @@ impl Stage { } else if mask.contains(PrimStatus::ABSTRACT) { status.set(PrimStatus::ABSTRACT, prim.is_abstract()?); } + Self::set_own_status(&prim, mask, &mut status)?; + Ok(status) + } + + /// [`prim_status_masked`](Self::prim_status_masked) for a prim whose + /// parent resolved active, loaded, defined and not abstract. Those bits + /// then come from the prim's own opinions, with no walk up its ancestors, + /// as C++ `Usd_PrimData` composes a prim's flags from its parent's. + fn child_status_masked(&self, prim: &sdf::Path, mask: PrimStatus) -> Result { + let (active, defined, is_abstract) = self.masked(prim, |g, c| c.local_status(g, prim))?; + let prim = super::Prim::new(self, prim.clone()); + let mut status = PrimStatus::empty(); + status.set(PrimStatus::ACTIVE, active); + if mask.contains(PrimStatus::LOADED) { + status.set(PrimStatus::LOADED, prim.is_loaded_below_loaded(active)?); + } + status.set(PrimStatus::DEFINED, defined); + status.set(PrimStatus::ABSTRACT, is_abstract); + status &= mask; + Self::set_own_status(&prim, mask, &mut status)?; + Ok(status) + } + + /// The status bits a prim's ancestors do not decide. + fn set_own_status(prim: &super::Prim, mask: PrimStatus, status: &mut PrimStatus) -> Result<()> { if mask.contains(PrimStatus::INSTANCE) { status.set(PrimStatus::INSTANCE, prim.is_instance()?); } @@ -3222,7 +3247,7 @@ impl Stage { if mask.contains(PrimStatus::IN_PROTOTYPE) { status.set(PrimStatus::IN_PROTOTYPE, prim.is_in_prototype()?); } - Ok(status) + Ok(()) } /// Borrows the stage's composition cache, first draining any pending layer @@ -3416,18 +3441,26 @@ impl Stage { /// excludes those regions. pub fn traverse(&self, predicate: PrimPredicate, mut visitor: impl FnMut(&sdf::Path)) -> Result<()> { let needed = predicate.consulted_bits(); - let mut stack = vec![sdf::Path::abs_root()]; - - while let Some(path) = stack.pop() { + let inherited = PrimPredicate::INHERITED_REQUIRED.union(PrimPredicate::INHERITED_REJECTED); + let threads = needed.contains(inherited); + // Each prim carries the population epoch under which its parent + // resolved active, loaded, defined and not abstract, if it did. While + // the population is unchanged the prim then reads only its own + // opinions instead of walking its ancestors. + let mut stack = vec![(sdf::Path::abs_root(), None)]; + + while let Some((path, parent)) = stack.pop() { + let epoch = self.population_epoch(); + let mut passed = threads.then_some(epoch); if path != sdf::Path::abs_root() { - // TODO(perf): each `prim_status_masked` call recomputes the - // inherited bits (active/loaded/defined/abstract/model) by - // walking this prim's ancestor chain to the root, and several - // predicates re-walk it for the same fields. Since traversal is - // top-down, the parent's resolved inherited status could be - // threaded down the stack so each prim only consults its own - // local opinion — turning the per-prim O(depth) walk into O(1). - let status = self.prim_status_masked(&path, needed)?; + let status = if parent == Some(epoch) { + self.child_status_masked(&path, needed)? + } else { + self.prim_status_masked(&path, needed)? + }; + let default = status.contains(PrimPredicate::INHERITED_REQUIRED) + && !status.intersects(PrimPredicate::INHERITED_REJECTED); + passed = passed.filter(|_| default && self.population_epoch() == epoch); if predicate.matches(status) { visitor(&path); } @@ -3445,7 +3478,7 @@ impl Stage { // Push in reverse so first child is visited first. for name in children.iter().rev() { if let Ok(child) = path.append_path(name.as_str()) { - stack.push(child); + stack.push((child, passed)); } } } diff --git a/crates/openusd/tests/stage.rs b/crates/openusd/tests/stage.rs index a4367657..703dc979 100644 --- a/crates/openusd/tests/stage.rs +++ b/crates/openusd/tests/stage.rs @@ -4899,6 +4899,116 @@ fn prototype_root_active() -> Result<()> { Ok(()) } +/// Traversal reads a prim's inherited status from its parent's, and visits +/// exactly the prims whose own full status matches, in the same order: over +/// inactive, `over` and `class` branches, an instance, deep nesting, and +/// payloads with and without load rules. +#[test] +fn traversal_matches_prim_status() -> Result<()> { + let dir = tempfile::tempdir()?; + fs::write( + dir.path().join("payload.usda"), + "#usda 1.0\ndef \"P\"\n{\n def \"Inside\"\n {\n def \"More\"\n {\n }\n }\n}\n", + )?; + let root = dir.path().join("root.usda"); + fs::write( + &root, + r#"#usda 1.0 +def "World" +{ + def "Active" + { + def "Leaf" + { + } + } + def "Off" ( + active = false + ) + { + def "Leaf" + { + } + } + over "Over" + { + def "Leaf" + { + } + } + class "Class" + { + def "Leaf" + { + } + } + def "Loaded" ( + payload = @./payload.usda@

+ ) + { + } + def "Unloaded" ( + payload = @./payload.usda@

+ ) + { + } + def "Inst" ( + instanceable = true + references = + ) + { + } + def "Deep" + { + over "A" + { + def "B" + { + } + } + def "C" + { + class "D" + { + def "E" + { + } + } + } + } +} +"#, + )?; + let root = root.to_str().expect("utf-8 temp path"); + for load in [InitialLoadSet::LoadAll, InitialLoadSet::LoadNone] { + let stage = Stage::builder().load(load).open(root)?; + stage.prim("/World/Loaded")?.load(LoadPolicy::WithDescendants); + let mut all = Vec::new(); + stage.traverse(PrimPredicate::ALL, |path| all.push(path.clone()))?; + for predicate in [PrimPredicate::DEFAULT, PrimPredicate::DEFAULT_PROXIES] { + let mut expected = Vec::new(); + let mut instances: Vec = Vec::new(); + for path in &all { + let below_instance = instances + .iter() + .any(|instance| path.has_prefix(instance) && path != instance); + let status = stage.prim_status(path.clone())?; + if status.contains(PrimStatus::INSTANCE) { + instances.push(path.clone()); + } + if predicate.matches(status) && (predicate == PrimPredicate::DEFAULT_PROXIES || !below_instance) { + expected.push(path.clone()); + } + } + let mut visited = Vec::new(); + stage.traverse(predicate, |path| visited.push(path.clone()))?; + assert_eq!(visited, expected, "{load:?} {predicate:?}"); + assert!(visited.len() > 5); + } + } + Ok(()) +} + /// A prim sorts its properties into attributes and relationships by their /// composed spec, and a prototype root, which reads no opinions, has neither. #[test]