Skip to content

Commit

Permalink
Revert of Add cross axis alignment for BoxLayout. (https://codereview…
Browse files Browse the repository at this point in the history 
….chromium.org/333403005/)

Reason for revert:
Lots of unitialized reads appeared related to GetPreferredSize(). This is the only views cl in http://build.chromium.org/p/chromium.memory.fyi/builders/Chromium%20OS%20%28valgrind%29%283%29/builds/32878/ , so reverting somewhat sepculatively.

Original issue's description:
> Add cross axis alignment for BoxLayout.
> 
> This CL adds a CrossAxisAlignment setting to BoxLayout which functions
> like the CSS align-items property. This property aligns the children
> items of a view in the non-orientation axis of the BoxLayout.
> 
> BUG=386475
> 
> Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=279257

TBR=benwells@chromium.org,sky@chromium.org,calamity@chromium.org
NOTREECHECKS=true
NOTRY=true
BUG=386475

Review URL: https://codereview.chromium.org/358523003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@279542 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
thakis@chromium.org committed Jun 24, 2014
1 parent 7f9706c commit 91877f0
Show file tree
Hide file tree
Showing 6 changed files with 40 additions and 224 deletions.
4 changes: 1 addition & 3 deletions ui/app_list/views/start_page_view.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ const int kTopMargin = 30;
const int kInstantContainerSpacing = 20;

// WebView constants.
const int kWebViewWidth = 500;
const int kWebViewWidth = 200;
const int kWebViewHeight = 105;

// DummySearchBoxView constants.
Expand Down Expand Up @@ -130,8 +130,6 @@ void StartPageView::InitInstantContainer() {
gfx::Insets(kTopMargin, 0, kInstantContainerSpacing, 0));
instant_layout_manager->set_main_axis_alignment(
views::BoxLayout::MAIN_AXIS_ALIGNMENT_END);
instant_layout_manager->set_cross_axis_alignment(
views::BoxLayout::CROSS_AXIS_ALIGNMENT_CENTER);
instant_container_->SetLayoutManager(instant_layout_manager);

views::View* web_view = view_delegate_->CreateStartPageWebView(
Expand Down
103 changes: 30 additions & 73 deletions ui/views/layout/box_layout.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,7 @@ BoxLayout::BoxLayout(BoxLayout::Orientation orientation,
inside_border_vertical_spacing,
inside_border_horizontal_spacing),
between_child_spacing_(between_child_spacing),
main_axis_alignment_(MAIN_AXIS_ALIGNMENT_START),
cross_axis_alignment_(CROSS_AXIS_ALIGNMENT_STRETCH) {
main_axis_alignment_(MAIN_AXIS_ALIGNMENT_START) {
}

BoxLayout::~BoxLayout() {
Expand All @@ -39,8 +38,13 @@ void BoxLayout::Layout(View* host) {
View* child = host->child_at(i);
if (!child->visible())
continue;
total_main_axis_size += MainAxisSizeForView(child, child_area.width()) +
between_child_spacing_;
if (orientation_ == kHorizontal) {
total_main_axis_size +=
child->GetPreferredSize().width() + between_child_spacing_;
} else {
total_main_axis_size += child->GetHeightForWidth(child_area.width()) +
between_child_spacing_;
}
++num_visible;
}

Expand Down Expand Up @@ -72,28 +76,21 @@ void BoxLayout::Layout(View* host) {
}
}

int main_position = MainAxisPosition(child_area);
int x = child_area.x();
int y = child_area.y();
for (int i = 0; i < host->child_count(); ++i) {
View* child = host->child_at(i);
if (child->visible()) {
gfx::Rect bounds(child_area);
SetMainAxisPosition(main_position, &bounds);
if (cross_axis_alignment_ != CROSS_AXIS_ALIGNMENT_STRETCH) {
int free_space = CrossAxisSize(bounds) - CrossAxisSizeForView(child);
int position = CrossAxisPosition(bounds);
if (cross_axis_alignment_ == CROSS_AXIS_ALIGNMENT_CENTER) {
position += free_space / 2;
} else if (cross_axis_alignment_ == CROSS_AXIS_ALIGNMENT_END) {
position += free_space;
}
SetCrossAxisPosition(position, &bounds);
SetCrossAxisSize(CrossAxisSizeForView(child), &bounds);
gfx::Rect bounds(x, y, child_area.width(), child_area.height());
if (orientation_ == kHorizontal) {
bounds.set_width(child->GetPreferredSize().width() + padding);
if (bounds.width() > 0)
x += bounds.width() + between_child_spacing_;
} else {
bounds.set_height(child->GetHeightForWidth(bounds.width()) + padding);
if (bounds.height() > 0)
y += bounds.height() + between_child_spacing_;
}
int child_main_axis_size = MainAxisSizeForView(child, child_area.width());
SetMainAxisSize(child_main_axis_size + padding, &bounds);
if (MainAxisSize(bounds) > 0)
main_position += MainAxisSize(bounds) + between_child_spacing_;

// Clamp child view bounds to |child_area|.
bounds.Intersect(child_area);
child->SetBoundsRect(bounds);
Expand Down Expand Up @@ -122,64 +119,26 @@ int BoxLayout::GetPreferredHeightForWidth(const View* host, int width) const {
return GetPreferredSizeForChildWidth(host, child_width).height();
}

int BoxLayout::MainAxisSize(const gfx::Rect& rect) const {
return orientation_ == kHorizontal ? rect.width() : rect.height();
int BoxLayout::MainAxisSize(const gfx::Rect& child_area) const {
return orientation_ == kHorizontal ? child_area.width() : child_area.height();
}

int BoxLayout::MainAxisPosition(const gfx::Rect& rect) const {
return orientation_ == kHorizontal ? rect.x() : rect.y();
int BoxLayout::MainAxisPosition(const gfx::Rect& child_area) const {
return orientation_ == kHorizontal ? child_area.x() : child_area.y();
}

void BoxLayout::SetMainAxisSize(int size, gfx::Rect* rect) const {
void BoxLayout::SetMainAxisSize(int size, gfx::Rect* child_area) const {
if (orientation_ == kHorizontal)
rect->set_width(size);
child_area->set_width(size);
else
rect->set_height(size);
child_area->set_height(size);
}

void BoxLayout::SetMainAxisPosition(int position, gfx::Rect* rect) const {
void BoxLayout::SetMainAxisPosition(int position, gfx::Rect* child_area) const {
if (orientation_ == kHorizontal)
rect->set_x(position);
else
rect->set_y(position);
}

int BoxLayout::CrossAxisSize(const gfx::Rect& rect) const {
return orientation_ == kVertical ? rect.width() : rect.height();
}

int BoxLayout::CrossAxisPosition(const gfx::Rect& rect) const {
return orientation_ == kVertical ? rect.x() : rect.y();
}

void BoxLayout::SetCrossAxisSize(int size, gfx::Rect* rect) const {
if (orientation_ == kVertical)
rect->set_width(size);
else
rect->set_height(size);
}

void BoxLayout::SetCrossAxisPosition(int position, gfx::Rect* rect) const {
if (orientation_ == kVertical)
rect->set_x(position);
child_area->set_x(position);
else
rect->set_y(position);
}

int BoxLayout::MainAxisSizeForView(const View* view,
int child_area_width) const {
return orientation_ == kHorizontal
? view->GetPreferredSize().width()
: view->GetHeightForWidth(cross_axis_alignment_ ==
CROSS_AXIS_ALIGNMENT_STRETCH
? child_area_width
: view->GetPreferredSize().width());
}

int BoxLayout::CrossAxisSizeForView(const View* view) const {
return orientation_ == kVertical
? view->GetPreferredSize().width()
: view->GetHeightForWidth(view->GetPreferredSize().width());
child_area->set_y(position);
}

gfx::Size BoxLayout::GetPreferredSizeForChildWidth(const View* host,
Expand Down Expand Up @@ -211,9 +170,7 @@ gfx::Size BoxLayout::GetPreferredSizeForChildWidth(const View* host,
if (!child->visible())
continue;

// Use the child area width for getting the height if the child is
// supposed to stretch. Use its preferred size otherwise.
int extra_height = MainAxisSizeForView(child, child_area_width);
int extra_height = child->GetHeightForWidth(child_area_width);
// Only add |between_child_spacing_| if this is not the only child.
if (height != 0 && extra_height > 0)
height += between_child_spacing_;
Expand Down
47 changes: 8 additions & 39 deletions ui/views/layout/box_layout.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,16 +47,8 @@ class VIEWS_EXPORT BoxLayout : public LayoutManager {
// in-between the child views.
};

// This specifies where along the cross axis the children should be laid out.
// e.g. a horizontal layout of CROSS_AXIS_ALIGNMENT_END will result in the
// child views being bottom-aligned.
enum CrossAxisAlignment {
// This causes the child view to stretch to fit the host in the cross axis.
CROSS_AXIS_ALIGNMENT_STRETCH,
CROSS_AXIS_ALIGNMENT_START,
CROSS_AXIS_ALIGNMENT_CENTER,
CROSS_AXIS_ALIGNMENT_END,
};
// TODO(calamity): Add CrossAxisAlignment property to allow cross axis
// alignment.

// Use |inside_border_horizontal_spacing| and
// |inside_border_vertical_spacing| to add additional space between the child
Expand All @@ -72,10 +64,6 @@ class VIEWS_EXPORT BoxLayout : public LayoutManager {
main_axis_alignment_ = main_axis_alignment;
}

void set_cross_axis_alignment(CrossAxisAlignment cross_axis_alignment) {
cross_axis_alignment_ = cross_axis_alignment;
}

void set_inside_border_insets(const gfx::Insets& insets) {
inside_border_insets_ = insets;
}
Expand All @@ -87,28 +75,13 @@ class VIEWS_EXPORT BoxLayout : public LayoutManager {
int width) const OVERRIDE;

private:
// Returns the size and position along the main axis of |rect|.
int MainAxisSize(const gfx::Rect& rect) const;
int MainAxisPosition(const gfx::Rect& rect) const;

// Sets the size and position along the main axis of |rect|.
void SetMainAxisSize(int size, gfx::Rect* rect) const;
void SetMainAxisPosition(int position, gfx::Rect* rect) const;
// Returns the size and position along the main axis of |child_area|.
int MainAxisSize(const gfx::Rect& child_area) const;
int MainAxisPosition(const gfx::Rect& child_area) const;

// Returns the size and position along the cross axis of |rect|.
int CrossAxisSize(const gfx::Rect& rect) const;
int CrossAxisPosition(const gfx::Rect& rect) const;

// Sets the size and position along the cross axis of |rect|.
void SetCrossAxisSize(int size, gfx::Rect* rect) const;
void SetCrossAxisPosition(int size, gfx::Rect* rect) const;

// Returns the main axis size for the given view. |child_area_width| is needed
// to calculate the height of the view when the orientation is vertical.
int MainAxisSizeForView(const View* view, int child_area_width) const;

// Returns the cross axis size for the given view.
int CrossAxisSizeForView(const View* view) const;
// Sets the size and position along the main axis of |child_area|.
void SetMainAxisSize(int size, gfx::Rect* child_area) const;
void SetMainAxisPosition(int position, gfx::Rect* child_area) const;

// The preferred size for the dialog given the width of the child area.
gfx::Size GetPreferredSizeForChildWidth(const View* host,
Expand All @@ -130,10 +103,6 @@ class VIEWS_EXPORT BoxLayout : public LayoutManager {
// MAIN_AXIS_ALIGNMENT_START by default.
MainAxisAlignment main_axis_alignment_;

// The alignment of children in the cross axis. This is
// CROSS_AXIS_ALIGNMENT_STRETCH by default.
CrossAxisAlignment cross_axis_alignment_;

DISALLOW_IMPLICIT_CONSTRUCTORS(BoxLayout);
};

Expand Down
97 changes: 1 addition & 96 deletions ui/views/layout/box_layout_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -162,8 +162,7 @@ TEST_F(BoxLayoutTest, UseHeightForWidth) {
layout_.reset(new BoxLayout(BoxLayout::kVertical, 0, 0, 0));
View* v1 = new StaticSizedView(gfx::Size(20, 10));
host_->AddChildView(v1);
ProportionallySizedView* v2 = new ProportionallySizedView(2);
v2->set_preferred_width(10);
View* v2 = new ProportionallySizedView(2);
host_->AddChildView(v2);
EXPECT_EQ(gfx::Size(20, 50), layout_->GetPreferredSize(host_.get()));

Expand All @@ -173,18 +172,6 @@ TEST_F(BoxLayoutTest, UseHeightForWidth) {
EXPECT_EQ(gfx::Rect(0, 10, 20, 40), v2->bounds());

EXPECT_EQ(110, layout_->GetPreferredHeightForWidth(host_.get(), 50));

// Test without horizontal stretching of the views.
layout_->set_cross_axis_alignment(BoxLayout::CROSS_AXIS_ALIGNMENT_END);
EXPECT_EQ(gfx::Size(20, 30).ToString(),
layout_->GetPreferredSize(host_.get()).ToString());

host_->SetBounds(0, 0, 20, 30);
layout_->Layout(host_.get());
EXPECT_EQ(gfx::Rect(0, 0, 20, 10), v1->bounds());
EXPECT_EQ(gfx::Rect(10, 10, 10, 20), v2->bounds());

EXPECT_EQ(30, layout_->GetPreferredHeightForWidth(host_.get(), 50));
}

TEST_F(BoxLayoutTest, EmptyPreferredSize) {
Expand Down Expand Up @@ -279,86 +266,4 @@ TEST_F(BoxLayoutTest, MainAxisAlignmentVertical) {
EXPECT_EQ(gfx::Rect(10, 80, 20, 10).ToString(), v2->bounds().ToString());
}

TEST_F(BoxLayoutTest, CrossAxisAlignmentHorizontal) {
layout_.reset(new BoxLayout(BoxLayout::kHorizontal, 10, 10, 10));

View* v1 = new StaticSizedView(gfx::Size(20, 20));
host_->AddChildView(v1);
View* v2 = new StaticSizedView(gfx::Size(10, 10));
host_->AddChildView(v2);

host_->SetBounds(0, 0, 100, 60);

// Stretch children to fill the available height by default.
layout_->Layout(host_.get());
EXPECT_EQ(gfx::Rect(10, 10, 20, 40).ToString(), v1->bounds().ToString());
EXPECT_EQ(gfx::Rect(40, 10, 10, 40).ToString(), v2->bounds().ToString());

// Ensure same results for CROSS_AXIS_ALIGNMENT_STRETCH.
layout_->set_cross_axis_alignment(BoxLayout::CROSS_AXIS_ALIGNMENT_STRETCH);
layout_->Layout(host_.get());
EXPECT_EQ(gfx::Rect(10, 10, 20, 40).ToString(), v1->bounds().ToString());
EXPECT_EQ(gfx::Rect(40, 10, 10, 40).ToString(), v2->bounds().ToString());

// Aligns children to the start vertically.
layout_->set_cross_axis_alignment(BoxLayout::CROSS_AXIS_ALIGNMENT_START);
layout_->Layout(host_.get());
EXPECT_EQ(gfx::Rect(10, 10, 20, 20).ToString(), v1->bounds().ToString());
EXPECT_EQ(gfx::Rect(40, 10, 10, 10).ToString(), v2->bounds().ToString());

// Aligns children to the center vertically.
layout_->set_cross_axis_alignment(BoxLayout::CROSS_AXIS_ALIGNMENT_CENTER);
layout_->Layout(host_.get());
EXPECT_EQ(gfx::Rect(10, 20, 20, 20).ToString(), v1->bounds().ToString());
EXPECT_EQ(gfx::Rect(40, 25, 10, 10).ToString(), v2->bounds().ToString());

// Aligns children to the end of the host vertically, accounting for the
// inside border spacing.
layout_->set_cross_axis_alignment(BoxLayout::CROSS_AXIS_ALIGNMENT_END);
layout_->Layout(host_.get());
EXPECT_EQ(gfx::Rect(10, 30, 20, 20).ToString(), v1->bounds().ToString());
EXPECT_EQ(gfx::Rect(40, 40, 10, 10).ToString(), v2->bounds().ToString());
}

TEST_F(BoxLayoutTest, CrossAxisAlignmentVertical) {
layout_.reset(new BoxLayout(BoxLayout::kVertical, 10, 10, 10));

View* v1 = new StaticSizedView(gfx::Size(20, 20));
host_->AddChildView(v1);
View* v2 = new StaticSizedView(gfx::Size(10, 10));
host_->AddChildView(v2);

host_->SetBounds(0, 0, 60, 100);

// Stretch children to fill the available width by default.
layout_->Layout(host_.get());
EXPECT_EQ(gfx::Rect(10, 10, 40, 20).ToString(), v1->bounds().ToString());
EXPECT_EQ(gfx::Rect(10, 40, 40, 10).ToString(), v2->bounds().ToString());

// Ensure same results for CROSS_AXIS_ALIGNMENT_STRETCH.
layout_->set_cross_axis_alignment(BoxLayout::CROSS_AXIS_ALIGNMENT_STRETCH);
layout_->Layout(host_.get());
EXPECT_EQ(gfx::Rect(10, 10, 40, 20).ToString(), v1->bounds().ToString());
EXPECT_EQ(gfx::Rect(10, 40, 40, 10).ToString(), v2->bounds().ToString());

// Aligns children to the start horizontally.
layout_->set_cross_axis_alignment(BoxLayout::CROSS_AXIS_ALIGNMENT_START);
layout_->Layout(host_.get());
EXPECT_EQ(gfx::Rect(10, 10, 20, 20).ToString(), v1->bounds().ToString());
EXPECT_EQ(gfx::Rect(10, 40, 10, 10).ToString(), v2->bounds().ToString());

// Aligns children to the center horizontally.
layout_->set_cross_axis_alignment(BoxLayout::CROSS_AXIS_ALIGNMENT_CENTER);
layout_->Layout(host_.get());
EXPECT_EQ(gfx::Rect(20, 10, 20, 20).ToString(), v1->bounds().ToString());
EXPECT_EQ(gfx::Rect(25, 40, 10, 10).ToString(), v2->bounds().ToString());

// Aligns children to the end of the host horizontally, accounting for the
// inside border spacing.
layout_->set_cross_axis_alignment(BoxLayout::CROSS_AXIS_ALIGNMENT_END);
layout_->Layout(host_.get());
EXPECT_EQ(gfx::Rect(30, 10, 20, 20).ToString(), v1->bounds().ToString());
EXPECT_EQ(gfx::Rect(40, 40, 10, 10).ToString(), v2->bounds().ToString());
}

} // namespace views
6 changes: 0 additions & 6 deletions ui/views/test/test_views.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,4 @@ int ProportionallySizedView::GetHeightForWidth(int w) const {
return w * factor_;
}

gfx::Size ProportionallySizedView::GetPreferredSize() const {
if (preferred_width_ >= 0)
return gfx::Size(preferred_width_, GetHeightForWidth(preferred_width_));
return View::GetPreferredSize();
}

} // namespace views
Loading

0 comments on commit 91877f0

Please sign in to comment.