From b5dfb0468ead331593b706bb116d6a6aed97d88c Mon Sep 17 00:00:00 2001 From: David Steele Date: Mon, 22 Jan 2018 15:15:48 +0000 Subject: [PATCH] Prevent texture removal after TextureManager destroyed On shutdown, if there is an ImageView actor with an AnimatedImageVisual still in the tree, then on destruction it tries to kill off textures. However, TextureManager has already been shutdown and can't be called. Adding a lifecycle observer so AnimatedImageVisual helper classes can shutdown cleanly. Change-Id: I5f07288291b850e69f45df879cce8c27bccb3f6e --- .../visuals/animated-image/fixed-image-cache.cpp | 7 +++-- .../visuals/animated-image/image-cache.cpp | 13 ++++++++- .../internal/visuals/animated-image/image-cache.h | 9 +++++- .../animated-image/rolling-gif-image-cache.cpp | 10 +++++-- .../visuals/animated-image/rolling-image-cache.cpp | 9 ++++-- .../internal/visuals/texture-manager-impl.cpp | 33 ++++++++++++++++++++++ .../internal/visuals/texture-manager-impl.h | 29 +++++++++++++++++-- 7 files changed, 98 insertions(+), 12 deletions(-) diff --git a/dali-toolkit/internal/visuals/animated-image/fixed-image-cache.cpp b/dali-toolkit/internal/visuals/animated-image/fixed-image-cache.cpp index b1713d2..dd06d31 100644 --- a/dali-toolkit/internal/visuals/animated-image/fixed-image-cache.cpp +++ b/dali-toolkit/internal/visuals/animated-image/fixed-image-cache.cpp @@ -45,9 +45,12 @@ FixedImageCache::FixedImageCache( FixedImageCache::~FixedImageCache() { - for( std::size_t i = 0; i < mImageUrls.size() ; ++i ) + if( mTextureManagerAlive ) { - mTextureManager.Remove( mImageUrls[i].mTextureId ); + for( std::size_t i = 0; i < mImageUrls.size() ; ++i ) + { + mTextureManager.Remove( mImageUrls[i].mTextureId ); + } } } diff --git a/dali-toolkit/internal/visuals/animated-image/image-cache.cpp b/dali-toolkit/internal/visuals/animated-image/image-cache.cpp index 84e5f66..78ca796 100644 --- a/dali-toolkit/internal/visuals/animated-image/image-cache.cpp +++ b/dali-toolkit/internal/visuals/animated-image/image-cache.cpp @@ -31,12 +31,23 @@ ImageCache::ImageCache( TextureManager& textureManager, mBatchSize( batchSize ), mUrlIndex(0u), mWaitingForReadyFrame(false), - mRequestingLoad(false) + mRequestingLoad(false), + mTextureManagerAlive(true) { + mTextureManager.AddObserver( *this ); } ImageCache::~ImageCache() { + if( mTextureManagerAlive ) + { + mTextureManager.RemoveObserver( *this ); + } +} + +void ImageCache::TextureManagerDestroyed() +{ + mTextureManagerAlive = false; } } //namespace Internal diff --git a/dali-toolkit/internal/visuals/animated-image/image-cache.h b/dali-toolkit/internal/visuals/animated-image/image-cache.h index 1a7ae75..e8277de 100644 --- a/dali-toolkit/internal/visuals/animated-image/image-cache.h +++ b/dali-toolkit/internal/visuals/animated-image/image-cache.h @@ -28,7 +28,7 @@ namespace Toolkit namespace Internal { -class ImageCache +class ImageCache : public TextureManager::LifecycleObserver { public: /** @@ -85,6 +85,12 @@ public: */ virtual TextureSet NextFrame() = 0; +private: + /** + * Called before the texture manager is destroyed. + */ + virtual void TextureManagerDestroyed() override final; + protected: TextureManager& mTextureManager; FrameReadyObserver& mObserver; @@ -92,6 +98,7 @@ protected: unsigned int mUrlIndex; bool mWaitingForReadyFrame:1; bool mRequestingLoad:1; + bool mTextureManagerAlive:1; }; } //namespace Internal diff --git a/dali-toolkit/internal/visuals/animated-image/rolling-gif-image-cache.cpp b/dali-toolkit/internal/visuals/animated-image/rolling-gif-image-cache.cpp index ed61adb..9f9e623 100644 --- a/dali-toolkit/internal/visuals/animated-image/rolling-gif-image-cache.cpp +++ b/dali-toolkit/internal/visuals/animated-image/rolling-gif-image-cache.cpp @@ -74,13 +74,17 @@ RollingGifImageCache::RollingGifImageCache( RollingGifImageCache::~RollingGifImageCache() { - while( !mQueue.IsEmpty() ) + if( mTextureManagerAlive ) { - ImageFrame imageFrame = mQueue.PopFront(); - Dali::Toolkit::TextureManager::RemoveTexture( mImageUrls[ imageFrame.mFrameNumber ].mUrl ); + while( !mQueue.IsEmpty() ) + { + ImageFrame imageFrame = mQueue.PopFront(); + Dali::Toolkit::TextureManager::RemoveTexture( mImageUrls[ imageFrame.mFrameNumber ].mUrl ); + } } } + TextureSet RollingGifImageCache::FirstFrame() { return GetFrontTextureSet(); diff --git a/dali-toolkit/internal/visuals/animated-image/rolling-image-cache.cpp b/dali-toolkit/internal/visuals/animated-image/rolling-image-cache.cpp index 37c7a7d..bee7234 100644 --- a/dali-toolkit/internal/visuals/animated-image/rolling-image-cache.cpp +++ b/dali-toolkit/internal/visuals/animated-image/rolling-image-cache.cpp @@ -70,10 +70,13 @@ RollingImageCache::RollingImageCache( RollingImageCache::~RollingImageCache() { - while( !mQueue.IsEmpty() ) + if( mTextureManagerAlive ) { - ImageFrame imageFrame = mQueue.PopFront(); - mTextureManager.Remove( mImageUrls[ imageFrame.mUrlIndex ].mTextureId ); + while( !mQueue.IsEmpty() ) + { + ImageFrame imageFrame = mQueue.PopFront(); + mTextureManager.Remove( mImageUrls[ imageFrame.mUrlIndex ].mTextureId ); + } } } diff --git a/dali-toolkit/internal/visuals/texture-manager-impl.cpp b/dali-toolkit/internal/visuals/texture-manager-impl.cpp index a12fae0..272ad4e 100644 --- a/dali-toolkit/internal/visuals/texture-manager-impl.cpp +++ b/dali-toolkit/internal/visuals/texture-manager-impl.cpp @@ -120,6 +120,14 @@ TextureManager::TextureManager() { } +TextureManager::~TextureManager() +{ + for( auto iter = mLifecycleObservers.Begin(), endIter = mLifecycleObservers.End(); iter != endIter; ++iter) + { + (*iter)->TextureManagerDestroyed(); + } +} + TextureSet TextureManager::LoadTexture( const VisualUrl& url, Dali::ImageDimensions desiredSize, Dali::FittingMode::Type fittingMode, Dali::SamplingMode::Type samplingMode, const MaskingDataPointer& maskInfo, @@ -574,6 +582,31 @@ TextureSet TextureManager::RemoveExternalTexture( const std::string& url ) return TextureSet(); } + +void TextureManager::AddObserver( TextureManager::LifecycleObserver& observer ) +{ + // make sure an observer doesn't observe the same object twice + // otherwise it will get multiple calls to ObjectDestroyed() + DALI_ASSERT_DEBUG( mLifecycleObservers.End() == std::find( mLifecycleObservers.Begin(), mLifecycleObservers.End(), &observer)); + mLifecycleObservers.PushBack( &observer ); +} + +void TextureManager::RemoveObserver( TextureManager::LifecycleObserver& observer) +{ + // Find the observer... + auto endIter = mLifecycleObservers.End(); + for( auto iter = mLifecycleObservers.Begin(); iter != endIter; ++iter) + { + if( (*iter) == &observer) + { + mLifecycleObservers.Erase( iter ); + break; + } + } + DALI_ASSERT_DEBUG(endIter != mLifecycleObservers.End()); +} + + bool TextureManager::LoadTexture( TextureInfo& textureInfo ) { bool success = true; diff --git a/dali-toolkit/internal/visuals/texture-manager-impl.h b/dali-toolkit/internal/visuals/texture-manager-impl.h index 5b6a98e..8685ec6 100644 --- a/dali-toolkit/internal/visuals/texture-manager-impl.h +++ b/dali-toolkit/internal/visuals/texture-manager-impl.h @@ -136,6 +136,18 @@ public: }; using MaskingDataPointer = std::unique_ptr; + + /** + * Class to provide lifecycle event on destruction of texture manager. + */ + struct LifecycleObserver + { + /** + * Called shortly before the texture manager is destroyed. + */ + virtual void TextureManagerDestroyed() = 0; + }; + /** * Constructor. */ @@ -144,8 +156,7 @@ public: /** * Destructor. */ - ~TextureManager() = default; - + ~TextureManager(); // TextureManager Main API: @@ -329,6 +340,19 @@ public: */ TextureSet RemoveExternalTexture( const std::string& url ); + /** + * Add an observer to the object. + * @param[in] observer The observer to add. + */ + void AddObserver( TextureManager::LifecycleObserver& observer ); + + /** + * Remove an observer from the object + * @pre The observer has already been added. + * @param[in] observer The observer to remove. + */ + void RemoveObserver( TextureManager::LifecycleObserver& observer ); + private: /** @@ -710,6 +734,7 @@ private: // Member Variables: RoundRobinContainerView< AsyncLoadingHelper > mAsyncLocalLoaders; ///< The Asynchronous image loaders used to provide all local async loads RoundRobinContainerView< AsyncLoadingHelper > mAsyncRemoteLoaders; ///< The Asynchronous image loaders used to provide all remote async loads std::vector< ExternalTextureInfo > mExternalTextures; ///< Externally provided textures + Dali::Vector mLifecycleObservers; ///< Lifecycle observers of texture manager TextureId mCurrentTextureId; ///< The current value used for the unique Texture Id generation }; -- 2.7.4