Skip to content

Add type alias support to the reverse bindings (#USDU-3 part 2) #260

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Aug 31, 2021

Conversation

judubu
Copy link
Contributor

@judubu judubu commented Jul 31, 2021

Purpose of this PR

Ticket/Jira #:

USD allows certain attribute type to be given roles (aliases) to help unambiguously determine how data should be interpreted.
Ex:

  • float2 can be given the texcoord2 when used for a uv primvar
  • float3 can be given the point/normal/vector role ...

This is a problem when using our reverse bindings which let the USD type drive the C# type the primvar data should deserialized to. The reverse bindings are useful in the case a given attribute can be defined as multiple types. Typically UVs primvars can be defined as float2/float3/half2/half3/double2/double3 and as such can be also given the role of texcoord2f/texcoord3f/texcoord2h/texcoord3h/texcoord2d/texcoord3d .... As we already have a 1-to-1 mapping to Unity type for the standard types (float2, float3, ...) we can't augment the mapping to support roles.

This PR adds a way to define aliases for a given type and allow the reverse binding to convert roles to their canonic type before doing the type lookup.

Testing

Functional Testing status:

Performance Testing status:

Overall Product Risks

Complexity:

Halo Effect:

Additional information

Note to reviewers:

Reminder:

  • Add entry in CHANGELOG.md (if applicable)

Copy link
Contributor

@jcowles jcowles left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is awesome!

I do think we should keep the data private (typeAliases) and wrap it with a public API, but other than that, it looks great.

Copy link
Contributor

@clusty clusty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aggree with @jcowles 's comments

@clusty clusty removed the request for review from cguer August 19, 2021 14:19
@judubu judubu requested a review from jcowles August 24, 2021 19:18
Copy link
Contributor

@jcowles jcowles left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ship it!

Base automatically changed from USDU-3/01_ignore_abstract_prims to dev August 30, 2021 19:22
@judubu judubu merged commit 22689c8 into dev Aug 31, 2021
@judubu judubu deleted the USDU-3/02_add_type_alias_support branch August 31, 2021 12:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants