Improve the performance of Font::canRenderCombiningCharacterSequence()
authormmaxfield@apple.com <mmaxfield@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 26 May 2018 03:44:49 +0000 (03:44 +0000)
committermmaxfield@apple.com <mmaxfield@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 26 May 2018 03:44:49 +0000 (03:44 +0000)
https://bugs.webkit.org/show_bug.cgi?id=185933

Reviewed by Ryosuke Niwa.

PerformanceTests:

* Layout/ComplexLongUnique.html: Added.

Source/WebCore:

We don't need to create a whole CTLine just to determine whether or not a font supports rendering a grapheme cluster.
Instead, the right way to do it is just see if the font's cmap table supports every code point in the cluster.

This patch reports a 2% progression on the attached PerformanceTest.

Test: Layout/ComplexLongUnique.html

* platform/graphics/Font.cpp:
(WebCore::Font::canRenderCombiningCharacterSequence const):
* platform/graphics/Font.h:
* platform/graphics/cocoa/FontCocoa.mm:
(WebCore::provideStringAndAttributes): Deleted.
(WebCore::Font::canRenderCombiningCharacterSequence const): Deleted.
* platform/graphics/freetype/SimpleFontDataFreeType.cpp:
(WebCore::Font::canRenderCombiningCharacterSequence const): Deleted.

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

PerformanceTests/ChangeLog
PerformanceTests/Layout/ComplexLongUnique.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/Font.cpp
Source/WebCore/platform/graphics/Font.h
Source/WebCore/platform/graphics/cocoa/FontCocoa.mm
Source/WebCore/platform/graphics/freetype/SimpleFontDataFreeType.cpp

index c19cc6c..0827290 100644 (file)
@@ -1,3 +1,12 @@
+2018-05-25  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        Improve the performance of Font::canRenderCombiningCharacterSequence()
+        https://bugs.webkit.org/show_bug.cgi?id=185933
+
+        Reviewed by Ryosuke Niwa.
+
+        * Layout/ComplexLongUnique.html: Added.
+
 2018-05-25  Saam Barati  <sbarati@apple.com>
 
         Have a memory test where we can validate JSCs mini memory mode
diff --git a/PerformanceTests/Layout/ComplexLongUnique.html b/PerformanceTests/Layout/ComplexLongUnique.html
new file mode 100644 (file)
index 0000000..31d4592
--- /dev/null
@@ -0,0 +1,35 @@
+<!DOCTYPE html>
+<html>
+<head>
+<meta charset="utf-8">
+<script src="../resources/runner.js"></script>
+</head>
+<body>
+<div id="target" style="width: 1000000px; display: none;"></div>
+<script>
+var target = document.getElementById("target");
+var style = target.style;
+
+var s = "e\u0300";
+var length = 10000;
+var startCode = 0x4E00;
+for (var i = 0; i < length; ++i) {
+    s = s + String.fromCharCode(i + startCode) + "\u0300";
+}
+
+function test() {
+    if (window.internals)
+        window.internals.invalidateFontCache();
+
+    style.display = "block";
+    target.offsetLeft;
+    target.textContent = s;
+    target.offsetLeft;
+    target.textContent = "";
+    style.display = "none";
+}
+
+PerfTestRunner.measureRunsPerSecond({ run: test });
+</script>
+</body>
+</html>
index c0617d4..0b5eb7a 100644 (file)
@@ -1,3 +1,26 @@
+2018-05-25  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        Improve the performance of Font::canRenderCombiningCharacterSequence()
+        https://bugs.webkit.org/show_bug.cgi?id=185933
+
+        Reviewed by Ryosuke Niwa.
+
+        We don't need to create a whole CTLine just to determine whether or not a font supports rendering a grapheme cluster.
+        Instead, the right way to do it is just see if the font's cmap table supports every code point in the cluster.
+
+        This patch reports a 2% progression on the attached PerformanceTest.
+
+        Test: Layout/ComplexLongUnique.html
+
+        * platform/graphics/Font.cpp:
+        (WebCore::Font::canRenderCombiningCharacterSequence const):
+        * platform/graphics/Font.h:
+        * platform/graphics/cocoa/FontCocoa.mm:
+        (WebCore::provideStringAndAttributes): Deleted.
+        (WebCore::Font::canRenderCombiningCharacterSequence const): Deleted.
+        * platform/graphics/freetype/SimpleFontDataFreeType.cpp:
+        (WebCore::Font::canRenderCombiningCharacterSequence const): Deleted.
+
 2018-05-25  Eric Carlson  <eric.carlson@apple.com>
 
         Captions are sized incorrectly in PiP mode
index 2e2ad39..2cdae74 100644 (file)
@@ -513,4 +513,14 @@ bool Font::variantCapsSupportsCharacterForSynthesis(FontVariantCaps fontVariantC
 }
 #endif
 
+bool Font::canRenderCombiningCharacterSequence(const UChar* characters, size_t length) const
+{
+    ASSERT(isMainThread());
+
+    for (UChar32 codePoint : StringView(characters, length).codePoints()) {
+        if (!glyphForCharacter(codePoint))
+            return false;
+    }
+    return true;
+}
 } // namespace WebCore
index ed432ed..73cdf2a 100644 (file)
@@ -199,10 +199,7 @@ public:
     const BitVector& glyphsSupportedByAllPetiteCaps() const;
 #endif
 
-#if PLATFORM(COCOA) || USE(HARFBUZZ)
     bool canRenderCombiningCharacterSequence(const UChar*, size_t) const;
-#endif
-
     bool applyTransforms(GlyphBufferGlyph*, GlyphBufferAdvance*, size_t glyphCount, bool enableKerning, bool requiresShaping) const;
 
 #if PLATFORM(WIN)
@@ -297,10 +294,6 @@ private:
     mutable std::optional<BitVector> m_glyphsSupportedByAllPetiteCaps;
 #endif
 
-#if PLATFORM(COCOA) || USE(HARFBUZZ)
-    mutable std::unique_ptr<HashMap<String, bool>> m_combiningCharacterSequenceSupport;
-#endif
-
 #if PLATFORM(WIN)
     mutable SCRIPT_CACHE m_scriptCache;
     mutable SCRIPT_FONTPROPERTIES* m_scriptFontProperties;
index 13518a0..2239a83 100644 (file)
@@ -597,53 +597,4 @@ float Font::platformWidthForGlyph(Glyph glyph) const
     return advance.width + m_syntheticBoldOffset;
 }
 
-struct ProviderInfo {
-    const UChar* characters;
-    size_t length;
-    CFDictionaryRef attributes;
-};
-
-static const UniChar* provideStringAndAttributes(CFIndex stringIndex, CFIndex* count, CFDictionaryRef* attributes, void* context)
-{
-    ProviderInfo* info = static_cast<struct ProviderInfo*>(context);
-    if (stringIndex < 0 || static_cast<size_t>(stringIndex) >= info->length)
-        return 0;
-
-    *count = info->length - stringIndex;
-    *attributes = info->attributes;
-    return info->characters + stringIndex;
-}
-
-bool Font::canRenderCombiningCharacterSequence(const UChar* characters, size_t length) const
-{
-    ASSERT(isMainThread());
-
-    if (!m_combiningCharacterSequenceSupport)
-        m_combiningCharacterSequenceSupport = std::make_unique<HashMap<String, bool>>();
-
-    WTF::HashMap<String, bool>::AddResult addResult = m_combiningCharacterSequenceSupport->add(String(characters, length), false);
-    if (!addResult.isNewEntry)
-        return addResult.iterator->value;
-
-    RetainPtr<CFTypeRef> fontEqualityObject = platformData().objectForEqualityCheck();
-
-    ProviderInfo info = { characters, length, getCFStringAttributes(false, platformData().orientation()) };
-    RetainPtr<CTLineRef> line = adoptCF(CTLineCreateWithUniCharProvider(&provideStringAndAttributes, 0, &info));
-
-    CFArrayRef runArray = CTLineGetGlyphRuns(line.get());
-    CFIndex runCount = CFArrayGetCount(runArray);
-
-    for (CFIndex r = 0; r < runCount; r++) {
-        CTRunRef ctRun = static_cast<CTRunRef>(CFArrayGetValueAtIndex(runArray, r));
-        ASSERT(CFGetTypeID(ctRun) == CTRunGetTypeID());
-        CFDictionaryRef runAttributes = CTRunGetAttributes(ctRun);
-        CTFontRef runFont = static_cast<CTFontRef>(CFDictionaryGetValue(runAttributes, kCTFontAttributeName));
-        if (!CFEqual(fontEqualityObject.get(), FontPlatformData::objectForEqualityCheck(runFont).get()))
-            return false;
-    }
-
-    addResult.iterator->value = true;
-    return true;
-}
-
 } // namespace WebCore
index e38290b..48be5a8 100644 (file)
@@ -187,45 +187,4 @@ float Font::platformWidthForGlyph(Glyph glyph) const
     return width ? width : m_spaceWidth;
 }
 
-#if USE(HARFBUZZ)
-bool Font::canRenderCombiningCharacterSequence(const UChar* characters, size_t length) const
-{
-    if (!m_combiningCharacterSequenceSupport)
-        m_combiningCharacterSequenceSupport = std::make_unique<HashMap<String, bool>>();
-
-    WTF::HashMap<String, bool>::AddResult addResult = m_combiningCharacterSequenceSupport->add(String(characters, length), false);
-    if (!addResult.isNewEntry)
-        return addResult.iterator->value;
-
-    UErrorCode error = U_ZERO_ERROR;
-    Vector<UChar, 4> normalizedCharacters(length);
-#if COMPILER(GCC_OR_CLANG)
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
-#endif
-    int32_t normalizedLength = unorm_normalize(characters, length, UNORM_NFC, UNORM_UNICODE_3_2, &normalizedCharacters[0], length, &error);
-#if COMPILER(GCC_OR_CLANG)
-#pragma GCC diagnostic pop
-#endif
-    if (U_FAILURE(error))
-        return false;
-
-    CairoFtFaceLocker cairoFtFaceLocker(m_platformData.scaledFont());
-    FT_Face face = cairoFtFaceLocker.ftFace();
-    if (!face)
-        return false;
-
-    UChar32 character;
-    unsigned clusterLength = 0;
-    SurrogatePairAwareTextIterator iterator(normalizedCharacters.data(), 0, normalizedLength, normalizedLength);
-    for (iterator.advance(clusterLength); iterator.consume(character, clusterLength); iterator.advance(clusterLength)) {
-        if (!FcFreeTypeCharIndex(face, character))
-            return false;
-    }
-
-    addResult.iterator->value = true;
-    return true;
-}
-#endif
-
 }