Decrease Render::Renderer index map if detached

Render::Renderer keep all nodes uniform index map per each nodes.
Since there was no signal to remove that renderer detached from node,
that uniform index map might increase long times.

To avoid that kind of memory leak, let we send messages
whenever renderer is detached from node.

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

diff --git a/automated-tests/src/dali/utc-Dali-Actor.cpp b/automated-tests/src/dali/utc-Dali-Actor.cpp
index a2680de..19e6de3 100644 (file)
--- a/automated-tests/src/dali/utc-Dali-Actor.cpp
+++ b/automated-tests/src/dali/utc-Dali-Actor.cpp
@@ -4494,6 +4494,76 @@ int UtcDaliActorRemoveRendererP2(void)
   END_TEST;
 }
 
+int UtcDaliActorRemoveRendererP3(void)
+{
+  tet_infoline("Testing Actor::RemoveRenderer");
+  TestApplication application;
+
+  Actor actor1 = Actor::New();
+  Actor actor2 = Actor::New();
+  Actor actor3 = Actor::New();
+
+  application.GetScene().Add(actor1);
+  application.GetScene().Add(actor2);
+  application.GetScene().Add(actor3);
+
+  // Make each actors size bigger than zero, so we can assuem that actor is rendered
+  actor1.SetProperty(Actor::Property::SIZE, Vector2(10.0f, 10.0f));
+  actor2.SetProperty(Actor::Property::SIZE, Vector2(10.0f, 10.0f));
+  actor3.SetProperty(Actor::Property::SIZE, Vector2(10.0f, 10.0f));
+
+  // Register some dummy property to seperate actor1 and actor2 in Render::Renderer
+  actor1.RegisterProperty("dummy1", 1);
+  actor2.RegisterProperty("dummy2", 2.0f);
+  actor3.RegisterProperty("dummy3", Vector2(3.0f, 4.0f));
+
+  DALI_TEST_EQUALS(actor1.GetRendererCount(), 0u, TEST_LOCATION);
+  DALI_TEST_EQUALS(actor2.GetRendererCount(), 0u, TEST_LOCATION);
+  DALI_TEST_EQUALS(actor3.GetRendererCount(), 0u, TEST_LOCATION);
+
+  Geometry geometry = CreateQuadGeometry();
+  Shader   shader   = CreateShader();
+  Renderer renderer1 = Renderer::New(geometry, shader);
+  Renderer renderer2 = Renderer::New(geometry, shader);
+
+  actor1.AddRenderer(renderer1);
+  actor1.AddRenderer(renderer2);
+  actor2.AddRenderer(renderer1);
+  actor2.AddRenderer(renderer2);
+  actor3.AddRenderer(renderer1);
+  actor3.AddRenderer(renderer2);
+  application.SendNotification();
+  application.Render();
+
+  DALI_TEST_EQUALS(actor1.GetRendererCount(), 2u, TEST_LOCATION);
+  DALI_TEST_EQUALS(actor1.GetRendererAt(0), renderer1, TEST_LOCATION);
+  DALI_TEST_EQUALS(actor1.GetRendererAt(1), renderer2, TEST_LOCATION);
+
+  DALI_TEST_EQUALS(actor2.GetRendererCount(), 2u, TEST_LOCATION);
+  DALI_TEST_EQUALS(actor2.GetRendererAt(0), renderer1, TEST_LOCATION);
+  DALI_TEST_EQUALS(actor2.GetRendererAt(1), renderer2, TEST_LOCATION);
+
+  DALI_TEST_EQUALS(actor3.GetRendererCount(), 2u, TEST_LOCATION);
+  DALI_TEST_EQUALS(actor3.GetRendererAt(0), renderer1, TEST_LOCATION);
+  DALI_TEST_EQUALS(actor3.GetRendererAt(1), renderer2, TEST_LOCATION);
+
+  actor1.RemoveRenderer(0);
+  actor2.RemoveRenderer(1);
+  actor3.RemoveRenderer(0);
+  application.SendNotification();
+  application.Render();
+
+  DALI_TEST_EQUALS(actor1.GetRendererCount(), 1u, TEST_LOCATION);
+  DALI_TEST_EQUALS(actor1.GetRendererAt(0), renderer2, TEST_LOCATION);
+  DALI_TEST_EQUALS(actor2.GetRendererCount(), 1u, TEST_LOCATION);
+  DALI_TEST_EQUALS(actor2.GetRendererAt(0), renderer1, TEST_LOCATION);
+  DALI_TEST_EQUALS(actor3.GetRendererCount(), 1u, TEST_LOCATION);
+  DALI_TEST_EQUALS(actor3.GetRendererAt(0), renderer2, TEST_LOCATION);
+
+  // Shut down whilst holding onto the renderer handle.
+  END_TEST;
+}
+
 int UtcDaliActorRemoveRendererN(void)
 {
   tet_infoline("Testing Actor::RemoveRenderer");

diff --git a/dali/internal/event/actors/actor-impl.cpp b/dali/internal/event/actors/actor-impl.cpp
index 4893f2f..ee63c41 100644 (file)
--- a/dali/internal/event/actors/actor-impl.cpp
+++ b/dali/internal/event/actors/actor-impl.cpp
@@ -1125,11 +1125,16 @@ Actor::~Actor()
   // Remove mParent pointers from children even if we're destroying core,
   // to guard against GetParent() & Unparent() calls from CustomActor destructors.
   UnparentChildren();
-  delete mRenderers;
 
   // Guard to allow handle destruction after Core has been destroyed
   if(EventThreadServices::IsCoreRunning())
   {
+    if(mRenderers)
+    {
+      // Detach all renderers before delete container.
+      mRenderers->RemoveAll(GetNode());
+    }
+
     // Root layer will destroy its node in its own destructor
     if(!mIsRoot)
     {
@@ -1138,6 +1143,8 @@ Actor::~Actor()
       GetEventThreadServices().UnregisterObject(this);
     }
   }
+  // Cleanup renderer list
+  delete mRenderers;
 
   // Cleanup optional gesture data
   delete mGestureData;

diff --git a/dali/internal/event/actors/actor-renderer-container.cpp b/dali/internal/event/actors/actor-renderer-container.cpp
index 5e84416..e47c495 100644 (file)
--- a/dali/internal/event/actors/actor-renderer-container.cpp
+++ b/dali/internal/event/actors/actor-renderer-container.cpp
@@ -62,6 +62,15 @@ void RendererContainer::Remove(const SceneGraph::Node& node, uint32_t index)
   }
 }
 
+void RendererContainer::RemoveAll(const SceneGraph::Node& node)
+{
+  for(auto&& renderer : mRenderers)
+  {
+    DetachRendererMessage(mEventThreadServices, node, renderer->GetRendererSceneObject());
+  }
+  mRenderers.clear();
+}
+
 uint32_t RendererContainer::GetCount() const
 {
   return static_cast<uint32_t>(mRenderers.size());

diff --git a/dali/internal/event/actors/actor-renderer-container.h b/dali/internal/event/actors/actor-renderer-container.h
index 4bd61a7..3594a97 100644 (file)
--- a/dali/internal/event/actors/actor-renderer-container.h
+++ b/dali/internal/event/actors/actor-renderer-container.h
@@ -69,6 +69,12 @@ public:
   void Remove(const SceneGraph::Node& node, uint32_t index);
 
   /**
+   * Remove all renderers
+   * @param[in] node The node to remove these renderer's scene graph object from
+   */
+  void RemoveAll(const SceneGraph::Node& node);
+
+  /**
    * Return the number of renderers
    * @return the number of renderers
    */

diff --git a/dali/internal/render/renderers/render-renderer.cpp b/dali/internal/render/renderers/render-renderer.cpp
index 1ccfc43..70d0d03 100644 (file)
--- a/dali/internal/render/renderers/render-renderer.cpp
+++ b/dali/internal/render/renderers/render-renderer.cpp
@@ -958,6 +958,46 @@ Vector4 Renderer::GetVisualTransformedUpdateArea(BufferIndex bufferIndex, const
   return mRenderDataProvider->GetVisualTransformedUpdateArea(bufferIndex, originalUpdateArea);
 }
 
+void Renderer::DetachFromNodeDataProvider(const SceneGraph::NodeDataProvider& node)
+{
+  // All nodes without uniformMap will share same UniformIndexMap, contains only render data providers.
+  // It is special case so we don't need to remove index map in this case. Fast out.
+  if(node.GetNodeUniformMap().Count() == 0u)
+  {
+    return;
+  }
+
+  // Remove mNodeIndexMap and mUniformIndexMaps.
+  auto iter = std::find_if(mNodeIndexMap.begin(), mNodeIndexMap.end(), [&node](RenderItemLookup& element) { return element.node == &node; });
+
+  if(iter != mNodeIndexMap.end())
+  {
+    // Swap between end of mUniformIndexMaps and removed.
+    auto nodeIndex           = iter->index;
+    auto uniformIndexMapSize = mUniformIndexMaps.size();
+
+    // Remove node index map.
+    mNodeIndexMap.erase(iter);
+
+    if(nodeIndex + 1 != uniformIndexMapSize)
+    {
+      std::swap(mUniformIndexMaps[nodeIndex], mUniformIndexMaps[uniformIndexMapSize - 1u]);
+      // Change node index map.
+      for(auto&& renderItemLookup : mNodeIndexMap)
+      {
+        if(renderItemLookup.index == uniformIndexMapSize - 1u)
+        {
+          renderItemLookup.index = nodeIndex;
+          break;
+        }
+      }
+    }
+
+    // Remove uniform index maps.
+    mUniformIndexMaps.pop_back();
+  }
+}
+
 Graphics::Pipeline& Renderer::PrepareGraphicsPipeline(
   Program&                                             program,
   const Dali::Internal::SceneGraph::RenderInstruction& instruction,

diff --git a/dali/internal/render/renderers/render-renderer.h b/dali/internal/render/renderers/render-renderer.h
index 2c6b806..8a7b183 100644 (file)
--- a/dali/internal/render/renderers/render-renderer.h
+++ b/dali/internal/render/renderers/render-renderer.h
@@ -503,6 +503,12 @@ public:
    */
   Vector4 GetVisualTransformedUpdateArea(BufferIndex bufferIndex, const Vector4& originalUpdateArea) const noexcept;
 
+  /**
+   * Detach a Renderer from the node provider.
+   * @param[in] node The node data provider to be detached renderer.
+   */
+  void DetachFromNodeDataProvider(const SceneGraph::NodeDataProvider& node);
+
 private:
   struct UniformIndexMap;
 

diff --git a/dali/internal/update/nodes/node.cpp b/dali/internal/update/nodes/node.cpp
index e999b1b..65e6ab3 100644 (file)
--- a/dali/internal/update/nodes/node.cpp
+++ b/dali/internal/update/nodes/node.cpp
@@ -244,6 +244,8 @@ void Node::RemoveRenderer(const RendererKey& renderer)
   {
     if(mRenderers[i] == renderer)
     {
+      renderer->DetachFromNodeDataProvider(*this);
+
       SetUpdated(true);
       mRenderers.Erase(mRenderers.Begin() + i);
       return;

diff --git a/dali/internal/update/rendering/scene-graph-renderer.cpp b/dali/internal/update/rendering/scene-graph-renderer.cpp
index c9951c3..5d9ad5f 100644 (file)
--- a/dali/internal/update/rendering/scene-graph-renderer.cpp
+++ b/dali/internal/update/rendering/scene-graph-renderer.cpp
@@ -603,6 +603,20 @@ DevelRenderer::Rendering::Type Renderer::GetRenderingBehavior() const
   return mRenderingBehavior;
 }
 
+void Renderer::DetachFromNodeDataProvider(const NodeDataProvider& node)
+{
+  // TODO : Can we send this by Resend flag, or message?
+  // Currently, we call DetachFromNodeDataProvider function even if Renderer is destroyed case.
+  // We don't need to call that function if mRenderer is destroyed.
+  //
+  // But also, there is no way to validate node's lifecycle. So just detach synchronously.
+  if(mRenderer)
+  {
+    Render::Renderer* rendererPtr = mRenderer.Get();
+    rendererPtr->DetachFromNodeDataProvider(node);
+  }
+}
+
 // Called when SceneGraph::Renderer is added to update manager ( that happens when an "event-thread renderer" is created )
 void Renderer::ConnectToSceneGraph(SceneController& sceneController, BufferIndex bufferIndex)
 {

diff --git a/dali/internal/update/rendering/scene-graph-renderer.h b/dali/internal/update/rendering/scene-graph-renderer.h
index a1787aa..033dfc1 100644 (file)
--- a/dali/internal/update/rendering/scene-graph-renderer.h
+++ b/dali/internal/update/rendering/scene-graph-renderer.h
@@ -67,6 +67,7 @@ namespace Internal
 {
 namespace SceneGraph
 {
+class NodeDataProvider;
 using RendererContainer = Dali::Vector<RendererKey>;
 using RendererIter      = RendererContainer::Iterator;
 using RendererConstIter = RendererContainer::ConstIterator;
@@ -415,6 +416,12 @@ public:
   void DisconnectFromSceneGraph(SceneController& sceneController, BufferIndex bufferIndex);
 
   /**
+   * Detached from the scene graph object.
+   * @param[in] node node who detach this renderer.
+   */
+  void DetachFromNodeDataProvider(const NodeDataProvider& node);
+
+  /**
    * @copydoc RenderDataProvider::GetUniformMapDataProvider()
    */
   const UniformMapDataProvider& GetUniformMapDataProvider() const override