Changeset 146630 in webkit

Timestamp:

Mar 22, 2013 10:51:11 AM (11 years ago)

Author:

allan.jensen@digia.com

Message:

[Qt] New fast/text/word-space-with-kerning-3.html fails on Qt.
​https://bugs.webkit.org/show_bug.cgi?id=112668

Reviewed by Jocelyn Turcotte.

Source/WebCore:

Qt adds word-spacing to leading spaces, but WebCore only expects
us to add for trailing spaces. We only corrected for this in width
calculation but do need to also do it for drawing.

Instead of subtracting the extra word-spacing we now configure the
FormatRange not to apply to leading spaces. This means this behavior
will be applied everywhere reliably.

• platform/graphics/Font.h:

(Font):

• platform/graphics/qt/FontQt.cpp:

(WebCore::Font::drawComplexText):
(WebCore::Font::floatWidthForComplexText):
(WebCore::Font::offsetForPositionForComplexText):
(WebCore::Font::selectionRectForComplexText):
(WebCore::Font::initFormatForTextLayout):

LayoutTests:

• platform/qt/TestExpectations:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/platform/qt/TestExpectations (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/platform/graphics/Font.h (modified) (1 diff)
• Source/WebCore/platform/graphics/qt/FontQt.cpp (modified) (6 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2013-03-22  Allan Sandfeld Jensen  <allan.jensen@digia.com>
+
+        [Qt] New fast/text/word-space-with-kerning-3.html fails on Qt.
+        https://bugs.webkit.org/show_bug.cgi?id=112668
+
+        Reviewed by Jocelyn Turcotte.
+
+        * platform/qt/TestExpectations:
+
 2013-03-22  Nate Chapin  <japhet@chromium.org>
 

trunk/LayoutTests/platform/qt/TestExpectations

@@ -1824 +1824 @@
 fast/text/international/spaces-combined-in-vertical-text.html
 
-webkit.org/b/112668 fast/text/word-space-with-kerning-3.html [ Skip ]
-
 fast/writing-mode/border-styles-vertical-lr.html
 fast/writing-mode/border-styles-vertical-rl.html

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2013-03-22  Allan Sandfeld Jensen  <allan.jensen@digia.com>
+
+        [Qt] New fast/text/word-space-with-kerning-3.html fails on Qt.
+        https://bugs.webkit.org/show_bug.cgi?id=112668
+
+        Reviewed by Jocelyn Turcotte.
+
+        Qt adds word-spacing to leading spaces, but WebCore only expects
+        us to add for trailing spaces. We only corrected for this in width
+        calculation but do need to also do it for drawing.
+
+        Instead of subtracting the extra word-spacing we now configure the
+        FormatRange not to apply to leading spaces. This means this behavior
+        will be applied everywhere reliably.
+
+        * platform/graphics/Font.h:
+        (Font):
+        * platform/graphics/qt/FontQt.cpp:
+        (WebCore::Font::drawComplexText):
+        (WebCore::Font::floatWidthForComplexText):
+        (WebCore::Font::offsetForPositionForComplexText):
+        (WebCore::Font::selectionRectForComplexText):
+        (WebCore::Font::initFormatForTextLayout):
+
 2013-03-22  Joshua Bell  <jsbell@chromium.org>
 

trunk/Source/WebCore/platform/graphics/Font.h

@@ -298 +298 @@
 
 #if PLATFORM(QT)
-    void initFormatForTextLayout(QTextLayout*) const;
+    void initFormatForTextLayout(QTextLayout*, const TextRun&) const;
 #endif
 

trunk/Source/WebCore/platform/graphics/qt/FontQt.cpp

@@ -183 +183 @@
     QTextLayout layout(string);
     layout.setRawFont(rawFont());
-    initFormatForTextLayout(&layout);
+    initFormatForTextLayout(&layout, run);
     QTextLine line = setupLayout(&layout, run);
     const QPointF adjustedPoint(point.x(), point.y() - line.ascent());
@@ -207 +207 @@
     QTextLayout layout(string);
     layout.setRawFont(rawFont());
-    initFormatForTextLayout(&layout);
+    initFormatForTextLayout(&layout, run);
     QTextLine line = setupLayout(&layout, run);
     float x1 = line.cursorToX(0);
     float x2 = line.cursorToX(run.length());
     float width = qAbs(x2 - x1);
-    // RenderBlockLineLayout expects us to only add word-spacing for trailing spaces, not for leading spaces.
-    if (treatAsSpace(run[0]))
-        width -= m_wordSpacing;
 
     return width + run.expansion();
@@ -226 +223 @@
     QTextLayout layout(string);
     layout.setRawFont(rawFont());
-    initFormatForTextLayout(&layout);
+    initFormatForTextLayout(&layout, run);
     QTextLine line = setupLayout(&layout, run);
     return line.xToCursor(position);
@@ -238 +235 @@
     QTextLayout layout(string);
     layout.setRawFont(rawFont());
-    initFormatForTextLayout(&layout);
+    initFormatForTextLayout(&layout, run);
     QTextLine line = setupLayout(&layout, run);
 
@@ -249 +246 @@
 }
 
-void Font::initFormatForTextLayout(QTextLayout* layout) const
+void Font::initFormatForTextLayout(QTextLayout* layout, const TextRun& run) const
 {
     QTextLayout::FormatRange range;
-    range.start = 0;
-    range.length = layout->text().length();
+    // WebCore expects word-spacing to be ignored on leading spaces contrary to what Qt does.
+    // To avoid word-spacing on any leading spaces, we exclude them from FormatRange which
+    // word-spacing along with other options would be applied to. This is safe since the other
+    // formatting options does not affect spaces.
+    unsigned length = layout->text().length();
+    for (range.start = 0; treatAsSpace(run[range.start]) && range.start < length; ++range.start) { }
+    range.length = length - range.start;
+
     if (m_wordSpacing)
         range.format.setFontWordSpacing(m_wordSpacing);
@@ -263 +266 @@
         range.format.setFontCapitalization(QFont::SmallCaps);
 
-    if (range.format.propertyCount())
+    if (range.format.propertyCount() && range.length)
         layout->setAdditionalFormats(QList<QTextLayout::FormatRange>() << range);
 }