Prevent Control from positioning controls with margin/padding 20/182920/5
authorAgnelo Vaz <agnelo.vaz@samsung.com>
Fri, 29 Jun 2018 11:02:03 +0000 (12:02 +0100)
committerAgnelo Vaz <agnelo.vaz@samsung.com>
Fri, 29 Jun 2018 15:35:01 +0000 (16:35 +0100)
Fixes the issue in which adding margins or padding to a Layout causes
the child control's Relayout to re-position itself using
this margin/padding.
This should be done by the layoutgroup not in Control.

Change-Id: Ic50b94f2d6c9503313ca93bfc7de413129e0824b

automated-tests/src/dali-toolkit/utc-Dali-Layouting.cpp
dali-toolkit/devel-api/layouting/layout-item-impl.cpp
dali-toolkit/public-api/controls/control-impl.cpp

index 5ba721b..714a4eb 100644 (file)
@@ -790,7 +790,7 @@ int UtcDaliLayouting_HboxLayout_Padding02(void)
 int UtcDaliLayouting_HboxLayout_Padding03(void)
 {
   ToolkitTestApplication application;
-  tet_infoline("UtcDaliLayouting_HboxLayout_Padding03 - Adding Changing padding on a single child");
+  tet_infoline("UtcDaliLayouting_HboxLayout_Padding03 - Changing padding on a single child");
 
   Stage stage = Stage::GetCurrent();
   auto hbox = Control::New();
@@ -871,6 +871,227 @@ int UtcDaliLayouting_HboxLayout_Padding03(void)
   END_TEST;
 }
 
+int UtcDaliLayouting_HboxLayout_Padding04(void)
+{
+  ToolkitTestApplication application;
+  tet_infoline("UtcDaliLayouting_HboxLayout_Padding04 - Adding Padding to the hbox");
+
+  // Adding padding to the layout should offset the positioning of the children.
+
+  const Extents LAYOUT_PADDING = Extents(5, 10, 20, 2 );
+  const Size CONTROL_SIZE = Size( 40, 40 );
+
+  Stage stage = Stage::GetCurrent();
+  // Create a root layout, ideally Dali would have a default layout in the root layer.
+  // Without this root layer the LinearLayout (or any other layout) will not
+  // honour WIDTH_SPECIFICATION or HEIGHT_SPECIFICATION settings.
+  // It uses the default stage size and ideally should have a layout added to it.
+  auto rootLayoutControl = Control::New();
+  rootLayoutControl.SetName( "AbsoluteLayout");
+  auto rootLayout = AbsoluteLayout::New();
+  DevelControl::SetLayout( rootLayoutControl, rootLayout );
+  rootLayoutControl.SetAnchorPoint( AnchorPoint::CENTER );
+  rootLayoutControl.SetParentOrigin( ParentOrigin::CENTER );
+  stage.Add( rootLayoutControl );
+
+  auto hbox = Control::New();
+  auto hboxLayout = LinearLayout::New();
+  hboxLayout.SetOrientation( LinearLayout::Orientation::HORIZONTAL );
+  DevelControl::SetLayout( hbox, hboxLayout );
+  hbox.SetName( "HBox");
+  hbox.SetProperty(Toolkit::Control::Property::PADDING, LAYOUT_PADDING );
+  hbox.SetProperty( Toolkit::LayoutItem::ChildProperty::WIDTH_SPECIFICATION, ChildLayoutData::WRAP_CONTENT );
+  hbox.SetProperty( Toolkit::LayoutItem::ChildProperty::HEIGHT_SPECIFICATION, ChildLayoutData::WRAP_CONTENT );
+
+  std::vector< Control > controls;
+  controls.push_back( CreateLeafControl( CONTROL_SIZE.width, CONTROL_SIZE.height ) );
+  controls.push_back( CreateLeafControl( CONTROL_SIZE.width, CONTROL_SIZE.height ) );
+  controls.push_back( CreateLeafControl( CONTROL_SIZE.width, CONTROL_SIZE.height ) );
+  controls.push_back( CreateLeafControl( CONTROL_SIZE.width, CONTROL_SIZE.height ) );
+
+  for( auto&& iter : controls )
+  {
+    hbox.Add( iter );
+  }
+
+  hbox.SetParentOrigin( ParentOrigin::CENTER );
+  hbox.SetAnchorPoint( AnchorPoint::CENTER );
+  rootLayoutControl.Add( hbox );
+
+  // Ensure layouting happens
+  application.SendNotification();
+  application.Render();
+
+  // Extra update needed to Relayout one more time. Catches any position updates, false positive without this seen.
+  application.SendNotification();
+
+  // hbox centers elements vertically, it fills test harness stage, which is 480x800.
+  // hbox left justifies elements
+  tet_infoline("Test Child Actor Position");
+
+  auto controlXPosition=0.0f;
+
+  controlXPosition = LAYOUT_PADDING.start;  // First child positioned at offset defined by the padding
+  DALI_TEST_EQUALS( controls[0].GetProperty<Vector3>( Actor::Property::POSITION ), Vector3( LAYOUT_PADDING.start,
+                                                                                            LAYOUT_PADDING.top,
+                                                                                            0.0f ), 0.0001f, TEST_LOCATION );
+
+  controlXPosition+=CONTROL_SIZE.width; // Second child positioned is the position of the first child + the first child's width.
+  DALI_TEST_EQUALS( controls[1].GetProperty<Vector3>( Actor::Property::POSITION ), Vector3( controlXPosition,
+                                                                                            LAYOUT_PADDING.top,
+                                                                                            0.0f ),
+                                                                                            0.0001f, TEST_LOCATION );
+
+  controlXPosition+=CONTROL_SIZE.width; // Third child positioned adjacent to second
+  DALI_TEST_EQUALS( controls[2].GetProperty<Vector3>( Actor::Property::POSITION ), Vector3( controlXPosition,
+                                                                                            LAYOUT_PADDING.top,
+                                                                                            0.0f ), 0.0001f, TEST_LOCATION );
+
+  controlXPosition+=CONTROL_SIZE.width; // Forth passed adjacent to the third
+  DALI_TEST_EQUALS( controls[3].GetProperty<Vector3>( Actor::Property::POSITION ), Vector3( controlXPosition,
+                                                                                            LAYOUT_PADDING.top,
+                                                                                            0.0f ), 0.0001f, TEST_LOCATION );
+
+  auto totalControlsWidth = CONTROL_SIZE.width * controls.size();
+  auto totalControlsHeight = CONTROL_SIZE.height;
+
+  DALI_TEST_EQUALS( hbox.GetProperty<Vector3>( Actor::Property::SIZE ), Vector3( totalControlsWidth + LAYOUT_PADDING.start + LAYOUT_PADDING.end,
+                                                                                 totalControlsHeight + LAYOUT_PADDING.top + LAYOUT_PADDING.bottom,
+                                                                                 0.0f ), 0.0001f, TEST_LOCATION );
+
+
+  END_TEST;
+}
+
+int UtcDaliLayouting_HboxLayout_Padding05(void)
+{
+  ToolkitTestApplication application;
+  tet_infoline("UtcDaliLayouting_HboxLayout_Padding05 - Changing the hbox Padding");
+
+  // Adding padding to the layout should offset the positioning of the children.
+
+  const Extents LAYOUT_PADDING = Extents(5, 10, 20, 2 );
+  const Size CONTROL_SIZE = Size( 40, 40 );
+
+  Stage stage = Stage::GetCurrent();
+  // Create a root layout, ideally Dali would have a default layout in the root layer.
+  // Without this root layer the LinearLayout (or any other layout) will not
+  // honour WIDTH_SPECIFICATION or HEIGHT_SPECIFICATION settings.
+  // It uses the default stage size and ideally should have a layout added to it.
+  auto rootLayoutControl = Control::New();
+  rootLayoutControl.SetName( "AbsoluteLayout");
+  auto rootLayout = AbsoluteLayout::New();
+  DevelControl::SetLayout( rootLayoutControl, rootLayout );
+  rootLayoutControl.SetAnchorPoint( AnchorPoint::CENTER );
+  rootLayoutControl.SetParentOrigin( ParentOrigin::CENTER );
+  stage.Add( rootLayoutControl );
+
+  auto hbox = Control::New();
+  auto hboxLayout = LinearLayout::New();
+  hboxLayout.SetOrientation( LinearLayout::Orientation::HORIZONTAL );
+  DevelControl::SetLayout( hbox, hboxLayout );
+  hbox.SetName( "HBox");
+  hbox.SetProperty(Toolkit::Control::Property::PADDING, LAYOUT_PADDING );
+  hbox.SetProperty( Toolkit::LayoutItem::ChildProperty::WIDTH_SPECIFICATION, ChildLayoutData::WRAP_CONTENT );
+  hbox.SetProperty( Toolkit::LayoutItem::ChildProperty::HEIGHT_SPECIFICATION, ChildLayoutData::WRAP_CONTENT );
+
+  std::vector< Control > controls;
+  controls.push_back( CreateLeafControl( CONTROL_SIZE.width, CONTROL_SIZE.height ) );
+  controls.push_back( CreateLeafControl( CONTROL_SIZE.width, CONTROL_SIZE.height ) );
+  controls.push_back( CreateLeafControl( CONTROL_SIZE.width, CONTROL_SIZE.height ) );
+  controls.push_back( CreateLeafControl( CONTROL_SIZE.width, CONTROL_SIZE.height ) );
+
+  for( auto&& iter : controls )
+  {
+    hbox.Add( iter );
+  }
+
+  hbox.SetParentOrigin( ParentOrigin::CENTER );
+  hbox.SetAnchorPoint( AnchorPoint::CENTER );
+  rootLayoutControl.Add( hbox );
+
+  // Ensure layouting happens
+  application.SendNotification();
+  application.Render();
+
+  // Extra update needed to Relayout one more time. Catches any position updates, false positive without this seen.
+  application.SendNotification();
+
+  // hbox centers elements vertically, it fills test harness stage, which is 480x800.
+  // hbox left justifies elements
+  tet_infoline("Test Child Actor Position");
+
+  auto controlXPosition=0.0f;
+
+  controlXPosition = LAYOUT_PADDING.start;  // First child positioned at offset defined by the padding
+  DALI_TEST_EQUALS( controls[0].GetProperty<Vector3>( Actor::Property::POSITION ), Vector3( LAYOUT_PADDING.start,
+                                                                                            LAYOUT_PADDING.top,
+                                                                                            0.0f ), 0.0001f, TEST_LOCATION );
+
+  controlXPosition+=CONTROL_SIZE.width; // Second child positioned is the position of the first child + the first child's width.
+  DALI_TEST_EQUALS( controls[1].GetProperty<Vector3>( Actor::Property::POSITION ), Vector3( controlXPosition,
+                                                                                            LAYOUT_PADDING.top,
+                                                                                            0.0f ),
+                                                                                            0.0001f, TEST_LOCATION );
+
+  controlXPosition+=CONTROL_SIZE.width; // Third child positioned adjacent to second
+  DALI_TEST_EQUALS( controls[2].GetProperty<Vector3>( Actor::Property::POSITION ), Vector3( controlXPosition,
+                                                                                            LAYOUT_PADDING.top,
+                                                                                            0.0f ), 0.0001f, TEST_LOCATION );
+
+  controlXPosition+=CONTROL_SIZE.width; // Forth passed adjacent to the third
+  DALI_TEST_EQUALS( controls[3].GetProperty<Vector3>( Actor::Property::POSITION ), Vector3( controlXPosition,
+                                                                                            LAYOUT_PADDING.top,
+                                                                                            0.0f ), 0.0001f, TEST_LOCATION );
+
+  auto totalControlsWidth = CONTROL_SIZE.width * controls.size();
+  auto totalControlsHeight = CONTROL_SIZE.height;
+
+  DALI_TEST_EQUALS( hbox.GetProperty<Vector3>( Actor::Property::SIZE ), Vector3( totalControlsWidth + LAYOUT_PADDING.start + LAYOUT_PADDING.end,
+                                                                                 totalControlsHeight + LAYOUT_PADDING.top + LAYOUT_PADDING.bottom,
+                                                                                 0.0f ), 0.0001f, TEST_LOCATION );
+
+  // Change layout padding
+  const Extents NEW_LAYOUT_PADDING = Extents(5, 20, 10, 2 );
+  tet_printf( "\nChanging Padding to control at index 1 \n" );
+  hbox.SetProperty(Toolkit::Control::Property::PADDING, NEW_LAYOUT_PADDING );
+
+  // Ensure layouting happens
+  application.SendNotification();
+  application.Render();
+
+  // Extra update needed to Relayout one more time. Catches any position updates, false positive without this seen.
+  application.SendNotification();
+
+  controlXPosition = NEW_LAYOUT_PADDING.start;  // First child positioned at offset defined by the padding
+  DALI_TEST_EQUALS( controls[0].GetProperty<Vector3>( Actor::Property::POSITION ), Vector3( NEW_LAYOUT_PADDING.start,
+                                                                                            NEW_LAYOUT_PADDING.top,
+                                                                                            0.0f ), 0.0001f, TEST_LOCATION );
+
+  controlXPosition+=CONTROL_SIZE.width; // Second child positioned is the position of the first child + the first child's width.
+  DALI_TEST_EQUALS( controls[1].GetProperty<Vector3>( Actor::Property::POSITION ), Vector3( controlXPosition,
+                                                                                            NEW_LAYOUT_PADDING.top,
+                                                                                            0.0f ),
+                                                                                            0.0001f, TEST_LOCATION );
+
+  controlXPosition+=CONTROL_SIZE.width; // Third child positioned adjacent to second
+  DALI_TEST_EQUALS( controls[2].GetProperty<Vector3>( Actor::Property::POSITION ), Vector3( controlXPosition,
+                                                                                            NEW_LAYOUT_PADDING.top,
+                                                                                            0.0f ), 0.0001f, TEST_LOCATION );
+
+  controlXPosition+=CONTROL_SIZE.width; // Forth passed adjacent to the third
+  DALI_TEST_EQUALS( controls[3].GetProperty<Vector3>( Actor::Property::POSITION ), Vector3( controlXPosition,
+                                                                                            NEW_LAYOUT_PADDING.top,
+                                                                                            0.0f ), 0.0001f, TEST_LOCATION );
+  totalControlsWidth = CONTROL_SIZE.width * controls.size();
+  totalControlsHeight = CONTROL_SIZE.height;
+
+  DALI_TEST_EQUALS( hbox.GetProperty<Vector3>( Actor::Property::SIZE ), Vector3( totalControlsWidth + NEW_LAYOUT_PADDING.start + NEW_LAYOUT_PADDING.end,
+                                                                                 totalControlsHeight + NEW_LAYOUT_PADDING.top + NEW_LAYOUT_PADDING.bottom,
+                                                                                 0.0f ), 0.0001f, TEST_LOCATION );
+  END_TEST;
+}
+
 // Margin Tests
 
 int UtcDaliLayouting_HboxLayout_Margin01(void)
index 4213cd8..b5e7720 100644 (file)
@@ -414,7 +414,7 @@ bool LayoutItem::SetFrame( LayoutLength left, LayoutLength top, LayoutLength rig
 {
   bool changed = false;
 
-  DALI_LOG_INFO( gLayoutFilter, Debug::Verbose, "LayoutItem::SetFrame(%d, %d, %d, %d)\n", left.mValue, top.mValue, right.mValue, bottom.mValue );
+  DALI_LOG_INFO( gLayoutFilter, Debug::Verbose, "LayoutItem::SetFrame enter(%d, %d, %d, %d)\n", left.mValue, top.mValue, right.mValue, bottom.mValue );
 
   if( mImpl->mLeft != left || mImpl->mRight != right || mImpl->mTop != top || mImpl->mBottom != bottom )
   {
@@ -462,6 +462,9 @@ bool LayoutItem::SetFrame( LayoutLength left, LayoutLength top, LayoutLength rig
       SizeChange( LayoutSize( newWidth, newHeight ), LayoutSize( oldWidth, oldHeight ) );
     }
   }
+
+  DALI_LOG_INFO( gLayoutFilter, Debug::Verbose, "LayoutItem::SetFrame  exit(%d, %d, %d, %d)\n", left.mValue, top.mValue, right.mValue, bottom.mValue );
+
   return changed;
 }
 
index df78b15..8a79594 100755 (executable)
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2017 Samsung Electronics Co., Ltd.
+ * 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.
@@ -696,33 +696,44 @@ void Control::OnRelayout( const Vector2& size, RelayoutContainer& container )
     Actor child = Self().GetChildAt( i );
     Vector2 newChildSize( size );
 
-    // When set the padding or margin on the control, child should be resized and repositioned.
+    // When setting the padding or margin on the control child should be resized and repositioned for legacy reasons.
     if( ( mImpl->mPadding.start != 0 ) || ( mImpl->mPadding.end != 0 ) || ( mImpl->mPadding.top != 0 ) || ( mImpl->mPadding.bottom != 0 ) ||
         ( mImpl->mMargin.start != 0 ) || ( mImpl->mMargin.end != 0 ) || ( mImpl->mMargin.top != 0 ) || ( mImpl->mMargin.bottom != 0 ) )
     {
-      Extents padding = mImpl->mPadding;
-
-      Dali::CustomActor ownerActor(GetOwner());
-      Dali::LayoutDirection::Type layoutDirection = static_cast<Dali::LayoutDirection::Type>( ownerActor.GetProperty( Dali::Actor::Property::LAYOUT_DIRECTION ).Get<int>() );
+      // Cannot use childs Position property as it can already have margin applied on it,
+      // so we end up cumulatively applying them over and over again.
+      Toolkit::Control childControl = Toolkit::Control::DownCast( child );
 
-      if( Dali::LayoutDirection::RIGHT_TO_LEFT == layoutDirection )
+      // If control not a LayoutItem layout then must be the old Relayout algorithm hence account
+      // for margins and padding.
+      // Padding is incorrect but may have to keep this functionality for compatibility.
+      if ( childControl && ! Toolkit::DevelControl::GetLayout( childControl ) )
       {
-        std::swap( padding.start, padding.end );
-      }
+        Extents padding = mImpl->mPadding;
 
-      newChildSize.width = size.width - ( padding.start + padding.end );
-      newChildSize.height = size.height - ( padding.top + padding.bottom );
+        Dali::CustomActor ownerActor(GetOwner());
+        Dali::LayoutDirection::Type layoutDirection = static_cast<Dali::LayoutDirection::Type>( ownerActor.GetProperty( Dali::Actor::Property::LAYOUT_DIRECTION ).Get<int>() );
 
-      // Cannot use childs Position property as it can already have padding and margin applied on it,
-      // so we end up cumulatively applying them over and over again.
-      Vector2 childOffset( 0.f, 0.f );
-      childOffset.x += ( mImpl->mMargin.start + padding.start );
-      childOffset.y += ( mImpl->mMargin.top + padding.top );
+        if( Dali::LayoutDirection::RIGHT_TO_LEFT == layoutDirection )
+        {
+          std::swap( padding.start, padding.end );
+        }
+
+        // Child size should include padding, this is the wrong use of padding but kept for compatibility.
+        newChildSize.width = size.width - ( padding.start + padding.end );
+        newChildSize.height = size.height - ( padding.top + padding.bottom );
 
-      child.SetPosition( childOffset.x, childOffset.y );
+        // Cannot use childs Position property as it can already have padding and margin applied on it,
+        // so we end up cumulatively applying them over and over again.
+        Vector2 childOffset( 0.f, 0.f );
+        childOffset.x += ( mImpl->mMargin.start + padding.start );
+        childOffset.y += ( mImpl->mMargin.top + padding.top );
+
+        child.SetPosition( childOffset.x, childOffset.y );
+      }
     }
 
-    container.Add( child, newChildSize );
+    container.Add( child, size );
   }
 
   Toolkit::Visual::Base visual = mImpl->GetVisual( Toolkit::Control::Property::BACKGROUND );