[css-scroll-snap] Add strategy for resnapping to targets

Added a flag to EndPositionStrategy which allows it to prioritize the
previously snapped targets when finding a snap point. If no target is
found then it defaults to finding the closest snap point as it does
normally. Also added unit tests.

This is currently not used, but will be used in a follow-up patch for
resnapping the container when layout changes occur.

Bug: 866127
Change-Id: I9c8d6db608a5219f4178afe0a79136b81cfb3378
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1929644
Commit-Queue: Kaan Alsan <alsan@google.com>
Reviewed-by: Majid Valipour <majidvp@chromium.org>
Reviewed-by: Yi Gu <yigu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#718274}

cc/input/scroll_snap_data.cc

@@ -126,63 +126,102 @@ bool SnapContainerData::FindSnapPosition(
   if (!should_snap_on_x && !should_snap_on_y)
     return false;
 
-  base::Optional<SnapSearchResult> closest_x, closest_y;
-  // A region that includes every reachable scroll position.
-  gfx::RectF scrollable_region(0, 0, max_position_.x(), max_position_.y());
+  bool should_prioritize_x_target =
+      strategy.ShouldPrioritizeSnapTargets() &&
+      target_snap_area_element_ids_.x != ElementId();
+  bool should_prioritize_y_target =
+      strategy.ShouldPrioritizeSnapTargets() &&
+      target_snap_area_element_ids_.y != ElementId();
+
+  base::Optional<SnapSearchResult> selected_x, selected_y;
   if (should_snap_on_x) {
-    // Start from current position in the cross axis. The search algorithm
-    // expects the cross axis position to be inside scroller bounds. But since
-    // we cannot always assume that the incoming value fits this criteria we
-    // clamp it to the bounds to ensure this variant.
-    SnapSearchResult initial_snap_position_y = {
-        base::ClampToRange(base_position.y(), 0.f, max_position_.y()),
-        gfx::RangeF(0, max_position_.x())};
-    closest_x =
-        FindClosestValidArea(SearchAxis::kX, strategy, initial_snap_position_y);
+    if (should_prioritize_x_target) {
+      // TODO(http://crbug.com/866127): If the target snap area is covering the
+      // snapport then we should fallback to the default "closest-area" method
+      // instead.
+      selected_x = GetTargetSnapAreaSearchResult(SearchAxis::kX);
+      DCHECK(selected_x.has_value());
+    } else {
+      // Start from current position in the cross axis. The search algorithm
+      // expects the cross axis position to be inside scroller bounds. But since
+      // we cannot always assume that the incoming value fits this criteria we
+      // clamp it to the bounds to ensure this variant.
+      SnapSearchResult initial_snap_position_y = {
+          base::ClampToRange(base_position.y(), 0.f, max_position_.y()),
+          gfx::RangeF(0, max_position_.x())};
+
+      selected_x = FindClosestValidArea(SearchAxis::kX, strategy,
+                                        initial_snap_position_y);
+    }
   }
   if (should_snap_on_y) {
-    SnapSearchResult initial_snap_position_x = {
-        base::ClampToRange(base_position.x(), 0.f, max_position_.x()),
-        gfx::RangeF(0, max_position_.y())};
-    closest_y =
-        FindClosestValidArea(SearchAxis::kY, strategy, initial_snap_position_x);
+    if (should_prioritize_y_target) {
+      selected_y = GetTargetSnapAreaSearchResult(SearchAxis::kY);
+      DCHECK(selected_y.has_value());
+    } else {
+      SnapSearchResult initial_snap_position_x = {
+          base::ClampToRange(base_position.x(), 0.f, max_position_.x()),
+          gfx::RangeF(0, max_position_.y())};
+      selected_y = FindClosestValidArea(SearchAxis::kY, strategy,
+                                        initial_snap_position_x);
+    }
   }
 
-  if (!closest_x.has_value() && !closest_y.has_value())
+  if (!selected_x.has_value() && !selected_y.has_value())
     return false;
 
   // 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_offset() - base_position.x()) <=
-        std::abs(closest_y.value().snap_offset() - base_position.y());
-    if (candidate_on_x_axis_is_closer) {
-      closest_y =
-          FindClosestValidArea(SearchAxis::kY, strategy, closest_x.value());
+  // In this case, first check if we need to prioritize the snap area from
+  // one axis over the other and select that axis, or if we don't prioritize an
+  // axis over the other, we choose the axis whose snap area is closer.
+  // Then find a new snap area on the other axis that is mutually visible with
+  // the selected axis' snap area.
+  if (selected_x.has_value() && selected_y.has_value() &&
+      !IsMutualVisible(selected_x.value(), selected_y.value())) {
+    bool keep_candidate_on_x = should_prioritize_x_target;
+    if (should_prioritize_x_target == should_prioritize_y_target) {
+      keep_candidate_on_x =
+          std::abs(selected_x.value().snap_offset() - base_position.x()) <=
+          std::abs(selected_y.value().snap_offset() - base_position.y());
+    }
+    if (keep_candidate_on_x) {
+      selected_y =
+          FindClosestValidArea(SearchAxis::kY, strategy, selected_x.value());
     } else {
-      closest_x =
-          FindClosestValidArea(SearchAxis::kX, strategy, closest_y.value());
+      selected_x =
+          FindClosestValidArea(SearchAxis::kX, strategy, selected_y.value());
     }
   }
 
   *snap_position = strategy.current_position();
-  if (closest_x.has_value()) {
-    snap_position->set_x(closest_x.value().snap_offset());
-    target_element_ids->x = closest_x.value().element_id();
+  if (selected_x.has_value()) {
+    snap_position->set_x(selected_x.value().snap_offset());
+    target_element_ids->x = selected_x.value().element_id();
   }
 
-  if (closest_y.has_value()) {
-    snap_position->set_y(closest_y.value().snap_offset());
-    target_element_ids->y = closest_y.value().element_id();
+  if (selected_y.has_value()) {
+    snap_position->set_y(selected_y.value().snap_offset());
+    target_element_ids->y = selected_y.value().element_id();
   }
 
   return true;
 }
 
+base::Optional<SnapSearchResult>
+SnapContainerData::GetTargetSnapAreaSearchResult(SearchAxis axis) const {
+  ElementId target_id = axis == SearchAxis::kX
+                            ? target_snap_area_element_ids_.x
+                            : target_snap_area_element_ids_.y;
+  if (target_id == ElementId())
+    return base::nullopt;
+  for (const SnapAreaData& area : snap_area_list_) {
+    if (area.element_id == target_id)
+      return GetSnapSearchResult(axis, area);
+  }
+  return base::nullopt;
+}
+
 const TargetSnapAreaElementIds& SnapContainerData::GetTargetSnapAreaElementIds()
     const {
   return target_snap_area_element_ids_;

cc/input/scroll_snap_data.h

@@ -271,6 +271,11 @@ class CC_EXPORT SnapContainerData {
       const SnapSelectionStrategy& strategy,
       const SnapSearchResult& cross_axis_snap_result) const;
 
+  // Finds the snap area associated with the target snap area element id for the
+  // given axis.
+  base::Optional<SnapSearchResult> GetTargetSnapAreaSearchResult(
+      SearchAxis axis) const;
+
   // Returns all the info needed to snap at this area on the given axis,
   // including:
   // - The offset at which the snap area and the snap container meet the

cc/input/scroll_snap_data_unittest.cc

@@ -468,4 +468,140 @@ TEST_F(ScrollSnapDataTest, SnapStopAlwaysNotInterferingWithDirectionStrategy) {
             target_elements);
 }
 
+TEST_F(ScrollSnapDataTest, SnapToOneTargetElementOnX) {
+  SnapContainerData container(
+      ScrollSnapType(false, SnapAxis::kBoth, SnapStrictness::kMandatory),
+      gfx::RectF(0, 0, 200, 300), gfx::ScrollOffset(600, 800));
+
+  SnapAreaData closer_area_x(ScrollSnapAlign(SnapAlignment::kStart),
+                             gfx::RectF(100, 0, 1, 1), false, ElementId(10));
+  SnapAreaData target_area_x(ScrollSnapAlign(SnapAlignment::kStart),
+                             gfx::RectF(200, 100, 1, 1), false, ElementId(20));
+  SnapAreaData closer_area_y(ScrollSnapAlign(SnapAlignment::kStart),
+                             gfx::RectF(300, 50, 1, 1), false, ElementId(30));
+
+  container.AddSnapAreaData(closer_area_x);
+  container.AddSnapAreaData(target_area_x);
+  container.AddSnapAreaData(closer_area_y);
+  container.SetTargetSnapAreaElementIds(
+      TargetSnapAreaElementIds(ElementId(20), ElementId()));
+
+  // Even though closer_area_x is closer to the scroll offset, the container
+  // should snap to the target for the x-axis. However, since the target is not
+  // set for the y-axis, the target on the y-axis should be closer_area_y.
+  std::unique_ptr<SnapSelectionStrategy> target_element_strategy =
+      SnapSelectionStrategy::CreateForTargetElement(gfx::ScrollOffset(0, 0));
+
+  gfx::ScrollOffset snap_position = gfx::ScrollOffset();
+  EXPECT_TRUE(container.FindSnapPosition(*target_element_strategy,
+                                         &snap_position, &target_elements));
+
+  EXPECT_EQ(200, snap_position.x());
+  EXPECT_EQ(50, snap_position.y());
+  EXPECT_EQ(TargetSnapAreaElementIds(ElementId(20), ElementId(30)),
+            target_elements);
+}
+
+TEST_F(ScrollSnapDataTest, SnapToOneTargetElementOnY) {
+  SnapContainerData container(
+      ScrollSnapType(false, SnapAxis::kBoth, SnapStrictness::kMandatory),
+      gfx::RectF(0, 0, 200, 300), gfx::ScrollOffset(600, 800));
+
+  SnapAreaData closer_area_y(ScrollSnapAlign(SnapAlignment::kStart),
+                             gfx::RectF(0, 100, 1, 1), false, ElementId(10));
+  SnapAreaData target_area_y(ScrollSnapAlign(SnapAlignment::kStart),
+                             gfx::RectF(100, 200, 1, 1), false, ElementId(20));
+  SnapAreaData closer_area_x(ScrollSnapAlign(SnapAlignment::kStart),
+                             gfx::RectF(50, 300, 1, 1), false, ElementId(30));
+
+  container.AddSnapAreaData(closer_area_y);
+  container.AddSnapAreaData(target_area_y);
+  container.AddSnapAreaData(closer_area_x);
+  container.SetTargetSnapAreaElementIds(
+      TargetSnapAreaElementIds(ElementId(), ElementId(20)));
+
+  // Even though closer_area_y is closer to the scroll offset, the container
+  // should snap to the target for the y-axis. However, since the target is not
+  // set for the x-axis, the target on the x-axis should be closer_area_x.
+  std::unique_ptr<SnapSelectionStrategy> target_element_strategy =
+      SnapSelectionStrategy::CreateForTargetElement(gfx::ScrollOffset(0, 0));
+
+  gfx::ScrollOffset snap_position = gfx::ScrollOffset();
+  EXPECT_TRUE(container.FindSnapPosition(*target_element_strategy,
+                                         &snap_position, &target_elements));
+
+  EXPECT_EQ(50, snap_position.x());
+  EXPECT_EQ(200, snap_position.y());
+  EXPECT_EQ(TargetSnapAreaElementIds(ElementId(30), ElementId(20)),
+            target_elements);
+}
+
+TEST_F(ScrollSnapDataTest, SnapToTwoTargetElementsMutualVisible) {
+  SnapContainerData container(
+      ScrollSnapType(false, SnapAxis::kBoth, SnapStrictness::kMandatory),
+      gfx::RectF(0, 0, 300, 300), gfx::ScrollOffset(600, 800));
+
+  SnapAreaData target_area_x(ScrollSnapAlign(SnapAlignment::kStart),
+                             gfx::RectF(100, 200, 1, 1), false, ElementId(10));
+  SnapAreaData target_area_y(ScrollSnapAlign(SnapAlignment::kStart),
+                             gfx::RectF(200, 100, 1, 1), false, ElementId(20));
+  SnapAreaData closer_area_both(ScrollSnapAlign(SnapAlignment::kStart),
+                                gfx::RectF(0, 0, 1, 1), false, ElementId(30));
+
+  container.AddSnapAreaData(target_area_x);
+  container.AddSnapAreaData(target_area_y);
+  container.AddSnapAreaData(closer_area_both);
+  container.SetTargetSnapAreaElementIds(
+      TargetSnapAreaElementIds(ElementId(10), ElementId(20)));
+
+  // The container should snap to both target areas since they are mutually
+  // visible, while ignoring the snap area that is closest to the scroll offset.
+  std::unique_ptr<SnapSelectionStrategy> target_element_strategy =
+      SnapSelectionStrategy::CreateForTargetElement(gfx::ScrollOffset(0, 0));
+
+  gfx::ScrollOffset snap_position = gfx::ScrollOffset();
+  EXPECT_TRUE(container.FindSnapPosition(*target_element_strategy,
+                                         &snap_position, &target_elements));
+
+  EXPECT_EQ(100, snap_position.x());
+  EXPECT_EQ(100, snap_position.y());
+  EXPECT_EQ(TargetSnapAreaElementIds(ElementId(10), ElementId(20)),
+            target_elements);
+}
+
+TEST_F(ScrollSnapDataTest, SnapToTwoTargetElementsNotMutualVisible) {
+  SnapContainerData container(
+      ScrollSnapType(false, SnapAxis::kBoth, SnapStrictness::kMandatory),
+      gfx::RectF(0, 0, 300, 300), gfx::ScrollOffset(600, 800));
+
+  SnapAreaData target_area_x(ScrollSnapAlign(SnapAlignment::kStart),
+                             gfx::RectF(100, 500, 1, 1), false, ElementId(10));
+  SnapAreaData target_area_y(ScrollSnapAlign(SnapAlignment::kStart),
+                             gfx::RectF(500, 100, 1, 1), false, ElementId(20));
+  SnapAreaData area_mutually_visible_to_targets(
+      ScrollSnapAlign(SnapAlignment::kStart), gfx::RectF(350, 350, 1, 1), false,
+      ElementId(30));
+
+  container.AddSnapAreaData(target_area_x);
+  container.AddSnapAreaData(target_area_y);
+  container.AddSnapAreaData(area_mutually_visible_to_targets);
+  container.SetTargetSnapAreaElementIds(
+      TargetSnapAreaElementIds(ElementId(10), ElementId(20)));
+
+  // The container cannot snap to both targets, so it should snap to the one
+  // closer to the scroll offset, and then snap to the closest mutually visible
+  // snap area on the other axis.
+  std::unique_ptr<SnapSelectionStrategy> target_element_strategy =
+      SnapSelectionStrategy::CreateForTargetElement(gfx::ScrollOffset(10, 0));
+
+  gfx::ScrollOffset snap_position = gfx::ScrollOffset();
+  EXPECT_TRUE(container.FindSnapPosition(*target_element_strategy,
+                                         &snap_position, &target_elements));
+
+  EXPECT_EQ(100, snap_position.x());
+  EXPECT_EQ(350, snap_position.y());
+  EXPECT_EQ(TargetSnapAreaElementIds(ElementId(10), ElementId(30)),
+            target_elements);
+}
+
 }  // namespace cc

cc/input/snap_selection_strategy.cc

@@ -10,9 +10,10 @@ std::unique_ptr<SnapSelectionStrategy>
 SnapSelectionStrategy::CreateForEndPosition(
     const gfx::ScrollOffset& current_position,
     bool scrolled_x,
-    bool scrolled_y) {
+    bool scrolled_y,
+    SnapTargetsPrioritization prioritization) {
   return std::make_unique<EndPositionStrategy>(current_position, scrolled_x,
-                                               scrolled_y);
+                                               scrolled_y, prioritization);
 }
 
 std::unique_ptr<SnapSelectionStrategy>
@@ -30,6 +31,14 @@ SnapSelectionStrategy::CreateForEndAndDirection(
                                                    displacement);
 }
 
+std::unique_ptr<SnapSelectionStrategy>
+SnapSelectionStrategy::CreateForTargetElement(
+    gfx::ScrollOffset current_position) {
+  return std::make_unique<EndPositionStrategy>(
+      current_position, true /* scrolled_x */, true /* scrolled_y */,
+      SnapTargetsPrioritization::kRequire);
+}
+
 bool SnapSelectionStrategy::HasIntendedDirection() const {
   return true;
 }
@@ -45,6 +54,10 @@ bool SnapSelectionStrategy::IsValidSnapArea(SearchAxis axis,
              : area.scroll_snap_align.alignment_block != SnapAlignment::kNone;
 }
 
+bool SnapSelectionStrategy::ShouldPrioritizeSnapTargets() const {
+  return false;
+}
+
 bool EndPositionStrategy::ShouldSnapOnX() const {
   return scrolled_x_;
 }
@@ -72,6 +85,10 @@ bool EndPositionStrategy::HasIntendedDirection() const {
   return false;
 }
 
+bool EndPositionStrategy::ShouldPrioritizeSnapTargets() const {
+  return snap_targets_prioritization_ == SnapTargetsPrioritization::kRequire;
+}
+
 const base::Optional<SnapSearchResult>& EndPositionStrategy::PickBestResult(
     const base::Optional<SnapSearchResult>& closest,
     const base::Optional<SnapSearchResult>& covering) const {

cc/input/snap_selection_strategy.h

@@ -12,6 +12,7 @@
 namespace cc {
 
 enum class SnapStopAlwaysFilter { kIgnore, kRequire };
+enum class SnapTargetsPrioritization { kIgnore, kRequire };
 
 // This class represents an abstract strategy that decide which snap selection
 // should be considered valid. There are concrete implementations for three core
@@ -24,7 +25,9 @@ class CC_EXPORT SnapSelectionStrategy {
   static std::unique_ptr<SnapSelectionStrategy> CreateForEndPosition(
       const gfx::ScrollOffset& current_position,
       bool scrolled_x,
-      bool scrolled_y);
+      bool scrolled_y,
+      SnapTargetsPrioritization prioritization =
+          SnapTargetsPrioritization::kIgnore);
   static std::unique_ptr<SnapSelectionStrategy> CreateForDirection(
       gfx::ScrollOffset current_position,
       gfx::ScrollOffset step,
@@ -33,10 +36,20 @@ class CC_EXPORT SnapSelectionStrategy {
       gfx::ScrollOffset current_position,
       gfx::ScrollOffset displacement);
 
+  // Creates a selection strategy that attempts to snap to previously snapped
+  // targets if possible, but defaults to finding the closest snap point if
+  // the target no longer exists.
+  static std::unique_ptr<SnapSelectionStrategy> CreateForTargetElement(
+      gfx::ScrollOffset current_position);
+
   // Returns whether it's snappable on x or y depending on the scroll performed.
   virtual bool ShouldSnapOnX() const = 0;
   virtual bool ShouldSnapOnY() const = 0;
 
+  // Returns whether snapping should attempt to snap to the previously snapped
+  // area if possible.
+  virtual bool ShouldPrioritizeSnapTargets() const;
+
   // Returns the end position of the scroll if no snap interferes.
   virtual gfx::ScrollOffset intended_position() const = 0;
   // Returns the scroll position from which the snap position should minimize
@@ -88,10 +101,12 @@ class EndPositionStrategy : public SnapSelectionStrategy {
  public:
   EndPositionStrategy(const gfx::ScrollOffset& current_position,
                       bool scrolled_x,
-                      bool scrolled_y)
+                      bool scrolled_y,
+                      SnapTargetsPrioritization snap_targets_prioritization)
       : SnapSelectionStrategy(current_position),
         scrolled_x_(scrolled_x),
-        scrolled_y_(scrolled_y) {}
+        scrolled_y_(scrolled_y),
+        snap_targets_prioritization_(snap_targets_prioritization) {}
   ~EndPositionStrategy() override = default;
 
   bool ShouldSnapOnX() const override;
@@ -102,6 +117,7 @@ class EndPositionStrategy : public SnapSelectionStrategy {
 
   bool IsValidSnapPosition(SearchAxis axis, float position) const override;
   bool HasIntendedDirection() const override;
+  bool ShouldPrioritizeSnapTargets() const override;
 
   const base::Optional<SnapSearchResult>& PickBestResult(
       const base::Optional<SnapSearchResult>& closest,
@@ -111,6 +127,7 @@ class EndPositionStrategy : public SnapSelectionStrategy {
   // Whether the x axis and y axis have been scrolled in this scroll gesture.
   const bool scrolled_x_;
   const bool scrolled_y_;
+  SnapTargetsPrioritization snap_targets_prioritization_;
 };
 
 // Examples for intended direction scrolls include