1459158 - macOS variable font Skia renders as black-extended by default

Status: VERIFIED FIXED

(Core :: Layout: Text and Fonts, enhancement)

Description

[Jonathan Kew [:jfkthame]]

Testcase (for macOS only):

  data:text/html,<div style="font:40px Skia">Hello <b>Skia <b>World

Expected result:

  First word should be in the Regular face of Skia, second Bold, and third Black.

Actual result:

  All the text renders as Black Extended.

This happens because Skia is an old GX-style variation font where the 'wght' and 'wdth' axis use a "normalized" scale around a default value of 1.0. So then when we try to apply the default CSS properties of weight:400 and stretch:100%, we immediately get the maximum available for both axes.

To fix, we should recognize such fonts (which we already try to do in gfxFontEntry::SetupVariationRanges) and avoid mapping the CSS properties to the variation axes in gfxFontEntry::GetVariationsForStyle (which is what results in the poor behavior).

Patch coming.

Comment 1

[Jonathan Kew [:jfkthame]]

Attachment: Don't apply variation values from CSS font-weight/font-stretch properties if the font's variation axes appear to use a non-CSS-like scale

Fortunately, we have some spare flag bits we can use to mark the face as being inappropriate for this mapping. With this patch, Skia again behaves nicely. Authors can still use font-variation-settings directly to control the variation axes, using the font's scale for the values.

Comment 2

[Jonathan Watt [:jwatt]]

Comment on attachment 8973152 [details] [diff] [review]
Don't apply variation values from CSS font-weight/font-stretch properties if the font's variation axes appear to use a non-CSS-like scale

Review of attachment 8973152 [details] [diff] [review]:
-----------------------------------------------------------------

I guess this is okay to r+ for now, but I'll have questions (here and on IRC) and maybe ask for some follow-up changes/commenting.

::: gfx/thebes/gfxFontEntry.cpp
@@ +1025,5 @@
>          case HB_TAG('w','g','h','t'):
>              // If the axis range looks like it doesn't fit the CSS font-weight
> +            // scale, we don't hook up the high-level property, and we mark
> +            // the face (in mRangeFlags) as having non-standard weight. This
> +            // means we won't map CSS font-weight to the axis. Setting 'wght'

It's unclear to me why we couldn't normalize the range to the CSS range.

@@ +1042,5 @@
>                  mWeightRange =
>                      WeightRange(FontWeight(std::max(1.0f, axis.mMinValue)),
>                                  FontWeight(axis.mMaxValue));
> +            } else {
> +                mRangeFlags |= RangeFlags::eNonCSSWeight;

So we're relying on the minimum bound being less than 0? Is that always the case for these fonts, or could we end up not catching some of them?

Comment 3

[Jonathan Kew [:jfkthame]]

(In reply to Jonathan Watt [:jwatt] from comment #2)
> Comment on attachment 8973152 [details] [diff] [review]
> Don't apply variation values from CSS font-weight/font-stretch properties if
> the font's variation axes appear to use a non-CSS-like scale
> 
> Review of attachment 8973152 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I guess this is okay to r+ for now, but I'll have questions (here and on
> IRC) and maybe ask for some follow-up changes/commenting.
> 
> ::: gfx/thebes/gfxFontEntry.cpp
> @@ +1025,5 @@
> >          case HB_TAG('w','g','h','t'):
> >              // If the axis range looks like it doesn't fit the CSS font-weight
> > +            // scale, we don't hook up the high-level property, and we mark
> > +            // the face (in mRangeFlags) as having non-standard weight. This
> > +            // means we won't map CSS font-weight to the axis. Setting 'wght'
> 
> It's unclear to me why we couldn't normalize the range to the CSS range.

I think it's unclear what the mapping should be; if a font has a weight range such as 0.62 - 3.2, it's fine to say that 1.0 maps to CSS weight 400, but what would the extremes map to? CSS weights 0 and 1000? But the design might not be intended to represent "ultra-light" to "ultra-black", but some more moderate subrange. If the font has a range 1.0 - 2.0, does that map to 400-700 (normal to bold) or 400-1000 (normal to maximum weight)?

IIRC, Webkit does actually have some code that attempts to provide a mapping, but it's basically just a couple of scale factors based on one particular font, and there's no reason to expect it to give reasonable results for an arbitrary font that's using a non-OpenType/CSS scale.

> 
> @@ +1042,5 @@
> >                  mWeightRange =
> >                      WeightRange(FontWeight(std::max(1.0f, axis.mMinValue)),
> >                                  FontWeight(axis.mMaxValue));
> > +            } else {
> > +                mRangeFlags |= RangeFlags::eNonCSSWeight;
> 
> So we're relying on the minimum bound being less than 0? Is that always the
> case for these fonts, or could we end up not catching some of them?

No, the condition is more than that: we only treat the axis as matching CSS weights if (a) the minumum is non-negative; (b) the maximum is <= 1000; and (c) the weight as declared by the font's OpenType properties (which will be the initial value of Weight() before we've attempted to set up a variation range) falls within the min/max of the axis.

So Skia, for example, whose Regular face (OpenType weight 400) has a "wght" variation with a range of 0.62 to 3.4 or something like that, will fail on that last condition, and I'd expect other legacy GX fonts to be similar.

If the CSS WG figures out and standardizes an algorithm or heuristic for mapping GX-style variation values to CSS property values, we can set a "use GX mapping" flag on such faces instead and apply that mapping, but at this point I don't think there is any agreed solution.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f9c96567050b
Don't apply variation values from CSS font-weight/font-stretch properties if the font's variation axes appear to use a non-CSS-like scale. r=jwatt

Comment 5

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/f9c96567050b

Comment 6

[roxleitan]

Reproduced this issue on Mac OS X 10.13 on Nightly 61.0a1(2018-05-04) with the STR from description.

Verified fixed on Mac OS X 10.13 using the latest Nightly 61.0a1(2018-05-07).