Unify glyph metrics to use the same Skia API

Use identical glyph metrics based bounds in HarfBuzz callbacks as well
as ShapeResult's glyph bounds calculation, except on Mac, due to Skia
bug https://bugs.chromium.org/p/skia/issues/detail?id=5328

* Using the same glyph bounds callback in HarfBuzz shaping and in
  ShapeResult's glyph bounds calculation enables reusing of the same
  cache in Skia, as opposed to using the cached glyph metrics based
  cache vs. using a second cache for path based metrics before.

Local benchmarking of the blink_perf.layout shows 12.44%
improvements for the character_fallback test and 1.8-5%
improvements on chapter-reflow-once, line-layout and
latin-complex-test. There are also some hits on flexbox-lots of
data and flexbox-row-nowrap. I am not 100% convinced that the
local benchmarks are accurate enough and would like to observe
the results on the bot. Overall, we should see a layout speed
improvement, perhaps even in the page cyclers.

BUG=610313
R=kojii,tzik

Review-Url: https://codereview.chromium.org/1980913002
Cr-Commit-Position: refs/heads/master@{#407755}

third_party/WebKit/Source/core/layout/compositing/CompositedLayerMappingTest.cpp

@@ -504,12 +504,14 @@ TEST_F(CompositedLayerMappingTest, InterestRectShouldNotChangeOnPaintInvalidatio
 TEST_F(CompositedLayerMappingTest, InterestRectOfSquashingLayerWithNegativeOverflow)
 {
     setBodyInnerHTML(
-        "<style>body { margin: 0 }</style>"
+        "<style>body { margin: 0; font-size: 16px; }</style>"
         "<div style='position: absolute; top: -500px; width: 200px; height: 700px; will-change: transform'></div>"
-        "<div id='squashed' style='position: absolute; top: 190px'>"
-        "  <div style='width: 100px; height: 100px; text-indent: -10000px'>text</div>"
+        "<div id='squashed' style='position: absolute; top: 190px;'>"
+        "  <div id='inside' style='width: 100px; height: 100px; text-indent: -10000px'>text</div>"
         "</div>");
 
+    EXPECT_EQ(document().getElementById("inside")->layoutBox()->visualOverflowRect().size().height(), 100);
+
     CompositedLayerMapping* groupedMapping = document().getElementById("squashed")->layoutBox()->layer()->groupedMapping();
     // The squashing layer is at (-10000, 190, 10100, 100) in viewport coordinates.
     // The following rect is at (-4000, 190, 4100, 100) in viewport coordinates.

third_party/WebKit/Source/platform/blink_platform.gypi

@@ -489,6 +489,8 @@
       'fonts/shaping/SimpleShaper.cpp',
       'fonts/shaping/SimpleShaper.h',
       'fonts/skia/FontCacheSkia.cpp',
+      'fonts/skia/SkiaTextMetrics.h',
+      'fonts/skia/SkiaTextMetrics.cpp',
       'fonts/win/FontCacheSkiaWin.cpp',
       'fonts/win/FontFallbackWin.cpp',
       'fonts/win/FontFallbackWin.h',

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

@@ -29,13 +29,13 @@
 
 #include "platform/fonts/SimpleFontData.h"
 
-#include "SkPaint.h"
 #include "SkPath.h"
 #include "SkTypeface.h"
 #include "SkTypes.h"
 #include "platform/fonts/FontDescription.h"
 #include "platform/fonts/GlyphPage.h"
 #include "platform/fonts/VDMXParser.h"
+#include "platform/fonts/skia/SkiaTextMetrics.h"
 #include "platform/geometry/FloatRect.h"
 #include "wtf/MathExtras.h"
 #include "wtf/PtrUtil.h"
@@ -91,12 +91,12 @@ void SimpleFontData::platformInit()
         return;
     }
 
-    SkPaint paint;
     SkPaint::FontMetrics metrics;
 
-    m_platformData.setupPaint(&paint);
-    paint.getFontMetrics(&metrics);
-    SkTypeface* face = paint.getTypeface();
+    m_platformData.setupPaint(&m_paint);
+    m_paint.setTextEncoding(SkPaint::kGlyphID_TextEncoding);
+    m_paint.getFontMetrics(&metrics);
+    SkTypeface* face = m_paint.getTypeface();
     ASSERT(face);
 
     int vdmxAscent = 0, vdmxDescent = 0;
@@ -108,9 +108,9 @@ void SimpleFontData::platformInit()
     // This code should be pushed into FreeType (hinted font metrics).
     static const uint32_t vdmxTag = SkSetFourByteTag('V', 'D', 'M', 'X');
     int pixelSize = m_platformData.size() + 0.5;
-    if (!paint.isAutohinted()
-        &&    (paint.getHinting() == SkPaint::kFull_Hinting
-            || paint.getHinting() == SkPaint::kNormal_Hinting))
+    if (!m_paint.isAutohinted()
+        &&    (m_paint.getHinting() == SkPaint::kFull_Hinting
+            || m_paint.getHinting() == SkPaint::kNormal_Hinting))
     {
         size_t vdmxSize = face->getTableSize(vdmxTag);
         if (vdmxSize && vdmxSize < maxVDMXTableSize) {
@@ -371,54 +371,19 @@ PassRefPtr<SimpleFontData> SimpleFontData::createScaledFontData(const FontDescri
     return SimpleFontData::create(FontPlatformData(m_platformData, scaledSize), isCustomFont() ? CustomFontData::create() : nullptr);
 }
 
-static inline void getSkiaBoundsForGlyph(SkPaint& paint, Glyph glyph, SkRect& bounds)
-{
-    paint.setTextEncoding(SkPaint::kGlyphID_TextEncoding);
-
-    SkPath path;
-    paint.getTextPath(&glyph, sizeof(glyph), 0, 0, &path);
-    bounds = path.getBounds();
-
-    if (!paint.isSubpixelText()) {
-        SkIRect ir;
-        bounds.round(&ir);
-        bounds.set(ir);
-    }
-}
-
 FloatRect SimpleFontData::platformBoundsForGlyph(Glyph glyph) const
 {
-    if (!m_platformData.size())
-        return FloatRect();
-
-    SkASSERT(sizeof(glyph) == 2); // compile-time assert
-
-    SkPaint paint;
-    m_platformData.setupPaint(&paint);
-
     SkRect bounds;
-    getSkiaBoundsForGlyph(paint, glyph, bounds);
+    SkiaTextMetrics(&m_paint).getSkiaBoundsForGlyph(glyph, &bounds);
     return FloatRect(bounds);
 }
 
 float SimpleFontData::platformWidthForGlyph(Glyph glyph) const
 {
-    if (!m_platformData.size())
-        return 0;
-
-    SkASSERT(sizeof(glyph) == 2); // compile-time assert
-
-    SkPaint paint;
-
-    m_platformData.setupPaint(&paint);
-
-    paint.setTextEncoding(SkPaint::kGlyphID_TextEncoding);
-    SkScalar width = paint.measureText(&glyph, 2);
-    if (!paint.isSubpixelText())
-        width = SkScalarRoundToInt(width);
-    return SkScalarToFloat(width);
+    return SkiaTextMetrics(&m_paint).getSkiaWidthForGlyph(glyph);
 }
 
+
 bool SimpleFontData::fillGlyphPage(GlyphPage* pageToFill, unsigned offset, unsigned length, UChar* buffer, unsigned bufferLength) const
 {
     if (U16_IS_LEAD(buffer[bufferLength-1])) {

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

@@ -37,6 +37,7 @@
 #include "platform/geometry/FloatRect.h"
 #include "wtf/PtrUtil.h"
 #include "wtf/text/StringHash.h"
+#include <SkPaint.h>
 #include <memory>
 
 namespace blink {
@@ -95,8 +96,8 @@ public:
     void setAvgCharWidth(float avgCharWidth) { m_avgCharWidth = avgCharWidth; }
 
     FloatRect boundsForGlyph(Glyph) const;
-    float widthForGlyph(Glyph glyph) const;
     FloatRect platformBoundsForGlyph(Glyph) const;
+    float widthForGlyph(Glyph) const;
     float platformWidthForGlyph(Glyph) const;
 
     float spaceWidth() const { return m_spaceWidth; }
@@ -143,9 +144,9 @@ private:
     float m_avgCharWidth;
 
     FontPlatformData m_platformData;
-
     mutable std::unique_ptr<GlyphMetricsMap<FloatRect>> m_glyphToBoundsMap;
     mutable GlyphMetricsMap<float> m_glyphToWidthMap;
+    SkPaint m_paint;
 
     bool m_isTextOrientationFallback;
     RefPtr<OpenTypeVerticalData> m_verticalData;
@@ -182,24 +183,35 @@ private:
     RefPtr<CustomFontData> m_customFontData;
 };
 
+
 ALWAYS_INLINE FloatRect SimpleFontData::boundsForGlyph(Glyph glyph) const
 {
-    FloatRect bounds;
+    if (!m_platformData.size())
+        return FloatRect();
+
+    static_assert(sizeof(glyph) == 2, "Glyph id should not be truncated.");
+
+    FloatRect boundsResult;
     if (m_glyphToBoundsMap) {
-        bounds = m_glyphToBoundsMap->metricsForGlyph(glyph);
-        if (bounds.width() != cGlyphSizeUnknown)
-            return bounds;
+        boundsResult = m_glyphToBoundsMap->metricsForGlyph(glyph);
+        if (boundsResult.width() != cGlyphSizeUnknown)
+            return boundsResult;
     }
 
-    bounds = platformBoundsForGlyph(glyph);
+    boundsResult = platformBoundsForGlyph(glyph);
     if (!m_glyphToBoundsMap)
         m_glyphToBoundsMap = wrapUnique(new GlyphMetricsMap<FloatRect>);
-    m_glyphToBoundsMap->setMetricsForGlyph(glyph, bounds);
-    return bounds;
+    m_glyphToBoundsMap->setMetricsForGlyph(glyph, boundsResult);
+
+    return boundsResult;
 }
 
 ALWAYS_INLINE float SimpleFontData::widthForGlyph(Glyph glyph) const
 {
+    if (!m_platformData.size())
+        return 0;
+    static_assert(sizeof(glyph) == 2, "Glyph id should not be truncated.");
+
     float width = m_glyphToWidthMap.metricsForGlyph(glyph);
     if (width != cGlyphSizeUnknown)
         return width;

third_party/WebKit/Source/platform/fonts/shaping/HarfBuzzFace.cpp

@@ -36,6 +36,7 @@
 #include "platform/fonts/SimpleFontData.h"
 #include "platform/fonts/UnicodeRangeSet.h"
 #include "platform/fonts/shaping/HarfBuzzShaper.h"
+#include "platform/fonts/skia/SkiaTextMetrics.h"
 #include "wtf/HashMap.h"
 #include "wtf/MathExtras.h"
 #include "wtf/PtrUtil.h"
@@ -164,37 +165,6 @@ static hb_position_t SkiaScalarToHarfBuzzPosition(SkScalar value)
     return clampTo<int>(value * kHbPosition1);
 }
 
-static void SkiaGetGlyphWidthAndExtents(SkPaint* paint, hb_codepoint_t codepoint, hb_position_t* width, hb_glyph_extents_t* extents)
-{
-    ASSERT(codepoint <= 0xFFFF);
-    paint->setTextEncoding(SkPaint::kGlyphID_TextEncoding);
-
-    SkScalar skWidth;
-    SkRect skBounds;
-    uint16_t glyph = codepoint;
-
-    paint->getTextWidths(&glyph, sizeof(glyph), &skWidth, &skBounds);
-    if (width) {
-        if (!paint->isSubpixelText())
-            skWidth = SkScalarRoundToInt(skWidth);
-        *width = SkiaScalarToHarfBuzzPosition(skWidth);
-    }
-    if (extents) {
-        if (!paint->isSubpixelText()) {
-            // Use roundOut() rather than round() to avoid rendering glyphs
-            // outside the visual overflow rect. crbug.com/452914.
-            SkIRect ir;
-            skBounds.roundOut(&ir);
-            skBounds.set(ir);
-        }
-        // Invert y-axis because Skia is y-grows-down but we set up HarfBuzz to be y-grows-up.
-        extents->x_bearing = SkiaScalarToHarfBuzzPosition(skBounds.fLeft);
-        extents->y_bearing = SkiaScalarToHarfBuzzPosition(-skBounds.fTop);
-        extents->width = SkiaScalarToHarfBuzzPosition(skBounds.width());
-        extents->height = SkiaScalarToHarfBuzzPosition(-skBounds.height());
-    }
-}
-
 static hb_bool_t harfBuzzGetGlyph(hb_font_t* hbFont, void* fontData, hb_codepoint_t unicode, hb_codepoint_t variationSelector, hb_codepoint_t* glyph, void* userData)
 {
     HarfBuzzFontData* hbFontData = reinterpret_cast<HarfBuzzFontData*>(fontData);
@@ -211,7 +181,7 @@ static hb_position_t harfBuzzGetGlyphHorizontalAdvance(hb_font_t* hbFont, void*
     HarfBuzzFontData* hbFontData = reinterpret_cast<HarfBuzzFontData*>(fontData);
     hb_position_t advance = 0;
 
-    SkiaGetGlyphWidthAndExtents(&hbFontData->m_paint, glyph, &advance, 0);
+    SkiaTextMetrics(&hbFontData->m_paint).getGlyphWidthAndExtentsForHarfBuzz(glyph, &advance, 0);
     return advance;
 }
 
@@ -268,7 +238,7 @@ static hb_bool_t harfBuzzGetGlyphExtents(hb_font_t* hbFont, void* fontData, hb_c
 {
     HarfBuzzFontData* hbFontData = reinterpret_cast<HarfBuzzFontData*>(fontData);
 
-    SkiaGetGlyphWidthAndExtents(&hbFontData->m_paint, glyph, 0, extents);
+    SkiaTextMetrics(&hbFontData->m_paint).getGlyphWidthAndExtentsForHarfBuzz(glyph, 0, extents);
     return true;
 }
 
@@ -373,6 +343,7 @@ PassRefPtr<HbFontCacheEntry> createHbFontCacheEntry(hb_face_t* face)
 hb_font_t* HarfBuzzFace::getScaledFont(PassRefPtr<UnicodeRangeSet> rangeSet) const
 {
     m_platformData->setupPaint(&m_harfBuzzFontData->m_paint);
+    m_harfBuzzFontData->m_paint.setTextEncoding(SkPaint::kGlyphID_TextEncoding);
     m_harfBuzzFontData->m_rangeSet = rangeSet;
     m_harfBuzzFontData->m_simpleFontData = FontCache::fontCache()->fontDataFromFontPlatformData(m_platformData).get();
     ASSERT(m_harfBuzzFontData->m_simpleFontData);

third_party/WebKit/Source/platform/fonts/skia/SkiaTextMetrics.cpp

+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "SkiaTextMetrics.h"
+
+#include "wtf/MathExtras.h"
+
+#include <SkPath.h>
+
+namespace blink {
+
+static hb_position_t SkiaScalarToHarfBuzzPosition(SkScalar value)
+{
+    // We treat HarfBuzz hb_position_t as 16.16 fixed-point.
+    static const int kHbPosition1 = 1 << 16;
+    return clampTo<int>(value * kHbPosition1);
+}
+
+SkiaTextMetrics::SkiaTextMetrics(const SkPaint* paint)
+    : m_paint(paint)
+{
+    CHECK(m_paint->getTextEncoding() == SkPaint::kGlyphID_TextEncoding);
+}
+
+void SkiaTextMetrics::getGlyphWidthAndExtentsForHarfBuzz(hb_codepoint_t codepoint, hb_position_t* width, hb_glyph_extents_t* extents)
+{
+    DCHECK_LE(codepoint, 0xFFFFu);
+
+    SkScalar skWidth;
+    SkRect skBounds;
+    uint16_t glyph = codepoint;
+
+    m_paint->getTextWidths(&glyph, sizeof(glyph), &skWidth, &skBounds);
+    if (width) {
+        if (!m_paint->isSubpixelText())
+            skWidth = SkScalarRoundToInt(skWidth);
+        *width = SkiaScalarToHarfBuzzPosition(skWidth);
+    }
+    if (extents) {
+        if (!m_paint->isSubpixelText()) {
+            // Use roundOut() rather than round() to avoid rendering glyphs
+            // outside the visual overflow rect. crbug.com/452914.
+            SkIRect ir;
+            skBounds.roundOut(&ir);
+            skBounds.set(ir);
+        }
+        // Invert y-axis because Skia is y-grows-down but we set up HarfBuzz to be y-grows-up.
+        extents->x_bearing = SkiaScalarToHarfBuzzPosition(skBounds.fLeft);
+        extents->y_bearing = SkiaScalarToHarfBuzzPosition(-skBounds.fTop);
+        extents->width = SkiaScalarToHarfBuzzPosition(skBounds.width());
+        extents->height = SkiaScalarToHarfBuzzPosition(-skBounds.height());
+    }
+}
+
+void SkiaTextMetrics::getSkiaBoundsForGlyph(Glyph glyph, SkRect *bounds)
+{
+#if OS(MACOSX)
+    // TODO(drott): Remove this once we have better metrics bounds
+    // on Mac, https://bugs.chromium.org/p/skia/issues/detail?id=5328
+    SkPath path;
+    m_paint->getTextPath(&glyph, sizeof(glyph), 0, 0, &path);
+    *bounds = path.getBounds();
+#else
+    m_paint->getTextWidths(&glyph, sizeof(glyph), nullptr, bounds);
+#endif
+
+    if (!m_paint->isSubpixelText()) {
+        SkIRect ir;
+        bounds->roundOut(&ir);
+        bounds->set(ir);
+    }
+
+}
+
+float SkiaTextMetrics::getSkiaWidthForGlyph(Glyph glyph)
+{
+    SkScalar skWidth;
+    m_paint->getTextWidths(&glyph, sizeof(glyph), &skWidth, nullptr);
+
+    if (!m_paint->isSubpixelText())
+        skWidth = SkScalarRoundToInt(skWidth);
+
+    return SkScalarToFloat(skWidth);
+}
+
+
+} // namespace blink

third_party/WebKit/Source/platform/fonts/skia/SkiaTextMetrics.h

+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef SkiaTextMetrics_h
+#define SkiaTextMetrics_h
+
+#include "platform/fonts/Glyph.h"
+
+#include <SkPaint.h>
+#include <hb.h>
+
+namespace blink {
+
+class SkiaTextMetrics final {
+public:
+    SkiaTextMetrics(const SkPaint*);
+
+    void getGlyphWidthAndExtentsForHarfBuzz(hb_codepoint_t, hb_position_t* width, hb_glyph_extents_t*);
+
+    void getSkiaBoundsForGlyph(Glyph, SkRect* bounds);
+    float getSkiaWidthForGlyph(Glyph);
+
+private:
+    const SkPaint* m_paint;
+};
+
+} // namespace blink
+
+#endif