REGRESSION (r237565): >20 Find in Page highlights in one tile results in a single...
authortimothy_horton@apple.com <timothy_horton@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 13 Dec 2018 00:48:56 +0000 (00:48 +0000)
committertimothy_horton@apple.com <timothy_horton@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 13 Dec 2018 00:48:56 +0000 (00:48 +0000)
https://bugs.webkit.org/show_bug.cgi?id=192642
<rdar://problem/46498246>

Reviewed by Geoffrey Garen.

Source/WebCore:

No new tests; adjusted an existing test instead.

* platform/graphics/PathUtilities.cpp:
(WebCore::PathUtilities::pathsWithShrinkWrappedRects):
Instead of uniting when we fail to shrink-wrap, just return the original rects.
This seems like a more reasonable default in most cases.

LayoutTests:

* fast/shrink-wrap/rect-shrink-wrap-expected.html:
* fast/shrink-wrap/rect-shrink-wrap.html:
Add a test case with >20 tiny rects.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@239135 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/shrink-wrap/rect-shrink-wrap-expected.html
LayoutTests/fast/shrink-wrap/rect-shrink-wrap.html
Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/PathUtilities.cpp

index 1b7bc97..057589b 100644 (file)
@@ -1,3 +1,15 @@
+2018-12-12  Tim Horton  <timothy_horton@apple.com>
+
+        REGRESSION (r237565): >20 Find in Page highlights in one tile results in a single giant highlight
+        https://bugs.webkit.org/show_bug.cgi?id=192642
+        <rdar://problem/46498246>
+
+        Reviewed by Geoffrey Garen.
+
+        * fast/shrink-wrap/rect-shrink-wrap-expected.html:
+        * fast/shrink-wrap/rect-shrink-wrap.html:
+        Add a test case with >20 tiny rects.
+
 2018-12-12  Michael Catanzaro  <mcatanzaro@igalia.com>
 
         Unreviewed manual rollout of r239100-r239102 and r239116
index d7fd06d..20a8682 100644 (file)
@@ -211,5 +211,33 @@ body {
         <rect x="15" y="15" width="35" height="20" fill="rgba(0, 0, 0, 0.2)" stroke="rgba(0, 0, 0, 0.5)"></rect>
         <rect x="0" y="30" width="50" height="20" fill="rgba(0, 0, 0, 0.2)" stroke="rgba(0, 0, 0, 0.5)"></rect>
         <path d="M 30 7.5 L 30 7.5 C 30 3.35786 33.3579 6.66106e-16 37.5 0 L 42.5 0 L 42.5 0 C 46.6421 2.53633e-16 50 3.35786 50 7.5 L 50 12.5 L 50 15 L 50 17.5 L 50 20 L 50 27.5 L 50 30 L 50 32.5 L 50 35 L 50 42.5 L 50 42.5 C 50 46.6421 46.6421 50 42.5 50 L 8 50 L 8 50 C 3.58172 50 5.41083e-16 46.4183 0 42 L 0 37.5 L 0 37.5 C 5.07265e-16 33.3579 3.35786 30 7.5 30 L 7.5 30 L 7.5 30 C 11.6421 30 15 26.6421 15 22.5 L 15 22.5 L 15 22.5 C 15 18.3579 18.3579 15 22.5 15 L 22.5 15 L 22.5 15 C 26.6421 15 30 11.6421 30 7.5 Z" fill="none" stroke="blue" stroke-width="3"></path>
-</g>
+    </g>
+    <g transform="translate(580, 300)">
+        <rect x="0" y="0" width="1" height="1" fill="rgba(0, 0, 0, 0.2)" stroke="rgba(0, 0, 0, 0.5)"></rect>
+        <rect x="4" y="0" width="1" height="1" fill="rgba(0, 0, 0, 0.2)" stroke="rgba(0, 0, 0, 0.5)"></rect>
+        <rect x="8" y="0" width="1" height="1" fill="rgba(0, 0, 0, 0.2)" stroke="rgba(0, 0, 0, 0.5)"></rect>
+        <rect x="12" y="0" width="1" height="1" fill="rgba(0, 0, 0, 0.2)" stroke="rgba(0, 0, 0, 0.5)"></rect>
+        <rect x="16" y="0" width="1" height="1" fill="rgba(0, 0, 0, 0.2)" stroke="rgba(0, 0, 0, 0.5)"></rect>
+        <rect x="0" y="4" width="1" height="1" fill="rgba(0, 0, 0, 0.2)" stroke="rgba(0, 0, 0, 0.5)"></rect>
+        <rect x="4" y="4" width="1" height="1" fill="rgba(0, 0, 0, 0.2)" stroke="rgba(0, 0, 0, 0.5)"></rect>
+        <rect x="8" y="4" width="1" height="1" fill="rgba(0, 0, 0, 0.2)" stroke="rgba(0, 0, 0, 0.5)"></rect>
+        <rect x="12" y="4" width="1" height="1" fill="rgba(0, 0, 0, 0.2)" stroke="rgba(0, 0, 0, 0.5)"></rect>
+        <rect x="16" y="4" width="1" height="1" fill="rgba(0, 0, 0, 0.2)" stroke="rgba(0, 0, 0, 0.5)"></rect>
+        <rect x="0" y="8" width="1" height="1" fill="rgba(0, 0, 0, 0.2)" stroke="rgba(0, 0, 0, 0.5)"></rect>
+        <rect x="4" y="8" width="1" height="1" fill="rgba(0, 0, 0, 0.2)" stroke="rgba(0, 0, 0, 0.5)"></rect>
+        <rect x="8" y="8" width="1" height="1" fill="rgba(0, 0, 0, 0.2)" stroke="rgba(0, 0, 0, 0.5)"></rect>
+        <rect x="12" y="8" width="1" height="1" fill="rgba(0, 0, 0, 0.2)" stroke="rgba(0, 0, 0, 0.5)"></rect>
+        <rect x="16" y="8" width="1" height="1" fill="rgba(0, 0, 0, 0.2)" stroke="rgba(0, 0, 0, 0.5)"></rect>
+        <rect x="0" y="12" width="1" height="1" fill="rgba(0, 0, 0, 0.2)" stroke="rgba(0, 0, 0, 0.5)"></rect>
+        <rect x="4" y="12" width="1" height="1" fill="rgba(0, 0, 0, 0.2)" stroke="rgba(0, 0, 0, 0.5)"></rect>
+        <rect x="8" y="12" width="1" height="1" fill="rgba(0, 0, 0, 0.2)" stroke="rgba(0, 0, 0, 0.5)"></rect>
+        <rect x="12" y="12" width="1" height="1" fill="rgba(0, 0, 0, 0.2)" stroke="rgba(0, 0, 0, 0.5)"></rect>
+        <rect x="16" y="12" width="1" height="1" fill="rgba(0, 0, 0, 0.2)" stroke="rgba(0, 0, 0, 0.5)"></rect>
+        <rect x="0" y="16" width="1" height="1" fill="rgba(0, 0, 0, 0.2)" stroke="rgba(0, 0, 0, 0.5)"></rect>
+        <rect x="4" y="16" width="1" height="1" fill="rgba(0, 0, 0, 0.2)" stroke="rgba(0, 0, 0, 0.5)"></rect>
+        <rect x="8" y="16" width="1" height="1" fill="rgba(0, 0, 0, 0.2)" stroke="rgba(0, 0, 0, 0.5)"></rect>
+        <rect x="12" y="16" width="1" height="1" fill="rgba(0, 0, 0, 0.2)" stroke="rgba(0, 0, 0, 0.5)"></rect>
+        <rect x="16" y="16" width="1" height="1" fill="rgba(0, 0, 0, 0.2)" stroke="rgba(0, 0, 0, 0.5)"></rect>
+        <path d="M 1 0.5 L 1 0.5 C 1 0.776142 0.776142 1 0.5 1 L 0.5 1 C 0.223858 1 3.38177e-17 0.776142 0 0.5 L 0 0.5 C 3.38177e-17 0.223858 0.223858 9.41135e-17 0.5 1.11022e-16 L 0.5 1.11022e-16 C 0.776142 6.66152e-17 1 0.223858 1 0.5 Z M 5 0.5 L 5 0.5 C 5 0.776142 4.77614 1 4.5 1 L 4.5 1 C 4.22386 1 4 0.776142 4 0.5 L 4 0.5 C 4 0.223858 4.22386 9.41135e-17 4.5 1.11022e-16 L 4.5 1.11022e-16 C 4.77614 6.66152e-17 5 0.223858 5 0.5 Z M 9 0.5 L 9 0.5 C 9 0.776142 8.77614 1 8.5 1 L 8.5 1 C 8.22386 1 8 0.776142 8 0.5 L 8 0.5 C 8 0.223858 8.22386 9.41135e-17 8.5 1.11022e-16 L 8.5 1.11022e-16 C 8.77614 6.66152e-17 9 0.223858 9 0.5 Z M 13 0.5 L 13 0.5 C 13 0.776142 12.7761 1 12.5 1 L 12.5 1 C 12.2239 1 12 0.776142 12 0.5 L 12 0.5 C 12 0.223858 12.2239 9.41135e-17 12.5 1.11022e-16 L 12.5 1.11022e-16 C 12.7761 6.66152e-17 13 0.223858 13 0.5 Z M 17 0.5 L 17 0.5 C 17 0.776142 16.7761 1 16.5 1 L 16.5 1 C 16.2239 1 16 0.776142 16 0.5 L 16 0.5 C 16 0.223858 16.2239 9.41135e-17 16.5 1.11022e-16 L 16.5 1.11022e-16 C 16.7761 6.66152e-17 17 0.223858 17 0.5 Z M 1 4.5 L 1 4.5 C 1 4.77614 0.776142 5 0.5 5 L 0.5 5 C 0.223858 5 3.38177e-17 4.77614 0 4.5 L 0 4.5 C 3.38177e-17 4.22386 0.223858 4 0.5 4 L 0.5 4 C 0.776142 4 1 4.22386 1 4.5 Z M 5 4.5 L 5 4.5 C 5 4.77614 4.77614 5 4.5 5 L 4.5 5 C 4.22386 5 4 4.77614 4 4.5 L 4 4.5 C 4 4.22386 4.22386 4 4.5 4 L 4.5 4 C 4.77614 4 5 4.22386 5 4.5 Z M 9 4.5 L 9 4.5 C 9 4.77614 8.77614 5 8.5 5 L 8.5 5 C 8.22386 5 8 4.77614 8 4.5 L 8 4.5 C 8 4.22386 8.22386 4 8.5 4 L 8.5 4 C 8.77614 4 9 4.22386 9 4.5 Z M 13 4.5 L 13 4.5 C 13 4.77614 12.7761 5 12.5 5 L 12.5 5 C 12.2239 5 12 4.77614 12 4.5 L 12 4.5 C 12 4.22386 12.2239 4 12.5 4 L 12.5 4 C 12.7761 4 13 4.22386 13 4.5 Z M 17 4.5 L 17 4.5 C 17 4.77614 16.7761 5 16.5 5 L 16.5 5 C 16.2239 5 16 4.77614 16 4.5 L 16 4.5 C 16 4.22386 16.2239 4 16.5 4 L 16.5 4 C 16.7761 4 17 4.22386 17 4.5 Z M 1 8.5 L 1 8.5 C 1 8.77614 0.776142 9 0.5 9 L 0.5 9 C 0.223858 9 3.38177e-17 8.77614 0 8.5 L 0 8.5 C 3.38177e-17 8.22386 0.223858 8 0.5 8 L 0.5 8 C 0.776142 8 1 8.22386 1 8.5 Z M 5 8.5 L 5 8.5 C 5 8.77614 4.77614 9 4.5 9 L 4.5 9 C 4.22386 9 4 8.77614 4 8.5 L 4 8.5 C 4 8.22386 4.22386 8 4.5 8 L 4.5 8 C 4.77614 8 5 8.22386 5 8.5 Z M 9 8.5 L 9 8.5 C 9 8.77614 8.77614 9 8.5 9 L 8.5 9 C 8.22386 9 8 8.77614 8 8.5 L 8 8.5 C 8 8.22386 8.22386 8 8.5 8 L 8.5 8 C 8.77614 8 9 8.22386 9 8.5 Z M 13 8.5 L 13 8.5 C 13 8.77614 12.7761 9 12.5 9 L 12.5 9 C 12.2239 9 12 8.77614 12 8.5 L 12 8.5 C 12 8.22386 12.2239 8 12.5 8 L 12.5 8 C 12.7761 8 13 8.22386 13 8.5 Z M 17 8.5 L 17 8.5 C 17 8.77614 16.7761 9 16.5 9 L 16.5 9 C 16.2239 9 16 8.77614 16 8.5 L 16 8.5 C 16 8.22386 16.2239 8 16.5 8 L 16.5 8 C 16.7761 8 17 8.22386 17 8.5 Z M 1 12.5 L 1 12.5 C 1 12.7761 0.776142 13 0.5 13 L 0.5 13 C 0.223858 13 3.38177e-17 12.7761 0 12.5 L 0 12.5 C 3.38177e-17 12.2239 0.223858 12 0.5 12 L 0.5 12 C 0.776142 12 1 12.2239 1 12.5 Z M 5 12.5 L 5 12.5 C 5 12.7761 4.77614 13 4.5 13 L 4.5 13 C 4.22386 13 4 12.7761 4 12.5 L 4 12.5 C 4 12.2239 4.22386 12 4.5 12 L 4.5 12 C 4.77614 12 5 12.2239 5 12.5 Z M 9 12.5 L 9 12.5 C 9 12.7761 8.77614 13 8.5 13 L 8.5 13 C 8.22386 13 8 12.7761 8 12.5 L 8 12.5 C 8 12.2239 8.22386 12 8.5 12 L 8.5 12 C 8.77614 12 9 12.2239 9 12.5 Z M 13 12.5 L 13 12.5 C 13 12.7761 12.7761 13 12.5 13 L 12.5 13 C 12.2239 13 12 12.7761 12 12.5 L 12 12.5 C 12 12.2239 12.2239 12 12.5 12 L 12.5 12 C 12.7761 12 13 12.2239 13 12.5 Z M 17 12.5 L 17 12.5 C 17 12.7761 16.7761 13 16.5 13 L 16.5 13 C 16.2239 13 16 12.7761 16 12.5 L 16 12.5 C 16 12.2239 16.2239 12 16.5 12 L 16.5 12 C 16.7761 12 17 12.2239 17 12.5 Z M 1 16.5 L 1 16.5 C 1 16.7761 0.776142 17 0.5 17 L 0.5 17 C 0.223858 17 3.38177e-17 16.7761 0 16.5 L 0 16.5 C 3.38177e-17 16.2239 0.223858 16 0.5 16 L 0.5 16 C 0.776142 16 1 16.2239 1 16.5 Z M 5 16.5 L 5 16.5 C 5 16.7761 4.77614 17 4.5 17 L 4.5 17 C 4.22386 17 4 16.7761 4 16.5 L 4 16.5 C 4 16.2239 4.22386 16 4.5 16 L 4.5 16 C 4.77614 16 5 16.2239 5 16.5 Z M 9 16.5 L 9 16.5 C 9 16.7761 8.77614 17 8.5 17 L 8.5 17 C 8.22386 17 8 16.7761 8 16.5 L 8 16.5 C 8 16.2239 8.22386 16 8.5 16 L 8.5 16 C 8.77614 16 9 16.2239 9 16.5 Z M 13 16.5 L 13 16.5 C 13 16.7761 12.7761 17 12.5 17 L 12.5 17 C 12.2239 17 12 16.7761 12 16.5 L 12 16.5 C 12 16.2239 12.2239 16 12.5 16 L 12.5 16 C 12.7761 16 13 16.2239 13 16.5 Z M 17 16.5 L 17 16.5 C 17 16.7761 16.7761 17 16.5 17 L 16.5 17 C 16.2239 17 16 16.7761 16 16.5 L 16 16.5 C 16 16.2239 16.2239 16 16.5 16 L 16.5 16 C 16.7761 16 17 16.2239 17 16.5 Z" fill="none" stroke="blue" stroke-width="3"></path>
+    </g>
 </svg>
index aa42ab9..5c88fcc 100644 (file)
@@ -1,5 +1,9 @@
 <script>
 
+// One easy way to generate new expected results for this test: add a
+// waitUntilDone here, run with --no-timeout, attach with the Web Inspector,
+// and copy out the generated SVG path.
+
 if (!window.internals)
         document.write("This test must be run in a test runner.")
 
@@ -229,6 +233,35 @@ window.onload = function () {
         [30, 0, 20, 20],
         [15, 15, 35, 20],
         [0, 30, 50, 20]]);
+
+    // More than 20 (our shrinkwrapping limit):
+
+    testRects([580, 300], [
+        [0, 0, 1, 1],
+        [4, 0, 1, 1],
+        [8, 0, 1, 1],
+        [12, 0, 1, 1],
+        [16, 0, 1, 1],
+        [0, 4, 1, 1],
+        [4, 4, 1, 1],
+        [8, 4, 1, 1],
+        [12, 4, 1, 1],
+        [16, 4, 1, 1],
+        [0, 8, 1, 1],
+        [4, 8, 1, 1],
+        [8, 8, 1, 1],
+        [12, 8, 1, 1],
+        [16, 8, 1, 1],
+        [0, 12, 1, 1],
+        [4, 12, 1, 1],
+        [8, 12, 1, 1],
+        [12, 12, 1, 1],
+        [16, 12, 1, 1],
+        [0, 16, 1, 1],
+        [4, 16, 1, 1],
+        [8, 16, 1, 1],
+        [12, 16, 1, 1],
+        [16, 16, 1, 1]]);    
 }
 
 </script>
index 105b0d5..4acc932 100644 (file)
@@ -1,3 +1,18 @@
+2018-12-12  Tim Horton  <timothy_horton@apple.com>
+
+        REGRESSION (r237565): >20 Find in Page highlights in one tile results in a single giant highlight
+        https://bugs.webkit.org/show_bug.cgi?id=192642
+        <rdar://problem/46498246>
+
+        Reviewed by Geoffrey Garen.
+
+        No new tests; adjusted an existing test instead.
+
+        * platform/graphics/PathUtilities.cpp:
+        (WebCore::PathUtilities::pathsWithShrinkWrappedRects):
+        Instead of uniting when we fail to shrink-wrap, just return the original rects.
+        This seems like a more reasonable default in most cases.
+
 2018-12-12  Vivek Seth  <v_seth@apple.com>
 
         HTTPS Upgrade: Figure out if/how to tell clients that the HTTPS upgrade happened
index 914a0a2..68ba059 100644 (file)
@@ -291,7 +291,8 @@ Vector<Path> PathUtilities::pathsWithShrinkWrappedRects(const Vector<FloatRect>&
 
     if (rects.size() > 20) {
         Path path;
-        path.addRoundedRect(unionRect(rects), FloatSize(radius, radius));
+        for (const auto& rect : rects)
+            path.addRoundedRect(rect, FloatSize(radius, radius));
         paths.append(path);
         return paths;
     }
@@ -300,7 +301,8 @@ Vector<Path> PathUtilities::pathsWithShrinkWrappedRects(const Vector<FloatRect>&
     Vector<FloatPointGraph::Polygon> polys = polygonsForRect(rects, graph);
     if (polys.isEmpty()) {
         Path path;
-        path.addRoundedRect(unionRect(rects), FloatSize(radius, radius));
+        for (const auto& rect : rects)
+            path.addRoundedRect(rect, FloatSize(radius, radius));
         paths.append(path);
         return paths;
     }