FontGlyphs::glyphDataAndPageForCharacter cleanups
authorantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 12 Dec 2014 20:31:30 +0000 (20:31 +0000)
committerantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 12 Dec 2014 20:31:30 +0000 (20:31 +0000)
https://bugs.webkit.org/show_bug.cgi?id=139584

Reviewed by Andreas Kling.

Split it up and use helper functions.

* WebCore.exp.in:
* platform/graphics/FontGlyphs.cpp:
(WebCore::FontGlyphs::determinePitch):
(WebCore::FontGlyphs::realizeFontDataAt):
(WebCore::FontGlyphs::glyphDataAndPageForSystemFallback):
(WebCore::FontGlyphs::glyphDataAndPageForVariant):

    Add private helpers.

(WebCore::FontGlyphs::glyphDataAndPageForCharacter):

    Do the m_pages cache lookup only once and use the pointer reference afterwards.

* platform/graphics/FontGlyphs.h:
(WebCore::FontGlyphs::fontSelector):
(WebCore::FontGlyphs::widthCache):
(WebCore::FontGlyphs::primaryFontData):
(WebCore::FontGlyphs::isFixedPitch):
(WebCore::FontGlyphs::primarySimpleFontData):

    Also removed the mutable attribute from the fields (almost everything was marked mutable) and made the inteface
    non-const. The FontGlyphs member of Font is already mutable so this is not needed.

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

Source/WebCore/ChangeLog
Source/WebCore/WebCore.exp.in
Source/WebCore/platform/graphics/FontGlyphs.cpp
Source/WebCore/platform/graphics/FontGlyphs.h

index fdff829..301e773 100644 (file)
@@ -1,3 +1,35 @@
+2014-12-12  Antti Koivisto  <antti@apple.com>
+
+        FontGlyphs::glyphDataAndPageForCharacter cleanups
+        https://bugs.webkit.org/show_bug.cgi?id=139584
+
+        Reviewed by Andreas Kling.
+
+        Split it up and use helper functions.
+
+        * WebCore.exp.in:
+        * platform/graphics/FontGlyphs.cpp:
+        (WebCore::FontGlyphs::determinePitch):
+        (WebCore::FontGlyphs::realizeFontDataAt):
+        (WebCore::FontGlyphs::glyphDataAndPageForSystemFallback):
+        (WebCore::FontGlyphs::glyphDataAndPageForVariant):
+
+            Add private helpers.
+
+        (WebCore::FontGlyphs::glyphDataAndPageForCharacter):
+
+            Do the m_pages cache lookup only once and use the pointer reference afterwards.
+
+        * platform/graphics/FontGlyphs.h:
+        (WebCore::FontGlyphs::fontSelector):
+        (WebCore::FontGlyphs::widthCache):
+        (WebCore::FontGlyphs::primaryFontData):
+        (WebCore::FontGlyphs::isFixedPitch):
+        (WebCore::FontGlyphs::primarySimpleFontData):
+
+            Also removed the mutable attribute from the fields (almost everything was marked mutable) and made the inteface
+            non-const. The FontGlyphs member of Font is already mutable so this is not needed.
+
 2014-12-12  Chris Dumez  <cdumez@apple.com>
 
         Use is<>() / downcast<>() for StyleRuleBase subclasses
index 7689d3c..5efe2a4 100644 (file)
@@ -77,6 +77,7 @@ __ZN7WebCore10CredentialC1EP15NSURLCredential
 __ZN7WebCore10FloatPointC1ERK7CGPoint
 __ZN7WebCore10FloatPointC1ERKNS_8IntPointE
 __ZN7WebCore10FontGlyphs15releaseFontDataEv
+__ZN7WebCore10FontGlyphs17realizeFontDataAtERKNS_15FontDescriptionEj
 __ZN7WebCore10JSDocument6s_infoE
 __ZN7WebCore10JSDocument9toWrappedEN3JSC7JSValueE
 __ZN7WebCore10LayoutRect5scaleEf
@@ -1583,7 +1584,6 @@ __ZNK3WTF6String14createCFStringEv
 __ZNK7WebCore10Credential12nsCredentialEv
 __ZNK7WebCore10Credential7isEmptyEv
 __ZNK7WebCore10FloatPointcv7CGPointEv
-__ZNK7WebCore10FontGlyphs17realizeFontDataAtERKNS_15FontDescriptionEj
 __ZNK7WebCore10LayoutRect8containsERKS0_
 __ZNK7WebCore10PluginData16supportsMimeTypeERKN3WTF6StringENS0_18AllowedPluginTypesE
 __ZNK7WebCore10RenderText16firstRunLocationEv
index 9321635..e01c612 100644 (file)
@@ -74,7 +74,7 @@ void FontGlyphs::releaseFontData()
     }
 }
 
-void FontGlyphs::determinePitch(const FontDescription& description) const
+void FontGlyphs::determinePitch(const FontDescription& description)
 {
     const FontData* fontData = primaryFontData(description);
     if (is<SimpleFontData>(*fontData))
@@ -89,7 +89,7 @@ void FontGlyphs::determinePitch(const FontDescription& description) const
     }
 }
 
-const FontData* FontGlyphs::realizeFontDataAt(const FontDescription& description, unsigned realizedFontIndex) const
+const FontData* FontGlyphs::realizeFontDataAt(const FontDescription& description, unsigned realizedFontIndex)
 {
     if (realizedFontIndex < m_realizedFontData.size())
         return m_realizedFontData[realizedFontIndex].get(); // This fallback font is already in our list.
@@ -256,115 +256,10 @@ static inline std::pair<GlyphData, GlyphPage*> glyphDataAndPageForNonCJKCharacte
     return std::make_pair(data, page);
 }
 
-std::pair<GlyphData, GlyphPage*> FontGlyphs::glyphDataAndPageForCharacter(const FontDescription& description, UChar32 c, bool mirror, FontDataVariant variant) const
+std::pair<GlyphData, GlyphPage*> FontGlyphs::glyphDataAndPageForSystemFallback(UChar32 c, const FontDescription& description, FontDataVariant variant, unsigned pageNumber, GlyphPageTreeNode& node)
 {
-    ASSERT(isMainThread());
-
-    if (variant == AutoVariant) {
-        if (description.smallCaps() && !primarySimpleFontData(description)->isSVGFont()) {
-            UChar32 upperC = u_toupper(c);
-            if (upperC != c) {
-                c = upperC;
-                variant = SmallCapsVariant;
-            } else
-                variant = NormalVariant;
-        } else
-            variant = NormalVariant;
-    }
-
-    if (mirror)
-        c = u_charMirror(c);
-
-    unsigned pageNumber = (c / GlyphPage::size);
-
-    GlyphPageTreeNode* node = pageNumber ? m_pages.get(pageNumber) : m_pageZero;
-    if (!node) {
-        node = GlyphPageTreeNode::getRootChild(realizeFontDataAt(description, 0), pageNumber);
-        if (pageNumber)
-            m_pages.set(pageNumber, node);
-        else
-            m_pageZero = node;
-    }
-
-    GlyphPage* page = 0;
-    if (variant == NormalVariant) {
-        // Fastest loop, for the common case (normal variant).
-        while (true) {
-            page = node->page();
-            if (page) {
-                GlyphData data = page->glyphDataForCharacter(c);
-                if (data.fontData && (data.fontData->platformData().orientation() == Horizontal || data.fontData->isTextOrientationFallback()))
-                    return std::make_pair(data, page);
-
-                if (data.fontData) {
-                    if (Font::isCJKIdeographOrSymbol(c)) {
-                        if (!data.fontData->hasVerticalGlyphs()) {
-                            // Use the broken ideograph font data. The broken ideograph font will use the horizontal width of glyphs
-                            // to make sure you get a square (even for broken glyphs like symbols used for punctuation).
-                            variant = BrokenIdeographVariant;
-                            break;
-                        }
-#if PLATFORM(COCOA)
-                        else if (data.fontData->platformData().syntheticOblique())
-                            return glyphDataAndPageForCJKCharacterWithoutSyntheticItalic(c, data, page, pageNumber);
-#endif
-                    } else
-                        return glyphDataAndPageForNonCJKCharacterWithGlyphOrientation(c, description.nonCJKGlyphOrientation(), data, page, pageNumber);
-
-                    return std::make_pair(data, page);
-                }
-
-                if (node->isSystemFallback())
-                    break;
-            }
-
-            node = node->getChild(realizeFontDataAt(description, node->level()), pageNumber);
-            if (pageNumber)
-                m_pages.set(pageNumber, node);
-            else
-                m_pageZero = node;
-        }
-    }
-    if (variant != NormalVariant) {
-        while (true) {
-            page = node->page();
-            if (page) {
-                GlyphData data = page->glyphDataForCharacter(c);
-                if (data.fontData) {
-                    // The variantFontData function should not normally return 0.
-                    // But if it does, we will just render the capital letter big.
-                    RefPtr<SimpleFontData> variantFontData = data.fontData->variantFontData(description, variant);
-                    if (!variantFontData)
-                        return std::make_pair(data, page);
-
-                    GlyphPageTreeNode* variantNode = GlyphPageTreeNode::getRootChild(variantFontData.get(), pageNumber);
-                    GlyphPage* variantPage = variantNode->page();
-                    if (variantPage) {
-                        GlyphData data = variantPage->glyphDataForCharacter(c);
-                        if (data.fontData)
-                            return std::make_pair(data, variantPage);
-                    }
-
-                    // Do not attempt system fallback off the variantFontData. This is the very unlikely case that
-                    // a font has the lowercase character but the small caps font does not have its uppercase version.
-                    return std::make_pair(variantFontData->missingGlyphData(), page);
-                }
-
-                if (node->isSystemFallback())
-                    break;
-            }
-
-            node = node->getChild(realizeFontDataAt(description, node->level()), pageNumber);
-            if (pageNumber)
-                m_pages.set(pageNumber, node);
-            else
-                m_pageZero = node;
-        }
-    }
-
-    ASSERT(page);
-    ASSERT(node->isSystemFallback());
-
+    ASSERT(node.page());
+    ASSERT(node.isSystemFallback());
     // System fallback is character-dependent. When we get here, we
     // know that the character in question isn't in the system fallback
     // font's glyph page. Try to lazily create it here.
@@ -392,23 +287,113 @@ std::pair<GlyphData, GlyphPage*> FontGlyphs::glyphDataAndPageForCharacter(const
         GlyphData data = fallbackPage && fallbackPage->fontDataForCharacter(c) ? fallbackPage->glyphDataForCharacter(c) : characterFontData->missingGlyphData();
         // Cache it so we don't have to do system fallback again next time.
         if (variant == NormalVariant) {
-            page->setGlyphDataForCharacter(c, data.glyph, data.fontData);
-            data.fontData->setMaxGlyphPageTreeLevel(std::max(data.fontData->maxGlyphPageTreeLevel(), node->level()));
+            node.page()->setGlyphDataForCharacter(c, data.glyph, data.fontData);
+            data.fontData->setMaxGlyphPageTreeLevel(std::max(data.fontData->maxGlyphPageTreeLevel(), node.level()));
             if (!Font::isCJKIdeographOrSymbol(c) && data.fontData->platformData().orientation() != Horizontal && !data.fontData->isTextOrientationFallback())
                 return glyphDataAndPageForNonCJKCharacterWithGlyphOrientation(c, description.nonCJKGlyphOrientation(), data, fallbackPage, pageNumber);
         }
-        return std::make_pair(data, page);
+        return std::make_pair(data, node.page());
     }
 
     // Even system fallback can fail; use the missing glyph in that case.
     // FIXME: It would be nicer to use the missing glyph from the last resort font instead.
     GlyphData data = primarySimpleFontData(description)->missingGlyphData();
     if (variant == NormalVariant) {
-        page->setGlyphDataForCharacter(c, data.glyph, data.fontData);
-        data.fontData->setMaxGlyphPageTreeLevel(std::max(data.fontData->maxGlyphPageTreeLevel(), node->level()));
+        node.page()->setGlyphDataForCharacter(c, data.glyph, data.fontData);
+        data.fontData->setMaxGlyphPageTreeLevel(std::max(data.fontData->maxGlyphPageTreeLevel(), node.level()));
     }
-    return std::make_pair(data, page);
+    return std::make_pair(data, node.page());
 }
 
+std::pair<GlyphData, GlyphPage*> FontGlyphs::glyphDataAndPageForVariant(UChar32 c, const FontDescription& description, FontDataVariant variant, unsigned pageNumber, GlyphPageTreeNode*& node)
+{
+    while (true) {
+        if (GlyphPage* page = node->page()) {
+            GlyphData data = page->glyphDataForCharacter(c);
+            if (data.fontData) {
+                // The variantFontData function should not normally return 0.
+                // But if it does, we will just render the capital letter big.
+                RefPtr<SimpleFontData> variantFontData = data.fontData->variantFontData(description, variant);
+                if (!variantFontData)
+                    return std::make_pair(data, page);
+
+                GlyphPageTreeNode* variantNode = GlyphPageTreeNode::getRootChild(variantFontData.get(), pageNumber);
+                GlyphPage* variantPage = variantNode->page();
+                if (variantPage) {
+                    GlyphData data = variantPage->glyphDataForCharacter(c);
+                    if (data.fontData)
+                        return std::make_pair(data, variantPage);
+                }
+
+                // Do not attempt system fallback off the variantFontData. This is the very unlikely case that
+                // a font has the lowercase character but the small caps font does not have its uppercase version.
+                return std::make_pair(variantFontData->missingGlyphData(), page);
+            }
+
+            if (node->isSystemFallback())
+                return glyphDataAndPageForSystemFallback(c, description, variant, pageNumber, *node);
+        }
+
+        node = node->getChild(realizeFontDataAt(description, node->level()), pageNumber);
+    }
+}
+
+std::pair<GlyphData, GlyphPage*> FontGlyphs::glyphDataAndPageForCharacter(const FontDescription& description, UChar32 c, bool mirror, FontDataVariant variant)
+{
+    ASSERT(isMainThread());
+
+    if (variant == AutoVariant) {
+        if (description.smallCaps() && !primarySimpleFontData(description)->isSVGFont()) {
+            UChar32 upperC = u_toupper(c);
+            if (upperC != c) {
+                c = upperC;
+                variant = SmallCapsVariant;
+            } else
+                variant = NormalVariant;
+        } else
+            variant = NormalVariant;
+    }
+
+    if (mirror)
+        c = u_charMirror(c);
+
+    const unsigned pageNumber = (c / GlyphPage::size);
+
+    GlyphPageTreeNode*& node = pageNumber ? m_pages.add(pageNumber, nullptr).iterator->value : m_pageZero;
+    if (!node)
+        node = GlyphPageTreeNode::getRootChild(realizeFontDataAt(description, 0), pageNumber);
+
+    if (variant != NormalVariant)
+        return glyphDataAndPageForVariant(c, description, variant, pageNumber, node);
+
+    while (true) {
+        if (GlyphPage* page = node->page()) {
+            GlyphData data = page->glyphDataForCharacter(c);
+            if (data.fontData) {
+                if (data.fontData->platformData().orientation() == Vertical && !data.fontData->isTextOrientationFallback()) {
+                    if (!Font::isCJKIdeographOrSymbol(c))
+                        return glyphDataAndPageForNonCJKCharacterWithGlyphOrientation(c, description.nonCJKGlyphOrientation(), data, page, pageNumber);
+
+                    if (!data.fontData->hasVerticalGlyphs()) {
+                        // Use the broken ideograph font data. The broken ideograph font will use the horizontal width of glyphs
+                        // to make sure you get a square (even for broken glyphs like symbols used for punctuation).
+                        return glyphDataAndPageForVariant(c, description, BrokenIdeographVariant, pageNumber, node);
+                    }
+#if PLATFORM(COCOA)
+                    if (data.fontData->platformData().syntheticOblique())
+                        return glyphDataAndPageForCJKCharacterWithoutSyntheticItalic(c, data, page, pageNumber);
+#endif
+                }
+
+                return std::make_pair(data, page);
+            }
+
+            if (node->isSystemFallback())
+                return glyphDataAndPageForSystemFallback(c, description, variant, pageNumber, *node);
+        }
+
+        node = node->getChild(realizeFontDataAt(description, node->level()), pageNumber);
+    }
+}
 
 }
index 11d3a9c..386635f 100644 (file)
@@ -75,52 +75,56 @@ public:
 
     bool isForPlatformFont() const { return m_isForPlatformFont; }
 
-    std::pair<GlyphData, GlyphPage*> glyphDataAndPageForCharacter(const FontDescription&, UChar32, bool mirror, FontDataVariant) const;
-    
-    bool isFixedPitch(const FontDescription&) const;
-    void determinePitch(const FontDescription&) const;
+    std::pair<GlyphData, GlyphPage*> glyphDataAndPageForCharacter(const FontDescription&, UChar32, bool mirror, FontDataVariant);
+
+    bool isFixedPitch(const FontDescription&);
+    void determinePitch(const FontDescription&);
 
     bool loadingCustomFonts() const { return m_loadingCustomFonts; }
 
-    FontSelector* fontSelector() const { return m_fontSelector.get(); }
+    FontSelector* fontSelector() { return m_fontSelector.get(); }
     // FIXME: It should be possible to combine fontSelectorVersion and generation.
     unsigned fontSelectorVersion() const { return m_fontSelectorVersion; }
     unsigned generation() const { return m_generation; }
 
-    WidthCache& widthCache() const { return m_widthCache; }
+    WidthCache& widthCache() { return m_widthCache; }
+    const WidthCache& widthCache() const { return m_widthCache; }
 
-    const SimpleFontData* primarySimpleFontData(const FontDescription&) const;
-    const FontData* primaryFontData(const FontDescription& description) const { return realizeFontDataAt(description, 0); }
-    WEBCORE_EXPORT const FontData* realizeFontDataAt(const FontDescription&, unsigned index) const;
+    const SimpleFontData* primarySimpleFontData(const FontDescription&);
+    const FontData* primaryFontData(const FontDescription& description) { return realizeFontDataAt(description, 0); }
+    WEBCORE_EXPORT const FontData* realizeFontDataAt(const FontDescription&, unsigned index);
 
 private:
     FontGlyphs(PassRefPtr<FontSelector>);
     FontGlyphs(const FontPlatformData&);
 
+    std::pair<GlyphData, GlyphPage*> glyphDataAndPageForSystemFallback(UChar32, const FontDescription&, FontDataVariant, unsigned pageNumber, GlyphPageTreeNode&);
+    std::pair<GlyphData, GlyphPage*> glyphDataAndPageForVariant(UChar32, const FontDescription&, FontDataVariant, unsigned pageNumber, GlyphPageTreeNode*&);
+
     WEBCORE_EXPORT void releaseFontData();
     
-    mutable Vector<RefPtr<FontData>, 1> m_realizedFontData;
-    mutable GlyphPages m_pages;
-    mutable GlyphPageTreeNode* m_pageZero;
-    mutable const SimpleFontData* m_cachedPrimarySimpleFontData;
+    Vector<RefPtr<FontData>, 1> m_realizedFontData;
+    GlyphPages m_pages;
+    GlyphPageTreeNode* m_pageZero;
+    const SimpleFontData* m_cachedPrimarySimpleFontData;
     RefPtr<FontSelector> m_fontSelector;
-    mutable WidthCache m_widthCache;
+    WidthCache m_widthCache;
     unsigned m_fontSelectorVersion;
-    mutable int m_familyIndex;
+    int m_familyIndex;
     unsigned short m_generation;
-    mutable unsigned m_pitch : 3; // Pitch
-    mutable bool m_loadingCustomFonts : 1;
-    bool m_isForPlatformFont : 1;
+    unsigned m_pitch : 3; // Pitch
+    unsigned m_loadingCustomFonts : 1;
+    unsigned m_isForPlatformFont : 1;
 };
 
-inline bool FontGlyphs::isFixedPitch(const FontDescription& description) const
+inline bool FontGlyphs::isFixedPitch(const FontDescription& description)
 {
     if (m_pitch == UnknownPitch)
         determinePitch(description);
     return m_pitch == FixedPitch;
 };
 
-inline const SimpleFontData* FontGlyphs::primarySimpleFontData(const FontDescription& description) const
+inline const SimpleFontData* FontGlyphs::primarySimpleFontData(const FontDescription& description)
 {
     ASSERT(isMainThread());
     if (!m_cachedPrimarySimpleFontData)