Skip to content
Permalink
Browse files
[css-scroll-snap] choose closest snap target if neither are visible
https://bugs.webkit.org/show_bug.cgi?id=245164
<rdar://99897147>

Reviewed by Wenson Hsieh.

When taking the closest snap target for each axes results in neither target being visible, pick
the closest target to the user scroll position.

* LayoutTests/imported/w3c/web-platform-tests/css/css-scroll-snap/snap-to-visible-areas-both-expected.txt:
* Source/WebCore/page/scrolling/ScrollSnapOffsetsInfo.cpp:
(WebCore::offsetHasVisibleSnapArea):
(WebCore::findCompatibleSnapArea):
(WebCore::adjustPreviousAndNextForOnScreenSnapAreas):
(WebCore::ensureVisibleTarget):
(WebCore::LayoutScrollSnapOffsetsInfo::closestSnapOffset const):
(WebCore::FloatScrollSnapOffsetsInfo::closestSnapOffset const):
(WebCore::hasCompatibleSnapArea): Deleted.

Canonical link: https://commits.webkit.org/254982@main
  • Loading branch information
nmoucht committed Sep 29, 2022
1 parent f20edbe commit 0efae6b49e5cb1c1287480ae860dbde5a3d269b1
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 13 deletions.
@@ -6020,13 +6020,13 @@ imported/w3c/web-platform-tests/css/css-ui/compute-kind-widget-generated/kind-of

webkit.org/b/233267 imported/w3c/web-platform-tests/html/semantics/embedded-content/the-iframe-element/iframe_sandbox_window_open_download_block_downloads.tentative.html [ Pass Failure ]

webkit.org/b/245164 css3/scroll-snap/scroll-snap-drag-scrollbar-thumb-with-relayouts.html [ Failure Pass ]

# Flaky because of network ordering
imported/w3c/web-platform-tests/html/semantics/scripting-1/the-script-element/css-module/charset.html [ Pass Failure ]
imported/w3c/web-platform-tests/html/semantics/scripting-1/the-script-element/css-module/cors-crossorigin-requests.html [ Pass Failure ]
imported/w3c/web-platform-tests/html/semantics/scripting-1/the-script-element/json-module/cors-crossorigin-requests.html [ Pass Failure ]

webkit.org/b/245164 css3/scroll-snap/scroll-snap-drag-scrollbar-thumb-with-relayouts.html [ Failure Pass ]

# Screen Wake Lock tests that are timing out since their import.
imported/w3c/web-platform-tests/screen-wake-lock/wakelock-disabled-by-feature-policy.https.sub.html [ Skip ]
imported/w3c/web-platform-tests/screen-wake-lock/wakelock-enabled-by-feature-policy-attribute-redirect-on-load.https.sub.html [ Skip ]
@@ -1,3 +1,3 @@

FAIL Only snap to visible areas in the case where taking the closest snap point of each axis does not snap to a visible area assert_equals: expected 800 but got 0
PASS Only snap to visible areas in the case where taking the closest snap point of each axis does not snap to a visible area

@@ -106,24 +106,42 @@ static UnitType componentForAxis(PointType point, ScrollEventAxis axis)
return axis == ScrollEventAxis::Horizontal ? point.x() : point.y();
}

template <typename InfoType, typename UnitType, typename PointType, typename SizeType>
static bool hasCompatibleSnapArea(const InfoType& info, const SnapOffset<UnitType>& snapOffset, ScrollEventAxis axis, const SizeType& viewportSize, PointType destinationOffsetPoint)
template <typename InfoType, typename UnitType, typename SizeType>
static bool offsetHasVisibleSnapArea(const InfoType& info, const SnapOffset<UnitType>& snapOffset, const SnapOffset<UnitType>& snapOffsetOther, ScrollEventAxis axis, const SizeType& viewportSize)
{
auto otherAxis = axis == ScrollEventAxis::Horizontal ? ScrollEventAxis::Vertical : ScrollEventAxis::Horizontal;
auto scrollDestinationInOtherAxis = componentForAxis<UnitType, PointType>(destinationOffsetPoint, otherAxis);
auto viewportLengthInOtherAxis = axis == ScrollEventAxis::Horizontal ? viewportSize.height() : viewportSize.width();

for (auto index : snapOffset.snapAreaIndices) {
const auto& snapArea = info.snapAreas[index];
auto [otherAxisMin, otherAxisMax] = rangeForAxis<UnitType>(snapArea, otherAxis);
if ((snapOffsetOther.offset + viewportLengthInOtherAxis) > otherAxisMin && snapOffsetOther.offset < otherAxisMax)
return true;
}
return false;
}

template <typename InfoType, typename UnitType, typename PointType, typename SizeType>
static size_t findCompatibleSnapArea(const InfoType& info, const SnapOffset<UnitType>& snapOffset, ScrollEventAxis axis, const SizeType& viewportSize, PointType destinationOffsetPoint)
{
auto otherAxis = axis == ScrollEventAxis::Horizontal ? ScrollEventAxis::Vertical : ScrollEventAxis::Horizontal;
auto viewportLength = axis == ScrollEventAxis::Horizontal ? viewportSize.width() : viewportSize.height();
auto viewportLengthInOtherAxis = otherAxis == ScrollEventAxis::Horizontal ? viewportSize.width() : viewportSize.height();
auto scrollDestinationInOtherAxis = componentForAxis<UnitType, PointType>(destinationOffsetPoint, otherAxis);
return snapOffset.snapAreaIndices.findIf([&] (auto index) {
const auto& snapArea = info.snapAreas[index];
auto [axisMin, axisMax] = rangeForAxis<UnitType>(snapArea, axis);
auto [otherAxisMin, otherAxisMax] = rangeForAxis<UnitType>(snapArea, otherAxis);
return (scrollDestinationInOtherAxis + viewportLengthInOtherAxis) > otherAxisMin && scrollDestinationInOtherAxis < otherAxisMax;
}) != notFound;
if (info.offsetsForAxis(otherAxis).isEmpty() && ((scrollDestinationInOtherAxis + viewportLengthInOtherAxis) < otherAxisMin || scrollDestinationInOtherAxis > otherAxisMax))
return false;
return (snapOffset.offset + viewportLength) > axisMin && snapOffset.offset < axisMax;
});
}

template <typename InfoType, typename UnitType, typename PointType, typename SizeType>
static void adjustPreviousAndNextForOnScreenSnapAreas(const InfoType& info, ScrollEventAxis axis, const SizeType& viewportSize, PointType destinationOffsetPoint, PotentialSnapPointSearchResult<UnitType>& searchResult)
{
// hasCompatibleSnapArea needs to look at all compatible snap areas, which might be a large
// findCompatibleSnapArea needs to look at all compatible snap areas, which might be a large
// number for snap areas arranged in a grid. Since this might be expensive, this code tries
// to look at the mostly closest compatible snap areas first.
const auto& snapOffsets = info.offsetsForAxis(axis);
@@ -133,7 +151,7 @@ static void adjustPreviousAndNextForOnScreenSnapAreas(const InfoType& info, Scro
for (unsigned offset = 0; offset <= oldIndex; offset++) {
unsigned index = oldIndex - offset;
const auto& snapOffset = snapOffsets[index];
if (hasCompatibleSnapArea(info, snapOffset, axis, viewportSize, destinationOffsetPoint)) {
if (findCompatibleSnapArea(info, snapOffset, axis, viewportSize, destinationOffsetPoint) != notFound) {
searchResult.previous = { snapOffset.offset, index };
break;
}
@@ -145,7 +163,7 @@ static void adjustPreviousAndNextForOnScreenSnapAreas(const InfoType& info, Scro
searchResult.next.reset();
for (unsigned index = oldIndex; index < snapOffsets.size(); index++) {
const auto& snapOffset = snapOffsets[index];
if (hasCompatibleSnapArea(info, snapOffset, axis, viewportSize, destinationOffsetPoint)) {
if (findCompatibleSnapArea(info, snapOffset, axis, viewportSize, destinationOffsetPoint) != notFound) {
searchResult.next = { snapOffset.offset, index };
break;
}
@@ -460,16 +478,37 @@ FloatScrollSnapOffsetsInfo LayoutScrollSnapOffsetsInfo::convertUnits(float devic

}

template <typename InfoType, typename UnitType, typename SizeType, typename PointType>
std::pair<UnitType, std::optional<unsigned>> static ensureVisibleTarget(const InfoType& info, std::pair<UnitType, std::optional<unsigned>>& horizontal, std::pair<UnitType, std::optional<unsigned>>& vertical, ScrollEventAxis axis, const SizeType& viewportSize, PointType scrollDestinationOffset)
{
const auto& snapOffsetsHorizontal = info.offsetsForAxis(ScrollEventAxis::Horizontal);
const auto& snapOffsetsVertical = info.offsetsForAxis(ScrollEventAxis::Vertical);

if (horizontal.second && vertical.second && !offsetHasVisibleSnapArea(info, snapOffsetsHorizontal[*horizontal.second], snapOffsetsVertical[*vertical.second], ScrollEventAxis::Horizontal, viewportSize) && !offsetHasVisibleSnapArea(info, snapOffsetsVertical[*vertical.second], snapOffsetsHorizontal[*horizontal.second], ScrollEventAxis::Vertical, viewportSize)) {
auto horizontalSnapArea = info.snapAreas[snapOffsetsHorizontal[*horizontal.second].snapAreaIndices[findCompatibleSnapArea(info, snapOffsetsHorizontal[*horizontal.second], ScrollEventAxis::Horizontal, viewportSize, scrollDestinationOffset)]];
auto verticalSnapArea = info.snapAreas[snapOffsetsVertical[*vertical.second].snapAreaIndices[findCompatibleSnapArea(info, snapOffsetsVertical[*vertical.second], ScrollEventAxis::Vertical, viewportSize, scrollDestinationOffset)]];

auto closerSnapArea = (horizontalSnapArea.x() - scrollDestinationOffset.x()) * (horizontalSnapArea.x() - scrollDestinationOffset.x()) + (horizontalSnapArea.y() - scrollDestinationOffset.y()) * (horizontalSnapArea.y() - scrollDestinationOffset.y()) > (verticalSnapArea.x() - scrollDestinationOffset.x()) * (verticalSnapArea.x() - scrollDestinationOffset.x()) + (verticalSnapArea.y() - scrollDestinationOffset.y()) * (verticalSnapArea.y() - scrollDestinationOffset.y()) ? verticalSnapArea : horizontalSnapArea;
horizontal = { closerSnapArea.x(), std::nullopt };
vertical = { closerSnapArea.y(), std::nullopt };
}
return axis == ScrollEventAxis::Horizontal ? horizontal : vertical;
}

template <> template <>
std::pair<LayoutUnit, std::optional<unsigned>> LayoutScrollSnapOffsetsInfo::closestSnapOffset(ScrollEventAxis axis, const LayoutSize& viewportSize, LayoutPoint scrollDestinationOffset, float velocity, std::optional<LayoutUnit> originalPositionForDirectionalSnapping) const
{
return closestSnapOffsetWithInfoAndAxis(*this, axis, viewportSize, scrollDestinationOffset, velocity, originalPositionForDirectionalSnapping);
auto horizontal = closestSnapOffsetWithInfoAndAxis(*this, ScrollEventAxis::Horizontal, viewportSize, scrollDestinationOffset, velocity, originalPositionForDirectionalSnapping);
auto vertical = closestSnapOffsetWithInfoAndAxis(*this, ScrollEventAxis::Vertical, viewportSize, scrollDestinationOffset, velocity, originalPositionForDirectionalSnapping);
return ensureVisibleTarget(*this, horizontal, vertical, axis, viewportSize, scrollDestinationOffset);
}

template <> template<>
std::pair<float, std::optional<unsigned>> FloatScrollSnapOffsetsInfo::closestSnapOffset(ScrollEventAxis axis, const FloatSize& viewportSize, FloatPoint scrollDestinationOffset, float velocity, std::optional<float> originalPositionForDirectionalSnapping) const
{
return closestSnapOffsetWithInfoAndAxis(*this, axis, viewportSize, scrollDestinationOffset, velocity, originalPositionForDirectionalSnapping);
auto horizontal = closestSnapOffsetWithInfoAndAxis(*this, ScrollEventAxis::Horizontal, viewportSize, scrollDestinationOffset, velocity, originalPositionForDirectionalSnapping);
auto vertical = closestSnapOffsetWithInfoAndAxis(*this, ScrollEventAxis::Vertical, viewportSize, scrollDestinationOffset, velocity, originalPositionForDirectionalSnapping);
return ensureVisibleTarget(*this, horizontal, vertical, axis, viewportSize, scrollDestinationOffset);
}

}

0 comments on commit 0efae6b

Please sign in to comment.