Bug 167068 – [Cocoa] Eliminate one of the two CTFonts inside FontPlatformData

• Status: RESOLVED
• Resolution: FIXED
• Component: Text

[Myles C. Maxfield]

[Cocoa] Eliminate one of the two CTFonts inside FontPlatformData

[Myles C. Maxfield]

Created attachment 298907 [details]
WIP

[Myles C. Maxfield]

Created attachment 298908 [details]
WIP

[Build Bot]

Comment on attachment 298908 [details]
WIP

Attachment 298908 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/2895319

Number of test failures exceeded the failure limit.

[Build Bot]

Created attachment 298916 [details]
Archive of layout-test-results from ews103 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 298908 [details]
WIP

Attachment 298908 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/2895309

Number of test failures exceeded the failure limit.

[Build Bot]

Created attachment 298924 [details]
Archive of layout-test-results from ews122 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews122  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 298908 [details]
WIP

Attachment 298908 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/2895384

Number of test failures exceeded the failure limit.

[Build Bot]

Created attachment 298925 [details]
Archive of layout-test-results from ews117 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews117  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Myles C. Maxfield]

CTFontCreateForCharactersWithLanguage() always returns LastResort D:

[Myles C. Maxfield]

Created attachment 299175 [details]
WIP

[Myles C. Maxfield]

Comment on attachment 299175 [details]
WIP

View in context: https://bugs.webkit.org/attachment.cgi?id=299175&action=review

> Source/WebCore/platform/graphics/cocoa/FontCacheCoreText.cpp:893
> +    auto emptyCascadeList = adoptCF(CFArrayCreate(kCFAllocatorDefault, nullptr, 0, &kCFTypeArrayCallBacks));
> +    CFTypeRef keys[] = { kCTFontCascadeListAttribute };
> +    CFTypeRef values[] = { emptyCascadeList.get() };
> +    auto attributes = adoptCF(CFDictionaryCreate(kCFAllocatorDefault, keys, values, 1, &kCFTypeDictionaryKeyCallBacks, &kCFTypeDictionaryValueCallBacks));
> +    auto modification = adoptCF(CTFontDescriptorCreateWithAttributes(attributes.get()));
> +    auto modifiedFont = adoptCF(CTFontCreateCopyWithAttributes(font, CTFontGetSize(font), nullptr, modification.get()));

This is probably a performance hit.

[Build Bot]

Comment on attachment 299175 [details]
WIP

Attachment 299175 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/2911401

New failing tests:
imported/w3c/web-platform-tests/html/browsers/history/the-location-interface/location-protocol-setter-non-broken.html

[Build Bot]

Created attachment 299188 [details]
Archive of layout-test-results from ews101 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 299175 [details]
WIP

Attachment 299175 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/2911411

New failing tests:
fast/events/force-click-on-link-navigation.html
fast/events/cancelled-force-click-link-navigation.html
fast/events/force-click-link-selection-behavior.html

[Build Bot]

Created attachment 299190 [details]
Archive of layout-test-results from ews106 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 299175 [details]
WIP

Attachment 299175 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/2911415

New failing tests:
fast/inline/padding-ellipsis-right.html
fast/text/fallback-language-han-2.html

[Build Bot]

Created attachment 299192 [details]
Archive of layout-test-results from ews124 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews124  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.6

[Myles C. Maxfield]

Assuming https://bugs.webkit.org/show_bug.cgi?id=167300 is completed, there are two different approaches for how to do this:

Approach 1: Use m_font everywhere and eliminate m_ctFont
Almost all the places which use ctFont() are using it to access some font-specific information, which means the cascade list won't be accessed, which means that these uses can be converted directly. However, ComplexTextController::collectComplexTextRuns() and ComplexTextController::collectComplexTextRunsForCharacters() actually use the new LastResort cascade list. If we can educate those places to treat a fallback font the same as LastResort, then we can eliminate m_ctFont().

Approach 2: Use m_ctFont everywhere and eliminate m_font
We can immediately add the cascade list to the font upon ingestion into the system. The one trick with this is that CTFontCreateForCharactersWithLanguage() (inside FontCache::systemFallbackForCharacters()) actually uses the cascade list to select a fallback font. We don't want to create a transient font every time this function is run. Instead, this could possibly be worked around by identifying the key pieces of information within a font which CTFontCreateForCharactersWithLanguage() is sensitive to, and just cache results using that information as a key. This information will probably have to contain weight, italics, and the information inside the font's class contained within its symbolic traits (https://developer.apple.com/reference/coretext/ctfontsymbolictraits/1509032-classmasktrait). Or we could work with the CoreText team to come up with a different solution.

[Myles C. Maxfield]

*** Bug 144141 has been marked as a duplicate of this bug. ***

[Myles C. Maxfield]

*** Bug 186002 has been marked as a duplicate of this bug. ***

[Myles C. Maxfield]

See duplicates

[fantasai]

<rdar://problem/129891005>

[fantasai]

Pull request: https://github.com/WebKit/WebKit/pull/30775

[EWS]

Committed 281022@main (d621347fbb52): <https://commits.webkit.org/281022@main>

Reviewed commits have been landed. Closing PR #30775 and removing active labels.

[EWS]

Committed 280938.41@integration/ci/129891005_d621347fbb_safari-7619-branch (2e5ecf75c7a0): <https://commits.webkit.org/280938.41@integration/ci/129891005_d621347fbb_safari-7619-branch>

Reviewed commits have been landed. Closing PR #1398 and removing active labels.