Skip to content

Commit

Permalink
NodeGadget, ConnectionGadget : Add updateFromContextTracker()
Browse files Browse the repository at this point in the history
And remove the less flexible `activeForFocusNode()` method that it replaces. This allows us to use the appropriate "focus aware" context for the disabled node strike-through.
  • Loading branch information
johnhaddon committed Jul 25, 2024
1 parent bc3c7e6 commit c6e4047
Show file tree
Hide file tree
Showing 8 changed files with 77 additions and 50 deletions.
3 changes: 3 additions & 0 deletions Changes.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ Improvements
- GraphGadget :
- Improved highlighting of active nodes, with more accurate tracking of Loop node iterations.
- Annotation `{plug}` substitutions are now evaluated in a context determined relative to the focus node.
- The strike-through for disabled nodes is now evaluated in a context determined relative to the focus node.

Fixes
-----
Expand All @@ -35,6 +36,7 @@ API
- PlugValueWidget :
- A `DeprecationWarning` is now emitted for any subclasses still implementing the legacy `_updateFromPlug()` or `_updateFromPlugs()` methods. Implement `_updateFromValues()`, `_updateFromMetadata()` and `_updateFromEditable()` instead.
- A `DeprecationWarning` is now emitted by `_plugConnections()`. Use `_blockedUpdateFromValues()` instead.
- NodeGadget, ConnectionGadget : Added `updateFromContextTracker()` virtual methods.

Breaking Changes
----------------
Expand All @@ -52,6 +54,7 @@ Breaking Changes
- Removed `setContext()` methods.
- Deprecated `getContext()` methods. Use `context()` instead.
- Loop : Removed `nextIterationContext()` method.
- NodeGadget, ConnectionGadget : Removed `activeForFocusNode()` virtual methods. Override `updateFromContextTracker()` instead.

1.4.x.x (relative to 1.4.9.0)
=======
Expand Down
8 changes: 6 additions & 2 deletions include/GafferUI/ConnectionGadget.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
namespace GafferUI
{

IE_CORE_FORWARDDECLARE( ContextTracker )
IE_CORE_FORWARDDECLARE( Nodule )
IE_CORE_FORWARDDECLARE( ConnectionGadget )

Expand Down Expand Up @@ -120,8 +121,11 @@ class GAFFERUI_API ConnectionGadget : public ConnectionCreator
static ConnectionGadgetPtr creator( NodulePtr srcNodule, NodulePtr dstNodule ) { return new T( srcNodule, dstNodule ); };
};

virtual void activeForFocusNode( bool active );

/// May be overridden to update the UI state to reflect changes in the ContextTracker.
/// Calls to this are made by the parent GraphGadget, to avoid every individual
/// ConnectionGadget needing to connect to the ContextTracker signals itself.
virtual void updateFromContextTracker( const ContextTracker *contextTracker );
/// Friendship to allow calling `updateFromContextTracker()`.
friend class GraphGadget;

bool m_active;
Expand Down
8 changes: 6 additions & 2 deletions include/GafferUI/NodeGadget.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
namespace GafferUI
{

IE_CORE_FORWARDDECLARE( ContextTracker )
IE_CORE_FORWARDDECLARE( Nodule )
IE_CORE_FORWARDDECLARE( NodeGadget )
IE_CORE_FORWARDDECLARE( ConnectionCreator )
Expand Down Expand Up @@ -113,8 +114,11 @@ class GAFFERUI_API NodeGadget : public Gadget
static NodeGadgetPtr creator( Gaffer::NodePtr node ) { return new T( node ); };
};

virtual void activeForFocusNode( bool active );

/// May be overridden to update the UI state to reflect changes in the ContextTracker.
/// Calls to this are made by the parent GraphGadget, to avoid every individual
/// NodeGadget needing to connect to the ContextTracker signals itself.
virtual void updateFromContextTracker( const ContextTracker *contextTracker );
/// Friendship to allow calling `updateFromContextTracker()`.
friend class GraphGadget;

bool m_active;
Expand Down
7 changes: 4 additions & 3 deletions include/GafferUI/StandardNodeGadget.h
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ class GAFFERUI_API StandardNodeGadget : public NodeGadget

const Imath::Color3f *userColor() const;

void activeForFocusNode( bool active ) override;
void updateFromContextTracker( const ContextTracker *contextTracker ) override;

private :

Expand Down Expand Up @@ -158,7 +158,7 @@ class GAFFERUI_API StandardNodeGadget : public NodeGadget
bool updateUserColor();
void updateMinWidth();
void updatePadding();
void updateNodeEnabled( const Gaffer::Plug *dirtiedPlug = nullptr );
void updateStrikeThroughVisibility( const Gaffer::Plug *dirtiedPlug = nullptr );
void updateIcon();
bool updateShape();
void updateFocusGadgetVisibility();
Expand All @@ -169,7 +169,8 @@ class GAFFERUI_API StandardNodeGadget : public NodeGadget
void error( const Gaffer::Plug *plug, const Gaffer::Plug *source, const std::string &message );
void displayError( Gaffer::ConstPlugPtr plug, const std::string &message );

bool m_nodeEnabled;
std::optional<bool> m_nodeEnabledInContextTracker;
bool m_strikeThroughVisible;
bool m_labelsVisibleOnHover;
// We accept drags onto the NodeGadget itself and
// use them to create a connection to the
Expand Down
3 changes: 2 additions & 1 deletion src/GafferUI/ConnectionGadget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -202,8 +202,9 @@ ConnectionGadget::NamedCreatorMap &ConnectionGadget::namedCreators()
return *m;
}

void ConnectionGadget::activeForFocusNode( bool active )
void ConnectionGadget::updateFromContextTracker( const ContextTracker *contextTracker )
{
const bool active = contextTracker->targetNode() ? contextTracker->isTracked( m_dstNodule->plug() ) : true;
if( m_active != active )
{
m_active = active;
Expand Down
9 changes: 4 additions & 5 deletions src/GafferUI/GraphGadget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1690,7 +1690,7 @@ NodeGadget *GraphGadget::addNodeGadget( Gaffer::Node *node )
{
nodeGadget->setHighlighted( true );
}
nodeGadget->activeForFocusNode( m_scriptNode->getFocus() ? m_contextTracker->isActive( node ) : true );
nodeGadget->updateFromContextTracker( m_contextTracker.get() );
}

updateNodeGadgetTransform( nodeGadget.get() );
Expand Down Expand Up @@ -1797,7 +1797,7 @@ void GraphGadget::addConnectionGadget( Nodule *dstNodule )

if( m_scriptNode )
{
connection->activeForFocusNode( m_scriptNode->getFocus() ? m_contextTracker->isActive( dstPlug ) : true );
connection->updateFromContextTracker( m_contextTracker.get() );
}

m_connectionGadgets[dstNodule] = connection.get();
Expand Down Expand Up @@ -1877,16 +1877,15 @@ void GraphGadget::updateConnectionGadgetMinimisation( ConnectionGadget *gadget )

void GraphGadget::applyFocusContexts()
{
const bool haveFocus = m_scriptNode->getFocus();
for( auto &g : children() )
{
if( ConnectionGadget *c = runTimeCast<ConnectionGadget>( g.get() ) )
{
c->activeForFocusNode( haveFocus ? m_contextTracker->isActive( c->dstNodule()->plug() ) : true );
c->updateFromContextTracker( m_contextTracker.get() );
}
else if( NodeGadget *n = runTimeCast<NodeGadget>( g.get() ) )
{
n->activeForFocusNode( haveFocus ? m_contextTracker->isActive( n->node() ) : true );
n->updateFromContextTracker( m_contextTracker.get() );
}
}
/// \todo This should not be necessary. It emulates an old behaviour where we always
Expand Down
4 changes: 3 additions & 1 deletion src/GafferUI/NodeGadget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@

#include "GafferUI/NodeGadget.h"

#include "GafferUI/ContextTracker.h"
#include "GafferUI/LinearContainer.h"
#include "GafferUI/NameGadget.h"
#include "GafferUI/Nodule.h"
Expand Down Expand Up @@ -216,8 +217,9 @@ std::string NodeGadget::getToolTip( const IECore::LineSegment3f &line ) const
return result;
}

void NodeGadget::activeForFocusNode( bool active )
void NodeGadget::updateFromContextTracker( const ContextTracker *contextTracker )
{
const bool active = contextTracker->targetNode() ? contextTracker->isTracked( m_node ) : true;
if( m_active != active )
{
m_active = active;
Expand Down
85 changes: 49 additions & 36 deletions src/GafferUI/StandardNodeGadget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
#include "Gaffer/Metadata.h"
#include "Gaffer/MetadataAlgo.h"
#include "Gaffer/ParallelAlgo.h"
#include "Gaffer/PlugAlgo.h"
#include "Gaffer/ScriptNode.h"
#include "Gaffer/StandardSet.h"
#include "Gaffer/TypedObjectPlug.h"
Expand Down Expand Up @@ -522,7 +523,7 @@ StandardNodeGadget::StandardNodeGadget( Gaffer::NodePtr node )
// can optionally use it without needing to inherit from StandardNodeGadget
StandardNodeGadget::StandardNodeGadget( Gaffer::NodePtr node, bool auxiliary )
: NodeGadget( node ),
m_nodeEnabled( true ),
m_strikeThroughVisible( false ),
m_labelsVisibleOnHover( true ),
m_dragDestination( nullptr ),
m_userColor( 0 ),
Expand Down Expand Up @@ -659,7 +660,6 @@ StandardNodeGadget::StandardNodeGadget( Gaffer::NodePtr node, bool auxiliary )
updateUserColor();
updateMinWidth();
updatePadding();
updateNodeEnabled();
updateIcon();
updateShape();
updateFocusGadgetVisibility();
Expand Down Expand Up @@ -715,7 +715,7 @@ void StandardNodeGadget::renderLayer( Layer layer, const Style *style, RenderRea
{
const Box3f b = bound();

if( !m_nodeEnabled && !isSelectionRender( reason ) )
if( m_strikeThroughVisible && !isSelectionRender( reason ) )
{
/// \todo Replace renderLine() with a specific method (renderNodeStrikeThrough?) on the Style class
/// so that styles can do customised drawing based on knowledge of what is being drawn.
Expand Down Expand Up @@ -772,10 +772,27 @@ void StandardNodeGadget::setHighlighted( bool highlighted )
updateTextDimming();
}

void StandardNodeGadget::activeForFocusNode( bool active )
void StandardNodeGadget::updateFromContextTracker( const ContextTracker *contextTracker )
{
NodeGadget::activeForFocusNode( active );
NodeGadget::updateFromContextTracker( contextTracker );
updateTextDimming();
if( contextTracker->isTracked( node() ) )
{
if( auto dependencyNode = IECore::runTimeCast<DependencyNode>( node() ) )
{
m_nodeEnabledInContextTracker = contextTracker->isEnabled( dependencyNode );
}
else
{
m_nodeEnabledInContextTracker = true;
}
/// \todo Should we clear ErrorGadget when the context changes?
}
else
{
m_nodeEnabledInContextTracker = std::nullopt;
}
updateStrikeThroughVisibility();
}

void StandardNodeGadget::updateTextDimming()
Expand Down Expand Up @@ -1035,7 +1052,7 @@ bool StandardNodeGadget::getLabelsVisibleOnHover() const

void StandardNodeGadget::plugDirtied( const Gaffer::Plug *plug )
{
updateNodeEnabled( plug );
updateStrikeThroughVisibility( plug );
if( ErrorGadget *e = errorGadget( /* createIfMissing = */ false ) )
{
e->removeError( plug );
Expand Down Expand Up @@ -1260,44 +1277,40 @@ void StandardNodeGadget::updatePadding()
paddingRow()->setPadding( Box3f( V3f( -padding ), V3f( padding ) ) );
}

void StandardNodeGadget::updateNodeEnabled( const Gaffer::Plug *dirtiedPlug )
void StandardNodeGadget::updateStrikeThroughVisibility( const Gaffer::Plug *dirtiedPlug )
{
DependencyNode *dependencyNode = IECore::runTimeCast<DependencyNode>( node() );
if( !dependencyNode )
{
return;
}

const Gaffer::BoolPlug *enabledPlug = dependencyNode->enabledPlug();
if( !enabledPlug )
{
return;
}

if( dirtiedPlug && dirtiedPlug != enabledPlug )
bool strikeThroughVisible = false;
if( m_nodeEnabledInContextTracker )
{
return;
strikeThroughVisible = !*m_nodeEnabledInContextTracker;
}

const ValuePlug *source = enabledPlug->source<ValuePlug>();
bool enabled = true;
if( source->direction() != Plug::Out || !IECore::runTimeCast<const ComputeNode>( source->node() ) )
else
{
// Only evaluate `enabledPlug` if it won't trigger a compute.
// We don't want to hang the UI waiting, and we don't really
// know what context to perform the compute in anyway.
/// \todo We could consider doing this in the background, using
/// an upstream traversal from the focus node to determine context.
enabled = enabledPlug->getValue();
if( auto dependencyNode = IECore::runTimeCast<DependencyNode>( node() ) )
{
strikeThroughVisible = false;
if( auto enabledPlug = dependencyNode->enabledPlug() )
{
if( dirtiedPlug && dirtiedPlug != enabledPlug )
{
return;
}
// Only evaluate `enabledPlug` if it won't trigger a compute.
// We don't want to hang the UI waiting, and we don't really
// know what context to perform the compute in anyway.
if( !PlugAlgo::dependsOnCompute( enabledPlug ) )
{
strikeThroughVisible = !enabledPlug->getValue();
}
}
}
}

if( enabled == m_nodeEnabled )
if( strikeThroughVisible != m_strikeThroughVisible )
{
return;
m_strikeThroughVisible = strikeThroughVisible;
dirty( DirtyType::Render );
}

m_nodeEnabled = enabled;
dirty( DirtyType::Render );
}

void StandardNodeGadget::updateIcon()
Expand Down

0 comments on commit c6e4047

Please sign in to comment.