(FrameCallback) Ensure setting a color is reset every frame 32/191932/3
authorAdeel Kazmi <adeel.kazmi@samsung.com>
Thu, 25 Oct 2018 15:13:51 +0000 (16:13 +0100)
committerAdeel Kazmi <adeel.kazmi@samsung.com>
Tue, 30 Oct 2018 14:57:45 +0000 (14:57 +0000)
- 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

automated-tests/src/dali/utc-Dali-FrameCallbackInterface.cpp
dali/devel-api/update/update-proxy.cpp
dali/devel-api/update/update-proxy.h
dali/internal/update/manager/frame-callback-processor.cpp
dali/internal/update/manager/frame-callback-processor.h
dali/internal/update/manager/scene-graph-frame-callback.cpp
dali/internal/update/manager/scene-graph-frame-callback.h
dali/internal/update/manager/update-manager.cpp
dali/internal/update/manager/update-proxy-impl.cpp
dali/internal/update/manager/update-proxy-impl.h
dali/internal/update/manager/update-proxy-property-modifier.h [new file with mode: 0644]

index c932e02..d854148 100644 (file)
@@ -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;
 }
 
index 7480569..92b788c 100644 (file)
@@ -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 );
 }
index 218739a..40be070 100644 (file)
@@ -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
 
index 57459cf..5c713f2 100644 (file)
@@ -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 );
 }
index 06f03c4..09f4acd 100644 (file)
@@ -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
index 14cbe6f..faf6c73 100644 (file)
@@ -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 );
 }
 
index ae801da..2f876e2 100644 (file)
@@ -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
 
index 308b6a5..1c834fb 100644 (file)
@@ -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 )
index 4763bc5..22b45a3 100644 (file)
@@ -18,6 +18,9 @@
 // CLASS HEADER
 #include <dali/internal/update/manager/update-proxy-impl.h>
 
+// INTERNAL INCLUDES
+#include <dali/internal/update/manager/update-proxy-property-modifier.h>
+
 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
index 44d97f6..88fefbb 100644 (file)
@@ -20,6 +20,7 @@
 
 // EXTERNAL INCLUDES
 #include <cstdint>
+#include <memory>
 
 // INTERNAL INCLUDES
 #include <dali/public-api/common/vector-wrapper.h>
@@ -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 (file)
index 0000000..1ac572d
--- /dev/null
@@ -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 <dali/internal/common/owner-pointer.h>
+#include <dali/internal/update/common/property-resetter.h>
+#include <dali/internal/update/manager/update-manager.h>
+#include <dali/internal/update/manager/update-proxy-impl.h>
+
+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