Changeset 177637 in webkit

Timestamp:

Dec 22, 2014 11:14:58 AM (9 years ago)

Author:

Antti Koivisto

Message:

Generic font code should not know about SVG font missing glyph
​https://bugs.webkit.org/show_bug.cgi?id=139864

Reviewed by Andreas Kling and Myles Maxfield.

The defined missing glyph is an SVG font concept and should be handled in SVG code.

• platform/graphics/FontGlyphs.cpp:

(WebCore::FontGlyphs::glyphDataForSystemFallback):
(WebCore::FontGlyphs::glyphDataForVariant):

Return null glyph instead of the missing glyph (the missing glyph was already a null glyph in all non-svg-font cases).
Use early return style.

• platform/graphics/FontGlyphs.h:
• platform/graphics/SegmentedFontData.cpp:
• platform/graphics/SimpleFontData.cpp:

(WebCore::SimpleFontData::platformGlyphInit):

• platform/graphics/SimpleFontData.h:

Remove the missingGlyph member.

• platform/graphics/WidthIterator.cpp:

(WebCore::WidthIterator::advanceInternal):

Explicitly skip over null glyphs. Before they had non-null fontData and would get skipped implicitly.

• platform/graphics/mac/SimpleFontDataMac.mm:
• rendering/svg/SVGTextRunRenderingContext.cpp:

(WebCore::missingGlyphForFont):

Get the missing glyph from the SVG font element.

(WebCore::SVGTextRunRenderingContext::glyphDataForCharacter):

Return the missing glyph if the normal lookup didn't produce results.

• svg/SVGFontData.cpp:

(WebCore::SVGFontData::initializeFontData):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• platform/graphics/FontGlyphs.cpp (modified) (3 diffs)
• platform/graphics/FontGlyphs.h (modified) (1 diff)
• platform/graphics/SegmentedFontData.cpp (modified) (1 diff)
• platform/graphics/SimpleFontData.cpp (modified) (3 diffs)
• platform/graphics/SimpleFontData.h (modified) (4 diffs)
• platform/graphics/WidthIterator.cpp (modified) (1 diff)
• platform/graphics/mac/SimpleFontDataMac.mm (modified) (1 diff)
• rendering/svg/SVGTextRunRenderingContext.cpp (modified) (2 diffs)
• svg/SVGFontData.cpp (modified) (2 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2014-12-22  Antti Koivisto  <antti@apple.com>
+
+        Generic font code should not know about SVG font missing glyph
+        https://bugs.webkit.org/show_bug.cgi?id=139864
+
+        Reviewed by Andreas Kling and Myles Maxfield.
+
+        The defined missing glyph is an SVG font concept and should be handled in SVG code.
+
+        * platform/graphics/FontGlyphs.cpp:
+        (WebCore::FontGlyphs::glyphDataForSystemFallback):
+        (WebCore::FontGlyphs::glyphDataForVariant):
+
+            Return null glyph instead of the missing glyph (the missing glyph was already a null glyph in all non-svg-font cases).
+            Use early return style.
+
+        * platform/graphics/FontGlyphs.h:
+        * platform/graphics/SegmentedFontData.cpp:
+        * platform/graphics/SimpleFontData.cpp:
+        (WebCore::SimpleFontData::platformGlyphInit):
+        * platform/graphics/SimpleFontData.h:
+
+            Remove the missingGlyph member.
+
+        * platform/graphics/WidthIterator.cpp:
+        (WebCore::WidthIterator::advanceInternal):
+
+            Explicitly skip over null glyphs. Before they had non-null fontData and would get skipped implicitly.
+
+        * platform/graphics/mac/SimpleFontDataMac.mm:
+        * rendering/svg/SVGTextRunRenderingContext.cpp:
+        (WebCore::missingGlyphForFont):
+
+            Get the missing glyph from the SVG font element.
+
+        (WebCore::SVGTextRunRenderingContext::glyphDataForCharacter):
+
+            Return the missing glyph if the normal lookup didn't produce results.
+
+        * svg/SVGFontData.cpp:
+        (WebCore::SVGFontData::initializeFontData):
+
 2014-12-22  Chris Dumez  <cdumez@apple.com>
 

trunk/Source/WebCore/platform/graphics/FontGlyphs.cpp

@@ -32 +32 @@
 #include "Font.h"
 #include "FontCache.h"
+#include "GlyphPageTreeNode.h"
 #include "SegmentedFontData.h"
 
@@ -278 +279 @@
     const SimpleFontData* originalFontData = primaryFontData(description)->fontDataForCharacter(c);
     RefPtr<SimpleFontData> characterFontData = fontCache().systemFallbackForCharacters(description, originalFontData, m_isForPlatformFont, codeUnits, codeUnitsLength);
-    if (characterFontData) {
-        if (characterFontData->platformData().orientation() == Vertical && !characterFontData->hasVerticalGlyphs() && Font::isCJKIdeographOrSymbol(c))
-            variant = BrokenIdeographVariant;
-        if (variant != NormalVariant)
-            characterFontData = characterFontData->variantFontData(description, variant);
-    }
-    if (characterFontData) {
-        // Got the fallback glyph and font.
-        GlyphPage* fallbackPage = GlyphPageTreeNode::getRootChild(characterFontData.get(), pageNumber)->page();
-        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) {
-            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 glyphDataForNonCJKCharacterWithGlyphOrientation(c, description.nonCJKGlyphOrientation(), data, pageNumber);
-        }
-        return data;
-    }
-
-    // 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) {
+    if (!characterFontData)
+        return GlyphData();
+
+    if (characterFontData->platformData().orientation() == Vertical && !characterFontData->hasVerticalGlyphs() && Font::isCJKIdeographOrSymbol(c))
+        variant = BrokenIdeographVariant;
+    if (variant != NormalVariant) {
+        characterFontData = characterFontData->variantFontData(description, variant);
+        ASSERT(characterFontData);
+    }
+
+    GlyphData data;
+    if (GlyphPage* fallbackPage = GlyphPageTreeNode::getRootChild(characterFontData.get(), pageNumber)->page())
+        data = fallbackPage->glyphDataForCharacter(c);
+
+    // Cache it so we don't have to do system fallback again next time.
+    if (variant == NormalVariant && data.glyph) {
         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 glyphDataForNonCJKCharacterWithGlyphOrientation(c, description.nonCJKGlyphOrientation(), data, pageNumber);
     }
     return data;
@@ -322 +317 @@
                 GlyphPageTreeNode* variantNode = GlyphPageTreeNode::getRootChild(variantFontData.get(), pageNumber);
                 GlyphPage* variantPage = variantNode->page();
-                if (variantPage) {
-                    GlyphData data = variantPage->glyphDataForCharacter(c);
-                    if (data.fontData)
-                        return data;
-                }
+                if (variantPage)
+                    return variantPage->glyphDataForCharacter(c);
 
                 // 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 variantFontData->missingGlyphData();
+                return GlyphData();
             }
 

trunk/Source/WebCore/platform/graphics/FontGlyphs.h

@@ -23 +23 @@
 
 #include "FontSelector.h"
+#include "GlyphPage.h"
 #include "SimpleFontData.h"
 #include "WidthCache.h"

trunk/Source/WebCore/platform/graphics/SegmentedFontData.cpp

@@ -27 +27 @@
 #include "SegmentedFontData.h"
 
+#include "GlyphPageTreeNode.h"
 #include "SimpleFontData.h"
 #include <wtf/Assertions.h>

trunk/Source/WebCore/platform/graphics/SimpleFontData.cpp

@@ -36 +36 @@
 #include "Font.h"
 #include "FontCache.h"
+#include "GlyphPageTreeNode.h"
 #include "OpenTypeMathData.h"
 #include <wtf/MathExtras.h>
@@ -125 +126 @@
         determinePitch();
         m_zeroWidthSpaceGlyph = 0;
-        m_missingGlyphData.fontData = this;
-        m_missingGlyphData.glyph = 0;
         return;
     }
@@ -150 +149 @@
     if (m_zeroWidthSpaceGlyph == m_spaceGlyph)
         m_zeroWidthSpaceGlyph = 0;
-
-    m_missingGlyphData.fontData = this;
-    m_missingGlyphData.glyph = 0;
 }
 

trunk/Source/WebCore/platform/graphics/SimpleFontData.h

@@ -32 +32 @@
 #include "GlyphBuffer.h"
 #include "GlyphMetricsMap.h"
-#include "GlyphPageTreeNode.h"
 #include "OpenTypeMathData.h"
 #if ENABLE(OPENTYPE_VERTICAL)
@@ -61 +60 @@
 namespace WebCore {
 
+class GlyphPage;
 class FontDescription;
 class SharedBuffer;
+struct GlyphData;
 struct WidthIterator;
 
@@ -181 +182 @@
     virtual bool isSegmented() const override;
 
-    const GlyphData& missingGlyphData() const { return m_missingGlyphData; }
-    void setMissingGlyphData(const GlyphData& glyphData) { m_missingGlyphData = glyphData; }
-
 #ifndef NDEBUG
     virtual String description() const override;
@@ -272 +270 @@
 
     Glyph m_zeroWidthSpaceGlyph;
-
-    GlyphData m_missingGlyphData;
 
     struct DerivedFontData {

trunk/Source/WebCore/platform/graphics/WidthIterator.cpp

@@ -180 +180 @@
         const GlyphData& glyphData = glyphDataForCharacter(character, rtl, currentCharacter, advanceLength, normalizedSpacesStringCache);
         Glyph glyph = glyphData.glyph;
+        if (!glyph) {
+            textIterator.advance(advanceLength);
+            continue;
+        }
         const SimpleFontData* fontData = glyphData.fontData;
-
         ASSERT(fontData);
 

trunk/Source/WebCore/platform/graphics/mac/SimpleFontDataMac.mm

@@ -36 +36 @@
 #import "FontCache.h"
 #import "FontDescription.h"
+#import "GlyphPageTreeNode.h"
 #import "SharedBuffer.h"
 #import "WebCoreSystemInterface.h"

trunk/Source/WebCore/rendering/svg/SVGTextRunRenderingContext.cpp

@@ -291 +291 @@
 }
 
+static GlyphData missingGlyphForFont(const Font& font)
+{
+    const SimpleFontData* primaryFontData = font.primaryFont();
+    if (!primaryFontData->isSVGFont())
+        return GlyphData();
+    SVGFontElement* fontElement;
+    SVGFontFaceElement* fontFaceElement;
+    svgFontAndFontFaceElementForFontData(primaryFontData, fontFaceElement, fontElement);
+    return GlyphData(fontElement->missingGlyph(), primaryFontData);
+}
+
 GlyphData SVGTextRunRenderingContext::glyphDataForCharacter(const Font& font, WidthIterator& iterator, UChar32 character, bool mirror, int currentCharacter, unsigned& advanceLength, String& normalizedSpacesStringCache)
 {
-    const SimpleFontData* primaryFont = font.primaryFont();
-    ASSERT(primaryFont);
-
     GlyphData glyphData = font.glyphDataForCharacter(character, mirror, AutoVariant);
-
-    // Check if we have the missing glyph data, in which case we can just return.
-    GlyphData missingGlyphData = primaryFont->missingGlyphData();
-    if (glyphData.glyph == missingGlyphData.glyph && glyphData.fontData == missingGlyphData.fontData) {
-        ASSERT(glyphData.fontData);
-        return glyphData;
-    }
+    if (!glyphData.glyph)
+        return missingGlyphForFont(font);
+
+    ASSERT(glyphData.fontData);
 
     // Characters enclosed by an <altGlyph> element, may not be registered in the GlyphPage.
-    if (glyphData.fontData && !glyphData.fontData->isSVGFont()) {
+    if (!glyphData.fontData->isSVGFont()) {
         auto& elementRenderer = is<RenderElement>(renderer()) ? downcast<RenderElement>(renderer()) : *renderer().parent();
         if (Element* parentRendererElement = elementRenderer.element()) {
             if (is<SVGAltGlyphElement>(*parentRendererElement))
-                glyphData.fontData = primaryFont;
+                glyphData.fontData = font.primaryFont();
         }
     }
 
-    const SimpleFontData* fontData = glyphData.fontData;
-    if (!fontData || !fontData->isSVGFont())
+    if (!glyphData.fontData->isSVGFont())
         return glyphData;
 
     SVGFontElement* fontElement = nullptr;
     SVGFontFaceElement* fontFaceElement = nullptr;
-    const SVGFontData* svgFontData = svgFontAndFontFaceElementForFontData(fontData, fontFaceElement, fontElement);
+    const SVGFontData* svgFontData = svgFontAndFontFaceElementForFontData(glyphData.fontData, fontFaceElement, fontElement);
     if (!svgFontData)
         return glyphData;
@@ -330 +334 @@
     if (svgFontData->applySVGGlyphSelection(iterator, glyphData, mirror, currentCharacter, advanceLength, normalizedSpacesStringCache))
         return glyphData;
+
+    GlyphData missingGlyphData = missingGlyphForFont(font);
     if (missingGlyphData.glyph)
         return missingGlyphData;

trunk/Source/WebCore/svg/SVGFontData.cpp

@@ -23 +23 @@
 #include "SVGFontData.h"
 
+#include "GlyphPageTreeNode.h"
 #include "RenderElement.h"
 #include "SVGAltGlyphElement.h"
@@ -64 +65 @@
     ASSERT(svgFontFaceElement);
 
-    SVGFontElement* svgFontElement = svgFontFaceElement->associatedFontElement();
-    ASSERT(svgFontElement);
-    GlyphData missingGlyphData;
-    missingGlyphData.fontData = fontData;
-    missingGlyphData.glyph = svgFontElement->missingGlyph();
-    fontData->setMissingGlyphData(missingGlyphData);
-
     fontData->setZeroWidthSpaceGlyph(0);
     fontData->determinePitch();