From 72da3d8099d2e890f3941acfa13001684b9da7a4 Mon Sep 17 00:00:00 2001 From: Joshua Horton Date: Fri, 16 Aug 2024 14:59:28 +0700 Subject: [PATCH 1/4] refactor(ios): optimize font registration --- .../Classes/Resource Data/FontManager.swift | 151 ++++++++++++------ 1 file changed, 102 insertions(+), 49 deletions(-) diff --git a/ios/engine/KMEI/KeymanEngine/Classes/Resource Data/FontManager.swift b/ios/engine/KMEI/KeymanEngine/Classes/Resource Data/FontManager.swift index d64019cc27c..4a506501000 100644 --- a/ios/engine/KMEI/KeymanEngine/Classes/Resource Data/FontManager.swift +++ b/ios/engine/KMEI/KeymanEngine/Classes/Resource Data/FontManager.swift @@ -26,6 +26,17 @@ public class FontManager { fonts[url] = RegisteredFont(name: name, isRegistered: false) return name } + + private func cachedFont(at url: URL) -> RegisteredFont? { + if let font = fonts[url] { + return font + } + guard let name = readFontName(at: url) else { + return nil + } + fonts[url] = RegisteredFont(name: name, isRegistered: false) + return fonts[url] + } /// Registers all new fonts found in the font path. Call this after you have preloaded all your font files /// with `preloadFontFile(atPath:shouldOverwrite:)` @@ -33,8 +44,45 @@ public class FontManager { guard let keyboardDirs = Storage.active.keyboardDirs else { return } + + var fontSet: Set = [] for dir in keyboardDirs { - registerFonts(in: dir) + fontSet = fontSet.union(listFonts(in: dir)) + } + + registerListedFonts(fontSet) + } + + /** + * Iterates across all listed fonts, registering those not yet registered. + * Checks for, and filters out, any fonts already registered on the system. + */ + private func registerListedFonts(_ initialFontSet: Set) { + // If we are unable to read the font file's properties sufficiently, + // skip it. We also don't need to register anything already registered. + var fontSet = initialFontSet.filter { !(cachedFont(at: $0)?.isRegistered ?? true) } + + // The prior line filters out any entries where cachedFont(at: $0) would be nil. + // Batch-lookups all fonts lacking cache-confirmation of prior registration. + var fontNamesToRegister = missingFonts(from: Set(fontSet.map { cachedFont(at: $0)!.name })) + + for fontUrl in fontSet { + let fontName = cachedFont(at: fontUrl)!.name + + guard fontNamesToRegister.contains(fontName) else { + let message = "Did not register font at \(fontUrl) because font name \(fontName) is already registered" + os_log("%{public}s", log:KeymanEngineLogger.resources, type: .info, message) + continue + } + + let didRegister = _registerFont(at: fontUrl) + fonts[fontUrl] = RegisteredFont(name: fontName, isRegistered: didRegister) + + // We no longer need to register a font with this name, so drop it from + // the set to register. + if didRegister { + fontNamesToRegister.remove(fontName) + } } } @@ -43,6 +91,8 @@ public class FontManager { guard let keyboardDirs = Storage.active.keyboardDirs else { return } + // This doesn't use the expensive looped lookup operation seen in missingFonts, + // so there's no need to batch similar operations here. for dir in keyboardDirs { unregisterFonts(in: dir) } @@ -63,44 +113,29 @@ public class FontManager { } return name as String } + + private func _registerFont(at url: URL) -> Bool { + var errorRef: Unmanaged? + let fontName = fontName(at: url)! + let didRegister = CTFontManagerRegisterFontsForURL(url as CFURL, .none, &errorRef) + let error = errorRef?.takeRetainedValue() // Releases errorRef + if !didRegister { + let message = "Failed to register font \(fontName) at \(url) reason: \(String(describing: error))" + os_log("%{public}s", log:KeymanEngineLogger.resources, type: .error, message) + } else { + let message = "Registered font \(fontName) at \(url)" + os_log("%{public}s", log:KeymanEngineLogger.resources, type: .info, message) + } + + return didRegister + } /// - Parameters: /// - url: URL of the font to register /// - Returns: Font is registered. public func registerFont(at url: URL) -> Bool { - let fontName: String - if let font = fonts[url] { - if font.isRegistered { - return true - } - fontName = font.name - } else { - guard let name = readFontName(at: url) else { - return false - } - fontName = name - } - - let didRegister: Bool - if !fontExists(fontName) { - var errorRef: Unmanaged? - didRegister = CTFontManagerRegisterFontsForURL(url as CFURL, .none, &errorRef) - let error = errorRef?.takeRetainedValue() // Releases errorRef - if !didRegister { - let message = "Failed to register font \(fontName) at \(url) reason: \(String(describing: error))" - os_log("%{public}s", log:KeymanEngineLogger.resources, type: .error, message) - } else { - let message = "Registered font \(fontName) at \(url)" - os_log("%{public}s", log:KeymanEngineLogger.resources, type: .info, message) - } - } else { - didRegister = false - let message = "Did not register font at \(url) because font name \(fontName) is already registered" - os_log("%{public}s", log:KeymanEngineLogger.resources, type: .info, message) - } - let font = RegisteredFont(name: fontName, isRegistered: didRegister) - fonts[url] = font - return didRegister + registerListedFonts([url]) + return fonts[url]?.isRegistered ?? false } /// - Parameters: @@ -133,32 +168,50 @@ public class FontManager { return font.isRegistered } - - public func registerFonts(in directory: URL) { + + private func listFonts(in directory: URL) -> [URL] { guard let urls = try? FileManager.default.contentsOfDirectory(at: directory, includingPropertiesForKeys: nil) else { let message = "Could not list contents of directory \(directory)" os_log("%{public}s", log:KeymanEngineLogger.resources, type: .error, message) - return - } - for url in urls where url.lastPathComponent.hasFontExtension { - _ = registerFont(at: url) + return [] } + return urls.filter { $0.lastPathComponent.hasFontExtension } + } + + public func registerFonts(in directory: URL) { + let fontsToRegister = listFonts(in: directory) + registerListedFonts(Set(fontsToRegister)) } public func unregisterFonts(in directory: URL, fromSystemOnly: Bool = true) { - guard let urls = try? FileManager.default.contentsOfDirectory(at: directory, includingPropertiesForKeys: nil) else { - let message = "Could not list contents of directory \(directory)" - os_log("%{public}s", log:KeymanEngineLogger.resources, type: .error, message) - return - } - for url in urls where url.lastPathComponent.hasFontExtension { + let fontsToUnregister = listFonts(in: directory) + for url in fontsToUnregister { _ = unregisterFont(at: url, fromSystemOnly: fromSystemOnly) } } - private func fontExists(_ fontName: String) -> Bool { - return UIFont.familyNames.contains { familyName in - UIFont.fontNames(forFamilyName: familyName).contains(fontName) + /** + * Queries the system for existing registrations for the specified fonts with a single batch run. + * Only fonts that could not be found will be returned within the result set. + */ + private func missingFonts(from fontNames: Set) -> Set { + // Arrays are treated 'by value'; it's already a shallow copy. + var fontsToFind = fontNames + + UIFont.familyNames.forEach { familyName in + let familyFonts = UIFont.fontNames(forFamilyName: familyName) + + for font in familyFonts { + if let index = fontsToFind.firstIndex(where: { $0 == font }) { + fontsToFind.remove(at: index) + } + + if fontsToFind.count == 0 { + break + } + } } + + return fontsToFind } } From e28d8a341a02477cab1b7e841187679917d5048c Mon Sep 17 00:00:00 2001 From: Joshua Horton Date: Fri, 16 Aug 2024 15:15:32 +0700 Subject: [PATCH 2/4] change(ios): cleaner set-based fonts-to-find maintenance --- .../KMEI/KeymanEngine/Classes/Resource Data/FontManager.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ios/engine/KMEI/KeymanEngine/Classes/Resource Data/FontManager.swift b/ios/engine/KMEI/KeymanEngine/Classes/Resource Data/FontManager.swift index 4a506501000..6dd114d375c 100644 --- a/ios/engine/KMEI/KeymanEngine/Classes/Resource Data/FontManager.swift +++ b/ios/engine/KMEI/KeymanEngine/Classes/Resource Data/FontManager.swift @@ -202,8 +202,8 @@ public class FontManager { let familyFonts = UIFont.fontNames(forFamilyName: familyName) for font in familyFonts { - if let index = fontsToFind.firstIndex(where: { $0 == font }) { - fontsToFind.remove(at: index) + if fontsToFind.contains(font) { + fontsToFind.remove(font) } if fontsToFind.count == 0 { From 289dda0fcab73ae94938e9a69ec599f2c7efb0cf Mon Sep 17 00:00:00 2001 From: Joshua Horton Date: Fri, 16 Aug 2024 15:17:05 +0700 Subject: [PATCH 3/4] change(ios): better early-exit if all font data found --- .../KMEI/KeymanEngine/Classes/Resource Data/FontManager.swift | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ios/engine/KMEI/KeymanEngine/Classes/Resource Data/FontManager.swift b/ios/engine/KMEI/KeymanEngine/Classes/Resource Data/FontManager.swift index 6dd114d375c..9f0047d7f7b 100644 --- a/ios/engine/KMEI/KeymanEngine/Classes/Resource Data/FontManager.swift +++ b/ios/engine/KMEI/KeymanEngine/Classes/Resource Data/FontManager.swift @@ -199,6 +199,10 @@ public class FontManager { var fontsToFind = fontNames UIFont.familyNames.forEach { familyName in + if fontsToFind.count == 0 { + return + } + let familyFonts = UIFont.fontNames(forFamilyName: familyName) for font in familyFonts { From c712d35984d4bad2d515a2852953af17d804ffe5 Mon Sep 17 00:00:00 2001 From: Joshua Horton Date: Mon, 19 Aug 2024 12:16:04 +0700 Subject: [PATCH 4/4] chore(ios): adjustments per review --- .../KeymanEngine/Classes/Resource Data/FontManager.swift | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/ios/engine/KMEI/KeymanEngine/Classes/Resource Data/FontManager.swift b/ios/engine/KMEI/KeymanEngine/Classes/Resource Data/FontManager.swift index 9f0047d7f7b..6070de2c96f 100644 --- a/ios/engine/KMEI/KeymanEngine/Classes/Resource Data/FontManager.swift +++ b/ios/engine/KMEI/KeymanEngine/Classes/Resource Data/FontManager.swift @@ -56,10 +56,15 @@ public class FontManager { /** * Iterates across all listed fonts, registering those not yet registered. * Checks for, and filters out, any fonts already registered on the system. + * Also initializes the registration cache per URL if needed. */ private func registerListedFonts(_ initialFontSet: Set) { // If we are unable to read the font file's properties sufficiently, - // skip it. We also don't need to register anything already registered. + // skip it. We also don't need to register anything already registered or + // that cannot be registered due to loading/parsing errors. + // + // Calls to `cachedFont` after the `.filter` below may be assumed to have + // non-nil return values. var fontSet = initialFontSet.filter { !(cachedFont(at: $0)?.isRegistered ?? true) } // The prior line filters out any entries where cachedFont(at: $0) would be nil. @@ -195,7 +200,6 @@ public class FontManager { * Only fonts that could not be found will be returned within the result set. */ private func missingFonts(from fontNames: Set) -> Set { - // Arrays are treated 'by value'; it's already a shallow copy. var fontsToFind = fontNames UIFont.familyNames.forEach { familyName in