Add more code.
Created attachment 399139 [details] patch
Created attachment 399142 [details] patch
Comment on attachment 399142 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=399142&action=review > Source/WebCore/page/scrolling/mac/ScrollingTreeMac.mm:106 > + return eventRegion && eventRegion->contains(IntPoint(localPoint)); Maybe roundedIntPoint()? > Source/WebCore/page/scrolling/mac/ScrollingTreeMac.mm:216 > + return eventRegion->eventListenerRegionTypesForPoint(IntPoint(point)); roundedIntPoint?
Comment on attachment 399142 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=399142&action=review Is IntPoint right here? I’d like to understand better some day when it should be int, float, CGFloat/double, or LayoutUnit. > Source/WebCore/page/scrolling/mac/ScrollingTreeMac.mm:194 > + LockHolder lockHolder(m_layerHitTestMutex); Does this lock need to be held all the way to the end? I don’t fully understand what is being guarded. > Source/WebCore/page/scrolling/mac/ScrollingTreeMac.mm:208 > + auto platformCALayer = PlatformCALayer::platformCALayerForLayer((__bridge void*)hitLayer); Seems like we eventually need to fix this function so we don’t have to cast to void* to use it. > Source/WebCore/platform/graphics/ca/cocoa/PlatformCALayerCocoa.h:171 > + const EventRegion* eventRegion() const override { return &m_eventRegion; } final?
Comment on attachment 399142 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=399142&action=review >> Source/WebCore/page/scrolling/mac/ScrollingTreeMac.mm:194 >> + LockHolder lockHolder(m_layerHitTestMutex); > > Does this lock need to be held all the way to the end? I don’t fully understand what is being guarded. This lock guards CALayers and PlatformCALayers, so the answer is yes.
Created attachment 399245 [details] patch
> final? The class is marked final, using override for consistency.
Committed r261602: <https://trac.webkit.org/changeset/261602> All reviewed patches have been landed. Closing bug and clearing flags on attachment 399245 [details].
<rdar://problem/63172645>
> Is IntPoint right here? I’d like to understand better some day when it > should be int, float, CGFloat/double, or LayoutUnit. The underlying data structure here (Region) is currently integer based, that's why IntPoint. For its current use cases that level of accuracy is sufficient. I think we should move to 64 bit LayoutUnit at some point. That could allow using a single unit type through the layout code.