OpenUSD scene: replace object in the USD value model with a UsdValue union - #4160
Open
marcschier wants to merge 5 commits into
Open
OpenUSD scene: replace object in the USD value model with a UsdValue union#4160marcschier wants to merge 5 commits into
marcschier wants to merge 5 commits into
Conversation
A readonly struct scoping an authored USD value to its possible shapes, so the scene document model no longer needs object. Implements INullable, exposes TryGet accessors only and uses ArrayOf<UsdValue> for the recursive tuple, array and matrix cases. Not yet consumed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8cbb8cd0-f0cb-4ab0-bea2-6202fbf69485
UsdAttribute.Value, UsdAttribute.TimeSamples, UsdPrim.Metadata and UsdTimeSample.Value now carry UsdValue instead of object, and the reader, writer, coercion and signature code speak it end to end. UsdValueCoercion.Decoerce takes the Variant directly and reads it through its typed accessors, which removes the Variant.AsBoxedObject(BoxingBehavior.Legacy) call in UsdSceneExporter and keeps ArrayOf<T>/MatrixOf<T> shapes intact. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8cbb8cd0-f0cb-4ab0-bea2-6202fbf69485
Adds UsdValue unit tests and moves the existing suites onto the new type without changing any expectation, so the fidelity and round trip tests still prove the conversion is lossless. They caught three real defects: - the georeference dual authoring path still read values through an object typed TryToDouble, which no longer saw CLR primitives; - a fixed size math scalar such as double3 was emitted as [1.0, 2.0, 3.0] because the writer chose brackets from the value kind. The TypeName decides the shape, so UsdVal now renders a composite as a parenthesised tuple unless the type is an array type; - UsdValue stored its components as ArrayOf<UsdValue>, making the struct layout recursive. The .NET Framework type loader cannot resolve that and threw TypeLoadException for every use on net48. The payload is now a plain array, which is a reference type and breaks the cycle, while the public accessors still hand out ArrayOf<UsdValue>. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8cbb8cd0-f0cb-4ab0-bea2-6202fbf69485
Contributor
There was a problem hiding this comment.
Pull request overview
This PR replaces object/object? in the OpenUSD scene document value model with a UsdValue discriminated-union-style readonly struct, and updates the reader/writer, coercion layer, signature computation, materializer/exporter, and tests to use the new typed representation end-to-end (including removing the OpenUSD-side usage of Variant boxing accessors).
Changes:
- Introduces
UsdValue/UsdValueKindand migratesUsdAttribute.Value,UsdAttribute.TimeSamples, andUsdPrim.MetadatatoUsdValue. - Refactors
.usdaparsing/writing, materialization/export, signature computation, and conversion/coercion to consume/produceUsdValueand to readVariantvia typed accessors (no legacy boxing). - Updates/extends test suites and package documentation to exercise the new value model (including nested composites and metadata).
Reviewed changes
Copilot reviewed 43 out of 43 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Opc.Ua.OpenUsd.Tests/VariantBranchTests.cs | Updates variant-branch construction to use UsdValue instead of raw scalars. |
| tests/Opc.Ua.OpenUsd.Tests/VariantBranchConversionTests.cs | Replaces direct equality against primitives with UsdValue-aware assertions. |
| tests/Opc.Ua.OpenUsd.Tests/UsdValueTests.cs | Adds unit coverage for UsdValue kinds, accessors, composites, and equality/hash basics. |
| tests/Opc.Ua.OpenUsd.Tests/UsdTimeSampleTests.cs | Migrates time-sample parsing expectations to UsdValue and adds helper-based assertions. |
| tests/Opc.Ua.OpenUsd.Tests/UsdTimeSampleEqualityTests.cs | Updates UsdTimeSample equality tests to use UsdValue. |
| tests/Opc.Ua.OpenUsd.Tests/UsdTestHelpers.cs | Adds helper constructors/assertions for UsdValue (tuples/arrays/dicts and typed asserts). |
| tests/Opc.Ua.OpenUsd.Tests/UsdSceneSignatureTests.cs | Updates signature tests for UsdValue-typed attribute values and metadata. |
| tests/Opc.Ua.OpenUsd.Tests/UsdSceneExporterFallbackTests.cs | Updates exporter fallback tests to author UsdValue metadata/values. |
| tests/Opc.Ua.OpenUsd.Tests/UsdSceneDiscoveryTests.cs | Updates authored attribute values to UsdValue. |
| tests/Opc.Ua.OpenUsd.Tests/UsdaWriterInjectionTests.cs | Updates writer injection test to author string values via UsdValue. |
| tests/Opc.Ua.OpenUsd.Tests/UsdaValueParsingTests.cs | Migrates parsing tests from object type tests/casts to UsdValue accessors. |
| tests/Opc.Ua.OpenUsd.Tests/UsdaReaderPlantTests.cs | Updates plant corpus assertions to read values through UsdValue accessors. |
| tests/Opc.Ua.OpenUsd.Tests/UsdaReaderCellTests.cs | Updates cell corpus assertions to read values through UsdValue accessors. |
| tests/Opc.Ua.OpenUsd.Tests/TimeSampleMaterializationTests.cs | Updates historization/materialization tests to use UsdValue for defaults and samples. |
| tests/Opc.Ua.OpenUsd.Tests/TargetNodeIdAuthoringTests.cs | Updates authored stage fixtures to use UsdValue. |
| tests/Opc.Ua.OpenUsd.Tests/SceneQuery.cs | Normalizes attributes by setting UsdValue.Null instead of null. |
| tests/Opc.Ua.OpenUsd.Tests/SceneFidelityRoundTripTests.cs | Updates fidelity assertions to use UsdValue comparisons/accessors. |
| tests/Opc.Ua.OpenUsd.Tests/RobotAssetContractTests.cs | Adjusts asset contract verification to flatten/inspect UsdValue composites. |
| tests/Opc.Ua.OpenUsd.Tests/PrimMetadataMaterializationTests.cs | Migrates prim metadata authoring/round-trip checks to UsdValue and updated type expectations. |
| tests/Opc.Ua.OpenUsd.Tests/OptionalMemberMaterializationTests.cs | Updates authored attribute values to UsdValue. |
| tests/Opc.Ua.OpenUsd.Tests/MetadataCoercionTests.cs | Refactors metadata coercion tests to supply UsdValue inputs and updated scalar/sequence cases. |
| tests/Opc.Ua.OpenUsd.Tests/MaterializerTests.cs | Updates live-stage fixture values to UsdValue. |
| tests/Opc.Ua.OpenUsd.Tests/MaterializerFallbackTests.cs | Updates fallback cases to use UsdValue composites/helpers. |
| tests/Opc.Ua.OpenUsd.Tests/GeoreferenceTypedPrimTests.cs | Updates georeference fixtures to use UsdValue for numeric authoring. |
| tests/Opc.Ua.OpenUsd.Tests/GeoreferenceTests.cs | Updates georeference fixtures to use UsdValue for numeric authoring. |
| tests/Opc.Ua.OpenUsd.Tests/GeoreferenceAnchorCoercionTests.cs | Updates anchor coercion inputs to use UsdValue and adjusts helper signature accordingly. |
| tests/Opc.Ua.OpenUsd.Tests/ConversionFixTests.cs | Updates conversion/coercion tests to use UsdValue shapes (tuple/array nesting) instead of object containers. |
| tests/Opc.Ua.OpenUsd.Tests/ConversionEmitPathTests.cs | Updates end-to-end emit path tests to use Variant -> UsdValue decoercion and UsdValue composites. |
| tests/Opc.Ua.OpenUsd.Tests/ConversionAsymmetryTests.cs | Migrates asymmetry tests to UsdValue parsing/accessors for arrays/asset paths/targets. |
| tests/Opc.Ua.OpenUsd.Tests/ConnectionFidelityTests.cs | Updates authored attribute values to UsdValue while preserving connection-order assertions. |
| tests/Opc.Ua.OpenUsd.Tests/AddressSpaceRoundTripTests.cs | Updates address-space round-trip fixtures to use UsdValue composites. |
| src/Opc.Ua.OpenUsdScene/Scene/UsdValueKind.cs | Adds the UsdValueKind enum describing authored USD value shapes. |
| src/Opc.Ua.OpenUsdScene/Scene/UsdValue.cs | Introduces the UsdValue readonly struct union and typed accessors for scalars/composites/dictionaries. |
| src/Opc.Ua.OpenUsdScene/Scene/UsdPrim.cs | Replaces prim metadata dictionary value type with UsdValue. |
| src/Opc.Ua.OpenUsdScene/Scene/UsdAttribute.cs | Replaces attribute value/time-sample types with UsdValue. |
| src/Opc.Ua.OpenUsdScene/NugetREADME.md | Documents the new UsdValue-based scene value model and usage patterns. |
| src/Opc.Ua.OpenUsdScene/Conversion/UsdValueCoercion.cs | Refactors coercion/decoercion to operate on UsdValue and typed Variant accessors (no boxing). |
| src/Opc.Ua.OpenUsdScene/Conversion/UsdSceneSignature.cs | Updates signature normalization to operate on UsdValue and include dictionary normalization. |
| src/Opc.Ua.OpenUsdScene/Conversion/UsdaWriter.cs | Migrates writer rendering paths to UsdValue for attributes, time samples, and metadata emission. |
| src/Opc.Ua.OpenUsdScene/Conversion/UsdaReader.cs | Migrates reader parsing and metadata parsing to produce UsdValue (including composite/dictionary forms). |
| src/Opc.Ua.OpenUsdScene.Server/UsdSceneMaterializer.Properties.cs | Updates metadata materialization/coercion to consume UsdValue and preserve typed variants. |
| src/Opc.Ua.OpenUsdScene.Server/UsdSceneMaterializer.cs | Updates UsdTimeSample to carry UsdValue instead of object?. |
| src/Opc.Ua.OpenUsdScene.Server/UsdSceneExporter.cs | Removes legacy boxing from export by passing Variant directly into UsdValueCoercion.Decoerce, and updates metadata reconstruction to UsdValue. |
Suppressed comments (2)
src/Opc.Ua.OpenUsdScene/Conversion/UsdValueCoercion.cs:238
- DecoerceArray/Wrap for UInt64 also casts each element to long unconditionally, which will wrap values > long.MaxValue. This can corrupt large unsigned values when exporting arrays back into the scene model.
case BuiltInType.UInt64:
return value.TryGetValue(out ArrayOf<ulong> ul)
? Wrap(ul, static x => UsdValue.From((long)x))
: UsdValue.Null;
src/Opc.Ua.OpenUsdScene/Conversion/UsdValueCoercion.cs:283
- DecoerceMatrix/Regroup for UInt64 casts each element to long unconditionally, which wraps values > long.MaxValue and can corrupt large unsigned matrix elements during export.
case BuiltInType.UInt64:
return value.TryGetValue(out MatrixOf<ulong> ul)
? Regroup(ul, static x => UsdValue.From((long)x))
: UsdValue.Null;
- UsdValue.GetHashCode now folds the dictionary entries (an order independent sum of key/value hashes) instead of only the entry count, so dictionaries of the same size no longer all collide in hash based collections. - UsdValue.ToString renders a Dictionary as its entries ordered by key instead of falling through to the null m_text default, so a caller that falls back to the textual form (metadata materialization) no longer silently drops data. - UsdValueCoercion decoerces a UInt64 above long.MaxValue to its invariant decimal text instead of casting to long, which silently wrapped it into a negative integer, for scalars, arrays and matrices alike. TryAsUInt64 reads that form back, and UsdaReader parses an integral literal that overflows a signed 64 bit integer as a token holding its exact digits (it previously threw OverflowException), so the value survives the full export/import round trip. - Fixed CA1307/CA2249 on the string quote probe added by this PR. Adds unit tests for the dictionary hash/rendering and the uint64 round trip. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d089bd1c-b795-4f2b-a872-eb3090c11536
Codecov reported 74.47% patch coverage; the gap was almost entirely the conversion layer's per-element-type switches, which had no per-type test. - Adds UsdValueCoercionTests: every element type the 6.2 bindings can name, in both directions (TryCoerce and Decoerce) and at all three ranks, plus the fail-closed paths (out-of-range integers, a structured value bound to a scalar, an unsupported element type or rank, a float that overflows, text that is not a number or a boolean) and the widening paths (bool as a number, integer authored as a double, scalar as a one element sequence). - UsdValueTests: rendering of every scalar kind and of composites, an empty dictionary, matrix accessor, non numeric TryGetNumber, and the equality and hash edges (different lengths, differing dictionary keys or sizes, boxed Equals). - UsdSceneSignatureTests: normalization of a boolean and of a nested dictionary value, including that entry order does not change the signature. - ConversionEmitPathTests / ConversionFixTests: color3f[] regrouping from a flat component run and from grouped rows, composite prim metadata, a bool attribute, and an opaque boolean. - ConversionAsymmetryTests: relationship and connection targets authored as quoted strings. Local patch coverage over the changed source lines is now 99.0% (was 76.3%); the residual lines are defensive branches with no reachable input. Test only change - no production code touched. 772 tests pass on net10.0 and net48. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d089bd1c-b795-4f2b-a872-eb3090c11536
cristipogacean
approved these changes
Aug 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #4151.
The problem
objectin public API, and every consumer forced into type tests and casts. It was also whyUsdSceneExporterhad to callVariant.AsBoxedObject(Variant.BoxingBehavior.Legacy), itself against guidance.Variantcannot stand in for it. The USD value model is recursive and ragged: tuples (float3), arrays, arrays of tuples (color3f[]), matrices authored as a tuple of row tuples, plus asset paths and prim path references that must round-trip as their own syntax.Variant/MatrixOf<T>cover rectangular scalar arrays but not the nested "array of tuple" shape.What this does
Introduces
UsdValue, a readonly struct scoping a value to the shapes a.usdadocument can express, and converts the model to it end to end - reader, writer, coercion and signature all speakUsdValue, so there is one representation rather than conversions at the edges.INullable, so absence isUsdValue.Null/.IsNull- neverUsdValue?.TryGet*accessors only. No boxing accessor, nothing returningobject.ArrayOf<UsdValue>for the recursive tuple/array/matrix cases.TupleandArraystay distinct kinds because USD prints(1, 2, 3)and[1, 2, 3]differently, as doTokenandString.Decoercenow takes theVariantdirectly and reads it through its typed accessors, which is what removes theAsBoxedObjectcall.The attribute's
TypeNamestays authoritative for rendering, so the kinds add type safety without changing a single byte of emitted.usda. That is what makes this provably lossless: the existing fidelity and round-trip suites pass with their expectations untouched.Three defects the tests caught
Worth calling out, because each would have shipped silently:
object-typedTryToDouble, which stopped seeing CLR primitives once values becameUsdValue.double3scalar was emitted as[1.0, 2.0, 3.0]instead of(1.0, 2.0, 3.0), because the writer picked brackets from the value's kind. The shape is decided by the TypeName, not the container - soUsdValnow renders a composite as a parenthesised tuple unless the type is an array type. This is the same contract the H-1 regression guard documents.UsdValuedid not load at all on .NET Framework. Storing components asArrayOf<UsdValue>made the struct layout recursive (UsdValue->ArrayOf<UsdValue>->ReadOnlyMemory<UsdValue>); CoreCLR resolves that, the net48 type loader throwsTypeLoadExceptionon every use. Only the net48 run surfaced it - net10.0 was entirely green. The payload is now a plain array, a reference type, which breaks the cycle while the public accessors still hand outArrayOf<UsdValue>.Review feedback addressed
UsdValue.GetHashCodefolds the dictionary entries (an order-independent sum of key/value hashes) instead of only the entry count, so same-size dictionaries no longer all collide.UsdValue.ToStringrenders aDictionaryas its entries ordered by key instead of falling through to the empty-string default, which the metadata fallback paths rely on.uint64abovelong.MaxValueis no longer cast tolong(which wrappedulong.MaxValueinto-1) - it is preserved as its invariant decimal text for scalars, arrays and matrices alike.TryAsUInt64reads that form back, andUsdaReadernow parses an over-long integral literal as a token holding its exact digits instead of throwingOverflowException, so the value survives the whole round trip.Validation
dotnet build UA.slnx -m:1 /p:UseSharedCompilation=false: 0 errors, 0 warnings.Opc.Ua.OpenUsd.Tests: 772 passing on net10.0 and on net48 (632 existing + 140 new tests), with 99.0% patch coverage over the changed source lines.Opc.Ua.Di.Tests364,Opc.Ua.Robotics.Tests103 - unchanged.object/object?left in theOpc.Ua.OpenUsdScenepublic API except theEquals(object?)override; noAsBoxedObject/AsBoxedValueanywhere in the OpenUSD assemblies.