Changeset 180563 in webkit


Ignore:
Timestamp:
Feb 24, 2015 9:07:46 AM (9 years ago)
Author:
commit-queue@webkit.org
Message:

[GTK] Fonts loaded via @font-face look bad
​https://bugs.webkit.org/show_bug.cgi?id=140994

Patch by Michael Catanzaro <Michael Catanzaro> on 2015-02-24
Reviewed by Martin Robinson.

We've had several complaints that woff fonts look bad on some websites. This seems to be a
combination of multiple issues. For one, we don't look at Fontconfig settings at all when
creating a web font. This commit changes FontPlatformData::initializeWithFontFace to instead
use sane default settings from Fontconfig when loading a web font, rather than accepting the
default settings from GTK+, which are normally overridden by Fontconfig when loading system
fonts. However, we will hardcode the hinting setting for web fonts to always force use of
the light autohinter, so that we do not use a font's native hints. This avoids compatibility
issues when fonts with poor native hinting are only tested in browsers that do not use the
native hints. It also allows us to sidestep future security vulnerabilities in FreeType's
bytecode interpreter.

The net result of this is that there should be little noticable difference if you have GTK+
set to use slight hinting (which forces use of the autohinter) unless you have customized
Fontconfig configuration, but a dramatic improvement with particular fonts if you use medium
or full hinting. This should reduce complaints about our font rendering on "fancy sites."

No new tests, since the affected fonts we've found are not freely redistributable.

  • platform/graphics/freetype/FontCustomPlatformDataFreeType.cpp:

(WebCore::FontCustomPlatformData::FontCustomPlatformData): Force web fonts to be autohinted.

  • platform/graphics/freetype/FontPlatformDataFreeType.cpp:

(WebCore::getDefaultCairoFontOptions): Renamed to disambiguate.
(WebCore::getDefaultFontconfigOptions): Added.
(WebCore::FontPlatformData::initializeWithFontFace): Always call
FontPlatformData::setCairoOptionsFromFontConfigPattern. If the FontPlatformData was not
created with an FcPattern (e.g. because this is a web font), call
getDefaultFontconfigOptions to get a sane default FcPattern.
(WebCore::FontPlatformData::setOrientation): Renamed call to getDefaultCairoFontOptions.
(WebCore::getDefaultFontOptions): Deleted.

Location:
trunk/Source/WebCore
Files:
3 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r180561 r180563  
     12015-02-24  Michael Catanzaro  <mcatanzaro@igalia.com>
     2
     3        [GTK] Fonts loaded via @font-face look bad
     4        https://bugs.webkit.org/show_bug.cgi?id=140994
     5
     6        Reviewed by Martin Robinson.
     7
     8        We've had several complaints that woff fonts look bad on some websites. This seems to be a
     9        combination of multiple issues. For one, we don't look at Fontconfig settings at all when
     10        creating a web font. This commit changes FontPlatformData::initializeWithFontFace to instead
     11        use sane default settings from Fontconfig when loading a web font, rather than accepting the
     12        default settings from GTK+, which are normally overridden by Fontconfig when loading system
     13        fonts. However, we will hardcode the hinting setting for web fonts to always force use of
     14        the light autohinter, so that we do not use a font's native hints. This avoids compatibility
     15        issues when fonts with poor native hinting are only tested in browsers that do not use the
     16        native hints. It also allows us to sidestep future security vulnerabilities in FreeType's
     17        bytecode interpreter.
     18
     19        The net result of this is that there should be little noticable difference if you have GTK+
     20        set to use slight hinting (which forces use of the autohinter) unless you have customized
     21        Fontconfig configuration, but a dramatic improvement with particular fonts if you use medium
     22        or full hinting. This should reduce complaints about our font rendering on "fancy sites."
     23
     24        No new tests, since the affected fonts we've found are not freely redistributable.
     25
     26        * platform/graphics/freetype/FontCustomPlatformDataFreeType.cpp:
     27        (WebCore::FontCustomPlatformData::FontCustomPlatformData): Force web fonts to be autohinted.
     28        * platform/graphics/freetype/FontPlatformDataFreeType.cpp:
     29        (WebCore::getDefaultCairoFontOptions): Renamed to disambiguate.
     30        (WebCore::getDefaultFontconfigOptions): Added.
     31        (WebCore::FontPlatformData::initializeWithFontFace): Always call
     32        FontPlatformData::setCairoOptionsFromFontConfigPattern. If the FontPlatformData was not
     33        created with an FcPattern (e.g. because this is a web font), call
     34        getDefaultFontconfigOptions to get a sane default FcPattern.
     35        (WebCore::FontPlatformData::setOrientation): Renamed call to getDefaultCairoFontOptions.
     36        (WebCore::getDefaultFontOptions): Deleted.
     37
    1382015-02-24  Andreas Kling  <akling@apple.com>
    239

  • trunk/Source/WebCore/platform/graphics/freetype/FontCustomPlatformDataFreeType.cpp

    r178717 r180563  
    3636
    3737FontCustomPlatformData::FontCustomPlatformData(FT_Face freeTypeFace, SharedBuffer& buffer)
    38     : m_fontFace(cairo_ft_font_face_create_for_ft_face(freeTypeFace, 0))
     38    : m_fontFace(cairo_ft_font_face_create_for_ft_face(freeTypeFace, FT_LOAD_FORCE_AUTOHINT))
    3939{
    40     // FIXME Should we be setting some hinting options here?
     40    // FT_LOAD_FORCE_AUTOHINT prohibits use of the font's native hinting. This
     41    // is a safe option for custom fonts because (a) some such fonts may have
     42    // broken hinting, which site admins may not notice if other browsers do not
     43    // use the native hints, and (b) allowing native hints exposes the FreeType
     44    // bytecode interpreter to potentially-malicious input. Treating web fonts
     45    // differently than system fonts is non-ideal, but the result of autohinting
     46    // is always decent, whereas native hints sometimes look terrible, and
     47    // unlike system fonts where Fontconfig may change the hinting settings on a
     48    // per-font basis, the same settings are used for all web fonts. Note that
     49    // Chrome is considering switching from autohinting to native hinting in
     50    // https://code.google.com/p/chromium/issues/detail?id=173207 but this is
     51    // more risk than we want to assume for now. See
     52    // https://bugs.webkit.org/show_bug.cgi?id=140994 before changing this.
    4153
    4254    buffer.ref(); // This is balanced by the buffer->deref() in releaseCustomFontData.

  • trunk/Source/WebCore/platform/graphics/freetype/FontPlatformDataFreeType.cpp

    r180281 r180563  
    2727
    2828#include "FontDescription.h"
     29#include "RefPtrCairo.h"
    2930#include <cairo-ft.h>
    3031#include <cairo.h>
    … …  
    104105}
    105106
    106 static cairo_font_options_t* getDefaultFontOptions()
     107static cairo_font_options_t* getDefaultCairoFontOptions()
    107108{
    108109#if PLATFORM(GTK)
    … …  
    114115#endif
    115116    return cairo_font_options_create();
     117}
     118
     119static FcPattern* getDefaultFontconfigOptions()
     120{
     121    // Get some generic default settings from fontconfig for web fonts. Strategy
     122    // from Behdad Esfahbod in https://code.google.com/p/chromium/issues/detail?id=173207#c35
     123    // For web fonts, the hint style is overridden in FontCustomPlatformData::FontCustomPlatformData
     124    // so Fontconfig will not affect the hint style, but it may disable hinting completely.
     125    static FcPattern* pattern = nullptr;
     126    static std::once_flag flag;
     127    std::call_once(flag, [](FcPattern*) {
     128        pattern = FcPatternCreate();
     129        FcConfigSubstitute(nullptr, pattern, FcMatchPattern);
     130        FcDefaultSubstitute(pattern);
     131        FcPatternDel(pattern, FC_FAMILY);
     132        FcConfigSubstitute(nullptr, pattern, FcMatchFont);
     133    }, pattern);
     134    return pattern;
    116135}
    117136
    … …  
    284303void FontPlatformData::initializeWithFontFace(cairo_font_face_t* fontFace, const FontDescription& fontDescription)
    285304{
    286     cairo_font_options_t* options = getDefaultFontOptions();
     305    cairo_font_options_t* options = getDefaultCairoFontOptions();
     306    FcPattern* optionsPattern = m_pattern ? m_pattern.get() : getDefaultFontconfigOptions();
     307    setCairoFontOptionsFromFontConfigPattern(options, optionsPattern);
    287308
    288309    cairo_matrix_t ctm;
    … …  
    293314    float realSize = m_size ? m_size : 1;
    294315
     316    // FontConfig may return a list of transformation matrices with the pattern, for instance,
     317    // for fonts that are oblique. We use that to initialize the cairo font matrix.
    295318    cairo_matrix_t fontMatrix;
    296     if (!m_pattern)
    297         cairo_matrix_init_scale(&fontMatrix, realSize, realSize);
    298     else {
    299         setCairoFontOptionsFromFontConfigPattern(options, m_pattern.get());
    300 
    301         // FontConfig may return a list of transformation matrices with the pattern, for instance,
    302         // for fonts that are oblique. We use that to initialize the cairo font matrix.
    303         FcMatrix fontConfigMatrix, *tempFontConfigMatrix;
    304         FcMatrixInit(&fontConfigMatrix);
    305 
    306         // These matrices may be stacked in the pattern, so it's our job to get them all and multiply them.
    307         for (int i = 0; FcPatternGetMatrix(m_pattern.get(), FC_MATRIX, i, &tempFontConfigMatrix) == FcResultMatch; i++)
    308             FcMatrixMultiply(&fontConfigMatrix, &fontConfigMatrix, tempFontConfigMatrix);
    309         cairo_matrix_init(&fontMatrix, fontConfigMatrix.xx, -fontConfigMatrix.yx,
    310                           -fontConfigMatrix.xy, fontConfigMatrix.yy, 0, 0);
    311 
    312         // We requested an italic font, but Fontconfig gave us one that was neither oblique nor italic.
    313         int actualFontSlant;
    314         if (fontDescription.italic() && FcPatternGetInteger(m_pattern.get(), FC_SLANT, 0, &actualFontSlant) == FcResultMatch)
    315             m_syntheticOblique = actualFontSlant == FC_SLANT_ROMAN;
    316 
    317         // The matrix from FontConfig does not include the scale.
    318         cairo_matrix_scale(&fontMatrix, realSize, realSize);
    319     }
     319    FcMatrix fontConfigMatrix, *tempFontConfigMatrix;
     320    FcMatrixInit(&fontConfigMatrix);
     321
     322    // These matrices may be stacked in the pattern, so it's our job to get them all and multiply them.
     323    for (int i = 0; FcPatternGetMatrix(m_pattern.get(), FC_MATRIX, i, &tempFontConfigMatrix) == FcResultMatch; i++)
     324        FcMatrixMultiply(&fontConfigMatrix, &fontConfigMatrix, tempFontConfigMatrix);
     325    cairo_matrix_init(&fontMatrix, fontConfigMatrix.xx, -fontConfigMatrix.yx,
     326        -fontConfigMatrix.xy, fontConfigMatrix.yy, 0, 0);
     327
     328    // We requested an italic font, but Fontconfig gave us one that was neither oblique nor italic.
     329    int actualFontSlant;
     330    if (fontDescription.italic() && FcPatternGetInteger(m_pattern.get(), FC_SLANT, 0, &actualFontSlant) == FcResultMatch)
     331        m_syntheticOblique = actualFontSlant == FC_SLANT_ROMAN;
     332
     333    // The matrix from FontConfig does not include the scale.
     334    cairo_matrix_scale(&fontMatrix, realSize, realSize);
    320335
    321336    if (syntheticOblique()) {
    … …  
    389404    cairo_scaled_font_get_font_matrix(m_scaledFont, &fontMatrix);
    390405
    391     cairo_font_options_t* options = getDefaultFontOptions();
     406    cairo_font_options_t* options = getDefaultCairoFontOptions();
    392407
    393408    // In case of vertical orientation, rotate the transformation matrix.
Note: See TracChangeset for help on using the changeset viewer.