Title: [185175] trunk
Revision
185175
Author
mmaxfi...@apple.com
Date
2015-06-03 15:48:27 -0700 (Wed, 03 Jun 2015)

Log Message

SoftBank Emoji are not transformed by shaping when in a run of their own
https://bugs.webkit.org/show_bug.cgi?id=145569
<rdar://problem/20671711>

Reviewed by Dean Jackson.

Normally, we don't perform shaping on single glyphs. However, these particular codepoints
need to have shaping run on them, even if they are alone.

Test: fast/text/softbank-emoji.html Note that this test may fail in different locales on
different platforms. This patch disables the test on iOS.

* Source/WebCore/platform/graphics/WidthIterator.cpp:
(WebCore::applyFontTransforms): Move the length-of-1 check into shouldApplyFontTransforms()
(WebCore::shouldApplyFontTransforms): Return true if we have one of these emoji characters in
a run of its own
(WebCore::advanceInternal): Keep track of the previous character, and use it to call
shouldApplyFontTransforms.
* Source/WebCore/platform/graphics/WidthIterator.h: shouldApplyFontTransforms() needs
some more information to perform its duties.

Modified Paths

Added Paths

Diff

Added: trunk/LayoutTests/fast/text/softbank-emoji.html (0 => 185175)


--- trunk/LayoutTests/fast/text/softbank-emoji.html	                        (rev 0)
+++ trunk/LayoutTests/fast/text/softbank-emoji.html	2015-06-03 22:48:27 UTC (rev 185175)
@@ -0,0 +1,10 @@
+<!DOCTYPE html>
+<html>
+<head>
+</head>
+<body>
+<div style="font: 100px sans-serif, AppleColorEmoji;">
+<div>&#xE001; a &#xE001;</div>
+<div>&#xE001;</div>
+</body>
+</html>

Modified: trunk/LayoutTests/platform/ios-simulator/TestExpectations (185174 => 185175)


--- trunk/LayoutTests/platform/ios-simulator/TestExpectations	2015-06-03 22:46:35 UTC (rev 185174)
+++ trunk/LayoutTests/platform/ios-simulator/TestExpectations	2015-06-03 22:48:27 UTC (rev 185175)
@@ -2616,3 +2616,7 @@
 
 # Not implemented
 webkit.org/b/145617 compositing/layer-creation/zoomed-clip-intersection.html [ Skip ]
+
+# This test depends on location, and it is impossible to mock the particular mechanism
+# this test uses to determine location.
+fast/text/softbank-emoji.html [ Failure Pass ]

Added: trunk/LayoutTests/platform/ios-simulator/fast/text/softbank-emoji-expected.txt (0 => 185175)


--- trunk/LayoutTests/platform/ios-simulator/fast/text/softbank-emoji-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/platform/ios-simulator/fast/text/softbank-emoji-expected.txt	2015-06-03 22:48:27 UTC (rev 185175)
@@ -0,0 +1,12 @@
+layer at (0,0) size 800x600
+  RenderView at (0,0) size 800x600
+layer at (0,0) size 800x250
+  RenderBlock {HTML} at (0,0) size 800x250
+    RenderBody {BODY} at (8,8) size 784x234
+      RenderBlock {DIV} at (0,0) size 784x234
+        RenderBlock {DIV} at (0,0) size 784x117
+          RenderText {#text} at (0,0) size 154x117
+            text run at (0,0) width 154: "\x{E001} a \x{E001}"
+        RenderBlock {DIV} at (0,117) size 784x117
+          RenderText {#text} at (0,0) size 21x117
+            text run at (0,0) width 21: "\x{E001}"

Added: trunk/LayoutTests/platform/mac/fast/text/softbank-emoji-expected.txt (0 => 185175)


--- trunk/LayoutTests/platform/mac/fast/text/softbank-emoji-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/platform/mac/fast/text/softbank-emoji-expected.txt	2015-06-03 22:48:27 UTC (rev 185175)
@@ -0,0 +1,12 @@
+layer at (0,0) size 800x600
+  RenderView at (0,0) size 800x600
+layer at (0,0) size 800x246
+  RenderBlock {HTML} at (0,0) size 800x246
+    RenderBody {BODY} at (8,8) size 784x230
+      RenderBlock {DIV} at (0,0) size 784x230
+        RenderBlock {DIV} at (0,0) size 784x115
+          RenderText {#text} at (0,0) size 112x115
+            text run at (0,0) width 112: "\x{E001} a \x{E001}"
+        RenderBlock {DIV} at (0,115) size 784x115
+          RenderText {#text} at (0,0) size 0x115
+            text run at (0,0) width 0: "\x{E001}"

Modified: trunk/Source/WebCore/ChangeLog (185174 => 185175)


--- trunk/Source/WebCore/ChangeLog	2015-06-03 22:46:35 UTC (rev 185174)
+++ trunk/Source/WebCore/ChangeLog	2015-06-03 22:48:27 UTC (rev 185175)
@@ -1,3 +1,26 @@
+2015-06-02  Myles C. Maxfield  <mmaxfi...@apple.com>
+
+        SoftBank Emoji are not transformed by shaping when in a run of their own
+        https://bugs.webkit.org/show_bug.cgi?id=145569
+        <rdar://problem/20671711>
+
+        Reviewed by Dean Jackson.
+
+        Normally, we don't perform shaping on single glyphs. However, these particular codepoints
+        need to have shaping run on them, even if they are alone.
+
+        Test: fast/text/softbank-emoji.html Note that this test may fail in different locales on
+        different platforms. This patch disables the test on iOS.
+
+        * Source/WebCore/platform/graphics/WidthIterator.cpp:
+        (WebCore::applyFontTransforms): Move the length-of-1 check into shouldApplyFontTransforms()
+        (WebCore::shouldApplyFontTransforms): Return true if we have one of these emoji characters in
+        a run of its own
+        (WebCore::advanceInternal): Keep track of the previous character, and use it to call
+        shouldApplyFontTransforms. 
+        * Source/WebCore/platform/graphics/WidthIterator.h: shouldApplyFontTransforms() needs
+        some more information to perform its duties.
+
 2015-06-03  Daniel Bates  <daba...@apple.com>
 
         Caps lock indicator should not be shown in read-only or disabled field

Modified: trunk/Source/WebCore/platform/graphics/WidthIterator.cpp (185174 => 185175)


--- trunk/Source/WebCore/platform/graphics/WidthIterator.cpp	2015-06-03 22:46:35 UTC (rev 185174)
+++ trunk/Source/WebCore/platform/graphics/WidthIterator.cpp	2015-06-03 22:48:27 UTC (rev 185175)
@@ -97,7 +97,7 @@
 
 typedef Vector<std::pair<int, OriginalAdvancesForCharacterTreatedAsSpace>, 64> CharactersTreatedAsSpace;
 
-static inline float applyFontTransforms(GlyphBuffer* glyphBuffer, bool ltr, int& lastGlyphCount, const Font* font, WidthIterator& iterator, TypesettingFeatures typesettingFeatures, CharactersTreatedAsSpace& charactersTreatedAsSpace)
+static inline float applyFontTransforms(GlyphBuffer* glyphBuffer, bool ltr, int& lastGlyphCount, const Font* font, WidthIterator& iterator, TypesettingFeatures typesettingFeatures, bool force, CharactersTreatedAsSpace& charactersTreatedAsSpace)
 {
     ASSERT(typesettingFeatures & (Kerning | Ligatures));
 
@@ -105,7 +105,7 @@
         return 0;
 
     int glyphBufferSize = glyphBuffer->size();
-    if (glyphBuffer->size() <= lastGlyphCount + 1) {
+    if (!force && glyphBufferSize <= lastGlyphCount + 1) {
         lastGlyphCount = glyphBufferSize;
         return 0;
     }
@@ -194,6 +194,20 @@
     return std::make_pair(expandLeft, expandRight);
 }
 
+static inline bool isSoftBankEmoji(UChar32 codepoint)
+{
+    return codepoint >= 0xE001 && codepoint <= 0xE537;
+}
+
+inline auto WidthIterator::shouldApplyFontTransforms(const GlyphBuffer* glyphBuffer, int lastGlyphCount, UChar32 previousCharacter) const -> TransformsType
+{
+    if (glyphBuffer && glyphBuffer->size() == lastGlyphCount + 1 && isSoftBankEmoji(previousCharacter))
+        return TransformsType::Forced;
+    if (m_run.length() <= 1 || !(m_typesettingFeatures & (Kerning | Ligatures)))
+        return TransformsType::None;
+    return TransformsType::NotForced;
+}
+
 template <typename TextIterator>
 inline unsigned WidthIterator::advanceInternal(TextIterator& textIterator, GlyphBuffer* glyphBuffer)
 {
@@ -217,6 +231,7 @@
     int lastGlyphCount = glyphBuffer ? glyphBuffer->size() : 0;
 
     UChar32 character = 0;
+    UChar32 previousCharacter = 0;
     unsigned clusterLength = 0;
     CharactersTreatedAsSpace charactersTreatedAsSpace;
     String normalizedSpacesStringCache;
@@ -252,8 +267,9 @@
         }
 
         if (font != lastFontData && width) {
-            if (shouldApplyFontTransforms()) {
-                m_runWidthSoFar += applyFontTransforms(glyphBuffer, m_run.ltr(), lastGlyphCount, lastFontData, *this, m_typesettingFeatures, charactersTreatedAsSpace);
+            auto transformsType = shouldApplyFontTransforms(glyphBuffer, lastGlyphCount, previousCharacter);
+            if (transformsType != TransformsType::None) {
+                m_runWidthSoFar += applyFontTransforms(glyphBuffer, m_run.ltr(), lastGlyphCount, lastFontData, *this, m_typesettingFeatures, transformsType == TransformsType::Forced, charactersTreatedAsSpace);
                 if (glyphBuffer)
                     glyphBuffer->shrink(lastGlyphCount);
             }
@@ -343,7 +359,8 @@
                 m_isAfterExpansion = false;
         }
 
-        if (shouldApplyFontTransforms() && glyphBuffer && FontCascade::treatAsSpace(character)) {
+        auto transformsType = shouldApplyFontTransforms(glyphBuffer, lastGlyphCount, previousCharacter);
+        if (transformsType != TransformsType::None && glyphBuffer && FontCascade::treatAsSpace(character)) {
             charactersTreatedAsSpace.append(std::make_pair(glyphBuffer->size(),
                 OriginalAdvancesForCharacterTreatedAsSpace(character == ' ', glyphBuffer->size() ? glyphBuffer->advanceAt(glyphBuffer->size() - 1).width() : 0, width)));
         }
@@ -398,6 +415,7 @@
             m_minGlyphBoundingBoxY = std::min(m_minGlyphBoundingBoxY, bounds.y());
             m_lastGlyphOverflow = std::max<float>(0, bounds.maxX() - width);
         }
+        previousCharacter = character;
     }
 
     if (leftoverJustificationWidth) {
@@ -407,8 +425,9 @@
             glyphBuffer->add(lastFontData->spaceGlyph(), lastFontData, leftoverJustificationWidth, m_run.length());
     }
 
-    if (shouldApplyFontTransforms()) {
-        m_runWidthSoFar += applyFontTransforms(glyphBuffer, m_run.ltr(), lastGlyphCount, lastFontData, *this, m_typesettingFeatures, charactersTreatedAsSpace);
+    auto transformsType = shouldApplyFontTransforms(glyphBuffer, lastGlyphCount, previousCharacter);
+    if (transformsType != TransformsType::None) {
+        m_runWidthSoFar += applyFontTransforms(glyphBuffer, m_run.ltr(), lastGlyphCount, lastFontData, *this, m_typesettingFeatures, transformsType == TransformsType::Forced, charactersTreatedAsSpace);
         if (glyphBuffer)
             glyphBuffer->shrink(lastGlyphCount);
     }

Modified: trunk/Source/WebCore/platform/graphics/WidthIterator.h (185174 => 185175)


--- trunk/Source/WebCore/platform/graphics/WidthIterator.h	2015-06-03 22:46:35 UTC (rev 185174)
+++ trunk/Source/WebCore/platform/graphics/WidthIterator.h	2015-06-03 22:48:27 UTC (rev 185175)
@@ -26,6 +26,7 @@
 #include "SVGGlyph.h"
 #include "TextRun.h"
 #include <wtf/HashSet.h>
+#include <wtf/Optional.h>
 #include <wtf/Vector.h>
 
 namespace WebCore {
@@ -88,7 +89,8 @@
     template <typename TextIterator>
     inline unsigned advanceInternal(TextIterator&, GlyphBuffer*);
 
-    bool shouldApplyFontTransforms() const { return m_run.length() > 1 && (m_typesettingFeatures & (Kerning | Ligatures)); }
+    enum class TransformsType { None, Forced, NotForced };
+    TransformsType shouldApplyFontTransforms(const GlyphBuffer*, int lastGlyphCount, UChar32 previousCharacter) const;
 
     TypesettingFeatures m_typesettingFeatures;
     HashSet<const Font*>* m_fallbackFonts;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to