Created attachment 446291 [details] Patch

The Gtk/WPE failures point out that FontCacheFreeType.cpp has its own SystemFallbackCache HashMap, with different key/value types. A bit of abstraction over these will be needed.

Actually they seem to be doing two slightly different things. The SystemFallbackCache that's in Font.cpp and which I'm moving out to a separate file is a per-character map of Font to the fallback Font to use. The one in FontCacheFreeType.cpp is a cache of FcPatterns and FcFontSets that FontCache::systemFallbackForCharacters uses to avoid the overhead of doing the FcConfigSubstitute calls for the same inputs. So I think for now we should just rename the type used in FontCacheFreeType.cpp, but it's another cache that needs to move to live on the FontCache object so that it can be thread-specific. (Or locking added around it.)

Created attachment 446437 [details] Patch

Comment on attachment 446437 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=446437&action=review > Source/WebCore/WebCore.xcodeproj/project.pbxproj:1015 > + 32112E0F274B47CF007047A8 /* SystemFallbackCache.h in Headers */ = {isa = PBXBuildFile; fileRef = 32899FB9274B410C00855A9D /* SystemFallbackCache.h */; settings = {ATTRIBUTES = (Private, ); }; }; Can this file have the word "font" in it somewhere? > Source/WebCore/platform/graphics/SystemFallbackCache.h:81 > + using CharacterFallbackMap = HashMap<CharacterFallbackMapKey, Font*, CharacterFallbackMapKeyHash, CharacterFallbackMapKeyHashTraits>; > + > + HashMap<const Font*, CharacterFallbackMap> m_characterFallbackMaps; Why is this a HashMap of HashMaps? I guess this patch is just moving code around, but we should probably at least have a bug filed about investigating whether we can use less memory and probably have better performance for this.

(In reply to Myles C. Maxfield from comment #5) > > Source/WebCore/platform/graphics/SystemFallbackCache.h:81 > > + using CharacterFallbackMap = HashMap<CharacterFallbackMapKey, Font*, CharacterFallbackMapKeyHash, CharacterFallbackMapKeyHashTraits>; > > + > > + HashMap<const Font*, CharacterFallbackMap> m_characterFallbackMaps; > > Why is this a HashMap of HashMaps? > > I guess this patch is just moving code around, but we should probably at > least have a bug filed about investigating whether we can use less memory > and probably have better performance for this. Filed https://bugs.webkit.org/show_bug.cgi?id=234121.

<rdar://problem/86505855>

Created attachment 459563 [details] Rebased patch

Created attachment 459564 [details] Patch

Created attachment 460997 [details] Patch

Created attachment 460998 [details] Patch

'Myles Maxfield' is not a reviewer, blocking PR #None

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

Committed 252618@main (38edf7c001dd): <https://commits.webkit.org/252618@main> Reviewed commits have been landed. Closing PR #2562 and removing active labels.