(FrameCallback) Actor & FrameCallback Lifecycle management

Change-Id: I2aaae83942bd344f8bab1d4caf1bc04e04e830d6

diff --git a/automated-tests/src/dali/utc-Dali-FrameCallbackInterface.cpp b/automated-tests/src/dali/utc-Dali-FrameCallbackInterface.cpp
index c77f391..9c4f31b 100644 (file)
--- a/automated-tests/src/dali/utc-Dali-FrameCallbackInterface.cpp
+++ b/automated-tests/src/dali/utc-Dali-FrameCallbackInterface.cpp
@@ -730,3 +730,120 @@ int UtcDaliFrameCallbackActorRemovedAndAdded(void)
 
   END_TEST;
 }
+
+int UtcDaliFrameCallbackMultipleCallbacks(void)
+{
+  // Test to ensure multiple frame-callbacks work as expected
+
+  TestApplication application;
+  Stage stage = Stage::GetCurrent();
+
+  Actor actor = Actor::New();
+  stage.Add( actor );
+
+  FrameCallbackBasic frameCallback1;
+  FrameCallbackBasic frameCallback2;
+  DevelStage::AddFrameCallback( stage, frameCallback1, stage.GetRootLayer() );
+  DevelStage::AddFrameCallback( stage, frameCallback2, stage.GetRootLayer() );
+
+  application.SendNotification();
+  application.Render();
+
+  DALI_TEST_EQUALS( frameCallback1.mCalled, true,  TEST_LOCATION );
+  DALI_TEST_EQUALS( frameCallback2.mCalled, true,  TEST_LOCATION );
+  frameCallback1.Reset();
+  frameCallback2.Reset();
+
+  // Remove the second frame-callback, only the first should be called
+
+  DevelStage::RemoveFrameCallback( stage, frameCallback2 );
+
+  application.SendNotification();
+  application.Render();
+
+  DALI_TEST_EQUALS( frameCallback1.mCalled, true,  TEST_LOCATION );
+  DALI_TEST_EQUALS( frameCallback2.mCalled, false, TEST_LOCATION );
+  frameCallback1.Reset();
+  frameCallback2.Reset();
+
+  // Re-add the second frame-callback and remove the first, only the second should be called
+
+  DevelStage::AddFrameCallback( stage, frameCallback2, stage.GetRootLayer() );
+  DevelStage::RemoveFrameCallback( stage, frameCallback1 );
+
+  application.SendNotification();
+  application.Render();
+
+  DALI_TEST_EQUALS( frameCallback1.mCalled, false, TEST_LOCATION );
+  DALI_TEST_EQUALS( frameCallback2.mCalled, true,  TEST_LOCATION );
+  frameCallback1.Reset();
+  frameCallback2.Reset();
+
+  // Attempt removal of the first frame-callback again, should be a no-op and yield the exact same results as the last run
+  DevelStage::RemoveFrameCallback( stage, frameCallback1 );
+
+  application.SendNotification();
+  application.Render();
+
+  DALI_TEST_EQUALS( frameCallback1.mCalled, false, TEST_LOCATION );
+  DALI_TEST_EQUALS( frameCallback2.mCalled, true,  TEST_LOCATION );
+  frameCallback1.Reset();
+  frameCallback2.Reset();
+
+  // Remove the second frame-callback as well, neither should be called
+  DevelStage::RemoveFrameCallback( stage, frameCallback2 );
+
+  application.SendNotification();
+  application.Render();
+
+  DALI_TEST_EQUALS( frameCallback1.mCalled, false, TEST_LOCATION );
+  DALI_TEST_EQUALS( frameCallback2.mCalled, false, TEST_LOCATION );
+
+  END_TEST;
+}
+
+int UtcDaliFrameCallbackActorDestroyed(void)
+{
+  TestApplication application;
+  Stage stage = Stage::GetCurrent();
+
+  Actor actor = Actor::New();
+  stage.Add( actor );
+
+  FrameCallbackBasic frameCallback1;
+  FrameCallbackBasic frameCallback2;
+  DevelStage::AddFrameCallback( stage, frameCallback1, actor );
+  DevelStage::AddFrameCallback( stage, frameCallback2, actor );
+
+  application.SendNotification();
+  application.Render();
+
+  DALI_TEST_EQUALS( frameCallback1.mCalled, true,  TEST_LOCATION );
+  DALI_TEST_EQUALS( frameCallback2.mCalled, true,  TEST_LOCATION );
+  frameCallback1.Reset();
+  frameCallback2.Reset();
+
+  // Remove the second frame-callback, only the first should be called
+
+  DevelStage::RemoveFrameCallback( stage, frameCallback2 );
+
+  application.SendNotification();
+  application.Render();
+
+  DALI_TEST_EQUALS( frameCallback1.mCalled, true,  TEST_LOCATION );
+  DALI_TEST_EQUALS( frameCallback2.mCalled, false, TEST_LOCATION );
+  frameCallback1.Reset();
+  frameCallback2.Reset();
+
+  // Remove and destroy the actor, the first one should not be called either
+  stage.Remove( actor );
+  actor.Reset();
+
+  application.SendNotification();
+  application.Render();
+
+  DALI_TEST_EQUALS( frameCallback1.mCalled, false,  TEST_LOCATION );
+  DALI_TEST_EQUALS( frameCallback2.mCalled, false, TEST_LOCATION );
+
+  END_TEST;
+}

diff --git a/dali/internal/update/manager/frame-callback-processor.cpp b/dali/internal/update/manager/frame-callback-processor.cpp
index 17423d1..9d16232 100644 (file)
--- a/dali/internal/update/manager/frame-callback-processor.cpp
+++ b/dali/internal/update/manager/frame-callback-processor.cpp
@@ -18,10 +18,12 @@
 // CLASS HEADER
 #include <dali/internal/update/manager/frame-callback-processor.h>
 
+// EXTERNAL INCLUDES
+#include <algorithm>
+
 // INTERNAL INCLUDES
 #include <dali/devel-api/update/frame-callback-interface.h>
 #include <dali/devel-api/update/update-proxy.h>
-#include <dali/internal/update/manager/update-proxy-impl.h>
 
 namespace Dali
 {
@@ -35,6 +37,9 @@ namespace SceneGraph
 namespace
 {
 
+/**
+ * Given a node, it matches all update-proxies using that node as their root-node.
+ */
 template< typename FrameCallbackInfoT >
 class MatchRootNode
 {
@@ -52,7 +57,7 @@ public:
 
 private:
 
-  PropertyOwner& mRootNode;
+  const PropertyOwner& mRootNode;
 };
 
 } // unnamed namespace
@@ -73,10 +78,6 @@ void FrameCallbackProcessor::AddFrameCallback( FrameCallbackInterface* frameCall
 {
   Node& node = const_cast< Node& >( *rootNode ); // Was sent as const from event thread, we need to be able to use non-const version here.
 
-  FrameCallbackInfo info;
-  info.frameCallback = frameCallback;
-  info.updateProxyImpl = new UpdateProxy( mTransformManager, node );
-
   // We want to be notified when the node is destroyed (if we're not observing it already)
   auto iter = std::find_if( mFrameCallbacks.begin(), mFrameCallbacks.end(), MatchRootNode< FrameCallbackInfo >( node ) );
   if( iter == mFrameCallbacks.end() )
@@ -84,20 +85,37 @@ void FrameCallbackProcessor::AddFrameCallback( FrameCallbackInterface* frameCall
     node.AddObserver( *this );
   }
 
-  mFrameCallbacks.push_back( info );
+  mFrameCallbacks.emplace_back( FrameCallbackInfo( frameCallback , new UpdateProxy( mTransformManager, node ) ) );
 }
 
 void FrameCallbackProcessor::RemoveFrameCallback( FrameCallbackInterface* frameCallback )
 {
-  auto iter = std::remove_if( mFrameCallbacks.begin(), mFrameCallbacks.end(),
-                              [ this, frameCallback ]
-                                ( const FrameCallbackInfo& info )
-                                {
-                                  info.updateProxyImpl->GetRootNode().RemoveObserver( *this );
-                                  delete info.updateProxyImpl;
-                                  return info.frameCallback == frameCallback;
-                                } );
+  std::vector< SceneGraph::Node* > nodesToStopObserving;
+
+  // Find and remove all matching frame-callbacks
+  auto iter =
+    std::remove_if( mFrameCallbacks.begin(), mFrameCallbacks.end(),
+                    [ frameCallback, &nodesToStopObserving ] ( FrameCallbackInfo& info )
+                    {
+                      bool match = false;
+                      if( info.frameCallback == frameCallback )
+                      {
+                        nodesToStopObserving.push_back( &info.updateProxyImpl->GetRootNode() );
+                        match = true;
+                      }
+                      return match;
+                    } );
   mFrameCallbacks.erase( iter, mFrameCallbacks.end() );
+
+  // Only stop observing the removed frame-callback nodes if none of the other frame-callbacks use them as root nodes
+  for( auto&& node : nodesToStopObserving )
+  {
+    auto nodeMatchingIter = std::find_if( mFrameCallbacks.begin(), mFrameCallbacks.end(), MatchRootNode< FrameCallbackInfo >( *node ) );
+    if( nodeMatchingIter == mFrameCallbacks.end() )
+    {
+      node->RemoveObserver( *this );
+    }
+  }
 }
 
 void FrameCallbackProcessor::Update( BufferIndex bufferIndex, float elapsedSeconds )

diff --git a/dali/internal/update/manager/frame-callback-processor.h b/dali/internal/update/manager/frame-callback-processor.h
index 15d7376..52e0ff6 100644 (file)
--- a/dali/internal/update/manager/frame-callback-processor.h
+++ b/dali/internal/update/manager/frame-callback-processor.h
@@ -18,10 +18,14 @@
  *
  */
 
+// EXTERNAL INCLUDES
+#include <memory>
+
 // INTERNAL INCLUDES
 #include <dali/public-api/common/vector-wrapper.h>
 #include <dali/internal/common/buffer-index.h>
 #include <dali/internal/update/common/property-owner.h>
+#include <dali/internal/update/manager/update-proxy-impl.h>
 
 namespace Dali
 {
@@ -31,8 +35,6 @@ class FrameCallbackInterface;
 namespace Internal
 {
 
-class UpdateProxy;
-
 namespace SceneGraph
 {
 
@@ -116,16 +118,37 @@ private:
 
   struct FrameCallbackInfo
   {
-    FrameCallbackInterface* frameCallback;
-    UpdateProxy* updateProxyImpl;
+    /**
+     * Default Constructor
+     * @param[in]  frameCallbackObject  A pointer to the frame-callback object
+     * @param[in]  updateProxyPtr       A raw pointer to the newly created updateProxy
+     * @note Ownership of @updateProxyPtr is passed to this class.
+     */
+    FrameCallbackInfo( FrameCallbackInterface* frameCallbackObject, UpdateProxy* updateProxyPtr )
+    : frameCallback( frameCallbackObject ),
+      updateProxyImpl( updateProxyPtr )
+    {
+    }
+
+    ~FrameCallbackInfo() = default; ///< Default destructor.
+
+    // Movable but not copyable
+    FrameCallbackInfo( const FrameCallbackInfo& )            = delete;  ///< Deleted copy constructor.
+    FrameCallbackInfo( FrameCallbackInfo&& )                 = default; ///< Default move constructor.
+    FrameCallbackInfo& operator=( const FrameCallbackInfo& ) = delete;  ///< Deleted copy assignment operator.
+    FrameCallbackInfo& operator=( FrameCallbackInfo&& )      = default; ///< Default move assignment operator.
+
+    // Data
+    FrameCallbackInterface* frameCallback{ nullptr }; ///< Pointer to the implementation of the FrameCallbackInterface.
+    std::unique_ptr< UpdateProxy > updateProxyImpl{ nullptr }; ///< A unique pointer to the implementation of the UpdateProxy.
   };
 
-  std::vector< FrameCallbackInfo > mFrameCallbacks;
+  std::vector< FrameCallbackInfo > mFrameCallbacks; ///< A container of all the frame-callbacks & accompanying update-proxies.
 
   TransformManager& mTransformManager;
   Node& mRootNode;
 
-  bool mNodeHierarchyChanged;
+  bool mNodeHierarchyChanged; ///< Set to true if the node hierarchy changes
 };
 
 } // namespace SceneGraph