search

dbsmythe / Mtlx_Markdown_Test

Apache License 2.0
0 stars 2 forks source link

Establish consistent order for noise node inputs #1

Open jstone-lucasfilm opened 1 year ago

jstone-lucasfilm commented 1 year ago

https://github.com/dbsmythe/Mtlx_Markdown_Test/blob/3236fdf39558e682cc58317d7e32c1a3598313d2/mtlx1.39_spec.md?plain=1#L765

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?

dbsmythe commented 1 year ago

Agreed that we should be consistent in the ordering of inputs, but I would suggest an ordering of:

  1. Parameters specific to the evaluation of the node function, with (in this case) "period" being last, as it's somewhat related to...
  2. Texture/position reference coordinates. The "in" input of cellnoise1d is a bit of an oddball case: normally an "in" input should go first, but in this case, it's actually a 1d reference coordinate, more like "position". I was never super happy with using the name "in" for this but it was better than "t", and in many ways it really is "the input" to what the node evaluates. I will attempt to create a pull request with changes to reflect the above ordering rules.
dbsmythe commented 1 year ago

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.

dbsmythe commented 1 year ago

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.