Fix 'text-underline-position: under' to use em height ascent/descent

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 d3224097198037903690caeb0d420a23b89898bf..d422288b92e5dbaf61536c2e2d4a46c26725a51d 100644
--- a/third_party/WebKit/Source/platform/fonts/SimpleFontData.cpp
+++ b/third_party/WebKit/Source/platform/fonts/SimpleFontData.cpp
@@ -39,6 +39,7 @@
 #include "platform/fonts/VDMXParser.h"
 #include "platform/fonts/skia/SkiaTextMetrics.h"
 #include "platform/geometry/FloatRect.h"
+#include "platform/wtf/ByteOrder.h"
 #include "platform/wtf/MathExtras.h"
 #include "platform/wtf/PtrUtil.h"
 #include "platform/wtf/allocator/Partitions.h"
@@ -379,6 +380,90 @@ PassRefPtr<SimpleFontData> SimpleFontData::CreateScaledFontData(
       IsCustomFont() ? CustomFontData::Create() : nullptr);
 }
 
+// Internal leadings can be distributed to ascent and descent.
+// -------------------------------------------
+//           | - Internal Leading (in ascent)
+//           |--------------------------------
+//  Ascent - |              |
+//           |              |
+//           |              | - Em height
+// ----------|--------------|
+//           |              |
+// Descent - |--------------------------------
+//           | - Internal Leading (in descent)
+// -------------------------------------------
+LayoutUnit SimpleFontData::EmHeightAscent(FontBaseline baseline_type) const {
+  if (baseline_type == kAlphabeticBaseline) {
+    if (!em_height_ascent_)
+      ComputeEmHeightMetrics();
+    return em_height_ascent_;
+  }
+  LayoutUnit em_height = LayoutUnit::FromFloatRound(PlatformData().size());
+  return em_height - em_height / 2;
+}
+
+LayoutUnit SimpleFontData::EmHeightDescent(FontBaseline baseline_type) const {
+  if (baseline_type == kAlphabeticBaseline) {
+    if (!em_height_descent_)
+      ComputeEmHeightMetrics();
+    return em_height_descent_;
+  }
+  LayoutUnit em_height = LayoutUnit::FromFloatRound(PlatformData().size());
+  return em_height / 2;
+}
+
+static std::pair<int16_t, int16_t> TypoAscenderAndDescender(
+    SkTypeface* typeface) {
+  // TODO(kojii): This should move to Skia once finalized. We can then move

[bungeman-chromium, 2017/05/03 13:56:37]

What is the reason for not using SkPaint::getFontMetrics and using the fAscent /
fDescent from that? Those are almost always these values anyway, and when
they're not they're some other value the font specified for this purpose. Isn't
that information already available? It appears you're already 'normalizing'
these anyway, so is there a reason that only and exactly the 'typo' versions are
valid?

+  // EmHeightAscender/Descender to FontMetrics.
+  int16_t buffer[2];
+  size_t size = typeface->getTableData(SkSetFourByteTag('O', 'S', '/', '2'), 68,

[drott, 2017/05/08 08:04:00]

What's 68 here? Could you make a constant for this?

[kojii, 2017/05/09 04:37:16]

It's the offset. Skia has access to os2table so they don't need it but we need
to hard code until this is moved to Skia. Should have commented, thank you
pointing out.

+                                       sizeof(buffer), buffer);
+  if (size == sizeof(buffer)) {
+    return std::make_pair(static_cast<int16_t>(ntohs(buffer[0])),
+                          -static_cast<int16_t>(ntohs(buffer[1])));
+  }
+  return std::make_pair(0, 0);
+}
+
+void SimpleFontData::ComputeEmHeightMetrics() const {
+  // Compute em height metrics from OS/2 sTypoAscender and sTypoDescender.
+  SkTypeface* typeface = platform_data_.Typeface();
+  int16_t typo_ascender, typo_descender;
+  std::tie(typo_ascender, typo_descender) = TypoAscenderAndDescender(typeface);
+  if (typo_ascender > 0 &&
+      NormalizeEmHeightMetrics(typo_ascender, typo_ascender + typo_descender)) {
+    return;
+  }
+
+  // As the last resort, compute em height metrics from our ascent/descent.
+  const FontMetrics& font_metrics = GetFontMetrics();
+  if (NormalizeEmHeightMetrics(font_metrics.FloatAscent(),
+                               font_metrics.FloatHeight())) {
+    return;
+  }
+  NOTREACHED();
+}
+
+bool SimpleFontData::NormalizeEmHeightMetrics(float ascent,
+                                              float height) const {
+  if (height <= 0 || ascent < 0 || ascent > height)
+    return false;
+  // While the OpenType specification recommends the sum of sTypoAscender and
+  // sTypoDescender to equal 1em, most fonts do not follow. Most Latin fonts
+  // set to smaller than 1em, and many tall scripts set to larger than 1em.
+  // https://www.microsoft.com/typography/otspec/recom.htm#OS2
+  // To ensure the sum of ascent and descent is the "em height", normalize by
+  // keeping the ratio of sTypoAscender:sTypoDescender.
+  // This matches to how Gecko computes "em height":

[drott, 2017/05/08 08:04:00]

Would be great if you had a Mozilla DXR link to where it happens in Gecko.

[kojii, 2017/05/09 04:37:16]

Yeah, not great at reading Mozilla code yet...

+  // https://github.com/whatwg/html/issues/2470#issuecomment-291425136
+  float em_height = PlatformData().size();
+  em_height_ascent_ = LayoutUnit::FromFloatRound(ascent * em_height / height);
+  em_height_descent_ =
+      LayoutUnit::FromFloatRound(em_height) - em_height_ascent_;
+  return true;
+}
+
 FloatRect SimpleFontData::PlatformBoundsForGlyph(Glyph glyph) const {
   if (!platform_data_.size())
     return FloatRect();