Attachment 341372 Details for Bug 185933 – Patch

[patch] Patch

bug-185933-20180525181343.patch (text/plain), 9.35 KB, created by Myles C. Maxfield on 2018-05-25 18:13:43 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Myles C. Maxfield

Created: 2018-05-25 18:13:43 PDT

Size: 9.35 KB

patch

obsolete

>Subversion Revision: 232134
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 894ae700af9d610da747ec7560936e244b7e4c75..a3c196969c944caf705182fb28c33545e6fb15fc 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,26 @@
>+2018-05-23  Myles C. Maxfield  <mmaxfield@apple.com>
>+
>+        Improve the performance of Font::canRenderCombiningCharacterSequence()
>+        https://bugs.webkit.org/show_bug.cgi?id=185933
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        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-23  Chris Dumez  <cdumez@apple.com>
> 
>         RenderLayer::scrollRectToVisible() should not propagate a subframe's scroll to its cross-origin parent
>diff --git a/Source/WebCore/platform/graphics/Font.cpp b/Source/WebCore/platform/graphics/Font.cpp
>index 2e2ad390dcc9c014b41427fe0561eb2aa1730da4..2cdae74e29fed09e169e31cab2d56db76ea30d80 100644
>--- a/Source/WebCore/platform/graphics/Font.cpp
>+++ b/Source/WebCore/platform/graphics/Font.cpp
>@@ -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
>diff --git a/Source/WebCore/platform/graphics/Font.h b/Source/WebCore/platform/graphics/Font.h
>index ed432ed46d0be75c4627982b77e4539f24bf41d0..73cdf2a852b7ad8382a70d47c6575df1be9994b0 100644
>--- a/Source/WebCore/platform/graphics/Font.h
>+++ b/Source/WebCore/platform/graphics/Font.h
>@@ -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;
>diff --git a/Source/WebCore/platform/graphics/cocoa/FontCocoa.mm b/Source/WebCore/platform/graphics/cocoa/FontCocoa.mm
>index 13518a051075d02ce07af0cfcfdf1cc7efe0bee0..2239a8358c53273d7ffb98f32b1a2c41d89923c0 100644
>--- a/Source/WebCore/platform/graphics/cocoa/FontCocoa.mm
>+++ b/Source/WebCore/platform/graphics/cocoa/FontCocoa.mm
>@@ -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
>diff --git a/Source/WebCore/platform/graphics/freetype/SimpleFontDataFreeType.cpp b/Source/WebCore/platform/graphics/freetype/SimpleFontDataFreeType.cpp
>index e38290b3acb95521daa99089d6f2f91ed631a55c..48be5a84082a4c1a7d76590a5028f0a0630416e2 100644
>--- a/Source/WebCore/platform/graphics/freetype/SimpleFontDataFreeType.cpp
>+++ b/Source/WebCore/platform/graphics/freetype/SimpleFontDataFreeType.cpp
>@@ -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
>-
> }
>diff --git a/PerformanceTests/ChangeLog b/PerformanceTests/ChangeLog
>index 2a85872d879d87c4a1b0727d9e1f29aad13ca200..db2c7244fa80f59316c06f0ac1c01dc259d1faa0 100644
>--- a/PerformanceTests/ChangeLog
>+++ b/PerformanceTests/ChangeLog
>@@ -1,3 +1,12 @@
>+2018-05-23  Myles C. Maxfield  <mmaxfield@apple.com>
>+
>+        Improve the performance of Font::canRenderCombiningCharacterSequence()
>+        https://bugs.webkit.org/show_bug.cgi?id=185933
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * Layout/ComplexLongUnique.html: Added.
>+
> 2018-05-22  Ryan Haddad  <ryanhaddad@apple.com>
> 
>         Unreviewed, rolling out r232052.
>diff --git a/PerformanceTests/Layout/ComplexLongUnique.html b/PerformanceTests/Layout/ComplexLongUnique.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..31d4592bce8bfff5ed7994950565ad4eeb78bd95
>--- /dev/null
>+++ b/PerformanceTests/Layout/ComplexLongUnique.html
>@@ -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>

Actions: View | Formatted Diff | Diff

Attachments on bug 185933: 341162 | 341167 | 341177 | 341323 | 341335 | 341337 | 341353 | 341358 | 341360 | 341364 | 341372