2011-02-02 Evan Martin <evan@chromium.org>
authorevan@chromium.org <evan@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 3 Feb 2011 17:49:44 +0000 (17:49 +0000)
committerevan@chromium.org <evan@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 3 Feb 2011 17:49:44 +0000 (17:49 +0000)
        Reviewed by Tony Chang.

        [chromium] complex joining characters positioned in wrong place
        https://bugs.webkit.org/show_bug.cgi?id=53637

        Add a test that includes some characters that exhibited the problem.
        Unfortunately it's purely a rendering issue, so it is a pixel test.

        * platform/chromium-linux/fast/text/international/complex-joining-using-gpos-expected.checksum: Added.
        * platform/chromium-linux/fast/text/international/complex-joining-using-gpos-expected.png: Added.
        * platform/chromium-linux/fast/text/international/complex-joining-using-gpos-expected.txt: Added.
        * platform/chromium-linux/fast/text/international/complex-joining-using-gpos.html: Added.
2011-02-02  Evan Martin  <evan@chromium.org>

        Reviewed by Tony Chang.

        [chromium] complex joining characters positioned in wrong place
        https://bugs.webkit.org/show_bug.cgi?id=53637

        Provide the correct font metrics to Harfbuzz related to the font design space.
        There are used in some fonts for GPOS positioning.

        Test: platform/chromium-linux/fast/text/international/complex-joining-using-gpos.html

        * platform/graphics/chromium/ComplexTextControllerLinux.cpp:
        (WebCore::ComplexTextController::setupFontForScriptRun):
        (WebCore::ComplexTextController::allocHarfbuzzFont):
        * platform/graphics/chromium/FontPlatformDataLinux.cpp:
        (WebCore::FontPlatformData::FontPlatformData):
        (WebCore::FontPlatformData::emSizeInFontUnits):
        (WebCore::FontPlatformData::operator=):
        * platform/graphics/chromium/FontPlatformDataLinux.h:
        (WebCore::FontPlatformData::FontPlatformData):

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@77504 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/platform/chromium-linux/fast/text/international/complex-joining-using-gpos-expected.checksum [new file with mode: 0644]
LayoutTests/platform/chromium-linux/fast/text/international/complex-joining-using-gpos-expected.png [new file with mode: 0644]
LayoutTests/platform/chromium-linux/fast/text/international/complex-joining-using-gpos-expected.txt [new file with mode: 0644]
LayoutTests/platform/chromium-linux/fast/text/international/complex-joining-using-gpos.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/chromium/ComplexTextControllerLinux.cpp
Source/WebCore/platform/graphics/chromium/FontPlatformDataLinux.cpp
Source/WebCore/platform/graphics/chromium/FontPlatformDataLinux.h

index 9b852b4a4927b0ee7031d758f329ad9b11eb229e..23f54e90b8650801661deceb5a11f4e1ff195801 100644 (file)
@@ -1,3 +1,18 @@
+2011-02-02  Evan Martin  <evan@chromium.org>
+
+        Reviewed by Tony Chang.
+
+        [chromium] complex joining characters positioned in wrong place
+        https://bugs.webkit.org/show_bug.cgi?id=53637
+
+        Add a test that includes some characters that exhibited the problem.
+        Unfortunately it's purely a rendering issue, so it is a pixel test.
+
+        * platform/chromium-linux/fast/text/international/complex-joining-using-gpos-expected.checksum: Added.
+        * platform/chromium-linux/fast/text/international/complex-joining-using-gpos-expected.png: Added.
+        * platform/chromium-linux/fast/text/international/complex-joining-using-gpos-expected.txt: Added.
+        * platform/chromium-linux/fast/text/international/complex-joining-using-gpos.html: Added.
+
 2011-02-03  Ryosuke Niwa  <rniwa@webkit.org>
 
         Unreviewed; remove the line I intended to remove.
diff --git a/LayoutTests/platform/chromium-linux/fast/text/international/complex-joining-using-gpos-expected.checksum b/LayoutTests/platform/chromium-linux/fast/text/international/complex-joining-using-gpos-expected.checksum
new file mode 100644 (file)
index 0000000..8f01378
--- /dev/null
@@ -0,0 +1 @@
+7c22145ac1cd3b77fee1e3499be82786
\ No newline at end of file
diff --git a/LayoutTests/platform/chromium-linux/fast/text/international/complex-joining-using-gpos-expected.png b/LayoutTests/platform/chromium-linux/fast/text/international/complex-joining-using-gpos-expected.png
new file mode 100644 (file)
index 0000000..e120743
Binary files /dev/null and b/LayoutTests/platform/chromium-linux/fast/text/international/complex-joining-using-gpos-expected.png differ
diff --git a/LayoutTests/platform/chromium-linux/fast/text/international/complex-joining-using-gpos-expected.txt b/LayoutTests/platform/chromium-linux/fast/text/international/complex-joining-using-gpos-expected.txt
new file mode 100644 (file)
index 0000000..147eeef
--- /dev/null
@@ -0,0 +1,28 @@
+layer at (0,0) size 800x600
+  RenderView at (0,0) size 800x600
+layer at (0,0) size 800x600
+  RenderBlock {HTML} at (0,0) size 800x600
+    RenderBody {BODY} at (8,8) size 784x584
+      RenderBlock {P} at (0,0) size 784x20
+        RenderText {#text} at (0,0) size 417x19
+          text run at (0,0) width 417: "This pixel test verifies that we position combining characters properly."
+      RenderBlock {P} at (0,36) size 784x20
+        RenderText {#text} at (0,0) size 668x19
+          text run at (0,0) width 668: "The backslash-looking mark should connect into the the character on the right, not be positioned off to the side."
+      RenderBlock {DIV} at (0,72) size 784x83
+        RenderText {#text} at (0,1) size 249x80
+          text run at (0,1) width 249: "\x{915}+\x{947} = \x{915}\x{947}"
+      RenderBlock {DIV} at (0,155) size 784x35
+        RenderText {#text} at (0,0) size 107x34
+          text run at (0,0) width 107: "\x{915}+\x{947} = \x{915}\x{947}"
+      RenderBlock {P} at (0,206) size 784x20
+        RenderText {#text} at (0,0) size 586x19
+          text run at (0,0) width 586: "The three words should be separated by spaces, and there should be no marks above the spaces."
+      RenderBlock {DIV} at (0,242) size 784x83
+        RenderText {#text} at (0,1) size 779x80
+          text run at (0,1) width 427: "\x{E40}\x{E01}\x{E21}\x{E2A}\x{E4C} \x{E1F}\x{E31}\x{E07}\x{E40}\x{E1E}\x{E25}\x{E07} "
+          text run at (427,1) width 352: "\x{E42}\x{E1B}\x{E23}\x{E42}\x{E21}\x{E17}\x{E40}\x{E27}\x{E47}\x{E1A}"
+      RenderBlock {DIV} at (0,325) size 784x35
+        RenderText {#text} at (0,0) size 335x34
+          text run at (0,0) width 183: "\x{E40}\x{E01}\x{E21}\x{E2A}\x{E4C} \x{E1F}\x{E31}\x{E07}\x{E40}\x{E1E}\x{E25}\x{E07} "
+          text run at (183,0) width 152: "\x{E42}\x{E1B}\x{E23}\x{E42}\x{E21}\x{E17}\x{E40}\x{E27}\x{E47}\x{E1A}"
diff --git a/LayoutTests/platform/chromium-linux/fast/text/international/complex-joining-using-gpos.html b/LayoutTests/platform/chromium-linux/fast/text/international/complex-joining-using-gpos.html
new file mode 100644 (file)
index 0000000..36e98d0
--- /dev/null
@@ -0,0 +1,17 @@
+<meta charset=utf-8>
+
+<style>
+.big { font-size: 70px }
+.med { font-size: 30px }
+</style>
+
+<p>This pixel test verifies that we position combining characters properly.</p>
+
+<p>The backslash-looking mark should connect into the the character on the right, not be positioned off to the side.</p>
+<div class=big>क+े = के</div>
+<div class=med>क+े = के</div>
+
+<p>The three words should be separated by spaces, and there should be no
+marks above the spaces.</p>
+<div class=big>เกมส์ ฟังเพลง  โปรโมทเว็บ</div>
+<div class=med>เกมส์ ฟังเพลง  โปรโมทเว็บ</div>
index 88afcc03596d75d350e72565acacc9d6fce146c9..0e13a97176c02c6608fdcd3c128198a3937df631 100644 (file)
@@ -1,3 +1,25 @@
+2011-02-02  Evan Martin  <evan@chromium.org>
+
+        Reviewed by Tony Chang.
+
+        [chromium] complex joining characters positioned in wrong place
+        https://bugs.webkit.org/show_bug.cgi?id=53637
+
+        Provide the correct font metrics to Harfbuzz related to the font design space.
+        There are used in some fonts for GPOS positioning.
+
+        Test: platform/chromium-linux/fast/text/international/complex-joining-using-gpos.html
+
+        * platform/graphics/chromium/ComplexTextControllerLinux.cpp:
+        (WebCore::ComplexTextController::setupFontForScriptRun):
+        (WebCore::ComplexTextController::allocHarfbuzzFont):
+        * platform/graphics/chromium/FontPlatformDataLinux.cpp:
+        (WebCore::FontPlatformData::FontPlatformData):
+        (WebCore::FontPlatformData::emSizeInFontUnits):
+        (WebCore::FontPlatformData::operator=):
+        * platform/graphics/chromium/FontPlatformDataLinux.h:
+        (WebCore::FontPlatformData::FontPlatformData):
+
 2011-02-02  Dimitri Glazkov  <dglazkov@chromium.org>
 
         Reviewed by Kent Tamura.
index 0566f0eb5ec35a40ffa1775c07d8f5b8df67271b..7f6a6d04516c27570902477db8b7de9e9cbbdc11 100644 (file)
@@ -204,6 +204,16 @@ void ComplexTextController::setupFontForScriptRun()
     m_item.face = platformData.harfbuzzFace();
     void* opaquePlatformData = const_cast<FontPlatformData*>(&platformData);
     m_item.font->userData = opaquePlatformData;
+
+    int size = platformData.size();
+    m_item.font->x_ppem = size;
+    m_item.font->y_ppem = size;
+    // x_ and y_scale are the conversion factors from font design space (fEmSize) to 1/64th of device pixels in 16.16 format.
+    const int devicePixelFraction = 64;
+    const int multiplyFor16Dot16 = 1 << 16;
+    int scale = devicePixelFraction * size * multiplyFor16Dot16 / platformData.emSizeInFontUnits();
+    m_item.font->x_scale = scale;
+    m_item.font->y_scale = scale;
 }
 
 HB_FontRec* ComplexTextController::allocHarfbuzzFont()
@@ -212,13 +222,6 @@ HB_FontRec* ComplexTextController::allocHarfbuzzFont()
     memset(font, 0, sizeof(HB_FontRec));
     font->klass = &harfbuzzSkiaClass;
     font->userData = 0;
-    // The values which harfbuzzSkiaClass returns are already scaled to
-    // pixel units, so we just set all these to one to disable further
-    // scaling.
-    font->x_ppem = 1;
-    font->y_ppem = 1;
-    font->x_scale = 1;
-    font->y_scale = 1;
 
     return font;
 }
index a1ea012eaeb54684c00374fb05e6bf1749fe3fa6..6f9009fc2ab82e6f24effc86234b93a1a9505825 100644 (file)
@@ -36,6 +36,7 @@
 #include "PlatformBridge.h"
 #include "PlatformString.h"
 
+#include "SkAdvancedTypefaceMetrics.h"
 #include "SkPaint.h"
 #include "SkTypeface.h"
 
@@ -71,6 +72,7 @@ FontPlatformData::FontPlatformData(const FontPlatformData& src)
     : m_typeface(src.m_typeface)
     , m_family(src.m_family)
     , m_textSize(src.m_textSize)
+    , m_emSizeInFontUnits(src.m_emSizeInFontUnits)
     , m_fakeBold(src.m_fakeBold)
     , m_fakeItalic(src.m_fakeItalic)
     , m_orientation(src.m_orientation)
@@ -84,6 +86,7 @@ FontPlatformData::FontPlatformData(SkTypeface* tf, const char* family, float tex
     : m_typeface(tf)
     , m_family(family)
     , m_textSize(textSize)
+    , m_emSizeInFontUnits(0)
     , m_fakeBold(fakeBold)
     , m_fakeItalic(fakeItalic)
     , m_orientation(orientation)
@@ -96,6 +99,7 @@ FontPlatformData::FontPlatformData(const FontPlatformData& src, float textSize)
     : m_typeface(src.m_typeface)
     , m_family(src.m_family)
     , m_textSize(textSize)
+    , m_emSizeInFontUnits(src.m_emSizeInFontUnits)
     , m_fakeBold(src.m_fakeBold)
     , m_fakeItalic(src.m_fakeItalic)
     , m_harfbuzzFace(src.m_harfbuzzFace)
@@ -109,6 +113,17 @@ FontPlatformData::~FontPlatformData()
     SkSafeUnref(m_typeface);
 }
 
+int FontPlatformData::emSizeInFontUnits() const
+{
+    if (m_emSizeInFontUnits)
+        return m_emSizeInFontUnits;
+
+    SkAdvancedTypefaceMetrics* metrics = m_typeface->getAdvancedTypefaceMetrics(false);
+    m_emSizeInFontUnits = metrics->fEmSize;
+    metrics->unref();
+    return m_emSizeInFontUnits;
+}
+
 FontPlatformData& FontPlatformData::operator=(const FontPlatformData& src)
 {
     SkRefCnt_SafeAssign(m_typeface, src.m_typeface);
@@ -120,6 +135,7 @@ FontPlatformData& FontPlatformData::operator=(const FontPlatformData& src)
     m_harfbuzzFace = src.m_harfbuzzFace;
     m_orientation = src.m_orientation;
     m_style = src.m_style;
+    m_emSizeInFontUnits = src.m_emSizeInFontUnits;
 
     return *this;
 }
index 43771d740d20dcac79714aa745970791bc5be69d..d9ebb61fa1798ea8df33451e6f33345c19516690 100644 (file)
@@ -63,6 +63,7 @@ public:
     FontPlatformData(WTF::HashTableDeletedValueType)
         : m_typeface(hashTableDeletedFontValue())
         , m_textSize(0)
+        , m_emSizeInFontUnits(0)
         , m_fakeBold(false)
         , m_fakeItalic(false)
         { }
@@ -70,6 +71,7 @@ public:
     FontPlatformData()
         : m_typeface(0)
         , m_textSize(0)
+        , m_emSizeInFontUnits(0)
         , m_fakeBold(false)
         , m_fakeItalic(false)
         , m_orientation(Horizontal)
@@ -78,6 +80,7 @@ public:
     FontPlatformData(float textSize, bool fakeBold, bool fakeItalic)
         : m_typeface(0)
         , m_textSize(textSize)
+        , m_emSizeInFontUnits(0)
         , m_fakeBold(fakeBold)
         , m_fakeItalic(fakeItalic)
         , m_orientation(Horizontal)
@@ -107,6 +110,7 @@ public:
 
     unsigned hash() const;
     float size() const { return m_textSize; }
+    int emSizeInFontUnits() const;
 
     FontOrientation orientation() const { return m_orientation; }
 
@@ -153,6 +157,7 @@ private:
     SkTypeface* m_typeface;
     CString m_family;
     float m_textSize;
+    mutable int m_emSizeInFontUnits;
     bool m_fakeBold;
     bool m_fakeItalic;
     FontOrientation m_orientation;