Add maxcomponent and mincomponent nodes#2909
Conversation
|
Hey @jstone-lucasfilm as discussed here, this code change implements both the max and min node components. This PR's scope is only to implement the nodes and for now no use-cases have been updated. Lemme know your thoughts! Thanks |
| Output the maximum component of the incoming vector or color stream. | ||
| --> | ||
| <nodegraph name="NG_maxcomponent_color3" nodedef="ND_maxcomponent_color3"> | ||
| <extract name="N_extract_0" type="float"> |
There was a problem hiding this comment.
You should be able to use separate nodes instead of multiple extracts. This also avoids storing and testing intermediate values in case they become precision sensitive.
There was a problem hiding this comment.
@chinmaychahar I think @kwokcb's suggestion above is a good one, and using separateN nodes would be better aligned with existing graph definitions than extract. Does this seem like a reasonable improvement to you as well?
There was a problem hiding this comment.
Yes, let me fix this
| </transpose> | ||
| <output name="out" type="vector4" nodename="transformvector1" /> | ||
| </nodegraph> | ||
| <nodegraph name="maxcomponent_color3"> |
There was a problem hiding this comment.
For visual testing, I'd suggest to make the max value be the same for all tests. You might also want to add a test where 2 or more components differ only by a very small delta to test precision.
|
This looks like a great start, @chinmaychahar, and my one addition to @kwokcb's suggestions above would be to move your specification definition of these nodes from the Proposals document to the Standard Nodes document. This will allow us to merge the specification and data library changes as a single PR, keeping them in sync. |
|
Sure, lemme take a look and address the comments today |
chinmaychahar
force-pushed
the
feature/maxcomponent-mincomponent-implementation
branch
from
7315c55 to
70e5820
Compare
Signed-off-by: chinmaychahar <chinmay.cc.06@gmail.com>
Signed-off-by: chinmaychahar <chinmay.cc.06@gmail.com>
Signed-off-by: chinmaychahar <chinmay.cc.06@gmail.com>
chinmaychahar
force-pushed
the
feature/maxcomponent-mincomponent-implementation
branch
from
66ea19e to
42f09ad
Compare
Signed-off-by: Jonathan Stone <jstone@lucasfilm.com>
Signed-off-by: Jonathan Stone <jstone@lucasfilm.com>
Signed-off-by: Jonathan Stone <jstone@lucasfilm.com>
jstone-lucasfilm
left a comment
jstone-lucasfilm
left a comment
There was a problem hiding this comment.
This looks great to me, thanks @chinmaychahar!
jstone-lucasfilm
changed the title
maxcomponent and mincomponent nodes
jstone-lucasfilm
merged commit 9b2eb05
into
AcademySoftwareFoundation:main
3 participants
Summary
Implement
maxcomponentandmincomponentnodes using graph definitions, as proposed in #2802.The specification proposal was merged in #2876
Description
This PR adds node definitions, nodegraph implementations, and test suite entries for
maxcomponentandmincomponent. These nodes compute the maximum and minimum component of vectorN and colorN values, outputting a float.