Bug 232690

Summary: [Cocoa] Migrate from CTFontCopyVariationAxes() to CTFontCopyVariationAxesInternal() if possible
Product: WebKit Reporter: Myles C. Maxfield <mmaxfield>
Component: New BugsAssignee: Myles C. Maxfield <mmaxfield>
Status: RESOLVED FIXED    
Severity: Normal CC: annulen, commit-queue, ews-watchlist, gyuyoung.kim, heycam, hi, joepeck, pangle, ryuan.choi, sergio, simon.fraser, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 232894    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
ews-feeder: commit-queue-
Patch
none
Patch
none
Patch simon.fraser: review+

Description Myles C. Maxfield 2021-11-03 16:59:10 PDT 
[Cocoa] Migrate from CTFontCopyVariationAxes() to CTFontCopyVariationAxesInternal() if possible
Comment 1 Myles C. Maxfield 2021-11-03 17:01:12 PDT 
Created attachment 443255 [details]
Patch
Comment 2 Myles C. Maxfield 2021-11-03 21:23:34 PDT 
Created attachment 443271 [details]
Patch
Comment 3 Myles C. Maxfield 2021-11-03 21:25:24 PDT 
Created attachment 443272 [details]
Patch
Comment 4 Myles C. Maxfield 2021-11-03 21:31:15 PDT 
Created attachment 443273 [details]
Patch
Comment 5 Myles C. Maxfield 2021-11-03 21:33:56 PDT 
Created attachment 443275 [details]
Patch
Comment 6 Myles C. Maxfield 2021-11-04 00:08:43 PDT 
Created attachment 443279 [details]
Patch
Comment 7 Simon Fraser (smfr) 2021-11-04 14:06:44 PDT 
Comment on attachment 443279 [details]
Patch

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

> Source/WebCore/platform/graphics/ShouldLocalizeAxisNames.h:35
> +enum class ShouldLocalizeAxisNames : uint8_t {
> +    No,
> +    Yes
> +};

Is there a reason this can't just live in FontPlatformData.h?
Comment 8 Cameron McCormack (:heycam) 2021-11-04 14:08:05 PDT 
Comment on attachment 443279 [details]
Patch

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

> Source/WebCore/platform/graphics/ShouldLocalizeAxisNames.h:2
> + * Copyright (C) 2010 Apple Inc. All rights reserved.

2021 (if you don't end up moving it)

> Source/WebCore/platform/graphics/cocoa/FontCacheCoreText.cpp:305
> +#if defined(HAVE_CTFontCopyVariationAxesInternal) // This macro is defined inside CoreText, not WebKit.

Thanks for the comment otherwise I would've complained about the capitalization here. :-)
Comment 9 Myles C. Maxfield 2021-11-04 16:18:32 PDT 
Committed r285318 (243890@main): <https://commits.webkit.org/243890@main>
Comment 10 Myles C. Maxfield 2021-11-04 16:19:05 PDT 
Comment on attachment 443279 [details]
Patch

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

>> Source/WebCore/platform/graphics/ShouldLocalizeAxisNames.h:35
>> +};
> 
> Is there a reason this can't just live in FontPlatformData.h?

I would have preferred to put this in a common place, but this has to be visible from both FontPlatformData.h and FontCacheCoreText.h. There is no header (that isn't single-purpose like FontTaggedSettings.h or FontSelectionAlgorithm.h) which is included from both of those places.
Comment 11 Radar WebKit Bug Importer 2021-11-04 16:19:29 PDT 
<rdar://problem/85038305>
Comment 12 WebKit Commit Bot 2021-11-09 11:02:34 PST 
Re-opened since this is blocked by bug 232894
Comment 13 Myles C. Maxfield 2021-11-09 17:58:16 PST 
Committed r285552 (244066@main): <https://commits.webkit.org/244066@main>