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

Adding explicit default units #1400

Merged
merged 1 commit into from
Jun 14, 2024
Merged

Conversation

zicher3d
Copy link
Collaborator

@zicher3d zicher3d commented Jun 7, 2024

Seemed like units defaulted to meters also for angle units. Adding explicit defaults so that conversion does not try to convert meters to degrees.

Seemed like units defaulted to meters also for angle units.
Adding explicit defaults so that conversion does not  try to convert meters to degrees.
@zicher3d zicher3d requested a review from ashwinbhat June 7, 2024 18:35
@zicher3d zicher3d self-assigned this Jun 7, 2024
Copy link
Collaborator

@ashwinbhat ashwinbhat left a comment

Choose a reason for hiding this comment

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

Don't we need to define them in the example mtlx for legacy_checker?

@zicher3d
Copy link
Collaborator Author

zicher3d commented Jun 8, 2024

There is no specific test for legacy_checker (or example) and the only procedural used in the graphs is legacy_noise.
(Checker is there because a customer could use checker in a custom material. It's just one of the many protein procedurals, and I made it mainly to learn and test the process.)

Legacy_noise does have inputs for distance and angle values, and they have been adjusted in this commit. But the sample file testing all the protein classes do not provide values and relies on defaults, mainly because I cannot specify a unittype and unit in the graph editor for legacy_noise nodes that exist within a graph (no connections to nodedef inputs).

We might have to do some ad-hoc tests to test these, keeping in mind that most of the legacy_noise nodes are used for bump maps and so their effect is not visible as procedurals do not work as bump.
When those work, we can then adjust the nodegraphs manually, remembering to check that the units attributes are maintained if we edit the graphs again in the graph_editor.

@ashwinbhat ashwinbhat self-requested a review June 14, 2024 16:20
@zicher3d zicher3d merged commit 6099659 into adsk_contrib/dev Jun 14, 2024
26 of 32 checks passed
@zicher3d zicher3d deleted the zicher/legacy_defs_units branch June 14, 2024 17:36
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.

2 participants