Skip to content

Commit

Permalink
Snap Point selection should satisfy visibility requirement.
Browse files Browse the repository at this point in the history 
According to spec, https://www.w3.org/TR/css-scroll-snap-1/#snap-scope
we should only consider the snap points of visible snap areas.
This patch implements the visibility requirement by adding a field of
visible region for each snap point. It also handles the case of
visibility conflict when snapping to two axes individually.

Bug: 778257
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel
Change-Id: I7a3d2aa6b9c1293df3f05118152f105fa3265d29
Reviewed-on: https://chromium-review.googlesource.com/814874
Commit-Queue: Sandra Sun <sunyunjia@chromium.org>
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Reviewed-by: Ali Juma <ajuma@chromium.org>
Reviewed-by: Majid Valipour <majidvp@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543111}
  • Loading branch information
@sunyunjia
sunyunjia authored and Commit Bot committed Mar 14, 2018
1 parent 64a9b1d commit 32dd135
Show file tree
Hide file tree
Showing 8 changed files with 396 additions and 106 deletions.
142 changes: 114 additions & 28 deletions cc/input/scroll_snap_data.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,79 @@

#include "cc/input/scroll_snap_data.h"

#include <cmath>
#include "base/optional.h"

namespace cc {
namespace {

bool IsVisible(const gfx::ScrollOffset& point,
const gfx::RectF& visible_region) {
return point.x() >= visible_region.x() &&
point.x() <= visible_region.right() &&
point.y() >= visible_region.y() &&
point.y() <= visible_region.bottom();
}

bool IsMutualVisible(const SnapAreaData& area_x, const SnapAreaData& area_y) {
gfx::ScrollOffset position(area_x.snap_position.x(),
area_y.snap_position.y());
return IsVisible(position, area_x.visible_region) &&
IsVisible(position, area_y.visible_region);
}

bool SnappableOnAxis(const SnapAreaData& area, SearchAxis search_axis) {
return search_axis == SearchAxis::kX ? area.snap_axis == SnapAxis::kX ||
area.snap_axis == SnapAxis::kBoth
: area.snap_axis == SnapAxis::kY ||
area.snap_axis == SnapAxis::kBoth;
}

// Finds the best SnapArea candidate that minimizes the distance between current
// and candidate positions, while satisfying three invariants:
// - |candidate_position| is in |target_region|
// - |current_position| is in candidate's visible region
// - |target_position| is in candidate's visible region

// |current_position| is the scroll position of the container before snapping.
// |target_position| is the snap position we have found on the other axis.
// |target_region| is the visible region of the target position's area.
base::Optional<SnapAreaData> FindClosestValidArea(
SearchAxis search_axis,
const gfx::ScrollOffset& current_position,
const gfx::ScrollOffset& target_position,
const gfx::RectF& target_region,
const SnapAreaList& list) {
if (list.empty())
return base::nullopt;

base::Optional<SnapAreaData> closest_area;
float smallest_distance = std::numeric_limits<float>::max();
for (const SnapAreaData& area : list) {
if (!SnappableOnAxis(area, search_axis))
continue;

gfx::ScrollOffset candidate_position =
search_axis == SearchAxis::kX
? gfx::ScrollOffset(area.snap_position.x(), target_position.y())
: gfx::ScrollOffset(target_position.x(), area.snap_position.y());
if (!IsVisible(candidate_position, target_region) ||
!IsVisible(current_position, area.visible_region) ||
!IsVisible(target_position, area.visible_region))
continue;

gfx::ScrollOffset offset = current_position - candidate_position;
float distance = search_axis == SearchAxis::kX ? std::abs(offset.x())
: std::abs(offset.y());
if (distance < smallest_distance) {
smallest_distance = distance;
closest_area = area;
}
}
return closest_area;
}

} // namespace

SnapContainerData::SnapContainerData() = default;

Expand Down Expand Up @@ -47,42 +119,56 @@ gfx::ScrollOffset SnapContainerData::FindSnapPosition(
if (!should_snap_on_x && !should_snap_on_y)
return current_position;

float smallest_distance_x = std::numeric_limits<float>::max();
float smallest_distance_y = std::numeric_limits<float>::max();

gfx::ScrollOffset snap_position = current_position;
base::Optional<SnapAreaData> closest_x, closest_y;

// A region that includes every reachable scroll position.
gfx::RectF scrollable_region(0, 0, max_position_.x(), max_position_.y());
if (should_snap_on_x) {
closest_x =
FindClosestValidArea(SearchAxis::kX, current_position, current_position,
scrollable_region, snap_area_list_);
}
if (should_snap_on_y) {
closest_y =
FindClosestValidArea(SearchAxis::kY, current_position, current_position,
scrollable_region, snap_area_list_);
}

for (const SnapAreaData& snap_area_data : snap_area_list_) {
// TODO(sunyunjia): We should consider visiblity when choosing snap offset.
if (should_snap_on_x && (snap_area_data.snap_axis == SnapAxis::kX ||
snap_area_data.snap_axis == SnapAxis::kBoth)) {
float offset = snap_area_data.snap_position.x();
if (offset == SnapAreaData::kInvalidScrollPosition)
continue;
float distance = std::abs(current_position.x() - offset);
if (distance < smallest_distance_x) {
smallest_distance_x = distance;
snap_position.set_x(offset);
}
}
if (should_snap_on_y && (snap_area_data.snap_axis == SnapAxis::kY ||
snap_area_data.snap_axis == SnapAxis::kBoth)) {
float offset = snap_area_data.snap_position.y();
if (offset == SnapAreaData::kInvalidScrollPosition)
continue;
float distance = std::abs(current_position.y() - offset);
if (distance < smallest_distance_y) {
smallest_distance_y = distance;
snap_position.set_y(offset);
}
// If snapping in one axis pushes off-screen the other snap area, this snap
// position is invalid. https://drafts.csswg.org/css-scroll-snap-1/#snap-scope
// In this case, we choose the axis whose snap area is closer, and find a
// mutual visible snap area on the other axis.
if (closest_x.has_value() && closest_y.has_value() &&
!IsMutualVisible(closest_x.value(), closest_y.value())) {
bool candidate_on_x_axis_is_closer =
std::abs(closest_x.value().snap_position.x() - current_position.x()) <=
std::abs(closest_y.value().snap_position.y() - current_position.y());
if (candidate_on_x_axis_is_closer) {
gfx::ScrollOffset snapped(closest_x.value().snap_position.x(),
current_position.y());
closest_y = FindClosestValidArea(
SearchAxis::kY, current_position, snapped,
closest_x.value().visible_region, snap_area_list_);
} else {
gfx::ScrollOffset snapped(current_position.x(),
closest_y.value().snap_position.y());
closest_x = FindClosestValidArea(
SearchAxis::kX, current_position, snapped,
closest_y.value().visible_region, snap_area_list_);
}
}
if (closest_x.has_value())
snap_position.set_x(closest_x.value().snap_position.x());
if (closest_y.has_value())
snap_position.set_y(closest_y.value().snap_position.y());

return snap_position;
}

std::ostream& operator<<(std::ostream& ostream, const SnapAreaData& area_data) {
return ostream << area_data.snap_position.x() << ", "
<< area_data.snap_position.y();
return ostream << area_data.snap_position.ToString() << "\t"
<< "visible in: " << area_data.visible_region.ToString();
}

std::ostream& operator<<(std::ostream& ostream,
Expand Down
22 changes: 20 additions & 2 deletions cc/input/scroll_snap_data.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <vector>

#include "cc/cc_export.h"
#include "ui/gfx/geometry/rect_f.h"
#include "ui/gfx/geometry/scroll_offset.h"

namespace cc {
Expand All @@ -21,6 +22,9 @@ enum class SnapAxis : unsigned {
kInline,
};

// A helper enum to specify the the axis when doing calculations.
enum class SearchAxis : unsigned { kX, kY };

// See https://www.w3.org/TR/css-scroll-snap-1/#snap-strictness
// TODO(sunyunjia): Add kNone for SnapStrictness to match the spec.
// crbug.com/791663
Expand Down Expand Up @@ -90,12 +94,19 @@ struct SnapAreaData {

SnapAreaData() {}

SnapAreaData(SnapAxis axis, gfx::ScrollOffset position, bool msnap)
: snap_axis(axis), snap_position(position), must_snap(msnap) {}
SnapAreaData(SnapAxis axis,
gfx::ScrollOffset position,
gfx::RectF visible,
bool msnap)
: snap_axis(axis),
snap_position(position),
visible_region(visible),
must_snap(msnap) {}

bool operator==(const SnapAreaData& other) const {
return (other.snap_axis == snap_axis) &&
(other.snap_position == snap_position) &&
(other.visible_region == visible_region) &&
(other.must_snap == must_snap);
}

Expand All @@ -111,6 +122,11 @@ struct SnapAreaData {
// overflow rect.
gfx::ScrollOffset snap_position;

// The area is only visible when the current scroll offset is within
// |visible_region|.
// See https://drafts.csswg.org/css-scroll-snap-1/#snap-scope
gfx::RectF visible_region;

// Whether this area has scroll-snap-stop: always.
// See https://www.w3.org/TR/css-scroll-snap-1/#scroll-snap-stop
bool must_snap;
Expand All @@ -119,6 +135,8 @@ struct SnapAreaData {
// snapping.
};

typedef std::vector<SnapAreaData> SnapAreaList;

// Snap container is a scroll container that has non-'none' value for
// scroll-snap-type. It can be snapped to one of its snap areas when a scroll
// happens.
Expand Down
65 changes: 57 additions & 8 deletions cc/input/scroll_snap_data_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,12 @@ TEST_F(ScrollSnapDataTest, FindsClosestSnapPositionIndependently) {
gfx::ScrollOffset current_position(100, 100);
SnapAreaData snap_x_only(
SnapAxis::kX, gfx::ScrollOffset(80, SnapAreaData::kInvalidScrollPosition),
false);
gfx::RectF(0, 0, 360, 380), false);
SnapAreaData snap_y_only(
SnapAxis::kY, gfx::ScrollOffset(SnapAreaData::kInvalidScrollPosition, 70),
false);
SnapAreaData snap_on_both(SnapAxis::kBoth, gfx::ScrollOffset(50, 150), false);
gfx::RectF(0, 0, 360, 380), false);
SnapAreaData snap_on_both(SnapAxis::kBoth, gfx::ScrollOffset(50, 150),
gfx::RectF(0, 0, 360, 380), false);
data.AddSnapAreaData(snap_x_only);
data.AddSnapAreaData(snap_y_only);
data.AddSnapAreaData(snap_on_both);
Expand All @@ -38,11 +39,12 @@ TEST_F(ScrollSnapDataTest, FindsClosestSnapPositionOnAxisValueBoth) {
gfx::ScrollOffset current_position(40, 150);
SnapAreaData snap_x_only(
SnapAxis::kX, gfx::ScrollOffset(80, SnapAreaData::kInvalidScrollPosition),
false);
gfx::RectF(0, 0, 360, 380), false);
SnapAreaData snap_y_only(
SnapAxis::kY, gfx::ScrollOffset(SnapAreaData::kInvalidScrollPosition, 70),
false);
SnapAreaData snap_on_both(SnapAxis::kBoth, gfx::ScrollOffset(50, 150), false);
gfx::RectF(0, 0, 360, 380), false);
SnapAreaData snap_on_both(SnapAxis::kBoth, gfx::ScrollOffset(50, 150),
gfx::RectF(0, 0, 360, 380), false);
data.AddSnapAreaData(snap_x_only);
data.AddSnapAreaData(snap_y_only);
data.AddSnapAreaData(snap_on_both);
Expand All @@ -59,10 +61,10 @@ TEST_F(ScrollSnapDataTest, DoesNotSnapOnNonScrolledAxis) {
gfx::ScrollOffset current_position(100, 100);
SnapAreaData snap_x_only(
SnapAxis::kX, gfx::ScrollOffset(80, SnapAreaData::kInvalidScrollPosition),
false);
gfx::RectF(0, 0, 360, 380), false);
SnapAreaData snap_y_only(
SnapAxis::kY, gfx::ScrollOffset(SnapAreaData::kInvalidScrollPosition, 70),
false);
gfx::RectF(0, 0, 360, 380), false);
data.AddSnapAreaData(snap_x_only);
data.AddSnapAreaData(snap_y_only);
gfx::ScrollOffset snap_position =
Expand All @@ -71,4 +73,51 @@ TEST_F(ScrollSnapDataTest, DoesNotSnapOnNonScrolledAxis) {
EXPECT_EQ(100, snap_position.y());
}

TEST_F(ScrollSnapDataTest, DoesNotSnapOnNonVisibleAreas) {
SnapContainerData data(
ScrollSnapType(false, SnapAxis::kBoth, SnapStrictness::kMandatory),
gfx::ScrollOffset(360, 380));
gfx::ScrollOffset current_position(100, 100);
SnapAreaData non_visible_x(SnapAxis::kBoth, gfx::ScrollOffset(70, 70),
gfx::RectF(0, 0, 90, 200), false);
SnapAreaData non_visible_y(SnapAxis::kBoth, gfx::ScrollOffset(70, 70),
gfx::RectF(0, 0, 200, 90), false);
data.AddSnapAreaData(non_visible_x);
data.AddSnapAreaData(non_visible_y);
gfx::ScrollOffset snap_position =
data.FindSnapPosition(current_position, true, true);
EXPECT_EQ(100, snap_position.x());
EXPECT_EQ(100, snap_position.y());
}

TEST_F(ScrollSnapDataTest, SnapOnClosestAxisFirstIfVisibilityConflicts) {
SnapContainerData data(
ScrollSnapType(false, SnapAxis::kBoth, SnapStrictness::kMandatory),
gfx::ScrollOffset(360, 380));
gfx::ScrollOffset current_position(100, 100);

// Both the areas are currently visible.
// However, if we snap to them on x and y independently, none is visible after
// snapping. So we only snap on the axis that has a closer snap point first.
// After that, we look for another snap point on y axis which does not
// conflict with the snap point on x.
SnapAreaData snap_x(
SnapAxis::kX, gfx::ScrollOffset(80, SnapAreaData::kInvalidScrollPosition),
gfx::RectF(60, 60, 60, 60), false);
SnapAreaData snap_y1(
SnapAxis::kY,
gfx::ScrollOffset(SnapAreaData::kInvalidScrollPosition, 130),
gfx::RectF(90, 90, 60, 60), false);
SnapAreaData snap_y2(
SnapAxis::kY, gfx::ScrollOffset(SnapAreaData::kInvalidScrollPosition, 60),
gfx::RectF(50, 50, 60, 60), false);
data.AddSnapAreaData(snap_x);
data.AddSnapAreaData(snap_y1);
data.AddSnapAreaData(snap_y2);
gfx::ScrollOffset snap_position =
data.FindSnapPosition(current_position, true, true);
EXPECT_EQ(80, snap_position.x());
EXPECT_EQ(60, snap_position.y());
}

} // namespace cc
3 changes: 2 additions & 1 deletion cc/trees/layer_tree_host_impl_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -688,7 +688,8 @@ class LayerTreeHostImplTest : public testing::Test,
SnapContainerData container_data(
ScrollSnapType(false, SnapAxis::kBoth, SnapStrictness::kMandatory),
gfx::ScrollOffset(300, 300));
SnapAreaData area_data(SnapAxis::kBoth, gfx::ScrollOffset(50, 50), false);
SnapAreaData area_data(SnapAxis::kBoth, gfx::ScrollOffset(50, 50),
gfx::RectF(0, 0, 300, 300), false);
container_data.AddSnapAreaData(area_data);
overflow->test_properties()->snap_container_data.emplace(container_data);
host_impl_->active_tree()->BuildPropertyTreesForTesting();
Expand Down
Loading

0 comments on commit 32dd135

Please sign in to comment.