[iOS] CTFontDescriptorCreateWithTextStyleAndAttributes() is faster than CTFontDescriptorCreateWithTextStyle()/CTFontDescriptorCreateCopyWithAttributes()
Created attachment 406949 [details] Patch
<rdar://problem/67887880>
Created attachment 408079 [details] Patch
Comment on attachment 408079 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=408079&action=review > Source/WebCore/ChangeLog:4 > + [iOS] CTFontDescriptorCreateWithTextStyleAndAttributes() is faster than CTFontDescriptorCreateWithTextStyle()/CTFontDescriptorCreateCopyWithAttributes() > + https://bugs.webkit.org/show_bug.cgi?id=215707 Do you have any perf measurements for this claim? > Source/WTF/wtf/PlatformHave.h:740 > +#define HAVE_CTFONTDESCRIPTORCREATEWITHTEXTSTYLEANDATTRIBUTES 1 Please throw some underscores in there. This is not really readable as is.
Comment on attachment 408079 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=408079&action=review > Source/WebCore/rendering/RenderThemeIOS.mm:1403 > + RetainPtr<CTFontDescriptorRef> emphasizedFontDescriptor = adoptCF(CTFontDescriptorCreateWithTextStyleAndAttributes(kCTUIFontTextStyleFootnote, RenderThemeIOS::contentSizeCategory(), (CFDictionaryRef)@{ (id)kCTFontTraitsAttribute: @{ (id)kCTFontSymbolicTrait: @(kCTFontTraitTightLeading | kCTFontTraitEmphasized) } })); auto
<rdar://problem/63930892>
Oh, I see. Even if CTFontDescriptorCreateWithTextStyleAndAttributes() doesn’t exist, we should still modify the existing code to use (id)kCTFontSymbolicTrait: @(kCTFontTraitTightLeading | kCTFontTraitEmphasized). This is about correctness, not performance.
Created attachment 408147 [details] Patch
Comment on attachment 408147 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=408147&action=review > Source/WebCore/rendering/RenderThemeIOS.mm:1408 > +#if HAVE(CTFONTDESCRIPTOR_CREATE_WITH_TEXT_STYLE_AND_ATTRIBUTES) > + auto emphasizedFontDescriptor = adoptCF(CTFontDescriptorCreateWithTextStyleAndAttributes(kCTUIFontTextStyleFootnote, RenderThemeIOS::contentSizeCategory(), (CFDictionaryRef)@{ (id)kCTFontTraitsAttribute: @{ (id)kCTFontSymbolicTrait: @(kCTFontTraitTightLeading | kCTFontTraitEmphasized) } })); > +#else > + auto fontDescriptor = adoptCF(CTFontDescriptorCreateWithTextStyle(kCTUIFontTextStyleShortFootnote, RenderThemeIOS::singleton().contentSizeCategory(), 0)); > + auto emphasizedFontDescriptor = adoptCF(CTFontDescriptorCreateCopyWithAttributes(fontDescriptor.get(), > + (CFDictionaryRef)@{ (id)kCTFontTraitsAttribute: @{ (id)kCTFontSymbolicTrait: @(kCTFontTraitTightLeading | kCTFontTraitEmphasized) } })); > +#endif Maybe we could put the attributes dictionary outside the #if? I wouldn’t mind having one more local variable for it.
Created attachment 408151 [details] Patch
Comment on attachment 408151 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=408151&action=review > Source/WebCore/rendering/RenderThemeIOS.mm:1403 > + auto emphasizedFontDescriptor = adoptCF(CTFontDescriptorCreateWithTextStyleAndAttributes(kCTUIFontTextStyleFootnote, RenderThemeIOS::contentSizeCategory(), (CFDictionaryRef)@{ (id)kCTFontTraitsAttribute: @{ (id)kCTFontSymbolicTrait: @(kCTFontTraitTightLeading | kCTFontTraitEmphasized) } })); This calls RenderThemeIOS::contentSizeCategory(). > Source/WebCore/rendering/RenderThemeIOS.mm:1405 > + auto fontDescriptor = adoptCF(CTFontDescriptorCreateWithTextStyle(kCTUIFontTextStyleShortFootnote, RenderThemeIOS::singleton().contentSizeCategory(), 0)); This calls RenderThemeIOS::singleton().contentSizeCategory(). Why the difference? I suggest refactoring so only the CTFontDescriptorCreateWithTextStyleAndAttributes call has an #if in it, and all the arguments are shared.
Created attachment 408152 [details] Patch
Created attachment 408155 [details] Patch
Comment on attachment 408155 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=408155&action=review > Source/WebCore/rendering/RenderThemeIOS.mm:1404 > + CFStringRef style = kCTUIFontTextStyleFootnote; > + CFStringRef size = RenderThemeIOS::singleton().contentSizeCategory(); > + CFDictionaryRef attributes = static_cast<CFDictionaryRef>(@{ (id)kCTFontTraitsAttribute: @{ (id)kCTFontSymbolicTrait: @(kCTFontTraitTightLeading | kCTFontTraitEmphasized) } }); I would have used auto for all three of these.
Created attachment 408157 [details] Patch for committing
Committed r266693: <https://trac.webkit.org/changeset/266693> All reviewed patches have been landed. Closing bug and clearing flags on attachment 408157 [details].