Destroy removal visuals after idler

There was some bug when visual destructor called during
it's emit ResourceReady.

To avoid this case, Let we keep visuals more long term,
and Discard + Destroy after some idler called.

To avoid multiple Idler callback register,
let we make that idler callback in VisualFactory.

Change-Id: Id47083b158f91bb81666d6f2100811dedb0d70f1
Signed-off-by: Eunki, Hong <eunkiki.hong@samsung.com>

diff --git a/automated-tests/src/dali-toolkit/dali-toolkit-test-utils/toolkit-adaptor.cpp b/automated-tests/src/dali-toolkit/dali-toolkit-test-utils/toolkit-adaptor.cpp
index 7c887f8..c675fef 100644 (file)
--- a/automated-tests/src/dali-toolkit/dali-toolkit-test-utils/toolkit-adaptor.cpp
+++ b/automated-tests/src/dali-toolkit/dali-toolkit-test-utils/toolkit-adaptor.cpp
@@ -92,8 +92,8 @@ bool Adaptor::AddIdle(CallbackBase* callback, bool hasReturnValue)
 
 void Adaptor::RemoveIdle(CallbackBase* callback)
 {
-  mCallbacks.Erase(std::find_if(mCallbacks.Begin(), mCallbacks.End(), [&callback](CallbackBase* current) { return callback == current; }));
-  mReturnCallbacks.Erase(std::find_if(mReturnCallbacks.Begin(), mReturnCallbacks.End(), [&callback](CallbackBase* current) { return callback == current; }));
+  mCallbacks.Erase(std::remove_if(mCallbacks.Begin(), mCallbacks.End(), [&callback](CallbackBase* current) { return callback == current; }), mCallbacks.End());
+  mReturnCallbacks.Erase(std::remove_if(mReturnCallbacks.Begin(), mReturnCallbacks.End(), [&callback](CallbackBase* current) { return callback == current; }), mReturnCallbacks.End());
 }
 
 void Adaptor::RunIdles()

diff --git a/automated-tests/src/dali-toolkit/utc-Dali-Slider.cpp b/automated-tests/src/dali-toolkit/utc-Dali-Slider.cpp
index 22b4d2f..1e86e0d 100644 (file)
--- a/automated-tests/src/dali-toolkit/utc-Dali-Slider.cpp
+++ b/automated-tests/src/dali-toolkit/utc-Dali-Slider.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022 Samsung Electronics Co., Ltd.
+ * Copyright (c) 2023 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.
@@ -346,10 +346,10 @@ int UtcDaliSliderSignals2(void)
   END_TEST;
 }
 
-int UtcDaliSetPropertyP(void)
+int UtcDaliSliderSetPropertyP(void)
 {
   ToolkitTestApplication application;
-  tet_infoline("UtcDaliSetPropertyP");
+  tet_infoline("UtcDaliSliderSetPropertyP");
 
   Slider slider = Slider::New();
   slider.SetProperty(Actor::Property::PARENT_ORIGIN, ParentOrigin::TOP_LEFT);

diff --git a/dali-toolkit/devel-api/visual-factory/visual-factory.cpp b/dali-toolkit/devel-api/visual-factory/visual-factory.cpp
index 6575c7d..93dcb4b 100644 (file)
--- a/dali-toolkit/devel-api/visual-factory/visual-factory.cpp
+++ b/dali-toolkit/devel-api/visual-factory/visual-factory.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2020 Samsung Electronics Co., Ltd.
+ * Copyright (c) 2023 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.
@@ -111,6 +111,11 @@ bool VisualFactory::GetPreMultiplyOnLoad() const
   return GetImplementation(*this).GetPreMultiplyOnLoad();
 }
 
+void VisualFactory::DiscardVisual(Visual::Base visual)
+{
+  GetImplementation(*this).DiscardVisual(visual);
+}
+
 } // namespace Toolkit
 
 } // namespace Dali

diff --git a/dali-toolkit/devel-api/visual-factory/visual-factory.h b/dali-toolkit/devel-api/visual-factory/visual-factory.h
index 1d6be15..1b7d9b5 100644 (file)
--- a/dali-toolkit/devel-api/visual-factory/visual-factory.h
+++ b/dali-toolkit/devel-api/visual-factory/visual-factory.h
@@ -2,7 +2,7 @@
 #define DALI_TOOLKIT_VISUAL_FACTORY_H
 
 /*
- * Copyright (c) 2020 Samsung Electronics Co., Ltd.
+ * Copyright (c) 2023 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.
@@ -126,6 +126,13 @@ public:
    */
   bool GetPreMultiplyOnLoad() const;
 
+  /**
+   * @brief Discard visual base. It will keep reference of visual until idle callback called.
+   *
+   * @param[in] visual Discarded visual base.
+   */
+  void DiscardVisual(Visual::Base visual);
+
 private:
   explicit DALI_INTERNAL VisualFactory(Internal::VisualFactory* impl);
 };

diff --git a/dali-toolkit/internal/controls/control/control-data-impl.cpp b/dali-toolkit/internal/controls/control/control-data-impl.cpp
index 427a394..548a4c9 100644 (file)
--- a/dali-toolkit/internal/controls/control/control-data-impl.cpp
+++ b/dali-toolkit/internal/controls/control/control-data-impl.cpp
@@ -233,6 +233,23 @@ void MoveVisual(RegisteredVisualContainer::Iterator sourceIter, RegisteredVisual
 }
 
 /**
+ * Discard visual from source to visual factory.
+ */
+void DiscardVisual(RegisteredVisualContainer::Iterator sourceIter, RegisteredVisualContainer& source)
+{
+  Toolkit::Visual::Base visual = (*sourceIter)->visual;
+  if(visual)
+  {
+    if(Stage::IsInstalled())
+    {
+      Toolkit::VisualFactory::Get().DiscardVisual(visual);
+    }
+  }
+
+  source.Erase(sourceIter);
+}
+
+/**
  * Performs actions as requested using the action name.
  * @param[in] object The object on which to perform the action.
  * @param[in] actionName The action to perform.
@@ -873,8 +890,9 @@ void Control::Impl::UnregisterVisual(Property::Index index)
     Actor self(mControlImpl.Self());
     Toolkit::GetImplementation((*iter)->visual).SetOffScene(self);
     (*iter)->pending = false;
-    (*iter)->visual.Reset();
-    mRemoveVisuals.Erase(iter);
+
+    // Discard removed visual. It will be destroyed at next Idle time.
+    DiscardVisual(iter, mRemoveVisuals);
   }
 }
 
@@ -1005,7 +1023,9 @@ void Control::Impl::ResourceReady(Visual::Base& object)
     {
       Toolkit::GetImplementation((*visualToRemoveIter)->visual).SetOffScene(self);
     }
-    mRemoveVisuals.Erase(visualToRemoveIter);
+
+    // Discard removed visual. It will be destroyed at next Idle time.
+    DiscardVisual(visualToRemoveIter, mRemoveVisuals);
   }
 
   // A visual is ready so control may need relayouting if staged
@@ -2001,17 +2021,25 @@ void Control::Impl::OnSceneDisconnection()
 
   // Visuals pending replacement can now be taken out of the removal list and set off scene
   // Iterate through all replacement visuals and add to a move queue then set off scene
-  for(auto removalIter = mRemoveVisuals.Begin(), end = mRemoveVisuals.End(); removalIter != end; removalIter++)
+
+  if(!mRemoveVisuals.Empty())
   {
-    Toolkit::GetImplementation((*removalIter)->visual).SetOffScene(self);
+    std::reverse(mRemoveVisuals.Begin(), mRemoveVisuals.End());
+
+    while(!mRemoveVisuals.Empty())
+    {
+      auto removalIter = mRemoveVisuals.End() - 1u;
+      Toolkit::GetImplementation((*removalIter)->visual).SetOffScene(self);
+
+      // Discard removed visual. It will be destroyed at next Idle time.
+      DiscardVisual(removalIter, mRemoveVisuals);
+    }
   }
 
   for(auto replacedIter = mVisuals.Begin(), end = mVisuals.End(); replacedIter != end; replacedIter++)
   {
     (*replacedIter)->pending = false;
   }
-
-  mRemoveVisuals.Clear();
 }
 
 void Control::Impl::SetMargin(Extents margin)

diff --git a/dali-toolkit/internal/visuals/visual-factory-impl.cpp b/dali-toolkit/internal/visuals/visual-factory-impl.cpp
index c1cf764..7cc5119 100644 (file)
--- a/dali-toolkit/internal/visuals/visual-factory-impl.cpp
+++ b/dali-toolkit/internal/visuals/visual-factory-impl.cpp
@@ -19,6 +19,7 @@
 
 // EXTERNAL INCLUDES
 #include <dali/devel-api/scripting/scripting.h>
+#include <dali/integration-api/adaptor-framework/adaptor.h>
 #include <dali/integration-api/debug.h>
 #include <dali/public-api/object/property-array.h>
 #include <dali/public-api/object/type-registry-helper.h>
@@ -81,6 +82,7 @@ VisualFactory::VisualFactory(bool debugEnabled)
   mImageVisualShaderFactory(),
   mTextVisualShaderFactory(),
   mSlotDelegate(this),
+  mIdleCallback(nullptr),
   mDebugEnabled(debugEnabled),
   mPreMultiplyOnLoad(true)
 {
@@ -88,6 +90,12 @@ VisualFactory::VisualFactory(bool debugEnabled)
 
 VisualFactory::~VisualFactory()
 {
+  if(mIdleCallback && Adaptor::IsAvailable())
+  {
+    // Removes the callback from the callback manager in case the control is destroyed before the callback is executed.
+    Adaptor::Get().RemoveIdle(mIdleCallback);
+    mIdleCallback = nullptr;
+  }
 }
 
 void VisualFactory::OnStyleChangedSignal(Toolkit::StyleManager styleManager, StyleChange::Type type)
@@ -378,6 +386,13 @@ bool VisualFactory::GetPreMultiplyOnLoad() const
   return mPreMultiplyOnLoad;
 }
 
+void VisualFactory::DiscardVisual(Toolkit::Visual::Base visual)
+{
+  mDiscardedVisuals.emplace_back(visual);
+
+  RegisterDiscardCallback();
+}
+
 Internal::TextureManager& VisualFactory::GetTextureManager()
 {
   return GetFactoryCache().GetTextureManager();
@@ -437,6 +452,31 @@ TextVisualShaderFactory& VisualFactory::GetTextVisualShaderFactory()
   return *mTextVisualShaderFactory;
 }
 
+void VisualFactory::OnDiscardCallback()
+{
+  mIdleCallback = nullptr;
+
+  // Discard visual now.
+  mDiscardedVisuals.clear();
+}
+
+void VisualFactory::RegisterDiscardCallback()
+{
+  if(!mIdleCallback && Adaptor::IsAvailable())
+  {
+    // The callback manager takes the ownership of the callback object.
+    mIdleCallback = MakeCallback(this, &VisualFactory::OnDiscardCallback);
+
+    Adaptor& adaptor = Adaptor::Get();
+
+    if(!adaptor.AddIdle(mIdleCallback, false))
+    {
+      // Fail to add idle. (Maybe adaptor is paused.)
+      mIdleCallback = nullptr;
+    }
+  }
+}
+
 } // namespace Internal
 
 } // namespace Toolkit

diff --git a/dali-toolkit/internal/visuals/visual-factory-impl.h b/dali-toolkit/internal/visuals/visual-factory-impl.h
index 1e5ac93..c0b95b4 100644 (file)
--- a/dali-toolkit/internal/visuals/visual-factory-impl.h
+++ b/dali-toolkit/internal/visuals/visual-factory-impl.h
@@ -18,6 +18,7 @@
  */
 
 // EXTERNAL INCLUDES
+#include <dali/public-api/common/vector-wrapper.h>
 #include <dali/public-api/object/base-object.h>
 
 // INTERNAL INCLUDES
@@ -85,6 +86,11 @@ public:
   bool GetPreMultiplyOnLoad() const;
 
   /**
+   * @copydoc Toolkit::VisualFactory::DiscardVisual()
+   */
+  void DiscardVisual(Toolkit::Visual::Base visual);
+
+  /**
    * @return the reference to texture manager
    */
   Internal::TextureManager& GetTextureManager();
@@ -117,6 +123,16 @@ private:
    */
   TextVisualShaderFactory& GetTextVisualShaderFactory();
 
+  /**
+   * @brief Callbacks called for clear discarded visuals.
+   */
+  void OnDiscardCallback();
+
+  /**
+   * @brief Register idle callback for discard visuals if need.
+   */
+  void RegisterDiscardCallback();
+
   VisualFactory(const VisualFactory&) = delete;
 
   VisualFactory& operator=(const VisualFactory& rhs) = delete;
@@ -126,8 +142,13 @@ private:
   std::unique_ptr<ImageVisualShaderFactory> mImageVisualShaderFactory;
   std::unique_ptr<TextVisualShaderFactory>  mTextVisualShaderFactory;
   SlotDelegate<VisualFactory>               mSlotDelegate;
-  bool                                      mDebugEnabled : 1;
-  bool                                      mPreMultiplyOnLoad : 1; ///< Local store for this flag
+  CallbackBase*                             mIdleCallback;
+
+  using DiscardedVisualContainer = std::vector<Toolkit::Visual::Base>;
+  DiscardedVisualContainer mDiscardedVisuals{};
+
+  bool mDebugEnabled : 1;
+  bool mPreMultiplyOnLoad : 1; ///< Local store for this flag
 };
 
 /**