diff --git a/src/usda/parser.rs b/src/usda/parser.rs index 56aabce..3c8a5dc 100644 --- a/src/usda/parser.rs +++ b/src/usda/parser.rs @@ -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 @@ -1182,7 +1182,23 @@ impl<'a> Parser<'a> { } /// Parse a time sample map: `{ time : value, time : value, ... }`. - fn parse_time_samples(&mut self) -> Result { + /// + /// 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 { let mut samples = Vec::new(); self.parse_block('{', '}', |this| { let time_str = this.fetch_next()?; @@ -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 { let mut reference = sdf::Reference::default(); @@ -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@");