Closed Bug 1455569 Opened 2 years ago Closed 2 years ago

Rendering of system-installed variation fonts on macOS broken on High Sierra

Categories

(Core :: Graphics: Text, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla61
Tracking Status
firefox-esr52 --- unaffected
firefox59 --- unaffected
firefox60 --- unaffected
firefox61 --- fixed

People

(Reporter: jfkthame, Assigned: jfkthame)

References

Details

(Keywords: regression)

Attachments

(1 file, 1 obsolete file)

The patch in bug 1454094 fixed the crashiness seen on High Sierra with some (not all) downloadable variation fonts. However, it sadly seems to have broken the rendering of system-installed variation fonts (like Skia and the macOS system font) when non-default variation values are applied; they just vanish.

This is most readily seen by loading the "Old Default" page at axis-praxis (https://www.axis-praxis.org/specimens/__DEFAULT__OLD), where most of the Skia text is AWOL.

This is also the underlying cause of bug 1455494 and its dupes, which appeared to be a regression from bug 1454598 but that's just because the patch there hooked up font-weight to the weight variation axis in the macOS system font, exposing this issue for lots of content that didn't explicitly try to use font-variation-settings.
So I guess to fix this, we need to update the checks in the various CGFontCreateCopyWithVariations implementations such that we explicitly copy settings from the CGFont to the new CTFont in either of two cases: (1) we're running on 10.12, or (2) we're on 10.13+ *and* the font is a system-installed font rather than a downloaded resource.

At some of the points where we do this, I don't think we currently have any info as to whether it's an installed system font or a webfont on hand, so we'll need to figure out how to make this available.
This is quite sad, but AFAICT it resolves the brokenness here and doesn't reintroduce the crashes from the previous bug. Note that I didn't tweak the version of CreateCTFontFromCGFontWithVariations in cairo-quartz-font.c for now, as I haven't found a case where this one is a problem; but if we do come up with a scenario where it fails, we may need to make a similar adjustment there (I've left a comment for future reference).
Attachment #8969652 - Flags: review?(lsalzman)
Assignee: nobody → jfkthame
Status: NEW → ASSIGNED
Comment on attachment 8969652 [details] [diff] [review]
Handle variation settings of system-installed variation fonts when creating CTFont from CGFont on High Sierra

There is already a GetUnscaledFont accessor on ScaledFont. Why do you need to add another one to ScaledFontMac?
(In reply to Lee Salzman [:lsalzman] from comment #3)
> Comment on attachment 8969652 [details] [diff] [review]
> Handle variation settings of system-installed variation fonts when creating
> CTFont from CGFont on High Sierra
> 
> There is already a GetUnscaledFont accessor on ScaledFont. Why do you need
> to add another one to ScaledFontMac?

Oops, I must've overlooked that. :\ Will remove it.
Attachment #8969652 - Attachment is obsolete: true
Attachment #8969652 - Flags: review?(lsalzman)
Attachment #8969659 - Flags: review?(lsalzman) → review+
Pushed by jkew@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/32b9dbc41590
Handle variation settings of system-installed variation fonts when creating CTFont from CGFont on High Sierra. r=lsalzman
https://hg.mozilla.org/mozilla-central/rev/32b9dbc41590
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
You need to log in before you can comment on or make changes to this bug.