Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
177 changes: 174 additions & 3 deletions src/usda/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -599,7 +599,7 @@ impl<'a> Parser<'a> {
if matches!(suffix, Some(Token::TimeSamples)) {
push_unique(suffixed_properties, name);
self.ensure_pun('=')?;
let samples = self.parse_time_samples()?;
let samples = self.parse_time_samples(type_info)?;
let path = current_path.append_property(name)?;

let spec = data
Expand Down Expand Up @@ -1182,7 +1182,23 @@ impl<'a> Parser<'a> {
}

/// Parse a time sample map: `{ time : value, time : value, ... }`.
fn parse_time_samples(&mut self) -> Result<sdf::TimeSampleMap> {
///
/// Per-time values are dispatched two ways:
///
/// - When the property's declared type expects a tuple
/// (`vector3f`, `quatf`, `matrix4d`, …) AND the next token
/// actually opens a tuple / array-of-tuples (`(` or `[`),
/// route through [`parse_value`] so the value lands in the
/// matching `Value::Vec3f` / `QuatfVec` / `Matrix4d` variant.
/// This is what makes Pixar's `HumanFemale.walk.usd` and
/// every other UsdSkel rig parse.
///
/// - Otherwise fall through to [`parse_property_metadata_value`]
/// so malformed-but-historically-accepted samples still load
/// — the spec corpus's `attributes.usda` deliberately authors
/// bare scalars (`5.67`, `-7`) and `None` against typed
/// `vector3f` properties to verify the parser's tolerance.
fn parse_time_samples(&mut self, info: TypeInfo<'_>) -> Result<sdf::TimeSampleMap> {
let mut samples = Vec::new();
self.parse_block('{', '}', |this| {
let time_str = this.fetch_next()?;
Expand All @@ -1191,13 +1207,53 @@ impl<'a> Parser<'a> {
other => bail!("Expected time value, got {other:?}"),
};
this.ensure_pun(':')?;
let value = this.parse_property_metadata_value()?;
let value = if this.next_is_typed_tuple_value(info) {
this.parse_value(info)?
} else {
this.parse_property_metadata_value()?
};
samples.push((time, value));
Ok(())
})?;
Ok(samples)
}

/// Heuristic: should the next token be parsed under [`parse_value`]
/// for `info`, or is the type-blind metadata-value path safer?
///
/// Returns `true` when the property's type expects a tuple
/// (vector / quat / matrix) AND the next token opens one
/// (`(`) or opens an array of them (`[`). Anything else
/// (scalar literal, `None`, identifier) flows through the
/// type-blind path.
fn next_is_typed_tuple_value(&mut self, info: TypeInfo<'_>) -> bool {
let wants_tuple = matches!(
info.ty,
Type::Int2
| Type::Int3
| Type::Int4
| Type::Half2
| Type::Half3
| Type::Half4
| Type::Float2
| Type::Float3
| Type::Float4
| Type::Double2
| Type::Double3
| Type::Double4
| Type::Quath
| Type::Quatf
| Type::Quatd
| Type::Matrix2d
| Type::Matrix3d
| Type::Matrix4d
);
if !wants_tuple {
return false;
}
matches!(self.peek_next(), Some(Ok(Token::Punctuation('(' | '['))))
}

/// Parse one reference entry, including optional target prim path and layer offset.
fn parse_reference(&mut self) -> Result<sdf::Reference> {
let mut reference = sdf::Reference::default();
Expand Down Expand Up @@ -2733,6 +2789,121 @@ def Xform "root" {
);
}

/// Regression: per-time values inside `.timeSamples = { ... }` must be
/// parsed under the property's declared type so typed-tuple forms
/// (`(w, x, y, z)` for `quatf[]`, `(r, g, b)` for `float3[]`,
/// matrix rows for `matrix4d`) round-trip into the matching
/// `Value::QuatfVec` / `Vec3fVec` / `Matrix4d` variants. Pixar's
/// `UsdSkelExamples/HumanFemale.walk.usd` is the canonical example
/// — its rotation samples are arrays of quaternion tuples and
/// failed with `Unsupported property metadata value token: Punctuation('(')`
/// before the type-aware dispatch landed.
#[test]
fn parse_typed_tuple_time_samples() {
let mut parser = Parser::new(
r#"#usda 1.0
def Xform "Anim"
{
quatf[] rotations.timeSamples = {
0: [(1, 0, 0, 0), (0.7071, 0, 0.7071, 0)],
1: [(0.7071, 0, 0.7071, 0), (0, 0, 1, 0)],
}
float3[] translations.timeSamples = {
0: [(0, 0, 0), (1, 2, 3)],
}
matrix4d xformOp:transform.timeSamples = {
0: ((1, 0, 0, 0), (0, 1, 0, 0), (0, 0, 1, 0), (0, 0, 0, 1)),
}
}
"#,
);
let specs = parser.parse().expect("typed timeSamples parsed");

let rotations = specs
.get(&sdf::Path::new("/Anim.rotations").unwrap())
.and_then(|s| s.get(FieldKey::TimeSamples.as_str()))
.expect("rotations.timeSamples present");
let samples = match rotations {
sdf::Value::TimeSamples(s) => s,
other => panic!("expected TimeSamples, got {other:?}"),
};
assert_eq!(samples.len(), 2);
match &samples[0].1 {
sdf::Value::QuatfVec(v) => {
assert_eq!(v.len(), 2);
assert_eq!(v[0], [1.0, 0.0, 0.0, 0.0]);
}
other => panic!("expected QuatfVec for quatf[] sample, got {other:?}"),
}

let translations = specs
.get(&sdf::Path::new("/Anim.translations").unwrap())
.and_then(|s| s.get(FieldKey::TimeSamples.as_str()))
.expect("translations.timeSamples present");
let samples = match translations {
sdf::Value::TimeSamples(s) => s,
other => panic!("expected TimeSamples, got {other:?}"),
};
match &samples[0].1 {
sdf::Value::Vec3fVec(v) => {
assert_eq!(v.len(), 2);
assert_eq!(v[1], [1.0, 2.0, 3.0]);
}
other => panic!("expected Vec3fVec for float3[] sample, got {other:?}"),
}

let xform = specs
.get(&sdf::Path::new("/Anim.xformOp:transform").unwrap())
.and_then(|s| s.get(FieldKey::TimeSamples.as_str()))
.expect("xformOp:transform.timeSamples present");
let samples = match xform {
sdf::Value::TimeSamples(s) => s,
other => panic!("expected TimeSamples, got {other:?}"),
};
match &samples[0].1 {
sdf::Value::Matrix4d(m) => {
assert_eq!(m[0], 1.0);
assert_eq!(m[5], 1.0);
assert_eq!(m[10], 1.0);
assert_eq!(m[15], 1.0);
}
other => panic!("expected Matrix4d for matrix4d sample, got {other:?}"),
}
}

/// Regression: bare scalars and `None` authored against a typed
/// vector property's `.timeSamples` must still parse — the spec
/// corpus's `attributes.usda` tests parser tolerance with
/// `vector3f my:attribute.timeSamples = { 3 : 5.67, 6.78 : None, ... }`,
/// and we don't want the type-aware tuple dispatch to regress
/// that.
#[test]
fn parse_lenient_time_samples_keep_scalar_and_none() {
let mut parser = Parser::new(
r#"#usda 1.0
def Xform "X"
{
custom uniform vector3f my:attribute.timeSamples = {
3 : 5.67,
6.78 : None,
3567.234: -7,
}
}
"#,
);
let specs = parser.parse().expect("lenient timeSamples parsed");
let value = specs
.get(&sdf::Path::new("/X.my:attribute").unwrap())
.and_then(|s| s.get(FieldKey::TimeSamples.as_str()))
.expect("timeSamples present");
let samples = match value {
sdf::Value::TimeSamples(s) => s,
other => panic!("expected TimeSamples, got {other:?}"),
};
assert_eq!(samples.len(), 3);
assert!(matches!(samples[1].1, sdf::Value::ValueBlock));
}

#[test]
fn parse_reference_asset_only() {
let mut parser = Parser::new("@./model.usda@");
Expand Down