-
Notifications
You must be signed in to change notification settings - Fork 1
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
Establish consistent order for noise node inputs #1
Comments
Agreed that we should be consistent in the ordering of inputs, but I would suggest an ordering of:
|
Actually, not sure I can. If I understand things, before I can do a pull request, I have to first make a fork of the project and make the changes in the fork. But I can't make a fork of Mtlx_Markdown_Test into my own account/organization, because that's where the original already is. I would have to create the fork in the ASWF/MaterialX area, the only area GitHub says I have permissions to create a new fork into, which doesn't sounds like something that should be done. At any rate, filing an Issue to discuss specific passages in the Spec seems like an okay workflow, though it looks like one can only comment on entire lines, not specific highlighted phrases within a line (which in this document, is basically an entire paragraph). Not as good as Google Docs, but workable. |
Slight update to my suggested ordering rules above: for the worleynoise nodes, "jitter" is a parameter that affects the sampling "period", so as a modifier I think it should go right after period. This would make the ordering "metric", "period", "jitter", "texcoord/position". And the worleynoises are the only two nodes with a change in ordering to reflect the new rules. |
Mtlx_Markdown_Test/mtlx1.39_spec.md
Line 765 in 3236fdf
We should establish a consistent order for common inputs to the noise nodes, making the parallels between them easier to understand. Perhaps "period" should come last in the list and the spatial input (e.g. "position", "texcoord", "in") should come first?
The text was updated successfully, but these errors were encountered: