Correcting text baseline for tiny fonts

third_party/WebKit/Source/platform/fonts/SimpleFontData.cpp

Index: third_party/WebKit/Source/platform/fonts/SimpleFontData.cpp
diff --git a/third_party/WebKit/Source/platform/fonts/SimpleFontData.cpp b/third_party/WebKit/Source/platform/fonts/SimpleFontData.cpp
index ec4db7a45e1e83a10db17dfc2517cdcb22b0c53e..1bad4deee0563157ca9e4566a892eb66e25e87a5 100644
--- a/third_party/WebKit/Source/platform/fonts/SimpleFontData.cpp
+++ b/third_party/WebKit/Source/platform/fonts/SimpleFontData.cpp
@@ -58,7 +58,8 @@ static const size_t maxVDMXTableSize = 1024 * 1024;  // 1 MB
 
 SimpleFontData::SimpleFontData(const FontPlatformData& platformData,
                                PassRefPtr<CustomFontData> customData,
-                               bool isTextOrientationFallback)
+                               bool isTextOrientationFallback,
+                               bool subpixelAscentDescent)
     : m_maxCharWidth(-1),
       m_avgCharWidth(-1),
       m_platformData(platformData),
@@ -66,7 +67,7 @@ SimpleFontData::SimpleFontData(const FontPlatformData& platformData,
       m_verticalData(nullptr),
       m_hasVerticalGlyphs(false),
       m_customFontData(customData) {
-  platformInit();
+  platformInit(subpixelAscentDescent);
   platformGlyphInit();
   if (platformData.isVerticalAnyUpright() && !isTextOrientationFallback) {
     m_verticalData = platformData.verticalData();
@@ -86,7 +87,7 @@ SimpleFontData::SimpleFontData(PassRefPtr<CustomFontData> customData,
       m_hasVerticalGlyphs(false),
       m_customFontData(customData) {}
 
-void SimpleFontData::platformInit() {
+void SimpleFontData::platformInit(bool subpixelAscentDescent) {
   if (!m_platformData.size()) {
     m_fontMetrics.reset();
     m_avgCharWidth = 0;
@@ -131,14 +132,24 @@ void SimpleFontData::platformInit() {
   float descent;
 
   // Beware those who step here: This code is designed to match Win32 font
-  // metrics *exactly* (except the adjustment of ascent/descent on
-  // Linux/Android).
+  // metrics *exactly* except:
+  // - the adjustment of ascent/descent on Linux/Android
+  // - metrics.fAscent and .fDesscent are not rounded to int for tiny fonts
   if (isVDMXValid) {
     ascent = vdmxAscent;
     descent = -vdmxDescent;
   } else {
-    ascent = SkScalarRoundToInt(-metrics.fAscent);
-    descent = SkScalarRoundToInt(metrics.fDescent);
+    // For tiny fonts, the rounding of fAscent and fDescent results in equal
+    // baseline for different types of text baselines (crbug.com/338908).
+    // Please see CanvasRenderingContext2D::getFontBaseline for the heuristic.
+    if (subpixelAscentDescent &&

[drott, 2016/10/18 10:42:17]

zakerinasab, did you say the same problem applies to regular text and SVG?

eae@, what do you think, would it perhaps make sense to make this the default
here using a decision function in FontCache.h for example, something like
"shouldUseSubpixelMetricsForAscentDescent(float, float) - and synchronize it
that way with the code in Canvas, instead of comments and piping through the
bool flag?

I'm also slightly worried that the bool flag is not part of
FontDescription::cacheKey. In an unlucky situation we might mix up a
SimpleFontData with subpixel baseline and without.m

+        (-metrics.fAscent < 3 || -metrics.fAscent + metrics.fDescent < 2)) {
+      ascent = -metrics.fAscent;
+      descent = metrics.fDescent;
+    } else {
+      ascent = SkScalarRoundToScalar(-metrics.fAscent);
+      descent = SkScalarRoundToScalar(metrics.fDescent);
+    }
 #if OS(LINUX) || OS(ANDROID)
     // When subpixel positioning is enabled, if the descent is rounded down, the
     // descent part of the glyph may be truncated when displayed in a 'overflow: