Guarantee notifications are served in the right order in the case that event thread... 93/24093/1
authorKimmo Hoikka <kimmo.hoikka@samsung.com>
Wed, 25 Jun 2014 10:58:20 +0000 (11:58 +0100)
committerAdeel Kazmi <adeel.kazmi@samsung.com>
Tue, 8 Jul 2014 13:19:39 +0000 (14:19 +0100)
Change-Id: I30374a8a86a93bb022ee83abe6a9459e78c9acab
Signed-off-by: Adeel Kazmi <adeel.kazmi@samsung.com>
automated-tests/src/dali/utc-Dali-Animation.cpp
automated-tests/src/dali/utc-Dali-PropertyNotification.cpp
dali/internal/common/message-container.h [deleted file]
dali/internal/common/owner-container.h
dali/internal/event/common/notification-manager.cpp
dali/internal/event/common/notification-manager.h

index 537abb8..1d85c0d 100644 (file)
@@ -8947,4 +8947,56 @@ int UtcDaliAnimationUpdateManager(void)
   END_TEST;
 }
 
+int UtcDaliAnimationSignalOrder(void)
+{
+  TestApplication application;
+
+  Actor actor = Actor::New();
+  Stage::GetCurrent().Add( actor );
+
+  // Build the animations
+  Animation animation1 = Animation::New( 0.0f ); // finishes first frame
+  Animation animation2 = Animation::New( 0.02f ); // finishes in 20 ms
+
+  bool signal1Received = false;
+  animation1.FinishedSignal().Connect( &application, AnimationFinishCheck( signal1Received ) );
+
+  bool signal2Received = false;
+  animation2.FinishedSignal().Connect( &application, AnimationFinishCheck( signal2Received ) );
+
+  // Apply animations to actor
+  animation1.AnimateTo( Property(actor, Actor::POSITION), Vector3( 3.0f, 2.0f, 1.0f ), AlphaFunctions::Linear );
+  animation1.Play();
+  animation2.AnimateTo( Property(actor, Actor::SIZE ), Vector3( 10.0f, 20.0f, 30.0f ), AlphaFunctions::Linear );
+  animation2.Play();
+
+  DALI_TEST_EQUALS( signal1Received, false, TEST_LOCATION );
+  DALI_TEST_EQUALS( signal2Received, false, TEST_LOCATION );
+
+  application.SendNotification();
+  application.UpdateOnly( 10 ); // 10ms progress
+
+  // no notifications yet
+  DALI_TEST_EQUALS( signal1Received, false, TEST_LOCATION );
+  DALI_TEST_EQUALS( signal2Received, false, TEST_LOCATION );
+
+  application.SendNotification();
+
+  // first completed
+  DALI_TEST_EQUALS( signal1Received, true, TEST_LOCATION );
+  DALI_TEST_EQUALS( signal2Received, false, TEST_LOCATION );
+  signal1Received = false;
+
+  // 1st animation is complete now, do another update with no ProcessEvents in between
+  application.UpdateOnly( 20 ); // 20ms progress
+
+  // ProcessEvents
+  application.SendNotification();
+
+  // 2nd should complete now
+  DALI_TEST_EQUALS( signal1Received, false, TEST_LOCATION );
+  DALI_TEST_EQUALS( signal2Received, true, TEST_LOCATION );
+
+  END_TEST;
+}
 
index 4373116..69de8ce 100644 (file)
@@ -842,3 +842,57 @@ int UtcDaliPropertyNotificationVariableStep(void)
   }
   END_TEST;
 }
+
+static bool gCallBack2Called = false;
+void TestCallback2(PropertyNotification& source)
+{
+  gCallBack2Called = true;
+}
+
+int UtcDaliPropertyNotificationOrder(void)
+{
+  TestApplication application; // Reset all test adapter return codes
+
+  Actor actor = Actor::New();
+  Stage::GetCurrent().Add(actor);
+  // this should complete in first frame
+  PropertyNotification notification1 = actor.AddPropertyNotification( Actor::POSITION_X, GreaterThanCondition(90.0f) );
+  notification1.NotifySignal().Connect( &TestCallback );
+  // this should complete in second frame
+  PropertyNotification notification2 = actor.AddPropertyNotification( Actor::POSITION_X, GreaterThanCondition(150.0f) );
+  notification2.NotifySignal().Connect( &TestCallback2 );
+  Animation animation = Animation::New( 0.032f ); // finishes in 32 ms
+  animation.AnimateTo( Property(actor, Actor::POSITION ), Vector3( 200.0f, 0.0f, 0.0f ), AlphaFunctions::Linear );
+  animation.Play();
+
+  // flush the queue
+  application.SendNotification();
+  // first frame
+  application.Render(RENDER_FRAME_INTERVAL);
+  // no notifications yet
+  DALI_TEST_EQUALS( gCallBackCalled, false, TEST_LOCATION );
+  DALI_TEST_EQUALS( gCallBack2Called, false, TEST_LOCATION );
+  gCallBackCalled = false;
+  gCallBack2Called = false;
+
+  // dont serve the notifications but run another update & render
+  // this simulates situation where there is a notification in event side but it's not been picked up by event thread
+  // second frame
+  application.Render(RENDER_FRAME_INTERVAL);
+  DALI_TEST_EQUALS( gCallBackCalled, false, TEST_LOCATION );
+  DALI_TEST_EQUALS( gCallBack2Called, false, TEST_LOCATION );
+
+  // serve the notifications
+  application.SendNotification();
+  DALI_TEST_EQUALS( gCallBackCalled, true, TEST_LOCATION );
+  DALI_TEST_EQUALS( gCallBack2Called, true, TEST_LOCATION );
+
+  gCallBackCalled = false;
+  gCallBack2Called = false;
+  application.Render(RENDER_FRAME_INTERVAL);
+  application.SendNotification();
+  DALI_TEST_EQUALS( gCallBackCalled, false, TEST_LOCATION );
+  DALI_TEST_EQUALS( gCallBack2Called, false, TEST_LOCATION );
+
+  END_TEST;
+}
diff --git a/dali/internal/common/message-container.h b/dali/internal/common/message-container.h
deleted file mode 100644 (file)
index 0f17516..0000000
+++ /dev/null
@@ -1,37 +0,0 @@
-#ifndef __DALI_INTERNAL_MESSAGE_CONTAINER_H__
-#define __DALI_INTERNAL_MESSAGE_CONTAINER_H__
-
-/*
- * Copyright (c) 2014 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.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- *
- */
-
-// INTERNAL INCLUDES
-#include <dali/internal/common/message.h>
-#include <dali/internal/common/owner-container.h>
-
-namespace Dali
-{
-
-namespace Internal
-{
-
-typedef OwnerContainer< MessageBase* > MessageContainer;
-
-} // namespace Internal
-
-} // namespace Dali
-
-#endif // __DALI_INTERNAL_MESSAGE_CONTAINER_H__
index 60b9d28..cbf16e4 100644 (file)
@@ -129,6 +129,34 @@ public:
     Vector< T >::Resize( size );
   }
 
+  /**
+   * Move the ownership of objects from another OwnerContainer to this one
+   * without deleting them. It will keep the original items here as well.
+   * @param[in] source where to move elements from to this OwnerContainer
+   */
+  void MoveFrom( OwnerContainer& source )
+  {
+    // Optimisation for the case that this is empty
+    if( IsEmpty() )
+    {
+      Swap( source );
+    }
+    else
+    {
+      // make space for new items
+      Reserve( VectorBase::Count() + source.Count() );
+      Iterator iter = source.Begin();
+      ConstIterator end = source.End();
+      for( ; iter != end; ++iter )
+      {
+        T pointer = *iter;
+        PushBack( pointer );
+      }
+      // cannot call Clear on OwnerContainer as that deletes the elements
+      source.Vector< T >::Clear();
+    }
+  }
+
 private:
 
   // Undefined copy constructor.
index dbebbec..bbddf2d 100644 (file)
@@ -17,6 +17,8 @@
 
 // CLASS HEADER
 #include <dali/internal/event/common/notification-manager.h>
+#include <dali/internal/common/owner-container.h>
+#include <dali/internal/common/message.h>
 
 // EXTERNAL INCLUDES
 #ifdef __clang__
@@ -35,9 +37,6 @@
 // INTERNAL INCLUDES
 #include <dali/public-api/common/dali-common.h>
 #include <dali/internal/event/common/property-notification-impl.h>
-#include <dali/internal/common/message-container.h>
-
-using namespace std;
 
 namespace Dali
 {
@@ -46,12 +45,18 @@ namespace Internal
 {
 
 typedef boost::mutex MessageQueueMutex;
+typedef OwnerContainer< MessageBase* > MessageContainer;
 
 struct NotificationManager::Impl
 {
   Impl()
   : notificationCount(0)
   {
+    // reserve space on the vectors to avoid reallocs
+    // applications typically have upto 20-30 notifications at startup
+    updateCompletedQueue.Reserve( 32 );
+    updateWorkingQueue.Reserve( 32 );
+    eventQueue.Reserve( 32 );
   }
 
   ~Impl()
@@ -82,7 +87,7 @@ void NotificationManager::QueueMessage( MessageBase* message )
 {
   DALI_ASSERT_DEBUG( NULL != message );
 
-  // queueMutex must be locked whilst accessing queue
+  // queueMutex must be locked whilst accessing queues
   MessageQueueMutex::scoped_lock lock( mImpl->queueMutex );
 
   mImpl->updateWorkingQueue.PushBack( message );
@@ -90,15 +95,18 @@ void NotificationManager::QueueMessage( MessageBase* message )
 
 void NotificationManager::UpdateCompleted()
 {
-  // queueMutex must be locked whilst accessing queue
+  // queueMutex must be locked whilst accessing queues
   MessageQueueMutex::scoped_lock lock( mImpl->queueMutex );
-  // Swap the queue, original queue ends up empty, then release the lock
-  mImpl->updateCompletedQueue.Swap( mImpl->updateWorkingQueue );
+  // Move messages from update working queue to completed queue
+  // note that in theory its possible for update completed to have last frames
+  // events as well still hanging around. we need to keep them as well
+  mImpl->updateCompletedQueue.MoveFrom( mImpl->updateWorkingQueue );
+  // finally the lock is released
 }
 
 bool NotificationManager::MessagesToProcess()
 {
-  // queueMutex must be locked whilst accessing queue
+  // queueMutex must be locked whilst accessing queues
   MessageQueueMutex::scoped_lock lock( mImpl->queueMutex );
 
   return ( false == mImpl->updateCompletedQueue.IsEmpty() );
@@ -109,12 +117,14 @@ void NotificationManager::ProcessMessages()
   // Done before messages are processed, for notification count comparisons
   ++mImpl->notificationCount;
 
-  // queueMutex must be locked whilst accessing queue
+  // queueMutex must be locked whilst accessing queues
   {
     MessageQueueMutex::scoped_lock lock( mImpl->queueMutex );
 
-    // Swap the queue, original queue ends up empty, then release the lock
-    mImpl->updateCompletedQueue.Swap( mImpl->eventQueue );
+    // Move messages from update completed queue to event queue
+    // note that in theory its possible for event queue to have
+    // last frames events as well still hanging around so need to keep them
+    mImpl->eventQueue.MoveFrom( mImpl->updateCompletedQueue );
   }
   // end of scope, lock is released
 
index 23b2e4e..00be5d3 100644 (file)
@@ -19,7 +19,6 @@
  */
 
 // INTERNAL INCLUDES
-#include <dali/internal/common/message.h>
 
 namespace Dali
 {
@@ -28,6 +27,7 @@ namespace Internal
 {
 
 class PropertyNotification;
+class MessageBase;
 
 /**
  * Provides notifications to the event-thread regarding the changes in previous update(s).