Attachment 359678 Details for Bug 193649 – patch

[patch] patch

scrolling-delegate-coordinates-3.patch (text/plain), 9.94 KB, created by Antti Koivisto on 2019-01-21 01:52:51 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Antti Koivisto

Created: 2019-01-21 01:52:51 PST

Size: 9.94 KB

patch

obsolete

>Index: Source/WebCore/ChangeLog
>===================================================================
>--- Source/WebCore/ChangeLog	(revision 240230)
>+++ Source/WebCore/ChangeLog	(working copy)
>@@ -1,3 +1,33 @@
>+2019-01-21  Antti Koivisto  <antti@apple.com>
>+
>+        Move delegatesScrolling() tests to lower level conversion function in ScrollView
>+        https://bugs.webkit.org/show_bug.cgi?id=193649
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        This reduces places where these tests are needed and avoid mistakes.
>+
>+        * dom/Document.cpp:
>+        (WebCore::computeIntersectionState):
>+        * page/FrameView.cpp:
>+        (WebCore::FrameView::convertFromRendererToContainingView const):
>+        (WebCore::FrameView::convertFromContainingViewToRenderer const):
>+        * page/ios/EventHandlerIOS.mm:
>+        (WebCore::EventHandler::startSelectionAutoscroll):
>+        (WebCore::EventHandler::targetPositionInWindowForSelectionAutoscroll const):
>+
>+        Keep m_targetAutoscrollPositionInWindow in window coordinates as indicated by the name.
>+        The wrong conversions canceled each other out.
>+
>+        * platform/ScrollView.cpp:
>+        (WebCore::ScrollView::viewToContents const):
>+        (WebCore::ScrollView::contentsToView const):
>+        (WebCore::ScrollView::contentsToContainingViewContents const):
>+        (WebCore::ScrollView::rootViewToContents const):
>+        (WebCore::ScrollView::contentsToRootView const):
>+        (WebCore::ScrollView::windowToContents const):
>+        (WebCore::ScrollView::contentsToWindow const):
>+
> 2019-01-20  Myles C. Maxfield  <mmaxfield@apple.com>
> 
>         [WHLSL] Implement Metal code generation
>Index: Source/WebCore/dom/Document.cpp
>===================================================================
>--- Source/WebCore/dom/Document.cpp	(revision 240227)
>+++ Source/WebCore/dom/Document.cpp	(working copy)
>@@ -7919,7 +7919,7 @@ static Optional<IntersectionObservationS
>         if (&targetRenderer->frame() == &rootRenderer->frame())
>             intersectionState.absoluteIntersectionRect = rootAbsoluteIntersectionRect;
>         else {
>-            FloatRect rootViewIntersectionRect = frameView.delegatesScrolling() ? rootAbsoluteIntersectionRect : frameView.contentsToView(rootAbsoluteIntersectionRect);
>+            FloatRect rootViewIntersectionRect = frameView.contentsToView(rootAbsoluteIntersectionRect);
>             intersectionState.absoluteIntersectionRect = targetRenderer->view().frameView().rootViewToContents(rootViewIntersectionRect);
>         }
>     }
>Index: Source/WebCore/page/FrameView.cpp
>===================================================================
>--- Source/WebCore/page/FrameView.cpp	(revision 240227)
>+++ Source/WebCore/page/FrameView.cpp	(working copy)
>@@ -4654,19 +4654,12 @@ IntRect FrameView::convertFromRendererTo
> {
>     IntRect rect = snappedIntRect(enclosingLayoutRect(renderer->localToAbsoluteQuad(FloatRect(rendererRect)).boundingBox()));
> 
>-    if (!delegatesScrolling())
>-        rect = contentsToView(rect);
>-
>-    return rect;
>+    return contentsToView(rect);
> }
> 
> IntRect FrameView::convertFromContainingViewToRenderer(const RenderElement* renderer, const IntRect& viewRect) const
> {
>-    IntRect rect = viewRect;
>-    
>-    // Convert from FrameView coords into page ("absolute") coordinates.
>-    if (!delegatesScrolling())
>-        rect = viewToContents(rect);
>+    IntRect rect = viewToContents(viewRect);
> 
>     // FIXME: we don't have a way to map an absolute rect down to a local quad, so just
>     // move the rect for now.
>@@ -4676,11 +4669,7 @@ IntRect FrameView::convertFromContaining
> 
> FloatRect FrameView::convertFromContainingViewToRenderer(const RenderElement* renderer, const FloatRect& viewRect) const
> {
>-    FloatRect rect = viewRect;
>-
>-    // Convert from FrameView coords into page ("absolute") coordinates.
>-    if (!delegatesScrolling())
>-        rect = viewToContents(rect);
>+    FloatRect rect = viewToContents(viewRect);
> 
>     return (renderer->absoluteToLocalQuad(rect)).boundingBox();
> }
>@@ -4689,11 +4678,7 @@ IntPoint FrameView::convertFromRendererT
> {
>     IntPoint point = roundedIntPoint(renderer->localToAbsolute(rendererPoint, UseTransforms));
> 
>-    // Convert from page ("absolute") to FrameView coordinates.
>-    if (!delegatesScrolling())
>-        point = contentsToView(point);
>-
>-    return point;
>+    return contentsToView(point);
> }
> 
> IntPoint FrameView::convertFromContainingViewToRenderer(const RenderElement* renderer, const IntPoint& viewPoint) const
>Index: Source/WebCore/page/ios/EventHandlerIOS.mm
>===================================================================
>--- Source/WebCore/page/ios/EventHandlerIOS.mm	(revision 240227)
>+++ Source/WebCore/page/ios/EventHandlerIOS.mm	(working copy)
>@@ -568,7 +568,7 @@ void EventHandler::startSelectionAutoscr
> {
>     Ref<Frame> protectedFrame(m_frame);
> 
>-    m_targetAutoscrollPositionInWindow = protectedFrame->view()->contentsToView(roundedIntPoint(positionInWindow));
>+    m_targetAutoscrollPositionInWindow = roundedIntPoint(positionInWindow);
>     
>     m_isAutoscrolling = true;
>     m_autoscrollController->startAutoscrollForSelection(renderer);
>@@ -632,8 +632,7 @@ IntPoint EventHandler::targetPositionInW
>     
>     FloatRect unobscuredContentRect = protectedFrame->view()->unobscuredContentRect();
>     
>-    // Manually need to convert viewToContents, as it will be skipped because delegatedScrolling is on iOS
>-    IntPoint contentPosition = protectedFrame->view()->viewToContents(protectedFrame->view()->convertFromContainingWindow(m_targetAutoscrollPositionInWindow));
>+    IntPoint contentPosition = protectedFrame->view()->convertFromContainingWindow(m_targetAutoscrollPositionInWindow);
>     IntSize adjustPosition = autoscrollAdjustmentFactorForScreenBoundaries(contentPosition, unobscuredContentRect, protectedFrame->page()->pageScaleFactor());
>     return contentPosition + adjustPosition;
> }
>Index: Source/WebCore/platform/ScrollView.cpp
>===================================================================
>--- Source/WebCore/platform/ScrollView.cpp	(revision 240227)
>+++ Source/WebCore/platform/ScrollView.cpp	(working copy)
>@@ -815,34 +815,52 @@ void ScrollView::scrollContentsSlowPath(
> 
> IntPoint ScrollView::viewToContents(const IntPoint& point) const
> {
>+    if (delegatesScrolling())
>+        return point;
>+
>     return point + toIntSize(documentScrollPositionRelativeToViewOrigin());
> }
> 
> IntPoint ScrollView::contentsToView(const IntPoint& point) const
> {
>+    if (delegatesScrolling())
>+        return point;
>+
>     return point - toIntSize(documentScrollPositionRelativeToViewOrigin());
> }
> 
> IntRect ScrollView::viewToContents(IntRect rect) const
> {
>+    if (delegatesScrolling())
>+        return rect;
>+
>     rect.moveBy(documentScrollPositionRelativeToViewOrigin());
>     return rect;
> }
> 
> FloatRect ScrollView::viewToContents(FloatRect rect) const
> {
>+    if (delegatesScrolling())
>+        return rect;
>+
>     rect.moveBy(documentScrollPositionRelativeToViewOrigin());
>     return rect;
> }
> 
> IntRect ScrollView::contentsToView(IntRect rect) const
> {
>+    if (delegatesScrolling())
>+        return rect;
>+
>     rect.moveBy(-documentScrollPositionRelativeToViewOrigin());
>     return rect;
> }
> 
> FloatRect ScrollView::contentsToView(FloatRect rect) const
> {
>+    if (delegatesScrolling())
>+        return rect;
>+
>     rect.moveBy(-documentScrollPositionRelativeToViewOrigin());
>     return rect;
> }
>@@ -859,9 +877,6 @@ IntPoint ScrollView::contentsToContainin
> 
> IntRect ScrollView::contentsToContainingViewContents(IntRect rect) const
> {
>-    if (delegatesScrolling())
>-        return convertToContainingView(contentsToView(rect));
>-    
>     if (const ScrollView* parentScrollView = parent()) {
>         IntRect rectInContainingView = convertToContainingView(contentsToView(rect));
>         return parentScrollView->viewToContents(rectInContainingView);
>@@ -872,33 +887,21 @@ IntRect ScrollView::contentsToContaining
> 
> IntPoint ScrollView::rootViewToContents(const IntPoint& rootViewPoint) const
> {
>-    if (delegatesScrolling())
>-        return convertFromRootView(rootViewPoint);
>-
>     return viewToContents(convertFromRootView(rootViewPoint));
> }
> 
> IntPoint ScrollView::contentsToRootView(const IntPoint& contentsPoint) const
> {
>-    if (delegatesScrolling())
>-        return convertToRootView(contentsPoint);
>-
>     return convertToRootView(contentsToView(contentsPoint));
> }
> 
> IntRect ScrollView::rootViewToContents(const IntRect& rootViewRect) const
> {
>-    if (delegatesScrolling())
>-        return convertFromRootView(rootViewRect);
>-
>     return viewToContents(convertFromRootView(rootViewRect));
> }
> 
> FloatRect ScrollView::rootViewToContents(const FloatRect& rootViewRect) const
> {
>-    if (delegatesScrolling())
>-        return convertFromRootView(rootViewRect);
>-
>     return viewToContents(convertFromRootView(rootViewRect));
> }
> 
>@@ -914,41 +917,26 @@ IntPoint ScrollView::rootViewToTotalCont
> 
> IntRect ScrollView::contentsToRootView(const IntRect& contentsRect) const
> {
>-    if (delegatesScrolling())
>-        return convertToRootView(contentsRect);
>-
>     return convertToRootView(contentsToView(contentsRect));
> }
> 
> IntPoint ScrollView::windowToContents(const IntPoint& windowPoint) const
> {
>-    if (delegatesScrolling())
>-        return convertFromContainingWindow(windowPoint);
>-
>     return viewToContents(convertFromContainingWindow(windowPoint));
> }
> 
> IntPoint ScrollView::contentsToWindow(const IntPoint& contentsPoint) const
> {
>-    if (delegatesScrolling())
>-        return convertToContainingWindow(contentsPoint);
>-
>     return convertToContainingWindow(contentsToView(contentsPoint));
> }
> 
> IntRect ScrollView::windowToContents(const IntRect& windowRect) const
> {
>-    if (delegatesScrolling())
>-        return convertFromContainingWindow(windowRect);
>-
>     return viewToContents(convertFromContainingWindow(windowRect));
> }
> 
> IntRect ScrollView::contentsToWindow(const IntRect& contentsRect) const
> {
>-    if (delegatesScrolling())
>-        return convertToContainingWindow(contentsRect);
>-
>     return convertToContainingWindow(contentsToView(contentsRect));
> }
>

Actions: View | Formatted Diff | Diff

Attachments on bug 193649: 359678 | 359679