Skip to content

Commit

Permalink
ImageGadget : Don't clear dirty flag after cancellation
Browse files Browse the repository at this point in the history
In most cases this didn't matter, because we were only getting cancelled by a graph edit which would eventually dirty the input plug anyway. But cancellation could be due to an edit in an unrelated part of the graph, in which case our input plug won't be dirtied again, but we do want to restart the update.

Fixes #6043
  • Loading branch information
johnhaddon committed Sep 19, 2024
1 parent 73e45e2 commit 5e04892
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 2 deletions.
1 change: 1 addition & 0 deletions Changes.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ Fixes
- USD asset paths on Windows now use `/` as the path separator, instead of `\`. Set `IECOREUSD_FORCE_ASSET_PATH_FORWARD_SLASH=0` to keep the previous behaviour.
- SceneWriter : Fixed crash when writing to a USD file that is already open elsewhere.
- Viewer, ImageGadget :
- 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.
- Constraint : The `target` browser now shows locations from the `targetScene` if it has an input connection. Before it always showed locations from the main input.
Expand Down
23 changes: 21 additions & 2 deletions src/GafferImageUI/ImageGadget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
#include "Gaffer/BackgroundTask.h"
#include "Gaffer/Context.h"
#include "Gaffer/Node.h"
#include "Gaffer/Process.h"
#include "Gaffer/ScriptNode.h"

#include "IECore/MessageHandler.h"
Expand Down Expand Up @@ -891,6 +892,7 @@ void ImageGadget::updateTiles()
{
m_tiles[TileIndex(tileOrigin, channelName)].resetActive();
}
throw;
}

};
Expand All @@ -902,8 +904,24 @@ void ImageGadget::updateTiles()
// OK to capture `this` via raw pointer, because ~ImageGadget waits for
// the background process to complete.
[ this, channelsToCompute, dataWindow, tileFunctor ] {
ImageAlgo::parallelProcessTiles( m_image.get(), tileFunctor, dataWindow );
m_dirtyFlags &= ~TilesDirty;

try
{
ImageAlgo::parallelProcessTiles( m_image.get(), tileFunctor, dataWindow );
m_dirtyFlags &= ~TilesDirty;
}
catch( const Gaffer::ProcessException & )
{
// No point starting a new compute if it's just
// going to error again.
m_dirtyFlags &= ~TilesDirty;
}
catch( const IECore::Cancelled & )
{
// Don't clear dirty flag, so that we restart
// on the next redraw.
}

if( refCount() )
{
ImageGadgetPtr thisRef = this;
Expand All @@ -913,6 +931,7 @@ void ImageGadget::updateTiles()
}
);
}

}
);

Expand Down

0 comments on commit 5e04892

Please sign in to comment.