Commit 03653b02 authored by Andrew Xu's avatar Andrew Xu Committed by Commit Bot

Ensure that RTL is handled by bounds animator

One recent CL (https://crrev.com/c/2055982) adds the option to use layer
transformation for bounds animation. However, it fails to handle RTL
properly. Different from View::SetBounds which handles RTL,
View::SetTransform does not do that.

This CL mirrors the start bounds and target bounds of transform
animation when RTL should be handled.

Bug: 1067033
Change-Id: I1c5aceec54e4f167ac2a80d93fc0c156e0afa9a8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2140774
Commit-Queue: Andrew Xu <andrewxu@chromium.org>
Reviewed-by: default avatarScott Violet <sky@chromium.org>
Reviewed-by: default avatarXiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#761021}
parent 62713891
...@@ -64,8 +64,12 @@ void BoundsAnimator::AnimateViewTo( ...@@ -64,8 +64,12 @@ void BoundsAnimator::AnimateViewTo(
// Calculate the target transform. Note that we don't reset the transform if // Calculate the target transform. Note that we don't reset the transform if
// there already was one, otherwise users will end up with visual bounds // there already was one, otherwise users will end up with visual bounds
// different than what they set. // different than what they set.
// Note that View::SetTransform() does not handle RTL, which is different
// from View::SetBounds(). So mirror the start bounds and target bounds
// manually if necessary.
const gfx::Transform target_transform = gfx::TransformBetweenRects( const gfx::Transform target_transform = gfx::TransformBetweenRects(
gfx::RectF(data.start_bounds), gfx::RectF(data.target_bounds)); gfx::RectF(parent_->GetMirroredRect(data.start_bounds)),
gfx::RectF(parent_->GetMirroredRect(data.target_bounds)));
data.target_transform = target_transform; data.target_transform = target_transform;
} }
......
...@@ -6,6 +6,7 @@ ...@@ -6,6 +6,7 @@
#include "base/macros.h" #include "base/macros.h"
#include "base/run_loop.h" #include "base/run_loop.h"
#include "base/test/icu_test_util.h"
#include "base/test/task_environment.h" #include "base/test/task_environment.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
#include "ui/gfx/animation/slide_animation.h" #include "ui/gfx/animation/slide_animation.h"
...@@ -78,6 +79,56 @@ class TestView : public View { ...@@ -78,6 +79,56 @@ class TestView : public View {
DISALLOW_COPY_AND_ASSIGN(TestView); DISALLOW_COPY_AND_ASSIGN(TestView);
}; };
class RTLAnimationTestDelegate : public gfx::AnimationDelegate {
public:
RTLAnimationTestDelegate(const gfx::Rect& start,
const gfx::Rect& target,
View* view,
base::RepeatingClosure quit_closure)
: start_(start),
target_(target),
view_(view),
quit_closure_(std::move(quit_closure)) {}
~RTLAnimationTestDelegate() override = default;
private:
// gfx::AnimationDelegate:
void AnimationProgressed(const Animation* animation) override {
gfx::Transform transform = view_->GetTransform();
ASSERT_TRUE(!transform.IsIdentity());
// In this test, assume that |parent| is root view.
View* parent = view_->parent();
const gfx::Rect start_rect_in_screen = parent->GetMirroredRect(start_);
const gfx::Rect target_rect_in_screen = parent->GetMirroredRect(target_);
gfx::RectF current_bounds_in_screen(
parent->GetMirroredRect(view_->bounds()));
transform.TransformRect(&current_bounds_in_screen);
// Verify that |view_|'s current bounds in screen are valid.
EXPECT_GE(current_bounds_in_screen.x(),
std::min(start_rect_in_screen.x(), target_rect_in_screen.x()));
EXPECT_LE(
current_bounds_in_screen.right(),
std::max(start_rect_in_screen.right(), target_rect_in_screen.right()));
quit_closure_.Run();
}
// Animation initial bounds.
gfx::Rect start_;
// Animation target bounds.
gfx::Rect target_;
// view to be animated.
View* view_;
base::RepeatingClosure quit_closure_;
};
} // namespace } // namespace
class BoundsAnimatorTest : public testing::Test { class BoundsAnimatorTest : public testing::Test {
...@@ -268,4 +319,36 @@ TEST_F(BoundsAnimatorTest, UseTransformsAnimateViewToEmptySrc) { ...@@ -268,4 +319,36 @@ TEST_F(BoundsAnimatorTest, UseTransformsAnimateViewToEmptySrc) {
EXPECT_EQ(target_bounds, child()->bounds()); EXPECT_EQ(target_bounds, child()->bounds());
} }
// Verify that the bounds animation which updates the transform of views work
// as expected under RTL (https://crbug.com/1067033).
TEST_F(BoundsAnimatorTest, VerifyBoundsAnimatorUnderRTL) {
// Enable RTL.
base::test::ScopedRestoreICUDefaultLocale scoped_locale("he");
RecreateAnimator(/*use_transform=*/true);
parent()->SetBounds(0, 0, 40, 40);
const gfx::Rect initial_bounds(0, 0, 10, 10);
child()->SetBoundsRect(initial_bounds);
const gfx::Rect target_bounds(10, 10, 10, 10);
const base::TimeDelta animation_duration =
base::TimeDelta::FromMilliseconds(10);
animator()->SetAnimationDuration(animation_duration);
child()->set_repaint_count(0);
animator()->AnimateViewTo(child(), target_bounds);
base::RunLoop run_loop;
animator()->SetAnimationDelegate(
child(),
std::make_unique<RTLAnimationTestDelegate>(
initial_bounds, target_bounds, child(), run_loop.QuitClosure()));
// The animator should be animating now.
EXPECT_TRUE(animator()->IsAnimating());
EXPECT_TRUE(animator()->IsAnimating(child()));
run_loop.Run();
EXPECT_FALSE(animator()->IsAnimating(child()));
}
} // namespace views } // namespace views
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