diff --git a/Changes.md b/Changes.md index e53c52a0ac3..a22c8f457a1 100644 --- a/Changes.md +++ b/Changes.md @@ -1,10 +1,13 @@ + 1.3.16.x (relative to 1.3.16.8) ======== Fixes ----- -- Viewer, ImageGadget : Fixed unnecessary texture updates when specific image tiles don't change. +- Viewer, ImageGadget : + - 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. 1.3.16.8 (relative to 1.3.16.7) ======== diff --git a/include/GafferImageUI/ImageGadget.h b/include/GafferImageUI/ImageGadget.h index 0c8bfe36fa2..0cd2f02dab5 100644 --- a/include/GafferImageUI/ImageGadget.h +++ b/include/GafferImageUI/ImageGadget.h @@ -292,6 +292,7 @@ class GAFFERIMAGEUI_API ImageGadget : public GafferUI::Gadget // Applies previously computed updates for several tiles // such that they become visible to the UI thread together. static void applyUpdates( const std::vector &updates ); + void resetActive(); // Called from the UI thread. const IECoreGL::Texture *texture( bool &active ); diff --git a/src/GafferImageUI/ImageGadget.cpp b/src/GafferImageUI/ImageGadget.cpp index 00423dec669..d7ee10afdd2 100644 --- a/src/GafferImageUI/ImageGadget.cpp +++ b/src/GafferImageUI/ImageGadget.cpp @@ -717,25 +717,18 @@ ImageGadget::Tile::Tile( const Tile &other ) ImageGadget::Tile::Update ImageGadget::Tile::computeUpdate( const GafferImage::ImagePlug *image ) { - try - { - const IECore::MurmurHash h = image->channelDataPlug()->hash(); - Mutex::scoped_lock lock( m_mutex ); - if( m_channelDataHash != MurmurHash() && m_channelDataHash == h ) - { - return Update{ this, nullptr, MurmurHash() }; - } - - m_active = true; - m_activeStartTime = std::chrono::steady_clock::now(); - lock.release(); // Release while doing expensive calculation so UI thread doesn't wait. - ConstFloatVectorDataPtr channelData = image->channelDataPlug()->getValue( &h ); - return Update{ this, channelData, h }; - } - catch( ... ) + const IECore::MurmurHash h = image->channelDataPlug()->hash(); + Mutex::scoped_lock lock( m_mutex ); + if( m_channelDataHash != MurmurHash() && m_channelDataHash == h ) { return Update{ this, nullptr, MurmurHash() }; } + + m_active = true; + m_activeStartTime = std::chrono::steady_clock::now(); + lock.release(); // Release while doing expensive calculation so UI thread doesn't wait. + ConstFloatVectorDataPtr channelData = image->channelDataPlug()->getValue( &h ); + return Update{ this, channelData, h }; } void ImageGadget::Tile::applyUpdates( const std::vector &updates ) @@ -761,6 +754,12 @@ void ImageGadget::Tile::applyUpdates( const std::vector &updates ) } } +void ImageGadget::Tile::resetActive() +{ + Mutex::scoped_lock lock( m_mutex ); + m_active = false; +} + const IECoreGL::Texture *ImageGadget::Tile::texture( bool &active ) { const auto now = std::chrono::steady_clock::now(); @@ -858,28 +857,42 @@ void ImageGadget::updateTiles() auto tileFunctor = [this, channelsToCompute] ( const ImagePlug *image, const V2i &tileOrigin ) { - vector updates; - ImagePlug::ChannelDataScope channelScope( Context::current() ); - for( auto &channelName : channelsToCompute ) + try { - channelScope.setChannelName( &channelName ); - Tile &tile = m_tiles[TileIndex(tileOrigin, channelName)]; - updates.push_back( tile.computeUpdate( image ) ); - } + vector updates; + ImagePlug::ChannelDataScope channelScope( Context::current() ); + for( auto &channelName : channelsToCompute ) + { + channelScope.setChannelName( &channelName ); + Tile &tile = m_tiles[TileIndex(tileOrigin, channelName)]; + updates.push_back( tile.computeUpdate( image ) ); + } - Tile::applyUpdates( updates ); + Tile::applyUpdates( updates ); - if( refCount() && !m_renderRequestPending.exchange( true ) ) + if( refCount() && !m_renderRequestPending.exchange( true ) ) + { + // Must hold a reference to stop us dying before our UI thread call is scheduled. + ImageGadgetPtr thisRef = this; + ParallelAlgo::callOnUIThread( + [thisRef] { + thisRef->m_renderRequestPending = false; + thisRef->Gadget::dirty( DirtyType::Render ); + } + ); + } + } + catch( ... ) { - // Must hold a reference to stop us dying before our UI thread call is scheduled. - ImageGadgetPtr thisRef = this; - ParallelAlgo::callOnUIThread( - [thisRef] { - thisRef->m_renderRequestPending = false; - thisRef->Gadget::dirty( DirtyType::Render ); - } - ); + // We don't want to call `Tile::applyUpdates()` because we won't have + // a complete set of updates for all channels. But we do need to turn off + // the active flag for each tile. + for( auto &channelName : channelsToCompute ) + { + m_tiles[TileIndex(tileOrigin, channelName)].resetActive(); + } } + }; Context::Scope scopedContext( m_context.get() );