Skip to content

Commit

Permalink
[ScrollingTree mac] Fix scrolling when event region and scroll contai…
Browse files Browse the repository at this point in the history
…ner are distinct

https://bugs.webkit.org/show_bug.cgi?id=258255
rdar://110954175

Reviewed by Simon Fraser.

The current code (mac) assumes that the platform layer which reacts to
a scroll event is also the one containing the scroll container but it's not always the case
(like WPT example: when the scroll container has pointer-event:none but some child still reacts to event).

* LayoutTests/imported/w3c/web-platform-tests/pointerevents/pointerevent_hit_test_scroll_visible_descendant-expected.txt:
* Source/WebCore/page/scrolling/mac/ScrollingTreeMac.mm:
(ScrollingTreeMac::scrollingNodeForPoint):
* Source/WebCore/platform/graphics/cocoa/WebCoreCALayerExtras.h:
(WebCore::collectDescendantLayersAtPoint):
* Source/WebCore/platform/graphics/cocoa/WebCoreCALayerExtras.mm:
(WebCore::collectDescendantLayersAtPoint):
(WebCore::layersAtPointToCheckForScrolling):
* Source/WebKit/UIProcess/RemoteLayerTree/mac/RemoteScrollingTreeMac.mm:
(WebKit::RemoteScrollingTreeMac::scrollingNodeForPoint):

Canonical link: https://commits.webkit.org/270094@main
  • Loading branch information
mdubet committed Nov 1, 2023
1 parent d31887d commit 2b7aa25
Show file tree
Hide file tree
Showing 5 changed files with 89 additions and 27 deletions.
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@

FAIL Wheel-scroll over pointer-events: auto descendant scrolls pointer-events: none scroller. assert_equals: Incorrect element scrolled expected "overlay" but got "root"
PASS Wheel-scroll over pointer-events: auto descendant scrolls pointer-events: none scroller.

46 changes: 33 additions & 13 deletions Source/WebCore/page/scrolling/mac/ScrollingTreeMac.mm
Original file line number Diff line number Diff line change
Expand Up @@ -123,38 +123,58 @@ static bool isScrolledBy(const ScrollingTree& tree, ScrollingNodeID scrollingNod

RefPtr<ScrollingTreeNode> ScrollingTreeMac::scrollingNodeForPoint(FloatPoint point)
{
auto* rootScrollingNode = rootNode();
RefPtr rootScrollingNode = rootNode();
if (!rootScrollingNode)
return nullptr;

Locker locker { m_layerHitTestMutex };

auto rootContentsLayer = static_cast<ScrollingTreeFrameScrollingNodeMac*>(rootScrollingNode)->rootContentsLayer();
auto rootContentsLayer = static_cast<ScrollingTreeFrameScrollingNodeMac*>(rootScrollingNode.get())->rootContentsLayer();
FloatPoint scrollOrigin = rootScrollingNode->scrollOrigin();
auto pointInContentsLayer = point;
pointInContentsLayer.moveBy(scrollOrigin);

Vector<LayerAndPoint, 16> layersAtPoint;
collectDescendantLayersAtPoint(layersAtPoint, rootContentsLayer.get(), pointInContentsLayer, layerEventRegionContainsPoint);
bool hasAnyNonInteractiveScrollingLayers = false;
auto layersAtPoint = layersAtPointToCheckForScrolling(layerEventRegionContainsPoint, scrollingNodeIDForLayer, rootContentsLayer.get(), pointInContentsLayer, hasAnyNonInteractiveScrollingLayers);

LOG_WITH_STREAM(Scrolling, stream << "ScrollingTreeMac " << this << " scrollingNodeForPoint " << point << " found " << layersAtPoint.size() << " layers");
#if !LOG_DISABLED
for (auto [layer, point] : WTF::makeReversedRange(layersAtPoint))
for (auto [layer, point] : layersAtPoint)
LOG_WITH_STREAM(Scrolling, stream << " layer " << [layer description] << " scrolling node " << scrollingNodeIDForLayer(layer));
#endif

if (layersAtPoint.size()) {
auto* frontmostLayer = layersAtPoint.last().first;
for (auto [layer, point] : WTF::makeReversedRange(layersAtPoint)) {
auto nodeID = scrollingNodeIDForLayer(layer);

auto* scrollingNode = nodeForID(nodeID);
if (!is<ScrollingTreeScrollingNode>(scrollingNode))
auto* frontmostLayer = layersAtPoint.first().first;
for (size_t i = 0 ; i < layersAtPoint.size() ; i++) {
auto [layer, point] = layersAtPoint[i];

if (!layerEventRegionContainsPoint(layer, point))
continue;

if (isScrolledBy(*this, nodeID, frontmostLayer))

auto scrollingNodeForLayer = [&] (auto layer, auto point) -> RefPtr<ScrollingTreeNode> {
UNUSED_PARAM(point);
auto nodeID = scrollingNodeIDForLayer(layer);
RefPtr scrollingNode = nodeForID(nodeID);
if (!is<ScrollingTreeScrollingNode>(scrollingNode))
return nullptr;
if (isScrolledBy(*this, nodeID, frontmostLayer)) {
LOG_WITH_STREAM(Scrolling, stream << "ScrollingTreeMac " << this << " scrollingNodeForPoint " << point << " found scrolling node " << nodeID);
return scrollingNode;
}
return nullptr;
};

if (RefPtr scrollingNode = scrollingNodeForLayer(layer, point))
return scrollingNode;

// This layer may be scrolled by some other layer further back which may itself be non-interactive.
if (hasAnyNonInteractiveScrollingLayers) {
for (size_t j = i + 1; j < layersAtPoint.size() ; j++) {
auto [behindLayer, behindPoint] = layersAtPoint[j];
if (RefPtr scrollingNode = scrollingNodeForLayer(behindLayer, behindPoint))
return scrollingNode;
}
}
// FIXME: Hit-test scroll indicator layers.
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,9 @@
namespace WebCore {

using LayerAndPoint = std::pair<CALayer *, FloatPoint>;
WEBCORE_EXPORT void collectDescendantLayersAtPoint(Vector<LayerAndPoint, 16>& layersAtPoint, CALayer *parent, CGPoint, const std::function<bool(CALayer *, CGPoint localPoint)>& pointInLayerFunction);
WEBCORE_EXPORT void collectDescendantLayersAtPoint(Vector<LayerAndPoint, 16>& layersAtPoint, CALayer *parent, CGPoint, const std::function<bool(CALayer *, CGPoint localPoint)>& pointInLayerFunction = { });

WEBCORE_EXPORT Vector<LayerAndPoint, 16> layersAtPointToCheckForScrolling(std::function<bool(CALayer*, CGPoint)> layerEventRegionContainsPoint, std::function<uint64_t(CALayer*)> scrollingNodeIDForLayer, CALayer*, const FloatPoint&, bool& hasAnyNonInteractiveScrollingLayers);

} // namespace WebCore

Expand Down
22 changes: 21 additions & 1 deletion Source/WebCore/platform/graphics/cocoa/WebCoreCALayerExtras.mm
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,10 @@ void collectDescendantLayersAtPoint(Vector<LayerAndPoint, 16>& layersAtPoint, CA
if (![layerWithResolvedAnimations containsPoint:subviewPoint])
return false;

return pointInLayerFunction(layer, subviewPoint);
if (pointInLayerFunction)
return pointInLayerFunction(layer, subviewPoint);

return true;
}();

if (handlesEvent)
Expand All @@ -185,4 +188,21 @@ void collectDescendantLayersAtPoint(Vector<LayerAndPoint, 16>& layersAtPoint, CA
};
}

Vector<LayerAndPoint, 16> layersAtPointToCheckForScrolling(std::function<bool(CALayer*, CGPoint)> layerEventRegionContainsPoint, std::function<uint64_t(CALayer*)> scrollingNodeIDForLayer, CALayer* layer, const FloatPoint& point, bool& hasAnyNonInteractiveScrollingLayers)
{
Vector<LayerAndPoint, 16> layersAtPoint;
collectDescendantLayersAtPoint(layersAtPoint, layer, point, [&] (auto layer, auto point) {
if (layerEventRegionContainsPoint(layer, point))
return true;
if (scrollingNodeIDForLayer(layer)) {
hasAnyNonInteractiveScrollingLayers = true;
return true;
}
return false;
});
// Hit-test front to back.
layersAtPoint.reverse();
return layersAtPoint;
}

} // namespace WebCore
Original file line number Diff line number Diff line change
Expand Up @@ -465,7 +465,7 @@ static bool layerEventRegionContainsPoint(CALayer *layer, CGPoint localPoint)

RefPtr<ScrollingTreeNode> RemoteScrollingTreeMac::scrollingNodeForPoint(FloatPoint point)
{
auto* rootScrollingNode = rootNode();
RefPtr rootScrollingNode = rootNode();
if (!rootScrollingNode)
return nullptr;

Expand All @@ -477,28 +477,48 @@ static bool layerEventRegionContainsPoint(CALayer *layer, CGPoint localPoint)
auto pointInContentsLayer = point;
pointInContentsLayer.moveBy(rootContentsLayerPosition);

Vector<LayerAndPoint, 16> layersAtPoint;
collectDescendantLayersAtPoint(layersAtPoint, scrolledContentsLayer.get(), pointInContentsLayer, layerEventRegionContainsPoint);
bool hasAnyNonInteractiveScrollingLayers = false;
auto layersAtPoint = layersAtPointToCheckForScrolling(layerEventRegionContainsPoint, scrollingNodeIDForLayer, scrolledContentsLayer.get(), pointInContentsLayer, hasAnyNonInteractiveScrollingLayers);

LOG_WITH_STREAM(UIHitTesting, stream << "RemoteScrollingTreeMac " << this << " scrollingNodeForPoint " << point << " (converted to layer point " << pointInContentsLayer << ") found " << layersAtPoint.size() << " layers");
#if !LOG_DISABLED
for (auto [layer, point] : WTF::makeReversedRange(layersAtPoint))
for (auto [layer, point] : layersAtPoint)
LOG_WITH_STREAM(UIHitTesting, stream << " layer " << [layer description] << " scrolling node " << scrollingNodeIDForLayer(layer));
#endif

if (layersAtPoint.size()) {
auto* frontmostLayer = layersAtPoint.last().first;
for (auto [layer, point] : WTF::makeReversedRange(layersAtPoint)) {
auto nodeID = scrollingNodeIDForLayer(layer);
auto* frontmostLayer = layersAtPoint.first().first;
for (size_t i = 0 ; i < layersAtPoint.size() ; i++) {
auto [layer, point] = layersAtPoint[i];

auto* scrollingNode = nodeForID(nodeID);
if (!is<ScrollingTreeScrollingNode>(scrollingNode))
if (!layerEventRegionContainsPoint(layer, point))
continue;

if (isScrolledBy(*this, nodeID, frontmostLayer)) {
LOG_WITH_STREAM(UIHitTesting, stream << "RemoteScrollingTreeMac " << this << " scrollingNodeForPoint " << point << " found scrolling node " << nodeID);
auto scrollingNodeForLayer = [&] (auto layer, auto point) -> RefPtr<ScrollingTreeNode> {
UNUSED_PARAM(point);
auto nodeID = scrollingNodeIDForLayer(layer);
RefPtr scrollingNode = nodeForID(nodeID);
if (!is<ScrollingTreeScrollingNode>(scrollingNode))
return nullptr;
if (isScrolledBy(*this, nodeID, frontmostLayer)) {
LOG_WITH_STREAM(UIHitTesting, stream << "RemoteScrollingTreeMac " << this << " scrollingNodeForPoint " << point << " found scrolling node " << nodeID);
return scrollingNode;
}
return nullptr;
};

if (RefPtr scrollingNode = scrollingNodeForLayer(layer, point))
return scrollingNode;

// This layer may be scrolled by some other layer further back which may itself be non-interactive.
if (hasAnyNonInteractiveScrollingLayers) {
for (size_t j = i + 1; j < layersAtPoint.size() ; j++) {
auto [behindLayer, behindPoint] = layersAtPoint[j];
if (RefPtr scrollingNode = scrollingNodeForLayer(behindLayer, behindPoint))
return scrollingNode;
}
}

}
}

Expand Down

0 comments on commit 2b7aa25

Please sign in to comment.