Skip to content

Commit

Permalink
snap-group: Refactor GetItemScale()s in Overview
Browse files Browse the repository at this point in the history
The current `GetItemScale()` takes gfx::Size as the input argument
however only height is taken into account. This cl changes the input
argument for `GetItemScale()` to be `height` only to make the code
more readable.

This cl also updates the temporary implementation of `GetItemScale()`
in `OverviewGroupItem`.

Fixed: b/303133543
Test: Run existing
Change-Id: Iffba471369e4e0c31669e79e61f1e76d5b170a40
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4910241
Commit-Queue: Michele Fan <michelefan@chromium.org>
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1209214}
  • Loading branch information
Michele Fan authored and Chromium LUCI CQ committed Oct 13, 2023
1 parent 5362172 commit 31c6eeb
Show file tree
Hide file tree
Showing 9 changed files with 36 additions and 37 deletions.
6 changes: 2 additions & 4 deletions ash/wm/overview/overview_grid.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1731,9 +1731,8 @@ void OverviewGrid::EndScroll() {
int OverviewGrid::CalculateWidthAndMaybeSetUnclippedBounds(
OverviewItemBase* item,
int height) {
const gfx::Size item_size(0, height);
gfx::SizeF target_size = item->GetTargetBoundsInScreen().size();
float scale = item->GetItemScale(item_size);
float scale = item->GetItemScale(height);
OverviewGridWindowFillMode grid_fill_mode = item->GetWindowDimensionsType();

// The drop target, unlike the other windows has its bounds set directly, so
Expand Down Expand Up @@ -1773,9 +1772,8 @@ int OverviewGrid::CalculateWidthAndMaybeSetUnclippedBounds(
target_size.SetToMin(gfx::SizeF(work_area_size));
}

const gfx::SizeF inset_size(0, height);
scale = ScopedOverviewTransformWindow::GetItemScale(
target_size, inset_size, GetTopViewInset(dragged_windows),
target_size.height(), height, GetTopViewInset(dragged_windows),
kHeaderHeightDp);
}
}
Expand Down
12 changes: 7 additions & 5 deletions ash/wm/overview/overview_group_item.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "ash/wm/overview/overview_item_base.h"
#include "ash/wm/overview/overview_item_view.h"
#include "ash/wm/window_util.h"
#include "base/check_op.h"
#include "base/containers/unique_ptr_adapters.h"
#include "ui/gfx/geometry/rect_f.h"
#include "ui/gfx/geometry/rounded_corners_f.h"
Expand Down Expand Up @@ -153,14 +154,15 @@ gfx::RectF OverviewGroupItem::GetTransformedBounds() const {
// actual implementation of this function.
CHECK_GE(overview_items_.size(), 1u);
CHECK_LE(overview_items_.size(), 2u);
return overview_items_.at(0)->GetTransformedBounds();
return overview_items_[0]->GetTransformedBounds();
}

float OverviewGroupItem::GetItemScale(const gfx::Size& size) {
float OverviewGroupItem::GetItemScale(int height) {
// TODO(michelefan): This is a temporary placeholder for the item scale
// calculation, which needs to be updated when we start working on the actual
// implementation of this function.
return overview_items_.at(0)->GetItemScale(size);
// calculation, which should be updated when we implement the overview for
// vertical split screen.
CHECK(!overview_items_.empty());
return overview_items_[0]->GetItemScale(height);
}

void OverviewGroupItem::ScaleUpSelectedItem(
Expand Down
2 changes: 1 addition & 1 deletion ash/wm/overview/overview_group_item.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ class OverviewGroupItem : public OverviewItemBase,
gfx::RectF GetTargetBoundsInScreen() const override;
gfx::RectF GetWindowTargetBoundsWithInsets() const override;
gfx::RectF GetTransformedBounds() const override;
float GetItemScale(const gfx::Size& size) override;
float GetItemScale(int height) override;
void ScaleUpSelectedItem(OverviewAnimationType animation_type) override;
void EnsureVisible() override;
OverviewFocusableView* GetFocusableView() const override;
Expand Down
5 changes: 2 additions & 3 deletions ash/wm/overview/overview_item.cc
Original file line number Diff line number Diff line change
Expand Up @@ -524,10 +524,9 @@ gfx::RectF OverviewItem::GetTransformedBounds() const {
return transform_window_.GetTransformedBounds();
}

float OverviewItem::GetItemScale(const gfx::Size& size) {
gfx::SizeF inset_size(size.width(), size.height());
float OverviewItem::GetItemScale(int height) {
return ScopedOverviewTransformWindow::GetItemScale(
GetTargetBoundsInScreen().size(), inset_size,
GetTargetBoundsInScreen().height(), height,
transform_window_.GetTopInset(), kHeaderHeightDp);
}

Expand Down
2 changes: 1 addition & 1 deletion ash/wm/overview/overview_item.h
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ class ASH_EXPORT OverviewItem : public OverviewItemBase,
OverviewAnimationType animation_type) override;
gfx::Transform ComputeTargetTransform(
const gfx::RectF& target_bounds) override;
float GetItemScale(const gfx::Size& size) override;
float GetItemScale(int height) override;
void ScaleUpSelectedItem(OverviewAnimationType animation_type) override;
void EnsureVisible() override;
gfx::RectF GetTargetBoundsInScreen() const override;
Expand Down
7 changes: 3 additions & 4 deletions ash/wm/overview/overview_item_base.h
Original file line number Diff line number Diff line change
Expand Up @@ -183,10 +183,9 @@ class ASH_EXPORT OverviewItemBase {
// Returns the transformed bound of this.
virtual gfx::RectF GetTransformedBounds() const = 0;

// Calculates and returns an optimal scale ratio. With MD this is only
// taking into account `size.height()` as the width can vary. Without MD this
// returns the scale that allows the item to fully fit within `size`.
virtual float GetItemScale(const gfx::Size& size) = 0;
// Calculates and returns an optimal scale ratio. Only the given `height` is
// taken into account as the width can vary.
virtual float GetItemScale(int height) = 0;

// Increases the bounds of the dragged item.
virtual void ScaleUpSelectedItem(OverviewAnimationType animation_type) = 0;
Expand Down
14 changes: 7 additions & 7 deletions ash/wm/overview/scoped_overview_transform_window.cc
Original file line number Diff line number Diff line change
Expand Up @@ -278,12 +278,12 @@ ScopedOverviewTransformWindow::~ScopedOverviewTransformWindow() {
}

// static
float ScopedOverviewTransformWindow::GetItemScale(const gfx::SizeF& source,
const gfx::SizeF& target,
float ScopedOverviewTransformWindow::GetItemScale(int source_height,
int target_height,
int top_view_inset,
int title_height) {
return std::min(2.0f, (target.height() - title_height) /
(source.height() - top_view_inset));
return std::min(2.0f, static_cast<float>(target_height - title_height) /
(source_height - top_view_inset));
}

// static
Expand Down Expand Up @@ -468,8 +468,8 @@ gfx::RectF ScopedOverviewTransformWindow::ShrinkRectToFitPreservingAspectRatio(
int title_height) const {
DCHECK(!rect.IsEmpty());
DCHECK_LE(top_view_inset, rect.height());
const float scale =
GetItemScale(rect.size(), bounds.size(), top_view_inset, title_height);
const float scale = GetItemScale(rect.height(), bounds.height(),
top_view_inset, title_height);
const float horizontal_offset = 0.5 * (bounds.width() - scale * rect.width());
const float width = bounds.width() - 2.f * horizontal_offset;
const float vertical_offset = title_height - scale * top_view_inset;
Expand Down Expand Up @@ -510,7 +510,7 @@ gfx::RectF ScopedOverviewTransformWindow::ShrinkRectToFitPreservingAspectRatio(
// to find out the height of the inset when the whole window,
// including the inset, is scaled down to `new_bounds`.
const float new_scale =
GetItemScale(rect.size(), new_bounds.size(), 0, 0);
GetItemScale(rect.height(), new_bounds.height(), 0, 0);
const float scaled_top_view_inset = top_view_inset * new_scale;
// Offset `new_bounds` to be at a point in the overview item frame
// where it will be centered when we clip the `top_view_inset`.
Expand Down
6 changes: 3 additions & 3 deletions ash/wm/overview/scoped_overview_transform_window.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,9 @@ class ASH_EXPORT ScopedOverviewTransformWindow
using ClippingData = std::pair<ClippingType, gfx::SizeF>;

// Calculates and returns an optimal scale ratio. This is only taking into
// account |size.height()| as the width can vary.
static float GetItemScale(const gfx::SizeF& source,
const gfx::SizeF& target,
// account height as the width can vary.
static float GetItemScale(int source_height,
int target_height,
int top_view_inset,
int title_height);

Expand Down
19 changes: 10 additions & 9 deletions ash/wm/overview/scoped_overview_transform_window_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,12 @@ namespace ash {

namespace {

float GetItemScale(const gfx::RectF& source,
const gfx::RectF& target,
float GetItemScale(int source_height,
int target_height,
int top_view_inset,
int title_height) {
return ScopedOverviewTransformWindow::GetItemScale(
source.size(), target.size(), top_view_inset, title_height);
source_height, target_height, top_view_inset, title_height);
}

} // namespace
Expand All @@ -47,33 +47,33 @@ TEST_F(ScopedOverviewTransformWindowTest, TransformedRectMaintainsAspect) {
gfx::RectF bounds(100.f, 100.f, 50.f, 50.f);
gfx::RectF transformed_rect =
transform_window.ShrinkRectToFitPreservingAspectRatio(rect, bounds, 0, 0);
float scale = GetItemScale(rect, bounds, 0, 0);
float scale = GetItemScale(rect.height(), bounds.height(), 0, 0);
EXPECT_NEAR(scale * rect.width(), transformed_rect.width(), 1);
EXPECT_NEAR(scale * rect.height(), transformed_rect.height(), 1);

rect = gfx::RectF(50.f, 50.f, 400.f, 200.f);
scale = GetItemScale(rect, bounds, 0, 0);
scale = GetItemScale(rect.height(), bounds.height(), 0, 0);
transformed_rect =
transform_window.ShrinkRectToFitPreservingAspectRatio(rect, bounds, 0, 0);
EXPECT_NEAR(scale * rect.width(), transformed_rect.width(), 1);
EXPECT_NEAR(scale * rect.height(), transformed_rect.height(), 1);

rect = gfx::RectF(50.f, 50.f, 25.f, 25.f);
scale = GetItemScale(rect, bounds, 0, 0);
scale = GetItemScale(rect.height(), bounds.height(), 0, 0);
transformed_rect =
transform_window.ShrinkRectToFitPreservingAspectRatio(rect, bounds, 0, 0);
EXPECT_NEAR(scale * rect.width(), transformed_rect.width(), 1);
EXPECT_NEAR(scale * rect.height(), transformed_rect.height(), 1);

rect = gfx::RectF(50.f, 50.f, 25.f, 50.f);
scale = GetItemScale(rect, bounds, 0, 0);
scale = GetItemScale(rect.height(), bounds.height(), 0, 0);
transformed_rect =
transform_window.ShrinkRectToFitPreservingAspectRatio(rect, bounds, 0, 0);
EXPECT_NEAR(scale * rect.width(), transformed_rect.width(), 1);
EXPECT_NEAR(scale * rect.height(), transformed_rect.height(), 1);

rect = gfx::RectF(50.f, 50.f, 50.f, 25.f);
scale = GetItemScale(rect, bounds, 0, 0);
scale = GetItemScale(rect.height(), bounds.height(), 0, 0);
transformed_rect =
transform_window.ShrinkRectToFitPreservingAspectRatio(rect, bounds, 0, 0);
EXPECT_NEAR(scale * rect.width(), transformed_rect.width(), 1);
Expand Down Expand Up @@ -109,7 +109,8 @@ TEST_F(ScopedOverviewTransformWindowTest, TransformedRectIsCenteredWithInset) {
gfx::RectF bounds(100.f, 100.f, 50.f, 50.f);
const int inset = 20;
const int header_height = 10;
const float scale = GetItemScale(rect, bounds, inset, header_height);
const float scale =
GetItemScale(rect.height(), bounds.height(), inset, header_height);
gfx::RectF transformed_rect =
transform_window.ShrinkRectToFitPreservingAspectRatio(rect, bounds, inset,
header_height);
Expand Down

0 comments on commit 31c6eeb

Please sign in to comment.