From 64da7865bdbb4c4332ccf226fef54e8ce86e22c6 Mon Sep 17 00:00:00 2001 From: Adeel Kazmi Date: Thu, 25 Oct 2018 16:13:51 +0100 Subject: [PATCH] (FrameCallback) Ensure setting a color is reset every frame - Added a property resetter so that the color value is reset to base value at the start of every frame. - Realised that the color was being set on the next frame so had to move calling the frame-callback earlier (i.e. before we go through the nodes). Change-Id: Id5debd0f0ee6d7fc0d994d5cae817f2fcf0c7f16 --- .../src/dali/utc-Dali-FrameCallbackInterface.cpp | 18 ++ dali/devel-api/update/update-proxy.cpp | 4 +- dali/devel-api/update/update-proxy.h | 4 +- .../update/manager/frame-callback-processor.cpp | 5 +- .../update/manager/frame-callback-processor.h | 7 +- .../update/manager/scene-graph-frame-callback.cpp | 4 +- .../update/manager/scene-graph-frame-callback.h | 5 +- dali/internal/update/manager/update-manager.cpp | 21 ++- dali/internal/update/manager/update-proxy-impl.cpp | 25 ++- dali/internal/update/manager/update-proxy-impl.h | 26 ++- .../manager/update-proxy-property-modifier.h | 201 +++++++++++++++++++++ 11 files changed, 292 insertions(+), 28 deletions(-) create mode 100644 dali/internal/update/manager/update-proxy-property-modifier.h diff --git a/automated-tests/src/dali/utc-Dali-FrameCallbackInterface.cpp b/automated-tests/src/dali/utc-Dali-FrameCallbackInterface.cpp index c932e02..d854148 100644 --- a/automated-tests/src/dali/utc-Dali-FrameCallbackInterface.cpp +++ b/automated-tests/src/dali/utc-Dali-FrameCallbackInterface.cpp @@ -381,6 +381,24 @@ int UtcDaliFrameCallbackSetters(void) DALI_TEST_EQUALS( actor.GetCurrentProperty( Actor::Property::COLOR ).Get< Vector4 >(), Color::WHITE, TEST_LOCATION ); DALI_TEST_EQUALS( actor.GetCurrentProperty( Actor::Property::SCALE ).Get< Vector3 >(), Vector3::ONE, TEST_LOCATION ); + // Render for a couple more frames to ensure the values are reset properly (some values are double-buffered) + + application.SendNotification(); + application.Render(); + + DALI_TEST_EQUALS( actor.GetCurrentProperty( Actor::Property::POSITION ).Get< Vector3 >(), Vector3::ZERO, TEST_LOCATION ); + DALI_TEST_EQUALS( actor.GetCurrentProperty( Actor::Property::SIZE ).Get< Vector3 >(), Vector3( actorSize ), TEST_LOCATION ); + DALI_TEST_EQUALS( actor.GetCurrentProperty( Actor::Property::COLOR ).Get< Vector4 >(), Color::WHITE, TEST_LOCATION ); + DALI_TEST_EQUALS( actor.GetCurrentProperty( Actor::Property::SCALE ).Get< Vector3 >(), Vector3::ONE, TEST_LOCATION ); + + application.SendNotification(); + application.Render(); + + DALI_TEST_EQUALS( actor.GetCurrentProperty( Actor::Property::POSITION ).Get< Vector3 >(), Vector3::ZERO, TEST_LOCATION ); + DALI_TEST_EQUALS( actor.GetCurrentProperty( Actor::Property::SIZE ).Get< Vector3 >(), Vector3( actorSize ), TEST_LOCATION ); + DALI_TEST_EQUALS( actor.GetCurrentProperty( Actor::Property::COLOR ).Get< Vector4 >(), Color::WHITE, TEST_LOCATION ); + DALI_TEST_EQUALS( actor.GetCurrentProperty( Actor::Property::SCALE ).Get< Vector3 >(), Vector3::ONE, TEST_LOCATION ); + END_TEST; } diff --git a/dali/devel-api/update/update-proxy.cpp b/dali/devel-api/update/update-proxy.cpp index 7480569..92b788c 100644 --- a/dali/devel-api/update/update-proxy.cpp +++ b/dali/devel-api/update/update-proxy.cpp @@ -79,12 +79,12 @@ bool UpdateProxy::GetColor( uint32_t id, Vector4& color ) const return mImpl.GetColor( id, color ); } -bool UpdateProxy::SetColor( uint32_t id, const Vector4& color ) const +bool UpdateProxy::SetColor( uint32_t id, const Vector4& color ) { return mImpl.SetColor( id, color ); } -bool UpdateProxy::BakeColor( uint32_t id, const Vector4& color ) const +bool UpdateProxy::BakeColor( uint32_t id, const Vector4& color ) { return mImpl.BakeColor( id, color ); } diff --git a/dali/devel-api/update/update-proxy.h b/dali/devel-api/update/update-proxy.h index 218739a..40be070 100644 --- a/dali/devel-api/update/update-proxy.h +++ b/dali/devel-api/update/update-proxy.h @@ -149,7 +149,7 @@ public: * @return Whether the method call was successful or not. * @note This will get reset to the internally calculated or previously baked value in the next frame, so will have to be set again. */ - bool SetColor( uint32_t id, const Vector4& color ) const; + bool SetColor( uint32_t id, const Vector4& color ); /** * @brief Allows baking an Actor's local color from the Frame callback function. @@ -158,7 +158,7 @@ public: * @return Whether the method call was successful or not. * @note The value is saved so will cause undesired effects if this property is being animated. */ - bool BakeColor( uint32_t id, const Vector4& color ) const; + bool BakeColor( uint32_t id, const Vector4& color ); public: // Not intended for application developers diff --git a/dali/internal/update/manager/frame-callback-processor.cpp b/dali/internal/update/manager/frame-callback-processor.cpp index 57459cf..5c713f2 100644 --- a/dali/internal/update/manager/frame-callback-processor.cpp +++ b/dali/internal/update/manager/frame-callback-processor.cpp @@ -34,8 +34,9 @@ namespace Internal namespace SceneGraph { -FrameCallbackProcessor::FrameCallbackProcessor( TransformManager& transformManager ) +FrameCallbackProcessor::FrameCallbackProcessor( UpdateManager& updateManager, TransformManager& transformManager ) : mFrameCallbacks(), + mUpdateManager( updateManager ), mTransformManager( transformManager ), mNodeHierarchyChanged( true ) { @@ -49,7 +50,7 @@ void FrameCallbackProcessor::AddFrameCallback( OwnerPointer< FrameCallback >& fr { Node& node = const_cast< Node& >( *rootNode ); // Was sent as const from event thread, we need to be able to use non-const version here. - frameCallback->ConnectToSceneGraph( mTransformManager, node ); + frameCallback->ConnectToSceneGraph( mUpdateManager, mTransformManager, node ); mFrameCallbacks.emplace_back( frameCallback ); } diff --git a/dali/internal/update/manager/frame-callback-processor.h b/dali/internal/update/manager/frame-callback-processor.h index 06f03c4..09f4acd 100644 --- a/dali/internal/update/manager/frame-callback-processor.h +++ b/dali/internal/update/manager/frame-callback-processor.h @@ -41,6 +41,7 @@ namespace SceneGraph class Node; class TransformManager; +class UpdateManager; /** * This class processes all the registered frame-callbacks. @@ -51,8 +52,10 @@ public: /** * Construct a new FrameCallbackProcessor. + * @param[in] updateManager A reference to the UpdateManager + * @param[in] transformManager A reference to the TransformManager */ - FrameCallbackProcessor( TransformManager& transformManager ); + FrameCallbackProcessor( UpdateManager& updateManager, TransformManager& transformManager ); /** * Non-virtual Destructor. @@ -98,6 +101,8 @@ private: std::vector< OwnerPointer< FrameCallback > > mFrameCallbacks; ///< A container of all the frame-callbacks & accompanying update-proxies. + UpdateManager& mUpdateManager; + TransformManager& mTransformManager; bool mNodeHierarchyChanged; ///< Set to true if the node hierarchy changes diff --git a/dali/internal/update/manager/scene-graph-frame-callback.cpp b/dali/internal/update/manager/scene-graph-frame-callback.cpp index 14cbe6f..faf6c73 100644 --- a/dali/internal/update/manager/scene-graph-frame-callback.cpp +++ b/dali/internal/update/manager/scene-graph-frame-callback.cpp @@ -54,9 +54,9 @@ FrameCallback::~FrameCallback() } } -void FrameCallback::ConnectToSceneGraph( TransformManager& transformManager, Node& rootNode ) +void FrameCallback::ConnectToSceneGraph( UpdateManager& updateManager, TransformManager& transformManager, Node& rootNode ) { - mUpdateProxy = std::unique_ptr< UpdateProxy >( new UpdateProxy( transformManager, rootNode ) ); + mUpdateProxy = std::unique_ptr< UpdateProxy >( new UpdateProxy( updateManager, transformManager, rootNode ) ); rootNode.AddObserver( *this ); } diff --git a/dali/internal/update/manager/scene-graph-frame-callback.h b/dali/internal/update/manager/scene-graph-frame-callback.h index ae801da..2f876e2 100644 --- a/dali/internal/update/manager/scene-graph-frame-callback.h +++ b/dali/internal/update/manager/scene-graph-frame-callback.h @@ -62,10 +62,11 @@ public: /** * Called from the update-thread when connecting to the scene-graph. - * @param[in] transformManager The transform Manager + * @param[in] updateManager The Update Manager + * @param[in] transformManager The Transform Manager * @param[in] rootNode The rootNode of this frame-callback */ - void ConnectToSceneGraph( TransformManager& transformManager, Node& rootNode ); + void ConnectToSceneGraph( UpdateManager& updateManager, TransformManager& transformManager, Node& rootNode ); // Movable but not copyable diff --git a/dali/internal/update/manager/update-manager.cpp b/dali/internal/update/manager/update-manager.cpp index 308b6a5..1c834fb 100644 --- a/dali/internal/update/manager/update-manager.cpp +++ b/dali/internal/update/manager/update-manager.cpp @@ -245,12 +245,13 @@ struct UpdateManager::Impl /** * Lazy init for FrameCallbackProcessor. + * @param[in] updateManager A reference to the update-manager */ - FrameCallbackProcessor& GetFrameCallbackProcessor() + FrameCallbackProcessor& GetFrameCallbackProcessor( UpdateManager& updateManager ) { if( ! frameCallbackProcessor ) { - frameCallbackProcessor = new FrameCallbackProcessor( transformManager ); + frameCallbackProcessor = new FrameCallbackProcessor( updateManager, transformManager ); } return *frameCallbackProcessor; } @@ -849,6 +850,12 @@ uint32_t UpdateManager::Update( float elapsedSeconds, } } + // Call the frame-callback-processor if set + if( mImpl->frameCallbackProcessor ) + { + mImpl->frameCallbackProcessor->Update( bufferIndex, elapsedSeconds ); + } + //Update node hierarchy, apply constraints and perform sorting / culling. //This will populate each Layer with a list of renderers which are ready. UpdateNodes( bufferIndex ); @@ -860,12 +867,6 @@ uint32_t UpdateManager::Update( float elapsedSeconds, //Update renderers and apply constraints UpdateRenderers( bufferIndex ); - // Call the frame-callback-processor if set - if( mImpl->frameCallbackProcessor ) - { - mImpl->frameCallbackProcessor->Update( bufferIndex, elapsedSeconds ); - } - //Update the transformations of all the nodes mImpl->transformManager.Update(); @@ -1076,12 +1077,12 @@ bool UpdateManager::IsDefaultSurfaceRectChanged() void UpdateManager::AddFrameCallback( OwnerPointer< FrameCallback >& frameCallback, const Node* rootNode ) { - mImpl->GetFrameCallbackProcessor().AddFrameCallback( frameCallback, rootNode ); + mImpl->GetFrameCallbackProcessor( *this ).AddFrameCallback( frameCallback, rootNode ); } void UpdateManager::RemoveFrameCallback( FrameCallbackInterface* frameCallback ) { - mImpl->GetFrameCallbackProcessor().RemoveFrameCallback( frameCallback ); + mImpl->GetFrameCallbackProcessor( *this ).RemoveFrameCallback( frameCallback ); } void UpdateManager::AddSampler( OwnerPointer< Render::Sampler >& sampler ) diff --git a/dali/internal/update/manager/update-proxy-impl.cpp b/dali/internal/update/manager/update-proxy-impl.cpp index 4763bc5..22b45a3 100644 --- a/dali/internal/update/manager/update-proxy-impl.cpp +++ b/dali/internal/update/manager/update-proxy-impl.cpp @@ -18,6 +18,9 @@ // CLASS HEADER #include +// INTERNAL INCLUDES +#include + namespace Dali { @@ -52,12 +55,14 @@ SceneGraph::Node* FindNodeInSceneGraph( uint32_t id, SceneGraph::Node& node ) } // unnamed namespace -UpdateProxy::UpdateProxy( SceneGraph::TransformManager& transformManager, SceneGraph::Node& rootNode ) +UpdateProxy::UpdateProxy( SceneGraph::UpdateManager& updateManager, SceneGraph::TransformManager& transformManager, SceneGraph::Node& rootNode ) : mNodeContainer(), mLastCachedIdNodePair( { 0u, NULL } ), mCurrentBufferIndex( 0u ), + mUpdateManager( updateManager ), mTransformManager( transformManager ), - mRootNode( rootNode ) + mRootNode( rootNode ), + mPropertyModifier( nullptr ) { } @@ -200,19 +205,21 @@ bool UpdateProxy::GetColor( uint32_t id, Vector4& color ) const return success; } -bool UpdateProxy::SetColor( uint32_t id, const Vector4& color ) const +bool UpdateProxy::SetColor( uint32_t id, const Vector4& color ) { bool success = false; SceneGraph::Node* node = GetNodeWithId( id ); if( node ) { node->mColor.Set( mCurrentBufferIndex, color ); + node->SetDirtyFlag( SceneGraph::NodePropertyFlags::COLOR ); + AddResetter( *node, node->mColor ); success = true; } return success; } -bool UpdateProxy::BakeColor( uint32_t id, const Vector4& color ) const +bool UpdateProxy::BakeColor( uint32_t id, const Vector4& color ) { bool success = false; SceneGraph::Node* node = GetNodeWithId( id ); @@ -228,6 +235,7 @@ void UpdateProxy::NodeHierarchyChanged() { mLastCachedIdNodePair = { 0u, NULL }; mNodeContainer.clear(); + mPropertyModifier.reset(); } SceneGraph::Node* UpdateProxy::GetNodeWithId( uint32_t id ) const @@ -267,6 +275,15 @@ SceneGraph::Node* UpdateProxy::GetNodeWithId( uint32_t id ) const return node; } +void UpdateProxy::AddResetter( SceneGraph::Node& node, SceneGraph::PropertyBase& propertyBase ) +{ + if( ! mPropertyModifier ) + { + mPropertyModifier = PropertyModifierPtr( new PropertyModifier( mUpdateManager ) ); + } + mPropertyModifier->AddResetter( node, propertyBase ); +} + } // namespace Internal } // namespace Dali diff --git a/dali/internal/update/manager/update-proxy-impl.h b/dali/internal/update/manager/update-proxy-impl.h index 44d97f6..88fefbb 100644 --- a/dali/internal/update/manager/update-proxy-impl.h +++ b/dali/internal/update/manager/update-proxy-impl.h @@ -20,6 +20,7 @@ // EXTERNAL INCLUDES #include +#include // INTERNAL INCLUDES #include @@ -35,6 +36,11 @@ namespace Dali namespace Internal { +namespace SceneGraph +{ +class UpdateManager; +} + /** * @brief The implementation of Dali::UpdateProxy. * @@ -48,10 +54,11 @@ public: /** * @brief Constructor. + * @param[in] updateManager Ref to the UpdateManager in order to add property resetters * @param[in] transformManager Ref to the TransformManager in order to set/get transform properties of nodes * @param[in] rootNode The root node for this proxy */ - UpdateProxy( SceneGraph::TransformManager& transformManager, SceneGraph::Node& rootNode ); + UpdateProxy( SceneGraph::UpdateManager& updateManager, SceneGraph::TransformManager& transformManager, SceneGraph::Node& rootNode ); /** * @brief Destructor. @@ -123,12 +130,12 @@ public: /** * @copydoc Dali::UpdateProxy::SetColor() */ - bool SetColor( uint32_t id, const Vector4& color ) const; + bool SetColor( uint32_t id, const Vector4& color ); /** * @copydoc Dali::UpdateProxy::BakeColor() */ - bool BakeColor( uint32_t id, const Vector4& color ) const; + bool BakeColor( uint32_t id, const Vector4& color ); /** * @brief Retrieves the root-node used by this class @@ -163,6 +170,13 @@ private: */ SceneGraph::Node* GetNodeWithId( uint32_t id ) const; + /** + * @brief Adds a property-resetter for non-transform properties so that they can be reset to their base value every frame. + * @param[in] node The node the property belongs to + * @param[in] propertyBase The property itself + */ + void AddResetter( SceneGraph::Node& node, SceneGraph::PropertyBase& propertyBase ); + private: /** @@ -174,12 +188,18 @@ private: SceneGraph::Node* node; ///< The node itself }; + class PropertyModifier; + using PropertyModifierPtr = std::unique_ptr< PropertyModifier >; + mutable std::vector< IdNodePair > mNodeContainer; ///< Used to store cached pointers to already searched for Nodes. mutable IdNodePair mLastCachedIdNodePair; ///< Used to cache the last retrieved id-node pair. BufferIndex mCurrentBufferIndex; + SceneGraph::UpdateManager& mUpdateManager; ///< Reference to the Update Manager. SceneGraph::TransformManager& mTransformManager; ///< Reference to the Transform Manager. SceneGraph::Node& mRootNode; ///< The root node of this update proxy. + + PropertyModifierPtr mPropertyModifier; ///< To ensure non-transform property modifications reset to base values. }; } // namespace Internal diff --git a/dali/internal/update/manager/update-proxy-property-modifier.h b/dali/internal/update/manager/update-proxy-property-modifier.h new file mode 100644 index 0000000..1ac572d --- /dev/null +++ b/dali/internal/update/manager/update-proxy-property-modifier.h @@ -0,0 +1,201 @@ +#ifndef DALI_INTERNAL_UPDATE_PROXY_PROPERTY_MODIFIER_H +#define DALI_INTERNAL_UPDATE_PROXY_PROPERTY_MODIFIER_H + +/* + * Copyright (c) 2018 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 +#include +#include +#include + +namespace Dali +{ + +namespace Internal +{ + +namespace SceneGraph +{ +class Node; +class PropertyBase; +} + +/** + * Keeps track of any non-transform manager properties that are modified by the UpdateProxy. + * + * This is required so the Update Manager can then reset the value to the base at the start of every frame. + */ +class UpdateProxy::PropertyModifier final +{ +public: + + using Resetter = SceneGraph::Resetter< PropertyModifier >; + + /** + * Observer to determine when the animator is no longer present + */ + class LifecycleObserver + { + public: + + /** + * Called shortly before the animator is destroyed. + */ + virtual void ObjectDestroyed() = 0; + + protected: + + /** + * Virtual destructor, no deletion through this interface + */ + virtual ~LifecycleObserver() = default; + }; + + /** + * Default Constructor. + * @param[in] updateManager A reference to the update-manager + */ + PropertyModifier( SceneGraph::UpdateManager& updateManager ) + : mProperties(), + mLifecycleObservers(), + mUpdateManager( &updateManager ) + { + } + + /** + * Non-virtual destructor. + */ + ~PropertyModifier() + { + for( auto& observer : mLifecycleObservers ) + { + observer->ObjectDestroyed(); + } + } + + // Movable but not copyable + + PropertyModifier( const PropertyModifier& ) = delete; ///< Deleted copy constructor. + PropertyModifier& operator=( const PropertyModifier& ) = delete; ///< Deleted assignment operator. + + /** + * Move constructor. + */ + PropertyModifier( PropertyModifier&& other ) + : mProperties( std::move( other.mProperties ) ), + mLifecycleObservers( std::move( other.mLifecycleObservers ) ), + mUpdateManager( std::move( other.mUpdateManager ) ) + { + // Clear other so that it does not remove any resetters unintentionally + other.mLifecycleObservers.clear(); + } + + /** + * Move assignment operator. + */ + PropertyModifier& operator=( PropertyModifier&& other ) + { + if( this != &other ) + { + mProperties = std::move( other.mProperties ); + mLifecycleObservers = std::move( other.mLifecycleObservers ); + mUpdateManager = std::move( other.mUpdateManager ); + + // Clear other so that it does not remove any resetters unintentionally + other.mLifecycleObservers.clear(); + } + return *this; + } + + /** + * Allows Resetter to track the life-cycle of this object. + * @param[in] observer The observer to add. + */ + void AddLifecycleObserver( LifecycleObserver& observer ) + { + mLifecycleObservers.push_back( &observer ); + } + + /** + * The Resetter no longer needs to track the life-cycle of this object. + * @param[in] observer The observer that to remove. + */ + void RemoveLifecycleObserver( LifecycleObserver& observer ) + { + std::remove( mLifecycleObservers.begin(), mLifecycleObservers.end(), &observer ); + } + + /** + * Adds a resetter to the given node and property if it hasn't already been added previously. + * @param[in] node The associated Node + * @param[in] propertyBase The associated PropertyBase + */ + void AddResetter( SceneGraph::Node& node, SceneGraph::PropertyBase& propertyBase ) + { + // Check if we've already added a resetter for this node and property to the update-manager + NodePropertyPair pair{ &node, &propertyBase }; + if( mUpdateManager && + ( mProperties.end() == std::find( mProperties.begin(), mProperties.end(), pair ) ) ) + { + // We haven't, add the pair to our container to ensure we don't add it again + // Then create a Resetter which will observe the life of this object + // Finally, add the resetter to the Update-Manager + // When this object is destroyed, the resetter will be informed and will automatically be removed + + mProperties.emplace_back( std::move( pair ) ); + OwnerPointer< SceneGraph::PropertyResetterBase > resetter( Resetter::New( node, propertyBase, *this ) ); + mUpdateManager->AddPropertyResetter( resetter ); + } + } + +public: + + /** + * Structure to store the Node & property-base pair + */ + struct NodePropertyPair + { + SceneGraph::Node* node; + SceneGraph::PropertyBase* propertyBase; + + NodePropertyPair( const NodePropertyPair& ) = delete; ///< Deleted copy constructor. + NodePropertyPair( NodePropertyPair&& ) = default; ///< Default move constructor. + NodePropertyPair& operator=( const NodePropertyPair& ) = delete; ///< Deleted assignment operator. + NodePropertyPair& operator=( NodePropertyPair&& ) = default; ///< Default move assignment operator. + + /** + * Comparison operator + */ + bool operator==( const NodePropertyPair& other ) + { + return ( other.node == node ) && + ( other.propertyBase == propertyBase ); + } + }; + + std::vector< NodePropertyPair > mProperties; + std::vector< LifecycleObserver* > mLifecycleObservers; + SceneGraph::UpdateManager* mUpdateManager; +}; + +} // namespace Internal + +} // namespace Dali + +#endif // DALI_INTERNAL_UPDATE_PROXY_PROPERTY_MODIFIER_H -- 2.7.4