From: Kimmo Hoikka Date: Tue, 24 Jun 2014 17:22:36 +0000 (+0100) Subject: Prevent event thread from picking up notifications too soon X-Git-Tag: dali_1.0.0~20 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=4c0a88c52820b7471e15465ac7caf3c7165a0790;p=platform%2Fcore%2Fuifw%2Fdali-core.git Prevent event thread from picking up notifications too soon [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 --- diff --git a/automated-tests/src/dali/tct-dali-core.h b/automated-tests/src/dali/tct-dali-core.h index 85e58e4..a2c2f6a 100644 --- a/automated-tests/src/dali/tct-dali-core.h +++ b/automated-tests/src/dali/tct-dali-core.h @@ -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}, diff --git a/automated-tests/src/dali/utc-Dali-Animation.cpp b/automated-tests/src/dali/utc-Dali-Animation.cpp index bdf7ad2..537abb8 100644 --- a/automated-tests/src/dali/utc-Dali-Animation.cpp +++ b/automated-tests/src/dali/utc-Dali-Animation.cpp @@ -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( index, UpdateManagerTestConstraint( application ) ); + actor.ApplyConstraint( constraint ); + + // Apply animation to actor + BounceFunc func(0.0f, 0.0f, -100.0f); + animation.Animate( Property(actor, Actor::POSITION), func, AlphaFunctions::Linear ); + + animation.Play(); + + application.SendNotification(); + application.UpdateOnly( 16 ); + + finishCheck.CheckSignalNotReceived(); + + application.SendNotification(); // Process events + + finishCheck.CheckSignalReceived(); + + END_TEST; +} + + diff --git a/dali/internal/event/common/notification-manager.cpp b/dali/internal/event/common/notification-manager.cpp index b38eda4..dbebbec 100644 --- a/dali/internal/event/common/notification-manager.cpp +++ b/dali/internal/event/common/notification-manager.cpp @@ -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 diff --git a/dali/internal/event/common/notification-manager.h b/dali/internal/event/common/notification-manager.h index aa4912e..23b2e4e 100644 --- a/dali/internal/event/common/notification-manager.h +++ b/dali/internal/event/common/notification-manager.h @@ -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. */ diff --git a/dali/internal/update/manager/update-manager.cpp b/dali/internal/update/manager/update-manager.cpp index a76536f..0a94ddf 100644 --- a/dali/internal/update/manager/update-manager.cpp +++ b/dali/internal/update/manager/update-manager.cpp @@ -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;