Prevent event thread from picking up notifications too soon 89/24089/1
authorKimmo Hoikka <kimmo.hoikka@samsung.com>
Tue, 24 Jun 2014 17:22:36 +0000 (18:22 +0100)
committerAdeel Kazmi <adeel.kazmi@samsung.com>
Tue, 8 Jul 2014 13:19:38 +0000 (14:19 +0100)
[problem]      Animation-finished and property-notifications can be fired off too soon.
[cause]        We queue the message from update straight away, even before the scene has been
               updated.
[solution]     Create another buffer in the notification-manager which is only swapped at the end
               of the update.

Change-Id: I7b72fa2485ed9faa606b52a3925e111df2931776
Signed-off-by: Adeel Kazmi <adeel.kazmi@samsung.com>
automated-tests/src/dali/tct-dali-core.h
automated-tests/src/dali/utc-Dali-Animation.cpp
dali/internal/event/common/notification-manager.cpp
dali/internal/event/common/notification-manager.h
dali/internal/update/manager/update-manager.cpp

index 85e58e4..a2c2f6a 100644 (file)
@@ -427,6 +427,7 @@ extern int UtcDaliAnimationAnimateBetweenActorColorFunction(void);
 extern int UtcDaliAnimationAnimateBetweenActorColorFunctionTimePeriod(void);
 extern int UtcDaliAnimationAnimateVector3Func(void);
 extern int UtcDaliAnimationCreateDestroy(void);
+extern int UtcDaliAnimationUpdateManager(void);
 extern int UtcDaliAnyConstructors(void);
 extern int UtcDaliAnyAssignmentOperators(void);
 extern int UtcDaliAnyNegativeAssignmentOperators(void);
@@ -1654,6 +1655,7 @@ testcase tc_array[] = {
     {"UtcDaliAnimationAnimateBetweenActorColorFunctionTimePeriod", UtcDaliAnimationAnimateBetweenActorColorFunctionTimePeriod, utc_dali_animation_startup, utc_dali_animation_cleanup},
     {"UtcDaliAnimationAnimateVector3Func", UtcDaliAnimationAnimateVector3Func, utc_dali_animation_startup, utc_dali_animation_cleanup},
     {"UtcDaliAnimationCreateDestroy", UtcDaliAnimationCreateDestroy, utc_dali_animation_startup, utc_dali_animation_cleanup},
+    {"UtcDaliAnimationUpdateManager", UtcDaliAnimationUpdateManager, utc_dali_animation_startup, utc_dali_animation_cleanup},
     {"UtcDaliAnyConstructors", UtcDaliAnyConstructors, utc_dali_any_startup, utc_dali_any_cleanup},
     {"UtcDaliAnyAssignmentOperators", UtcDaliAnyAssignmentOperators, utc_dali_any_startup, utc_dali_any_cleanup},
     {"UtcDaliAnyNegativeAssignmentOperators", UtcDaliAnyNegativeAssignmentOperators, utc_dali_any_startup, utc_dali_any_cleanup},
index bdf7ad2..537abb8 100644 (file)
@@ -8893,3 +8893,58 @@ int UtcDaliAnimationCreateDestroy(void)
   delete animation;
   END_TEST;
 }
+
+struct UpdateManagerTestConstraint
+{
+  UpdateManagerTestConstraint(TestApplication& application)
+  : mApplication(application)
+  {
+  }
+
+  Vector3 operator()(const Vector3& current)
+  {
+    mApplication.SendNotification();  // Process events
+    return current;
+  }
+
+  TestApplication& mApplication;
+};
+
+int UtcDaliAnimationUpdateManager(void)
+{
+  TestApplication application;
+
+  Actor actor = Actor::New();
+  Stage::GetCurrent().Add( actor );
+
+  // Build the animation
+  Animation animation = Animation::New( 0.0f );
+
+  bool signalReceived = false;
+  AnimationFinishCheck finishCheck( signalReceived );
+  animation.FinishedSignal().Connect( &application, finishCheck );
+
+  Vector3 startValue(1.0f, 1.0f, 1.0f);
+  Property::Index index = actor.RegisterProperty( "test-property", startValue );
+  Constraint constraint = Constraint::New<Vector3>( index, UpdateManagerTestConstraint( application ) );
+  actor.ApplyConstraint( constraint );
+
+  // Apply animation to actor
+  BounceFunc func(0.0f, 0.0f, -100.0f);
+  animation.Animate<Vector3>( Property(actor, Actor::POSITION), func, AlphaFunctions::Linear );
+
+  animation.Play();
+
+  application.SendNotification();
+  application.UpdateOnly( 16 );
+
+  finishCheck.CheckSignalNotReceived();
+
+  application.SendNotification();   // Process events
+
+  finishCheck.CheckSignalReceived();
+
+  END_TEST;
+}
+
+
index b38eda4..dbebbec 100644 (file)
@@ -63,7 +63,8 @@ struct NotificationManager::Impl
 
   // queueMutex must be locked whilst accessing queue
   MessageQueueMutex queueMutex;
-  MessageContainer updateQueue;
+  MessageContainer updateCompletedQueue;
+  MessageContainer updateWorkingQueue;
   MessageContainer eventQueue;
 };
 
@@ -84,7 +85,15 @@ void NotificationManager::QueueMessage( MessageBase* message )
   // queueMutex must be locked whilst accessing queue
   MessageQueueMutex::scoped_lock lock( mImpl->queueMutex );
 
-  mImpl->updateQueue.PushBack( message );
+  mImpl->updateWorkingQueue.PushBack( message );
+}
+
+void NotificationManager::UpdateCompleted()
+{
+  // queueMutex must be locked whilst accessing queue
+  MessageQueueMutex::scoped_lock lock( mImpl->queueMutex );
+  // Swap the queue, original queue ends up empty, then release the lock
+  mImpl->updateCompletedQueue.Swap( mImpl->updateWorkingQueue );
 }
 
 bool NotificationManager::MessagesToProcess()
@@ -92,7 +101,7 @@ bool NotificationManager::MessagesToProcess()
   // queueMutex must be locked whilst accessing queue
   MessageQueueMutex::scoped_lock lock( mImpl->queueMutex );
 
-  return ( false == mImpl->updateQueue.IsEmpty() );
+  return ( false == mImpl->updateCompletedQueue.IsEmpty() );
 }
 
 void NotificationManager::ProcessMessages()
@@ -105,7 +114,7 @@ void NotificationManager::ProcessMessages()
     MessageQueueMutex::scoped_lock lock( mImpl->queueMutex );
 
     // Swap the queue, original queue ends up empty, then release the lock
-    mImpl->updateQueue.Swap( mImpl->eventQueue );
+    mImpl->updateCompletedQueue.Swap( mImpl->eventQueue );
   }
   // end of scope, lock is released
 
index aa4912e..23b2e4e 100644 (file)
@@ -38,7 +38,7 @@ class NotificationManager
 public:
 
   /**
-   * Create an NotificationManager.
+   * Create an NotificationManager. Owned by Core in event thread side.
    */
   NotificationManager();
 
@@ -47,6 +47,8 @@ public:
    */
   virtual ~NotificationManager();
 
+/// Update side interface, can only be called from Update-thread
+
   /**
    * Queue a scene message. This method is thread-safe.
    * @param[in] message A newly allocated message; NotificationManager takes ownership.
@@ -54,6 +56,13 @@ public:
   void QueueMessage( MessageBase* message );
 
   /**
+   * Signal Notification Manager that update frame is completed so it can let event thread process the notifications
+   */
+  void UpdateCompleted();
+
+/// Event side interface, can only be called from Update-thread
+
+  /**
    * Query whether the NotificationManager has messages to process.
    * @return True if there are messages to process.
    */
index a76536f..0a94ddf 100644 (file)
@@ -838,6 +838,12 @@ void UpdateManager::Animate( float elapsedSeconds )
     }
   }
 
+  if ( mImpl->animationFinishedDuringUpdate )
+  {
+    // The application should be notified by NotificationManager, in another thread
+    mImpl->notificationManager.QueueMessage( AnimationFinishedMessage( mImpl->animationFinishedNotifier ) );
+  }
+
   PERF_MONITOR_END(PerformanceMonitor::ANIMATE_NODES);
 }
 
@@ -1174,18 +1180,12 @@ unsigned int UpdateManager::Update( float elapsedSeconds, unsigned int lastVSync
   keepUpdating |= KeepUpdating::MONITORING_PERFORMANCE;
 #endif
 
-  // Only queue the message at the end of the update so that animation finished notifications are
-  // not fired off before the scene has actually been updated.
-  // TODO: implement better queueing mechanism from update-to-event thread.
-  if ( mImpl->animationFinishedDuringUpdate )
-  {
-    // The application should be notified by NotificationManager, in another thread
-    mImpl->notificationManager.QueueMessage( AnimationFinishedMessage( mImpl->animationFinishedNotifier ) );
-  }
-
   // The update has finished; swap the double-buffering indices
   mSceneGraphBuffers.Swap();
 
+  // tell the update manager that we're done so the queue can be given to event thread
+  mImpl->notificationManager.UpdateCompleted();
+
   PERF_MONITOR_END(PerformanceMonitor::UPDATE);
 
   return keepUpdating;