Closed Bug 1675185 Opened 2 years ago Closed 2 years ago

Size-sensitive system font rendering broken on Big Sur with WebRender

Categories

(Core :: Graphics: Text, defect)

All
macOS
defect

Tracking

()

VERIFIED FIXED
84 Branch
Tracking Status
firefox-esr78 --- unaffected
firefox82 --- unaffected
firefox83 --- unaffected
firefox84 --- verified

People

(Reporter: mstange, Assigned: jfkthame)

References

(Blocks 1 open bug, Regression)

Details

(Keywords: regression)

Attachments

(3 files)

In the most recent Nightly, when WebRender is enabled, fonts on github look like they have increased letter spacing. It looks the same as it did when bug 1671660 happened.
Non-WebRender looks fine.

Summary: Size-sensitive system font selection broken on Big Sur with WebRender → Size-sensitive system font rendering broken on Big Sur with WebRender

Just the WR patch from bug 1672842 is enough to cause this.

Here's some logging from a build without that patch:


ctfont descriptor: "<CTFontDescriptor: 0x11bf0ed60>{attributes = {
    NSFontNameAttribute = ".SFNS-Regular";
}>}"

after create_copy_with_attributes: "<CTFontDescriptor: 0x11c20f580>{attributes = {
    NSCTFontFileURLAttribute = "file:///System/Library/Fonts/SFNS.ttf";
    NSCTFontVariationAttribute =     {
        2003265652 = 200;
        1869640570 = 17;
    };
}>}"
ctfont descriptor: "<CTFontDescriptor: 0x11bf0e880>{attributes = {
    NSFontNameAttribute = ".SFNS-Regular";
}>}"

after create_copy_with_attributes: "<CTFontDescriptor: 0x11bc6b340>{attributes = {
    NSCTFontFileURLAttribute = "file:///System/Library/Fonts/SFNS.ttf";
    NSCTFontVariationAttribute =     {
        1869640570 = "31.10000038146973";
    };
}>}"
after create_copy_with_attributes: "<CTFontDescriptor: 0x11bc6b2e0>{attributes = {
    NSCTFontFileURLAttribute = "file:///System/Library/Fonts/SFNS.ttf";
    NSCTFontVariationAttribute =     {
        1869640570 = "27.79999923706055";
    };
}>}"

And here's logging from a build with the patch:


ctfont: "<CTFont: 0x132945000>{name = .SFNS-Regular, size = 12.000000, matrix = 0x0, descriptor = <CTFontDescriptor: 0x10b36c280>{attributes = {
    NSFontNameAttribute = ".SFNS-Regular";
}>}}"
ctfont descriptor: "<CTFontDescriptor: 0x10b36c400>{attributes = {
    NSFontNameAttribute = ".SFNS-Regular";
    NSFontSizeAttribute = 12;
}>}"


after create_copy_with_attributes: "<CTFontDescriptor: 0x10b10e220>{attributes = {
    NSCTFontFileURLAttribute = "file:///System/Library/Fonts/SFNS.ttf";
    NSCTFontVariationAttribute =     {
        1869640570 = 20;
    };
    NSFontSizeAttribute = 12;
}>}"
after create_copy_with_attributes: "<CTFontDescriptor: 0x10ac0d3a0>{attributes = {
    NSCTFontFileURLAttribute = "file:///System/Library/Fonts/SFNS.ttf";
    NSCTFontVariationAttribute =     {
        2003265652 = 600;
        1869640570 = 20;
    };
    NSFontSizeAttribute = 12;
}>}"
after create_copy_with_attributes: "<CTFontDescriptor: 0x10b00d880>{attributes = {
    NSCTFontFileURLAttribute = "file:///System/Library/Fonts/SFNS.ttf";
    NSCTFontVariationAttribute =     {
        1869640570 = 32;
    };
    NSFontSizeAttribute = 12;
}>}"
after create_copy_with_attributes: "<CTFontDescriptor: 0x10af0f520>{attributes = {
    NSFontNameAttribute = ".SFNS-Regular";
    NSFontSizeAttribute = 12;
}>}"


after create_copy_with_attributes: "<CTFontDescriptor: 0x133ed1820>{attributes = {
    NSCTFontFileURLAttribute = "file:///System/Library/Fonts/SFNS.ttf";
    NSCTFontVariationAttribute =     {
        1869640570 = "35.59999847412109";
    };
    NSFontSizeAttribute = 12;
}>}"
after create_copy_with_attributes: "<CTFontDescriptor: 0x10ac2ad00>{attributes = {
    NSCTFontFileURLAttribute = "file:///System/Library/Fonts/SFNS.ttf";
    NSCTFontVariationAttribute =     {
        1869640570 = "31.79999923706055";
    };
    NSFontSizeAttribute = 12;
}>}"

The difference that stands out is that the CTFont from core_text::font::new_from_CGFont has an NSFontSizeAttribute entry in the attributes. My guess is that the presence of this attribute causes the font to ignore any variation attributes, but I haven't verified that.

I tried to remove the attribute from the CTFontDescriptor but I couldn't figure out how. There is no API to remove attributes, you can only override attributes. Overriding the font size with zero did not help.

            let attrs_dict = unsafe { CFDictionary::from_CFType_pairs(&[(CFString::wrap_under_get_rule(kCTFontSizeAttribute), CFNumber::from(0.0f32))]) };
            create_copy_with_attributes(&desc, attrs_dict.to_untyped()).unwrap()

I tried overriding with "null" but couldn't find a way to put a null value in the dictionary with our rust dictionary API.

Just the WR patch from bug 1672842 is enough to cause this.

If you revert that patch and try the (abandoned) alternative version from https://phabricator.services.mozilla.com/D95454, does that make any difference?

Flags: needinfo?(mstange.moz)

Set release status flags based on info from the regressing bug 1672842

It looks like we can strip the size using CTFontDescriptorCreateMatchingFontDescriptor()

(In reply to Jonathan Kew (:jfkthame) from comment #4)

Just the WR patch from bug 1672842 is enough to cause this.

If you revert that patch and try the (abandoned) alternative version from https://phabricator.services.mozilla.com/D95454, does that make any difference?

Yep. Seems like that fixes the issue.

All of these have WebRender enabled

Flags: needinfo?(mstange.moz)

Here's a testcase. The percentage glyph should look about the same at 17 and 18, much wider than at 25. But on Big Sur, with this bug, there is a jump between 17 and >17.
17 is narrow, 18 is wide, 25 is narrow., 18-25 transitions smoothly.

Markus, I marked it as S3. Please feel free to change it.

Severity: -- → S3
Assignee: nobody → jfkthame
Status: NEW → ASSIGNED

(This was my original "part 2" patch in bug 1672842, rebased and moved here.)

Pushed by jkew@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c8875713fd6d
For the hidden system font on macOS, instantiate Core Text fonts from a native CGFont rather than via a descriptor. r=jrmuizel
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 84 Branch

(In reply to Jeff Muizelaar [:jrmuizel] from comment #6)

It looks like we can strip the size using CTFontDescriptorCreateMatchingFontDescriptor()

For the record, this idea didn't pan out: It successfully stripped away the NSFontSizeAttribute, but doing so didn't fix the bug. It looked like the bug we were running into was a CoreText bug we previously encountered in bug 1457417 comment 2, where setting the opsz axis to its default value (17 in this case) didn't work but all other values worked.
We didn't fully investigate how the CTFontDescriptor-from-name and the CTFontDescriptor-from-CGFont differed - the former wasn't showing the "default opsz value ignored" problem, so there must be some different state inside it.
Instead, we chose to land the original "part 2" patch from bug 1672842 which was known to work.

Does this need uplifting, or does the same reasoning as https://bugzilla.mozilla.org/show_bug.cgi?id=1672842#c23 apply?

Flags: needinfo?(mstange.moz)

This bug happened regardless of SDK, but:

The patch in this bug fixes for a regression from bug 1672842, so it only needs uplifting to any branches that bug 1672842 was uplifted to. But we're not uplifting that, so this doesn't need to be uplifted either.

Flags: needinfo?(mstange.moz)

Confirmed the issue using the attached test case 84.0a1 (2020-11-03).
Patch verified with 84.0b2 on macOS 11.0.1.

Status: RESOLVED → VERIFIED
Flags: qe-verify+
Has Regression Range: --- → yes
You need to log in before you can comment on or make changes to this bug.