Attachment 287047 Details for Bug 161119 – WIP

[patch] WIP

bug-161119-20160825170323.patch (text/plain), 41.63 KB, created by Myles C. Maxfield on 2016-08-25 17:05:26 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Myles C. Maxfield

Created: 2016-08-25 17:05:26 PDT

Size: 41.63 KB

patch

obsolete

>Subversion Revision: 204870
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index c1689393e5697d60e7af3a601171abe26e0c8977..1b8efc18781411c7e3af00cb7c385b414db8d18e 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,30 @@
>+2016-08-23  Myles C. Maxfield  <mmaxfield@apple.com>
>+
>+        [Cocoa] Improve performance of glyph advance metrics gathering
>+        https://bugs.webkit.org/show_bug.cgi?id=161119
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        No new tests (OOPS!).
>+
>+        * platform/graphics/Font.cpp:
>+        (WebCore::Font::platformGlyphInit):
>+        * platform/graphics/Font.h:
>+        (WebCore::Font::widthForGlyph):
>+        * platform/graphics/GlyphPage.h:
>+        (WebCore::GlyphData::GlyphData):
>+        (WebCore::GlyphPage::advanceForCharacter):
>+        (WebCore::GlyphPage::glyphDataForIndex):
>+        (WebCore::GlyphPage::advanceForIndex):
>+        (WebCore::GlyphPage::setGlyphForIndex):
>+        * platform/graphics/WidthIterator.cpp:
>+        (WebCore::WidthIterator::advanceInternal):
>+        * platform/graphics/cocoa/FontCocoa.mm:
>+        (WebCore::Font::platformWidthForGlyph):
>+        * platform/graphics/mac/GlyphPageMac.cpp:
>+        (WebCore::GlyphPage::fill):
>+        * platform/spi/cocoa/CoreTextSPI.h:
>+
> 2016-08-23  Alex Christensen  <achristensen@webkit.org>
> 
>         Fix clean Windows build.
>diff --git a/Source/WebCore/platform/graphics/Font.cpp b/Source/WebCore/platform/graphics/Font.cpp
>index f9914e50fb7cad2c709d2acc98b772b4f27a4c75..f0619b29e3b480aa780b57a0877ea08a76979d60 100644
>--- a/Source/WebCore/platform/graphics/Font.cpp
>+++ b/Source/WebCore/platform/graphics/Font.cpp
>@@ -81,12 +81,11 @@ Font::Font(const FontPlatformData& platformData, bool isCustomFont, bool isLoadi
> // Estimates of avgCharWidth and maxCharWidth for platforms that don't support accessing these values from the font.
> void Font::initCharWidths()
> {
>-    auto* glyphPageZero = glyphPage(0);
>+    auto* glyphPageZero = glyphPage('0' / GlyphPage::size);
> 
>     // Treat the width of a '0' as the avgCharWidth.
>     if (m_avgCharWidth <= 0.f && glyphPageZero) {
>-        static const UChar32 digitZeroChar = '0';
>-        Glyph digitZeroGlyph = glyphPageZero->glyphDataForCharacter(digitZeroChar).glyph;
>+        Glyph digitZeroGlyph = glyphPageZero->glyphDataForCharacter('0').glyph;
>         if (digitZeroGlyph)
>             m_avgCharWidth = widthForGlyph(digitZeroGlyph);
>     }
>@@ -102,7 +101,9 @@ void Font::initCharWidths()
> void Font::platformGlyphInit()
> {
>     auto* glyphPageZero = glyphPage(0);
>-    if (!glyphPageZero) {
>+    auto* glyphPageCharacterZero = glyphPage('0' / GlyphPage::size);
>+    auto* glyphPageSpace = glyphPage(space / GlyphPage::size);
>+    if (!glyphPageZero || !glyphPageCharacterZero || !glyphPageSpace) {
>         determinePitch();
>         return;
>     }
>@@ -114,10 +115,10 @@ void Font::platformGlyphInit()
>     // Nasty hack to determine if we should round or ceil space widths.
>     // If the font is monospace or fake monospace we ceil to ensure that 
>     // every character and the space are the same width. Otherwise we round.
>-    m_spaceGlyph = glyphPageZero->glyphDataForCharacter(' ').glyph;
>+    m_spaceGlyph = glyphPageSpace->glyphDataForCharacter(space).glyph;
>     float width = widthForGlyph(m_spaceGlyph);
>     m_spaceWidth = width;
>-    m_zeroGlyph = glyphPageZero->glyphDataForCharacter('0').glyph;
>+    m_zeroGlyph = glyphPageCharacterZero->glyphDataForCharacter('0').glyph;
>     m_fontMetrics.setZeroWidth(widthForGlyph(m_zeroGlyph));
>     determinePitch();
>     m_adjustedSpaceWidth = m_treatAsFixedPitch ? ceilf(width) : roundf(width);
>@@ -152,7 +153,7 @@ static RefPtr<GlyphPage> createAndFillGlyphPage(unsigned pageNumber, const Font&
>     // FIXME: Times New Roman contains Arabic glyphs, but Core Text doesn't know how to shape them. See <rdar://problem/9823975>.
>     // Once we have the fix for <rdar://problem/9823975> then remove this code together with Font::shouldNotBeUsedForArabic()
>     // in <rdar://problem/12096835>.
>-    if (pageNumber == 6 && font.shouldNotBeUsedForArabic())
>+    if (pageNumber >= 0x60 && pageNumber < 0x70 && && font.shouldNotBeUsedForArabic())
>         return nullptr;
> #endif
> 
>@@ -165,33 +166,40 @@ static RefPtr<GlyphPage> createAndFillGlyphPage(unsigned pageNumber, const Font&
>         for (unsigned i = 0; i < GlyphPage::size; i++)
>             buffer[i] = start + i;
> 
>-        if (!start) {
>+        static_assert(GlyphPage::size == 16 && !(GlyphPage::size & (GlyphPage::size - 1)), "createAndFillGlyphPage() assumes GlyphPage::size is 16.");
>+        if (!start || start == 1) {
>             // Control characters must not render at all.
>-            for (unsigned i = 0; i < 0x20; ++i)
>-                buffer[i] = zeroWidthSpace;
>-            for (unsigned i = 0x7F; i < 0xA0; i++)
>+            for (unsigned i = 0; i < 0x10; ++i)
>                 buffer[i] = zeroWidthSpace;
>-            buffer[softHyphen] = zeroWidthSpace;
>-
>             // \n, \t, and nonbreaking space must render as a space.
>-            buffer[(int)'\n'] = ' ';
>-            buffer[(int)'\t'] = ' ';
>-            buffer[noBreakSpace] = ' ';
>+            buffer[(int)'\t'] = space;
>+            buffer[(int)'\n'] = space;
>+        } else if (start == (0x7F & ~(GlyphPage::size - 1))) {
>+            buffer[0x7F - start] = zeroWidthSpace;
>+        } else if (pageNumber >= 8 && pageNumber < 10) {
>+            for (unsigned i = 0; i < 0x10; ++i)
>+                buffer[i] = zeroWidthSpace;
>+        } else if (pageNumber == 10) {
>+            buffer[softHyphen - start] = zeroWidthSpace;
>+            buffer[noBreakSpace - start] = space;
>         } else if (start == (leftToRightMark & ~(GlyphPage::size - 1))) {
>             // LRM, RLM, LRE, RLE, ZWNJ, ZWJ, and PDF must not render at all.
>             buffer[leftToRightMark - start] = zeroWidthSpace;
>             buffer[rightToLeftMark - start] = zeroWidthSpace;
>+        } else if (start == (leftToRightEmbed & ~(GlyphPage::size - 1))) {
>             buffer[leftToRightEmbed - start] = zeroWidthSpace;
>             buffer[rightToLeftEmbed - start] = zeroWidthSpace;
>             buffer[leftToRightOverride - start] = zeroWidthSpace;
>             buffer[rightToLeftOverride - start] = zeroWidthSpace;
>+            buffer[popDirectionalFormatting - start] = zeroWidthSpace;
>+        } else if (start == (leftToRightIsolate & ~(GlyphPage::size - 1))) {
>             buffer[leftToRightIsolate - start] = zeroWidthSpace;
>             buffer[rightToLeftIsolate - start] = zeroWidthSpace;
>-            buffer[zeroWidthNonJoiner - start] = zeroWidthSpace;
>-            buffer[zeroWidthJoiner - start] = zeroWidthSpace;
>-            buffer[popDirectionalFormatting - start] = zeroWidthSpace;
>             buffer[popDirectionalIsolate - start] = zeroWidthSpace;
>             buffer[firstStrongIsolate - start] = zeroWidthSpace;
>+        } else if (start == (zeroWidthNonJoiner & ~(GlyphPage::size - 1))) {
>+            buffer[zeroWidthNonJoiner - start] = zeroWidthSpace;
>+            buffer[zeroWidthJoiner - start] = zeroWidthSpace;
>         } else if (start == (objectReplacementCharacter & ~(GlyphPage::size - 1))) {
>             // Object replacement character must not render at all.
>             buffer[objectReplacementCharacter - start] = zeroWidthSpace;
>@@ -224,11 +232,11 @@ static RefPtr<GlyphPage> createAndFillGlyphPage(unsigned pageNumber, const Font&
> 
> const GlyphPage* Font::glyphPage(unsigned pageNumber) const
> {
>-    if (!pageNumber) {
>+    /*if (!pageNumber) {
>         if (!m_glyphPageZero)
>             m_glyphPageZero = createAndFillGlyphPage(0, *this);
>         return m_glyphPageZero.get();
>-    }
>+    }*/
>     auto addResult = m_glyphPages.add(pageNumber, nullptr);
>     if (addResult.isNewEntry)
>         addResult.iterator->value = createAndFillGlyphPage(pageNumber, *this);
>diff --git a/Source/WebCore/platform/graphics/Font.h b/Source/WebCore/platform/graphics/Font.h
>index b49c4d7029d7ba0a688b9cd46947790be3cfd040..278661a82ab8604a117192b27572ce1ec3e97978 100644
>--- a/Source/WebCore/platform/graphics/Font.h
>+++ b/Source/WebCore/platform/graphics/Font.h
>@@ -227,7 +227,7 @@ private:
> 
>     const FontPlatformData m_platformData;
> 
>-    mutable RefPtr<GlyphPage> m_glyphPageZero;
>+    //mutable RefPtr<GlyphPage> m_glyphPageZero;
>     mutable HashMap<unsigned, RefPtr<GlyphPage>> m_glyphPages;
>     mutable std::unique_ptr<GlyphMetricsMap<FloatRect>> m_glyphToBoundsMap;
>     mutable GlyphMetricsMap<float> m_glyphToWidthMap;
>@@ -337,13 +337,13 @@ ALWAYS_INLINE float Font::widthForGlyph(Glyph glyph) const
>         return width;
> 
> #if ENABLE(OPENTYPE_VERTICAL)
>-    if (m_verticalData)
>+    if (m_verticalData) {
> #if USE(CG) || USE(CAIRO)
>         width = m_verticalData->advanceHeight(this, glyph) + m_syntheticBoldOffset;
> #else
>         width = m_verticalData->advanceHeight(this, glyph);
> #endif
>-    else
>+    } else
> #endif
>         width = platformWidthForGlyph(glyph);
> 
>diff --git a/Source/WebCore/platform/graphics/FontCascadeFonts.cpp b/Source/WebCore/platform/graphics/FontCascadeFonts.cpp
>index d36ed09274e3b7da852bea2caf9482bd4e1c8ef2..749cb43f0bd57a774c9a8b0dec830c3ba3dac2a9 100644
>--- a/Source/WebCore/platform/graphics/FontCascadeFonts.cpp
>+++ b/Source/WebCore/platform/graphics/FontCascadeFonts.cpp
>@@ -430,7 +430,7 @@ GlyphData FontCascadeFonts::glyphDataForCharacter(UChar32 c, const FontCascadeDe
> 
>     const unsigned pageNumber = c / GlyphPage::size;
> 
>-    auto& cacheEntry = pageNumber ? m_cachedPages.add(pageNumber, GlyphPageCacheEntry()).iterator->value : m_cachedPageZero;
>+    auto& cacheEntry = m_cachedPages.add(pageNumber, GlyphPageCacheEntry()).iterator->value;
> 
>     // Initialize cache with a full page of glyph mappings from a single font.
>     if (cacheEntry.isNull())
>@@ -452,8 +452,6 @@ void FontCascadeFonts::pruneSystemFallbacks()
>     if (m_systemFallbackFontSet.isEmpty())
>         return;
>     // Mutable glyph pages may reference fallback fonts.
>-    if (m_cachedPageZero.isMixedFont())
>-        m_cachedPageZero = { };
>     m_cachedPages.removeIf([](auto& keyAndValue) {
>         return keyAndValue.value.isMixedFont();
>     });
>diff --git a/Source/WebCore/platform/graphics/FontCascadeFonts.h b/Source/WebCore/platform/graphics/FontCascadeFonts.h
>index fcc1c37a9a18eebd682856083bbd7d3a9324bc4d..c08a133a7ec73008b5fc4f1fe61b13790eeabd8f 100644
>--- a/Source/WebCore/platform/graphics/FontCascadeFonts.h
>+++ b/Source/WebCore/platform/graphics/FontCascadeFonts.h
>@@ -99,7 +99,7 @@ private:
>         std::unique_ptr<MixedFontGlyphPage> m_mixedFont;
>     };
> 
>-    GlyphPageCacheEntry m_cachedPageZero;
>+    //GlyphPageCacheEntry m_cachedPageZero;
>     HashMap<int, GlyphPageCacheEntry> m_cachedPages;
> 
>     HashSet<RefPtr<Font>> m_systemFallbackFontSet;
>diff --git a/Source/WebCore/platform/graphics/GlyphMetricsMap.h b/Source/WebCore/platform/graphics/GlyphMetricsMap.h
>index 730c1d3a6124a920eecd6fa2226cf2d752daa1c4..4f0f58a587216068771c4a47bcf8d92ba5f5899b 100644
>--- a/Source/WebCore/platform/graphics/GlyphMetricsMap.h
>+++ b/Source/WebCore/platform/graphics/GlyphMetricsMap.h
>@@ -54,7 +54,7 @@ private:
>     class GlyphMetricsPage {
>         WTF_MAKE_FAST_ALLOCATED;
>     public:
>-        static const size_t size = 256; // Usually covers Latin-1 in a single page.
>+        static const size_t size = 16;
> 
>         GlyphMetricsPage() = default;
>         explicit GlyphMetricsPage(const T& initialValue)
>diff --git a/Source/WebCore/platform/graphics/GlyphPage.h b/Source/WebCore/platform/graphics/GlyphPage.h
>index 99e618be93b3879067c2d67574b926ce61f46ed8..801f636e0daf29c2411e862980f7b15459a0fb60 100644
>--- a/Source/WebCore/platform/graphics/GlyphPage.h
>+++ b/Source/WebCore/platform/graphics/GlyphPage.h
>@@ -31,10 +31,13 @@
> #define GlyphPage_h
> 
> #include "Glyph.h"
>+#include <array>
> #include <string.h>
> #include <unicode/utypes.h>
>+#include <wtf/Optional.h>
> #include <wtf/RefCounted.h>
> #include <wtf/Ref.h>
>+#include <wtf/Vector.h>
> 
> namespace WebCore {
> 
>@@ -43,9 +46,10 @@ class Font;
> // Holds the glyph index and the corresponding Font information for a given
> // character.
> struct GlyphData {
>-    GlyphData(Glyph g = 0, const Font* f = 0)
>+    GlyphData(Glyph g = 0, const Font* f = nullptr, Optional<float> advance = Nullopt)
>         : glyph(g)
>         , font(f)
>+        , advance(advance)
>     {
>     }
> 
>@@ -53,6 +57,7 @@ struct GlyphData {
> 
>     Glyph glyph;
>     const Font* font;
>+    Optional<float> advance;
> };
> 
> // A GlyphPage contains a fixed-size set of GlyphData mappings for a contiguous
>@@ -72,7 +77,7 @@ public:
> 
>     static unsigned count() { return s_count; }
> 
>-    static const size_t size = 256; // Covers Latin-1 in a single page.
>+    static constexpr size_t size = 16;
>     static unsigned indexForCharacter(UChar32 c) { return c % GlyphPage::size; }
> 
>     GlyphData glyphDataForCharacter(UChar32 c) const
>@@ -85,10 +90,15 @@ public:
>         return glyphForIndex(indexForCharacter(c));
>     }
> 
>+    Optional<float> advanceForCharacter(UChar32 c) const
>+    {
>+        return advanceForIndex(indexForCharacter(c));
>+    }
>+
>     GlyphData glyphDataForIndex(unsigned index) const
>     {
>         Glyph glyph = glyphForIndex(index);
>-        return GlyphData(glyph, glyph ? &m_font : nullptr);
>+        return GlyphData(glyph, glyph ? &m_font : nullptr, advanceForIndex(index));
>     }
> 
>     Glyph glyphForIndex(unsigned index) const
>@@ -97,11 +107,24 @@ public:
>         return m_glyphs[index];
>     }
> 
>+    Optional<float> advanceForIndex(unsigned index) const
>+    {
>+        ASSERT_WITH_SECURITY_IMPLICATION(index < size);
>+        if (m_advances.isEmpty())
>+            return Nullopt;
>+        return m_advances[index];
>+    }
>+
>     // FIXME: Pages are immutable after initialization. This should be private.
>-    void setGlyphForIndex(unsigned index, Glyph glyph)
>+    void setGlyphForIndex(unsigned index, Glyph glyph, Optional<float> advance = Nullopt)
>     {
>         ASSERT_WITH_SECURITY_IMPLICATION(index < size);
>         m_glyphs[index] = glyph;
>+        if (advance) {
>+            if (m_advances.isEmpty())
>+                m_advances.fill(0, size);
>+            m_advances[index] = advance.value();
>+        }
>     }
> 
>     const Font& font() const
>@@ -120,7 +143,8 @@ private:
>     }
> 
>     const Font& m_font;
>-    Glyph m_glyphs[size] { };
>+    std::array<Glyph, size> m_glyphs;
>+    Vector<float> m_advances;
> 
>     WEBCORE_EXPORT static unsigned s_count;
> };
>diff --git a/Source/WebCore/platform/graphics/WidthIterator.cpp b/Source/WebCore/platform/graphics/WidthIterator.cpp
>index 36fba0f9d06f87976e5aed0ba09aa66a59a507cd..3bae7712d7b7ca9211375e15d7c5428cf331ccb6 100644
>--- a/Source/WebCore/platform/graphics/WidthIterator.cpp
>+++ b/Source/WebCore/platform/graphics/WidthIterator.cpp
>@@ -207,7 +207,9 @@ inline unsigned WidthIterator::advanceInternal(TextIterator& textIterator, Glyph
>         if (character == '\t' && m_run.allowTabs())
>             width = m_font->tabWidth(*font, m_run.tabSize(), m_run.xPos() + m_runWidthSoFar + widthSinceLastRounding);
>         else {
>-            width = font->widthForGlyph(glyph);
>+            width = glyphData.advance.valueOrCompute([&] {
>+                return font->widthForGlyph(glyph);
>+            });
> 
>             // SVG uses horizontalGlyphStretch(), when textLength is used to stretch/squeeze text.
>             width *= m_run.horizontalGlyphStretch();
>diff --git a/Source/WebCore/platform/graphics/cocoa/FontCocoa.mm b/Source/WebCore/platform/graphics/cocoa/FontCocoa.mm
>index f3001f5061ca1d87870bb690ecdf5563c59785a1..51ce9db3091c3cd666b8f0b8977decce876a01cb 100644
>--- a/Source/WebCore/platform/graphics/cocoa/FontCocoa.mm
>+++ b/Source/WebCore/platform/graphics/cocoa/FontCocoa.mm
>@@ -596,44 +596,11 @@ FloatRect Font::platformBoundsForGlyph(Glyph glyph) const
>     return boundingBox;
> }
> 
>-static inline Optional<CGSize> advanceForColorBitmapFont(const FontPlatformData& platformData, Glyph glyph)
>-{
>-    CTFontRef font = platformData.font();
>-    if (!font || !platformData.isColorBitmapFont())
>-        return Nullopt;
>-    CGSize advance;
>-    CTFontGetAdvancesForGlyphs(font, kCTFontOrientationDefault, &glyph, &advance, 1);
>-    return advance;
>-}
>-
>-static inline bool canUseFastGlyphAdvanceGetter(const FontPlatformData& platformData, Glyph glyph, CGSize& advance, bool& populatedAdvance)
>-{
>-    if (platformData.isEmoji() || platformData.hasCustomTracking() || platformData.textRenderingMode() == OptimizeLegibility)
>-        return false;
>-    if (auto size = advanceForColorBitmapFont(platformData, glyph)) {
>-        populatedAdvance = true;
>-        advance = size.value();
>-        return false;
>-    }
>-    return true;
>-}
>-
> float Font::platformWidthForGlyph(Glyph glyph) const
> {
>     CGSize advance = CGSizeZero;
>     bool horizontal = platformData().orientation() == Horizontal;
>-    bool populatedAdvance = false;
>-    if ((horizontal || m_isBrokenIdeographFallback) && canUseFastGlyphAdvanceGetter(this->platformData(), glyph, advance, populatedAdvance)) {
>-        float pointSize = platformData().size();
>-        CGAffineTransform m = CGAffineTransformMakeScale(pointSize, pointSize);
>-        CGFontRenderingStyle style = kCGFontRenderingStyleAntialiasing | kCGFontRenderingStyleSubpixelPositioning | kCGFontRenderingStyleSubpixelQuantization | kCGFontAntialiasingStyleUnfiltered;
>-        if (!CGFontGetGlyphAdvancesForStyle(platformData().cgFont(), &m, style, &glyph, 1, &advance)) {
>-            RetainPtr<CFStringRef> fullName = adoptCF(CGFontCopyFullName(platformData().cgFont()));
>-            LOG_ERROR("Unable to cache glyph widths for %@ %f", fullName.get(), pointSize);
>-            advance.width = 0;
>-        }
>-    } else if (!populatedAdvance && platformData().size())
>-        CTFontGetAdvancesForGlyphs(m_platformData.ctFont(), horizontal ? kCTFontOrientationHorizontal : kCTFontOrientationVertical, &glyph, &advance, 1);
>+    CTFontGetAdvancesForGlyphs(m_platformData.ctFont(), horizontal ? kCTFontOrientationHorizontal : kCTFontOrientationVertical, &glyph, &advance, 1);
> 
>     return advance.width + m_syntheticBoldOffset;
> }
>diff --git a/Source/WebCore/platform/graphics/mac/GlyphPageMac.cpp b/Source/WebCore/platform/graphics/mac/GlyphPageMac.cpp
>index f39c8dc059f52866a26699c744c074f1471c38c1..cd71e39d09153b653ddf5aab75081b87a0bc078e 100644
>--- a/Source/WebCore/platform/graphics/mac/GlyphPageMac.cpp
>+++ b/Source/WebCore/platform/graphics/mac/GlyphPageMac.cpp
>@@ -63,16 +63,11 @@ bool GlyphPage::fill(UChar* buffer, unsigned bufferLength)
>     ASSERT(bufferLength == GlyphPage::size || bufferLength == 2 * GlyphPage::size);
> 
>     const Font& font = this->font();
>-    Vector<CGGlyph, 512> glyphs(bufferLength);
>-    unsigned glyphStep;
>-    if (!shouldUseCoreText(buffer, bufferLength, font)) {
>-        // We pass in either 256 or 512 UTF-16 characters: 256 for U+FFFF and less, 512 (double character surrogates)
>-        // for U+10000 and above. It is indeed possible to get back 512 glyphs back from the API, so the glyph buffer
>-        // we pass in must be 512. If we get back more than 256 glyphs though we'll ignore all the ones after 256,
>-        // this should not happen as the only time we pass in 512 characters is when they are surrogates.
>+    Vector<CGGlyph> glyphs(bufferLength);
>+    memset(glyphs.data(), 0, sizeof(CGGlyph) * bufferLength);
>+    if (!shouldUseCoreText(buffer, bufferLength, font))
>         CGFontGetGlyphsForUnichars(font.platformData().cgFont(), buffer, glyphs.data(), bufferLength);
>-        glyphStep = 1;
>-    } else {
>+    else {
>         // Because we know the implementation of shouldUseCoreText(), if the font isn't for text combine and it isn't a system font,
>         // we know it must have vertical glyphs.
>         if (font.platformData().isForTextCombine() || font.platformData().isSystemFont())
>@@ -82,13 +77,22 @@ bool GlyphPage::fill(UChar* buffer, unsigned bufferLength)
> 
>         // When buffer consists of surrogate pairs, CTFontGetVerticalGlyphsForCharacters and CTFontGetGlyphsForCharacters
>         // place the glyphs at indices corresponding to the first character of each pair.
>-        glyphStep = bufferLength / GlyphPage::size;
>+        unsigned glyphStep = bufferLength / GlyphPage::size;
>+        if (glyphStep != 1) {
>+            for (unsigned i = 0; i < GlyphPage::size; ++i)
>+                glyphs[i] = glyphs[i * glyphStep];
>+        }
>     }
> 
>+    Vector<CGSize> advances(GlyphPage::size);
>+    CTFontOrientation orientation = font.platformData().orientation() == Horizontal ? kCTFontOrientationHorizontal : kCTFontOrientationVertical;
>+    CGFontRenderingStyle style = kCGFontRenderingStyleAntialiasing | kCGFontRenderingStyleSubpixelPositioning | kCGFontRenderingStyleSubpixelQuantization | kCGFontAntialiasingStyleUnfiltered;
>+    CTFontGetUnsummedAdvancesForGlyphsAndStyle(font.platformData().ctFont(), orientation, style, glyphs.data(), advances.data(), GlyphPage::size);
>+
>     bool haveGlyphs = false;
>     for (unsigned i = 0; i < GlyphPage::size; ++i) {
>-        if (glyphs[i * glyphStep]) {
>-            setGlyphForIndex(i, glyphs[i * glyphStep]);
>+        if (glyphs[i]) {
>+            setGlyphForIndex(i, glyphs[i], advances[i].width);
>             haveGlyphs = true;
>         }
>     }
>diff --git a/Source/WebCore/platform/spi/cocoa/CoreTextSPI.h b/Source/WebCore/platform/spi/cocoa/CoreTextSPI.h
>index 08d510dbf3b9d7a35a81b05ddae410041dd19237..a21f48af96bb9e11230c55274418d4312d059ce2 100644
>--- a/Source/WebCore/platform/spi/cocoa/CoreTextSPI.h
>+++ b/Source/WebCore/platform/spi/cocoa/CoreTextSPI.h
>@@ -42,6 +42,18 @@ enum {
>     kCTFontUIFontSystemUltraLight = 104,
> };
> 
>+typedef CF_OPTIONS(uint32_t, CTFontTransformOptions)
>+{
>+    kCTFontTransformApplyShaping = (1 << 0),
>+    kCTFontTransformApplyPositioning = (1 << 1)
>+};
>+
>+typedef CF_OPTIONS(uint32_t, CTFontDescriptorOptions)
>+{
>+    kCTFontDescriptorOptionSystemUIFont = 1 << 1,
>+    kCTFontDescriptorOptionPreferAppleSystemFont = kCTFontOptionsPreferSystemFont
>+};
>+
> #endif
> 
> extern "C" {
>@@ -52,47 +64,28 @@ typedef void (*CTUniCharDisposeCallback)(const UniChar* chars, void* refCon);
> extern const CFStringRef kCTFontReferenceURLAttribute;
> extern const CFStringRef kCTFontOpticalSizeAttribute;
> 
>-#if PLATFORM(COCOA)
>-#if !USE(APPLE_INTERNAL_SDK)
>-typedef CF_OPTIONS(uint32_t, CTFontTransformOptions)
>-{
>-    kCTFontTransformApplyShaping = (1 << 0),
>-    kCTFontTransformApplyPositioning = (1 << 1)
>-};
>-#endif
> bool CTFontTransformGlyphs(CTFontRef, CGGlyph glyphs[], CGSize advances[], CFIndex count, CTFontTransformOptions);
>-#endif
> 
> CGSize CTRunGetInitialAdvance(CTRunRef run);
> CTLineRef CTLineCreateWithUniCharProvider(CTUniCharProviderCallback provide, CTUniCharDisposeCallback dispose, void* refCon);
> CTTypesetterRef CTTypesetterCreateWithUniCharProviderAndOptions(CTUniCharProviderCallback provide, CTUniCharDisposeCallback dispose, void* refCon, CFDictionaryRef options);
> bool CTFontGetVerticalGlyphsForCharacters(CTFontRef, const UniChar characters[], CGGlyph glyphs[], CFIndex count);
>+void CTFontGetUnsummedAdvancesForGlyphsAndStyle(CTFontRef, CTFontOrientation, CGFontRenderingStyle, const CGGlyph[], CGSize advances[], CFIndex count);
> 
> CTFontDescriptorRef CTFontDescriptorCreateForUIType(CTFontUIFontType, CGFloat size, CFStringRef language);
> CTFontDescriptorRef CTFontDescriptorCreateWithTextStyle(CFStringRef style, CFStringRef size, CFStringRef language);
> CTFontDescriptorRef CTFontDescriptorCreateCopyWithSymbolicTraits(CTFontDescriptorRef original, CTFontSymbolicTraits symTraitValue, CTFontSymbolicTraits symTraitMask);
> CFBitVectorRef CTFontCopyGlyphCoverageForFeature(CTFontRef, CFDictionaryRef feature);
> 
>-#if PLATFORM(COCOA)
>-#if !USE(APPLE_INTERNAL_SDK)
>-typedef CF_OPTIONS(uint32_t, CTFontDescriptorOptions)
>-{
>-    kCTFontDescriptorOptionSystemUIFont = 1 << 1,
>-    kCTFontDescriptorOptionPreferAppleSystemFont = kCTFontOptionsPreferSystemFont
>-};
>-
> CTFontDescriptorRef CTFontDescriptorCreateWithAttributesAndOptions(CFDictionaryRef attributes, CTFontDescriptorOptions);
> 
> extern const CFStringRef kCTFontDescriptorTextStyleAttribute;
> extern const CFStringRef kCTFontUIFontDesignTrait;
>-#endif
>-#endif
> 
> bool CTFontDescriptorIsSystemUIFont(CTFontDescriptorRef);
> CTFontRef CTFontCreateForCSS(CFStringRef name, uint16_t weight, CTFontSymbolicTraits, CGFloat size);
> CTFontRef CTFontCreateForCharactersWithLanguage(CTFontRef currentFont, const UTF16Char *characters, CFIndex length, CFStringRef language, CFIndex *coveredLength);
> 
>-#if PLATFORM(COCOA)
> extern const CFStringRef kCTUIFontTextStyleShortHeadline;
> extern const CFStringRef kCTUIFontTextStyleShortBody;
> extern const CFStringRef kCTUIFontTextStyleShortSubhead;
>@@ -118,14 +111,11 @@ extern const CGFloat kCTFontWeightSemibold;
> extern const CGFloat kCTFontWeightBold;
> extern const CGFloat kCTFontWeightHeavy;
> extern const CGFloat kCTFontWeightBlack;
>-#endif
> 
>-#if PLATFORM(IOS)
> extern const CFStringRef kCTUIFontTextStyleTitle1;
> extern const CFStringRef kCTUIFontTextStyleTitle2;
> extern const CFStringRef kCTUIFontTextStyleTitle3;
> CTFontDescriptorRef CTFontCreatePhysicalFontDescriptorForCharactersWithLanguage(CTFontRef currentFont, const UTF16Char* characters, CFIndex length, CFStringRef language, CFIndex* coveredLength);
>-#endif
> 
> CTFontRef CTFontCreatePhysicalFontForCharactersWithLanguage(CTFontRef, const UTF16Char* characters, CFIndex length, CFStringRef language, CFIndex* coveredLength);
> bool CTFontIsAppleColorEmoji(CTFontRef);
>diff --git a/Source/WebCore/rendering/mathml/MathOperator.cpp b/Source/WebCore/rendering/mathml/MathOperator.cpp
>index 4fb8f6aa8b21b774ebf9334f5d019597be65dfa4..c7081689edca3876a45b8de8506af3159d02a812 100644
>--- a/Source/WebCore/rendering/mathml/MathOperator.cpp
>+++ b/Source/WebCore/rendering/mathml/MathOperator.cpp
>@@ -94,7 +94,7 @@ void MathOperator::setOperator(const RenderStyle& style, UChar baseCharacter, Ty
> 
> void MathOperator::reset(const RenderStyle& style)
> {
>-    m_stretchType = StretchType::Unstretched;
>+    m_variantOrAssembly = Nullopt;
>     m_maxPreferredWidth = 0;
>     m_width = 0;
>     m_ascent = 0;
>@@ -131,8 +131,7 @@ void MathOperator::setSizeVariant(const GlyphData& sizeVariant)
> {
>     ASSERT(sizeVariant.font);
>     ASSERT(sizeVariant.font->mathData());
>-    m_stretchType = StretchType::SizeVariant;
>-    m_variant = sizeVariant;
>+    m_variantOrAssembly = GlyphDataOrGlyphAssemblyData(sizeVariant);
>     m_width = advanceWidthForGlyph(sizeVariant);
>     getAscentAndDescentForGlyph(sizeVariant, m_ascent, m_descent);
> }
>@@ -140,28 +139,27 @@ void MathOperator::setSizeVariant(const GlyphData& sizeVariant)
> void MathOperator::setGlyphAssembly(const GlyphAssemblyData& assemblyData)
> {
>     ASSERT(m_operatorType == Type::VerticalOperator || m_operatorType == Type::HorizontalOperator);
>-    m_stretchType = StretchType::GlyphAssembly;
>-    m_assembly = assemblyData;
>+    m_variantOrAssembly = GlyphDataOrGlyphAssemblyData(assemblyData);
>     if (m_operatorType == Type::VerticalOperator) {
>         m_width = 0;
>-        m_width = std::max<LayoutUnit>(m_width, advanceWidthForGlyph(m_assembly.topOrRight));
>-        m_width = std::max<LayoutUnit>(m_width, advanceWidthForGlyph(m_assembly.extension));
>-        m_width = std::max<LayoutUnit>(m_width, advanceWidthForGlyph(m_assembly.bottomOrLeft));
>-        m_width = std::max<LayoutUnit>(m_width, advanceWidthForGlyph(m_assembly.middle));
>+        m_width = std::max<LayoutUnit>(m_width, advanceWidthForGlyph(assemblyData.topOrRight));
>+        m_width = std::max<LayoutUnit>(m_width, advanceWidthForGlyph(assemblyData.extension));
>+        m_width = std::max<LayoutUnit>(m_width, advanceWidthForGlyph(assemblyData.bottomOrLeft));
>+        m_width = std::max<LayoutUnit>(m_width, advanceWidthForGlyph(assemblyData.middle));
>     } else {
>         m_ascent = 0;
>         m_descent = 0;
>         LayoutUnit ascent, descent;
>-        getAscentAndDescentForGlyph(m_assembly.bottomOrLeft, ascent, descent);
>+        getAscentAndDescentForGlyph(assemblyData.bottomOrLeft, ascent, descent);
>         m_ascent = std::max(m_ascent, ascent);
>         m_descent = std::max(m_descent, descent);
>-        getAscentAndDescentForGlyph(m_assembly.extension, ascent, descent);
>+        getAscentAndDescentForGlyph(assemblyData.extension, ascent, descent);
>         m_ascent = std::max(m_ascent, ascent);
>         m_descent = std::max(m_descent, descent);
>-        getAscentAndDescentForGlyph(m_assembly.topOrRight, ascent, descent);
>+        getAscentAndDescentForGlyph(assemblyData.topOrRight, ascent, descent);
>         m_ascent = std::max(m_ascent, ascent);
>         m_descent = std::max(m_descent, descent);
>-        getAscentAndDescentForGlyph(m_assembly.middle, ascent, descent);
>+        getAscentAndDescentForGlyph(assemblyData.middle, ascent, descent);
>         m_ascent = std::max(m_ascent, ascent);
>         m_descent = std::max(m_descent, descent);
>     }
>@@ -439,7 +437,7 @@ void MathOperator::stretchTo(const RenderStyle& style, LayoutUnit targetSize)
> {
>     ASSERT(m_operatorType == Type::VerticalOperator || m_operatorType == Type::HorizontalOperator);
>     calculateStretchyData(style, false, targetSize);
>-    if (m_stretchType == StretchType::GlyphAssembly) {
>+    if (isAssembly()) {
>         if (m_operatorType == Type::VerticalOperator) {
>             m_ascent = targetSize;
>             m_descent = 0;
>@@ -503,8 +501,9 @@ LayoutRect MathOperator::paintGlyph(const RenderStyle& style, PaintInfo& info, c
> void MathOperator::fillWithVerticalExtensionGlyph(const RenderStyle& style, PaintInfo& info, const LayoutPoint& from, const LayoutPoint& to)
> {
>     ASSERT(m_operatorType == Type::VerticalOperator);
>-    ASSERT(m_stretchType == StretchType::GlyphAssembly);
>-    ASSERT(m_assembly.extension.font);
>+    ASSERT(isAssembly());
>+    auto& assembly = this->assembly();
>+    ASSERT(assembly.extension.font);
>     ASSERT(from.y() <= to.y());
> 
>     // If there is no space for the extension glyph, we don't need to do anything.
>@@ -513,7 +512,7 @@ void MathOperator::fillWithVerticalExtensionGlyph(const RenderStyle& style, Pain
> 
>     GraphicsContextStateSaver stateSaver(info.context());
> 
>-    FloatRect glyphBounds = boundsForGlyph(m_assembly.extension);
>+    FloatRect glyphBounds = boundsForGlyph(assembly.extension);
> 
>     // Clipping the extender region here allows us to draw the bottom extender glyph into the
>     // regions of the bottom glyph without worrying about overdraw (hairy pixels) and simplifies later clipping.
>@@ -529,7 +528,7 @@ void MathOperator::fillWithVerticalExtensionGlyph(const RenderStyle& style, Pain
> 
>     // In practice, only small stretch sizes are requested but we limit the number of glyphs to avoid hangs.
>     for (unsigned extensionCount = 0; lastPaintedGlyphRect.maxY() < to.y() && extensionCount < kMaximumExtensionCount; extensionCount++) {
>-        lastPaintedGlyphRect = paintGlyph(style, info, m_assembly.extension, glyphOrigin, TrimTopAndBottom);
>+        lastPaintedGlyphRect = paintGlyph(style, info, assembly.extension, glyphOrigin, TrimTopAndBottom);
>         glyphOrigin.setY(glyphOrigin.y() + lastPaintedGlyphRect.height());
> 
>         // There's a chance that if the font size is small enough the glue glyph has been reduced to an empty rectangle
>@@ -542,8 +541,9 @@ void MathOperator::fillWithVerticalExtensionGlyph(const RenderStyle& style, Pain
> void MathOperator::fillWithHorizontalExtensionGlyph(const RenderStyle& style, PaintInfo& info, const LayoutPoint& from, const LayoutPoint& to)
> {
>     ASSERT(m_operatorType == Type::HorizontalOperator);
>-    ASSERT(m_stretchType == StretchType::GlyphAssembly);
>-    ASSERT(m_assembly.extension.font);
>+    ASSERT(isAssembly());
>+    auto& assembly = this->assembly();
>+    ASSERT(assembly.extension.font);
>     ASSERT(from.x() <= to.x());
>     ASSERT(from.y() == to.y());
> 
>@@ -567,7 +567,7 @@ void MathOperator::fillWithHorizontalExtensionGlyph(const RenderStyle& style, Pa
> 
>     // In practice, only small stretch sizes are requested but we limit the number of glyphs to avoid hangs.
>     for (unsigned extensionCount = 0; lastPaintedGlyphRect.maxX() < to.x() && extensionCount < kMaximumExtensionCount; extensionCount++) {
>-        lastPaintedGlyphRect = paintGlyph(style, info, m_assembly.extension, glyphOrigin, TrimLeftAndRight);
>+        lastPaintedGlyphRect = paintGlyph(style, info, assembly.extension, glyphOrigin, TrimLeftAndRight);
>         glyphOrigin.setX(glyphOrigin.x() + lastPaintedGlyphRect.width());
> 
>         // There's a chance that if the font size is small enough the glue glyph has been reduced to an empty rectangle
>@@ -580,28 +580,29 @@ void MathOperator::fillWithHorizontalExtensionGlyph(const RenderStyle& style, Pa
> void MathOperator::paintVerticalGlyphAssembly(const RenderStyle& style, PaintInfo& info, const LayoutPoint& paintOffset)
> {
>     ASSERT(m_operatorType == Type::VerticalOperator);
>-    ASSERT(m_stretchType == StretchType::GlyphAssembly);
>-    ASSERT(m_assembly.topOrRight.font);
>-    ASSERT(m_assembly.bottomOrLeft.font);
>+    ASSERT(isAssembly());
>+    auto& assembly = this->assembly();
>+    ASSERT(assembly.topOrRight.font);
>+    ASSERT(assembly.bottomOrLeft.font);
> 
>     // We are positioning the glyphs so that the edge of the tight glyph bounds line up exactly with the edges of our paint box.
>     LayoutPoint operatorTopLeft = paintOffset;
>-    FloatRect topGlyphBounds = boundsForGlyph(m_assembly.topOrRight);
>+    FloatRect topGlyphBounds = boundsForGlyph(assembly.topOrRight);
>     LayoutPoint topGlyphOrigin(operatorTopLeft.x(), operatorTopLeft.y() - topGlyphBounds.y());
>-    LayoutRect topGlyphPaintRect = paintGlyph(style, info, m_assembly.topOrRight, topGlyphOrigin, TrimBottom);
>+    LayoutRect topGlyphPaintRect = paintGlyph(style, info, assembly.topOrRight, topGlyphOrigin, TrimBottom);
> 
>-    FloatRect bottomGlyphBounds = boundsForGlyph(m_assembly.bottomOrLeft);
>+    FloatRect bottomGlyphBounds = boundsForGlyph(assembly.bottomOrLeft);
>     LayoutPoint bottomGlyphOrigin(operatorTopLeft.x(), operatorTopLeft.y() + stretchSize() - (bottomGlyphBounds.height() + bottomGlyphBounds.y()));
>-    LayoutRect bottomGlyphPaintRect = paintGlyph(style, info, m_assembly.bottomOrLeft, bottomGlyphOrigin, TrimTop);
>+    LayoutRect bottomGlyphPaintRect = paintGlyph(style, info, assembly.bottomOrLeft, bottomGlyphOrigin, TrimTop);
> 
>-    if (m_assembly.middle.font) {
>+    if (assembly.middle.font) {
>         // Center the glyph origin between the start and end glyph paint extents. Then shift it half the paint height toward the bottom glyph.
>-        FloatRect middleGlyphBounds = boundsForGlyph(m_assembly.middle);
>+        FloatRect middleGlyphBounds = boundsForGlyph(assembly.middle);
>         LayoutPoint middleGlyphOrigin(operatorTopLeft.x(), topGlyphOrigin.y());
>         middleGlyphOrigin.moveBy(LayoutPoint(0, (bottomGlyphPaintRect.y() - topGlyphPaintRect.maxY()) / 2.0));
>         middleGlyphOrigin.moveBy(LayoutPoint(0, middleGlyphBounds.height() / 2.0));
> 
>-        LayoutRect middleGlyphPaintRect = paintGlyph(style, info, m_assembly.middle, middleGlyphOrigin, TrimTopAndBottom);
>+        LayoutRect middleGlyphPaintRect = paintGlyph(style, info, assembly.middle, middleGlyphOrigin, TrimTopAndBottom);
>         fillWithVerticalExtensionGlyph(style, info, topGlyphPaintRect.minXMaxYCorner(), middleGlyphPaintRect.minXMinYCorner());
>         fillWithVerticalExtensionGlyph(style, info, middleGlyphPaintRect.minXMaxYCorner(), bottomGlyphPaintRect.minXMinYCorner());
>     } else
>@@ -611,25 +612,26 @@ void MathOperator::paintVerticalGlyphAssembly(const RenderStyle& style, PaintInf
> void MathOperator::paintHorizontalGlyphAssembly(const RenderStyle& style, PaintInfo& info, const LayoutPoint& paintOffset)
> {
>     ASSERT(m_operatorType == Type::HorizontalOperator);
>-    ASSERT(m_stretchType == StretchType::GlyphAssembly);
>-    ASSERT(m_assembly.bottomOrLeft.font);
>-    ASSERT(m_assembly.topOrRight.font);
>+    ASSERT(isAssembly());
>+    auto& assembly = this->assembly();
>+    ASSERT(assembly.bottomOrLeft.font);
>+    ASSERT(assembly.topOrRight.font);
> 
>     // We are positioning the glyphs so that the edge of the tight glyph bounds line up exactly with the edges of our paint box.
>     LayoutPoint operatorTopLeft = paintOffset;
>     LayoutUnit baselineY = operatorTopLeft.y() + m_ascent;
>     LayoutPoint leftGlyphOrigin(operatorTopLeft.x(), baselineY);
>-    LayoutRect leftGlyphPaintRect = paintGlyph(style, info, m_assembly.bottomOrLeft, leftGlyphOrigin, TrimRight);
>+    LayoutRect leftGlyphPaintRect = paintGlyph(style, info, assembly.bottomOrLeft, leftGlyphOrigin, TrimRight);
> 
>-    FloatRect rightGlyphBounds = boundsForGlyph(m_assembly.topOrRight);
>+    FloatRect rightGlyphBounds = boundsForGlyph(assembly.topOrRight);
>     LayoutPoint rightGlyphOrigin(operatorTopLeft.x() + stretchSize() - rightGlyphBounds.width(), baselineY);
>-    LayoutRect rightGlyphPaintRect = paintGlyph(style, info, m_assembly.topOrRight, rightGlyphOrigin, TrimLeft);
>+    LayoutRect rightGlyphPaintRect = paintGlyph(style, info, assembly.topOrRight, rightGlyphOrigin, TrimLeft);
> 
>-    if (m_assembly.middle.font) {
>+    if (assembly.middle.font) {
>         // Center the glyph origin between the start and end glyph paint extents.
>         LayoutPoint middleGlyphOrigin(operatorTopLeft.x(), baselineY);
>         middleGlyphOrigin.moveBy(LayoutPoint((rightGlyphPaintRect.x() - leftGlyphPaintRect.maxX()) / 2.0, 0));
>-        LayoutRect middleGlyphPaintRect = paintGlyph(style, info, m_assembly.middle, middleGlyphOrigin, TrimLeftAndRight);
>+        LayoutRect middleGlyphPaintRect = paintGlyph(style, info, assembly.middle, middleGlyphOrigin, TrimLeftAndRight);
>         fillWithHorizontalExtensionGlyph(style, info, LayoutPoint(leftGlyphPaintRect.maxX(), baselineY), LayoutPoint(middleGlyphPaintRect.x(), baselineY));
>         fillWithHorizontalExtensionGlyph(style, info, LayoutPoint(middleGlyphPaintRect.maxX(), baselineY), LayoutPoint(rightGlyphPaintRect.x(), baselineY));
>     } else
>@@ -656,7 +658,7 @@ void MathOperator::paint(const RenderStyle& style, PaintInfo& info, const Layout
>         }
>     }
> 
>-    if (m_stretchType == StretchType::GlyphAssembly) {
>+    if (isAssembly()) {
>         if (m_operatorType == Type::VerticalOperator)
>             paintVerticalGlyphAssembly(style, info, paintOffset);
>         else
>@@ -665,13 +667,14 @@ void MathOperator::paint(const RenderStyle& style, PaintInfo& info, const Layout
>     }
> 
>     GlyphData glyphData;
>-    ASSERT(m_stretchType == StretchType::Unstretched || m_stretchType == StretchType::SizeVariant);
>-    if (m_stretchType == StretchType::Unstretched) {
>+    ASSERT(!m_variantOrAssembly || isVariant());
>+    if (!m_variantOrAssembly) {
>         if (!getBaseGlyph(style, glyphData))
>             return;
>-    } else if (m_stretchType == StretchType::SizeVariant) {
>-        ASSERT(m_variant.font);
>-        glyphData = m_variant;
>+    } else if (isVariant()) {
>+        auto& variant = this->variant();
>+        ASSERT(variant.font);
>+        glyphData = variant;
>     }
>     GlyphBuffer buffer;
>     buffer.add(glyphData.glyph, glyphData.font, advanceWidthForGlyph(glyphData));
>@@ -681,6 +684,58 @@ void MathOperator::paint(const RenderStyle& style, PaintInfo& info, const Layout
>     paintInfo.context().drawGlyphs(style.fontCascade(), *glyphData.font, buffer, 0, 1, operatorOrigin);
> }
> 
>+bool MathOperator::isVariant() const
>+{
>+    if (!m_variantOrAssembly)
>+        return false;
>+
>+    bool result = false;
>+    auto visitor = WTF::makeVisitor(
>+        [&](const GlyphData&) { result = true; },
>+        [&](const GlyphAssemblyData&) { result = false; }
>+    );
>+    std::experimental::visit(visitor, m_variantOrAssembly.value());
>+    return result;
>+}
>+
>+bool MathOperator::isAssembly() const
>+{
>+    if (!m_variantOrAssembly)
>+        return false;
>+
>+    bool result = false;
>+    auto visitor = WTF::makeVisitor(
>+        [&](const GlyphData&) { result = false; },
>+        [&](const GlyphAssemblyData&) { result = true; }
>+    );
>+    std::experimental::visit(visitor, m_variantOrAssembly.value());
>+    return result;
>+}
>+
>+GlyphData& MathOperator::variant()
>+{
>+    ASSERT(m_variantOrAssembly);
>+    GlyphData* result = nullptr;
>+    auto visitor = WTF::makeVisitor(
>+        [&](GlyphData& m_variant) { result = &m_variant; },
>+        [&](GlyphAssemblyData&) { CRASH_WITH_SECURITY_IMPLICATION(); }
>+    );
>+    std::experimental::visit(visitor, m_variantOrAssembly.value());
>+    return *result;
>+}
>+
>+auto MathOperator::assembly() -> GlyphAssemblyData&
>+{
>+    ASSERT(m_variantOrAssembly);
>+    GlyphAssemblyData* result = nullptr;
>+    auto visitor = WTF::makeVisitor(
>+        [&](GlyphData&) { CRASH_WITH_SECURITY_IMPLICATION(); },
>+        [&](GlyphAssemblyData& m_assembly) { result = &m_assembly; }
>+    );
>+    std::experimental::visit(visitor, m_variantOrAssembly.value());
>+    return *result;
>+}
>+
> }
> 
> #endif
>diff --git a/Source/WebCore/rendering/mathml/MathOperator.h b/Source/WebCore/rendering/mathml/MathOperator.h
>index cf0c549e6a1edf96cbe57cbbf2fe42672a0fbbf2..1e4587fc6b2752a909f6b2258c21f4400e1be328 100644
>--- a/Source/WebCore/rendering/mathml/MathOperator.h
>+++ b/Source/WebCore/rendering/mathml/MathOperator.h
>@@ -31,6 +31,8 @@
> #include "OpenTypeMathData.h"
> #include "PaintInfo.h"
> #include <unicode/utypes.h>
>+#include <wtf/Optional.h>
>+#include <wtf/Variant.h>
> 
> namespace WebCore {
> 
>@@ -61,7 +63,6 @@ private:
>         GlyphData bottomOrLeft;
>         GlyphData middle;
>     };
>-    enum class StretchType { Unstretched, SizeVariant, GlyphAssembly };
>     enum GlyphPaintTrimming {
>         TrimTop,
>         TrimBottom,
>@@ -87,13 +88,15 @@ private:
>     void paintVerticalGlyphAssembly(const RenderStyle&, PaintInfo&, const LayoutPoint&);
>     void paintHorizontalGlyphAssembly(const RenderStyle&, PaintInfo&, const LayoutPoint&);
> 
>+    bool isVariant() const;
>+    bool isAssembly() const;
>+    GlyphData& variant();
>+    GlyphAssemblyData& assembly();
>+
>     UChar m_baseCharacter { 0 };
>     Type m_operatorType { Type::NormalOperator };
>-    StretchType m_stretchType { StretchType::Unstretched };
>-    union {
>-        GlyphData m_variant;
>-        GlyphAssemblyData m_assembly;
>-    };
>+    typedef std::experimental::variant<GlyphData, GlyphAssemblyData> GlyphDataOrGlyphAssemblyData;
>+    Optional<GlyphDataOrGlyphAssemblyData> m_variantOrAssembly;
>     LayoutUnit m_maxPreferredWidth { 0 };
>     LayoutUnit m_width { 0 };
>     LayoutUnit m_ascent { 0 };

Actions: View | Formatted Diff | Diff

Attachments on bug 161119: 286808 | 286823 | 286828 | 287002 | 287003 | 287008 | 287012 | 287013 | 287047 | 287050 | 287086 | 287117 | 287122 | 287214 | 287922 | 288169 | 288183 | 288210 | 288223 | 288228 | 288230 | 288231 | 288282 | 288293 | 288295 | 288297 | 288299 | 288319 | 288330 | 288343 | 288365 | 288368 | 288392 | 288393