Changeset 54020 in webkit

Timestamp:

Jan 28, 2010 2:17:23 PM (14 years ago)

Author:

evan@chromium.org

Message:

2010-01-27 Evan Martin <​evan@chromium.org>

Reviewed by David Levin.

[chromium] hebrew vowel marks incorrectly positioned
​https://bugs.webkit.org/show_bug.cgi?id=34234

Add a layout test containing some Hebrew vowels.

• fast/text/international/hebrew-vowels.html: Added.

2010-01-27 Evan Martin <​evan@chromium.org>

Reviewed by David Levin.

[chromium] hebrew vowel marks incorrectly positioned
​https://bugs.webkit.org/show_bug.cgi?id=34234

Fix glyph metrics computation such that the glyph offsets
are correct, so we can now make use of them.

Test: fast/text/international/hebrew-vowels.html

• platform/graphics/chromium/FontLinux.cpp: (WebCore::TextRunWalker::createGlyphArrays): initialize per-glyph offset table. (WebCore::TextRunWalker::setGlyphXPositions): use per-glyph offsets.
• platform/graphics/chromium/HarfbuzzSkia.cpp: (WebCore::getGlyphMetrics): fix metrics computation such that per-glyph offsets are computed correctly.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/text/international/hebrew-vowels.html (added)
• LayoutTests/platform/mac/fast/text/international/hebrew-vowels-expected.checksum (added)
• LayoutTests/platform/mac/fast/text/international/hebrew-vowels-expected.png (added)
• LayoutTests/platform/mac/fast/text/international/hebrew-vowels-expected.txt (added)
• WebCore/ChangeLog (modified) (1 diff)
• WebCore/platform/graphics/chromium/FontLinux.cpp (modified) (2 diffs)
• WebCore/platform/graphics/chromium/HarfbuzzSkia.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2010-01-27  Evan Martin  <evan@chromium.org>
+
+        Reviewed by David Levin.
+
+        [chromium] hebrew vowel marks incorrectly positioned
+        https://bugs.webkit.org/show_bug.cgi?id=34234
+
+        Add a layout test containing some Hebrew vowels.
+
+        * fast/text/international/hebrew-vowels.html: Added.
+
 2010-01-28  Dimitri Glazkov  <dglazkov@chromium.org>
 

trunk/WebCore/ChangeLog

@@ +1 @@
+2010-01-27  Evan Martin  <evan@chromium.org>
+
+        Reviewed by David Levin.
+
+        [chromium] hebrew vowel marks incorrectly positioned
+        https://bugs.webkit.org/show_bug.cgi?id=34234
+
+        Fix glyph metrics computation such that the glyph offsets
+        are correct, so we can now make use of them.
+
+        Test: fast/text/international/hebrew-vowels.html
+
+        * platform/graphics/chromium/FontLinux.cpp:
+        (WebCore::TextRunWalker::createGlyphArrays): initialize per-glyph
+        offset table.
+        (WebCore::TextRunWalker::setGlyphXPositions): use per-glyph
+        offsets.
+        * platform/graphics/chromium/HarfbuzzSkia.cpp:
+        (WebCore::getGlyphMetrics): fix metrics computation such that
+        per-glyph offsets are computed correctly.
+
 2010-01-28  Laszlo Gombos  <laszlo.1.gombos@nokia.com>
 

trunk/WebCore/platform/graphics/chromium/FontLinux.cpp

@@ -401 +401 @@
         m_item.advances = new HB_Fixed[m_maxGlyphs];
         m_item.offsets = new HB_FixedPoint[m_maxGlyphs];
+        // HB_FixedPoint is a struct, so we must use memset to clear it.
+        memset(m_item.offsets, 0, m_maxGlyphs * sizeof(HB_FixedPoint));
         m_glyphs16 = new uint16_t[m_maxGlyphs];
         m_xPositions = new SkScalar[m_maxGlyphs];
@@ -437 +439 @@
     void setGlyphXPositions(bool isRTL)
     {
-        m_pixelWidth = 0;
-        for (unsigned i = 0; i < m_item.num_glyphs; ++i) {
-            int index;
-            if (isRTL)
-                index = m_item.num_glyphs - (i + 1);
-            else
-                index = i;
+        double position = 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.
+            int i = isRTL ? m_item.num_glyphs - iter - 1 : iter;
 
             m_glyphs16[i] = m_item.glyphs[i];
-            m_xPositions[index] = m_offsetX + m_pixelWidth;
-            m_pixelWidth += truncateFixedPointToInteger(m_item.advances[index]);
-        }
+            double offsetX = truncateFixedPointToInteger(m_item.offsets[i].x);
+            m_xPositions[i] = m_offsetX + position + offsetX;
+
+            double advance = truncateFixedPointToInteger(m_item.advances[i]);
+            position += advance;
+        }
+        m_pixelWidth = position;
         m_offsetX += m_pixelWidth;
     }

trunk/WebCore/platform/graphics/chromium/HarfbuzzSkia.cpp

@@ -168 +168 @@
     paint.getTextWidths(&glyph16, sizeof(glyph16), &width, &bounds);
 
-    metrics->x = SkiaScalarToHarfbuzzFixed(width);
+    metrics->x = SkiaScalarToHarfbuzzFixed(bounds.fLeft);
+    metrics->y = SkiaScalarToHarfbuzzFixed(bounds.fTop);
+    metrics->width = SkiaScalarToHarfbuzzFixed(bounds.width());
+    metrics->height = SkiaScalarToHarfbuzzFixed(bounds.height());
+
+    metrics->xOffset = SkiaScalarToHarfbuzzFixed(width);
     // We can't actually get the |y| correct because Skia doesn't export
     // the vertical advance. However, nor we do ever render vertical text at
     // the moment so it's unimportant.
-    metrics->y = 0;
-    metrics->width = SkiaScalarToHarfbuzzFixed(bounds.width());
-    metrics->height = SkiaScalarToHarfbuzzFixed(bounds.height());
-    metrics->xOffset = SkiaScalarToHarfbuzzFixed(bounds.fLeft);
-    metrics->yOffset = SkiaScalarToHarfbuzzFixed(bounds.fTop);
+    metrics->yOffset = 0;
 }