Title: [226194] trunk
Revision
226194
Author
dba...@webkit.org
Date
2017-12-20 11:59:35 -0800 (Wed, 20 Dec 2017)

Log Message

MarkerSubrange.SubdivideGrammarAndSelectionOverlap{Frontmost, FrontmostWithLongestEffectiveRange} are failing
https://bugs.webkit.org/show_bug.cgi?id=181014

Reviewed by Simon Fraser.

Source/WebCore:

Fixes an issue in the subdivision algorithm where the returned subranges may not be paint order
or reverse paint order when using the default overlap strategy (OverlapStrategy::None) and
either OverlapStrategy::Frontmost or OverlapStrategy::FrontmostWithLongestEffectiveRange, respectively.

Currently we compute the overlapping subranges up to some point p_i on the line by sweeping from the
start of the line through all the unclosed subranges. The unclosed subranges are sorted along the line.
That is, they are not sorted by paint order or reverse paint order. Therefore we must take care to
ensure that we return the computed overlapping subranges with respect to paint order/reverse paint order.

* rendering/MarkerSubrange.cpp:
(WebCore::subdivide):

Tools:

Adds a new test to ensure we compute overlapping subranges in paint order for the default
overlap strategy. Enable tests MarkerSubrange.SubdivideGrammarAndSelectionOverlap{Frontmost, FrontmostWithLongestEffectiveRange}
now that they pass.

* TestWebKitAPI/Tests/WebCore/MarkerSubrange.cpp:
(TestWebKitAPI::TEST):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (226193 => 226194)


--- trunk/Source/WebCore/ChangeLog	2017-12-20 19:29:08 UTC (rev 226193)
+++ trunk/Source/WebCore/ChangeLog	2017-12-20 19:59:35 UTC (rev 226194)
@@ -1,3 +1,22 @@
+2017-12-20  Daniel Bates  <daba...@apple.com>
+
+        MarkerSubrange.SubdivideGrammarAndSelectionOverlap{Frontmost, FrontmostWithLongestEffectiveRange} are failing
+        https://bugs.webkit.org/show_bug.cgi?id=181014
+
+        Reviewed by Simon Fraser.
+
+        Fixes an issue in the subdivision algorithm where the returned subranges may not be paint order
+        or reverse paint order when using the default overlap strategy (OverlapStrategy::None) and
+        either OverlapStrategy::Frontmost or OverlapStrategy::FrontmostWithLongestEffectiveRange, respectively.
+
+        Currently we compute the overlapping subranges up to some point p_i on the line by sweeping from the
+        start of the line through all the unclosed subranges. The unclosed subranges are sorted along the line.
+        That is, they are not sorted by paint order or reverse paint order. Therefore we must take care to
+        ensure that we return the computed overlapping subranges with respect to paint order/reverse paint order.
+
+        * rendering/MarkerSubrange.cpp:
+        (WebCore::subdivide):
+
 2017-12-20  Youenn Fablet  <you...@apple.com>
 
         LayoutTest imported/w3c/web-platform-tests/service-workers/cache-storage/serviceworker/cache-match.https.html is a flaky failure

Modified: trunk/Source/WebCore/rendering/MarkerSubrange.cpp (226193 => 226194)


--- trunk/Source/WebCore/rendering/MarkerSubrange.cpp	2017-12-20 19:29:08 UTC (rev 226193)
+++ trunk/Source/WebCore/rendering/MarkerSubrange.cpp	2017-12-20 19:59:35 UTC (rev 226194)
@@ -70,7 +70,7 @@
             if (overlapStrategy == OverlapStrategy::Frontmost || overlapStrategy == OverlapStrategy::FrontmostWithLongestEffectiveRange) {
                 std::optional<unsigned> frontmost;
                 for (unsigned j = 0; j < i; ++j) {
-                    if (!processedSubranges.contains(offsets[j].subrange))
+                    if (!processedSubranges.contains(offsets[j].subrange) && (!frontmost || offsets[j].subrange->type > offsets[*frontmost].subrange->type))
                         frontmost = j;
                 }
                 if (frontmost) {
@@ -84,6 +84,7 @@
                         result.append({ offsetSoFar, offsets[i].value, offsets[frontmost.value()].subrange->type, offsets[frontmost.value()].subrange->marker });
                 }
             } else {
+                // The appended subranges may not be in paint order. We will fix this up at the end of this function.
                 for (unsigned j = 0; j < i; ++j) {
                     if (!processedSubranges.contains(offsets[j].subrange))
                         result.append({ offsetSoFar, offsets[i].value, offsets[j].subrange->type, offsets[j].subrange->marker });
@@ -94,6 +95,9 @@
         if (offsets[i].kind == Offset::End)
             processedSubranges.add(offsets[i].subrange);
     }
+    // Fix up; sort the subranges so that they are in paint order.
+    if (overlapStrategy == OverlapStrategy::None)
+        std::sort(result.begin(), result.end(), [] (const MarkerSubrange& a, const MarkerSubrange& b) { return a.startOffset < b.startOffset || (a.startOffset == b.startOffset && a.type < b.type); });
     return result;
 }
 

Modified: trunk/Tools/ChangeLog (226193 => 226194)


--- trunk/Tools/ChangeLog	2017-12-20 19:29:08 UTC (rev 226193)
+++ trunk/Tools/ChangeLog	2017-12-20 19:59:35 UTC (rev 226194)
@@ -1,5 +1,19 @@
 2017-12-20  Daniel Bates  <daba...@apple.com>
 
+        MarkerSubrange.SubdivideGrammarAndSelectionOverlap{Frontmost, FrontmostWithLongestEffectiveRange} are failing
+        https://bugs.webkit.org/show_bug.cgi?id=181014
+
+        Reviewed by Simon Fraser.
+
+        Adds a new test to ensure we compute overlapping subranges in paint order for the default
+        overlap strategy. Enable tests MarkerSubrange.SubdivideGrammarAndSelectionOverlap{Frontmost, FrontmostWithLongestEffectiveRange}
+        now that they pass.
+
+        * TestWebKitAPI/Tests/WebCore/MarkerSubrange.cpp:
+        (TestWebKitAPI::TEST):
+
+2017-12-20  Daniel Bates  <daba...@apple.com>
+
         Remove Alternative Presentation Button
         https://bugs.webkit.org/show_bug.cgi?id=180500
         <rdar://problem/35891047>

Modified: trunk/Tools/TestWebKitAPI/Tests/WebCore/MarkerSubrange.cpp (226193 => 226194)


--- trunk/Tools/TestWebKitAPI/Tests/WebCore/MarkerSubrange.cpp	2017-12-20 19:29:08 UTC (rev 226193)
+++ trunk/Tools/TestWebKitAPI/Tests/WebCore/MarkerSubrange.cpp	2017-12-20 19:59:35 UTC (rev 226194)
@@ -174,7 +174,7 @@
         EXPECT_EQ(expectedSubranges[i], results[i]);
 }
 
-TEST(MarkerSubrange, DISABLED_SubdivideGrammarAndSelectionOverlapFrontmost)
+TEST(MarkerSubrange, SubdivideGrammarAndSelectionOverlap)
 {
     Vector<MarkerSubrange> subranges {
         MarkerSubrange { 0, 40, MarkerSubrange::GrammarError },
@@ -183,10 +183,31 @@
     };
     Vector<MarkerSubrange> expectedSubranges {
         MarkerSubrange { 0, 2, MarkerSubrange::GrammarError },
+        MarkerSubrange { 2, 40, MarkerSubrange::GrammarError },
         MarkerSubrange { 2, 40, MarkerSubrange::Selection },
         MarkerSubrange { 40, 50, MarkerSubrange::Selection },
+        MarkerSubrange { 50, 60, MarkerSubrange::GrammarError },
         MarkerSubrange { 50, 60, MarkerSubrange::Selection },
     };
+    auto results = subdivide(subranges);
+    ASSERT_EQ(expectedSubranges.size(), results.size());
+    for (size_t i = 0; i < expectedSubranges.size(); ++i)
+        EXPECT_EQ(expectedSubranges[i], results[i]);
+}
+
+TEST(MarkerSubrange, SubdivideGrammarAndSelectionOverlapFrontmost)
+{
+    Vector<MarkerSubrange> subranges {
+        MarkerSubrange { 0, 40, MarkerSubrange::GrammarError },
+        MarkerSubrange { 2, 60, MarkerSubrange::Selection },
+        MarkerSubrange { 50, 60, MarkerSubrange::GrammarError },
+    };
+    Vector<MarkerSubrange> expectedSubranges {
+        MarkerSubrange { 0, 2, MarkerSubrange::GrammarError },
+        MarkerSubrange { 2, 40, MarkerSubrange::Selection },
+        MarkerSubrange { 40, 50, MarkerSubrange::Selection },
+        MarkerSubrange { 50, 60, MarkerSubrange::Selection },
+    };
     auto results = subdivide(subranges, OverlapStrategy::Frontmost);
     ASSERT_EQ(expectedSubranges.size(), results.size());
     for (size_t i = 0; i < expectedSubranges.size(); ++i)
@@ -193,7 +214,7 @@
         EXPECT_EQ(expectedSubranges[i], results[i]);
 }
 
-TEST(MarkerSubrange, DISABLED_SubdivideGrammarAndSelectionOverlapFrontmostWithLongestEffectiveRange)
+TEST(MarkerSubrange, SubdivideGrammarAndSelectionOverlapFrontmostWithLongestEffectiveRange)
 {
     Vector<MarkerSubrange> subranges {
         MarkerSubrange { 0, 40, MarkerSubrange::GrammarError },
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to