Skip to content
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

added schema tests and foundation tests #10

Merged
merged 2 commits into from
Aug 31, 2022

Conversation

AlexSchwank
Copy link
Contributor

No description provided.

@AlexSchwank AlexSchwank requested a review from meshula August 19, 2022 04:46

[metersPerUnit_mix.usda](./metersPerUnit_mixed.usda)

This stage sets `metersPerUnit = 100` and also sublayers `metersPerUnit_1.usda` and `metersPerUnit_10.usda` with different values.
Copy link

@asluk asluk Aug 19, 2022

Choose a reason for hiding this comment

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

Is this intended to demonstrate a typical usage of mixed metersPerUnit (MPU), and/or to re-emphasize that USD itself won't handle any auto-scaling to reconcile divergent MPU across layer stacks? Would be great to call that out in the documentation; thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll extend the documentation. The file is mainly aimed at showing that auto-scaling doesn't happen in the current implementation.

Copy link
Contributor

@meshula meshula left a comment

Choose a reason for hiding this comment

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

Per discussion in the wg meeting, we could add some documentation, or land it and add documentation subsequently. @AlexSchwank do you have an idea on how you'd like to proceed?

@AlexSchwank
Copy link
Contributor Author

Per discussion in the wg meeting, we could add some documentation, or land it and add documentation subsequently. @AlexSchwank do you have an idea on how you'd like to proceed?

I suggest we leave the PR open for another week and then merge it. This gives folks some time to point out things (like the MPU comment above) where more documentation would be good and I'll add it. Afterwards people can either create their own PRs or open issues to report missing/insufficient/out-of-date documentation.

@hybridherbst
Copy link
Contributor

hybridherbst commented Aug 27, 2022

Some more questions of the more practical nature (for people trying to build USD files for specific cases based on these samples here):

  • are you planning to add tests for animations?
  • are you planning (which would be great) to document somewhere which of these tests are working on which of the QuickLook implementations? (are all of them?)

@meshula
Copy link
Contributor

meshula commented Aug 31, 2022

The week has elapsed, Alex has updated with initial updates from comments. I think we are in a good place to start iterating, so I'm merging. @hybridherbst perhaps open an issue on animation tests, and another on documenting with tests are working in QuickLook?

@meshula meshula merged commit 10cd18e into usd-wg:main Aug 31, 2022
@hybridherbst
Copy link
Contributor

Created issues #16 and #17 - thanks!

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.

4 participants