Title: [223552] trunk
Revision
223552
Author
[email protected]
Date
2017-10-17 10:04:51 -0700 (Tue, 17 Oct 2017)

Log Message

REGRESSION (r222670 and r222732): RTL truncated text may not be drawn
https://bugs.webkit.org/show_bug.cgi?id=178278
<rdar://problem/34982818>

Reviewed by Darin Adler.

Source/WebCore:

Revert r222732 and partially revert r222670. The underlying font rendering machinery implements
text truncation by taking a TextRun object that represents all of the text in the line fragment
and a subrange of the glyphs to render from this fragment. Only the glyphs in this subrange are
drawn and they are drawn in the same position they would be in had the entire line fragment been
drawn. Following r222670 InlineTextBox applies the truncation to the TextRun in InlineTextBox::text().
Together with r222732, which assumed that the number of glyphs to draw is equal to the length of
the TextRun, a truncated text run would be drawn at the wrong position on screen and could give
the impression that the text is not drawn. Instead InlineTextBox::text() should always return
the text for the entire line fragment without considering truncation and when calling TextPainter::paint()
we need to pass the truncated length of the line fragment.

Test: fast/text/ellipsis-text-rtl.html

* rendering/InlineTextBox.cpp:
(WebCore::InlineTextBox::paint): Compute the truncated length (number of glyphs) and pass this
to TextPainter::paint()
(WebCore::InlineTextBox::text const): Do not apply truncation to the text run. Truncation is
implemented by telling the underlying font rendering machinery to paint the subrange of the
text run that represents the non-truncated (visible) text.
* rendering/InlineTextBox.h:
* rendering/SimpleLineLayoutFunctions.cpp:
(WebCore::SimpleLineLayout::paintFlow): Pass the entire length of the text run as we did prior
to r222732.
* rendering/TextPainter.cpp:
(WebCore::TextPainter::paint): Take a length that represents the number of glyphs to draw from
the text run as we use to take prior to r222732.
* rendering/TextPainter.h:

LayoutTests:

Add a test to ensure that we draw right-to-left truncated text correctly.

* fast/text/ellipsis-text-rtl-expected.html: Added.
* fast/text/ellipsis-text-rtl.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (223551 => 223552)


--- trunk/LayoutTests/ChangeLog	2017-10-17 16:34:45 UTC (rev 223551)
+++ trunk/LayoutTests/ChangeLog	2017-10-17 17:04:51 UTC (rev 223552)
@@ -1,3 +1,16 @@
+2017-10-17  Daniel Bates  <[email protected]>
+
+        REGRESSION (r222670 and r222732): RTL truncated text may not be drawn
+        https://bugs.webkit.org/show_bug.cgi?id=178278
+        <rdar://problem/34982818>
+
+        Reviewed by Darin Adler.
+
+        Add a test to ensure that we draw right-to-left truncated text correctly.
+
+        * fast/text/ellipsis-text-rtl-expected.html: Added.
+        * fast/text/ellipsis-text-rtl.html: Added.
+
 2017-10-10  Yusuke Suzuki  <[email protected]>
 
         [JSC] __proto__ getter should be fast

Added: trunk/LayoutTests/fast/text/ellipsis-text-rtl-expected.html (0 => 223552)


--- trunk/LayoutTests/fast/text/ellipsis-text-rtl-expected.html	                        (rev 0)
+++ trunk/LayoutTests/fast/text/ellipsis-text-rtl-expected.html	2017-10-17 17:04:51 UTC (rev 223552)
@@ -0,0 +1,19 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+@font-face {
+    font-family: "Ahem";
+    src: url("../../resources/Ahem.ttf");
+}
+
+#expected {
+    font: 20px/1 "Ahem";
+    overflow: hidden;
+}
+</style>
+</head>
+<body>
+<div id="expected">ab</div>
+</body>
+</html>

Added: trunk/LayoutTests/fast/text/ellipsis-text-rtl.html (0 => 223552)


--- trunk/LayoutTests/fast/text/ellipsis-text-rtl.html	                        (rev 0)
+++ trunk/LayoutTests/fast/text/ellipsis-text-rtl.html	2017-10-17 17:04:51 UTC (rev 223552)
@@ -0,0 +1,24 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+@font-face {
+    font-family: "Ahem";
+    src: url("../../resources/Ahem.ttf");
+}
+
+#test {
+    font: 20px/1 "Ahem";
+    direction: rtl;
+    unicode-bidi: bidi-override;
+    text-overflow: ellipsis;
+    overflow: hidden;
+    white-space: nowrap;
+    width: 40px;
+}
+</style>
+</head>
+<body>
+<div id="test">abcXYZ</div>
+</body>
+</html>

Modified: trunk/Source/WebCore/ChangeLog (223551 => 223552)


--- trunk/Source/WebCore/ChangeLog	2017-10-17 16:34:45 UTC (rev 223551)
+++ trunk/Source/WebCore/ChangeLog	2017-10-17 17:04:51 UTC (rev 223552)
@@ -1,3 +1,39 @@
+2017-10-17  Daniel Bates  <[email protected]>
+
+        REGRESSION (r222670 and r222732): RTL truncated text may not be drawn
+        https://bugs.webkit.org/show_bug.cgi?id=178278
+        <rdar://problem/34982818>
+
+        Reviewed by Darin Adler.
+
+        Revert r222732 and partially revert r222670. The underlying font rendering machinery implements
+        text truncation by taking a TextRun object that represents all of the text in the line fragment
+        and a subrange of the glyphs to render from this fragment. Only the glyphs in this subrange are
+        drawn and they are drawn in the same position they would be in had the entire line fragment been
+        drawn. Following r222670 InlineTextBox applies the truncation to the TextRun in InlineTextBox::text().
+        Together with r222732, which assumed that the number of glyphs to draw is equal to the length of
+        the TextRun, a truncated text run would be drawn at the wrong position on screen and could give
+        the impression that the text is not drawn. Instead InlineTextBox::text() should always return
+        the text for the entire line fragment without considering truncation and when calling TextPainter::paint()
+        we need to pass the truncated length of the line fragment.
+
+        Test: fast/text/ellipsis-text-rtl.html
+
+        * rendering/InlineTextBox.cpp:
+        (WebCore::InlineTextBox::paint): Compute the truncated length (number of glyphs) and pass this
+        to TextPainter::paint()
+        (WebCore::InlineTextBox::text const): Do not apply truncation to the text run. Truncation is
+        implemented by telling the underlying font rendering machinery to paint the subrange of the
+        text run that represents the non-truncated (visible) text.
+        * rendering/InlineTextBox.h:
+        * rendering/SimpleLineLayoutFunctions.cpp:
+        (WebCore::SimpleLineLayout::paintFlow): Pass the entire length of the text run as we did prior
+        to r222732.
+        * rendering/TextPainter.cpp:
+        (WebCore::TextPainter::paint): Take a length that represents the number of glyphs to draw from
+        the text run as we use to take prior to r222732.
+        * rendering/TextPainter.h:
+
 2017-10-17  Zalan Bujtas  <[email protected]>
 
         [FrameView::layout cleanup] Move text auto sizing logic to a separate function

Modified: trunk/Source/WebCore/rendering/InlineTextBox.cpp (223551 => 223552)


--- trunk/Source/WebCore/rendering/InlineTextBox.cpp	2017-10-17 16:34:45 UTC (rev 223551)
+++ trunk/Source/WebCore/rendering/InlineTextBox.cpp	2017-10-17 17:04:51 UTC (rev 223552)
@@ -491,6 +491,8 @@
     auto text = this->text();
     TextRun textRun = createTextRun(text);
     unsigned length = textRun.length();
+    if (m_truncation != cNoTruncation)
+        length = m_truncation;
 
     unsigned selectionStart = 0;
     unsigned selectionEnd = 0;
@@ -549,7 +551,7 @@
         if (currentEnd < length)
             textPainter.paintRange(textRun, boxRect, textOrigin, currentEnd, length);
     } else
-        textPainter.paint(textRun, boxRect, textOrigin, selectionStart, selectionEnd, paintSelectedTextOnly, paintSelectedTextSeparately, paintNonSelectedTextOnly);
+        textPainter.paint(textRun, length, boxRect, textOrigin, selectionStart, selectionEnd, paintSelectedTextOnly, paintSelectedTextSeparately, paintNonSelectedTextOnly);
 
     // Paint decorations
     TextDecoration textDecorations = lineStyle.textDecorationsInEffect();
@@ -1077,8 +1079,6 @@
 
 String InlineTextBox::text(bool ignoreCombinedText, bool ignoreHyphen) const
 {
-    if (UNLIKELY(m_truncation == cFullTruncation))
-        return emptyString();
     if (auto* combinedText = this->combinedText()) {
         if (ignoreCombinedText)
             return renderer().text()->substring(m_start, m_len);
@@ -1089,7 +1089,7 @@
             return renderer().text()->substring(m_start, m_len);
         return makeString(StringView(renderer().text()).substring(m_start, m_len), lineStyle().hyphenString());
     }
-    return renderer().text()->substring(m_start, m_truncation != cNoTruncation ? m_truncation : m_len);
+    return renderer().text()->substring(m_start, m_len);
 }
 
 inline const RenderCombineText* InlineTextBox::combinedText() const

Modified: trunk/Source/WebCore/rendering/InlineTextBox.h (223551 => 223552)


--- trunk/Source/WebCore/rendering/InlineTextBox.h	2017-10-17 16:34:45 UTC (rev 223551)
+++ trunk/Source/WebCore/rendering/InlineTextBox.h	2017-10-17 17:04:51 UTC (rev 223552)
@@ -167,7 +167,7 @@
     const RenderCombineText* combinedText() const;
     const FontCascade& lineFont() const;
 
-    String text(bool ignoreCombinedText = false, bool ignoreHyphen = false) const; // The text to render.
+    String text(bool ignoreCombinedText = false, bool ignoreHyphen = false) const; // The effective text for the run.
     TextRun createTextRun(String&) const;
 
     ExpansionBehavior expansionBehavior() const;

Modified: trunk/Source/WebCore/rendering/SimpleLineLayoutFunctions.cpp (223551 => 223552)


--- trunk/Source/WebCore/rendering/SimpleLineLayoutFunctions.cpp	2017-10-17 16:34:45 UTC (rev 223551)
+++ trunk/Source/WebCore/rendering/SimpleLineLayoutFunctions.cpp	2017-10-17 17:04:51 UTC (rev 223552)
@@ -120,7 +120,7 @@
         TextRun textRun(run.hasHyphen() ? textWithHyphen : run.text(), 0, run.expansion(), run.expansionBehavior());
         textRun.setTabSize(!style.collapseWhiteSpace(), style.tabSize());
         FloatPoint textOrigin = FloatPoint(rect.x() + paintOffset.x(), roundToDevicePixel(run.baselinePosition() + paintOffset.y(), deviceScaleFactor));
-        textPainter.paint(textRun, rect, textOrigin);
+        textPainter.paint(textRun, textRun.length(), rect, textOrigin);
         if (textDecorationPainter) {
             textDecorationPainter->setWidth(rect.width());
             textDecorationPainter->paintTextDecoration(textRun, textOrigin, rect.location() + paintOffset);

Modified: trunk/Source/WebCore/rendering/TextPainter.cpp (223551 => 223552)


--- trunk/Source/WebCore/rendering/TextPainter.cpp	2017-10-17 16:34:45 UTC (rev 223551)
+++ trunk/Source/WebCore/rendering/TextPainter.cpp	2017-10-17 17:04:51 UTC (rev 223552)
@@ -190,11 +190,11 @@
     paintTextAndEmphasisMarksIfNeeded(textRun, boxRect, textOrigin, start, end, m_style, m_shadow);
 }
     
-void TextPainter::paint(const TextRun& textRun, const FloatRect& boxRect, const FloatPoint& textOrigin, unsigned selectionStart, unsigned selectionEnd,
+void TextPainter::paint(const TextRun& textRun, unsigned length, const FloatRect& boxRect, const FloatPoint& textOrigin, unsigned selectionStart, unsigned selectionEnd,
     bool paintSelectedTextOnly, bool paintSelectedTextSeparately, bool paintNonSelectedTextOnly)
 {
     ASSERT(m_font);
-    unsigned length = textRun.length();
+    ASSERT(length <= textRun.length());
     if (!paintSelectedTextOnly) {
         // For stroked painting, we have to change the text drawing mode. It's probably dangerous to leave that mutated as a side
         // effect, so only when we know we're stroking, do a save/restore.

Modified: trunk/Source/WebCore/rendering/TextPainter.h (223551 => 223552)


--- trunk/Source/WebCore/rendering/TextPainter.h	2017-10-17 16:34:45 UTC (rev 223551)
+++ trunk/Source/WebCore/rendering/TextPainter.h	2017-10-17 17:04:51 UTC (rev 223552)
@@ -62,7 +62,7 @@
     void setEmphasisMark(const AtomicString& mark, float offset, const RenderCombineText*);
 
     void paintRange(const TextRun&, const FloatRect& boxRect, const FloatPoint& textOrigin, unsigned start, unsigned end);
-    void paint(const TextRun&, const FloatRect& boxRect, const FloatPoint& textOrigin, unsigned selectionStart = 0, unsigned selectionEnd = 0, bool paintSelectedTextOnly = false, bool paintSelectedTextSeparately = false, bool paintNonSelectedTextOnly = false);
+    void paint(const TextRun&, unsigned length, const FloatRect& boxRect, const FloatPoint& textOrigin, unsigned selectionStart = 0, unsigned selectionEnd = 0, bool paintSelectedTextOnly = false, bool paintSelectedTextSeparately = false, bool paintNonSelectedTextOnly = false);
 
 private:
     void paintTextOrEmphasisMarks(const FontCascade&, const TextRun&, const AtomicString& emphasisMark, float emphasisMarkOffset,
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to