Closed
Bug 1455569
Opened 6 years ago
Closed 6 years ago
Rendering of system-installed variation fonts on macOS broken on High Sierra
Categories
(Core :: Graphics: Text, defect)
Core
Graphics: Text
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.
Assignee | ||
Comment 1•6 years ago
|
||
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.
Assignee | ||
Comment 2•6 years ago
|
||
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 | ||
Updated•6 years ago
|
Assignee: nobody → jfkthame
Status: NEW → ASSIGNED
Comment 3•6 years ago
|
||
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?
Assignee | ||
Comment 4•6 years ago
|
||
(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.
Assignee | ||
Comment 5•6 years ago
|
||
Now without the extra accessor.
Attachment #8969659 -
Flags: review?(lsalzman)
Assignee | ||
Updated•6 years ago
|
Attachment #8969652 -
Attachment is obsolete: true
Attachment #8969652 -
Flags: review?(lsalzman)
Updated•6 years ago
|
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
Comment 7•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/32b9dbc41590
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox61:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
Updated•6 years ago
|
Keywords: regression
Updated•6 years ago
|
status-firefox59:
--- → unaffected
status-firefox60:
--- → unaffected
status-firefox-esr52:
--- → unaffected
You need to log in
before you can comment on or make changes to this bug.
Description
•