[4.0] Ensured ImageView requests inside ResourceReady signal handler are queued. 24/172424/1
authorDavid Steele <david.steele@samsung.com>
Tue, 13 Mar 2018 20:38:09 +0000 (20:38 +0000)
committerHeeyong Song <heeyong.song@samsung.com>
Wed, 14 Mar 2018 02:20:14 +0000 (11:20 +0900)
Deferring ImageView load requests until after ResourceReady signal handler.
has completed ensures that attempting to re-load failed images doesn't
fail to send a second ResourceReady callback.

An application can still cause an infinite loop if it doesn't have a max
retry count when attempting to re-load failed images inside the signal
handler. This is considered to be an application bug, not a DALi bug.
( Control::ResourceReady signal is not a one-shot signal).

Change-Id: I2c505623ce5e02d3ae67e6e06fd80d5108dc8ade
Signed-off-by: David Steele <david.steele@samsung.com>
automated-tests/src/dali-toolkit/dali-toolkit-test-utils/toolkit-event-thread-callback.cpp
automated-tests/src/dali-toolkit/utc-Dali-ImageView.cpp
dali-toolkit/internal/visuals/texture-manager-impl.cpp
dali-toolkit/internal/visuals/texture-manager-impl.h

index 5b8a324..02f3d8f 100644 (file)
@@ -77,7 +77,7 @@ bool EventThreadCallback::WaitingForTrigger()
   struct timespec now;
   clock_gettime( CLOCK_REALTIME, &now );
   if( now.tv_nsec < 999900000 ) // 999, 900, 000
-    now.tv_nsec += 100000;
+    now.tv_nsec += 1000;
   else
   {
     now.tv_sec += 1;
@@ -124,6 +124,10 @@ bool WaitForEventThreadTrigger( int triggerCount, int timeoutInSeconds )
           Dali::CallbackBase::Execute( *callback );
           triggerCount--;
         }
+        if( triggerCount <= 0 )
+        {
+          break;
+        }
       }
     }
     clock_gettime( CLOCK_REALTIME, &now );
index c9cbdc5..25a397c 100644 (file)
@@ -1619,3 +1619,57 @@ int UtcDaliImageViewResourceReadySignalWithReusedImage02(void)
 
   END_TEST;
 }
+
+
+static int gFailCounter = 0;
+const int MAX_RETRIES(3);
+
+void ReloadImage(ImageView imageView)
+{
+  Property::Map imageImmediateLoadingMap;
+  imageImmediateLoadingMap[ ImageVisual::Property::URL ] = "Non-existant-image.jpg";
+  imageImmediateLoadingMap[ ImageVisual::Property::LOAD_POLICY ] =  ImageVisual::LoadPolicy::IMMEDIATE;
+
+  tet_infoline("Immediate load an image");
+  imageView.SetProperty( ImageView::Property::IMAGE, imageImmediateLoadingMap );
+}
+
+void ResourceFailedReload( Control control )
+{
+  gFailCounter++;
+  if( gFailCounter < MAX_RETRIES )
+  {
+    ReloadImage(ImageView::DownCast(control));
+  }
+}
+
+int UtcDaliImageViewReloadFailedOnResourceReadySignal(void)
+{
+  tet_infoline("Test Setting Image that was already loaded by another ImageView and still getting ResourceReadySignal when staged.");
+
+  ToolkitTestApplication application;
+
+  gFailCounter = 0;
+
+  ImageView imageView = ImageView::New();
+  imageView.ResourceReadySignal().Connect( &ResourceFailedReload );
+  DALI_TEST_EQUALS( gFailCounter, 0, TEST_LOCATION );
+  ReloadImage(imageView);
+
+  // loading started, this waits for the loader thread to complete
+  DALI_TEST_EQUALS( Test::WaitForEventThreadTrigger( 1 ), true, TEST_LOCATION );
+  application.SendNotification();
+
+  DALI_TEST_EQUALS( gFailCounter, 1, TEST_LOCATION );
+
+  DALI_TEST_EQUALS( Test::WaitForEventThreadTrigger( 1 ), true, TEST_LOCATION );
+  application.SendNotification();
+
+  DALI_TEST_EQUALS( gFailCounter, 2, TEST_LOCATION );
+
+  DALI_TEST_EQUALS( Test::WaitForEventThreadTrigger( 1 ), true, TEST_LOCATION );
+  application.SendNotification();
+  DALI_TEST_EQUALS( gFailCounter, 3, TEST_LOCATION );
+
+  END_TEST;
+}
index 7f62d22..4de3fb5 100755 (executable)
@@ -1,5 +1,5 @@
  /*
- * Copyright (c) 2017 Samsung Electronics Co., Ltd.
+ * Copyright (c) 2018 Samsung Electronics Co., Ltd.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -116,7 +116,10 @@ TextureManager::MaskingData::MaskingData()
 TextureManager::TextureManager()
 : mAsyncLocalLoaders( GetNumberOfLocalLoaderThreads(), [&]() { return AsyncLoadingHelper(*this); } ),
   mAsyncRemoteLoaders( GetNumberOfRemoteLoaderThreads(), [&]() { return AsyncLoadingHelper(*this); } ),
-  mCurrentTextureId( 0 )
+  mExternalTextures(),
+  mLoadQueue(),
+  mCurrentTextureId( 0 ),
+  mQueueLoadFlag(false)
 {
 }
 
@@ -374,8 +377,7 @@ TextureManager::TextureId TextureManager::RequestLoadInternal(
     case TextureManager::LOAD_FAILED: // Failed notifies observer which then stops observing.
     case TextureManager::NOT_STARTED:
     {
-      LoadTexture( textureInfo );
-      ObserveTexture( textureInfo, observer );
+      LoadOrQueueTexture( textureInfo, observer ); // If called inside NotifyObservers, queues until afterwards
       break;
     }
     case TextureManager::LOADING:
@@ -573,26 +575,67 @@ TextureSet TextureManager::RemoveExternalTexture( const std::string& url )
   return TextureSet();
 }
 
-bool TextureManager::LoadTexture( TextureInfo& textureInfo )
+void TextureManager::LoadOrQueueTexture( TextureInfo& textureInfo, TextureUploadObserver* observer )
+{
+  switch( textureInfo.loadState )
+  {
+    case NOT_STARTED:
+    case LOAD_FAILED:
+    {
+      if( mQueueLoadFlag )
+      {
+        QueueLoadTexture( textureInfo, observer );
+      }
+      else
+      {
+        LoadTexture( textureInfo, observer );
+      }
+      break;
+    }
+    case LOADING:
+    case UPLOADED:
+    case CANCELLED:
+    case LOAD_FINISHED:
+    case WAITING_FOR_MASK:
+    {
+      break;
+    }
+  }
+}
+
+void TextureManager::QueueLoadTexture( TextureInfo& textureInfo, TextureUploadObserver* observer )
 {
-  bool success = true;
+  auto textureId = textureInfo.textureId;
+  mLoadQueue.PushBack( LoadQueueElement( textureId, observer) );
+}
 
-  if( textureInfo.loadState == NOT_STARTED )
+void TextureManager::LoadTexture( TextureInfo& textureInfo, TextureUploadObserver* observer )
+{
+  textureInfo.loadState = LOADING;
+  if( !textureInfo.loadSynchronously )
   {
-    textureInfo.loadState = LOADING;
+    auto& loadersContainer = textureInfo.url.IsLocalResource() ? mAsyncLocalLoaders : mAsyncRemoteLoaders;
+    auto loadingHelperIt = loadersContainer.GetNext();
+    DALI_ASSERT_ALWAYS(loadingHelperIt != loadersContainer.End());
+    loadingHelperIt->Load(textureInfo.textureId, textureInfo.url,
+                          textureInfo.desiredSize, textureInfo.fittingMode,
+                          textureInfo.samplingMode, textureInfo.orientationCorrection );
+  }
+  ObserveTexture( textureInfo, observer );
+}
 
-    if( !textureInfo.loadSynchronously )
+void TextureManager::ProcessQueuedTextures()
+{
+  for( auto&& element : mLoadQueue )
+  {
+    int cacheIndex = GetCacheIndexFromId( element.mTextureId );
+    if( cacheIndex != INVALID_CACHE_INDEX )
     {
-      auto& loadersContainer = textureInfo.url.IsLocalResource() ? mAsyncLocalLoaders : mAsyncRemoteLoaders;
-      auto loadingHelperIt = loadersContainer.GetNext();
-      DALI_ASSERT_ALWAYS(loadingHelperIt != loadersContainer.End());
-      loadingHelperIt->Load(textureInfo.textureId, textureInfo.url,
-                            textureInfo.desiredSize, textureInfo.fittingMode,
-                            textureInfo.samplingMode, textureInfo.orientationCorrection );
+      TextureInfo& textureInfo( mTextureInfoContainer[cacheIndex] );
+      LoadTexture( textureInfo, element.mObserver );
     }
   }
-
-  return success;
+  mLoadQueue.Clear();
 }
 
 void TextureManager::ObserveTexture( TextureInfo& textureInfo,
@@ -733,7 +776,6 @@ void TextureManager::ApplyMask(
   pixelBuffer.ApplyMask( maskPixelBuffer, contentScale, cropToMask );
 }
 
-
 void TextureManager::UploadTexture( Devel::PixelBuffer& pixelBuffer, TextureInfo& textureInfo )
 {
   if( textureInfo.useAtlas != USE_ATLAS )
@@ -773,53 +815,47 @@ void TextureManager::NotifyObservers( TextureInfo& textureInfo, bool success )
 
   // If there is an observer: Notify the load is complete, whether successful or not,
   // and erase it from the list
-  unsigned int observerCount = textureInfo.observerList.Count();
   TextureInfo* info = &textureInfo;
 
-  while( observerCount )
+  mQueueLoadFlag = true;
+
+  while( info->observerList.Count() )
   {
     TextureUploadObserver* observer = info->observerList[0];
 
     // During UploadComplete() a Control ResourceReady() signal is emitted.
     // During that signal the app may add remove /add Textures (e.g. via
-    // ImageViews).  At this point no more observers can be added to the
-    // observerList, because textureInfo.loadState = UPLOADED. However it is
-    // possible for observers to be removed, hence we check the observer list
-    // count every iteration.
-
-    // The reference to the textureInfo struct can also become invalidated,
-    // because new load requests can modify the mTextureInfoContainer list
-    // (e.g. if more requests are pushed back it can cause the list to be
-    // resized invalidating the reference to the TextureInfo ).
+    // ImageViews).
+    // It is possible for observers to be removed from the observer list,
+    // and it is also possible for the mTextureInfoContainer to be modified,
+    // invalidating the reference to the textureInfo struct.
+    // Texture load requests for the same URL are deferred until the end of this
+    // method.
     observer->UploadComplete( success, info->textureId, info->textureSet, info->useAtlas, info->atlasRect,
                               info->preMultiplied );
     observer->DestructionSignal().Disconnect( this, &TextureManager::ObserverDestroyed );
 
-    // Get the textureInfo from the container again as it may have been
-    // invalidated,
-
+    // Get the textureInfo from the container again as it may have been invalidated.
     int textureInfoIndex = GetCacheIndexFromId( textureId );
     if( textureInfoIndex == INVALID_CACHE_INDEX)
     {
       return; // texture has been removed - can stop.
     }
-
     info = &mTextureInfoContainer[ textureInfoIndex ];
-    observerCount = info->observerList.Count();
-    if ( observerCount > 0 )
+
+    // remove the observer that was just triggered if it's still in the list
+    for( TextureInfo::ObserverListType::Iterator j = info->observerList.Begin(); j != info->observerList.End(); ++j )
     {
-      // remove the observer that was just triggered if it's still in the list
-      for( TextureInfo::ObserverListType::Iterator j = info->observerList.Begin(); j != info->observerList.End(); ++j )
+      if( *j == observer )
       {
-        if( *j == observer )
-        {
-          info->observerList.Erase( j );
-          observerCount--;
-          break;
-        }
+        info->observerList.Erase( j );
+        break;
       }
     }
   }
+
+  mQueueLoadFlag = false;
+  ProcessQueuedTextures();
 }
 
 TextureManager::TextureId TextureManager::GenerateUniqueTextureId()
index 5b6a98e..b0e2606 100755 (executable)
@@ -388,6 +388,8 @@ private:
 
   typedef size_t TextureHash; ///< The type used to store the hash used for Texture caching.
 
+  // Structs:
+
   /**
    * @brief This struct is used to manage the life-cycle of Texture loading and caching.
    */
@@ -459,7 +461,20 @@ private:
     bool preMultiplied:1;          ///< true if the image's color was multiplied by it's alpha
   };
 
-  // Structs:
+  /**
+   * Structure to hold info about a texture load queued during NotifyObservers
+   */
+  struct LoadQueueElement
+  {
+    LoadQueueElement( TextureId textureId, TextureUploadObserver* observer )
+    : mTextureId( textureId ),
+      mObserver( observer )
+    {
+    }
+
+    TextureId mTextureId; ///< The texture id of the requested load.
+    TextureUploadObserver* mObserver; ///< Observer of texture load.
+  };
 
   /**
    * Struct to hold information about a requested Async load.
@@ -483,16 +498,35 @@ private:
   typedef std::vector<TextureInfo>      TextureInfoContainerType;       ///< The container type used to manage the life-cycle and caching of Textures
 
   /**
+   * @brief Initiate a load or queue load if NotifyObservers is invoking callbacks
+   * @param[in] textureInfo The TextureInfo struct associated with the Texture
+   * @param[in] observer The observer wishing to observe the texture upload
+   */
+  void LoadOrQueueTexture( TextureInfo& textureInfo, TextureUploadObserver* observer );
+
+  /**
+   * @brief Queue a texture load to be subsequently handled by ProcessQueuedTextures.
+   * @param[in] textureInfo The TextureInfo struct associated with the Texture
+   * @param[in] observer The observer wishing to observe the texture upload
+   */
+  void QueueLoadTexture( TextureInfo& textureInfo, TextureUploadObserver* observer );
+
+  /**
    * @brief Used internally to initiate a load.
    * @param[in] textureInfo The TextureInfo struct associated with the Texture
-   * @return                True if the load was initiated
+   * @param[in] observer The observer wishing to observe the texture upload
+   */
+  void LoadTexture( TextureInfo& textureInfo, TextureUploadObserver* observer );
+
+  /**
+   * @brief Initiate load of textures queued whilst NotifyObservers invoking callbacks.
    */
-  bool LoadTexture( TextureInfo& textureInfo );
+  void ProcessQueuedTextures();
 
   /**
    * Add the observer to the observer list
    * @param[in] textureInfo The TextureInfo struct associated with the texture
-   * observer The observer wishing to observe the texture upload
+   * @param[in] observer The observer wishing to observe the texture upload
    */
   void ObserveTexture( TextureInfo & textureInfo, TextureUploadObserver* observer );
 
@@ -710,7 +744,9 @@ 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<LoadQueueElement>                mLoadQueue;            ///< Queue of textures to load after NotifyObservers
   TextureId                                     mCurrentTextureId;     ///< The current value used for the unique Texture Id generation
+  bool                                          mQueueLoadFlag;        ///< Flag that causes Load Textures to be queued.
 };