Changeset 216088 in webkit

Timestamp:

May 2, 2017 12:35:25 PM (7 years ago)

Author:

mmaxfield@apple.com

Message:

REGRESSION (r211382): Partial right-to-left text runs are painted at an offset (breaks Find indicators, Look Up, and custom ::selection style)
​https://bugs.webkit.org/show_bug.cgi?id=169517
<rdar://problem/30652443>

Reviewed by Dean Jackson.

Source/WebCore:

FontCascade::getGlyphsAndAdvancesForComplexText() is tasked with calculating paint advances for a
subrange of RTL text. It does this by creating a ComplexTextController, telling it to iterate to
the beginning of the subrange (outputting to a GlyphBuffer), then telling it to iterate to the end
of the subrange (outputting to another GlyphBuffer). Because the text is RTL, the sum of the
advances gathered so far is the distance from the right edge of the text to the left edge of the
subrange (because we advance in logical order). Therefore, the x-coordinate we are at now is the
total width minus the sum of both of the GlyphBuffers. For some reason, when I wrote this code I
forgot to add in the contribution from the first GlyphBuffer. Unfortunately, this particular
codepath is rarely hit in practice and completely untested, which made me miss it when I wrote it.

Test: fast/text/complex-text-selection.html

• platform/graphics/cocoa/FontCascadeCocoa.mm:

(WebCore::FontCascade::getGlyphsAndAdvancesForComplexText):

LayoutTests:

• fast/text/complex-text-selection-expected.html: Added.
• fast/text/complex-text-selection.html: Added.
• platform/ios/TestExpectations:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/text/complex-text-selection-expected.html (added)
• LayoutTests/fast/text/complex-text-selection.html (added)
• LayoutTests/platform/ios/TestExpectations (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/platform/graphics/cocoa/FontCascadeCocoa.mm (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2017-05-02  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        REGRESSION (r211382): Partial right-to-left text runs are painted at an offset (breaks Find indicators, Look Up, and custom ::selection style)
+        https://bugs.webkit.org/show_bug.cgi?id=169517
+        <rdar://problem/30652443>
+
+        Reviewed by Dean Jackson.
+
+        * fast/text/complex-text-selection-expected.html: Added.
+        * fast/text/complex-text-selection.html: Added.
+        * platform/ios/TestExpectations:
+
 2017-05-02  Joseph Pecoraro  <pecoraro@apple.com>
 

trunk/LayoutTests/platform/ios/TestExpectations

@@ -2947 +2947 @@
 # auto-sizing produces inconsistent image results
 css3/viewport-percentage-lengths/vh-auto-size.html [ ImageOnlyFailure ]
+
+# This test relies on the ::selection pseudoclass which isn't honored on iOS.
+webkit.org/b/169517 fast/text/complex-text-selection.html [ ImageOnlyFailure ]

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2017-05-02  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        REGRESSION (r211382): Partial right-to-left text runs are painted at an offset (breaks Find indicators, Look Up, and custom ::selection style)
+        https://bugs.webkit.org/show_bug.cgi?id=169517
+        <rdar://problem/30652443>
+
+        Reviewed by Dean Jackson.
+
+        FontCascade::getGlyphsAndAdvancesForComplexText() is tasked with calculating paint advances for a
+        subrange of RTL text. It does this by creating a ComplexTextController, telling it to iterate to
+        the beginning of the subrange (outputting to a GlyphBuffer), then telling it to iterate to the end
+        of the subrange (outputting to another GlyphBuffer). Because the text is RTL, the sum of the
+        advances gathered so far is the distance from the right edge of the text to the left edge of the
+        subrange (because we advance in logical order). Therefore, the x-coordinate we are at now is the
+        total width minus the sum of both of the GlyphBuffers. For some reason, when I wrote this code I
+        forgot to add in the contribution from the first GlyphBuffer. Unfortunately, this particular
+        codepath is rarely hit in practice and completely untested, which made me miss it when I wrote it.
+
+        Test: fast/text/complex-text-selection.html
+
+        * platform/graphics/cocoa/FontCascadeCocoa.mm:
+        (WebCore::FontCascade::getGlyphsAndAdvancesForComplexText):
+
 2017-05-02  Chris Dumez  <cdumez@apple.com>
 

trunk/Source/WebCore/platform/graphics/cocoa/FontCascadeCocoa.mm

@@ -523 +523 @@
         // the sum of the layout advances.
         initialAdvance = controller.totalWidth();
+        for (unsigned i = 0; i < dummyGlyphBuffer.size(); ++i)
+            initialAdvance -= dummyGlyphBuffer.advanceAt(i).width();
         for (unsigned i = 0; i < glyphBuffer.size(); ++i)
             initialAdvance -= glyphBuffer.advanceAt(i).width();