-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Overlay regions do not correctly handle sticky positioned scrolling nodes #3383
Conversation
61b666e
to
5096d86
Compare
@@ -94,7 +94,7 @@ class RemoteLayerTreeHost { | |||
HashMap<WebCore::GraphicsLayer::PlatformLayerID, std::unique_ptr<RemoteLayerTreeNode>> m_nodes; | |||
HashMap<WebCore::GraphicsLayer::PlatformLayerID, RetainPtr<WKAnimationDelegate>> m_animationDelegates; | |||
#if ENABLE(OVERLAY_REGIONS_IN_EVENT_REGION) | |||
HashMap<WebCore::GraphicsLayer::PlatformLayerID, CGRect> m_overlayRegionsWithIDs; | |||
HashMap<WebCore::GraphicsLayer::PlatformLayerID, const RemoteLayerTreeNode*> m_overlayRegionNodesWithIDs; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like a bad idea to store raw pointers here. Maybe a strong or weak reference?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, they're not refcounted...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(can you prove that this is safe?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh good point. I'll change this from a map to a set. The Map is storing <layerID, Node> but that is not needed, I can just store the layerID
@@ -555,11 +555,36 @@ - (CGPoint)_puic_contentOffsetForCrownInputSequencerOffset:(double)sequencerOffs | |||
#endif // HAVE(PEPPER_UI_CORE) | |||
|
|||
#if ENABLE(OVERLAY_REGIONS_IN_EVENT_REGION) | |||
- (bool)_updateOverlayRegions:(const Vector<CGRect> &)overlayRegions | |||
static void addDebugOverlays(CALayer *layer, const Vector<CGRect>& overlayRegions) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would probably land this in a separate patch; keeping the behavior change and the debugging UI separate.
5096d86
to
466ca8a
Compare
EWS run on previous version of this PR (hash 466ca8a)
|
EWS run on previous version of this PR (hash 5096d86)
|
EWS run on previous version of this PR (hash 61b666e)
|
466ca8a
to
b7a7eb3
Compare
EWS run on previous version of this PR (hash b7a7eb3)
|
EWS run on previous version of this PR (hash 5096d86)
|
EWS run on previous version of this PR (hash 61b666e)
|
b7a7eb3
to
dbe0f34
Compare
|
EWS run on previous version of this PR (hash 5096d86)
|
EWS run on previous version of this PR (hash 61b666e)
|
…odes https://bugs.webkit.org/show_bug.cgi?id=243973 <rdar://98640826> Reviewed by Tim Horton. Sticky node regions should be added to the overlay regions but their rectangles can move during scrolling. Also fixed or sticky rectangles can change during page zoom. To support this, instead of storing a list of rectangles, store a list of layer IDs with event regions and then compute the rectangles. * Source/WebKit/UIProcess/API/ios/WKWebViewIOS.mm: (addOverlayEventRegions): Store only the layer ID. (-[WKWebView _updateOverlayRegions:destroyedLayers:]): Recompute the rectangles from the current frames of the nodes whenever the layer properties change. Scrolling and page zoom where this would be impacted both trigger a change in layer properties. * Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeDrawingAreaProxy.h: Renamed function. * Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeHost.h: (WebKit::RemoteLayerTreeHost::overlayRegionIDs const): (WebKit::RemoteLayerTreeHost::updateOverlayRegionIDs): Renamed. * Source/WebKit/UIProcess/RemoteLayerTree/ios/RemoteScrollingCoordinatorProxyIOS.mm: (WebKit::RemoteScrollingCoordinatorProxy::connectStateNodeLayers): Call renamed function. Canonical link: https://commits.webkit.org/253603@main
dbe0f34
to
649ae3d
Compare
Committed 253603@main (649ae3d): https://commits.webkit.org/253603@main Reviewed commits have been landed. Closing PR #3383 and removing active labels. |
649ae3d
dbe0f34
🧪 ios-wk2🧪 api-ios🧪 api-mac🧪 mac-wk1🧪 mac-AS-debug-wk2