Optimized size negotiation 62/24162/1
authorKimmo Hoikka <kimmo.hoikka@samsung.com>
Fri, 13 Jun 2014 10:08:48 +0000 (11:08 +0100)
committerAdeel Kazmi <adeel.kazmi@samsung.com>
Tue, 8 Jul 2014 17:47:03 +0000 (18:47 +0100)
1. Remove size negotiation from scrollable things as they are not layout containers (yes, ItemView does not do size negotiations)

2. Make size negotiation containers members of relayout controller so we avoid hundreds of memory allocations per frame

3. Reduce the overhead of size negotiation request by having a static flag instead of hundreds of handle creations and dynamic casts etc

Change-Id: I9f11a45a70b262c0c6a07358350a8203c21fc2ce
Signed-off-by: Adeel Kazmi <adeel.kazmi@samsung.com>
base/dali-toolkit/internal/controls/relayout-controller-impl.cpp
base/dali-toolkit/internal/controls/relayout-controller-impl.h
base/dali-toolkit/internal/controls/relayout-controller.cpp
base/dali-toolkit/internal/controls/relayout-controller.h
base/dali-toolkit/internal/controls/scrollable/scrollable-impl.cpp
base/dali-toolkit/public-api/controls/control-impl.cpp
capi/dali-toolkit/public-api/controls/control-impl.h

index 8bfa297..e235d98 100644 (file)
 #include "relayout-controller-impl.h"
 
 // EXTERNAL INCLUDES
-
-#include <stack>
-#include <sstream>
 #include <dali/integration-api/debug.h>
+#if defined(DEBUG_ENABLED)
+#include <sstream>
+#endif // defined(DEBUG_ENABLED)
 
 // INTERNAL INCLUDES
-
-#include "dali-toolkit/public-api/controls/control.h"
-#include "dali-toolkit/public-api/controls/control-impl.h"
-#include "dali-toolkit/public-api/controls/text-view/text-view.h"
+#include <dali-toolkit/public-api/controls/text-view/text-view.h>
 
 namespace Dali
 {
@@ -37,9 +34,6 @@ namespace Dali
 namespace Toolkit
 {
 
-typedef std::pair< Control, Vector2 > ControlSizePair;
-typedef std::stack< ControlSizePair > ControlStack;
-
 namespace Internal
 {
 
@@ -139,7 +133,7 @@ void FindControls( Actor actor, ControlStack& controls, Vector2 size )
     // Only set the width and height if they are non zero.
     SetIfNotZero( size, controlSetSize );
 
-    controls.push( ControlSizePair( control, size ) );
+    controls.push_back( ControlSizePair( control, size ) );
   }
   else
   {
@@ -167,6 +161,15 @@ void PushToStack( ControlStack& controlStack, const ActorSizeContainer& containe
 
 } // unnamed namespace
 
+RelayoutControllerImpl::RelayoutControllerImpl( bool& relayoutFlag )
+: mRelayoutFlag( relayoutFlag ),
+  mRelayoutConnection( false )
+{
+  // make space for 32 controls to avoid having to copy construct a lot in the beginning
+  mControlStack.reserve( 32 );
+  mSizecontainer.reserve( 32 );
+}
+
 RelayoutControllerImpl::~RelayoutControllerImpl()
 {
 }
@@ -185,49 +188,40 @@ void RelayoutControllerImpl::Request()
 
 void RelayoutControllerImpl::Relayout()
 {
-  PRINT_HIERARCHY;
-
-  // 1. Finds all top-level controls from the root actor and allocate them the size of the stage
-  //    These controls are paired with the stage size and added to the stack.
-  ControlStack controlStack;
-  FindControls( Stage().GetCurrent().GetRootLayer(), controlStack, Stage::GetCurrent().GetSize() );
-
-  // 2. Iterate through the stack until it's empty.
-  while ( !controlStack.empty() )
+  // only do something when requested
+  if( mRelayoutFlag )
   {
-    ControlSizePair pair ( controlStack.top() );
-    Toolkit::Control control ( pair.first );
-    Vector2 size ( pair.second );
-    controlStack.pop();
+    // clear the flag as we're now doing the relayout
+    mRelayoutFlag = false;
+    PRINT_HIERARCHY;
 
-    DALI_LOG_INFO( gLogFilter, Debug::General, "Allocating %p (%.2f, %.2f)\n", control.GetObjectPtr(), size.width, size.height );
+    mControlStack.clear(); // we do not release memory, just empty the container
 
-    // 3. Negotiate the size with the current control. Pass it an empty container which the control
-    //    has to fill with all the actors it has not done any size negotiation for.
-    ActorSizeContainer container;
-    control.GetImplementation().NegotiateSize( size, container );
+    // 1. Finds all top-level controls from the root actor and allocate them the size of the stage
+    //    These controls are paired with the stage size and added to the stack.
+    FindControls( Stage().GetCurrent().GetRootLayer(), mControlStack, Stage::GetCurrent().GetSize() );
 
-    // 4. Push the controls from the actors in the container to the stack.
-    PushToStack( controlStack, container );
-  }
+    // 2. Iterate through the stack until it's empty.
+    while ( !mControlStack.empty() )
+    {
+      ControlSizePair pair ( mControlStack.back() );
+      Toolkit::Control control ( pair.first );
+      Vector2 size ( pair.second );
+      mControlStack.pop_back();
 
-  //Disconnect so that we relayout only when requested to do so.
-  Disconnect();
-}
+      DALI_LOG_INFO( gLogFilter, Debug::General, "Allocating %p (%.2f, %.2f)\n", control.GetObjectPtr(), size.width, size.height );
 
-void RelayoutControllerImpl::Disconnect()
-{
-  if( mRelayoutConnection )
-  {
-    Stage stage = Stage::GetCurrent();
-    stage.EventProcessingFinishedSignal().Disconnect( this, &RelayoutControllerImpl::Relayout );
-    mRelayoutConnection = false;
-  }
-}
+      mSizecontainer.clear();
+      // 3. Negotiate the size with the current control. Pass it an empty container which the control
+      //    has to fill with all the actors it has not done any size negotiation for.
+      control.GetImplementation().NegotiateSize( size, mSizecontainer );
 
-RelayoutControllerImpl::RelayoutControllerImpl()
-: mRelayoutConnection( false )
-{
+      // 4. Push the controls from the actors in the container to the stack.
+      PushToStack( mControlStack, mSizecontainer );
+    }
+  }
+  // should not disconnect the signal as that causes some control size negotiations to not work correctly
+  // this algorithm needs more optimization as well
 }
 
 } // namespace Internal
index 7f655c5..8339e99 100644 (file)
  *
  */
 
+// EXTERNAL INCLUDES
 #include <dali/dali.h>
+#include <dali/public-api/common/vector-wrapper.h>
+
+// INTERNAL INCLUDES
+#include <dali-toolkit/public-api/controls/control.h>
+#include <dali-toolkit/public-api/controls/control-impl.h>
 #include "relayout-controller.h"
 
 namespace Dali
@@ -32,6 +38,9 @@ namespace Internal
 
 class RelayoutController;
 
+typedef std::pair< Dali::Toolkit::Control, Vector2 > ControlSizePair;
+typedef std::vector< ControlSizePair > ControlStack;
+
 /**
  * @copydoc Toolkit::Internal::RelayoutController
  */
@@ -42,8 +51,9 @@ public:
   /**
    * Constructor.
    * We should only create a unique instance.
+   * @param relayoutFlag to avoid unnecessary calls inside a single frame
    */
-  RelayoutControllerImpl();
+  RelayoutControllerImpl( bool& relayoutFlag );
 
 
   /**
@@ -79,7 +89,11 @@ private:
 
 private:
 
-  bool mRelayoutConnection:1; ///< Whether EventProcessingFinishedSignal signal is connected.
+  bool& mRelayoutFlag;               ///< reference to relayout flag to avoid unnecessary calls
+  ControlStack mControlStack;        ///< stack for relayouting
+  ActorSizeContainer mSizecontainer; ///< size container
+  bool mRelayoutConnection:1;        ///< Whether EventProcessingFinishedSignal signal is connected.
+
 };
 
 } // namespace Internal
index c3a7097..8dce13a 100644 (file)
@@ -41,6 +41,14 @@ namespace Toolkit
 namespace Internal
 {
 
+namespace
+{
+// Flag to avoid doing more than one request per frame
+// getting the singleton handle can be expensive as it requires calling to adaptor, dynamic cast etc
+// and it can get called 100 times per frame easily in startup and new view initialization
+  bool gRelayoutRequestPending = false;
+}
+
 RelayoutController::RelayoutController()
 {
 
@@ -51,28 +59,33 @@ RelayoutController::~RelayoutController()
 
 }
 
-RelayoutController RelayoutController::Get()
+void RelayoutController::Request()
 {
-  RelayoutController controller;
-
-  // Check whether the RelayoutController is already created
-  Dali::Adaptor& adaptor = Dali::Adaptor::Get();
-  Dali::BaseHandle handle = adaptor.GetSingleton(typeid(RelayoutController));
-
-  if(handle)
+  // are we already going to do it this frame
+  if( !gRelayoutRequestPending )
   {
-    // If so, downcast the handle of singleton to RelayoutController
-    controller = RelayoutController(dynamic_cast<Internal::RelayoutControllerImpl*>(handle.GetObjectPtr()));
+    RelayoutController controller;
+
+    // Check whether the RelayoutController is already created
+    Dali::Adaptor& adaptor = Dali::Adaptor::Get();
+    Dali::BaseHandle handle = adaptor.GetSingleton(typeid(RelayoutController));
+
+    if(handle)
+    {
+      // If so, downcast the handle of singleton to RelayoutController
+      controller = RelayoutController(dynamic_cast<Internal::RelayoutControllerImpl*>(handle.GetObjectPtr()));
+    }
+
+    if(!controller)
+    {
+      // If not, create the RelayoutController and register it as a singleton
+      controller = RelayoutController( new Internal::RelayoutControllerImpl(gRelayoutRequestPending) );
+      adaptor.RegisterSingleton( typeid(controller), controller );
+    }
+
+    GetImpl(controller).Request();
+    gRelayoutRequestPending = true;
   }
-
-  if(!controller)
-  {
-    // If not, create the RelayoutController and register it as a singleton
-    controller = RelayoutController(new Internal::RelayoutControllerImpl());
-    adaptor.RegisterSingleton(typeid(controller), controller);
-  }
-
-  return controller;
 }
 
 RelayoutController::RelayoutController(Internal::RelayoutControllerImpl *impl)
@@ -80,12 +93,6 @@ RelayoutController::RelayoutController(Internal::RelayoutControllerImpl *impl)
 {
 }
 
-void RelayoutController::Request()
-{
-  GetImpl(*this).Request();
-}
-
-
 } // namespace Internal
 
 } // namespace Toolkit
index ec375e0..c9463e0 100644 (file)
@@ -51,19 +51,14 @@ public:
   virtual ~RelayoutController();
 
   /**
-   * Get the singleton of RelayoutController object.
-   * @return A handle to the RelayoutController.
-   */
-  static RelayoutController Get();
-
-  /**
    * Request to relayout.
    */
-  void Request();
+  static void Request();
 
 private:
 
-  RelayoutController(Internal::RelayoutControllerImpl *impl);
+  RelayoutController(Internal::RelayoutControllerImpl* impl);
+
 };
 
 } // namespace Internal
index 71e8be8..7b83883 100644 (file)
@@ -61,8 +61,10 @@ const Vector4     Scrollable::DEFAULT_OVERSHOOT_COLOUR(0.0f, 0.64f, 0.85f, 0.6f)
 // Scrollable
 ///////////////////////////////////////////////////////////////////////////////////////////////////
 
+// Scrollable controls are not layout containers so they dont need size negotiation..
+// we dont want size negotiation while scrolling if we can avoid it
 Scrollable::Scrollable()
-: Control( ControlBehaviour( REQUIRES_TOUCH_EVENTS | REQUIRES_STYLE_CHANGE_SIGNALS ) ),
+: Control( ControlBehaviour( REQUIRES_TOUCH_EVENTS | REQUIRES_STYLE_CHANGE_SIGNALS | NO_SIZE_NEGOTIATION ) ),
   mPropertyRelativePosition(Property::INVALID_INDEX),
   mPropertyPositionMin(Property::INVALID_INDEX),
   mPropertyPositionMax(Property::INVALID_INDEX),
index 9d64b1f..1e420b1 100644 (file)
@@ -778,7 +778,7 @@ void Control::OnChildAdd(Actor& child)
     return;
   }
 
-  // Request for relayout.
+  // Request for relayout as we may need to position the new child and old ones
   RelayoutRequest();
 
   // Notify derived classes.
@@ -793,7 +793,7 @@ void Control::OnChildRemove(Actor& child)
     return;
   }
 
-  // Request for relayout.
+  // Request for relayout as we may need to re-position the old child
   RelayoutRequest();
 
   // Notify derived classes.
@@ -1065,7 +1065,12 @@ void Control::ClearKeyInputFocus()
 
 void Control::RelayoutRequest()
 {
-  Internal::RelayoutController::Get().Request();
+  // unfortunate double negative but thats to guarantee new controls get size negotiation
+  // by default and have to "opt-out" if they dont want it
+  if( !(mImpl->mFlags & NO_SIZE_NEGOTIATION) )
+  {
+    Internal::RelayoutController::Request();
+  }
 }
 
 void Control::Relayout( Vector2 size, ActorSizeContainer& container )
index e989b77..6b52c92 100644 (file)
@@ -23,6 +23,9 @@
  * @{
  */
 
+// EXTERNAL INCLUDES
+#include <dali/public-api/common/vector-wrapper.h>
+
 // INTERNAL INCLUDES
 #include <dali-toolkit/public-api/controls/control.h>
 
@@ -548,9 +551,10 @@ protected: // Construction
   // Flags for the constructor
   enum ControlBehaviour
   {
-    CONTROL_BEHAVIOUR_NONE         = 0x0,
-    REQUIRES_TOUCH_EVENTS          = 0x1,     ///< True if the OnTouchEvent() callback is required.
-    REQUIRES_STYLE_CHANGE_SIGNALS  = 0x2      ///< True if needs to monitor style change signals such as theme/font change
+    CONTROL_BEHAVIOUR_NONE        = 0,
+    REQUIRES_TOUCH_EVENTS         = 1<<1,     ///< True if the OnTouchEvent() callback is required.
+    REQUIRES_STYLE_CHANGE_SIGNALS = 1<<2,     ///< True if needs to monitor style change signals such as theme/font change
+    NO_SIZE_NEGOTIATION           = 1<<3      ///< True if control does not need size negotiation, i.e. it can be skipped in the algorithm
   };
 
   /**
@@ -732,8 +736,6 @@ private:
    */
   bool EmitKeyEventSignal(const KeyEvent& event);
 
-
-
 private:
 
   // Undefined
@@ -741,7 +743,7 @@ private:
   Control& operator=(const Control&);
 
   class Impl;
-  Impl *mImpl;
+  ImplmImpl;
 
   friend class Internal::RelayoutControllerImpl;   ///< Relayout controller needs to call Relayout() which is private.
   friend class Internal::KeyInputFocusManager;     ///< KeyInputFocusManager needs to call which is private.