ShaderAssignment shader plug on left hand side #693
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fixes GafferHQ#157. That issue does discuss showing object counts and other things, of which I'm quite skeptical - I think it'll be too expensive. No doubt there is additional information which could go be displayed though, so the tooltip will also show a "summary" Metadata item if it has been registered - this will allow individual node types to add useful information by registering a dynamic Metadata item.
This just claims some space and doesn't render anything into it - useful for putting blank space into layouts.
It now uses the new Spacer class to get the size behaviour it wants, rather than lots of funky logic in bound() and doRender(). It also always has containers for nodules on all sides, rather than just at the top/bottom or left/right - current nodule location is still dictated by the orientation parameter as before.
This allows nodules to be placed arbitrarily on the left/right/bottom or top. Initially only the ShaderAssignment node makes use of this, placing the shader nodule on the left. Fixes GafferHQ#82.
|
Code looks fine. Only thing I wonder is if the title of the node description popup should be the node type instead of the name? The name is already visible in the graph, whereas the node type is not. Or possibly have both in the description for when you zoom out and can't see the names anymore? |
The name is already visible in the NodeGraph anyway.
|
Good point - updated to show node type instead. Should I make a ticket for the great string matching unification of 2014? |
|
Sure |
andrewkaufman
added a commit
that referenced
this pull request
Dec 17, 2013
ShaderAssignment shader plug on left hand side
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request implements #82 - placing the shader plug on the left hand side of the ShaderAssignment node in the NodeGraph. It allows all other node types to request similar specific positioning using a "nodeGadget:nodulePosition" Metadata entry. This necessitated reimplementing the Metadata class in C++ rather than Python, which we've needed for a while anyway. Since #157 was just waiting on that, I threw it in there as well.
I'm not entirely pleased with the way string matching is going in the API at the moment. Some of the factories (Nodule for instance) accept strings which are treated as regexes, others accept strings which are treated as fnmatch() patterns (PlugValueWidget for instance), and our string matching for the PathFilter uses something similar to fnmatch. Metadata was also using strings as fnmatch patterns, so I've had to implement a conversion to regexes on the way into C++ - this keeps compatibility with the existing python code but adds another permutation to what we've already got. I'd like to refactor all the string matching to use identical code at some point - does making a ticket for that seem reasonable? Currently I'm thinking that although regexes are powerful, glob style matching is actually better because it's easier to use for the average person (I certainly have difficulty reading more than the most basic regexes).