Skip to content

Commit

Permalink
[Ui-side compositing] Fix mouse position reporting to AppKit in WebSc…
Browse files Browse the repository at this point in the history
…rollerImpPairDelegateMac

https://bugs.webkit.org/show_bug.cgi?id=255188
rdar://107776583

Reviewed by Simon Fraser.

This patch aims to fix several issues with the way ScrollerPairMac and ScrollerMac handle
mouse events, with the intent of fixing hovering over overlay scrollbars in overflow scrolling
nodes. The first issue is that currently there isn't a good way to convert the frame relative
mouse position to a scrollbar relative mouse position. To work around this, bundle the scrollbar
relative mouse positions in the mouseMovedInContentArea event. When we know that we have updated
mouse positions from the WebProcess, return them in the various WebScrollerImpPairDelegateMac
functions. The second issue is that RemoteScrollingTree is only calling handleMouseEvent
on the root scroller. Since this mouse position was only being used as input to convertContentPoint,
it is ok to just return a zero point and use the converted mouse position from the Web Process.

* Source/WebCore/page/scrolling/ScrollingTree.h:
(WebCore::ScrollingTree::lastKnownMousePosition):
* Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.cpp:
(WebCore::ScrollingTreeScrollingNode::lastKnownMousePosition const):
* Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.h:
* Source/WebCore/page/scrolling/ScrollingTreeScrollingNodeDelegate.h:
(WebCore::ScrollingTreeScrollingNodeDelegate::lastKnownMousePosition const):
* Source/WebCore/page/scrolling/mac/ScrollerMac.mm:
(-[WebScrollerImpDelegateMac mouseLocationInScrollerForScrollerImp:]):
* Source/WebCore/page/scrolling/mac/ScrollerPairMac.h:
(WebCore::ScrollerPairMac::node const):
(WebCore::ScrollerPairMac::lastKnownMousePosition const): Deleted.
* Source/WebCore/page/scrolling/mac/ScrollerPairMac.mm:
(-[WebScrollerImpPairDelegateMac mouseLocationInContentAreaForScrollerImpPair:]):
(-[WebScrollerImpPairDelegateMac scrollerImpPair:convertContentPoint:toScrollerImp:]):
(WebCore::ScrollerPairMac::lastKnownMousePosition const):
* Source/WebCore/page/scrolling/mac/ScrollingTreeScrollingNodeDelegateMac.h:
* Source/WebCore/page/scrolling/mac/ScrollingTreeScrollingNodeDelegateMac.mm:
(WebCore::ScrollingTreeScrollingNodeDelegateMac::lastKnownMousePosition const):
* Source/WebKit/UIProcess/RemoteLayerTree/mac/RemoteScrollingTreeMac.h:
* Source/WebKit/UIProcess/RemoteLayerTree/mac/RemoteScrollingTreeMac.mm:
(WebKit::RemoteScrollingTreeMac::lastKnownMousePosition):
* Source/WebKit/UIProcess/RemoteLayerTree/mac/ScrollingTreeFrameScrollingNodeRemoteMac.cpp:
(WebKit::ScrollingTreeFrameScrollingNodeRemoteMac::lastKnownMousePosition):
* Source/WebKit/UIProcess/RemoteLayerTree/mac/ScrollingTreeFrameScrollingNodeRemoteMac.h:

Canonical link: https://commits.webkit.org/262995@main
  • Loading branch information
nmoucht committed Apr 14, 2023
1 parent a132303 commit caec1d3
Show file tree
Hide file tree
Showing 15 changed files with 69 additions and 68 deletions.
8 changes: 7 additions & 1 deletion Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp
Expand Up @@ -384,7 +384,13 @@ void AsyncScrollingCoordinator::setMouseMovedInContentArea(ScrollableArea& scrol
auto stateNode = dynamicDowncast<ScrollingStateScrollingNode>(m_scrollingStateTree->stateNodeForID(scrollingNodeID));
if (!stateNode)
return;
stateNode->setMouseMovedInContentArea();

auto mousePosition = scrollableArea.lastKnownMousePositionInView();
auto horizontalScrollbar = scrollableArea.horizontalScrollbar();
auto verticalScrollbar = scrollableArea.verticalScrollbar();

MouseLocationState state = { horizontalScrollbar ? horizontalScrollbar->convertFromContainingView(mousePosition) : IntPoint(), verticalScrollbar ? verticalScrollbar->convertFromContainingView(mousePosition) : IntPoint() };
stateNode->setMouseMovedInContentArea(state);
}

bool AsyncScrollingCoordinator::requestAnimatedScrollToPosition(ScrollableArea& scrollableArea, const ScrollPosition& scrollPosition, ScrollClamping clamping)
Expand Down
Expand Up @@ -49,6 +49,7 @@ ScrollingStateScrollingNode::ScrollingStateScrollingNode(const ScrollingStateScr
, m_snapOffsetsInfo(stateNode.m_snapOffsetsInfo)
#if PLATFORM(MAC)
, m_scrollbarHoverState(stateNode.scrollbarHoverState())
, m_mouseLocationState(stateNode.mouseLocationState())
, m_verticalScrollerImp(stateNode.verticalScrollerImp())
, m_horizontalScrollerImp(stateNode.horizontalScrollerImp())
#endif
Expand Down Expand Up @@ -286,8 +287,9 @@ void ScrollingStateScrollingNode::setMouseIsOverContentArea(bool flag)
setPropertyChanged(Property::ContentAreaHoverState);
}

void ScrollingStateScrollingNode::setMouseMovedInContentArea()
void ScrollingStateScrollingNode::setMouseMovedInContentArea(const MouseLocationState& mouseLocationState)
{
m_mouseLocationState = mouseLocationState;
setPropertyChanged(Property::MouseActivityState);
}

Expand Down
9 changes: 8 additions & 1 deletion Source/WebCore/page/scrolling/ScrollingStateScrollingNode.h
Expand Up @@ -53,6 +53,11 @@ struct ScrollbarHoverState {
}
};

struct MouseLocationState {
IntPoint locationInHorizontalScrollbar;
IntPoint locationInVerticalScrollbar;
};

class ScrollingStateScrollingNode : public ScrollingStateNode {
public:
virtual ~ScrollingStateScrollingNode();
Expand Down Expand Up @@ -128,7 +133,8 @@ class ScrollingStateScrollingNode : public ScrollingStateNode {
WEBCORE_EXPORT void setMouseIsOverContentArea(bool);
bool mouseIsOverContentArea() const { return m_mouseIsOverContentArea; }

WEBCORE_EXPORT void setMouseMovedInContentArea();
WEBCORE_EXPORT void setMouseMovedInContentArea(const MouseLocationState&);
const MouseLocationState& mouseLocationState() const { return m_mouseLocationState; }

protected:
ScrollingStateScrollingNode(ScrollingStateTree&, ScrollingNodeType, ScrollingNodeID);
Expand All @@ -154,6 +160,7 @@ class ScrollingStateScrollingNode : public ScrollingStateNode {
LayerRepresentation m_verticalScrollbarLayer;

ScrollbarHoverState m_scrollbarHoverState;
MouseLocationState m_mouseLocationState;

#if PLATFORM(MAC)
RetainPtr<NSScrollerImp> m_verticalScrollerImp;
Expand Down
Expand Up @@ -48,7 +48,6 @@ class ScrollingTreeScrollingNodeDelegate {
virtual void updateFromStateNode(const ScrollingStateScrollingNode&) { }

virtual void handleWheelEventPhase(const PlatformWheelEventPhase) { }
virtual bool handleMouseEventForScrollbars(const PlatformMouseEvent&) { return false; }

virtual void viewWillStartLiveResize() { }
virtual void viewWillEndLiveResize() { }
Expand Down
7 changes: 4 additions & 3 deletions Source/WebCore/page/scrolling/mac/ScrollerMac.h
Expand Up @@ -61,18 +61,19 @@ class ScrollerMac {
void updateScrollbarStyle();
void updatePairScrollerImps();

FloatPoint convertFromContent(const FloatPoint&) const;

void updateValues();

String scrollbarState() const;

void mouseEnteredScrollbar();
void mouseExitedScrollbar();


void setLastKnownMousePositionInScrollbar(IntPoint position) { m_lastKnownMousePositionInScrollbar = position; }
IntPoint lastKnownMousePositionInScrollbar() const;
private:
ScrollerPairMac& m_pair;
const ScrollbarOrientation m_orientation;
IntPoint m_lastKnownMousePositionInScrollbar;

RetainPtr<CALayer> m_hostLayer;
RetainPtr<NSScrollerImp> m_scrollerImp;
Expand Down
17 changes: 11 additions & 6 deletions Source/WebCore/page/scrolling/mac/ScrollerMac.mm
Expand Up @@ -30,6 +30,7 @@

#import "ScrollTypesMac.h"
#import "ScrollerPairMac.h"
#import "ScrollingTreeScrollingNode.h"
#import <QuartzCore/CALayer.h>
#import <WebCore/FloatPoint.h>
#import <WebCore/IntRect.h>
Expand Down Expand Up @@ -181,7 +182,7 @@ - (NSPoint)mouseLocationInScrollerForScrollerImp:(NSScrollerImp *)scrollerImp

ASSERT_UNUSED(scrollerImp, scrollerImp == _scroller->scrollerImp());

return _scroller->convertFromContent(_scroller->pair().lastKnownMousePosition());
return _scroller->lastKnownMousePositionInScrollbar();
}

- (NSRect)convertRectToLayer:(NSRect)rect
Expand Down Expand Up @@ -367,11 +368,6 @@ - (void)invalidate
updatePairScrollerImps();
}

FloatPoint ScrollerMac::convertFromContent(const FloatPoint& point) const
{
return FloatPoint { [m_hostLayer convertPoint:point fromLayer:[m_hostLayer superlayer]] };
}

void ScrollerMac::updatePairScrollerImps()
{
NSScrollerImp *scrollerImp = m_hostLayer ? m_scrollerImp.get() : nil;
Expand Down Expand Up @@ -409,6 +405,15 @@ - (void)invalidate
});
}

IntPoint ScrollerMac::lastKnownMousePositionInScrollbar() const
{
// When we dont have an update from the Web Process, return
// a point outside of the scrollbars
if (!m_pair.mouseInContentArea())
return { -1, -1 };
return m_lastKnownMousePositionInScrollbar;
}

String ScrollerMac::scrollbarState() const
{
if (!m_hostLayer || !m_scrollerImp)
Expand Down
11 changes: 5 additions & 6 deletions Source/WebCore/page/scrolling/mac/ScrollerPairMac.h
Expand Up @@ -40,7 +40,6 @@ OBJC_CLASS NSScrollerImpPair;
OBJC_CLASS WebScrollerImpPairDelegateMac;

namespace WebCore {
class PlatformMouseEvent;
class PlatformWheelEvent;
class ScrollingTreeScrollingNode;
}
Expand All @@ -62,7 +61,6 @@ class ScrollerPairMac : public ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr<S
~ScrollerPairMac();

void handleWheelEventPhase(PlatformWheelEventPhase);
bool handleMouseEvent(const PlatformMouseEvent&);

void setUsePresentationValues(bool);
bool isUsingPresentationValues() const { return m_usingPresentationValues; }
Expand All @@ -73,7 +71,6 @@ class ScrollerPairMac : public ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr<S
void updateValues();

FloatSize visibleSize() const;
IntPoint lastKnownMousePosition() const { return m_lastKnownMousePosition; }
bool useDarkAppearance() const;

struct Values {
Expand Down Expand Up @@ -105,12 +102,14 @@ class ScrollerPairMac : public ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr<S

void mouseEnteredContentArea();
void mouseExitedContentArea();
void mouseMovedInContentArea();
void mouseMovedInContentArea(const MouseLocationState&);
void mouseIsInScrollbar(ScrollbarHoverState);

NSScrollerImpPair *scrollerImpPair() const { return m_scrollerImpPair.get(); }
void ensureOnMainThreadWithProtectedThis(Function<void()>&&);

ScrollingTreeScrollingNode& node() const { return m_scrollingNode; }

bool mouseInContentArea() const { return m_mouseInContentArea; }
private:
ScrollerPairMac(ScrollingTreeScrollingNode&);

Expand All @@ -125,7 +124,6 @@ class ScrollerPairMac : public ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr<S
ScrollerMac m_verticalScroller;
ScrollerMac m_horizontalScroller;

IntPoint m_lastKnownMousePosition;
std::optional<FloatPoint> m_lastScrollOffset;

RetainPtr<NSScrollerImpPair> m_scrollerImpPair;
Expand All @@ -134,6 +132,7 @@ class ScrollerPairMac : public ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr<S
std::atomic<bool> m_usingPresentationValues { false };
std::atomic<ScrollbarStyle> m_scrollbarStyle { ScrollbarStyle::AlwaysVisible };
bool m_inLiveResize { false };
bool m_mouseInContentArea { false };
};

}
Expand Down
34 changes: 12 additions & 22 deletions Source/WebCore/page/scrolling/mac/ScrollerPairMac.mm
Expand Up @@ -30,10 +30,10 @@

#import "Logging.h"
#import "ScrollTypesMac.h"
#import "ScrollingTreeFrameScrollingNode.h"
#import <WebCore/FloatPoint.h>
#import <WebCore/IntRect.h>
#import <WebCore/NSScrollerImpDetails.h>
#import <WebCore/PlatformMouseEvent.h>
#import <WebCore/PlatformWheelEvent.h>
#import <WebCore/ScrollTypes.h>
#import <WebCore/ScrollableArea.h>
Expand Down Expand Up @@ -81,15 +81,15 @@ - (BOOL)inLiveResizeForScrollerImpPair:(NSScrollerImpPair *)scrollerImpPair
- (NSPoint)mouseLocationInContentAreaForScrollerImpPair:(NSScrollerImpPair *)scrollerImpPair
{
UNUSED_PARAM(scrollerImpPair);
if (!_scrollerPair)
return NSZeroPoint;

return _scrollerPair->lastKnownMousePosition();
// This location is only used when calling mouseLocationInScrollerForScrollerImp,
// where we will use the converted mouse position from the Web Process
return NSZeroPoint;
}

- (NSPoint)scrollerImpPair:(NSScrollerImpPair *)scrollerImpPair convertContentPoint:(NSPoint)pointInContentArea toScrollerImp:(NSScrollerImp *)scrollerImp
{
UNUSED_PARAM(scrollerImpPair);
UNUSED_PARAM(pointInContentArea);

if (!_scrollerPair || !scrollerImp)
return NSZeroPoint;
Expand All @@ -102,7 +102,7 @@ - (NSPoint)scrollerImpPair:(NSScrollerImpPair *)scrollerImpPair convertContentPo

ASSERT(scrollerImp == scroller->scrollerImp());

return scroller->convertFromContent(WebCore::IntPoint(pointInContentArea));
return scroller->lastKnownMousePositionInScrollbar();
}

- (void)scrollerImpPair:(NSScrollerImpPair *)scrollerImpPair setContentAreaNeedsDisplayInRect:(NSRect)rect
Expand Down Expand Up @@ -213,21 +213,6 @@ - (void)scrollerImpPair:(NSScrollerImpPair *)scrollerImpPair updateScrollerStyle
});
}

bool ScrollerPairMac::handleMouseEvent(const PlatformMouseEvent& event)
{
if (event.type() != PlatformEvent::Type::MouseMoved)
return false;

m_lastKnownMousePosition = event.position();

ensureOnMainThreadWithProtectedThis([this] {
[m_scrollerImpPair mouseMovedInContentArea];
});

// FIXME: this needs to return whether the event was handled.
return true;
}

void ScrollerPairMac::setUsePresentationValues(bool inMomentumPhase)
{
m_usingPresentationValues = inMomentumPhase;
Expand Down Expand Up @@ -362,6 +347,7 @@ - (void)scrollerImpPair:(NSScrollerImpPair *)scrollerImpPair updateScrollerStyle

void ScrollerPairMac::mouseExitedContentArea()
{
m_mouseInContentArea = false;
LOG_WITH_STREAM(OverlayScrollbars, stream << "ScrollerPairMac for [" << m_scrollingNode.scrollingNodeID() << "] mouseExitedContentArea");

ensureOnMainThreadWithProtectedThis([this] {
Expand All @@ -372,8 +358,12 @@ - (void)scrollerImpPair:(NSScrollerImpPair *)scrollerImpPair updateScrollerStyle
});
}

void ScrollerPairMac::mouseMovedInContentArea()
void ScrollerPairMac::mouseMovedInContentArea(const MouseLocationState& state)
{
m_mouseInContentArea = true;
horizontalScroller().setLastKnownMousePositionInScrollbar(state.locationInHorizontalScrollbar);
verticalScroller().setLastKnownMousePositionInScrollbar(state.locationInVerticalScrollbar);

ensureOnMainThreadWithProtectedThis([this] {
if ([m_scrollerImpPair overlayScrollerStateIsLocked])
return;
Expand Down
Expand Up @@ -61,7 +61,6 @@ class ScrollingTreeScrollingNodeDelegateMac final : public ThreadedScrollingTree
void updateScrollbarLayers() final;

void handleWheelEventPhase(const PlatformWheelEventPhase) final;
bool handleMouseEventForScrollbars(const PlatformMouseEvent&) final;
void viewWillStartLiveResize() final;
void viewWillEndLiveResize() final;
void viewSizeDidChange() final;
Expand Down
Expand Up @@ -92,7 +92,7 @@
}

if (scrollingStateNode.hasChangedProperty(ScrollingStateNode::Property::MouseActivityState))
m_scrollerPair->mouseMovedInContentArea();
m_scrollerPair->mouseMovedInContentArea(scrollingStateNode.mouseLocationState());

m_scrollerPair->updateValues();

Expand Down Expand Up @@ -328,11 +328,6 @@
m_scrollerPair->handleWheelEventPhase(wheelEventPhase);
}

bool ScrollingTreeScrollingNodeDelegateMac::handleMouseEventForScrollbars(const PlatformMouseEvent& mouseEvent)
{
return m_scrollerPair->handleMouseEvent(mouseEvent);
}

void ScrollingTreeScrollingNodeDelegateMac::viewWillStartLiveResize()
{
return m_scrollerPair->viewWillStartLiveResize();
Expand Down
Expand Up @@ -171,6 +171,12 @@ void ArgumentCoder<ScrollingStateScrollingNode>::encode(Encoder& encoder, const
encoder << mouseIsInScrollbar.mouseIsOverHorizontalScrollbar;
encoder << mouseIsInScrollbar.mouseIsOverVerticalScrollbar;
}

if (node.hasChangedProperty(ScrollingStateNode::Property::MouseActivityState)) {
auto mouseLocationState = node.mouseLocationState();
encoder << mouseLocationState.locationInHorizontalScrollbar;
encoder << mouseLocationState.locationInVerticalScrollbar;
}
}

void ArgumentCoder<ScrollingStateFrameScrollingNode>::encode(Encoder& encoder, const ScrollingStateFrameScrollingNode& node)
Expand Down Expand Up @@ -264,7 +270,7 @@ bool ArgumentCoder<ScrollingStateScrollingNode>::decode(Decoder& decoder, Scroll
node.setRequestedScrollData(WTFMove(requestedScrollData), ScrollingStateScrollingNode::CanMergeScrollData::No);
}
SCROLLING_NODE_DECODE(ScrollingStateNode::Property::KeyboardScrollData, RequestedKeyboardScrollData, setKeyboardScrollData);
SCROLLING_NODE_DECODE(ScrollingStateNode::Property::ContentAreaHoverState, bool, setMouseIsOverContentArea)
SCROLLING_NODE_DECODE(ScrollingStateNode::Property::ContentAreaHoverState, bool, setMouseIsOverContentArea);

if (node.hasChangedProperty(ScrollingStateNode::Property::ScrollContainerLayer)) {
std::optional<PlatformLayerIdentifier> layerID;
Expand Down Expand Up @@ -304,6 +310,17 @@ bool ArgumentCoder<ScrollingStateScrollingNode>::decode(Decoder& decoder, Scroll
return false;
node.setScrollbarHoverState({ didEnterScrollbarHorizontal, didEnterScrollbarVertical });
}

if (node.hasChangedProperty(ScrollingStateNode::Property::MouseActivityState)) {
IntPoint locationInHorizontalScrollbar;
if (!decoder.decode(locationInHorizontalScrollbar))
return false;

IntPoint locationInVerticalScrollbar;
if (!decoder.decode(locationInVerticalScrollbar))
return false;
node.setMouseMovedInContentArea({ locationInHorizontalScrollbar, locationInVerticalScrollbar });
}

return true;
}
Expand Down
Expand Up @@ -46,8 +46,6 @@ class RemoteScrollingTreeMac final : public RemoteScrollingTree {
explicit RemoteScrollingTreeMac(RemoteScrollingCoordinatorProxy&);
virtual ~RemoteScrollingTreeMac();

void handleMouseEvent(const WebCore::PlatformMouseEvent&) final;

private:
void handleWheelEventPhase(WebCore::ScrollingNodeID, WebCore::PlatformWheelEventPhase) override;
RefPtr<WebCore::ScrollingTreeNode> scrollingNodeForPoint(WebCore::FloatPoint) override;
Expand Down
Expand Up @@ -106,13 +106,6 @@
return ScrollingTreeFixedNodeCocoa::create(*this, nodeID);
}

void RemoteScrollingTreeMac::handleMouseEvent(const PlatformMouseEvent& event)
{
if (!rootNode())
return;
static_cast<ScrollingTreeFrameScrollingNodeRemoteMac&>(*rootNode()).handleMouseEvent(event);
}

void RemoteScrollingTreeMac::didCommitTree()
{
ASSERT(isMainRunLoop());
Expand Down
Expand Up @@ -59,11 +59,6 @@ void ScrollingTreeFrameScrollingNodeRemoteMac::handleWheelEventPhase(const Platf
m_delegate->handleWheelEventPhase(phase);
}

bool ScrollingTreeFrameScrollingNodeRemoteMac::handleMouseEvent(const PlatformMouseEvent& mouseEvent)
{
return m_delegate->handleMouseEventForScrollbars(mouseEvent);
}

void ScrollingTreeFrameScrollingNodeRemoteMac::viewWillStartLiveResize()
{
m_delegate->viewWillStartLiveResize();
Expand Down
Expand Up @@ -61,11 +61,6 @@ void ScrollingTreeOverflowScrollingNodeRemoteMac::handleWheelEventPhase(const Pl
m_delegate->handleWheelEventPhase(phase);
}

bool ScrollingTreeOverflowScrollingNodeRemoteMac::handleMouseEvent(const PlatformMouseEvent& mouseEvent)
{
return m_delegate->handleMouseEventForScrollbars(mouseEvent);
}

String ScrollingTreeOverflowScrollingNodeRemoteMac::scrollbarStateForOrientation(ScrollbarOrientation orientation) const
{
return m_delegate->scrollbarStateForOrientation(orientation);
Expand Down

0 comments on commit caec1d3

Please sign in to comment.