Observer StopperNode and ViewportGuide node at SG::RenderTask 96/316896/10
authorEunki, Hong <eunkiki.hong@samsung.com>
Fri, 30 Aug 2024 06:43:23 +0000 (15:43 +0900)
committerEunki Hong <eunkiki.hong@samsung.com>
Fri, 13 Sep 2024 02:21:55 +0000 (02:21 +0000)
Let we clear the SceneGraph::RenderTask node information what were they don't owned.

Without observing, we might try to access dead node.

The two node - mViewportGuideNode and mStopperNode was not observered, so
let we observe their lifetime, and remove nullptr if they terminated.

Change-Id: I8c55248c625542650e4b386ba538419b5b231546
Signed-off-by: Eunki, Hong <eunkiki.hong@samsung.com>
automated-tests/src/dali/utc-Dali-RenderTask.cpp
dali/internal/update/common/animatable-property.h
dali/internal/update/common/property-owner.cpp
dali/internal/update/common/property-owner.h
dali/internal/update/render-tasks/scene-graph-render-task.cpp

index dce5075ca2f185df9542049c126a359cb5ed3a6f..72b3893dc37df7fb2e03e91507ac9d09405a321d 100644 (file)
@@ -1286,6 +1286,114 @@ int UtcDaliRenderTaskRenderUntil05(void)
   END_TEST;
 }
 
+int UtcDaliRenderTaskRenderUntil06(void)
+{
+  TestApplication application;
+  tet_infoline("Testing RenderTask::RenderUntil(actor) Check that stopper actor scene off and destroyed");
+
+  // Get default rendertask
+  RenderTaskList taskList = application.GetScene().GetRenderTaskList();
+  RenderTask     task     = taskList.GetTask(0u);
+
+  Integration::Scene stage = application.GetScene();
+
+  auto CreateRenderableActorWithName = [](const char* name) -> Actor {
+    Actor actor = CreateRenderableActor();
+    actor.SetProperty(Actor::Property::SIZE, Vector2(10.0f, 10.0f));
+    actor.SetProperty(Actor::Property::ANCHOR_POINT, AnchorPoint::CENTER);
+    actor.SetProperty(Actor::Property::PARENT_ORIGIN, ParentOrigin::CENTER);
+    actor.SetProperty(Actor::Property::NAME, name);
+
+    return actor;
+  };
+
+  // Compose a tree
+  //
+  // stage (depth = 0)
+  // |- a0 (renderable)
+  // |- a1 (renderable)
+  //     |- target (stopper node)
+  // |- a2 (renderable)
+  Actor a0     = CreateRenderableActorWithName("a0");
+  Actor a1     = CreateRenderableActorWithName("a1");
+  Actor a2     = CreateRenderableActorWithName("a2");
+  Actor target = CreateRenderableActorWithName("target");
+
+  stage.Add(a0);
+  stage.Add(a1);
+  stage.Add(a2);
+  a1.Add(target);
+
+  // draw a0, a1 (2 items)
+  task.RenderUntil(target);
+
+  // Update & Render with the actor on-stage
+  TestGlAbstraction& gl        = application.GetGlAbstraction();
+  TraceCallStack&    drawTrace = gl.GetDrawTrace();
+  drawTrace.Enable(true);
+
+  // Update & Render
+  application.SendNotification();
+  application.Render();
+
+  // Check that rendering was cut
+  DALI_TEST_EQUALS(drawTrace.CountMethod("DrawElements"), 2, TEST_LOCATION);
+
+  drawTrace.Reset();
+
+  // Destroy target
+  //
+  // stage (depth = 0)
+  // |- a0 (renderable)
+  // |- a1 (renderable)
+  // |- a2 (renderable)
+  target.Unparent();
+  target.Reset();
+
+  DALI_TEST_EQUALS(task.GetStopperActor(), Actor(), TEST_LOCATION);
+
+  // After now, draw all actors, a0, a1, and a2.
+
+  // Update & Render
+  application.SendNotification();
+  application.Render();
+
+  // Check that rendering was cut
+  DALI_TEST_EQUALS(drawTrace.CountMethod("DrawElements"), 3, TEST_LOCATION);
+
+  drawTrace.Reset();
+
+  // To make ensure the destroyed target has same pointer with new one, try this test multiple times
+  for(int tryCount = 0; tryCount < 50; tryCount++)
+  {
+    // Create new target, which might have same pointer with previous one.
+    target = CreateRenderableActorWithName("target");
+
+    // Add target under a0.
+    //
+    // stage (depth = 0)
+    // |- a0 (renderable)
+    //     |- target (renderable)
+    // |- a1 (renderable)
+    // |- a2 (renderable)
+    a0.Add(target);
+
+    // After now, draw all actors (4 items).
+
+    // Update & Render
+    application.SendNotification();
+    application.Render();
+
+    // Check that rendering was cut
+    DALI_TEST_EQUALS(drawTrace.CountMethod("DrawElements"), 4, TEST_LOCATION);
+
+    drawTrace.Reset();
+    target.Unparent();
+  }
+
+  END_TEST;
+}
+
 int UtcDaliRenderTaskSetExclusive(void)
 {
   TestApplication application;
@@ -4609,6 +4717,120 @@ int UtcDaliRenderTaskViewportGuideActor04(void)
   END_TEST;
 }
 
+int UtcDaliRenderTaskViewportGuideActorDestroyed(void)
+{
+  TestApplication application;
+  tet_infoline("Testing RenderTask with ViewportGuideActor, and check viewport if guide actor destroyed");
+
+  Stage   stage = Stage::GetCurrent();
+  Vector2 stageSize(stage.GetSize());
+
+  Actor blue                                 = Actor::New();
+  blue[Dali::Actor::Property::NAME]          = "Blue";
+  blue[Dali::Actor::Property::ANCHOR_POINT]  = AnchorPoint::CENTER;
+  blue[Dali::Actor::Property::PARENT_ORIGIN] = ParentOrigin::CENTER;
+  blue[Dali::Actor::Property::SIZE]          = Vector2(300, 300);
+  blue[Dali::Actor::Property::POSITION]      = Vector2(0, 0);
+
+  Actor red                                 = Actor::New();
+  red[Dali::Actor::Property::NAME]          = "Red";
+  red[Dali::Actor::Property::ANCHOR_POINT]  = AnchorPoint::CENTER;
+  red[Dali::Actor::Property::PARENT_ORIGIN] = ParentOrigin::CENTER;
+  red[Dali::Actor::Property::SIZE]          = Vector2(300, 300);
+  red[Dali::Actor::Property::POSITION]      = Vector2(0, 0);
+
+  Geometry geometry = Geometry::New();
+  Shader   shader   = Shader::New("vertexSrc", "fragmentSrc");
+  Renderer renderer = Renderer::New(geometry, shader);
+  red.AddRenderer(renderer);
+
+  stage.Add(blue);
+  stage.Add(red);
+
+  RenderTaskList renderTaskList = stage.GetRenderTaskList();
+  RenderTask     renderTask     = renderTaskList.CreateTask();
+
+  Dali::CameraActor cameraActor                     = Dali::CameraActor::New(stageSize);
+  cameraActor[Dali::Actor::Property::ANCHOR_POINT]  = AnchorPoint::CENTER;
+  cameraActor[Dali::Actor::Property::PARENT_ORIGIN] = ParentOrigin::CENTER;
+  stage.Add(cameraActor);
+
+  renderTask.SetExclusive(true);
+  renderTask.SetInputEnabled(true);
+  renderTask.SetCameraActor(cameraActor);
+  renderTask.SetSourceActor(red);
+  application.SendNotification();
+  application.Render(16);
+
+  Vector2 viewportPosition = renderTask.GetCurrentViewportPosition();
+  Vector2 viewportSize     = renderTask.GetCurrentViewportSize();
+
+  // Check viewport return 0 without guide actor
+  DALI_TEST_EQUALS(viewportSize, Vector2(0, 0), TEST_LOCATION);
+  DALI_TEST_EQUALS(viewportPosition, Vector2(0, 0), TEST_LOCATION);
+
+  // Set ViewportGuideActor
+  renderTask.SetViewportGuideActor(blue);
+
+  // Render and notify
+  application.SendNotification();
+  application.Render(16);
+
+  viewportPosition = renderTask.GetCurrentViewportPosition();
+  viewportSize     = renderTask.GetCurrentViewportSize();
+
+  DALI_TEST_EQUALS(viewportSize, Vector2(300, 300), TEST_LOCATION);
+  DALI_TEST_EQUALS(viewportPosition, Vector2(90, 250), TEST_LOCATION);
+
+  // Render several frames.
+  application.SendNotification();
+  application.Render(16);
+  application.SendNotification();
+  application.Render(16);
+  application.SendNotification();
+  application.Render(16);
+
+  viewportPosition = renderTask.GetCurrentViewportPosition();
+  viewportSize     = renderTask.GetCurrentViewportSize();
+
+  DALI_TEST_EQUALS(viewportSize, Vector2(300, 300), TEST_LOCATION);
+  DALI_TEST_EQUALS(viewportPosition, Vector2(90, 250), TEST_LOCATION);
+
+  tet_printf("Destroy blue. RenderTask don't hold actor's reference, so ViewportGuideActor invalide now\n");
+
+  blue.Unparent();
+  blue.Reset();
+
+  DALI_TEST_EQUALS(renderTask.GetViewportGuideActor(), Actor(), TEST_LOCATION);
+
+  // Render and notify
+  application.SendNotification();
+  application.Render(16);
+
+  viewportPosition = renderTask.GetCurrentViewportPosition();
+  viewportSize     = renderTask.GetCurrentViewportSize();
+
+  // Check view port revert as origin well.
+  DALI_TEST_EQUALS(viewportSize, Vector2(0, 0), TEST_LOCATION);
+  DALI_TEST_EQUALS(viewportPosition, Vector2(0, 0), TEST_LOCATION);
+
+  // Render several frames.
+  application.SendNotification();
+  application.Render(16);
+  application.SendNotification();
+  application.Render(16);
+  application.SendNotification();
+  application.Render(16);
+
+  viewportPosition = renderTask.GetCurrentViewportPosition();
+  viewportSize     = renderTask.GetCurrentViewportSize();
+
+  DALI_TEST_EQUALS(viewportSize, Vector2(0, 0), TEST_LOCATION);
+  DALI_TEST_EQUALS(viewportPosition, Vector2(0, 0), TEST_LOCATION);
+
+  END_TEST;
+}
+
 int UtcDaliRenderTaskSetPartialUpdate(void)
 {
   TestApplication application(
index 2d11c66e92cf724eb79d6460e78c0217e2df1c9f..ded65b32582d57e11fc6f2c024dc2f177c298634 100644 (file)
@@ -981,6 +981,14 @@ public:
     OnBake();
   }
 
+  /**
+   * @brief Reset to base value without dirty flag check.
+   */
+  void ResetToBaseValueInternal(BufferIndex updateBufferIndex)
+  {
+    mValue[updateBufferIndex] = mBaseValue;
+  }
+
 private:
   // Undefined
   AnimatableProperty(const AnimatableProperty& property);
index 90ca9e5312a5388742bdc54d61dd34b251a40b1d..b3274b8b278d05fb79b4e3dde7bb7ce506e14bac 100644 (file)
@@ -46,26 +46,30 @@ void PropertyOwner::AddObserver(Observer& observer)
 {
   DALI_ASSERT_ALWAYS(!mObserverNotifying && "Cannot add observer while notifying PropertyOwner::Observers");
 
-  //Check for duplicates in debug builds
-  DALI_ASSERT_DEBUG(mObservers.End() == mObservers.Find(&observer));
-
-  mObservers.PushBack(&observer);
+  auto [iter, inserted] = mObservers.emplace(&observer, 1u);
+  if(!inserted)
+  {
+    // Increase the number of observer count
+    ++(iter->second);
+  }
 }
 
 void PropertyOwner::RemoveObserver(Observer& observer)
 {
   DALI_ASSERT_ALWAYS(!mObserverNotifying && "Cannot remove observer while notifying PropertyOwner::Observers");
 
-  const auto iter = mObservers.Find(&observer);
-  if(iter != mObservers.End())
+  auto iter = mObservers.find(&observer);
+  DALI_ASSERT_ALWAYS(iter != mObservers.end());
+
+  if(--(iter->second) == 0u)
   {
-    mObservers.Erase(iter);
+    mObservers.erase(iter);
   }
 }
 
 bool PropertyOwner::IsObserved()
 {
-  return mObservers.Count() != 0u;
+  return mObservers.size() != 0u;
 }
 
 void PropertyOwner::Destroy()
@@ -76,14 +80,14 @@ void PropertyOwner::Destroy()
   // Notification for observers
   for(auto&& item : mObservers)
   {
-    item->PropertyOwnerDestroyed(*this);
+    item.first->PropertyOwnerDestroyed(*this);
   }
 
   // Note : We don't need to restore mObserverNotifying to false as we are in delete the object.
   // If someone call AddObserver / RemoveObserver after this, assert.
 
   // Remove all observers
-  mObservers.Clear();
+  mObservers.clear();
 
   // Remove all constraints when disconnected from scene-graph
   mConstraints.Clear();
@@ -103,7 +107,7 @@ void PropertyOwner::ConnectToSceneGraph()
   // Notification for observers
   for(auto&& item : mObservers)
   {
-    item->PropertyOwnerConnected(*this);
+    item.first->PropertyOwnerConnected(*this);
   }
 
   mObserverNotifying = false;
@@ -119,16 +123,16 @@ void PropertyOwner::DisconnectFromSceneGraph(BufferIndex updateBufferIndex)
   mObserverNotifying = true;
 
   // Notification for observers
-  for(auto iter = mObservers.Begin(), endIter = mObservers.End(); iter != endIter;)
+  for(auto iter = mObservers.begin(); iter != mObservers.end();)
   {
-    auto returnValue = (*iter)->PropertyOwnerDisconnected(updateBufferIndex, *this);
+    auto returnValue = (*iter).first->PropertyOwnerDisconnected(updateBufferIndex, *this);
     if(returnValue == Observer::KEEP_OBSERVING)
     {
       ++iter;
     }
     else
     {
-      iter = mObservers.Erase(iter);
+      iter = mObservers.erase(iter);
     }
   }
 
index fd7698ab766af368552f74bbc2915e3939dd0e2b..3fe2c47dda352c2b79bc18288a992f4d65a46d01 100644 (file)
  *
  */
 
+// EXTERNAL INCLUDES
+#if defined(LOW_SPEC_MEMORY_MANAGEMENT_ENABLED)
+#include <dali/devel-api/common/map-wrapper.h>
+#else
+#include <unordered_map>
+#endif
+
 // INTERNAL INCLUDES
 #include <dali/devel-api/common/owner-container.h>
-#include <dali/integration-api/ordered-set.h>
 #include <dali/internal/common/const-string.h>
 #include <dali/internal/common/message.h>
 #include <dali/internal/update/animation/scene-graph-constraint-declarations.h>
@@ -288,9 +294,15 @@ protected:
   bool                   mIsConnectedToSceneGraph;
 
 private:
-  using ObserverContainer = Integration::OrderedSet<PropertyOwner::Observer, false>;
-  using ObserverIter      = ObserverContainer::Iterator;
-  using ConstObserverIter = ObserverContainer::ConstIterator;
+#if defined(LOW_SPEC_MEMORY_MANAGEMENT_ENABLED)
+  using ObserverContainer = std::map<PropertyOwner::Observer*, uint32_t>; ///< Observers container. We allow to add same observer multiple times.
+                                                                          ///< Key is a pointer to observer, value is the number of observer added.
+#else
+  using ObserverContainer = std::unordered_map<PropertyOwner::Observer*, uint32_t>; ///< Observers container. We allow to add same observer multiple times.
+                                                                                    ///< Key is a pointer to observer, value is the number of observer added.
+#endif
+  using ObserverIter      = ObserverContainer::iterator;
+  using ConstObserverIter = ObserverContainer::const_iterator;
 
   ObserverContainer mObservers; ///< Container of observer raw-pointers (not owned)
 
index 8044497fffccfb2be1508cda7d5cda290efc8a74..c5b65cae663ea9cc46ddc0a0cc0418b58dedfaf1 100644 (file)
@@ -51,10 +51,18 @@ RenderTask::~RenderTask()
       mSourceNode->RemoveExclusiveRenderTask(this);
     }
   }
+  if(mStopperNode)
+  {
+    mStopperNode->RemoveObserver(*this);
+  }
   if(mCameraNode)
   {
     mCameraNode->RemoveObserver(*this);
   }
+  if(mViewportGuideNode)
+  {
+    mViewportGuideNode->RemoveObserver(*this);
+  }
   if(mRenderSyncTracker)
   {
     if(DALI_LIKELY(mRenderMessageDispatcher))
@@ -99,7 +107,17 @@ Node* RenderTask::GetSourceNode() const
 
 void RenderTask::SetStopperNode(Node* node)
 {
+  if(mStopperNode)
+  {
+    mStopperNode->RemoveObserver(*this);
+  }
+
   mStopperNode = node;
+
+  if(mStopperNode)
+  {
+    mStopperNode->AddObserver(*this);
+  }
 }
 
 Node* RenderTask::GetStopperNode() const
@@ -109,7 +127,17 @@ Node* RenderTask::GetStopperNode() const
 
 void RenderTask::SetViewportGuideNode(Node* node)
 {
+  if(mViewportGuideNode)
+  {
+    mViewportGuideNode->RemoveObserver(*this);
+  }
+
   mViewportGuideNode = node;
+
+  if(mViewportGuideNode)
+  {
+    mViewportGuideNode->AddObserver(*this);
+  }
 }
 
 Node* RenderTask::GetViewportGuideNode() const
@@ -432,8 +460,11 @@ void RenderTask::UpdateViewport(BufferIndex updateBufferIndex, Vector2 sceneSize
     Vector2 screenPosition(halfSceneSize.width + worldPosition.x - halfNodeSize.x,
                            halfSceneSize.height + worldPosition.y - halfNodeSize.y);
 
-    /* This is an implicit constraint - the properties will be dirty until the node
+    /**
+     * This is an implicit constraint - the properties will be dirty until the node
      * is removed. (RenderTask::Impl manages this)
+     *
+     * TODO : Need to make Resseter for these properties.
      */
     mViewportPosition.Set(updateBufferIndex, screenPosition);
     mViewportSize.Set(updateBufferIndex, Vector2(nodeSize));
@@ -487,13 +518,19 @@ void RenderTask::ContextDestroyed()
 
 void RenderTask::PropertyOwnerConnected(PropertyOwner& owner)
 {
-  // check if we've gone from inactive to active
-  SetActiveStatus();
+  if(&owner == static_cast<PropertyOwner*>(mSourceNode) || &owner == static_cast<PropertyOwner*>(mCameraNode))
+  {
+    // check if we've gone from inactive to active
+    SetActiveStatus();
+  }
 }
 
 PropertyOwner::Observer::NotifyReturnType RenderTask::PropertyOwnerDisconnected(BufferIndex /*updateBufferIndex*/, PropertyOwner& owner)
 {
-  mActive = false; // if either source or camera disconnected, we're no longer active
+  if(&owner == static_cast<PropertyOwner*>(mSourceNode) || &owner == static_cast<PropertyOwner*>(mCameraNode))
+  {
+    mActive = false; // if either source or camera disconnected, we're no longer active
+  }
   return PropertyOwner::Observer::NotifyReturnType::KEEP_OBSERVING;
 }
 
@@ -502,13 +539,32 @@ void RenderTask::PropertyOwnerDestroyed(PropertyOwner& owner)
   if(static_cast<PropertyOwner*>(mSourceNode) == &owner)
   {
     mSourceNode = nullptr;
+
+    mActive = false; // if either source or camera destroyed, we're no longer active
   }
-  else if(static_cast<PropertyOwner*>(mCameraNode) == &owner)
+  if(static_cast<PropertyOwner*>(mCameraNode) == &owner)
   {
     mCameraNode = nullptr;
+
+    mActive = false; // if either source or camera destroyed, we're no longer active
+  }
+  if(static_cast<PropertyOwner*>(mStopperNode) == &owner)
+  {
+    mStopperNode = nullptr;
   }
+  if(static_cast<PropertyOwner*>(mViewportGuideNode) == &owner)
+  {
+    mViewportGuideNode = nullptr;
 
-  mActive = false; // if either source or camera destroyed, we're no longer active
+    if(DALI_LIKELY(!Dali::Stage::IsShuttingDown()))
+    {
+      // TODO : Until SG::RenderTask Resseter preopared, just call this internal API without dirty flag down.
+      mViewportPosition.ResetToBaseValueInternal(0);
+      mViewportPosition.ResetToBaseValueInternal(1);
+      mViewportSize.ResetToBaseValueInternal(0);
+      mViewportSize.ResetToBaseValueInternal(1);
+    }
+  }
 }
 
 void RenderTask::AddInitializeResetter(ResetterManager& manager) const