Commit a75d28c4 authored by Sammie Quon's avatar Sammie Quon Committed by Commit Bot

splitview: Fix rounded corners glitch on drag indicators.

There is a glitch when the drag indicators slide in or out. The corners
stretch or squash a bit during animation before going back to normal
after animation ends.

This is known issue, since we animate the entire rect by applying
transform. The transform effects the rounded corners as well. The
transform is removed and the bounds are set at the end of animation,
so that the glitch is only seen during animation.

This patch fixes that by changing the indicators to not use a single rounded
rect anymore, but instead to use a new class which has two rounded
rect and a solid rect between them, with the solid rect
 covering the rounded corners so the final element looks like one
big rounded rect. On bounds changed, the two rounded
rects will get translated only, therefore preserving the rounded
edges, while the middle section will be the one shrinking or expanding.

There is some tricky logic to position the components of the new
view due to the animation and rtl, so added new test cases.

Test: covered by tests, manually all orientations and rtl
Bug: 824860
Change-Id: I183fa3de4e0a481ad2c7a208feec33a286505ad8
Reviewed-on: https://chromium-review.googlesource.com/1029191
Commit-Queue: Sammie Quon <sammiequon@chromium.org>
Reviewed-by: default avatarXiaoqian Dai <xdai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556831}
parent 6123a89c
......@@ -1078,6 +1078,8 @@ component("ash") {
"wm/splitview/split_view_divider.h",
"wm/splitview/split_view_drag_indicators.cc",
"wm/splitview/split_view_drag_indicators.h",
"wm/splitview/split_view_highlight_view.cc",
"wm/splitview/split_view_highlight_view.h",
"wm/splitview/split_view_utils.cc",
"wm/splitview/split_view_utils.h",
"wm/stacking_controller.cc",
......@@ -1780,6 +1782,7 @@ test("ash_unittests") {
"wm/screen_pinning_controller_unittest.cc",
"wm/session_state_animator_impl_unittest.cc",
"wm/splitview/split_view_controller_unittest.cc",
"wm/splitview/split_view_highlight_view_unittest.cc",
"wm/stacking_controller_unittest.cc",
"wm/system_gesture_event_filter_unittest.cc",
"wm/system_modal_container_layout_manager_unittest.cc",
......@@ -2088,6 +2091,8 @@ static_library("test_support_common") {
"wm/cursor_manager_test_api.h",
"wm/lock_state_controller_test_api.cc",
"wm/lock_state_controller_test_api.h",
"wm/splitview/split_view_highlight_view_test_api.cc",
"wm/splitview/split_view_highlight_view_test_api.h",
"wm/test_activation_delegate.cc",
"wm/test_activation_delegate.h",
"wm/test_child_modal_parent.cc",
......
// Copyright 2018 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "ash/wm/splitview/split_view_highlight_view.h"
#include "ash/wm/overview/rounded_rect_view.h"
#include "ash/wm/splitview/split_view_utils.h"
#include "ui/gfx/canvas.h"
#include "ui/views/view.h"
namespace ash {
namespace {
// The amount of round applied to the corners of the highlight views.
constexpr int kHighlightScreenRoundRectRadiusDp = 4;
constexpr int kRoundRectPaddingDp = 10;
gfx::Transform CalculateTransformFromRects(const gfx::Rect& src,
const gfx::Rect& dst,
bool landscape) {
// In portrait, rtl will have no effect on this view.
const bool is_rtl = base::i18n::IsRTL() && landscape;
const bool should_scale =
src.width() != dst.width() || src.height() != dst.height();
// Add a translatation. In rtl, translate in the opposite direction to account
// for the flip.
gfx::Transform transform;
transform.Translate(is_rtl && !should_scale ? src.origin() - dst.origin()
: dst.origin() - src.origin());
if (should_scale) {
// In rtl a extra translation needs to be added to account for the flipped
// scaling.
if (is_rtl) {
int x_translation = 0;
if ((src.x() > dst.x() && src.width() < dst.width()) ||
(src.x() == dst.x() && src.width() > dst.width())) {
x_translation = std::abs(dst.width() - src.width());
} else {
x_translation = -std::abs(dst.width() - src.width());
}
transform.Translate(gfx::Vector2d(x_translation, 0));
}
transform.Scale(
static_cast<float>(dst.width()) / static_cast<float>(src.width()),
static_cast<float>(dst.height()) / static_cast<float>(src.height()));
}
return transform;
}
} // namespace
SplitViewHighlightView::SplitViewHighlightView(bool is_right_or_bottom)
: is_right_or_bottom_(is_right_or_bottom) {
left_top_ =
new RoundedRectView(kHighlightScreenRoundRectRadiusDp, SK_ColorWHITE);
right_bottom_ =
new RoundedRectView(kHighlightScreenRoundRectRadiusDp, SK_ColorWHITE);
left_top_->SetPaintToLayer();
left_top_->layer()->SetFillsBoundsOpaquely(false);
right_bottom_->SetPaintToLayer();
right_bottom_->layer()->SetFillsBoundsOpaquely(false);
middle_ = new views::View();
middle_->SetPaintToLayer(ui::LAYER_SOLID_COLOR);
middle_->layer()->SetColor(SK_ColorWHITE);
AddChildView(left_top_);
AddChildView(right_bottom_);
AddChildView(middle_);
}
SplitViewHighlightView::~SplitViewHighlightView() = default;
void SplitViewHighlightView::SetBounds(const gfx::Rect& bounds,
bool landscape,
bool animate) {
if (bounds == this->bounds() && landscape == landscape_)
return;
landscape_ = landscape;
const gfx::Rect old_bounds = this->bounds();
const gfx::Vector2d offset = old_bounds.origin() - bounds.origin();
SetBoundsRect(bounds);
// Shift the bounds of the right bottom view if needed before applying the
// transform.
const bool slides_from_right = base::i18n::IsRTL() && landscape
? !is_right_or_bottom_
: is_right_or_bottom_;
if (slides_from_right && animate && !offset.IsZero()) {
gfx::Rect old_left_top_bounds = left_top_->bounds();
gfx::Rect old_right_middle_bounds = right_bottom_->bounds();
gfx::Rect old_middle_bounds = middle_->bounds();
old_left_top_bounds.Offset(offset);
old_right_middle_bounds.Offset(offset);
old_middle_bounds.Offset(offset);
left_top_->SetBoundsRect(old_left_top_bounds);
right_bottom_->SetBoundsRect(old_right_middle_bounds);
middle_->SetBoundsRect(old_middle_bounds);
}
// Calculate the new bounds. The middle should take as much space as possible,
// and the other two should take just enough space so they can display rounded
// corners.
gfx::Rect left_top_bounds, right_bottom_bounds;
gfx::Rect middle_bounds = bounds;
// The thickness of the two outer views should be the amount of rounding, plus
// a little padding. There will be some overlap to simply the code (we use a
// rectangle that is rounded on all sides, but cover half the sides instead of
// creating a new class that is only rounded on half the sides).
const int thickness = kHighlightScreenRoundRectRadiusDp + kRoundRectPaddingDp;
if (landscape) {
left_top_bounds = gfx::Rect(0, 0, thickness, bounds.height());
right_bottom_bounds = left_top_bounds;
right_bottom_bounds.Offset(bounds.width() - thickness, 0);
middle_bounds.Offset(-bounds.x(), -bounds.y());
middle_bounds.Inset(kHighlightScreenRoundRectRadiusDp, 0);
} else {
left_top_bounds = gfx::Rect(0, 0, bounds.width(), thickness);
right_bottom_bounds = left_top_bounds;
right_bottom_bounds.Offset(0, bounds.height() - thickness);
middle_bounds.Offset(-bounds.x(), -bounds.y());
middle_bounds.Inset(0, kHighlightScreenRoundRectRadiusDp);
}
left_top_bounds = GetMirroredRect(left_top_bounds);
right_bottom_bounds = GetMirroredRect(right_bottom_bounds);
middle_bounds = GetMirroredRect(middle_bounds);
// If |animate|, calculate the needed transform from old bounds to new bounds
// and apply it. Otherwise set the new bounds and reset the transforms on all
// items.
if (animate) {
DoSplitviewTransformAnimation(
middle_->layer(), SPLITVIEW_ANIMATION_PREVIEW_AREA_SLIDE_IN_OUT,
CalculateTransformFromRects(middle_->bounds(), middle_bounds,
landscape),
nullptr);
DoSplitviewTransformAnimation(
left_top_->layer(), SPLITVIEW_ANIMATION_PREVIEW_AREA_SLIDE_IN_OUT,
CalculateTransformFromRects(left_top_->bounds(), left_top_bounds,
landscape),
nullptr);
DoSplitviewTransformAnimation(
right_bottom_->layer(), SPLITVIEW_ANIMATION_PREVIEW_AREA_SLIDE_IN_OUT,
CalculateTransformFromRects(right_bottom_->bounds(),
right_bottom_bounds, landscape),
nullptr);
} else {
left_top_->layer()->SetTransform(gfx::Transform());
right_bottom_->layer()->SetTransform(gfx::Transform());
middle_->layer()->SetTransform(gfx::Transform());
left_top_->SetBoundsRect(left_top_bounds);
right_bottom_->SetBoundsRect(right_bottom_bounds);
middle_->SetBoundsRect(middle_bounds);
}
}
void SplitViewHighlightView::SetColor(SkColor color) {
left_top_->SetBackgroundColor(color);
right_bottom_->SetBackgroundColor(color);
middle_->layer()->SetColor(color);
}
} // namespace ash
\ No newline at end of file
// Copyright 2018 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef ASH_WM_SPLITVIEW_SPLIT_VIEW_HIGHLIGHT_VIEW_H_
#define ASH_WM_SPLITVIEW_SPLIT_VIEW_HIGHLIGHT_VIEW_H_
#include "ash/ash_export.h"
#include "ui/views/view.h"
namespace ash {
class RoundedRectView;
class SplitViewHighlightViewTestApi;
// View that is used for displaying and animating the highlights which tell
// users where to drag windows to enter splitview, and previews the space which
// a snapped window will occupy. It is a view consisting of a rectangle with
// rounded corners on the left or top, a rectangle in the middle and a rectangle
// with rounded corners on the right or bottom. It is done this way to ensure
// rounded corners remain the same during the duration of an animation.
// (Transforming a rounded rect will stretch the corners, and having to repaint
// every animation tick is expensive.)
class ASH_EXPORT SplitViewHighlightView : public views::View {
public:
explicit SplitViewHighlightView(bool is_right_or_bottom);
~SplitViewHighlightView() override;
void SetBounds(const gfx::Rect& bounds, bool landscape, bool animate);
void SetColor(SkColor color);
private:
friend class SplitViewHighlightViewTestApi;
// The three components of this view.
RoundedRectView* left_top_ = nullptr;
RoundedRectView* right_bottom_ = nullptr;
views::View* middle_ = nullptr;
bool landscape_ = true;
// Determines whether this particular highlight view is located at the right
// or bottom of the screen. These highlights animate in the opposite direction
// as left or top highlights, so when we use SetBounds extra calucations have
// to be done to ensure the animation is correct.
const bool is_right_or_bottom_;
DISALLOW_COPY_AND_ASSIGN(SplitViewHighlightView);
};
} // namespace ash
#endif // ASH_WM_SPLITVIEW_SPLIT_VIEW_HIGHLIGHT_VIEW_H_
\ No newline at end of file
// Copyright 2018 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "ash/wm/splitview/split_view_highlight_view_test_api.h"
#include "ash/wm/overview/rounded_rect_view.h"
#include "ui/views/view.h"
namespace ash {
SplitViewHighlightViewTestApi::SplitViewHighlightViewTestApi(
SplitViewHighlightView* highlight_view)
: highlight_view_(highlight_view) {}
SplitViewHighlightViewTestApi::~SplitViewHighlightViewTestApi() = default;
views::View* SplitViewHighlightViewTestApi::GetLeftTopView() {
return static_cast<views::View*>(highlight_view_->left_top_);
}
views::View* SplitViewHighlightViewTestApi::GetRightBottomView() {
return static_cast<views::View*>(highlight_view_->right_bottom_);
}
views::View* SplitViewHighlightViewTestApi::GetMiddleView() {
return highlight_view_->middle_;
}
} // namespace ash
// Copyright 2018 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef ASH_WM_SPLITVIEW_SPLIT_VIEW_HIGHLIGHT_VIEW_TEST_API_H_
#define ASH_WM_SPLITVIEW_SPLIT_VIEW_HIGHLIGHT_VIEW_TEST_API_H_
#include "ash/wm/splitview/split_view_highlight_view.h"
#include "base/macros.h"
namespace views {
class View;
} // namespace views
namespace ash {
// Use the api in this class to test SplitViewHighlightView.
class SplitViewHighlightViewTestApi {
public:
explicit SplitViewHighlightViewTestApi(
SplitViewHighlightView* highlight_view);
~SplitViewHighlightViewTestApi();
views::View* GetLeftTopView();
views::View* GetRightBottomView();
views::View* GetMiddleView();
private:
SplitViewHighlightView* highlight_view_;
DISALLOW_COPY_AND_ASSIGN(SplitViewHighlightViewTestApi);
};
} // namespace ash
#endif // ASH_WM_SPLITVIEW_SPLIT_VIEW_HIGHLIGHT_VIEW_TEST_API_H_
\ No newline at end of file
// Copyright 2018 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "ash/wm/splitview/split_view_highlight_view.h"
#include "ash/test/ash_test_base.h"
#include "ash/wm/splitview/split_view_highlight_view_test_api.h"
#include "base/test/icu_test_util.h"
#include "ui/gfx/transform.h"
namespace ash {
namespace {
gfx::Transform GetTransform(views::View* view) {
DCHECK(view && view->layer());
return view->layer()->transform();
}
} // namespace
class SplitViewHighlightViewTest : public AshTestBase {
public:
SplitViewHighlightViewTest() = default;
~SplitViewHighlightViewTest() override = default;
SplitViewHighlightView* left_highlight() { return left_highlight_.get(); }
SplitViewHighlightView* right_highlight() { return right_highlight_.get(); }
// test::AshTestBase:
void SetUp() override {
AshTestBase::SetUp();
left_highlight_ = std::make_unique<SplitViewHighlightView>(false);
right_highlight_ = std::make_unique<SplitViewHighlightView>(true);
}
private:
std::unique_ptr<SplitViewHighlightView> left_highlight_;
std::unique_ptr<SplitViewHighlightView> right_highlight_;
DISALLOW_COPY_AND_ASSIGN(SplitViewHighlightViewTest);
};
// Tests setting and animating bounds for the split view highlight view in
// landscape mode.
TEST_F(SplitViewHighlightViewTest, LandscapeBounds) {
const gfx::Rect bounds(0, 0, 100, 100);
left_highlight()->SetBounds(bounds, /*landscape=*/true, /*animate=*/false);
// Tests that setting bounds without animations in landscape mode will set the
// bounds of the components correctly, without any transforms.
SplitViewHighlightViewTestApi test_api(left_highlight());
EXPECT_EQ(gfx::Rect(0, 0, 14, 100), test_api.GetLeftTopView()->bounds());
EXPECT_EQ(gfx::Rect(4, 0, 92, 100), test_api.GetMiddleView()->bounds());
EXPECT_EQ(gfx::Rect(86, 0, 14, 100), test_api.GetRightBottomView()->bounds());
EXPECT_TRUE(GetTransform(test_api.GetLeftTopView()).IsIdentity());
EXPECT_TRUE(GetTransform(test_api.GetMiddleView()).IsIdentity());
EXPECT_TRUE(GetTransform(test_api.GetRightBottomView()).IsIdentity());
// Tests that after animating to new bounds, the components have the same
// bounds, but have transforms.
const gfx::Rect new_bounds(0, 0, 200, 100);
left_highlight()->SetBounds(new_bounds, /*landscape=*/true, /*animate=*/true);
base::RunLoop().RunUntilIdle();
EXPECT_EQ(gfx::Rect(0, 0, 14, 100), test_api.GetLeftTopView()->bounds());
EXPECT_EQ(gfx::Rect(4, 0, 92, 100), test_api.GetMiddleView()->bounds());
EXPECT_EQ(gfx::Rect(86, 0, 14, 100), test_api.GetRightBottomView()->bounds());
EXPECT_TRUE(GetTransform(test_api.GetLeftTopView()).IsIdentity());
gfx::Transform expected_middle_transform;
expected_middle_transform.Scale(2.16, 1);
EXPECT_TRUE(GetTransform(test_api.GetMiddleView())
.ApproximatelyEqual(expected_middle_transform));
gfx::Transform expected_end_transform;
expected_end_transform.Translate(100, 0);
EXPECT_TRUE(GetTransform(test_api.GetRightBottomView())
.ApproximatelyEqual(expected_end_transform));
}
// Tests setting and animating bounds for the split view highlight view in
// landscape mode for rtl languages.
TEST_F(SplitViewHighlightViewTest, LandscapeBoundsInRtl) {
base::test::ScopedRestoreICUDefaultLocale scoped_locale("he");
const gfx::Rect bounds(0, 0, 100, 100);
left_highlight()->SetBounds(bounds, /*landscape=*/true, /*animate=*/false);
// Tests that setting bounds without animations in landscape mode will set the
// bounds of the components correctly, without any transforms. In rtl, the
// bounds of the outer components are swapped.
SplitViewHighlightViewTestApi test_api(left_highlight());
EXPECT_EQ(gfx::Rect(86, 0, 14, 100), test_api.GetLeftTopView()->bounds());
EXPECT_EQ(gfx::Rect(4, 0, 92, 100), test_api.GetMiddleView()->bounds());
EXPECT_EQ(gfx::Rect(0, 0, 14, 100), test_api.GetRightBottomView()->bounds());
EXPECT_TRUE(GetTransform(test_api.GetLeftTopView()).IsIdentity());
EXPECT_TRUE(GetTransform(test_api.GetMiddleView()).IsIdentity());
EXPECT_TRUE(GetTransform(test_api.GetRightBottomView()).IsIdentity());
// Tests that after animating to new bounds, the components have the same
// bounds, but have transforms. In rtl the beginning element is the one that
// is translated instead. The middle element has a extra translation in its
// transform to account for the flipped scaling.
const gfx::Rect new_bounds(0, 0, 200, 100);
left_highlight()->SetBounds(new_bounds, /*landscape=*/true, /*animate=*/true);
base::RunLoop().RunUntilIdle();
EXPECT_EQ(gfx::Rect(86, 0, 14, 100), test_api.GetLeftTopView()->bounds());
EXPECT_EQ(gfx::Rect(4, 0, 92, 100), test_api.GetMiddleView()->bounds());
EXPECT_EQ(gfx::Rect(0, 0, 14, 100), test_api.GetRightBottomView()->bounds());
gfx::Transform expected_begin_transform;
expected_begin_transform.Translate(-100, 0);
EXPECT_TRUE(GetTransform(test_api.GetLeftTopView())
.ApproximatelyEqual(expected_begin_transform));
gfx::Transform expected_middle_transform;
expected_middle_transform.Translate(-100, 0);
expected_middle_transform.Scale(2.16, 1);
EXPECT_TRUE(GetTransform(test_api.GetMiddleView())
.ApproximatelyEqual(expected_middle_transform));
EXPECT_TRUE(GetTransform(test_api.GetRightBottomView()).IsIdentity());
}
class SplitViewHighlightViewPortraitTest
: public SplitViewHighlightViewTest,
public testing::WithParamInterface<bool> {
public:
SplitViewHighlightViewPortraitTest()
: scoped_locale_(GetParam() ? "he" : "") {}
~SplitViewHighlightViewPortraitTest() override = default;
private:
// Restores locale to the default when destructor is called.
base::test::ScopedRestoreICUDefaultLocale scoped_locale_;
DISALLOW_COPY_AND_ASSIGN(SplitViewHighlightViewPortraitTest);
};
// Tests setting and animating bounds for the split view highlight view in
// portrait mode. The bounds should remain the same in ltr or rtl.
TEST_P(SplitViewHighlightViewPortraitTest, Bounds) {
const gfx::Rect bounds(0, 0, 100, 100);
left_highlight()->SetBounds(bounds, /*landscape=*/false, /*animate=*/false);
SplitViewHighlightViewTestApi test_api(left_highlight());
EXPECT_EQ(gfx::Rect(0, 0, 100, 14), test_api.GetLeftTopView()->bounds());
EXPECT_EQ(gfx::Rect(0, 4, 100, 92), test_api.GetMiddleView()->bounds());
EXPECT_EQ(gfx::Rect(0, 86, 100, 14), test_api.GetRightBottomView()->bounds());
const gfx::Rect new_bounds(0, 0, 100, 200);
left_highlight()->SetBounds(new_bounds, /*landscape=*/false,
/*animate=*/true);
base::RunLoop().RunUntilIdle();
EXPECT_EQ(gfx::Rect(0, 0, 100, 14), test_api.GetLeftTopView()->bounds());
EXPECT_EQ(gfx::Rect(0, 4, 100, 92), test_api.GetMiddleView()->bounds());
EXPECT_EQ(gfx::Rect(0, 86, 100, 14), test_api.GetRightBottomView()->bounds());
EXPECT_TRUE(GetTransform(test_api.GetLeftTopView()).IsIdentity());
gfx::Transform expected_middle_transform;
expected_middle_transform.Scale(1, 2.16);
EXPECT_TRUE(GetTransform(test_api.GetMiddleView())
.ApproximatelyEqual(expected_middle_transform));
gfx::Transform expected_end_transform;
expected_end_transform.Translate(0, 100);
EXPECT_TRUE(GetTransform(test_api.GetRightBottomView())
.ApproximatelyEqual(expected_end_transform));
}
INSTANTIATE_TEST_CASE_P(Bounds,
SplitViewHighlightViewPortraitTest,
testing::Bool());
TEST_F(SplitViewHighlightViewTest, RightBounds) {
const gfx::Rect bounds(100, 0, 100, 100);
right_highlight()->SetBounds(bounds, /*landscape=*/true, /*animate=*/false);
SplitViewHighlightViewTestApi test_api(right_highlight());
EXPECT_EQ(gfx::Rect(0, 0, 14, 100), test_api.GetLeftTopView()->bounds());
EXPECT_EQ(gfx::Rect(4, 0, 92, 100), test_api.GetMiddleView()->bounds());
EXPECT_EQ(gfx::Rect(86, 0, 14, 100), test_api.GetRightBottomView()->bounds());
const gfx::Rect new_bounds(0, 0, 200, 100);
right_highlight()->SetBounds(new_bounds, /*landscape=*/true,
/*animate=*/true);
base::RunLoop().RunUntilIdle();
EXPECT_EQ(gfx::Rect(100, 0, 14, 100), test_api.GetLeftTopView()->bounds());
EXPECT_EQ(gfx::Rect(104, 0, 92, 100), test_api.GetMiddleView()->bounds());
EXPECT_EQ(gfx::Rect(186, 0, 14, 100),
test_api.GetRightBottomView()->bounds());
gfx::Transform expected_begin_transform;
expected_begin_transform.Translate(-100, 0);
EXPECT_TRUE(GetTransform(test_api.GetLeftTopView())
.ApproximatelyEqual(expected_begin_transform));
gfx::Transform expected_middle_transform;
expected_middle_transform.Translate(-100, 0);
expected_middle_transform.Scale(2.16, 1);
EXPECT_TRUE(GetTransform(test_api.GetMiddleView())
.ApproximatelyEqual(expected_middle_transform));
EXPECT_TRUE(GetTransform(test_api.GetRightBottomView()).IsIdentity());
}
} // namespace ash
\ No newline at end of file
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment