Commit ac61ca2b authored by Sammie Quon's avatar Sammie Quon Committed by Chromium LUCI CQ

desks: Make it easier to quickly swipe to desks.

This patch tries to different between users who want to use the trackpad
to quickly switch desks and users who use the trackpad to look at the
contents of adjacent desks.

For the former, we introduce a threshold of 500ms. If the user ends
their swipe within 500ms, we will move them to the next desk if the
next desk is more than 10% visible.

For the latter, we keep the old logic of moving them to the desk
which is most visible when the swipe ends.

Also lowers the threshold when starting a swipe. We had some feedback
that this was too high.

Bug: 1158065, 1158067
Test: manual, added tests
Change-Id: I9feb8f8a8f591227f883d58bfe9c38b1653f7fa4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2617044
Commit-Queue: Sammie Quon <sammiequon@chromium.org>
Reviewed-by: default avatarAhmed Fakhry <afakhry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#842891}
parent 397365d2
......@@ -39,6 +39,12 @@ constexpr char kDeskUpdateGestureMaxLatencyHistogramName[] =
constexpr char kDeskEndGestureSmoothnessHistogramName[] =
"Ash.Desks.AnimationSmoothness.DeskEndGesture";
// Swipes which are below this threshold are considered fast, and
// RootWindowDeskSwitchAnimator will determine a different ending desk for these
// swipes.
constexpr base::TimeDelta kFastSwipeThresholdDuration =
base::TimeDelta::FromMilliseconds(500);
bool IsForContinuousGestures(DesksSwitchSource source) {
return source == DesksSwitchSource::kDeskSwitchTouchpad &&
features::IsEnhancedDeskAnimations();
......@@ -60,6 +66,7 @@ DeskActivationAnimation::DeskActivationAnimation(DesksController* controller,
switch_source_(source),
update_window_activation_(update_window_activation),
visible_desk_index_(starting_desk_index),
last_start_or_replace_time_(base::TimeTicks::Now()),
presentation_time_recorder_(CreatePresentationTimeHistogramRecorder(
desks_util::GetSelectedCompositorForPerformanceMetrics(),
kDeskUpdateGestureHistogramName,
......@@ -108,6 +115,8 @@ bool DeskActivationAnimation::Replace(bool moving_left,
ending_desk_index_ = new_ending_desk_index;
last_start_or_replace_time_ = base::TimeTicks::Now();
// Similar to on starting, for touchpad, the user can replace the animation
// without switching visible desks.
if (switch_source_ != DesksSwitchSource::kDeskSwitchTouchpad)
......@@ -195,8 +204,11 @@ bool DeskActivationAnimation::EndSwipeAnimation() {
// End the animation. The animator will determine which desk to animate to,
// and update their ending desk index. When the animation is finished we will
// activate that desk.
const bool is_fast_swipe =
base::TimeTicks::Now() - last_start_or_replace_time_ <
kFastSwipeThresholdDuration;
for (const auto& animator : desk_switch_animators_)
ending_desk_index_ = animator->EndSwipeAnimation();
ending_desk_index_ = animator->EndSwipeAnimation(is_fast_swipe);
return true;
}
......
......@@ -51,6 +51,10 @@ class ASH_EXPORT DeskActivationAnimation : public DeskAnimationBase {
// transform of the animation layer.
int visible_desk_index_;
// The last time an animation has been started or replaced. This is used to
// help determine which desk to animate to when EndSwipeAnimation is called.
base::TimeTicks last_start_or_replace_time_;
// Used to measure the presentation time of a continuous gesture swipe.
std::unique_ptr<PresentationTimeRecorder> presentation_time_recorder_;
};
......
......@@ -50,6 +50,11 @@ constexpr int kRemovedDeskWindowYTranslation = 20;
constexpr base::TimeDelta kRemovedDeskWindowTranslationDuration =
base::TimeDelta::FromMilliseconds(100);
// When ending a swipe that is deemed fast, the target desk only needs to be
// 10% shown for us to animate to that desk, compared to 50% shown for a non
// fast swipe.
constexpr float kFastSwipeVisibilityRatio = 0.1f;
// Create the layer that will be the parent of the screenshot layer, with a
// solid black color to act as the background showing behind the two
// screenshot layers in the |kDesksSpacing| region between them. It will get
......@@ -327,7 +332,7 @@ void RootWindowDeskSwitchAnimator::PrepareForEndingDeskScreenshot(
ending_desk_screenshot_taken_ = false;
}
int RootWindowDeskSwitchAnimator::EndSwipeAnimation() {
int RootWindowDeskSwitchAnimator::EndSwipeAnimation(bool is_fast_swipe) {
// If the starting screenshot has not finished, just let our delegate know
// that the desk animation is finished (and |this| will soon be deleted), and
// go back to the starting desk.
......@@ -350,10 +355,35 @@ int RootWindowDeskSwitchAnimator::EndSwipeAnimation() {
// In tests, StartAnimation() may trigger OnDeskSwitchAnimationFinished()
// right away which may delete |this|. Store the target index in a
// local so we do not try to access a member of a deleted object.
const int ending_desk_index = GetIndexOfMostVisibleDeskScreenshot();
ending_desk_index_ = ending_desk_index;
int local_ending_desk_index = -1;
// If the swipe we are ending with is deemed a fast swipe, we animate to
// |ending_desk_index_| if more than 10% of it is currently visible.
// Otherwise, we animate to the most visible desk.
if (is_fast_swipe) {
ui::Layer* layer = screenshot_layers_[ending_desk_index_];
if (layer) {
const gfx::Transform transform =
animation_layer_owner_->root()->transform();
gfx::RectF screenshot_bounds(layer->bounds());
transform.TransformRect(&screenshot_bounds);
const gfx::RectF root_window_bounds(root_window_->bounds());
const gfx::RectF intersection_rect =
gfx::IntersectRects(screenshot_bounds, root_window_bounds);
if (intersection_rect.width() >
root_window_bounds.width() * kFastSwipeVisibilityRatio) {
local_ending_desk_index = ending_desk_index_;
}
}
}
if (local_ending_desk_index == -1)
local_ending_desk_index = GetIndexOfMostVisibleDeskScreenshot();
ending_desk_index_ = local_ending_desk_index;
StartAnimation();
return ending_desk_index;
return local_ending_desk_index;
}
int RootWindowDeskSwitchAnimator::GetIndexOfMostVisibleDeskScreenshot() const {
......
......@@ -272,8 +272,10 @@ class ASH_EXPORT RootWindowDeskSwitchAnimator
void PrepareForEndingDeskScreenshot(int new_ending_desk_index);
// Called when a user ends a touchpad swipe. This will animate to the most
// visible desk, whose index is also returned.
int EndSwipeAnimation();
// visible desk, whose index is also returned. If |is_fast_swipe| is true, we
// will use a different logic to determine which ending desk index we want to
// end at.
int EndSwipeAnimation(bool is_fast_swipe);
// Gets the index of the desk whose screenshot of the animation layer is most
// visible to the user. That desk screenshot is the one which aligns the most
......
......@@ -424,7 +424,7 @@ TEST_F(RootWindowDeskSwitchAnimatorTest, EndSwipeAnimation) {
// should still be the most visible desk, so on ending the swipe animation,
// desk indexed 0 is the target desk.
animator()->UpdateSwipeAnimation(-touchpad_swipe_length_for_desk_change / 10);
animator()->EndSwipeAnimation();
animator()->EndSwipeAnimation(/*is_fast_swipe=*/false);
EXPECT_EQ(
Shell::GetPrimaryRootWindow()->bounds(),
GetTargetVisibleBounds(test_api()->GetScreenshotLayerOfDeskWithIndex(0),
......@@ -442,7 +442,30 @@ TEST_F(RootWindowDeskSwitchAnimatorTest, EndSwipeAnimation) {
// indexed 1 is the target desk.
animator()->UpdateSwipeAnimation(-9 * touchpad_swipe_length_for_desk_change /
10);
animator()->EndSwipeAnimation();
animator()->EndSwipeAnimation(/*is_fast_swipe=*/false);
EXPECT_EQ(
Shell::GetPrimaryRootWindow()->bounds(),
GetTargetVisibleBounds(test_api()->GetScreenshotLayerOfDeskWithIndex(1),
animation_layer));
}
// Tests that a fast swipe, even if it is small will result in switching desks.
TEST_F(RootWindowDeskSwitchAnimatorTest, FastSwipe) {
// Add one more desk as we need two desks for this test.
DesksController::Get()->NewDesk(DesksCreationRemovalSource::kButton);
InitAnimator(0, 1);
TakeStartingDeskScreenshotAndWait();
TakeEndingDeskScreenshotAndWait();
auto* animation_layer = test_api()->GetAnimationLayer();
const int touchpad_swipe_length_for_desk_change =
RootWindowDeskSwitchAnimator::kTouchpadSwipeLengthForDeskChange;
// Make a small left swipe headed towards desk indexed 1. We should still
// animate to desk indexed 1 even though desk indexed 0 is the most visible
// desk since it is a fast swipe.
animator()->UpdateSwipeAnimation(-touchpad_swipe_length_for_desk_change / 5);
animator()->EndSwipeAnimation(/*is_fast_swipe=*/true);
EXPECT_EQ(
Shell::GetPrimaryRootWindow()->bounds(),
GetTargetVisibleBounds(test_api()->GetScreenshotLayerOfDeskWithIndex(1),
......@@ -456,14 +479,14 @@ TEST_F(RootWindowDeskSwitchAnimatorTest,
EndSwipeAnimationBeforeScreenshotTaken) {
InitAnimator(0, 1);
animator()->TakeStartingDeskScreenshot();
animator()->EndSwipeAnimation();
animator()->EndSwipeAnimation(/*is_fast_swipe=*/false);
// Reinitialize the animator as each animator only supports one
// EndSwipeAnimation during its lifetime.
InitAnimator(0, 1);
TakeStartingDeskScreenshotAndWait();
animator()->TakeEndingDeskScreenshot();
animator()->EndSwipeAnimation();
animator()->EndSwipeAnimation(/*is_fast_swipe=*/false);
}
} // namespace ash
......@@ -29,7 +29,7 @@ class ASH_EXPORT WmGestureHandler {
// The amount in trackpad units the fingers must move in a direction before a
// continuous gesture animation is started. This is to minimize accidental
// scrolls.
static constexpr int kContinuousGestureMoveThresholdDp = 10;
static constexpr int kContinuousGestureMoveThresholdDp = 5;
WmGestureHandler();
WmGestureHandler(const WmGestureHandler&) = delete;
......
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