Title: [294614] trunk
Revision
294614
Author
timothy_hor...@apple.com
Date
2022-05-21 12:26:27 -0700 (Sat, 21 May 2022)

Log Message

InteractionRegion for wrapped text has multiple rects instead of one
https://bugs.webkit.org/show_bug.cgi?id=240748

Reviewed by Wenson Hsieh.

* Source/WebCore/page/InteractionRegion.h:
(WebCore::operator==):
(WebCore::InteractionRegion::encode const):
(WebCore::InteractionRegion::decode):
* Source/WebCore/rendering/EventRegion.cpp:
(WebCore::EventRegion::translate):
(WebCore::EventRegion::dump const):
* Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeInteractionRegionLayers.mm:
(WebKit::updateLayersForInteractionRegions):
* Source/WebCore/page/DebugPageOverlays.cpp:
(WebCore::pathsForRegion):
(WebCore::InteractionRegionOverlay::activeRegion):
(WebCore::InteractionRegionOverlay::drawRect):
* Source/WebCore/page/InteractionRegion.cpp:
(WebCore::regionForElement):
Maintain InteractionRegion geometry as a Region instead of a vector of rects,
so that we can smush overlapping rectangles together on a per-InteractionRegion basis.

(WebCore::operator<<):
Add dumping code for InteractionRegion (and adopt it in EventRegion).

* LayoutTests/TestExpectations:
* LayoutTests/interaction-region/click-handler-expected.txt: Added.
* LayoutTests/interaction-region/click-handler.html: Added.
* LayoutTests/interaction-region/inline-link-dark-background-expected.txt: Added.
* LayoutTests/interaction-region/inline-link-dark-background.html: Added.
* LayoutTests/interaction-region/inline-link-expected.txt: Added.
* LayoutTests/interaction-region/inline-link.html: Added.
* LayoutTests/interaction-region/split-inline-link-expected.txt: Added.
* LayoutTests/interaction-region/split-inline-link.html: Added.
* LayoutTests/interaction-region/wrapped-inline-link-expected.txt: Added.
* LayoutTests/interaction-region/wrapped-inline-link.html: Added.
Add some basic interaction region tests, currently disabled by default
because they can only be run if you turn on the build-time flag.

"wrapped-inline-link.html" covers the case we are fixing by adopting Region;
the others are more generic tests that we should have had before.

Canonical link: https://commits.webkit.org/250840@main

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/TestExpectations (294613 => 294614)


--- trunk/LayoutTests/TestExpectations	2022-05-21 15:15:06 UTC (rev 294613)
+++ trunk/LayoutTests/TestExpectations	2022-05-21 19:26:27 UTC (rev 294614)
@@ -88,6 +88,7 @@
 fast/dom/Range/mac [ Skip ]
 remote-layer-tree/ios [ Skip ]
 inspector/page/setScreenSizeOverride.html [ Skip ]
+interaction-region [ Skip ]
 
 # Requires async overflow scrolling
 compositing/shared-backing/overflow-scroll [ Skip ]

Added: trunk/LayoutTests/interaction-region/click-handler-expected.txt (0 => 294614)


--- trunk/LayoutTests/interaction-region/click-handler-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/interaction-region/click-handler-expected.txt	2022-05-21 19:26:27 UTC (rev 294614)
@@ -0,0 +1,23 @@
+ (GraphicsLayer
+  (anchor 0.00 0.00)
+  (bounds 800.00 600.00)
+  (children 1
+    (GraphicsLayer
+      (bounds 800.00 600.00)
+      (contentsOpaque 1)
+      (drawsContent 1)
+      (backgroundColor #FFFFFF)
+      (event region
+        (rect (0,0) width=800 height=600)
+
+      (interaction regions [
+        (region
+            (rect (0,0) width=100 height=100)
+)
+        (hasLightBackground 1)
+        (borderRadius 10.00)])
+      )
+    )
+  )
+)
+

Added: trunk/LayoutTests/interaction-region/click-handler.html (0 => 294614)


--- trunk/LayoutTests/interaction-region/click-handler.html	                        (rev 0)
+++ trunk/LayoutTests/interaction-region/click-handler.html	2022-05-21 19:26:27 UTC (rev 294614)
@@ -0,0 +1,29 @@
+<!DOCTYPE html>
+<html>
+<style>
+    body { margin: 0; }
+
+    #button {
+        display: inline-block;
+        width: 100px;
+        height: 100px;
+        background-color: green;
+        cursor: pointer;
+        border-radius: 10px;
+    }
+</style>
+<body>
+<div id="button" _onclick_="click()"></div>
+
+<pre id="results"></pre>
+<script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+
+window._onload_ = function () {
+    if (window.internals)
+       results.textContent = internals.layerTreeAsText(document, internals.LAYER_TREE_INCLUDES_EVENT_REGION | internals.LAYER_TREE_INCLUDES_ROOT_LAYER_PROPERTIES);
+};
+</script>
+</body>
+</html>

Added: trunk/LayoutTests/interaction-region/inline-link-dark-background-expected.txt (0 => 294614)


--- trunk/LayoutTests/interaction-region/inline-link-dark-background-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/interaction-region/inline-link-dark-background-expected.txt	2022-05-21 19:26:27 UTC (rev 294614)
@@ -0,0 +1,24 @@
+This is a link.
+(GraphicsLayer
+  (anchor 0.00 0.00)
+  (bounds 800.00 600.00)
+  (children 1
+    (GraphicsLayer
+      (bounds 800.00 600.00)
+      (contentsOpaque 1)
+      (drawsContent 1)
+      (backgroundColor #000000)
+      (event region
+        (rect (0,0) width=800 height=600)
+
+      (interaction regions [
+        (region
+            (rect (-3,-3) width=35 height=24)
+)
+        (hasLightBackground 0)
+        (borderRadius 0.00)])
+      )
+    )
+  )
+)
+

Added: trunk/LayoutTests/interaction-region/inline-link-dark-background.html (0 => 294614)


--- trunk/LayoutTests/interaction-region/inline-link-dark-background.html	                        (rev 0)
+++ trunk/LayoutTests/interaction-region/inline-link-dark-background.html	2022-05-21 19:26:27 UTC (rev 294614)
@@ -0,0 +1,24 @@
+<!DOCTYPE html>
+<html>
+<style>
+    body {
+        margin: 0;
+        background-color: black;
+        color: white;
+    }
+</style>
+<body>
+<a href="" is a link.
+
+<pre id="results"></pre>
+<script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+
+window._onload_ = function () {
+    if (window.internals)
+       results.textContent = internals.layerTreeAsText(document, internals.LAYER_TREE_INCLUDES_EVENT_REGION | internals.LAYER_TREE_INCLUDES_ROOT_LAYER_PROPERTIES);
+};
+</script>
+</body>
+</html>

Added: trunk/LayoutTests/interaction-region/inline-link-expected.txt (0 => 294614)


--- trunk/LayoutTests/interaction-region/inline-link-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/interaction-region/inline-link-expected.txt	2022-05-21 19:26:27 UTC (rev 294614)
@@ -0,0 +1,24 @@
+This is a link.
+(GraphicsLayer
+  (anchor 0.00 0.00)
+  (bounds 800.00 600.00)
+  (children 1
+    (GraphicsLayer
+      (bounds 800.00 600.00)
+      (contentsOpaque 1)
+      (drawsContent 1)
+      (backgroundColor #FFFFFF)
+      (event region
+        (rect (0,0) width=800 height=600)
+
+      (interaction regions [
+        (region
+            (rect (-3,-3) width=35 height=24)
+)
+        (hasLightBackground 1)
+        (borderRadius 0.00)])
+      )
+    )
+  )
+)
+

Added: trunk/LayoutTests/interaction-region/inline-link.html (0 => 294614)


--- trunk/LayoutTests/interaction-region/inline-link.html	                        (rev 0)
+++ trunk/LayoutTests/interaction-region/inline-link.html	2022-05-21 19:26:27 UTC (rev 294614)
@@ -0,0 +1,20 @@
+<!DOCTYPE html>
+<html>
+<style>
+    body { margin: 0; }
+</style>
+<body>
+<a href="" is a link.
+
+<pre id="results"></pre>
+<script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+
+window._onload_ = function () {
+    if (window.internals)
+       results.textContent = internals.layerTreeAsText(document, internals.LAYER_TREE_INCLUDES_EVENT_REGION | internals.LAYER_TREE_INCLUDES_ROOT_LAYER_PROPERTIES);
+};
+</script>
+</body>
+</html>

Added: trunk/LayoutTests/interaction-region/split-inline-link-expected.txt (0 => 294614)


--- trunk/LayoutTests/interaction-region/split-inline-link-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/interaction-region/split-inline-link-expected.txt	2022-05-21 19:26:27 UTC (rev 294614)
@@ -0,0 +1,28 @@
+A bunch of text before the first line
+short second line.
+(GraphicsLayer
+  (anchor 0.00 0.00)
+  (bounds 800.00 600.00)
+  (children 1
+    (GraphicsLayer
+      (bounds 800.00 600.00)
+      (contentsOpaque 1)
+      (drawsContent 1)
+      (backgroundColor #FFFFFF)
+      (event region
+        (rect (0,0) width=800 height=600)
+
+      (interaction regions [
+        (region
+            (rect (197,-3) width=31 height=18)
+            (rect (-3,15) width=115 height=6)
+            (rect (197,15) width=31 height=6)
+            (rect (-3,21) width=115 height=18)
+)
+        (hasLightBackground 1)
+        (borderRadius 0.00)])
+      )
+    )
+  )
+)
+

Added: trunk/LayoutTests/interaction-region/split-inline-link.html (0 => 294614)


--- trunk/LayoutTests/interaction-region/split-inline-link.html	                        (rev 0)
+++ trunk/LayoutTests/interaction-region/split-inline-link.html	2022-05-21 19:26:27 UTC (rev 294614)
@@ -0,0 +1,20 @@
+<!DOCTYPE html>
+<html>
+<style>
+    body { margin: 0; }
+</style>
+<body>
+A bunch of text before the first <a href="" second line</a>.
+
+<pre id="results"></pre>
+<script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+
+window._onload_ = function () {
+    if (window.internals)
+       results.textContent = internals.layerTreeAsText(document, internals.LAYER_TREE_INCLUDES_EVENT_REGION | internals.LAYER_TREE_INCLUDES_ROOT_LAYER_PROPERTIES);
+};
+</script>
+</body>
+</html>

Added: trunk/LayoutTests/interaction-region/wrapped-inline-link-expected.txt (0 => 294614)


--- trunk/LayoutTests/interaction-region/wrapped-inline-link-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/interaction-region/wrapped-inline-link-expected.txt	2022-05-21 19:26:27 UTC (rev 294614)
@@ -0,0 +1,27 @@
+Line
+Line
+Line
+Line
+(GraphicsLayer
+  (anchor 0.00 0.00)
+  (bounds 800.00 600.00)
+  (children 1
+    (GraphicsLayer
+      (bounds 800.00 600.00)
+      (contentsOpaque 1)
+      (drawsContent 1)
+      (backgroundColor #FFFFFF)
+      (event region
+        (rect (0,0) width=800 height=600)
+
+      (interaction regions [
+        (region
+            (rect (-3,-3) width=36 height=78)
+)
+        (hasLightBackground 1)
+        (borderRadius 0.00)])
+      )
+    )
+  )
+)
+

Added: trunk/LayoutTests/interaction-region/wrapped-inline-link.html (0 => 294614)


--- trunk/LayoutTests/interaction-region/wrapped-inline-link.html	                        (rev 0)
+++ trunk/LayoutTests/interaction-region/wrapped-inline-link.html	2022-05-21 19:26:27 UTC (rev 294614)
@@ -0,0 +1,20 @@
+<!DOCTYPE html>
+<html>
+<style>
+    body { margin: 0; }
+</style>
+<body>
+<a href=""
+
+<pre id="results"></pre>
+<script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+
+window._onload_ = function () {
+    if (window.internals)
+       results.textContent = internals.layerTreeAsText(document, internals.LAYER_TREE_INCLUDES_EVENT_REGION | internals.LAYER_TREE_INCLUDES_ROOT_LAYER_PROPERTIES);
+};
+</script>
+</body>
+</html>

Modified: trunk/Source/WebCore/page/DebugPageOverlays.cpp (294613 => 294614)


--- trunk/Source/WebCore/page/DebugPageOverlays.cpp	2022-05-21 15:15:06 UTC (rev 294613)
+++ trunk/Source/WebCore/page/DebugPageOverlays.cpp	2022-05-21 19:26:27 UTC (rev 294614)
@@ -295,9 +295,9 @@
 {
     static constexpr float radius = 4;
 
-    Vector<FloatRect> rects;
-    for (auto rect : region.rectsInContentCoordinates)
-        rects.append(rect);
+    Vector<FloatRect> rects = region.regionInLayerCoordinates.rects().map([] (auto rect) -> FloatRect {
+        return rect;
+    });
     return PathUtilities::pathsWithShrinkWrappedRects(rects, std::max(region.borderRadius, radius));
 }
 
@@ -309,7 +309,7 @@
     for (const auto& region : m_regions) {
         float area = 0;
         FloatRect boundingRect;
-        for (const auto& rect : region.rectsInContentCoordinates) {
+        for (const auto& rect : region.regionInLayerCoordinates.rects()) {
             if (boundingRect.isEmpty())
                 boundingRect = rect;
             else
@@ -433,8 +433,8 @@
         context.setStrokeColor(Color::green);
 
         for (const auto& region : m_regions) {
-            for (const auto& rect : region.rectsInContentCoordinates)
-            context.strokeRect(rect, 2);
+            for (const auto& rect : region.regionInLayerCoordinates.rects())
+                context.strokeRect(rect, 2);
         }
     }
 

Modified: trunk/Source/WebCore/page/InteractionRegion.cpp (294613 => 294614)


--- trunk/Source/WebCore/page/InteractionRegion.cpp	2022-05-21 15:15:06 UTC (rev 294613)
+++ trunk/Source/WebCore/page/InteractionRegion.cpp	2022-05-21 19:26:27 UTC (rev 294614)
@@ -111,8 +111,8 @@
         RoundedRect::Radii borderRadii = downcast<RenderBox>(*renderer).borderRadii();
         region.borderRadius = borderRadii.minimumRadius();
     }
-
-    region.rectsInContentCoordinates = compactMap(rectsInContentCoordinates, [&](auto rect) -> std::optional<FloatRect> {
+    
+    for (auto rect : rectsInContentCoordinates) {
         auto contentsRect = rect;
 
         if (&frameView != &mainFrameView)
@@ -119,12 +119,12 @@
             contentsRect.intersect(frameClipRect);
 
         if (contentsRect.isEmpty())
-            return std::nullopt;
+            continue;
 
-        return contentsRect;
-    });
+        region.regionInLayerCoordinates.unite(enclosingIntRect(contentsRect));
+    }
 
-    if (region.rectsInContentCoordinates.isEmpty())
+    if (region.regionInLayerCoordinates.isEmpty())
         return std::nullopt;
 
     return region;
@@ -186,4 +186,13 @@
     return regions;
 }
 
+TextStream& operator<<(TextStream& ts, const InteractionRegion& interactionRegion)
+{
+    ts.dumpProperty("region", interactionRegion.regionInLayerCoordinates);
+    ts.dumpProperty("hasLightBackground", interactionRegion.hasLightBackground);
+    ts.dumpProperty("borderRadius", interactionRegion.borderRadius);
+
+    return ts;
 }
+
+}

Modified: trunk/Source/WebCore/page/InteractionRegion.h (294613 => 294614)


--- trunk/Source/WebCore/page/InteractionRegion.h	2022-05-21 15:15:06 UTC (rev 294613)
+++ trunk/Source/WebCore/page/InteractionRegion.h	2022-05-21 19:26:27 UTC (rev 294614)
@@ -26,6 +26,7 @@
 #pragma once
 
 #include "FloatRect.h"
+#include "Region.h"
 
 namespace IPC {
 class Decoder;
@@ -32,17 +33,21 @@
 class Encoder;
 }
 
+namespace WTF {
+class TextStream;
+}
+
 namespace WebCore {
 
 class Page;
 
 struct InteractionRegion {
-    Vector<FloatRect> rectsInContentCoordinates;
+    Region regionInLayerCoordinates;
     bool hasLightBackground { false };
     float borderRadius { 0 };
-    
+
     WEBCORE_EXPORT ~InteractionRegion();
-    
+
     template<class Encoder> void encode(Encoder&) const;
     template<class Decoder> static std::optional<InteractionRegion> decode(Decoder&);
 };
@@ -49,7 +54,7 @@
 
 inline bool operator==(const InteractionRegion& a, const InteractionRegion& b)
 {
-    return a.rectsInContentCoordinates == b.rectsInContentCoordinates
+    return a.regionInLayerCoordinates == b.regionInLayerCoordinates
         && a.hasLightBackground == b.hasLightBackground
         && a.borderRadius == b.borderRadius;
 }
@@ -56,10 +61,12 @@
 
 WEBCORE_EXPORT Vector<InteractionRegion> interactionRegions(Page&, FloatRect rectInContentCoordinates);
 
+WTF::TextStream& operator<<(WTF::TextStream&, const InteractionRegion&);
+
 template<class Encoder>
 void InteractionRegion::encode(Encoder& encoder) const
 {
-    encoder << rectsInContentCoordinates;
+    encoder << regionInLayerCoordinates;
     encoder << hasLightBackground;
     encoder << borderRadius;
 }
@@ -67,9 +74,9 @@
 template<class Decoder>
 std::optional<InteractionRegion> InteractionRegion::decode(Decoder& decoder)
 {
-    std::optional<Vector<FloatRect>> rectsInContentCoordinates;
-    decoder >> rectsInContentCoordinates;
-    if (!rectsInContentCoordinates)
+    std::optional<Region> regionInLayerCoordinates;
+    decoder >> regionInLayerCoordinates;
+    if (!regionInLayerCoordinates)
         return std::nullopt;
     
     std::optional<bool> hasLightBackground;
@@ -83,7 +90,7 @@
         return std::nullopt;
 
     return { {
-        WTFMove(*rectsInContentCoordinates),
+        WTFMove(*regionInLayerCoordinates),
         WTFMove(*hasLightBackground),
         WTFMove(*borderRadius)
     } };

Modified: trunk/Source/WebCore/rendering/EventRegion.cpp (294613 => 294614)


--- trunk/Source/WebCore/rendering/EventRegion.cpp	2022-05-21 15:15:06 UTC (rev 294613)
+++ trunk/Source/WebCore/rendering/EventRegion.cpp	2022-05-21 19:26:27 UTC (rev 294614)
@@ -171,10 +171,8 @@
 #endif
 
 #if ENABLE(INTERACTION_REGIONS_IN_EVENT_REGION)
-    for (auto& region : m_interactionRegions) {
-        for (auto& rect : region.rectsInContentCoordinates)
-            rect.move(offset);
-    }
+    for (auto& region : m_interactionRegions)
+        region.regionInLayerCoordinates.translate(offset);
 #endif
 }
 
@@ -369,6 +367,13 @@
         ts << indent << ")\n";
     }
 #endif
+    
+#if ENABLE(INTERACTION_REGIONS_IN_EVENT_REGION)
+    if (!m_interactionRegions.isEmpty()) {
+        ts.dumpProperty("interaction regions", m_interactionRegions);
+        ts << "\n";
+    }
+#endif
 }
 
 TextStream& operator<<(TextStream& ts, TouchAction touchAction)

Modified: trunk/Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeInteractionRegionLayers.mm (294613 => 294614)


--- trunk/Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeInteractionRegionLayers.mm	2022-05-21 15:15:06 UTC (rev 294613)
+++ trunk/Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeInteractionRegionLayers.mm	2022-05-21 19:26:27 UTC (rev 294614)
@@ -98,8 +98,8 @@
     bool applyBackgroundColorForDebugging = [[NSUserDefaults standardUserDefaults] boolForKey:@"WKInteractionRegionDebugFill"];
 
     for (const WebCore::InteractionRegion& region : properties.eventRegion.interactionRegions()) {
-        for (FloatRect rect : region.rectsInContentCoordinates) {
-            auto layerIterator = interactionRegionLayers.find(enclosingIntRect(rect));
+        for (IntRect rect : region.regionInLayerCoordinates.rects()) {
+            auto layerIterator = interactionRegionLayers.find(rect);
 
             RetainPtr<CALayer> interactionRegionLayer;
             if (layerIterator != interactionRegionLayers.end()) {
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to