Closed Bug 1360128 Opened 3 years ago Closed 2 years ago

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

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla58
Tracking Status
firefox58 --- fixed

People

(Reporter: drott, Assigned: jfkthame)

References

(Blocks 1 open bug)

Details

Attachments

(3 files)

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.
This is probably blocking issue 1302685, but I don't have the required bug edit rights to model that. Thanks for taking a look.
In Chrome, this required a HarfBuzz update past 1.4.2 and sending the variation parameters down to HarfBuzz.
Yes, this is a part of variation support that we don't have hooked up yet.
Blocks: 1302685
Component: Untriaged → Layout: Text
Product: Firefox → Core
See Also: → 1360481
Priority: -- → P3
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).
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: GSUB rvrn FeatureVariations not working with variable fonts → Pass font variation settings through to harfbuzz shaping [was: GSUB rvrn FeatureVariations not working with variable fonts]
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...
(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.
Attachment #8912203 - Flags: review?(jmuizelaar)
Duplicate of this bug: 1360481
Attachment #8912203 - Flags: review?(jmuizelaar) → review+
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.
Attachment #8917736 - Flags: review?(jmuizelaar)
Assignee: nobody → jfkthame
Status: NEW → ASSIGNED
Attachment #8917736 - Flags: review?(jmuizelaar) → review+
Pushed by jkew@mozilla.com:
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
https://hg.mozilla.org/mozilla-central/rev/e09ffeb018f8
https://hg.mozilla.org/mozilla-central/rev/44caf00c1796
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
You need to log in before you can comment on or make changes to this bug.