-
Notifications
You must be signed in to change notification settings - Fork 23
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
Replacement of Checker procedural #1414
Conversation
The previous Checker was done in Graph Editor. This version imported Checker into LookdevX and re-authored it similarly to the other more recent procedurals. The shader uses the stdlib Checker, which does not have a Rotation option. The result was that Rotation was processed before Translation, which is the opposite of OGS. To address this, the stdlib Checker offset is not used any more, and the UV are translated and rotated before being sent to Checker. Color outside of the pattern when not fully tiled is now black to match other procedurals. Note that there are small inconsistencies in inputs and formats that will be addressed in bulk affecting multiple nodes at a later stage. Jiras already exist for that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, few questions about some missing uiname etc. Not blocking
<input name="rotate" type="float" value="0" xpos="1.978261" ypos="4.974138" uivisible="true" uiname="Rotate" unittype="angle" unit="degree" uisoftmin="0" uisoftmax="360" /> | ||
<input name="tile_X" type="boolean" value="true" xpos="1.934783" ypos="6.629310" uivisible="true" uiname="Tile X" /> | ||
<input name="tile_Y" type="boolean" value="true" xpos="1.934783" ypos="8.000000" uivisible="true" uiname="Tile Y" /> | ||
<nodedef name="ND_legacy_checker" node="legacy_checker"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't this need: nodegroup="adsk_legacy" >
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, part of the bulk changes I need to do. (OGSMOD-6688, 6621, and the long list in 4070)
<input name="tile_x" type="boolean" value="true" xpos="1.934783" ypos="6.629310" uivisible="true" uiname="" uifolder="" /> | ||
<input name="tile_y" type="boolean" value="true" xpos="1.934783" ypos="8.000000" uivisible="true" uiname="" uifolder="" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
uiname is missing for tiles? Is that expected?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bulk change to do later on. Uiname is missing in many places.
</subtract> | ||
<backdrop name="Tile_and_Alpha" xpos="10.0806" ypos="1.61987" width="8.73289" height="4.15297" /> | ||
<backdrop name="Offset_and_Rotation" xpos="2.5239" ypos="2.02557" width="5.26447" height="3.43858" /> | ||
<backdrop name="Awaiting_fix_in_MaterialX" xpos="10.9859" ypos="-0.350134" width="1.36111" height="1.73333" uicolor="#C85252" Autodesk-ldx_alpha="0.156863" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Awaiting_fix_in_MaterialX" - what's this about? Is there a git issue
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That backdrop surrounds the Blur node when you look at the LookdevX graph. Blur does not work with procedurals as we know. Once that is fixed this should start working and we will have to check the blur amount.
Currently Blur works for textures (bitmaps) but the amount is not what the documentation describe, so there might be more issues that just the procedural processing.
The previous Checker was done in Graph Editor. This version imported Checker into LookdevX and re-authored it similarly to the other more recent procedurals.
The shader uses the stdlib Checker, which does not have a Rotation option. The result was that Rotation was processed before Translation, which is the opposite of OGS. To address this, the stdlib Checker offset is not used any more, and the UV are translated and rotated before being sent to Checker.
Color outside of the pattern when not fully tiled is now black to match other procedurals.
Note that there are small inconsistencies in inputs and formats that will be addressed in bulk affecting multiple nodes at a later stage. Jiras already exist for that.