1360128 - Pass font variation settings through to harfbuzz shaping [was: GSUB rvrn FeatureVariations not working with variable fonts]

Status: RESOLVED FIXED

(Core :: Layout: Text and Fonts, defect, P3)

Description

[Dominik Röttsches (drott)]

Attachment: gsub_rvrn_test_webkit.zip

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_4) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/58.0.3029.81 Safari/537.36

Steps to reproduce:

The variabletest_box.ttf font I am attaching was
extended to have two new glyphs for r and R respectively displaying
"rvrn base" and "rvrn subst" (for "substituted"). When a new axis
'fvtt', ranging from 0 to 10, is set to >= 5, the rvrn_base glyph is
replaced with the rvrn_subst glyph. This enables creating a reftest of
where a lowercase r with font-variation-settings: "fvtt" 10; must match
the uppercase R, which selects the "rvrn_subst" glyph explicitly.

[1] https://www.microsoft.com/typography/OTSpec/features_pt.htm#rvrn

I am planning to upstream the font to web platform tests.

So, for reproducing, open the attached test case gsubtest_webkit/variable-gsub.html.



Actual results:

rvrn_base displayed twice.


Expected results:

rvrn_base & rvrn_subst should be shown.

Comment 1

[Dominik Röttsches (drott)]

This is probably blocking issue 1302685, but I don't have the required bug edit rights to model that. Thanks for taking a look.

Comment 2

[Dominik Röttsches (drott)]

In Chrome, this required a HarfBuzz update past 1.4.2 and sending the variation parameters down to HarfBuzz.

Comment 3

[Jonathan Kew [:jfkthame]]

Yes, this is a part of variation support that we don't have hooked up yet.

Comment 4

[Jonathan Kew [:jfkthame]]

Harfbuzz has API that should support this (hb_font_set_variations), so AFAICS we just need to hook this up to the variation values specified in CSS (when variation support is enabled).

Comment 5

[Jonathan Kew [:jfkthame]]

Attachment: Pass values from font-variation-settings through to the harfbuzz font object, so that shaping can take variations into account

In principle, this seems like it should be all we need to do here. However, it doesn't quite work for me; in my testing, it seems to fix the example from bug 1360481, but does not reliably fix the example here. The result is sometimes correct, and sometimes not, depending on reloading behavior and on the order of the two runs of text in the testcase; I think we must be mishandling some caching or something like that. So some debugging and an additional patch looks needed...

Comment 6

[Jonathan Kew [:jfkthame]]

(In reply to Jonathan Kew (:jfkthame) from comment #5)
> Created attachment 8912203 [details] [diff] [review]
> Pass values from font-variation-settings through to the harfbuzz font
> object, so that shaping can take variations into account
> 
> In principle, this seems like it should be all we need to do here. However,
> it doesn't quite work for me; in my testing, it seems to fix the example
> from bug 1360481, but does not reliably fix the example here. 

This is actually a bug in harfbuzz, just filed as https://github.com/behdad/harfbuzz/issues/549 (with PR pending to fix it). With that fixed, the patch here works as expected.

Comment 8

[Jonathan Kew [:jfkthame]]

Attachment: pt 2 - Cherry-pick commit 19e77e01bc13f44138e1d50533327d314dd0a018 from upstream harfbuzz, to avoid incorrect shape-plan caching in harfbuzz with variation fonts

As noted in comment 6, this feature currently doesn't work in all cases due to a bug in harfbuzz. This has been fixed upstream but not yet pushed out in a release. So until we take a full new HB release, let's just cherry-pick this commit to avoid the flaky behavior.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e09ffeb018f8
Pass values from font-variation-settings through to the harfbuzz font object, so that shaping can take variations into account. r=jrmuizel
https://hg.mozilla.org/integration/mozilla-inbound/rev/44caf00c1796
pt 2 - Cherry-pick commit 19e77e01bc13f44138e1d50533327d314dd0a018 from upstream harfbuzz, to avoid incorrect shape-plan caching in harfbuzz with variation fonts. r=jrmuizel

Comment 10

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/e09ffeb018f8
https://hg.mozilla.org/mozilla-central/rev/44caf00c1796