From 63ca57aca66eafc6b4daaac7c60fcad31e598804 Mon Sep 17 00:00:00 2001 From: Agnelo Vaz Date: Fri, 29 Jun 2018 12:02:03 +0100 Subject: [PATCH] Prevent Control from positioning controls with margin/padding 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 --- .../src/dali-toolkit/utc-Dali-Layouting.cpp | 223 ++++++++++++++++++++- .../devel-api/layouting/layout-item-impl.cpp | 5 +- dali-toolkit/public-api/controls/control-impl.cpp | 47 +++-- 3 files changed, 255 insertions(+), 20 deletions(-) diff --git a/automated-tests/src/dali-toolkit/utc-Dali-Layouting.cpp b/automated-tests/src/dali-toolkit/utc-Dali-Layouting.cpp index 5ba721b..714a4eb 100644 --- a/automated-tests/src/dali-toolkit/utc-Dali-Layouting.cpp +++ b/automated-tests/src/dali-toolkit/utc-Dali-Layouting.cpp @@ -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( 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( 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( 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( 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( 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( 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( 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( 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( 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( 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( 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( 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( 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( 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( 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) diff --git a/dali-toolkit/devel-api/layouting/layout-item-impl.cpp b/dali-toolkit/devel-api/layouting/layout-item-impl.cpp index 4213cd8..b5e7720 100644 --- a/dali-toolkit/devel-api/layouting/layout-item-impl.cpp +++ b/dali-toolkit/devel-api/layouting/layout-item-impl.cpp @@ -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; } diff --git a/dali-toolkit/public-api/controls/control-impl.cpp b/dali-toolkit/public-api/controls/control-impl.cpp index df78b15..8a79594 100755 --- a/dali-toolkit/public-api/controls/control-impl.cpp +++ b/dali-toolkit/public-api/controls/control-impl.cpp @@ -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( ownerActor.GetProperty( Dali::Actor::Property::LAYOUT_DIRECTION ).Get() ); + // 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( ownerActor.GetProperty( Dali::Actor::Property::LAYOUT_DIRECTION ).Get() ); - // 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 ); -- 2.7.4