Skip to content

Commit

Permalink
pip-pinch: Disallow offscreen bounds during resist
Browse files Browse the repository at this point in the history 
The resistance effect (b/298282501) uses `gfx::Transform.Scale()` to
visually change the size of the window, but it only changes the visual
screen size and the window's actual bounds are not scaled with the
transform. The bounds correction that keeps the PiP window inside the
screen uses the bounds to calculate the obstacle avoidance, so the PiP
window was allowed to momentarily go partially offscreen during the
effect.

This commit adds new `PipPositioner::GetBoundsForDrag` that takes
`gfx::Transform` as an argument and returns the bounds that avoids the
obstacles even with the scale effect applied.

during the resistance effect.

Bug: b/303150761
Test: Manually confirmed that the window does not go out of bounds even
Change-Id: Ic125f38ba0262007c0a2a4e3cafdef3a92517e97
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4923231
Reviewed-by: Kazuki Takise <takise@chromium.org>
Commit-Queue: Masa Fujita <massan@google.com>
Auto-Submit: Masa Fujita <massan@google.com>
Cr-Commit-Position: refs/heads/main@{#1210057}
  • Loading branch information
Masayuki Fujita authored and Chromium LUCI CQ committed Oct 16, 2023
1 parent 66bfd97 commit a7f4aca
Show file tree
Hide file tree
Showing 6 changed files with 160 additions and 46 deletions.
7 changes: 5 additions & 2 deletions ash/wm/pip/pip_positioner.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,15 @@ const float kPipDismissMovementProportion = 1.5f;
} // namespace

gfx::Rect PipPositioner::GetBoundsForDrag(const display::Display& display,
const gfx::Rect& bounds_in_screen) {
gfx::Rect drag_bounds = bounds_in_screen;
const gfx::Rect& bounds_in_screen,
const gfx::Transform& transform) {
gfx::Rect drag_bounds(bounds_in_screen.origin(),
transform.MapRect(bounds_in_screen).size());
drag_bounds.AdjustToFit(CollisionDetectionUtils::GetMovementArea(display));
drag_bounds = CollisionDetectionUtils::AvoidObstacles(
display, drag_bounds,
CollisionDetectionUtils::RelativePriority::kPictureInPicture);
drag_bounds.set_size(bounds_in_screen.size());
return drag_bounds;
}

Expand Down
6 changes: 5 additions & 1 deletion ash/wm/pip/pip_positioner.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,12 @@ class ASH_EXPORT PipPositioner {

// Adjusts bounds during a drag of a PIP window. For example, this will
// ensure that the PIP window cannot leave the PIP movement area.
// If the window is transformed with `gfx::Transform`, it returns
// bounds with unscaled size but with origin that avoids obstacles
// even when the scale is applied.
static gfx::Rect GetBoundsForDrag(const display::Display& display,
const gfx::Rect& bounds_in_screen);
const gfx::Rect& bounds_in_screen,
const gfx::Transform& transform);

// Based on the current PIP window position, finds a final location of where
// the PIP window should be animated to to show a dismissal off the side
Expand Down
54 changes: 44 additions & 10 deletions ash/wm/pip/pip_positioner_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,24 +100,26 @@ TEST_P(PipPositionerDisplayTest, PipAdjustPositionForDragClampsToMovementArea) {
// Adjust near top edge outside movement area.
EXPECT_EQ(ConvertToScreen(gfx::Rect(100, 8, 100, 100)),
PipPositioner::GetBoundsForDrag(
display, ConvertToScreen(gfx::Rect(100, -50, 100, 100))));
display, ConvertToScreen(gfx::Rect(100, -50, 100, 100)),
gfx::Transform()));

// Adjust near bottom edge outside movement area.
EXPECT_EQ(
ConvertToScreen(gfx::Rect(100, bottom - 108, 100, 100)),
PipPositioner::GetBoundsForDrag(
display, ConvertToScreen(gfx::Rect(100, bottom + 50, 100, 100))));
EXPECT_EQ(ConvertToScreen(gfx::Rect(100, bottom - 108, 100, 100)),
PipPositioner::GetBoundsForDrag(
display, ConvertToScreen(gfx::Rect(100, bottom + 50, 100, 100)),
gfx::Transform()));

// Adjust near left edge outside movement area.
EXPECT_EQ(ConvertToScreen(gfx::Rect(8, 100, 100, 100)),
PipPositioner::GetBoundsForDrag(
display, ConvertToScreen(gfx::Rect(-50, 100, 100, 100))));
display, ConvertToScreen(gfx::Rect(-50, 100, 100, 100)),
gfx::Transform()));

// Adjust near right edge outside movement area.
EXPECT_EQ(
ConvertToScreen(gfx::Rect(right - 108, 100, 100, 100)),
PipPositioner::GetBoundsForDrag(
display, ConvertToScreen(gfx::Rect(right + 50, 100, 100, 100))));
EXPECT_EQ(ConvertToScreen(gfx::Rect(right - 108, 100, 100, 100)),
PipPositioner::GetBoundsForDrag(
display, ConvertToScreen(gfx::Rect(right + 50, 100, 100, 100)),
gfx::Transform()));
}

TEST_P(PipPositionerDisplayTest,
Expand Down Expand Up @@ -187,6 +189,38 @@ TEST_P(PipPositionerDisplayTest, PipDismissedPositionPrefersHorizontal) {
gfx::Rect(right - 100, bottom - 100, 100, 100))));
}

TEST_P(PipPositionerDisplayTest,
ScaledPipAdjustPositionForDragClampsToMovementArea) {
auto display = GetDisplay();
int right = display.bounds().width();
int bottom = display.bounds().height();
gfx::Transform transform = gfx::Transform::MakeScale(2.f);

// Adjust near top edge outside movement area.
EXPECT_EQ(
ConvertToScreen(gfx::Rect(100, 8, 100, 100)),
PipPositioner::GetBoundsForDrag(
display, ConvertToScreen(gfx::Rect(100, -50, 100, 100)), transform));

// Adjust near bottom edge outside movement area.
EXPECT_EQ(ConvertToScreen(gfx::Rect(100, bottom - 208, 100, 100)),
PipPositioner::GetBoundsForDrag(
display, ConvertToScreen(gfx::Rect(100, bottom + 50, 100, 100)),
transform));

// Adjust near left edge outside movement area.
EXPECT_EQ(
ConvertToScreen(gfx::Rect(8, 100, 100, 100)),
PipPositioner::GetBoundsForDrag(
display, ConvertToScreen(gfx::Rect(-50, 100, 100, 100)), transform));

// Adjust near right edge outside movement area.
EXPECT_EQ(ConvertToScreen(gfx::Rect(right - 208, 100, 100, 100)),
PipPositioner::GetBoundsForDrag(
display, ConvertToScreen(gfx::Rect(right + 50, 100, 100, 100)),
transform));
}

INSTANTIATE_TEST_SUITE_P(
/* no prefix */,
PipPositionerDisplayTest,
Expand Down
112 changes: 82 additions & 30 deletions ash/wm/pip/pip_window_resizer.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
#include <utility>

#include "ash/metrics/pip_uma.h"
#include "ash/public/cpp/shell_window_ids.h"
#include "ash/shell.h"
#include "ash/wm/collision_detection/collision_detection_utils.h"
#include "ash/wm/pip/pip_positioner.h"
#include "ash/wm/window_util.h"
Expand All @@ -23,6 +25,7 @@
#include "ui/gfx/geometry/resize_utils.h"
#include "ui/gfx/geometry/size_f.h"
#include "ui/gfx/geometry/transform_util.h"
#include "ui/views/animation/animation_builder.h"
#include "ui/views/widget/widget.h"
#include "ui/wm/core/coordinate_conversion.h"

Expand Down Expand Up @@ -103,7 +106,9 @@ PipWindowResizer::~PipWindowResizer() {
void PipWindowResizer::Drag(const gfx::PointF& location_in_parent,
int event_flags) {
last_location_in_screen_ = location_in_parent;
::wm::ConvertPointToScreen(GetTarget()->parent(), &last_location_in_screen_);
last_event_was_pinch_ = false;
::wm::ConvertPointToScreen(GetTarget()->parent(),
&last_location_in_screen_.value());

gfx::Vector2dF movement_direction =
location_in_parent - details().initial_location_in_parent;
Expand Down Expand Up @@ -155,7 +160,8 @@ void PipWindowResizer::Drag(const gfx::PointF& location_in_parent,
if (!may_dismiss_horizontally_ && !may_dismiss_vertically_) {
// Reset opacity if it's not a dismiss gesture.
GetTarget()->layer()->SetOpacity(1.f);
new_bounds = PipPositioner::GetBoundsForDrag(display, new_bounds);
new_bounds = PipPositioner::GetBoundsForDrag(display, new_bounds,
GetTarget()->transform());
} else {
gfx::Rect dismiss_bounds = new_bounds;
dismiss_bounds.Intersect(area);
Expand Down Expand Up @@ -187,6 +193,8 @@ void PipWindowResizer::Drag(const gfx::PointF& location_in_parent,
void PipWindowResizer::Pinch(const gfx::PointF& location_in_parent,
const float scale) {
accumulated_scale_ *= scale;
last_location_in_screen_ = location_in_parent;
last_event_was_pinch_ = true;

// If the user is trying to enlarge the window further than the limit,
// we use `gfx::Transform` to visually scale the window to up to 115%
Expand All @@ -198,15 +206,17 @@ void PipWindowResizer::Pinch(const gfx::PointF& location_in_parent,
gfx::Rect new_bounds = CalculateBoundsForPinch(location_in_parent);

// We do everything in screen coordinates, so convert here.
wm::ConvertPointToScreen(GetTarget()->parent(), &last_location_in_screen_);
wm::ConvertPointToScreen(GetTarget()->parent(),
&last_location_in_screen_.value());
wm::ConvertRectToScreen(GetTarget()->parent(), &new_bounds);

// Ensure that the PiP window stays inside the PiP movement area.
// This has to be consistent with `PipWindowResizer::Drag()`, as otherwise
// it can cause jump during transition from pinch to drag. This could be
// due to change (b/292768858).
display::Display display = window_state()->GetDisplay();
new_bounds = PipPositioner::GetBoundsForDrag(display, new_bounds);
new_bounds = PipPositioner::GetBoundsForDrag(display, new_bounds,
GetTarget()->transform());

// Convert back to root window coordinates for setting bounds.
wm::ConvertRectFromScreen(GetTarget()->parent(), &new_bounds);
Expand Down Expand Up @@ -291,7 +301,8 @@ gfx::Transform PipWindowResizer::CalculateTransformForPinch() const {
void PipWindowResizer::CompleteDrag() {
const bool is_resize = details().bounds_change & kBoundsChange_Resizes;

window_state()->OnCompleteDrag(last_location_in_screen_);
window_state()->OnCompleteDrag(
last_location_in_screen_.value_or(gfx::PointF()));

window_state()->ClearRestoreBounds();
window_state()->set_bounds_changed_by_user(moved_or_resized_);
Expand All @@ -311,11 +322,22 @@ void PipWindowResizer::CompleteDrag() {
window_util::CloseWidgetForWindow(window_state()->window());
} else {
// Animate the PIP window to its resting position.
gfx::Rect bounds;
gfx::Rect intended_bounds;
if (!is_resize && fling_amount > kPipMovementFlingThresholdSquared) {
bounds = ComputeFlungPosition();
intended_bounds = ComputeFlungPosition();
} else {
bounds = GetTarget()->GetBoundsInScreen();
if (last_location_in_screen_.has_value()) {
// To calculate the resting position, we want to use the user's
// intended bounds (bounds that are not restricted by
// obstacles).
gfx::PointF location_in_parent = last_location_in_screen_.value();
wm::ConvertPointFromScreen(GetTarget()->parent(), &location_in_parent);
intended_bounds = last_event_was_pinch_
? CalculateBoundsForPinch(location_in_parent)
: CalculateBoundsForDrag(location_in_parent);
} else {
intended_bounds = GetTarget()->GetBoundsInScreen();
}
}

// The origin includes the compensation for the scaling effect with
Expand All @@ -329,40 +351,70 @@ void PipWindowResizer::CompleteDrag() {
(initial_location.x() - initial_bounds.x()) / initial_bounds.width();
const float top_ratio =
(initial_location.y() - initial_bounds.y()) / initial_bounds.height();
bounds.Offset(
gfx::Vector2d(bounds.width() * (scale.x() - 1.f) * left_ratio,
bounds.height() * (scale.y() - 1.f) * top_ratio));
intended_bounds.Offset(gfx::Vector2d(
intended_bounds.width() * (scale.x() - 1.f) * left_ratio,
intended_bounds.height() * (scale.y() - 1.f) * top_ratio));

// Compute resting position even if it was a fling to avoid obstacles.
bounds = CollisionDetectionUtils::GetRestingPosition(
window_state()->GetDisplay(), bounds,
gfx::Rect resting_bounds = CollisionDetectionUtils::GetRestingPosition(
window_state()->GetDisplay(), intended_bounds,
CollisionDetectionUtils::RelativePriority::kPictureInPicture);
::wm::ConvertRectFromScreen(GetTarget()->parent(), &resting_bounds);

// Animate the window to its resting position, and also animate the
// size back to its limit size if it has expanded or shrunk during
// resistance effect with `gfx::Transform`.
ui::Layer* layer = GetTarget()->layer();
base::TimeDelta duration =
base::Milliseconds(kPipSnapToEdgeAnimationDurationMs);
::wm::ConvertRectFromScreen(GetTarget()->parent(), &bounds);
SetBoundsWMEvent event(bounds, /*animate=*/true, duration);
window_state()->OnWMEvent(&event);

ui::Layer* layer = GetTarget()->layer();
ui::ScopedLayerAnimationSettings settings(layer->GetAnimator());
settings.SetPreemptionStrategy(
ui::LayerAnimator::IMMEDIATELY_ANIMATE_TO_NEW_TARGET);
settings.SetTransitionDuration(duration);

// Animate opacity back to normal opacity.
layer->SetOpacity(1.f);

// Animate the size back to its limit size if it has expanded or
// shrunk beyond it.
layer->SetTransform(gfx::Transform());
display::Display display = window_state()->GetDisplay();
views::AnimationBuilder()
.OnEnded(base::BindOnce(
[](gfx::Rect bounds, display::Display display,
aura::Window* window) {
// Look for `WindowState` of the PiP window to send
// `SetBoundsWMEvent` after the animation is done, to make
// sure that bounds are in sync with client.
// `window_state()` cannot be used because it might be
// dangling by the time this animation is called.
auto* pip_container = Shell::GetContainer(
Shell::GetRootWindowForDisplayId(display.id()),
kShellWindowId_PipContainer);
if (!pip_container) {
return;
}
auto pip_window_iter = base::ranges::find_if(
pip_container->children(), [](const aura::Window* window) {
return WindowState::Get(window)->IsPip();
});
if (pip_window_iter == pip_container->children().end()) {
return;
}

auto* window_state = WindowState::Get(*pip_window_iter);

// Make sure that the found window is the window we were
// looking for.
if (window_state->window() == window) {
SetBoundsWMEvent event(bounds, /*animate=*/false);
window_state->OnWMEvent(&event);
}
},
resting_bounds, display, window_state()->window()))
.SetPreemptionStrategy(
ui::LayerAnimator::IMMEDIATELY_ANIMATE_TO_NEW_TARGET)
.Once()
.SetDuration(duration)
.SetTransform(layer, gfx::Transform())
.SetBounds(GetTarget(), resting_bounds)
.SetOpacity(layer, 1.f);

// If the pip work area changes (e.g. message center, virtual keyboard),
// we want to restore to the last explicitly set position.
// TODO(edcourtney): This may not be the best place for this. Consider
// doing this a different way or saving these bounds at a later point when
// the work area changes.
PipPositioner::SaveSnapFraction(window_state(), bounds);
PipPositioner::SaveSnapFraction(window_state(), resting_bounds);
}
}

Expand Down
3 changes: 2 additions & 1 deletion ash/wm/pip/pip_window_resizer.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@ class ASH_EXPORT PipWindowResizer : public WindowResizer {

gfx::Rect ComputeFlungPosition();

gfx::PointF last_location_in_screen_;
absl::optional<gfx::PointF> last_location_in_screen_;
bool last_event_was_pinch_ = false;
int fling_velocity_x_ = 0;
int fling_velocity_y_ = 0;
float dismiss_fraction_ = 1.f;
Expand Down

0 comments on commit a7f4aca

Please sign in to comment.