[FreeType] Vertical CJK glyphs should not be rendered with synthetic oblique
authormrobinson@webkit.org <mrobinson@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 6 May 2015 17:52:43 +0000 (17:52 +0000)
committermrobinson@webkit.org <mrobinson@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 6 May 2015 17:52:43 +0000 (17:52 +0000)
https://bugs.webkit.org/show_bug.cgi?id=144612

Reviewed by Darin Adler.

Source/WebCore:

No new tests. This causes fast/text/international/synthesized-italic-vertical.html to pass.

* platform/graphics/Font.cpp:
(WebCore::Font::nonSyntheticItalicFont): Compile this method for Cairo as well.
* platform/graphics/FontCascadeFonts.cpp:
(WebCore::FontCascadeFonts::glyphDataForSystemFallback): When searching for the system fallback,
ensure that we do not use synthetic oblique when rendering vertical CJK glyphs.
(WebCore::FontCascadeFonts::glyphDataForNormalVariant): Extend the CJK fix to Cairo ports.
* platform/graphics/FontPlatformData.h:
(WebCore::FontPlatformData::setSyntheticOblique): Added this helper method, because
Freetype/Cairo ports needs to be able to recreate the scaled font matrix when the
synthetic oblique settings changes.
* platform/graphics/freetype/FontPlatformData.h: Rename initializeWithFontFace to buildScaledFont.
Remove the now unused m_horizontalOrientationMatrix member.
(WebCore::FontPlatformData::setSyntheticOblique): Added the helper here as well.
* platform/graphics/freetype/FontPlatformDataFreeType.cpp:
(WebCore::FontPlatformData::FontPlatformData): Calculate whether or not to use synthetic oblique here,
before buildScaledFont is called. Call buildScaledFont instead of initializeWithFontFace.
(WebCore::FontPlatformData::operator=):
(WebCore::FontPlatformData::buildScaledFont): Renamed from initializeWithFontFace, does
the same thing, except calculate whether or not to use synthetic oblique. Instead just
reads the value.
(WebCore::FontPlatformData::setOrientation): Instead of adjusting the font matrix, recreate
the entire font.
(WebCore::FontPlatformData::setSyntheticOblique): Added.
(WebCore::rotateCairoMatrixForVerticalOrientation): Deleted.
(WebCore::FontPlatformData::initializeWithFontFace): Deleted.

LayoutTests:

* platform/gtk/TestExpectations: Remove expectation for failing test.

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

LayoutTests/ChangeLog
LayoutTests/platform/gtk/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/Font.cpp
Source/WebCore/platform/graphics/FontCascadeFonts.cpp
Source/WebCore/platform/graphics/FontPlatformData.h
Source/WebCore/platform/graphics/freetype/FontPlatformData.h
Source/WebCore/platform/graphics/freetype/FontPlatformDataFreeType.cpp

index e62e65a..0c9ebc7 100644 (file)
@@ -1,5 +1,14 @@
 2015-05-06  Martin Robinson  <mrobinson@igalia.com>
 
+        [FreeType] Vertical CJK glyphs should not be rendered with synthetic oblique
+        https://bugs.webkit.org/show_bug.cgi?id=144612
+
+        Reviewed by Darin Adler.
+
+        * platform/gtk/TestExpectations: Remove expectation for failing test.
+
+2015-05-06  Martin Robinson  <mrobinson@igalia.com>
+
         Unreviewed GTK+ gardening
 
         * platform/gtk/TestExpectations: Remove a couple expectations tests.
index bcd4434..ec4f5dd 100644 (file)
@@ -2363,8 +2363,6 @@ webkit.org/b/144494 printing/quirks-percentage-height-body.html [ ImageOnlyFailu
 webkit.org/b/144494 printing/quirks-percentage-height.html [ ImageOnlyFailure ]
 webkit.org/b/144494 printing/standards-percentage-heights.html [ ImageOnlyFailure ]
 
-webkit.org/b/144573 fast/text/international/synthesized-italic-vertical.html [ ImageOnlyFailure ]
-
 webkit.org/b/144574 imported/mozilla/svg/objectBoundingBox-and-mask.svg [ ImageOnlyFailure ]
 webkit.org/b/144574 imported/mozilla/svg/text/mask-applied.svg [ ImageOnlyFailure ]
 webkit.org/b/144574 svg/masking/mask-negative-scale.svg [ ImageOnlyFailure ]
index 8361c05..a82aeee 100644 (file)
@@ -1,3 +1,38 @@
+2015-05-06  Martin Robinson  <mrobinson@igalia.com>
+
+        [FreeType] Vertical CJK glyphs should not be rendered with synthetic oblique
+        https://bugs.webkit.org/show_bug.cgi?id=144612
+
+        Reviewed by Darin Adler.
+
+        No new tests. This causes fast/text/international/synthesized-italic-vertical.html to pass.
+
+        * platform/graphics/Font.cpp:
+        (WebCore::Font::nonSyntheticItalicFont): Compile this method for Cairo as well.
+        * platform/graphics/FontCascadeFonts.cpp:
+        (WebCore::FontCascadeFonts::glyphDataForSystemFallback): When searching for the system fallback,
+        ensure that we do not use synthetic oblique when rendering vertical CJK glyphs.
+        (WebCore::FontCascadeFonts::glyphDataForNormalVariant): Extend the CJK fix to Cairo ports.
+        * platform/graphics/FontPlatformData.h:
+        (WebCore::FontPlatformData::setSyntheticOblique): Added this helper method, because
+        Freetype/Cairo ports needs to be able to recreate the scaled font matrix when the
+        synthetic oblique settings changes.
+        * platform/graphics/freetype/FontPlatformData.h: Rename initializeWithFontFace to buildScaledFont.
+        Remove the now unused m_horizontalOrientationMatrix member.
+        (WebCore::FontPlatformData::setSyntheticOblique): Added the helper here as well.
+        * platform/graphics/freetype/FontPlatformDataFreeType.cpp:
+        (WebCore::FontPlatformData::FontPlatformData): Calculate whether or not to use synthetic oblique here,
+        before buildScaledFont is called. Call buildScaledFont instead of initializeWithFontFace.
+        (WebCore::FontPlatformData::operator=):
+        (WebCore::FontPlatformData::buildScaledFont): Renamed from initializeWithFontFace, does
+        the same thing, except calculate whether or not to use synthetic oblique. Instead just
+        reads the value.
+        (WebCore::FontPlatformData::setOrientation): Instead of adjusting the font matrix, recreate
+        the entire font.
+        (WebCore::FontPlatformData::setSyntheticOblique): Added.
+        (WebCore::rotateCairoMatrixForVerticalOrientation): Deleted.
+        (WebCore::FontPlatformData::initializeWithFontFace): Deleted.
+
 2015-05-06  Alex Christensen  <achristensen@webkit.org>
 
         [Content Extensions] Test splitting NFAs by max NFA size.
index 4a477fd..603eba8 100644 (file)
@@ -335,8 +335,8 @@ PassRefPtr<Font> Font::nonSyntheticItalicFont() const
         m_derivedFontData = std::make_unique<DerivedFontData>(isCustomFont());
     if (!m_derivedFontData->nonSyntheticItalic) {
         FontPlatformData nonSyntheticItalicFontPlatformData(m_platformData);
-#if PLATFORM(COCOA)
-        nonSyntheticItalicFontPlatformData.m_syntheticOblique = false;
+#if PLATFORM(COCOA) || USE(CAIRO)
+        nonSyntheticItalicFontPlatformData.setSyntheticOblique(false);
 #endif
         m_derivedFontData->nonSyntheticItalic = create(nonSyntheticItalicFontPlatformData, isCustomFont());
     }
index dd3be1a..40e8c2c 100644 (file)
@@ -218,7 +218,7 @@ static bool shouldIgnoreRotation(UChar32 character)
     return false;
 }
 
-#if PLATFORM(COCOA)
+#if PLATFORM(COCOA) || USE(CAIRO)
 static GlyphData glyphDataForCJKCharacterWithoutSyntheticItalic(UChar32 character, GlyphData& data)
 {
     GlyphData nonItalicData = data.font->nonSyntheticItalicFont()->glyphDataForCharacter(character);
@@ -273,9 +273,13 @@ GlyphData FontCascadeFonts::glyphDataForSystemFallback(UChar32 c, const FontDesc
     else
         fallbackGlyphData = systemFallbackFont->variantFont(description, variant)->glyphDataForCharacter(c);
 
-    if (variant == NormalVariant && fallbackGlyphData.font) {
-        if (!FontCascade::isCJKIdeographOrSymbol(c) && fallbackGlyphData.font->platformData().orientation() == Vertical && !fallbackGlyphData.font->isTextOrientationFallback())
+    if (fallbackGlyphData.font && fallbackGlyphData.font->platformData().orientation() == Vertical && !fallbackGlyphData.font->isTextOrientationFallback()) {
+        if (variant == NormalVariant && !FontCascade::isCJKIdeographOrSymbol(c))
             fallbackGlyphData = glyphDataForNonCJKCharacterWithGlyphOrientation(c, description.nonCJKGlyphOrientation(), fallbackGlyphData);
+#if PLATFORM(COCOA) || USE(CAIRO)
+        if (fallbackGlyphData.font->platformData().syntheticOblique() && FontCascade::isCJKIdeographOrSymbol(c))
+            fallbackGlyphData = glyphDataForCJKCharacterWithoutSyntheticItalic(c, fallbackGlyphData);
+#endif
     }
 
     // Keep the system fallback fonts we use alive.
@@ -330,7 +334,7 @@ GlyphData FontCascadeFonts::glyphDataForNormalVariant(UChar32 c, const FontDescr
                     // to make sure you get a square (even for broken glyphs like symbols used for punctuation).
                     return glyphDataForVariant(c, description, BrokenIdeographVariant, fallbackIndex);
                 }
-#if PLATFORM(COCOA)
+#if PLATFORM(COCOA) || USE(CAIRO)
                 if (data.font->platformData().syntheticOblique())
                     return glyphDataForCJKCharacterWithoutSyntheticItalic(c, data);
 #endif
index 8acc596..2795feb 100644 (file)
@@ -139,6 +139,7 @@ public:
     FontWidthVariant widthVariant() const { return m_widthVariant; }
 
     void setOrientation(FontOrientation orientation) { m_orientation = orientation; }
+    void setSyntheticOblique(bool syntheticOblique) { m_syntheticOblique = syntheticOblique; }
 
 #if USE(CAIRO)
     cairo_scaled_font_t* scaledFont() const { return m_scaledFont; }
index 290ccae..393d594 100644 (file)
@@ -77,6 +77,7 @@ public:
     void setSize(float size) { m_size = size; }
     bool syntheticBold() const { return m_syntheticBold; }
     bool syntheticOblique() const { return m_syntheticOblique; }
+    void setSyntheticOblique(bool);
     bool hasCompatibleCharmap();
 
     FontOrientation orientation() const { return m_orientation; }
@@ -112,11 +113,10 @@ public:
     mutable RefPtr<HarfBuzzFace> m_harfBuzzFace;
 
 private:
-    void initializeWithFontFace(cairo_font_face_t*, const FontDescription& = FontDescription());
+    void buildScaledFont(cairo_font_face_t*);
     static cairo_scaled_font_t* hashTableDeletedFontValue() { return reinterpret_cast<cairo_scaled_font_t*>(-1); }
 
     FontOrientation m_orientation;
-    cairo_matrix_t m_horizontalOrientationMatrix;
 };
 
 }
index 9aefa6a..f7226b5 100644 (file)
@@ -134,16 +134,6 @@ static FcPattern* getDefaultFontconfigOptions()
     return pattern;
 }
 
-static void rotateCairoMatrixForVerticalOrientation(cairo_matrix_t* matrix)
-{
-    // The resulting transformation matrix for vertical glyphs (V) is a
-    // combination of rotation (R) and translation (T) applied on the
-    // horizontal matrix (H). V = H . R . T, where R rotates by -90 degrees
-    // and T translates by font size towards y axis.
-    cairo_matrix_rotate(matrix, -piOverTwoDouble);
-    cairo_matrix_translate(matrix, 0.0, 1.0);
-}
-
 FontPlatformData::FontPlatformData(FcPattern* pattern, const FontDescription& fontDescription)
     : m_pattern(pattern)
     , m_fallbacks(nullptr)
@@ -154,8 +144,8 @@ FontPlatformData::FontPlatformData(FcPattern* pattern, const FontDescription& fo
     , m_scaledFont(nullptr)
     , m_orientation(fontDescription.orientation())
 {
+    ASSERT(m_pattern);
     RefPtr<cairo_font_face_t> fontFace = adoptRef(cairo_ft_font_face_create_for_pattern(m_pattern.get()));
-    initializeWithFontFace(fontFace.get(), fontDescription);
 
     int spacing;
     if (FcPatternGetInteger(pattern, FC_SPACING, 0, &spacing) == FcResultMatch && spacing == FC_MONO)
@@ -173,6 +163,16 @@ FontPlatformData::FontPlatformData(FcPattern* pattern, const FontDescription& fo
         if (!m_syntheticBold && FcPatternGetInteger(pattern, FC_WEIGHT, 0, &weight) == FcResultMatch)
             m_syntheticBold = m_syntheticBold || weight < FC_WEIGHT_DEMIBOLD;
     }
+
+    // We requested an italic font, but Fontconfig gave us one that was neither oblique nor italic.
+    int actualFontSlant;
+    bool descriptionAllowsSyntheticOblique = fontDescription.fontSynthesis() & FontSynthesisStyle;
+    if (descriptionAllowsSyntheticOblique && fontDescription.italic()
+        && FcPatternGetInteger(pattern, FC_SLANT, 0, &actualFontSlant) == FcResultMatch) {
+        m_syntheticOblique = actualFontSlant == FC_SLANT_ROMAN;
+    }
+
+    buildScaledFont(fontFace.get());
 }
 
 FontPlatformData::FontPlatformData(float size, bool bold, bool italic)
@@ -196,7 +196,7 @@ FontPlatformData::FontPlatformData(cairo_font_face_t* fontFace, float size, bool
     , m_scaledFont(nullptr)
     , m_orientation(orientation)
 {
-    initializeWithFontFace(fontFace);
+    buildScaledFont(fontFace);
 
     FT_Face fontConfigFace = cairo_ft_scaled_font_lock_face(m_scaledFont);
     if (fontConfigFace) {
@@ -217,7 +217,6 @@ FontPlatformData& FontPlatformData::operator=(const FontPlatformData& other)
     m_fixedWidth = other.m_fixedWidth;
     m_pattern = other.m_pattern;
     m_orientation = other.m_orientation;
-    m_horizontalOrientationMatrix = other.m_horizontalOrientationMatrix;
 
     if (m_fallbacks) {
         FcFontSetDestroy(m_fallbacks);
@@ -252,7 +251,7 @@ FontPlatformData::FontPlatformData(const FontPlatformData& other, float size)
     // We need to reinitialize the instance, because the difference in size 
     // necessitates a new scaled font instance.
     m_size = size;
-    initializeWithFontFace(cairo_scaled_font_get_font_face(m_scaledFont));
+    buildScaledFont(cairo_scaled_font_get_font_face(m_scaledFont));
 }
 
 FontPlatformData::~FontPlatformData()
@@ -301,7 +300,7 @@ String FontPlatformData::description() const
 }
 #endif
 
-void FontPlatformData::initializeWithFontFace(cairo_font_face_t* fontFace, const FontDescription& fontDescription)
+void FontPlatformData::buildScaledFont(cairo_font_face_t* fontFace)
 {
     cairo_font_options_t* options = getDefaultCairoFontOptions();
     FcPattern* optionsPattern = m_pattern ? m_pattern.get() : getDefaultFontconfigOptions();
@@ -310,10 +309,6 @@ void FontPlatformData::initializeWithFontFace(cairo_font_face_t* fontFace, const
     cairo_matrix_t ctm;
     cairo_matrix_init_identity(&ctm);
 
-    // Scaling a font with width zero size leads to a failed cairo_scaled_font_t instantiations.
-    // Instead we scale we scale the font to a very tiny size and just abort rendering later on.
-    float realSize = m_size ? m_size : 1;
-
     // FontConfig may return a list of transformation matrices with the pattern, for instance,
     // for fonts that are oblique. We use that to initialize the cairo font matrix.
     cairo_matrix_t fontMatrix;
@@ -326,15 +321,10 @@ void FontPlatformData::initializeWithFontFace(cairo_font_face_t* fontFace, const
     cairo_matrix_init(&fontMatrix, fontConfigMatrix.xx, -fontConfigMatrix.yx,
         -fontConfigMatrix.xy, fontConfigMatrix.yy, 0, 0);
 
-    // We requested an italic font, but Fontconfig gave us one that was neither oblique nor italic.
-    int actualFontSlant;
-    bool descriptionAllowsSyntheticOblique = fontDescription.fontSynthesis() & FontSynthesisStyle;
-    if (descriptionAllowsSyntheticOblique && fontDescription.italic()
-        && FcPatternGetInteger(optionsPattern, FC_SLANT, 0, &actualFontSlant) == FcResultMatch) {
-        m_syntheticOblique = actualFontSlant == FC_SLANT_ROMAN;
-    }
-
-    // The matrix from FontConfig does not include the scale. 
+    // The matrix from FontConfig does not include the scale. Scaling a font with width zero size leads
+    // to a failed cairo_scaled_font_t instantiations. Instead we scale we scale the font to a very tiny
+    // size and just abort rendering later on.
+    float realSize = m_size ? m_size : 1;
     cairo_matrix_scale(&fontMatrix, realSize, realSize);
 
     if (syntheticOblique()) {
@@ -344,9 +334,17 @@ void FontPlatformData::initializeWithFontFace(cairo_font_face_t* fontFace, const
         cairo_matrix_multiply(&fontMatrix, m_orientation == Vertical ? &verticalSkew : &skew, &fontMatrix);
     }
 
-    m_horizontalOrientationMatrix = fontMatrix;
-    if (m_orientation == Vertical)
-        rotateCairoMatrixForVerticalOrientation(&fontMatrix);
+    if (m_orientation == Vertical) {
+        // The resulting transformation matrix for vertical glyphs (V) is a
+        // combination of rotation (R) and translation (T) applied on the
+        // horizontal matrix (H). V = H . R . T, where R rotates by -90 degrees
+        // and T translates by font size towards y axis.
+        cairo_matrix_rotate(&fontMatrix, -piOverTwoDouble);
+        cairo_matrix_translate(&fontMatrix, 0.0, 1.0);
+    }
+
+    if (m_scaledFont && m_scaledFont != hashTableDeletedFontValue())
+        cairo_scaled_font_destroy(m_scaledFont);
 
     m_scaledFont = cairo_scaled_font_create(fontFace, &fontMatrix, &ctm, options);
     cairo_font_options_destroy(options);
@@ -397,31 +395,22 @@ PassRefPtr<SharedBuffer> FontPlatformData::openTypeTable(uint32_t table) const
 
 void FontPlatformData::setOrientation(FontOrientation orientation)
 {
-    ASSERT(m_scaledFont);
-
     if (!m_scaledFont || (m_orientation == orientation))
         return;
 
-    cairo_matrix_t transformationMatrix;
-    cairo_matrix_init_identity(&transformationMatrix);
-
-    cairo_matrix_t fontMatrix;
-    cairo_scaled_font_get_font_matrix(m_scaledFont, &fontMatrix);
-
-    cairo_font_options_t* options = getDefaultCairoFontOptions();
+    ASSERT(m_scaledFont);
+    m_orientation = orientation;
+    buildScaledFont(cairo_scaled_font_get_font_face(m_scaledFont));
+}
 
-    // In case of vertical orientation, rotate the transformation matrix.
-    // Otherwise restore the horizontal orientation matrix.
-    if (orientation == Vertical)
-        rotateCairoMatrixForVerticalOrientation(&fontMatrix);
-    else
-        fontMatrix = m_horizontalOrientationMatrix;
+void FontPlatformData::setSyntheticOblique(bool newSyntheticObliqueValue)
+{
+    if (newSyntheticObliqueValue == syntheticOblique())
+        return;
 
-    cairo_font_face_t* fontFace = cairo_scaled_font_get_font_face(m_scaledFont);
-    cairo_scaled_font_destroy(m_scaledFont);
-    m_scaledFont = cairo_scaled_font_create(fontFace, &fontMatrix, &transformationMatrix, options);
-    cairo_font_options_destroy(options);
-    m_orientation = orientation;
+    ASSERT(m_scaledFont);
+    m_syntheticOblique = newSyntheticObliqueValue;
+    buildScaledFont(cairo_scaled_font_get_font_face(m_scaledFont));
 }
 
 }