GraphEditor : Add per-instance versions of static signals - #7060
Open
johnhaddon wants to merge 2 commits into
Open
GraphEditor : Add per-instance versions of static signals#7060johnhaddon wants to merge 2 commits into
johnhaddon wants to merge 2 commits into
Conversation
The static signals were convenient, but left us in the awkward position of not having an easy way to customise individual GraphEditors. It was also confusing that some signals were available statically but others were not, as documented in issue GafferHQ#137. It's an open question as to whether or not we will transition fully to only having instance signals or we will allow them to exist in a hybrid state. Much hinges on performance considerations when we look at tackling `PlugValueWidget.popupMenuSignal()` in the same way - will a `PlugValueWidget.instanceCreatedSignal()` and ensuing per-instance connections be prohibitively constly for this to be our only approach? That's for another day though. For now we at least have a clear path forwards for a consistent approach for signal-handling at the Editor level.
johnhaddon
force-pushed
the
nonStaticMenuSignals
branch
from
July 30, 2026 13:39
9f8adff to
3c36360
Compare
4 tasks
ericmehl
requested changes
Jul 31, 2026
ericmehl
left a comment
ericmehl
left a comment
Contributor
There was a problem hiding this comment.
Thanks for adding this additional signal mechanism, I think it will help clear up some of the confusion around editor signals.
The nodule hiding code is looking cleaner now too, thanks for the improvements there. It's working fine on the manual test cases I've been checking against, so I don't have anything to add there.
Just one comment inline and I think it is good to go.
| return cls.__plugContextMenuSignal | ||
| return classOrSelf.__plugContextMenuSignal | ||
|
|
||
| __connectionContextMenuSignal = Gaffer.Signals.Signal3() |
Contributor
There was a problem hiding this comment.
This, and the other private signals like it, are redundant now that they are declared in __init__(), correct?
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
The static signals were convenient, but left us in the awkward position of not having an easy way to customise individual GraphEditors. It was also confusing that some signals were available statically but others were not, as documented in issue #137, and bumped into when considering how to enable the features from #7020.
I've also included the hide-unconnected feature from #7020 in this PR, enabled using a
connectToEditor()function which I think will become our standard way of bolting on extensions to the UI now we're moving away from static signals. I've done some additional refactoring here, so that even the old plug menu item uses the same single-source-of-truth as the new node menu item and hotkey. See what you think @ericmehl - the code is a fair bit shorter now, but no doubt I've broken something at the same time.