Skip to content

Commit

Permalink
StandardNodeGadget : Fix lingering error badges
Browse files Browse the repository at this point in the history
Errors are caused by evaluating nodes with bad input, either in the form of plug values or of context variables. We have always cleared error badges when plugs are dirtied, which takes care of the situation where the user has fixed the input in the form of plug values. But until now we have stuck our head in the sand completely when it comes to errors due to bad contexts.

But now we have the ContextTracker, we can associate errors with tracked contexts, and when the tracked context changes, clear any errors from previous contexts.

Fixes GafferHQ#3820.
  • Loading branch information
johnhaddon committed Sep 24, 2024
1 parent 3e13d2e commit a8fd2ca
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 1 deletion.
2 changes: 2 additions & 0 deletions Changes.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ Fixes
- Fixed partial image updates when an unrelated InteractiveRender was running (#6043).
- Fixed "colour tearing", where updates to some image channels became visible before updates to others.
- Fixed unnecessary texture updates when specific image tiles don't change.
- GraphEditor :
- Fixed lingering error badges (#3820).

[^1]: To be omitted from 1.5.0.0 release notes.

Expand Down
31 changes: 30 additions & 1 deletion src/GafferUI/StandardNodeGadget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,10 @@ class StandardNodeGadget::ErrorGadget : public Gadget
{
PlugEntry &entry = m_errors[plug];
entry.error = error;
if( auto tracker = ContextTracker::acquireForFocus( plug.get() ) )
{
entry.trackedContext = tracker->context( plug.get() )->hash();
}
if( !entry.parentChangedConnection.connected() )
{
entry.parentChangedConnection = plug->parentChangedSignal().connect( boost::bind( &ErrorGadget::plugParentChanged, this, ::_1 ) );
Expand All @@ -395,6 +399,27 @@ class StandardNodeGadget::ErrorGadget : public Gadget
m_image->setVisible( m_errors.size() );
}

void removeStaleErrors( const ContextTracker *tracker )
{
// We call `removeError()` whenever a plug is dirtied, but that can
// still leave errors lingering if the original error was a problem
// of _context_ rather than plug values. So when the tracked context
// changes, we remove any errors which occurred within a different
// tracked context.
for( auto it = m_errors.begin(); it != m_errors.end(); )
{
if( it->second.trackedContext != tracker->context( it->first.get() )->hash() )
{
it = m_errors.erase( it );
}
else
{
++it;
}
}
m_image->setVisible( m_errors.size() );
}

std::string getToolTip( const IECore::LineSegment3f &position ) const override
{
std::string result = Gadget::getToolTip( position );
Expand Down Expand Up @@ -434,6 +459,7 @@ class StandardNodeGadget::ErrorGadget : public Gadget
struct PlugEntry
{
std::string error;
IECore::MurmurHash trackedContext;
Signals::ScopedConnection parentChangedConnection;
};

Expand Down Expand Up @@ -786,13 +812,16 @@ void StandardNodeGadget::updateFromContextTracker( const ContextTracker *context
{
m_nodeEnabledInContextTracker = true;
}
/// \todo Should we clear ErrorGadget when the context changes?
}
else
{
m_nodeEnabledInContextTracker = std::nullopt;
}
updateStrikeThroughVisibility();
if( auto g = errorGadget( /* createIfMissing = */ false ) )
{
g->removeStaleErrors( contextTracker );
}
}

void StandardNodeGadget::updateTextDimming()
Expand Down

0 comments on commit a8fd2ca

Please sign in to comment.