• Etienne Bergeron's avatar
    Fix font-height off-by one due to rounding · d3045ebd
    Etienne Bergeron authored
    This CL is fixes the unit tests for the PlatformFontSkiaOnWindows experiment.
      1) It fixes the off-by one due to rounding (e.g. font height computation).
      2) It uses the DirectWrite Skia font manager by default; otherwise the default
         is the GDI font manager which is not giving the appropriate metrics.
    
    1) Off-by-one due to rounding:
    
    PlatformFontWin is computing the font metrics with Skia in the
    function PlatformFontWin::CreateHFontRefFromSkia.
    
    See: https://cs.chromium.org/chromium/src/ui/gfx/platform_font_win.cc?l=515
    
        SkFontMetrics skia_metrics;
        font.getMetrics(&skia_metrics);
        const int height = std::ceil(skia_metrics.fDescent - skia_metrics.fAscent);
    
    PlatformFontSkia is computing it this way (ComputeMetricsIfNecessary):
    
    See: https://cs.chromium.org/chromium/src/ui/gfx/platform_font_skia.cc?l=342
    
        SkFontMetrics skia_metrics;
        font.getMetrics(&skia_metrics);
        ascent_pixels_ = SkScalarCeilToInt(-metrics.fAscent);
        height_pixels_ = ascent_pixels_ + SkScalarCeilToInt(metrics.fDescent);
    
    The height computation is also not consistent with the way it's computed on Mac(platform_font_mac).
    
    See: https://cs.chromium.org/chromium/src/ui/gfx/platform_font_mac.mm?q=platformfont+mac&dr=C&l=355
    
         height_ = ceil(ascent_ + std::abs([font descender]) + [font leading]);
    
    This difference is making the font height on linux/windows off by one pixel.
    Unittests were adjusted for these off-by-one on each platform.
    
    The migration of PlatformFontWin to PlatformFontSkia is making these tests
    to fail. Ideally, we should choose one implementation and fix the code.
    To ease the migration, this CL implements on Windows the same computation
    used in PlatformFontWin to minimize the visible changes in Chrome.
    
    2) DirectWrite Font Manager
    
    The unittests are not calling InitializeDirectWrite(..) to force an override of the
    Skia font manager. The browser code currently fails if the font manager is not set to
    DirectWrite. Unfortunately, the unittests are not calling the initialization function
    and were using by default the GDI adapter.
    
    The simple way to fix this issue is to ensures that Chrome code (browser and tests) are
    only using the DirectWrite version. This is fine since WinXP is no longer maintained
    and DirectWrite is always available on Win7+.
    
    Bug: 944227
    R=robliao@chromium.org, asvitkine@chromium.org
    
    Change-Id: Ide54b96d576c5e4ec862dabb5aef6c735a3ffdc5
    Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1721453
    Commit-Queue: Etienne Bergeron <etienneb@chromium.org>
    Reviewed-by: default avatarRobert Liao <robliao@chromium.org>
    Reviewed-by: default avatarAlexei Svitkine <asvitkine@chromium.org>
    Reviewed-by: default avatarScott Violet <sky@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#688715}
    d3045ebd
platform_font_skia.cc 14.8 KB