Avoid self-destruct during AnimatedImageVisual resource ready 43/313843/5
authorEunki, Hong <eunkiki.hong@samsung.com>
Tue, 2 Jul 2024 10:59:07 +0000 (19:59 +0900)
committerEunki Hong <eunkiki.hong@samsung.com>
Fri, 5 Jul 2024 02:16:53 +0000 (02:16 +0000)
Since AnimatedImageVisual's resource ready is not safe enough
for self-destruction case, we'd better use DiscardVisual
for every visual cases.

Change-Id: If316663699276696bfd0695b85ac53f1cbd2b8f2
Signed-off-by: Eunki, Hong <eunkiki.hong@samsung.com>
automated-tests/src/dali-toolkit/utc-Dali-ImageView.cpp
dali-toolkit/internal/controls/control/control-data-impl.cpp
dali-toolkit/internal/controls/image-view/image-view-impl.cpp
dali-toolkit/internal/controls/text-controls/text-label-impl.cpp

index ea8a7ca07ab0dd283a9717ffdd0b0eec6728394e..ef92cad0cd0f4529a62d3fd8cfe0e8125f74894b 100644 (file)
@@ -3993,6 +3993,22 @@ void OnResourceReadySignal11(Control control)
   }
 }
 
+void OnResourceReadySignal12(Control control)
+{
+  gResourceReadySignalCounter++;
+
+  if(gImageView1)
+  {
+    gImageView1.Unparent();
+    gImageView1.Reset(); // Destroy visual
+  }
+  if(gImageView2)
+  {
+    gImageView2.Unparent();
+    gImageView2.Reset(); // Destroy visual
+  }
+}
+
 } // namespace
 
 int UtcDaliImageViewSetImageOnResourceReadySignal01(void)
@@ -4999,6 +5015,87 @@ int UtcDaliImageViewSetImageOnResourceReadySignal11(void)
   END_TEST;
 }
 
+int UtcDaliImageViewSetImageOnResourceReadySignal12(void)
+{
+  tet_infoline("Test ResourceReady Add AnimatedImageVisual with invalid url, and then Remove immediately.");
+
+  ToolkitTestApplication application;
+
+  gResourceReadySignalCounter = 0;
+
+  // Clear image view for clear test
+
+  if(gImageView1)
+  {
+    gImageView1.Reset();
+  }
+  if(gImageView2)
+  {
+    gImageView2.Reset();
+  }
+  if(gImageView3)
+  {
+    gImageView3.Reset();
+  }
+
+  try
+  {
+    gImageView1 = ImageView::New();
+    gImageView1.SetProperty(Toolkit::ImageView::Property::IMAGE, "invalid.gif");
+    gImageView1.ResourceReadySignal().Connect(&OnResourceReadySignal12);
+    application.GetScene().Add(gImageView1); // It will call resourceReady signal 1 time.
+
+    gImageView2 = ImageView::New();
+    gImageView2.SetProperty(Toolkit::ImageView::Property::IMAGE, "invalid.gif");
+    gImageView2.ResourceReadySignal().Connect(&OnResourceReadySignal12);
+    application.GetScene().Add(gImageView2); // It will call resourceReady signal 1 time.
+
+    tet_printf("ResourceReady called %d times\n", gResourceReadySignalCounter);
+
+    DALI_TEST_EQUALS(gResourceReadySignalCounter, 0, TEST_LOCATION);
+
+    application.SendNotification();
+    application.Render();
+
+    // Load gImageView1
+    DALI_TEST_EQUALS(Test::WaitForEventThreadTrigger(1), true, TEST_LOCATION);
+
+    tet_printf("ResourceReady called %d times\n", gResourceReadySignalCounter);
+
+    DALI_TEST_EQUALS(gResourceReadySignalCounter, 1, TEST_LOCATION);
+
+    DALI_TEST_CHECK(true);
+  }
+  catch(...)
+  {
+    // Exception should not happened
+    DALI_TEST_CHECK(false);
+  }
+
+  // Clear cache.
+  application.SendNotification();
+  application.Render();
+
+  gResourceReadySignalCounter = 0;
+
+  // Clear image view for clear test
+
+  if(gImageView1)
+  {
+    gImageView1.Reset();
+  }
+  if(gImageView2)
+  {
+    gImageView2.Reset();
+  }
+  if(gImageView3)
+  {
+    gImageView3.Reset();
+  }
+
+  END_TEST;
+}
+
 int UtcDaliImageViewUseSameUrlWithAnimatedImageVisual(void)
 {
   tet_infoline("Test multiple views with same image in animated image visual");
@@ -5042,6 +5139,9 @@ int UtcDaliImageViewNpatchImageCacheTest01(void)
     // Ensure remove npatch cache if required.
     application.SendNotification();
     application.Render();
+    application.RunIdles();
+    application.SendNotification();
+    application.Render();
 
     imageView[index] = ImageView::New(nPatchImageUrl);
     imageView[index].SetProperty(Actor::Property::SIZE, Vector2(100.0f, 200.0f));
@@ -5078,6 +5178,9 @@ int UtcDaliImageViewNpatchImageCacheTest01(void)
   imageView[1].Unparent();
   imageView[1].Reset();
 
+  application.SendNotification();
+  application.Render();
+  application.RunIdles();
   application.SendNotification();
   application.Render();
 
@@ -5104,6 +5207,9 @@ int UtcDaliImageViewNpatchImageCacheTest01(void)
   imageView[1].Unparent();
   imageView[1].Reset();
 
+  application.SendNotification();
+  application.Render();
+  application.RunIdles();
   application.SendNotification();
   application.Render();
 
@@ -5586,11 +5692,11 @@ int UtcDaliImageViewTransitionEffect05(void)
   ToolkitTestApplication application;
 
   Property::Map map;
-  map["target"]      = "image";
-  map["property"]    = "opacity";
+  map["target"]       = "image";
+  map["property"]     = "opacity";
   map["initialValue"] = 0.2f;
-  map["targetValue"] = 1.0f;
-  map["animator"]    = Property::Map()
+  map["targetValue"]  = 1.0f;
+  map["animator"]     = Property::Map()
                       .Add("alphaFunction", "EASE_IN_OUT")
                       .Add("timePeriod", Property::Map().Add("delay", 0.0f).Add("duration", 2.0f))
                       .Add("animationType", "BETWEEN");
@@ -5625,11 +5731,11 @@ int UtcDaliImageViewTransitionEffect06(void)
   ToolkitTestApplication application;
 
   Property::Map map;
-  map["target"]      = "image";
-  map["property"]    = "opacity";
+  map["target"]       = "image";
+  map["property"]     = "opacity";
   map["initialValue"] = 0.2f;
-  map["targetValue"] = 1.0f;
-  map["animator"]    = Property::Map()
+  map["targetValue"]  = 1.0f;
+  map["animator"]     = Property::Map()
                       .Add("alphaFunction", "EASE_IN_OUT")
                       .Add("timePeriod", Property::Map().Add("delay", 0.0f).Add("duration", 2.0f))
                       .Add("animationType", "TO");
@@ -5658,8 +5764,6 @@ int UtcDaliImageViewTransitionEffect06(void)
   END_TEST;
 }
 
-
-
 int UtcDaliImageViewImageLoadFailureAndReload01(void)
 {
   tet_infoline("Try to load invalid image first, and then reload after that image valid.");
index efb0dee96cd49f79e791ef0df9ddc14c417634bb..a9e34206aa06f56a65efb38a9039845727a6908a 100644 (file)
@@ -582,14 +582,22 @@ Control::Impl::Impl(Control& controlImpl)
 
 Control::Impl::~Impl()
 {
-  for(auto&& iter : mVisuals)
+  while(!mVisuals.Empty())
   {
-    StopObservingVisual(iter->visual);
+    auto iter = mVisuals.End() - 1u;
+    StopObservingVisual((*iter)->visual);
+
+    // Discard removed visual. It will be destroyed at next Idle time.
+    DiscardVisual(iter, mVisuals);
   }
 
-  for(auto&& iter : mRemoveVisuals)
+  while(!mRemoveVisuals.Empty())
   {
-    StopObservingVisual(iter->visual);
+    auto removalIter = mRemoveVisuals.End() - 1u;
+    StopObservingVisual((*removalIter)->visual);
+
+    // Discard removed visual. It will be destroyed at next Idle time.
+    DiscardVisual(removalIter, mRemoveVisuals);
   }
 
   // All gesture detectors will be destroyed so no need to disconnect.
@@ -2232,35 +2240,38 @@ void Control::Impl::UpdateVisualProperties(const std::vector<std::pair<Dali::Pro
 
 void Control::Impl::EmitResourceReadySignal()
 {
-  if(!mIsEmittingResourceReadySignal)
+  if(DALI_LIKELY(Stage::IsInstalled())) ///< Avoid resource ready callback during shutting down
   {
-    // Guard against calls to emit the signal during the callback
-    mIsEmittingResourceReadySignal = true;
+    if(!mIsEmittingResourceReadySignal)
+    {
+      // Guard against calls to emit the signal during the callback
+      mIsEmittingResourceReadySignal = true;
 
-    // If the signal handler changes visual, it may become ready during this call & therefore this method will
-    // get called again recursively. If so, mIdleCallbackRegistered is set below, and we act on it after that secondary
-    // invocation has completed by notifying in an Idle callback to prevent further recursion.
-    Dali::Toolkit::Control handle(mControlImpl.GetOwner());
-    mResourceReadySignal.Emit(handle);
+      // If the signal handler changes visual, it may become ready during this call & therefore this method will
+      // get called again recursively. If so, mIdleCallbackRegistered is set below, and we act on it after that secondary
+      // invocation has completed by notifying in an Idle callback to prevent further recursion.
+      Dali::Toolkit::Control handle(mControlImpl.GetOwner());
+      mResourceReadySignal.Emit(handle);
 
-    mIsEmittingResourceReadySignal = false;
-  }
-  else
-  {
-    if(!mIdleCallbackRegistered)
+      mIsEmittingResourceReadySignal = false;
+    }
+    else
     {
-      mIdleCallbackRegistered = true;
-
-      // Add idler to emit the signal again
-      if(!mIdleCallback)
+      if(!mIdleCallbackRegistered)
       {
-        // The callback manager takes the ownership of the callback object.
-        mIdleCallback = MakeCallback(this, &Control::Impl::OnIdleCallback);
-        if(DALI_UNLIKELY(!Adaptor::Get().AddIdle(mIdleCallback, true)))
+        mIdleCallbackRegistered = true;
+
+        // Add idler to emit the signal again
+        if(!mIdleCallback)
         {
-          DALI_LOG_ERROR("Fail to add idle callback for control resource ready. Skip this callback.\n");
-          mIdleCallback           = nullptr;
-          mIdleCallbackRegistered = false;
+          // The callback manager takes the ownership of the callback object.
+          mIdleCallback = MakeCallback(this, &Control::Impl::OnIdleCallback);
+          if(DALI_UNLIKELY(!Adaptor::Get().AddIdle(mIdleCallback, true)))
+          {
+            DALI_LOG_ERROR("Fail to add idle callback for control resource ready. Skip this callback.\n");
+            mIdleCallback           = nullptr;
+            mIdleCallbackRegistered = false;
+          }
         }
       }
     }
index 91e33df1ed6e43da01f8c2bdd0500aaa9039b124..ef9ff62e7875388307513c14990d8a5f915dc721 100644 (file)
@@ -19,6 +19,7 @@
 #include "image-view-impl.h"
 
 // EXTERNAL INCLUDES
+#include <dali/devel-api/common/stage.h>
 #include <dali/devel-api/scripting/scripting.h>
 #include <dali/public-api/math/math-utils.h>
 #include <dali/public-api/object/type-registry-helper.h>
@@ -44,8 +45,8 @@ namespace Internal
 {
 namespace
 {
-constexpr float FULL_OPACITY = 1.0f;
-constexpr float LOW_OPACITY  = 0.2f;
+constexpr float FULL_OPACITY            = 1.0f;
+constexpr float LOW_OPACITY             = 0.2f;
 constexpr float TRANSITION_EFFECT_SPEED = 0.3f;
 
 constexpr int PLACEHOLDER_DEPTH_INDEX     = -2;
@@ -67,6 +68,20 @@ DALI_PROPERTY_REGISTRATION(Toolkit, ImageView, "transitionEffectOption", MAP, TR
 DALI_ANIMATABLE_PROPERTY_REGISTRATION_WITH_DEFAULT(Toolkit, ImageView, "pixelArea", Vector4(0.f, 0.f, 1.f, 1.f), PIXEL_AREA)
 DALI_TYPE_REGISTRATION_END()
 
+/**
+ * @brief Discard the given visual into VisualFactory. The visual will be destroyed at next idle time.
+ *
+ * @param[in,out] visual Visual to be discarded. It will be reset to an empty handle.
+ */
+void DiscardImageViewVisual(Dali::Toolkit::Visual::Base& visual)
+{
+  if(DALI_LIKELY(Dali::Stage::IsInstalled() && visual))
+  {
+    Dali::Toolkit::VisualFactory::Get().DiscardVisual(visual);
+  }
+  visual.Reset();
+}
+
 } // anonymous namespace
 
 using namespace Dali;
@@ -82,6 +97,9 @@ ImageView::ImageView(ControlBehaviour additionalBehaviour)
 
 ImageView::~ImageView()
 {
+  DiscardImageViewVisual(mVisual);
+  DiscardImageViewVisual(mPreviousVisual);
+  DiscardImageViewVisual(mPlaceholderVisual);
 }
 
 Toolkit::ImageView ImageView::New(ControlBehaviour additionalBehaviour)
@@ -130,6 +148,8 @@ void ImageView::SetImage(const Property::Map& map)
     // This previous visual will be deleted when transition effect is done.
     Internal::Control::Impl& controlDataImpl = Internal::Control::Impl::Get(*this);
     controlDataImpl.EnableReadyTransitionOverriden(mVisual, true);
+
+    DiscardImageViewVisual(mPreviousVisual);
     mPreviousVisual = mVisual;
   }
 
@@ -209,6 +229,8 @@ void ImageView::SetImage(const std::string& url, ImageDimensions size)
     // This previous visual will be deleted when transition effect is done.
     Internal::Control::Impl& controlDataImpl = Internal::Control::Impl::Get(*this);
     controlDataImpl.EnableReadyTransitionOverriden(mVisual, true);
+
+    DiscardImageViewVisual(mPreviousVisual);
     mPreviousVisual = mVisual;
   }
 
@@ -234,6 +256,7 @@ void ImageView::SetImage(const std::string& url, ImageDimensions size)
       visualImpl.SetFittingMode(Visual::FittingMode::FILL);
     }
 
+    // Don't set mVisual until it is ready and shown. Getters will still use current visual.
     if(!mVisual)
     {
       mVisual = visual;
@@ -262,7 +285,8 @@ void ImageView::ClearImageVisual()
   // Clear cached properties
   mPropertyMap.Clear();
   mUrl.clear();
-  mVisual.Reset();
+
+  DiscardImageViewVisual(mVisual);
 
   // Unregister the exsiting visual
   DevelControl::UnregisterVisual(*this, Toolkit::ImageView::Property::IMAGE);
@@ -301,7 +325,7 @@ void ImageView::SetPlaceholderUrl(const std::string& url)
   mPlaceholderUrl = url;
   if(!url.empty())
   {
-    mPlaceholderVisual.Reset();
+    DiscardImageViewVisual(mPlaceholderVisual);
     CreatePlaceholderImage();
   }
   else
@@ -313,7 +337,7 @@ void ImageView::SetPlaceholderUrl(const std::string& url)
       DevelControl::UnregisterVisual(*this, Toolkit::ImageView::Property::PLACEHOLDER_IMAGE);
     }
 
-    mPlaceholderVisual.Reset();
+    DiscardImageViewVisual(mPlaceholderVisual);
     mPlaceholderUrl = url;
   }
 }
@@ -464,7 +488,13 @@ void ImageView::OnResourceReady(Toolkit::Control control)
   }
 
   // Visual ready so update visual attached to this ImageView, following call to RelayoutRequest will use this visual.
-  mVisual = DevelControl::GetVisual(*this, Toolkit::ImageView::Property::IMAGE);
+  auto currentVisual = DevelControl::GetVisual(*this, Toolkit::ImageView::Property::IMAGE);
+  if(mVisual != currentVisual)
+  {
+    // If the current visual is not the same as the previous holded visual, then we need to discard old one.
+    DiscardImageViewVisual(mVisual);
+  }
+  mVisual = currentVisual;
 
   // Signal that a Relayout may be needed
 }
@@ -486,7 +516,7 @@ void ImageView::CreatePlaceholderImage()
   else
   {
     DevelControl::UnregisterVisual(*this, Toolkit::ImageView::Property::PLACEHOLDER_IMAGE);
-    mPlaceholderVisual.Reset();
+    DiscardImageViewVisual(mPlaceholderVisual);
   }
 }
 
@@ -543,9 +573,9 @@ void ImageView::TransitionImageWithEffect()
       if(!mTransitionEffectOptionMap.Empty())
       {
         // Set user's transition effect options
-        Dali::Toolkit::TransitionData transition = Toolkit::TransitionData::New(mTransitionEffectOptionMap);
-        Internal::Control::Impl& controlDataImpl = Internal::Control::Impl::Get(*this);
-        mTransitionAnimation  = controlDataImpl.CreateTransition(transition);
+        Dali::Toolkit::TransitionData transition      = Toolkit::TransitionData::New(mTransitionEffectOptionMap);
+        Internal::Control::Impl&      controlDataImpl = Internal::Control::Impl::Get(*this);
+        mTransitionAnimation                          = controlDataImpl.CreateTransition(transition);
         if(mTransitionAnimation)
         {
           mTransitionAnimation.SetEndAction(Animation::EndAction::DISCARD);
@@ -570,7 +600,7 @@ void ImageView::TransitionImageWithEffect()
           Dali::KeyFrames fadeinKeyFrames = Dali::KeyFrames::New();
           fadeinKeyFrames.Add(0.0f, LOW_OPACITY);
           fadeinKeyFrames.Add(1.0f, destinationAlpha);
-          mTransitionAnimation.AnimateBetween(DevelControl::GetVisualProperty(handle, Toolkit::ImageView::Property::IMAGE, Toolkit::Visual::Property::OPACITY), fadeinKeyFrames,  AlphaFunction::EASE_IN_OUT);
+          mTransitionAnimation.AnimateBetween(DevelControl::GetVisualProperty(handle, Toolkit::ImageView::Property::IMAGE, Toolkit::Visual::Property::OPACITY), fadeinKeyFrames, AlphaFunction::EASE_IN_OUT);
           imageVisual.SetDepthIndex(imageVisual.GetDepthIndex() + CURRENT_VISUAL_DEPTH_INDEX);
         }
 
@@ -591,7 +621,7 @@ void ImageView::ClearTransitionAnimation()
     Internal::Control::Impl& controlDataImpl = Internal::Control::Impl::Get(*this);
     controlDataImpl.EnableReadyTransitionOverriden(mVisual, false);
     Toolkit::GetImplementation(mPreviousVisual).SetOffScene(self);
-    mPreviousVisual.Reset();
+    DiscardImageViewVisual(mPreviousVisual);
   }
 
   if(mTransitionAnimation)
index b09994f7713c21b2aaeba50e739262000d8f623d..479239119699d1e345ce9ed13960427a68224e15 100644 (file)
@@ -232,6 +232,20 @@ void ParseTextFitProperty(Text::ControllerPtr& controller, const Property::Map*
   }
 }
 
+/**
+ * @brief Discard the given visual into VisualFactory. The visual will be destroyed at next idle time.
+ *
+ * @param[in,out] visual Visual to be discarded. It will be reset to an empty handle.
+ */
+void DiscardTextLabelVisual(Dali::Toolkit::Visual::Base& visual)
+{
+  if(DALI_LIKELY(Dali::Stage::IsInstalled() && visual))
+  {
+    Dali::Toolkit::VisualFactory::Get().DiscardVisual(visual);
+  }
+  visual.Reset();
+}
+
 } // namespace
 
 Toolkit::TextLabel TextLabel::New(ControlBehaviour additionalBehaviour)
@@ -1397,6 +1411,7 @@ TextLabel::TextLabel(ControlBehaviour additionalBehaviour)
 
 TextLabel::~TextLabel()
 {
+  DiscardTextLabelVisual(mVisual);
 }
 
 Vector<Vector2> TextLabel::GetTextSize(const uint32_t startIndex, const uint32_t endIndex) const