Title: [205703] trunk/Source/WebCore
Revision
205703
Author
mmaxfi...@apple.com
Date
2016-09-09 01:33:44 -0700 (Fri, 09 Sep 2016)

Log Message

[Cocoa] Improve performance of glyph advance metrics gathering
https://bugs.webkit.org/show_bug.cgi?id=161119

Reviewed by Simon Fraser.

Most of the glyphs in a GlyphPage are never read from. Therefore, we can get a performance boost
by not populating as many items in the GlyphPage. Because of the performance characteristics of
CTFontGetGlyphsForCharacters(), a better size for a GlyphPage is 16 items. This, coupled with
using CTFontGetUnsummedAdvancesForGlyphsAndStyle(), causes between a 0.01%-0.5% speedup on PLT.

No new tests because there is no behavior change.

* platform/graphics/Font.cpp:
(WebCore::Font::initCharWidths):
(WebCore::Font::platformGlyphInit):
(WebCore::createAndFillGlyphPage):
* platform/graphics/Font.h:
(WebCore::Font::widthForGlyph):
* platform/graphics/GlyphMetricsMap.h:
* platform/graphics/GlyphPage.h:
* platform/graphics/cocoa/FontCocoa.mm:
* platform/spi/cocoa/CoreTextSPI.h:
(WebCore::Font::platformWidthForGlyph):
(WebCore::canUseFastGlyphAdvanceGetter): Deleted.

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (205702 => 205703)


--- trunk/Source/WebCore/ChangeLog	2016-09-09 08:15:29 UTC (rev 205702)
+++ trunk/Source/WebCore/ChangeLog	2016-09-09 08:33:44 UTC (rev 205703)
@@ -1,3 +1,30 @@
+2016-09-09  Myles C. Maxfield  <mmaxfi...@apple.com>
+
+        [Cocoa] Improve performance of glyph advance metrics gathering
+        https://bugs.webkit.org/show_bug.cgi?id=161119
+
+        Reviewed by Simon Fraser.
+
+        Most of the glyphs in a GlyphPage are never read from. Therefore, we can get a performance boost
+        by not populating as many items in the GlyphPage. Because of the performance characteristics of
+        CTFontGetGlyphsForCharacters(), a better size for a GlyphPage is 16 items. This, coupled with
+        using CTFontGetUnsummedAdvancesForGlyphsAndStyle(), causes between a 0.01%-0.5% speedup on PLT.
+
+        No new tests because there is no behavior change.
+
+        * platform/graphics/Font.cpp:
+        (WebCore::Font::initCharWidths):
+        (WebCore::Font::platformGlyphInit):
+        (WebCore::createAndFillGlyphPage):
+        * platform/graphics/Font.h:
+        (WebCore::Font::widthForGlyph):
+        * platform/graphics/GlyphMetricsMap.h:
+        * platform/graphics/GlyphPage.h:
+        * platform/graphics/cocoa/FontCocoa.mm:
+        * platform/spi/cocoa/CoreTextSPI.h:
+        (WebCore::Font::platformWidthForGlyph):
+        (WebCore::canUseFastGlyphAdvanceGetter): Deleted.
+
 2016-09-09  Antti Koivisto  <an...@apple.com>
 
         v3: WebContent crash due to RELEASE_ASSERT in WebCore: WebCore::StyleResolver::styleForElement

Modified: trunk/Source/WebCore/platform/graphics/Font.cpp (205702 => 205703)


--- trunk/Source/WebCore/platform/graphics/Font.cpp	2016-09-09 08:15:29 UTC (rev 205702)
+++ trunk/Source/WebCore/platform/graphics/Font.cpp	2016-09-09 08:33:44 UTC (rev 205703)
@@ -81,12 +81,11 @@
 // 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(GlyphPage::pageNumberForCodePoint('0'));
 
     // 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,22 +101,23 @@
 void Font::platformGlyphInit()
 {
     auto* glyphPageZero = glyphPage(0);
-    if (!glyphPageZero) {
-        determinePitch();
-        return;
-    }
+    auto* glyphPageCharacterZero = glyphPage(GlyphPage::pageNumberForCodePoint('0'));
+    auto* glyphPageSpace = glyphPage(GlyphPage::pageNumberForCodePoint(space));
 
     // Ask for the glyph for 0 to avoid paging in ZERO WIDTH SPACE. Control characters, including 0,
     // are mapped to the ZERO WIDTH SPACE glyph.
-    m_zeroWidthSpaceGlyph = glyphPageZero->glyphDataForCharacter(0).glyph;
+    if (glyphPageZero)
+        m_zeroWidthSpaceGlyph = glyphPageZero->glyphDataForCharacter(0).glyph;
 
     // 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;
+    if (glyphPageSpace)
+        m_spaceGlyph = glyphPageSpace->glyphDataForCharacter(space).glyph;
     float width = widthForGlyph(m_spaceGlyph);
     m_spaceWidth = width;
-    m_zeroGlyph = glyphPageZero->glyphDataForCharacter('0').glyph;
+    if (glyphPageCharacterZero)
+        m_zeroGlyph = glyphPageCharacterZero->glyphDataForCharacter('0').glyph;
     m_fontMetrics.setZeroWidth(widthForGlyph(m_zeroGlyph));
     determinePitch();
     m_adjustedSpaceWidth = m_treatAsFixedPitch ? ceilf(width) : roundf(width);
@@ -152,56 +152,59 @@
     // 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 (GlyphPage::pageNumberIsUsedForArabic(pageNumber) && font.shouldNotBeUsedForArabic())
         return nullptr;
 #endif
 
-    unsigned start = pageNumber * GlyphPage::size;
-    UChar buffer[GlyphPage::size * 2 + 2];
+    unsigned glyphPageSize = GlyphPage::sizeForPageNumber(pageNumber);
+
+    unsigned start = GlyphPage::startingCodePointInPageNumber(pageNumber);
+    unsigned end = start + glyphPageSize;
+    Vector<UChar> buffer(glyphPageSize * 2 + 2);
     unsigned bufferLength;
     // Fill in a buffer with the entire "page" of characters that we want to look up glyphs for.
     if (U_IS_BMP(start)) {
-        bufferLength = GlyphPage::size;
-        for (unsigned i = 0; i < GlyphPage::size; i++)
+        bufferLength = glyphPageSize;
+        for (unsigned i = 0; i < bufferLength; i++)
             buffer[i] = start + i;
 
-        if (!start) {
-            // Control characters must not render at all.
-            for (unsigned i = 0; i < 0x20; ++i)
-                buffer[i] = zeroWidthSpace;
-            for (unsigned i = 0x7F; i < 0xA0; i++)
-                buffer[i] = zeroWidthSpace;
-            buffer[softHyphen] = zeroWidthSpace;
+        // Code points 0x0 - 0x20 and 0x7F - 0xA0 are control character and shouldn't render. Map them to ZERO WIDTH SPACE.
+        auto overwriteCodePoints = [&](unsigned minimum, unsigned maximum, UChar newCodePoint) {
+            unsigned begin = std::max(start, minimum);
+            unsigned complete = std::min(end, maximum);
+            for (unsigned i = begin; i < complete; ++i)
+                buffer[i - start] = newCodePoint;
+        };
 
-            // \n, \t, and nonbreaking space must render as a space.
-            buffer[(int)'\n'] = ' ';
-            buffer[(int)'\t'] = ' ';
-            buffer[noBreakSpace] = ' ';
-        } 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;
-            buffer[leftToRightEmbed - start] = zeroWidthSpace;
-            buffer[rightToLeftEmbed - start] = zeroWidthSpace;
-            buffer[leftToRightOverride - start] = zeroWidthSpace;
-            buffer[rightToLeftOverride - start] = zeroWidthSpace;
-            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 == (objectReplacementCharacter & ~(GlyphPage::size - 1))) {
-            // Object replacement character must not render at all.
-            buffer[objectReplacementCharacter - start] = zeroWidthSpace;
-        } else if (start == (zeroWidthNoBreakSpace & ~(GlyphPage::size - 1))) {
-            // ZWNBS/BOM must not render at all.
-            buffer[zeroWidthNoBreakSpace - start] = zeroWidthSpace;
-        }
+        auto overwriteCodePoint = [&](UChar codePoint, UChar newCodePoint) {
+            if (codePoint >= start && codePoint < end)
+                buffer[codePoint - start] = newCodePoint;
+        };
+
+        overwriteCodePoints(0x0, 0x20, zeroWidthSpace);
+        overwriteCodePoints(0x7F, 0xA0, zeroWidthSpace);
+        overwriteCodePoint(softHyphen, zeroWidthSpace);
+        overwriteCodePoint('\n', space);
+        overwriteCodePoint('\t', space);
+        overwriteCodePoint(noBreakSpace, space);
+        overwriteCodePoint(leftToRightMark, zeroWidthSpace);
+        overwriteCodePoint(rightToLeftMark, zeroWidthSpace);
+        overwriteCodePoint(leftToRightEmbed, zeroWidthSpace);
+        overwriteCodePoint(rightToLeftEmbed, zeroWidthSpace);
+        overwriteCodePoint(leftToRightOverride, zeroWidthSpace);
+        overwriteCodePoint(rightToLeftOverride, zeroWidthSpace);
+        overwriteCodePoint(leftToRightIsolate, zeroWidthSpace);
+        overwriteCodePoint(rightToLeftIsolate, zeroWidthSpace);
+        overwriteCodePoint(zeroWidthNonJoiner, zeroWidthSpace);
+        overwriteCodePoint(zeroWidthJoiner, zeroWidthSpace);
+        overwriteCodePoint(popDirectionalFormatting, zeroWidthSpace);
+        overwriteCodePoint(popDirectionalIsolate, zeroWidthSpace);
+        overwriteCodePoint(firstStrongIsolate, zeroWidthSpace);
+        overwriteCodePoint(objectReplacementCharacter, zeroWidthSpace);
+        overwriteCodePoint(zeroWidthNoBreakSpace, zeroWidthSpace);
     } else {
-        bufferLength = GlyphPage::size * 2;
-        for (unsigned i = 0; i < GlyphPage::size; i++) {
+        bufferLength = glyphPageSize * 2;
+        for (unsigned i = 0; i < glyphPageSize; i++) {
             int c = i + start;
             buffer[i * 2] = U16_LEAD(c);
             buffer[i * 2 + 1] = U16_TRAIL(c);
@@ -215,7 +218,7 @@
     // for only 128 out of 256 characters.
     Ref<GlyphPage> glyphPage = GlyphPage::create(font);
 
-    bool haveGlyphs = fillGlyphPage(glyphPage, buffer, bufferLength, font);
+    bool haveGlyphs = fillGlyphPage(glyphPage, buffer.data(), bufferLength, font);
     if (!haveGlyphs)
         return nullptr;
 
@@ -238,7 +241,7 @@
 
 Glyph Font::glyphForCharacter(UChar32 character) const
 {
-    auto* page = glyphPage(character / GlyphPage::size);
+    auto* page = glyphPage(GlyphPage::pageNumberForCodePoint(character));
     if (!page)
         return 0;
     return page->glyphForCharacter(character);
@@ -246,7 +249,7 @@
 
 GlyphData Font::glyphDataForCharacter(UChar32 character) const
 {
-    auto* page = glyphPage(character / GlyphPage::size);
+    auto* page = glyphPage(GlyphPage::pageNumberForCodePoint(character));
     if (!page)
         return GlyphData();
     return page->glyphDataForCharacter(character);

Modified: trunk/Source/WebCore/platform/graphics/Font.h (205702 => 205703)


--- trunk/Source/WebCore/platform/graphics/Font.h	2016-09-09 08:15:29 UTC (rev 205702)
+++ trunk/Source/WebCore/platform/graphics/Font.h	2016-09-09 08:33:44 UTC (rev 205703)
@@ -337,13 +337,13 @@
         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);
 

Modified: trunk/Source/WebCore/platform/graphics/FontCascadeFonts.cpp (205702 => 205703)


--- trunk/Source/WebCore/platform/graphics/FontCascadeFonts.cpp	2016-09-09 08:15:29 UTC (rev 205702)
+++ trunk/Source/WebCore/platform/graphics/FontCascadeFonts.cpp	2016-09-09 08:33:44 UTC (rev 205703)
@@ -48,7 +48,7 @@
 
     GlyphData glyphDataForCharacter(UChar32 c) const
     {
-        unsigned index = GlyphPage::indexForCharacter(c);
+        unsigned index = GlyphPage::indexForCodePoint(c);
         ASSERT_WITH_SECURITY_IMPLICATION(index < GlyphPage::size);
         return { m_glyphs[index], m_fonts[index] };
     }
@@ -55,7 +55,7 @@
 
     void setGlyphDataForCharacter(UChar32 c, GlyphData glyphData)
     {
-        setGlyphDataForIndex(GlyphPage::indexForCharacter(c), glyphData);
+        setGlyphDataForIndex(GlyphPage::indexForCodePoint(c), glyphData);
     }
 
 private:
@@ -428,7 +428,7 @@
     if (variant != NormalVariant)
         return glyphDataForVariant(c, description, variant, 0);
 
-    const unsigned pageNumber = c / GlyphPage::size;
+    const unsigned pageNumber = GlyphPage::pageNumberForCodePoint(c);
 
     auto& cacheEntry = pageNumber ? m_cachedPages.add(pageNumber, GlyphPageCacheEntry()).iterator->value : m_cachedPageZero;
 

Modified: trunk/Source/WebCore/platform/graphics/GlyphMetricsMap.h (205702 => 205703)


--- trunk/Source/WebCore/platform/graphics/GlyphMetricsMap.h	2016-09-09 08:15:29 UTC (rev 205702)
+++ trunk/Source/WebCore/platform/graphics/GlyphMetricsMap.h	2016-09-09 08:33:44 UTC (rev 205703)
@@ -54,7 +54,7 @@
     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)

Modified: trunk/Source/WebCore/platform/graphics/GlyphPage.h (205702 => 205703)


--- trunk/Source/WebCore/platform/graphics/GlyphPage.h	2016-09-09 08:15:29 UTC (rev 205702)
+++ trunk/Source/WebCore/platform/graphics/GlyphPage.h	2016-09-09 08:33:44 UTC (rev 205703)
@@ -57,7 +57,7 @@
 
 // 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.
+// starting from 0 and incrementing for each "size" number of glyphs.
 class GlyphPage : public RefCounted<GlyphPage> {
 public:
     static Ref<GlyphPage> create(const Font& font)
@@ -72,17 +72,22 @@
 
     static unsigned count() { return s_count; }
 
-    static const size_t size = 256; // Covers Latin-1 in a single page.
-    static unsigned indexForCharacter(UChar32 c) { return c % GlyphPage::size; }
+    static const unsigned size = 16;
 
+    static unsigned sizeForPageNumber(unsigned) { return 16; }
+    static unsigned indexForCodePoint(UChar32 c) { return c % size; }
+    static unsigned pageNumberForCodePoint(UChar32 c) { return c / size; }
+    static UChar32 startingCodePointInPageNumber(unsigned pageNumber) { return pageNumber * size; }
+    static bool pageNumberIsUsedForArabic(unsigned pageNumber) { return startingCodePointInPageNumber(pageNumber) >= 0x600 && startingCodePointInPageNumber(pageNumber) + sizeForPageNumber(pageNumber) < 0x700; }
+
     GlyphData glyphDataForCharacter(UChar32 c) const
     {
-        return glyphDataForIndex(indexForCharacter(c));
+        return glyphDataForIndex(indexForCodePoint(c));
     }
 
     Glyph glyphForCharacter(UChar32 c) const
     {
-        return glyphForIndex(indexForCharacter(c));
+        return glyphForIndex(indexForCodePoint(c));
     }
 
     GlyphData glyphDataForIndex(unsigned index) const

Modified: trunk/Source/WebCore/platform/graphics/cocoa/FontCocoa.mm (205702 => 205703)


--- trunk/Source/WebCore/platform/graphics/cocoa/FontCocoa.mm	2016-09-09 08:15:29 UTC (rev 205702)
+++ trunk/Source/WebCore/platform/graphics/cocoa/FontCocoa.mm	2016-09-09 08:33:44 UTC (rev 205703)
@@ -596,6 +596,7 @@
     return boundingBox;
 }
 
+#if !((PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101200) || (PLATFORM(IOS) && __IPHONE_OS_VERSION_MIN_REQUIRED >= 100000))
 static inline Optional<CGSize> advanceForColorBitmapFont(const FontPlatformData& platformData, Glyph glyph)
 {
     CTFontRef font = platformData.font();
@@ -617,16 +618,30 @@
     }
     return true;
 }
+#endif
 
 float Font::platformWidthForGlyph(Glyph glyph) const
 {
     CGSize advance = CGSizeZero;
     bool horizontal = platformData().orientation() == Horizontal;
+    CGFontRenderingStyle style = kCGFontRenderingStyleAntialiasing | kCGFontRenderingStyleSubpixelPositioning | kCGFontRenderingStyleSubpixelQuantization | kCGFontAntialiasingStyleUnfiltered;
+
+#if (PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101200) || (PLATFORM(IOS) && __IPHONE_OS_VERSION_MIN_REQUIRED >= 100000)
+    if (platformData().size()) {
+        CTFontOrientation orientation = horizontal || m_isBrokenIdeographFallback ? kCTFontOrientationHorizontal : kCTFontOrientationVertical;
+        // FIXME: Remove this special-casing when <rdar://problem/28197291> is fixed.
+        if (CTFontIsAppleColorEmoji(m_platformData.ctFont()))
+            CTFontGetAdvancesForGlyphs(m_platformData.ctFont(), orientation, &glyph, &advance, 1);
+        else
+            CTFontGetUnsummedAdvancesForGlyphsAndStyle(m_platformData.ctFont(), orientation, style, &glyph, &advance, 1);
+    }
+
+#else
+
     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);
@@ -634,6 +649,7 @@
         }
     } else if (!populatedAdvance && platformData().size())
         CTFontGetAdvancesForGlyphs(m_platformData.ctFont(), horizontal ? kCTFontOrientationHorizontal : kCTFontOrientationVertical, &glyph, &advance, 1);
+#endif
 
     return advance.width + m_syntheticBoldOffset;
 }

Modified: trunk/Source/WebCore/platform/spi/cocoa/CoreTextSPI.h (205702 => 205703)


--- trunk/Source/WebCore/platform/spi/cocoa/CoreTextSPI.h	2016-09-09 08:15:29 UTC (rev 205702)
+++ trunk/Source/WebCore/platform/spi/cocoa/CoreTextSPI.h	2016-09-09 08:33:44 UTC (rev 205703)
@@ -71,6 +71,7 @@
 void CTRunGetBaseAdvancesAndOrigins(CTRunRef, CFRange, CGSize baseAdvances[], CGPoint origins[]);
 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);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to