Split mixed font GlyphPage functionality to separate class
authorantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 9 Sep 2015 12:26:08 +0000 (12:26 +0000)
committerantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 9 Sep 2015 12:26:08 +0000 (12:26 +0000)
https://bugs.webkit.org/show_bug.cgi?id=148965

Reviewed by Myles Maxfield.

Currently GlyphPage class is used for both immutable single font case (in Font) and
for caching mixed font mappings (in FontCascadeFonts). It is cleaner to use separate
classed for these cases. This will also make future improvements easier.

* platform/graphics/Font.cpp:
(WebCore::Font::~Font):
(WebCore::fillGlyphPage):
(WebCore::createAndFillGlyphPage):
(WebCore::Font::glyphPage):
(WebCore::Font::glyphForCharacter):
(WebCore::Font::glyphDataForCharacter):
* platform/graphics/Font.h:
* platform/graphics/FontCascadeFonts.cpp:
(WebCore::MixedFontGlyphPage::MixedFontGlyphPage):
(WebCore::MixedFontGlyphPage::glyphDataForCharacter):
(WebCore::MixedFontGlyphPage::setGlyphDataForCharacter):
(WebCore::MixedFontGlyphPage::setGlyphDataForIndex):

    Mixed font pages are now an implementation detail of FontCascadeFonts.

(WebCore::FontCascadeFonts::GlyphPageCacheEntry::glyphDataForCharacter):
(WebCore::FontCascadeFonts::GlyphPageCacheEntry::setGlyphDataForCharacter):
(WebCore::FontCascadeFonts::GlyphPageCacheEntry::setSingleFontPage):

    Cache entry is either shared single font GlyphPage or mutable MixedFontGlyphPage.

(WebCore::FontCascadeFonts::FontCascadeFonts):
(WebCore::FontCascadeFonts::glyphDataForCharacter):
(WebCore::FontCascadeFonts::pruneSystemFallbacks):
* platform/graphics/FontCascadeFonts.h:
(WebCore::FontCascadeFonts::GlyphPageCacheEntry::isNull):
(WebCore::FontCascadeFonts::GlyphPageCacheEntry::isMixedFont):
* platform/graphics/GlyphPage.h:

    GlyphPage is now for single font mappings only.
    Use regular allocation instead of variable size tricks.
    It is always immutable after initialization (though currently a setter is still needed).

(WebCore::GlyphPage::create):
(WebCore::GlyphPage::~GlyphPage):
(WebCore::GlyphPage::count):
(WebCore::GlyphPage::indexForCharacter):
(WebCore::GlyphPage::glyphDataForCharacter):
(WebCore::GlyphPage::glyphForCharacter):
(WebCore::GlyphPage::glyphDataForIndex):
(WebCore::GlyphPage::glyphForIndex):
(WebCore::GlyphPage::setGlyphForIndex):
(WebCore::GlyphPage::font):
(WebCore::GlyphPage::GlyphPage):
(WebCore::GlyphPage::createForMixedFonts): Deleted.
(WebCore::GlyphPage::createCopyForMixedFonts): Deleted.
(WebCore::GlyphPage::createForSingleFont): Deleted.
(WebCore::GlyphPage::isImmutable): Deleted.
(WebCore::GlyphPage::setImmutable): Deleted.
(WebCore::GlyphPage::glyphAt): Deleted.
(WebCore::GlyphPage::fontForCharacter): Deleted.
(WebCore::GlyphPage::setGlyphDataForCharacter): Deleted.
(WebCore::GlyphPage::setGlyphDataForIndex): Deleted.
(WebCore::GlyphPage::hasPerGlyphFontData): Deleted.
* platform/graphics/freetype/GlyphPageTreeNodeFreeType.cpp:
(WebCore::GlyphPage::fill):
* platform/graphics/mac/GlyphPageMac.cpp:
(WebCore::GlyphPage::fill):
* platform/graphics/opentype/OpenTypeVerticalData.cpp:
(WebCore::OpenTypeVerticalData::substituteWithVerticalGlyphs):
* platform/graphics/win/GlyphPageTreeNodeCGWin.cpp:
(WebCore::GlyphPage::fill):
* platform/graphics/win/GlyphPageTreeNodeCairoWin.cpp:
(WebCore::GlyphPage::fill):
* svg/SVGFontData.cpp:
(WebCore::SVGFontData::applySVGGlyphSelection):
(WebCore::SVGFontData::fillSVGGlyphPage):
(WebCore::SVGFontData::fillBMPGlyphs):
(WebCore::SVGFontData::fillNonBMPGlyphs):
* svg/SVGFontData.h:
(WebCore::SVGFontData::verticalAdvanceY):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@189539 268f45cc-cd09-0410-ab3c-d52691b4dbfc

13 files changed:
Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/Font.cpp
Source/WebCore/platform/graphics/Font.h
Source/WebCore/platform/graphics/FontCascadeFonts.cpp
Source/WebCore/platform/graphics/FontCascadeFonts.h
Source/WebCore/platform/graphics/GlyphPage.h
Source/WebCore/platform/graphics/freetype/GlyphPageTreeNodeFreeType.cpp
Source/WebCore/platform/graphics/mac/GlyphPageMac.cpp
Source/WebCore/platform/graphics/opentype/OpenTypeVerticalData.cpp
Source/WebCore/platform/graphics/win/GlyphPageTreeNodeCGWin.cpp
Source/WebCore/platform/graphics/win/GlyphPageTreeNodeCairoWin.cpp
Source/WebCore/svg/SVGFontData.cpp
Source/WebCore/svg/SVGFontData.h

index b76ea44..6f06517 100644 (file)
@@ -1,3 +1,87 @@
+2015-09-09  Antti Koivisto  <antti@apple.com>
+
+        Split mixed font GlyphPage functionality to separate class
+        https://bugs.webkit.org/show_bug.cgi?id=148965
+
+        Reviewed by Myles Maxfield.
+
+        Currently GlyphPage class is used for both immutable single font case (in Font) and
+        for caching mixed font mappings (in FontCascadeFonts). It is cleaner to use separate
+        classed for these cases. This will also make future improvements easier.
+
+        * platform/graphics/Font.cpp:
+        (WebCore::Font::~Font):
+        (WebCore::fillGlyphPage):
+        (WebCore::createAndFillGlyphPage):
+        (WebCore::Font::glyphPage):
+        (WebCore::Font::glyphForCharacter):
+        (WebCore::Font::glyphDataForCharacter):
+        * platform/graphics/Font.h:
+        * platform/graphics/FontCascadeFonts.cpp:
+        (WebCore::MixedFontGlyphPage::MixedFontGlyphPage):
+        (WebCore::MixedFontGlyphPage::glyphDataForCharacter):
+        (WebCore::MixedFontGlyphPage::setGlyphDataForCharacter):
+        (WebCore::MixedFontGlyphPage::setGlyphDataForIndex):
+
+            Mixed font pages are now an implementation detail of FontCascadeFonts.
+
+        (WebCore::FontCascadeFonts::GlyphPageCacheEntry::glyphDataForCharacter):
+        (WebCore::FontCascadeFonts::GlyphPageCacheEntry::setGlyphDataForCharacter):
+        (WebCore::FontCascadeFonts::GlyphPageCacheEntry::setSingleFontPage):
+
+            Cache entry is either shared single font GlyphPage or mutable MixedFontGlyphPage.
+
+        (WebCore::FontCascadeFonts::FontCascadeFonts):
+        (WebCore::FontCascadeFonts::glyphDataForCharacter):
+        (WebCore::FontCascadeFonts::pruneSystemFallbacks):
+        * platform/graphics/FontCascadeFonts.h:
+        (WebCore::FontCascadeFonts::GlyphPageCacheEntry::isNull):
+        (WebCore::FontCascadeFonts::GlyphPageCacheEntry::isMixedFont):
+        * platform/graphics/GlyphPage.h:
+
+            GlyphPage is now for single font mappings only.
+            Use regular allocation instead of variable size tricks.
+            It is always immutable after initialization (though currently a setter is still needed).
+
+        (WebCore::GlyphPage::create):
+        (WebCore::GlyphPage::~GlyphPage):
+        (WebCore::GlyphPage::count):
+        (WebCore::GlyphPage::indexForCharacter):
+        (WebCore::GlyphPage::glyphDataForCharacter):
+        (WebCore::GlyphPage::glyphForCharacter):
+        (WebCore::GlyphPage::glyphDataForIndex):
+        (WebCore::GlyphPage::glyphForIndex):
+        (WebCore::GlyphPage::setGlyphForIndex):
+        (WebCore::GlyphPage::font):
+        (WebCore::GlyphPage::GlyphPage):
+        (WebCore::GlyphPage::createForMixedFonts): Deleted.
+        (WebCore::GlyphPage::createCopyForMixedFonts): Deleted.
+        (WebCore::GlyphPage::createForSingleFont): Deleted.
+        (WebCore::GlyphPage::isImmutable): Deleted.
+        (WebCore::GlyphPage::setImmutable): Deleted.
+        (WebCore::GlyphPage::glyphAt): Deleted.
+        (WebCore::GlyphPage::fontForCharacter): Deleted.
+        (WebCore::GlyphPage::setGlyphDataForCharacter): Deleted.
+        (WebCore::GlyphPage::setGlyphDataForIndex): Deleted.
+        (WebCore::GlyphPage::hasPerGlyphFontData): Deleted.
+        * platform/graphics/freetype/GlyphPageTreeNodeFreeType.cpp:
+        (WebCore::GlyphPage::fill):
+        * platform/graphics/mac/GlyphPageMac.cpp:
+        (WebCore::GlyphPage::fill):
+        * platform/graphics/opentype/OpenTypeVerticalData.cpp:
+        (WebCore::OpenTypeVerticalData::substituteWithVerticalGlyphs):
+        * platform/graphics/win/GlyphPageTreeNodeCGWin.cpp:
+        (WebCore::GlyphPage::fill):
+        * platform/graphics/win/GlyphPageTreeNodeCairoWin.cpp:
+        (WebCore::GlyphPage::fill):
+        * svg/SVGFontData.cpp:
+        (WebCore::SVGFontData::applySVGGlyphSelection):
+        (WebCore::SVGFontData::fillSVGGlyphPage):
+        (WebCore::SVGFontData::fillBMPGlyphs):
+        (WebCore::SVGFontData::fillNonBMPGlyphs):
+        * svg/SVGFontData.h:
+        (WebCore::SVGFontData::verticalAdvanceY):
+
 2015-09-09  Manuel Rego Casasnovas  <rego@igalia.com>
 
         [css-grid] Percentage columns shouldn't include border and padding
index b9f579a..db92fb0 100644 (file)
@@ -149,27 +149,27 @@ Font::~Font()
     removeFromSystemFallbackCache();
 }
 
-static bool fillGlyphPage(GlyphPage& pageToFill, UChar* buffer, unsigned bufferLength, const Font* font)
+static bool fillGlyphPage(GlyphPage& pageToFill, UChar* buffer, unsigned bufferLength, const Font& font)
 {
 #if ENABLE(SVG_FONTS)
-    if (auto* svgData = font->svgData())
-        return svgData->fillSVGGlyphPage(&pageToFill, buffer, bufferLength, font);
+    if (auto* svgData = font.svgData())
+        return svgData->fillSVGGlyphPage(&pageToFill, buffer, bufferLength);
 #endif
-    bool hasGlyphs = pageToFill.fill(buffer, bufferLength, font);
+    bool hasGlyphs = pageToFill.fill(buffer, bufferLength, &font);
 #if ENABLE(OPENTYPE_VERTICAL)
-    if (hasGlyphs && font->verticalData())
-        font->verticalData()->substituteWithVerticalGlyphs(font, &pageToFill);
+    if (hasGlyphs && font.verticalData())
+        font.verticalData()->substituteWithVerticalGlyphs(&font, &pageToFill);
 #endif
     return hasGlyphs;
 }
 
-static RefPtr<GlyphPage> createAndFillGlyphPage(unsigned pageNumber, const Font* font)
+static RefPtr<GlyphPage> createAndFillGlyphPage(unsigned pageNumber, const Font& font)
 {
 #if PLATFORM(IOS)
     // 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 == 6 && font.shouldNotBeUsedForArabic())
         return nullptr;
 #endif
 
@@ -226,26 +226,25 @@ static RefPtr<GlyphPage> createAndFillGlyphPage(unsigned pageNumber, const Font*
     // routine of our glyph map for actually filling in the page with the glyphs.
     // Success is not guaranteed. For example, Times fails to fill page 260, giving glyph data
     // for only 128 out of 256 characters.
-    RefPtr<GlyphPage> glyphPage = GlyphPage::createForSingleFont(font);
+    Ref<GlyphPage> glyphPage = GlyphPage::create(font);
 
-    bool haveGlyphs = fillGlyphPage(*glyphPage, buffer, bufferLength, font);
+    bool haveGlyphs = fillGlyphPage(glyphPage, buffer, bufferLength, font);
     if (!haveGlyphs)
         return nullptr;
 
-    glyphPage->setImmutable();
-    return glyphPage;
+    return WTF::move(glyphPage);
 }
 
 const GlyphPage* Font::glyphPage(unsigned pageNumber) const
 {
     if (!pageNumber) {
         if (!m_glyphPageZero)
-            m_glyphPageZero = createAndFillGlyphPage(0, this);
+            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);
+        addResult.iterator->value = createAndFillGlyphPage(pageNumber, *this);
 
     return addResult.iterator->value.get();
 }
@@ -255,7 +254,7 @@ Glyph Font::glyphForCharacter(UChar32 character) const
     auto* page = glyphPage(character / GlyphPage::size);
     if (!page)
         return 0;
-    return page->glyphAt(character % GlyphPage::size);
+    return page->glyphForCharacter(character);
 }
 
 GlyphData Font::glyphDataForCharacter(UChar32 character) const
index 594a30d..075cbe0 100644 (file)
@@ -76,7 +76,7 @@ public:
 
         virtual void initializeFont(Font*, float fontSize) = 0;
         virtual float widthForSVGGlyph(Glyph, float fontSize) const = 0;
-        virtual bool fillSVGGlyphPage(GlyphPage*, UChar* buffer, unsigned bufferLength, const Font*) const = 0;
+        virtual bool fillSVGGlyphPage(GlyphPage*, UChar* buffer, unsigned bufferLength) const = 0;
     };
 
     // Used to create platform fonts.
index 9a13b08..9578eef 100644 (file)
 
 namespace WebCore {
 
+class MixedFontGlyphPage {
+    WTF_MAKE_FAST_ALLOCATED;
+public:
+    MixedFontGlyphPage(const GlyphPage* initialPage)
+    {
+        if (initialPage) {
+            for (unsigned i = 0; i < GlyphPage::size; ++i)
+                setGlyphDataForIndex(i, initialPage->glyphDataForIndex(i));
+        }
+    }
+
+    GlyphData glyphDataForCharacter(UChar32 c) const
+    {
+        unsigned index = GlyphPage::indexForCharacter(c);
+        ASSERT_WITH_SECURITY_IMPLICATION(index < GlyphPage::size);
+        return { m_glyphs[index], m_fonts[index] };
+    }
+
+    void setGlyphDataForCharacter(UChar32 c, GlyphData glyphData)
+    {
+        setGlyphDataForIndex(GlyphPage::indexForCharacter(c), glyphData);
+    }
+
+private:
+    void setGlyphDataForIndex(unsigned index, const GlyphData& glyphData)
+    {
+        ASSERT_WITH_SECURITY_IMPLICATION(index < GlyphPage::size);
+        m_glyphs[index] = glyphData.glyph;
+        m_fonts[index] = glyphData.font;
+    }
+
+    Glyph m_glyphs[GlyphPage::size] { };
+    const Font* m_fonts[GlyphPage::size] { };
+};
+
+GlyphData FontCascadeFonts::GlyphPageCacheEntry::glyphDataForCharacter(UChar32 character)
+{
+    ASSERT(!(m_singleFont && m_mixedFont));
+    if (m_singleFont)
+        return m_singleFont->glyphDataForCharacter(character);
+    if (m_mixedFont)
+        return m_mixedFont->glyphDataForCharacter(character);
+    return 0;
+}
+
+void FontCascadeFonts::GlyphPageCacheEntry::setGlyphDataForCharacter(UChar32 character, GlyphData glyphData)
+{
+    ASSERT(!glyphDataForCharacter(character).glyph);
+    if (!m_mixedFont) {
+        m_mixedFont = std::make_unique<MixedFontGlyphPage>(m_singleFont.get());
+        m_singleFont = nullptr;
+    }
+    m_mixedFont->setGlyphDataForCharacter(character, glyphData);
+}
+
+void FontCascadeFonts::GlyphPageCacheEntry::setSingleFontPage(RefPtr<GlyphPage>&& page)
+{
+    ASSERT(isNull());
+    m_singleFont = page;
+}
 
 FontCascadeFonts::FontCascadeFonts(RefPtr<FontSelector>&& fontSelector)
     : m_cachedPrimaryFont(nullptr)
@@ -371,20 +431,20 @@ GlyphData FontCascadeFonts::glyphDataForCharacter(UChar32 c, const FontDescripti
 
     const unsigned pageNumber = c / GlyphPage::size;
 
-    RefPtr<GlyphPage>& cachedPage = pageNumber ? m_cachedPages.add(pageNumber, nullptr).iterator->value : m_cachedPageZero;
-    if (!cachedPage)
-        cachedPage = glyphPageFromFontRanges(pageNumber, realizeFallbackRangesAt(description, 0));
+    auto& cacheEntry = pageNumber ? m_cachedPages.add(pageNumber, GlyphPageCacheEntry()).iterator->value : m_cachedPageZero;
 
-    GlyphData glyphData = cachedPage ? cachedPage->glyphDataForCharacter(c) : GlyphData();
-    if (!glyphData.glyph) {
-        if (!cachedPage)
-            cachedPage = GlyphPage::createForMixedFonts();
-        else if (cachedPage->isImmutable())
-            cachedPage = GlyphPage::createCopyForMixedFonts(*cachedPage);
+    // Initialize cache with a full page of glyph mappings from a single font.
+    if (cacheEntry.isNull())
+        cacheEntry.setSingleFontPage(glyphPageFromFontRanges(pageNumber, realizeFallbackRangesAt(description, 0)));
 
+    GlyphData glyphData = cacheEntry.glyphDataForCharacter(c);
+    if (!glyphData.glyph) {
+        // No glyph, resolve per-character.
         glyphData = glyphDataForNormalVariant(c, description);
-        cachedPage->setGlyphDataForCharacter(c, glyphData.glyph, glyphData.font);
+        // Cache the results.
+        cacheEntry.setGlyphDataForCharacter(c, glyphData);
     }
+
     return glyphData;
 }
 
@@ -393,10 +453,10 @@ void FontCascadeFonts::pruneSystemFallbacks()
     if (m_systemFallbackFontSet.isEmpty())
         return;
     // Mutable glyph pages may reference fallback fonts.
-    if (m_cachedPageZero && !m_cachedPageZero->isImmutable())
-        m_cachedPageZero = nullptr;
+    if (m_cachedPageZero.isMixedFont())
+        m_cachedPageZero = { };
     m_cachedPages.removeIf([](decltype(m_cachedPages)::KeyValuePairType& keyAndValue) {
-        return !keyAndValue.value->isImmutable();
+        return keyAndValue.value.isMixedFont();
     });
     m_systemFallbackFontSet.clear();
 }
index 9f2c1c1..b3a4bae 100644 (file)
 
 namespace WebCore {
 
-class GraphicsContext;
-class IntRect;
 class FontDescription;
 class FontPlatformData;
 class FontSelector;
+class GraphicsContext;
+class IntRect;
+class MixedFontGlyphPage;
 
 class FontCascadeFonts : public RefCounted<FontCascadeFonts> {
     WTF_MAKE_NONCOPYABLE(FontCascadeFonts);
@@ -82,8 +83,24 @@ private:
     Vector<FontRanges, 1> m_realizedFallbackRanges;
     unsigned m_lastRealizedFallbackIndex { 0 };
 
-    RefPtr<GlyphPage> m_cachedPageZero;
-    HashMap<int, RefPtr<GlyphPage>> m_cachedPages;
+    class GlyphPageCacheEntry {
+    public:
+        GlyphData glyphDataForCharacter(UChar32);
+
+        void setSingleFontPage(RefPtr<GlyphPage>&&);
+        void setGlyphDataForCharacter(UChar32, GlyphData);
+
+        bool isNull() const { return !m_singleFont && !m_mixedFont; }
+        bool isMixedFont() const { return !!m_mixedFont; }
+    
+    private:
+        // Only one of these is non-null.
+        RefPtr<GlyphPage> m_singleFont;
+        std::unique_ptr<MixedFontGlyphPage> m_mixedFont;
+    };
+
+    GlyphPageCacheEntry m_cachedPageZero;
+    HashMap<int, GlyphPageCacheEntry> m_cachedPages;
 
     HashSet<RefPtr<Font>> m_systemFallbackFontSet;
 
index d19a9e8..d11b668 100644 (file)
@@ -53,37 +53,14 @@ struct GlyphData {
     const Font* font;
 };
 
-#if COMPILER(MSVC)
-#pragma warning(push)
-#pragma warning(disable: 4200) // Disable "zero-sized array in struct/union" warning
-#endif
-
 // A GlyphPage contains a fixed-size set of GlyphData mappings for a contiguous
 // range of characters in the Unicode code space. GlyphPages are indexed
 // starting from 0 and incrementing for each 256 glyphs.
-//
-// One page may actually include glyphs from other fonts if the characters are
-// missing in the primary font.
 class GlyphPage : public RefCounted<GlyphPage> {
 public:
-    static PassRefPtr<GlyphPage> createForMixedFonts()
-    {
-        void* slot = fastMalloc(sizeof(GlyphPage) + sizeof(Font*) * GlyphPage::size);
-        return adoptRef(new (NotNull, slot) GlyphPage(nullptr));
-    }
-
-    static PassRefPtr<GlyphPage> createCopyForMixedFonts(const GlyphPage& original)
+    static Ref<GlyphPage> create(const Font& font)
     {
-        RefPtr<GlyphPage> page = createForMixedFonts();
-        for (unsigned i = 0; i < GlyphPage::size; ++i)
-            page->setGlyphDataForIndex(i, original.glyphDataForIndex(i));
-        return page.release();
-    }
-
-    static PassRefPtr<GlyphPage> createForSingleFont(const Font* font)
-    {
-        ASSERT(font);
-        return adoptRef(new GlyphPage(font));
+        return adoptRef(*new GlyphPage(font));
     }
 
     ~GlyphPage()
@@ -91,99 +68,61 @@ public:
         --s_count;
     }
 
-    bool isImmutable() const { return m_isImmutable; }
-    void setImmutable() { m_isImmutable = true; }
-
     static unsigned count() { return s_count; }
 
     static const size_t size = 256; // Covers Latin-1 in a single page.
-    static_assert((!(0xD800 % size)) && (!(0xDC00 % size)) && (!(0xE000 % size)), "GlyphPages must never straddle code-unit length boundaries");
     static unsigned indexForCharacter(UChar32 c) { return c % GlyphPage::size; }
 
-    ALWAYS_INLINE GlyphData glyphDataForCharacter(UChar32 c) const
+    GlyphData glyphDataForCharacter(UChar32 c) const
     {
         return glyphDataForIndex(indexForCharacter(c));
     }
 
-    ALWAYS_INLINE GlyphData glyphDataForIndex(unsigned index) const
+    Glyph glyphForCharacter(UChar32 c) const
     {
-        ASSERT_WITH_SECURITY_IMPLICATION(index < size);
-        Glyph glyph = m_glyphs[index];
-        if (hasPerGlyphFontData())
-            return GlyphData(glyph, m_perGlyphFontData[index]);
-        return GlyphData(glyph, glyph ? m_fontForAllGlyphs : 0);
+        return glyphForIndex(indexForCharacter(c));
     }
 
-    ALWAYS_INLINE Glyph glyphAt(unsigned index) const
+    GlyphData glyphDataForIndex(unsigned index) const
     {
-        ASSERT_WITH_SECURITY_IMPLICATION(index < size);
-        return m_glyphs[index];
+        Glyph glyph = glyphForIndex(index);
+        return GlyphData(glyph, glyph ? &m_font : nullptr);
     }
 
-    ALWAYS_INLINE const Font* fontForCharacter(UChar32 c) const
+    Glyph glyphForIndex(unsigned index) const
     {
-        unsigned index = indexForCharacter(c);
-        if (hasPerGlyphFontData())
-            return m_perGlyphFontData[index];
-        return m_glyphs[index] ? m_fontForAllGlyphs : 0;
-    }
-
-    void setGlyphDataForCharacter(UChar32 c, Glyph g, const Font* f)
-    {
-        setGlyphDataForIndex(indexForCharacter(c), g, f);
+        ASSERT_WITH_SECURITY_IMPLICATION(index < size);
+        return m_glyphs[index];
     }
 
-    void setGlyphDataForIndex(unsigned index, Glyph glyph, const Font* font)
+    // FIXME: Pages are immutable after initialization. This should be private.
+    void setGlyphForIndex(unsigned index, Glyph glyph)
     {
         ASSERT_WITH_SECURITY_IMPLICATION(index < size);
-        ASSERT(!m_isImmutable);
-        
         m_glyphs[index] = glyph;
-
-        // GlyphPage getters will always return a null Font* for glyph #0 if there's no per-glyph font array.
-        if (hasPerGlyphFontData()) {
-            m_perGlyphFontData[index] = glyph ? font : 0;
-            return;
-        }
-
-        // A single-font GlyphPage already assigned m_fontForAllGlyphs in the constructor.
-        ASSERT(!glyph || font == m_fontForAllGlyphs);
     }
 
-    void setGlyphDataForIndex(unsigned index, const GlyphData& glyphData)
+    const Font& font() const
     {
-        setGlyphDataForIndex(index, glyphData.glyph, glyphData.font);
+        return m_font;
     }
 
     // Implemented by the platform.
     bool fill(UChar* characterBuffer, unsigned bufferLength, const Font*);
 
 private:
-    explicit GlyphPage(const Font* fontForAllGlyphs)
-        : m_fontForAllGlyphs(fontForAllGlyphs)
+    explicit GlyphPage(const Font& font)
+        : m_font(font)
     {
-        memset(m_glyphs, 0, sizeof(m_glyphs));
-        if (hasPerGlyphFontData())
-            memset(m_perGlyphFontData, 0, sizeof(Font*) * GlyphPage::size);
         ++s_count;
     }
 
-    bool hasPerGlyphFontData() const { return !m_fontForAllGlyphs; }
-
-    const Font* m_fontForAllGlyphs;
-    Glyph m_glyphs[size];
-
-    bool m_isImmutable { false };
-    // NOTE: This array has (GlyphPage::size) elements if m_fontForAllGlyphs is null.
-    const Font* m_perGlyphFontData[0];
+    const Font& m_font;
+    Glyph m_glyphs[size] { };
 
     WEBCORE_EXPORT static unsigned s_count;
 };
 
-#if COMPILER(MSVC)
-#pragma warning(pop)
-#endif
-
 } // namespace WebCore
 
 #endif // GlyphPage_h
index 6336bed..f42e00c 100644 (file)
@@ -57,9 +57,9 @@ bool GlyphPage::fill(UChar* buffer, unsigned bufferLength, const Font* fontData)
 
         Glyph glyph = FcFreeTypeCharIndex(face, character);
         if (!glyph)
-            setGlyphDataForIndex(i, 0, 0);
+            setGlyphForIndex(i, 0);
         else {
-            setGlyphDataForIndex(i, glyph, fontData);
+            setGlyphForIndex(i, glyph);
             haveGlyphs = true;
         }
     }
index 3cdc00f..5cae437 100644 (file)
@@ -60,23 +60,18 @@ static bool shouldUseCoreText(const UChar* buffer, unsigned bufferLength, const
 
 bool GlyphPage::fill(UChar* buffer, unsigned bufferLength, const Font* fontData)
 {
-    bool haveGlyphs = false;
+    ASSERT(fontData == &font());
+    ASSERT(bufferLength == GlyphPage::size || bufferLength == 2 * GlyphPage::size);
 
     Vector<CGGlyph, 512> glyphs(bufferLength);
+    unsigned glyphStep;
     if (!shouldUseCoreText(buffer, bufferLength, fontData)) {
         // 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.
         CGFontGetGlyphsForUnichars(fontData->platformData().cgFont(), buffer, glyphs.data(), bufferLength);
-        for (unsigned i = 0; i < GlyphPage::size; ++i) {
-            if (!glyphs[i])
-                setGlyphDataForIndex(i, 0, 0);
-            else {
-                setGlyphDataForIndex(i, glyphs[i], fontData);
-                haveGlyphs = true;
-            }
-        }
+        glyphStep = 1;
     } 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.
@@ -84,20 +79,19 @@ bool GlyphPage::fill(UChar* buffer, unsigned bufferLength, const Font* fontData)
             CTFontGetGlyphsForCharacters(fontData->platformData().ctFont(), buffer, glyphs.data(), bufferLength);
         else
             CTFontGetVerticalGlyphsForCharacters(fontData->platformData().ctFont(), buffer, glyphs.data(), bufferLength);
+
         // When buffer consists of surrogate pairs, CTFontGetVerticalGlyphsForCharacters and CTFontGetGlyphsForCharacters
         // place the glyphs at indices corresponding to the first character of each pair.
-        ASSERT(bufferLength == GlyphPage::size || bufferLength == 2 * GlyphPage::size);
-        unsigned glyphStep = bufferLength / GlyphPage::size;
-        for (unsigned i = 0; i < GlyphPage::size; ++i) {
-            if (!glyphs[i * glyphStep])
-                setGlyphDataForIndex(i, 0, 0);
-            else {
-                setGlyphDataForIndex(i, glyphs[i * glyphStep], fontData);
-                haveGlyphs = true;
-            }
-        }
+        glyphStep = bufferLength / GlyphPage::size;
     }
 
+    bool haveGlyphs = false;
+    for (unsigned i = 0; i < GlyphPage::size; ++i) {
+        if (glyphs[i * glyphStep]) {
+            setGlyphForIndex(i, glyphs[i * glyphStep]);
+            haveGlyphs = true;
+        }
+    }
     return haveGlyphs;
 }
 
index 07df69a..068f21a 100644 (file)
@@ -542,12 +542,12 @@ void OpenTypeVerticalData::substituteWithVerticalGlyphs(const Font* font, GlyphP
         return;
 
     for (unsigned index = 0; index < GlyphPage::size; ++index) {
-        Glyph glyph = glyphPage->glyphAt(index);
+        Glyph glyph = glyphPage->glyphForIndex(index);
         if (glyph) {
-            ASSERT(glyphPage->glyphDataForIndex(index).font == font);
+            ASSERT_UNUSED(font, &glyphPage->font() == font);
             Glyph to = map.get(glyph);
             if (to)
-                glyphPage->setGlyphDataForIndex(index, to, font);
+                glyphPage->setGlyphForIndex(index, to);
         }
     }
 }
index aac024a..ede6724 100644 (file)
@@ -47,9 +47,9 @@ bool GlyphPage::fill(UChar* buffer, unsigned bufferLength, const Font* fontData)
     for (unsigned i = 0; i < GlyphPage::size; i++) {
         Glyph glyph = localGlyphBuffer[i];
         if (!glyph)
-            setGlyphDataForIndex(i, 0, 0);
+            setGlyphForIndex(i, 0);
         else {
-            setGlyphDataForIndex(i, glyph, fontData);
+            setGlyphForIndex(i, glyph);
             haveGlyphs = true;
         }
     }
index 42dc3f1..6d2b3cd 100644 (file)
@@ -54,9 +54,9 @@ bool GlyphPage::fill(UChar* buffer, unsigned bufferLength, const Font* fontData)
         for (unsigned i = 0; i < GlyphPage::size; i++) {
             Glyph glyph = localGlyphBuffer[i];
             if (glyph == 0xffff)
-                setGlyphDataForIndex(i, 0, 0);
+                setGlyphForIndex(i, 0);
             else {
-                setGlyphDataForIndex(i, glyph, fontData);
+                setGlyphForIndex(i, glyph);
                 haveGlyphs = true;
             }
         }
index 5703c0b..8df508d 100644 (file)
@@ -204,11 +204,8 @@ bool SVGFontData::applySVGGlyphSelection(WidthIterator& iterator, GlyphData& gly
     return false;
 }
 
-bool SVGFontData::fillSVGGlyphPage(GlyphPage* pageToFill, UChar* buffer, unsigned bufferLength, const Font* font) const
+bool SVGFontData::fillSVGGlyphPage(GlyphPage* pageToFill, UChar* buffer, unsigned bufferLength) const
 {
-    ASSERT(font->isCustomFont());
-    ASSERT(font->isSVGFont());
-
     SVGFontFaceElement* fontFaceElement = this->svgFontFaceElement();
     ASSERT(fontFaceElement);
 
@@ -216,13 +213,13 @@ bool SVGFontData::fillSVGGlyphPage(GlyphPage* pageToFill, UChar* buffer, unsigne
     ASSERT(fontElement);
 
     if (bufferLength == GlyphPage::size)
-        return fillBMPGlyphs(fontElement, pageToFill, buffer, font);
+        return fillBMPGlyphs(fontElement, pageToFill, buffer);
 
     ASSERT(bufferLength == 2 * GlyphPage::size);
-    return fillNonBMPGlyphs(fontElement, pageToFill, buffer, font);
+    return fillNonBMPGlyphs(fontElement, pageToFill, buffer);
 }
 
-bool SVGFontData::fillBMPGlyphs(SVGFontElement* fontElement, GlyphPage* pageToFill, UChar* buffer, const Font* font) const
+bool SVGFontData::fillBMPGlyphs(SVGFontElement* fontElement, GlyphPage* pageToFill, UChar* buffer) const
 {
     bool haveGlyphs = false;
     Vector<SVGGlyph> glyphs;
@@ -230,7 +227,7 @@ bool SVGFontData::fillBMPGlyphs(SVGFontElement* fontElement, GlyphPage* pageToFi
         String lookupString(buffer + i, 1);
         fontElement->collectGlyphsForString(lookupString, glyphs);
         if (glyphs.isEmpty()) {
-            pageToFill->setGlyphDataForIndex(i, 0, 0);
+            pageToFill->setGlyphForIndex(i, 0);
             continue;
         }
 
@@ -239,14 +236,14 @@ bool SVGFontData::fillBMPGlyphs(SVGFontElement* fontElement, GlyphPage* pageToFi
         // care of matching to the correct glyph, if multiple ones are available, as that's
         // only possible within the context of a string (eg. arabic form matching).
         haveGlyphs = true;
-        pageToFill->setGlyphDataForIndex(i, glyphs.first().tableEntry, font);
+        pageToFill->setGlyphForIndex(i, glyphs.first().tableEntry);
         glyphs.clear();
     }
 
     return haveGlyphs;
 }
 
-bool SVGFontData::fillNonBMPGlyphs(SVGFontElement* fontElement, GlyphPage* pageToFill, UChar* buffer, const Font* font) const
+bool SVGFontData::fillNonBMPGlyphs(SVGFontElement* fontElement, GlyphPage* pageToFill, UChar* buffer) const
 {
     bool haveGlyphs = false;
     Vector<SVGGlyph> glyphs;
@@ -255,7 +252,7 @@ bool SVGFontData::fillNonBMPGlyphs(SVGFontElement* fontElement, GlyphPage* pageT
         String lookupString(buffer + i * 2, 2);
         fontElement->collectGlyphsForString(lookupString, glyphs);
         if (glyphs.isEmpty()) {
-            pageToFill->setGlyphDataForIndex(i, 0, 0);
+            pageToFill->setGlyphForIndex(i, 0);
             continue;
         }
 
@@ -264,7 +261,7 @@ bool SVGFontData::fillNonBMPGlyphs(SVGFontElement* fontElement, GlyphPage* pageT
         // care of matching to the correct glyph, if multiple ones are available, as that's
         // only possible within the context of a string (eg. arabic form matching).
         haveGlyphs = true;
-        pageToFill->setGlyphDataForIndex(i, glyphs.first().tableEntry, font);
+        pageToFill->setGlyphForIndex(i, glyphs.first().tableEntry);
         glyphs.clear();
     }
 
index d98adc4..2a6d394 100644 (file)
@@ -35,7 +35,7 @@ public:
 
     virtual void initializeFont(Font*, float fontSize) override;
     virtual float widthForSVGGlyph(Glyph, float fontSize) const override;
-    virtual bool fillSVGGlyphPage(GlyphPage*, UChar* buffer, unsigned bufferLength, const Font*) const override;
+    virtual bool fillSVGGlyphPage(GlyphPage*, UChar* buffer, unsigned bufferLength) const override;
 
     bool applySVGGlyphSelection(WidthIterator&, GlyphData&, bool mirror, int currentCharacter, unsigned& advanceLength, String& normalizedSpacesStringCache) const;
 
@@ -50,8 +50,8 @@ public:
     float verticalAdvanceY() const { return m_verticalAdvanceY; }
 
 private:
-    bool fillBMPGlyphs(SVGFontElement*, GlyphPage*, UChar* buffer, const Font*) const;
-    bool fillNonBMPGlyphs(SVGFontElement*, GlyphPage*, UChar* buffer, const Font*) const;
+    bool fillBMPGlyphs(SVGFontElement*, GlyphPage*, UChar* buffer) const;
+    bool fillNonBMPGlyphs(SVGFontElement*, GlyphPage*, UChar* buffer) const;
 
     bool applyTransforms(GlyphBufferGlyph*, GlyphBufferAdvance*, size_t, TypesettingFeatures) const = delete;