[SCM] WebKit Debian packaging branch, debian/experimental, updated. upstream/1.3.3-9427-gc2be6fc

agl at chromium.org agl at chromium.org
Wed Dec 22 12:09:49 UTC 2010


The following commit has been merged in the debian/experimental branch:
commit 62c6859db6a6f6518252d9f192d4eae7581df8df
Author: agl at chromium.org <agl at chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date:   Mon Aug 16 15:16:46 2010 +0000

    2010-08-16  Adam Langley  <agl at chromium.org>
    
            Reviewed by Tony Chang.
    
            [chromium] Improve spacing support for complex text on Linux
    
            https://bugs.webkit.org/show_bug.cgi?id=39014
    
            Previously, our complex text support ignored word-spacing,
            justification and letter-spacing. This fixes the first two issues and
            allows us to render Scribd's HTML5 documents much better.
    
            Test: fast/text/atsui-spacing-features.html
    
            * platform/graphics/chromium/FontLinux.cpp:
            (WebCore::TextRunWalker::TextRunWalker):
            (WebCore::TextRunWalker::setWordSpacingAdjustment):
            (WebCore::TextRunWalker::setLetterSpacingAdjustment):
            (WebCore::TextRunWalker::setPadding):
            (WebCore::TextRunWalker::setGlyphXPositions):
            (WebCore::TextRunWalker::isCodepointSpace):
            (WebCore::Font::drawComplexText):
            (WebCore::Font::floatWidthForComplexText):
            (WebCore::Font::offsetForPositionForComplexText):
            (WebCore::Font::selectionRectForComplexText):
    
    git-svn-id: http://svn.webkit.org/repository/webkit/trunk@65428 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/LayoutTests/platform/chromium-linux/fast/text/atsui-spacing-features-expected.checksum b/LayoutTests/platform/chromium-linux/fast/text/atsui-spacing-features-expected.checksum
index bfa1ae8..714b9bb 100644
--- a/LayoutTests/platform/chromium-linux/fast/text/atsui-spacing-features-expected.checksum
+++ b/LayoutTests/platform/chromium-linux/fast/text/atsui-spacing-features-expected.checksum
@@ -1 +1 @@
-a8ea417f7f3b83ce81b9b0951857971b
\ No newline at end of file
+637057b8d527cd9fa66f48c549d47e24
\ No newline at end of file
diff --git a/LayoutTests/platform/chromium-linux/fast/text/atsui-spacing-features-expected.png b/LayoutTests/platform/chromium-linux/fast/text/atsui-spacing-features-expected.png
index 972ba5d..8c7faf3 100644
Binary files a/LayoutTests/platform/chromium-linux/fast/text/atsui-spacing-features-expected.png and b/LayoutTests/platform/chromium-linux/fast/text/atsui-spacing-features-expected.png differ
diff --git a/LayoutTests/platform/chromium-linux/fast/text/atsui-spacing-features-expected.txt b/LayoutTests/platform/chromium-linux/fast/text/atsui-spacing-features-expected.txt
index 8d9f3a6..8909dbb 100644
--- a/LayoutTests/platform/chromium-linux/fast/text/atsui-spacing-features-expected.txt
+++ b/LayoutTests/platform/chromium-linux/fast/text/atsui-spacing-features-expected.txt
@@ -35,14 +35,14 @@ layer at (0,0) size 800x340
             RenderTableCell {TD} at (2,26) size 204x174 [r=1 c=0 rs=1 cs=1]
               RenderBlock {DIV} at (1,1) size 202x172
                 RenderBlock {DIV} at (0,0) size 202x38 [border: (1px solid #0000FF)]
-                  RenderText {#text} at (63,1) size 151x37
-                    text run at (63,1) width 138 RTL: "\x{5D9}\x{5B0}\x{5D4}\x{5B4}\x{5D9}, \x{5D0}\x{5B8}\x{5D7}\x{5B4}\x{5D9}, \x{5DC}\x{5B0}\x{5DA}\x{5B8} \x{5E1}\x{5B5}\x{5E4}\x{5B6}\x{5E8} \x{5E9}\x{5C1}\x{5B0}\x{5DC}\x{5B7}\x{5D7}\x{5B0}\x{5EA}\x{5BC}\x{5B4}\x{5D9}\x{5D5}"
-                    text run at (50,19) width 104 RTL: "\x{5E6}\x{5B0}\x{5DE}\x{5B4}\x{5D9}\x{5EA}\x{5D5}\x{5BC}\x{5EA} \x{5DC}\x{5B8}\x{5DA}\x{5B0} \x{5DE}\x{5B0}\x{5DB}\x{5B7}\x{5E8}\x{5B0}\x{5EA}\x{5BC}\x{5B4}\x{5D9}\x{5D5}."
-                    text run at (154,19) width 47 RTL: "\x{5D5}\x{5BC}\x{5DE}\x{5B4}\x{5DE}\x{5B0}\x{5DB}\x{5BC}\x{5B6}\x{5E8}\x{5B6}\x{5EA} "
+                  RenderText {#text} at (13,1) size 191x37
+                    text run at (13,1) width 188 RTL: "\x{5D9}\x{5B0}\x{5D4}\x{5B4}\x{5D9}, \x{5D0}\x{5B8}\x{5D7}\x{5B4}\x{5D9}, \x{5DC}\x{5B0}\x{5DA}\x{5B8} \x{5E1}\x{5B5}\x{5E4}\x{5B6}\x{5E8} \x{5E9}\x{5C1}\x{5B0}\x{5DC}\x{5B7}\x{5D7}\x{5B0}\x{5EA}\x{5BC}\x{5B4}\x{5D9}\x{5D5}"
+                    text run at (10,19) width 134 RTL: "\x{5E6}\x{5B0}\x{5DE}\x{5B4}\x{5D9}\x{5EA}\x{5D5}\x{5BC}\x{5EA} \x{5DC}\x{5B8}\x{5DA}\x{5B0} \x{5DE}\x{5B0}\x{5DB}\x{5B7}\x{5E8}\x{5B0}\x{5EA}\x{5BC}\x{5B4}\x{5D9}\x{5D5}."
+                    text run at (144,19) width 57 RTL: "\x{5D5}\x{5BC}\x{5DE}\x{5B4}\x{5DE}\x{5B0}\x{5DB}\x{5BC}\x{5B6}\x{5E8}\x{5B6}\x{5EA} "
                 RenderBlock {DIV} at (0,43) size 202x62 [border: (1px solid #0000FF)]
-                  RenderText {#text} at (1,1) size 176x59
-                    text run at (1,1) width 132: "Lore\x{300}m ipsum dolor sit"
-                    text run at (1,21) width 176: "ame\x{300}t, consectetuer adipiscing"
+                  RenderText {#text} at (1,1) size 196x59
+                    text run at (1,1) width 162: "Lore\x{300}m ipsum dolor sit"
+                    text run at (1,21) width 196: "ame\x{300}t, consectetuer adipiscing"
                     text run at (1,41) width 21: "e\x{300}lit."
                 RenderBlock {DIV} at (0,110) size 202x62 [border: (1px solid #008000)]
                   RenderText {#text} at (1,1) size 196x59
diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index f44b472..a86bda7 100644
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,29 @@
+2010-08-16  Adam Langley  <agl at chromium.org>
+
+        Reviewed by Tony Chang.
+
+        [chromium] Improve spacing support for complex text on Linux
+
+        https://bugs.webkit.org/show_bug.cgi?id=39014
+
+        Previously, our complex text support ignored word-spacing,
+        justification and letter-spacing. This fixes the first two issues and
+        allows us to render Scribd's HTML5 documents much better.
+
+        Test: fast/text/atsui-spacing-features.html
+
+        * platform/graphics/chromium/FontLinux.cpp:
+        (WebCore::TextRunWalker::TextRunWalker):
+        (WebCore::TextRunWalker::setWordSpacingAdjustment):
+        (WebCore::TextRunWalker::setLetterSpacingAdjustment):
+        (WebCore::TextRunWalker::setPadding):
+        (WebCore::TextRunWalker::setGlyphXPositions):
+        (WebCore::TextRunWalker::isCodepointSpace):
+        (WebCore::Font::drawComplexText):
+        (WebCore::Font::floatWidthForComplexText):
+        (WebCore::Font::offsetForPositionForComplexText):
+        (WebCore::Font::selectionRectForComplexText):
+
 2010-08-16  Csaba Osztrogonác  <ossy at webkit.org>
 
         Reviewed by Kenneth Rohde Christiansen.
diff --git a/WebCore/platform/graphics/chromium/FontLinux.cpp b/WebCore/platform/graphics/chromium/FontLinux.cpp
index 1953dd8..ec79b82 100644
--- a/WebCore/platform/graphics/chromium/FontLinux.cpp
+++ b/WebCore/platform/graphics/chromium/FontLinux.cpp
@@ -166,6 +166,9 @@ public:
         , m_offsetX(m_startingX)
         , m_run(getTextRun(run))
         , m_iterateBackwards(m_run.rtl())
+        , m_wordSpacingAdjustment(0)
+        , m_padding(0)
+        , m_padError(0)
     {
         // Do not use |run| inside this constructor. Use |m_run| instead.
 
@@ -207,6 +210,56 @@ public:
             delete[] m_item.string;
     }
 
+    // setWordSpacingAdjustment sets a delta (in pixels) which is applied at
+    // each word break in the TextRun.
+    void setWordSpacingAdjustment(int wordSpacingAdjustment)
+    {
+        m_wordSpacingAdjustment = wordSpacingAdjustment;
+    }
+
+    // setLetterSpacingAdjustment sets an additional number of pixels that is
+    // added to the advance after each output cluster. This matches the behaviour
+    // of WidthIterator::advance.
+    //
+    // (NOTE: currently does nothing because I don't know how to get the
+    // cluster information from Harfbuzz.)
+    void setLetterSpacingAdjustment(int letterSpacingAdjustment)
+    {
+        m_letterSpacing = letterSpacingAdjustment;
+    }
+
+    bool isWordBreak(unsigned i, bool isRTL)
+    {
+        if (!isRTL)
+            return i && isCodepointSpace(m_item.string[i]) && !isCodepointSpace(m_item.string[i - 1]);
+        return i != m_item.stringLength - 1 && isCodepointSpace(m_item.string[i]) && !isCodepointSpace(m_item.string[i + 1]);
+    }
+
+    // setPadding sets a number of pixels to be distributed across the TextRun.
+    // WebKit uses this to justify text.
+    void setPadding(int padding)
+    {
+        m_padding = padding;
+        if (!m_padding)
+            return;
+
+        // If we have padding to distribute, then we try to give an equal
+        // amount to each space. The last space gets the smaller amount, if
+        // any.
+        unsigned numWordBreaks = 0;
+        bool isRTL = m_iterateBackwards;
+
+        for (unsigned i = 0; i < m_item.stringLength; i++) {
+            if (isWordBreak(i, isRTL))
+                numWordBreaks++;
+        }
+
+        if (numWordBreaks)
+            m_padPerWordBreak = m_padding / numWordBreaks;
+        else
+            m_padPerWordBreak = 0;
+    }
+
     void reset()
     {
         if (m_iterateBackwards)
@@ -453,8 +506,15 @@ private:
     void setGlyphXPositions(bool isRTL)
     {
         double position = 0;
+        // logClustersIndex indexes logClusters for the first (or last when
+        // RTL) codepoint of the current glyph.  Each time we advance a glyph,
+        // we skip over all the codepoints that contributed to the current
+        // glyph.
+        unsigned logClustersIndex = isRTL ? m_item.num_glyphs - 1 : 0;
+
         for (int iter = 0; iter < m_item.num_glyphs; ++iter) {
-            // Glyphs are stored in logical order, but for layout purposes we always go left to right.
+            // Glyphs are stored in logical order, but for layout purposes we
+            // always go left to right.
             int i = isRTL ? m_item.num_glyphs - iter - 1 : iter;
 
             m_glyphs16[i] = m_item.glyphs[i];
@@ -462,12 +522,48 @@ private:
             m_xPositions[i] = m_offsetX + position + offsetX;
 
             double advance = truncateFixedPointToInteger(m_item.advances[i]);
+            unsigned glyphIndex = m_item.item.pos + logClustersIndex;
+            if (isWordBreak(glyphIndex, isRTL)) {
+                advance += m_wordSpacingAdjustment;
+
+                if (m_padding > 0) {
+                    unsigned toPad = roundf(m_padPerWordBreak + m_padError);
+                    m_padError += m_padPerWordBreak - toPad;
+
+                    if (m_padding < toPad)
+                        toPad = m_padding;
+                    m_padding -= toPad;
+                    advance += toPad;
+                }
+            }
+
+            // We would like to add m_letterSpacing after each cluster, but I
+            // don't know where the cluster information is. This is typically
+            // fine for Roman languages, but breaks more complex languages
+            // terribly.
+            // advance += m_letterSpacing;
+
+            if (isRTL) {
+                while (logClustersIndex > 0 && logClusters()[logClustersIndex] == i)
+                    logClustersIndex--;
+            } else {
+                while (logClustersIndex < m_item.num_glyphs && logClusters()[logClustersIndex] == i)
+                    logClustersIndex++;
+            }
+
             position += advance;
         }
+
         m_pixelWidth = position;
         m_offsetX += m_pixelWidth;
     }
 
+    static bool isCodepointSpace(HB_UChar16 c)
+    {
+        // This matches the logic in RenderBlock::findNextLineBreak
+        return c == ' ' || c == '\t';
+    }
+
     void mirrorCharacters(UChar* destination, const UChar* source, int length) const
     {
         int position = 0;
@@ -498,6 +594,14 @@ private:
     OwnArrayPtr<UChar> m_normalizedBuffer; // A buffer for normalized run.
     const TextRun& m_run;
     bool m_iterateBackwards;
+    int m_wordSpacingAdjustment; // delta adjustment (pixels) for each word break.
+    float m_padding; // pixels to be distributed over the line at word breaks.
+    float m_padPerWordBreak; // pixels to be added to each word break.
+    float m_padError; // |m_padPerWordBreak| might have a fractional component.
+                      // Since we only add a whole number of padding pixels at
+                      // each word break we accumulate error. This is the
+                      // number of pixels that we are behind so far.
+    unsigned m_letterSpacing; // pixels to be added after each glyph.
 };
 
 static void setupForTextPainting(SkPaint* paint, SkColor color)
@@ -534,6 +638,9 @@ void Font::drawComplexText(GraphicsContext* gc, const TextRun& run,
 
     TextRunWalker walker(run, point.x(), this);
     bool haveMultipleLayers = isCanvasMultiLayered(canvas);
+    walker.setWordSpacingAdjustment(wordSpacing());
+    walker.setLetterSpacingAdjustment(letterSpacing());
+    walker.setPadding(run.padding());
 
     while (walker.nextScriptRun()) {
         if (fill) {
@@ -553,6 +660,8 @@ void Font::drawComplexText(GraphicsContext* gc, const TextRun& run,
 float Font::floatWidthForComplexText(const TextRun& run, HashSet<const SimpleFontData*>* /* fallbackFonts */, GlyphOverflow* /* glyphOverflow */) const
 {
     TextRunWalker walker(run, 0, this);
+    walker.setWordSpacingAdjustment(wordSpacing());
+    walker.setLetterSpacingAdjustment(letterSpacing());
     return walker.widthOfFullRun();
 }
 
@@ -588,6 +697,8 @@ int Font::offsetForPositionForComplexText(const TextRun& run, float xFloat,
     // (Mac code ignores includePartialGlyphs, and they don't know what it's
     // supposed to do, so we just ignore it as well.)
     TextRunWalker walker(run, 0, this);
+    walker.setWordSpacingAdjustment(wordSpacing());
+    walker.setLetterSpacingAdjustment(letterSpacing());
 
     // If this is RTL text, the first glyph from the left is actually the last
     // code point. So we need to know how many code points there are total in
@@ -664,6 +775,8 @@ FloatRect Font::selectionRectForComplexText(const TextRun& run,
 {
     int fromX = -1, toX = -1, fromAdvance = -1, toAdvance = -1;
     TextRunWalker walker(run, 0, this);
+    walker.setWordSpacingAdjustment(wordSpacing());
+    walker.setLetterSpacingAdjustment(letterSpacing());
 
     // Base will point to the x offset for the current script run. Note that, in
     // the LTR case, width will be 0.

-- 
WebKit Debian packaging



More information about the Pkg-webkit-commits mailing list