[OS X] Upright vertical text is completely broken for multi-code-unit codepoints
authormmaxfield@apple.com <mmaxfield@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 20 Nov 2014 01:35:09 +0000 (01:35 +0000)
committermmaxfield@apple.com <mmaxfield@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 20 Nov 2014 01:35:09 +0000 (01:35 +0000)
https://bugs.webkit.org/show_bug.cgi?id=138891

Reviewed by Dan Bernstein.

Source/WebCore:

We were assuming that we can use the string index (in UTF-16) as the glyph
index. This falls down when a single codepoint (and glyph) contians
multiple code units.

Test: platform/mac/fast/text/multiple-codeunit-vertical-upright.html

* platform/graphics/GlyphPage.h:
* platform/graphics/mac/GlyphPageTreeNodeMac.cpp:
(WebCore::GlyphPage::fill):

LayoutTests:

Make sure a single upright vertical multi-code-unit codepoint is rendered the same as
the same codepoint rendered horizontally.

* platform/mac/fast/text/multiple-codeunit-vertical-upright-expected.html: Added.
* platform/mac/fast/text/multiple-codeunit-vertical-upright.html: Added.
* platform/mac/fast/text/resources/multiple-codeunit-vertical-upright.otf: Added.

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

LayoutTests/ChangeLog
LayoutTests/platform/mac/fast/text/multiple-codeunit-vertical-upright-expected.html [new file with mode: 0644]
LayoutTests/platform/mac/fast/text/multiple-codeunit-vertical-upright.html [new file with mode: 0644]
LayoutTests/platform/mac/fast/text/resources/multiple-codeunit-vertical-upright.otf [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/GlyphPage.h
Source/WebCore/platform/graphics/mac/GlyphPageTreeNodeMac.cpp

index ecde02021ad4c63c35f23bec05419dbd792340f2..00b0b3a2e5833fdf04002cb5e72fb96bb39f09c3 100644 (file)
@@ -1,3 +1,17 @@
+2014-11-19  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        [OS X] Upright vertical text is completely broken for multi-code-unit codepoints
+        https://bugs.webkit.org/show_bug.cgi?id=138891
+
+        Reviewed by Dan Bernstein.
+
+        Make sure a single upright vertical multi-code-unit codepoint is rendered the same as
+        the same codepoint rendered horizontally.
+
+        * platform/mac/fast/text/multiple-codeunit-vertical-upright-expected.html: Added.
+        * platform/mac/fast/text/multiple-codeunit-vertical-upright.html: Added.
+        * platform/mac/fast/text/resources/multiple-codeunit-vertical-upright.otf: Added.
+
 2014-11-16  Sam Weinig  <sam@webkit.org>
 
         Move the 'quotes' CSS property to the new StyleBuilder
diff --git a/LayoutTests/platform/mac/fast/text/multiple-codeunit-vertical-upright-expected.html b/LayoutTests/platform/mac/fast/text/multiple-codeunit-vertical-upright-expected.html
new file mode 100644 (file)
index 0000000..967eb96
--- /dev/null
@@ -0,0 +1,14 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+@font-face {
+    font-family: "Litherum";
+    src: url("resources/multiple-codeunit-vertical-upright.otf") format("opentype");
+}
+</style>
+</head>
+<body style="-webkit-font-smoothing: none;">
+<div style="font: 96px Litherum; position: relative; left: 3.5px; top: 1px;">&#x2000b;</div>
+</body>
+</html>
diff --git a/LayoutTests/platform/mac/fast/text/multiple-codeunit-vertical-upright.html b/LayoutTests/platform/mac/fast/text/multiple-codeunit-vertical-upright.html
new file mode 100644 (file)
index 0000000..3969ab1
--- /dev/null
@@ -0,0 +1,14 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+@font-face {
+    font-family: "Litherum";
+    src: url("resources/multiple-codeunit-vertical-upright.otf") format("opentype");
+}
+</style>
+</head>
+<body style="-webkit-font-smoothing: none;">
+<div style="font: 96px Litherum; -webkit-writing-mode: vertical-rl; -webkit-text-orientation: upright;">&#x2000b;</div>
+</body>
+</html>
diff --git a/LayoutTests/platform/mac/fast/text/resources/multiple-codeunit-vertical-upright.otf b/LayoutTests/platform/mac/fast/text/resources/multiple-codeunit-vertical-upright.otf
new file mode 100644 (file)
index 0000000..df68eac
Binary files /dev/null and b/LayoutTests/platform/mac/fast/text/resources/multiple-codeunit-vertical-upright.otf differ
index 9c699678447db9c4f6993229dd5008a9da10c93e..291072d1880ecaa6e2e1ea119e4803904703177d 100644 (file)
@@ -1,3 +1,20 @@
+2014-11-19  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        [OS X] Upright vertical text is completely broken for multi-code-unit codepoints
+        https://bugs.webkit.org/show_bug.cgi?id=138891
+
+        Reviewed by Dan Bernstein.
+
+        We were assuming that we can use the string index (in UTF-16) as the glyph
+        index. This falls down when a single codepoint (and glyph) contians
+        multiple code units.
+
+        Test: platform/mac/fast/text/multiple-codeunit-vertical-upright.html
+
+        * platform/graphics/GlyphPage.h:
+        * platform/graphics/mac/GlyphPageTreeNodeMac.cpp:
+        (WebCore::GlyphPage::fill):
+
 2014-11-19  Daniel Bates  <dabates@apple.com>
 
         Attempt to fix the Apple Internal Mavericks build after <https://trac.webkit.org/changeset/176347>
index e735e3a2169ee65edbe5962da5e17ca68e69b9a5..52a5e309b3c887913b1cf21f875689ecc7dd3a3c 100644 (file)
@@ -98,6 +98,7 @@ public:
     ~GlyphPage() { }
 
     static const size_t size = 256; // Covers Latin-1 in a single page.
+    static_assert((!(0xD800 % size)) && (!(0xDC00 % size)) && (!(0xE000 % size)), "GlyphPages must never straddle code-unit length boundaries");
     static unsigned indexForCharacter(UChar32 c) { return c % GlyphPage::size; }
 
     ALWAYS_INLINE GlyphData glyphDataForCharacter(UChar32 c) const
index ac531fc75ced5e794c2eac2ce51fa8b1d8f3c986..3e9962b7e30ba8dd9e0dfcf9b016a4e7204bb9bd 100644 (file)
@@ -84,6 +84,7 @@ bool GlyphPage::fill(unsigned offset, unsigned length, UChar* buffer, unsigned b
                : CTFontGetGlyphsForCharacters(fontData->platformData().ctFont(), buffer, glyphs.data(), bufferLength))) {
         // When buffer consists of surrogate pairs, CTFontGetVerticalGlyphsForCharacters and CTFontGetGlyphsForCharacters
         // place the glyphs at indices corresponding to the first character of each pair.
+        ASSERT(!(bufferLength % length) && (bufferLength / length == 1 || bufferLength / length == 2));
         unsigned glyphStep = bufferLength / length;
         for (unsigned i = 0; i < length; ++i) {
             if (!glyphs[i * glyphStep])
@@ -141,14 +142,18 @@ bool GlyphPage::fill(unsigned offset, unsigned length, UChar* buffer, unsigned b
                     stringIndices = indexVector.data();
                 }
 
+                // When buffer consists of surrogate pairs, CTRunGetStringIndicesPtr and CTRunGetStringIndices
+                // place the glyphs at indices corresponding to the first character of each pair.
+                ASSERT(!(bufferLength % length) && (bufferLength / length == 1 || bufferLength / length == 2));
+                unsigned glyphStep = bufferLength / length;
                 if (gotBaseFont) {
                     for (CFIndex i = 0; i < glyphCount; ++i) {
-                        if (stringIndices[i] >= static_cast<CFIndex>(length)) {
+                        if (stringIndices[i] >= static_cast<CFIndex>(bufferLength)) {
                             done = true;
                             break;
                         }
                         if (glyphs[i]) {
-                            setGlyphDataForIndex(offset + stringIndices[i], glyphs[i], fontData);
+                            setGlyphDataForIndex(offset + (stringIndices[i] / glyphStep), glyphs[i], fontData);
                             haveGlyphs = true;
                         }
                     }
@@ -157,12 +162,12 @@ bool GlyphPage::fill(unsigned offset, unsigned length, UChar* buffer, unsigned b
                     const SimpleFontData* runSimple = fontData->getCompositeFontReferenceFontData((NSFont *)runFont);
                     if (runSimple) {
                         for (CFIndex i = 0; i < glyphCount; ++i) {
-                            if (stringIndices[i] >= static_cast<CFIndex>(length)) {
+                            if (stringIndices[i] >= static_cast<CFIndex>(bufferLength)) {
                                 done = true;
                                 break;
                             }
                             if (glyphs[i]) {
-                                setGlyphDataForIndex(offset + stringIndices[i], glyphs[i], runSimple);
+                                setGlyphDataForIndex(offset + (stringIndices[i] / glyphStep), glyphs[i], runSimple);
                                 haveGlyphs = true;
                             }
                         }