1427641 - Support variation fonts in the Linux/fontconfig/freetype font rendering backend

Status: RESOLVED FIXED

(Core :: Graphics: Text, defect)

Description

[Jonathan Kew [:jfkthame]]

This involves hooking up the values from the CSS font-variation-settings property to the FreeType variation font APIs, ensuring they're passed through the various bits of plumbing involved (fontconfig, cairo, freetype, ...) so that the correct variations are applied when measuring and rendering.

Comment 1

[Jonathan Kew [:jfkthame]]

Attachment: patch 1 - Get glyph widths directly from the FreeType face, instead of via a cairo scaled_font wrapper

Comment 2

[Jonathan Kew [:jfkthame]]

Attachment: patch 2 - Convert variation values into FreeType's data type, and apply them to the FT_Face

Comment 3

[Jonathan Kew [:jfkthame]]

Attachment: patch 3 - Add variation data to the cairo_ft_*_font objects, so they can properly track instances of the same font resource used with different variation parameters

Comment 4

[Jonathan Kew [:jfkthame]]

Attachment: patch 4 - For downloadable fonts that have variations, create a separate FT_Face for each instance used

Comment 5

[Jonathan Kew [:jfkthame]]

Attachment: patch 5 - Also for system-installed fonts, create a separate FcPattern and face for each instance when variations are present

Comment 6

[Jonathan Kew [:jfkthame]]

Attachment: patch 6 - Tidy up and refactor code for creating FcPattern for an FT_Face and vice versa

Comment 7

[Jonathan Kew [:jfkthame]]

Attachment: patch 7 - Work around buggy FreeType metrics APIs when using variation fonts with FT versions prior to 2.8.2

Comment 8

[Jonathan Kew [:jfkthame]]

Attachment: patch 8 - Refactor FTFontBase glyph-width code so that we properly respect variations when getting character widths during InitMetrics

Comment 9

[Jonathan Kew [:jfkthame]]

The patches above are sufficient to make the Axis-Praxis demo page (see https://www.axis-praxis.org/) work pretty nicely, AFAICT. I'm sure there will be more to do before we're ready to turn this on, but I think it'd be good to start getting this landed so people can begin to test/experiment with it.

(I've left the patches in bite-size chunks for easier review, I hope, although in some cases -- e.g. 4, 5 and 6 -- subsequent patches may revise the code from earlier ones as they build on it. Folding it all together seemed like it would be harder to deal with, though.)

Pushed a try build at https://treeherder.mozilla.org/#/jobs?repo=try&revision=452a620645c8edcd4c4b9140136d09565173a5b4. To experiment with the Axis-Praxis demo in this (or a local build), set three prefs:

    gfx.downloadable_fonts.keep_variation_tables = true
    gfx.downloadable_fonts.otl_validation = false
    layout.css.font-variations.enabled = true

Then the various demos there should work (except for Skia and SFNSText, which aren't provided as webfonts).

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f9033e71d728
patch 1 - Get glyph widths directly from the FreeType face, instead of via a cairo scaled_font wrapper. r=lsalzman
https://hg.mozilla.org/integration/mozilla-inbound/rev/1070a455ef17
patch 2 - Convert variation values into FreeType's data type, and apply them to the FT_Face. r=lsalzman
https://hg.mozilla.org/integration/mozilla-inbound/rev/54896137200d
patch 3 - Add variation data to the cairo_ft_*_font objects, so they can properly track instances of the same font resource used with different variation parameters. r=lsalzman
https://hg.mozilla.org/integration/mozilla-inbound/rev/d6e329f15f6a
patch 4 - For downloadable fonts that have variations, create a separate FT_Face for each instance used. r=lsalzman
https://hg.mozilla.org/integration/mozilla-inbound/rev/78fe966a5d43
patch 5 - Also for system-installed fonts, create a separate FcPattern and face for each instance when variations are present. r=lsalzman
https://hg.mozilla.org/integration/mozilla-inbound/rev/89faa23196ea
patch 6 - Tidy up and refactor code for creating FcPattern for an FT_Face and vice versa. r=lsalzman
https://hg.mozilla.org/integration/mozilla-inbound/rev/944248bbe7ad
patch 7 - Work around buggy FreeType metrics APIs when using variation fonts with FT versions prior to 2.8.2. r=lsalzman
https://hg.mozilla.org/integration/mozilla-inbound/rev/a85c5795cc6f
patch 8 - Refactor gfxFT2FontBase glyph-width code so that we properly respect variations when getting character widths during InitMetrics. r=lsalzman

Comment 11

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/f9033e71d728
https://hg.mozilla.org/mozilla-central/rev/1070a455ef17
https://hg.mozilla.org/mozilla-central/rev/54896137200d
https://hg.mozilla.org/mozilla-central/rev/d6e329f15f6a
https://hg.mozilla.org/mozilla-central/rev/78fe966a5d43
https://hg.mozilla.org/mozilla-central/rev/89faa23196ea
https://hg.mozilla.org/mozilla-central/rev/944248bbe7ad
https://hg.mozilla.org/mozilla-central/rev/a85c5795cc6f

Comment 12

[Ionuț Goldan [:igoldan]]

This very well improved performance on at least 2 of our metrics!

== Change summary for alert #11104 (as of Sat, 06 Jan 2018 10:30:56 GMT) ==

Improvements:

 31%  tscrollx linux64 opt e10s     6.52 -> 4.47
 30%  tscrollx linux64 pgo e10s     6.30 -> 4.39
  4%  tsvgx linux64 pgo e10s        354.92 -> 339.44
  3%  tsvgx linux64 opt e10s        376.24 -> 363.36

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=11104

Comment 13

[Jonathan Kew [:jfkthame]]

(In reply to Ionuț Goldan [:igoldan], Performance Sheriffing from comment #12)
> This very well improved performance on at least 2 of our metrics!
> 
> == Change summary for alert #11104 (as of Sat, 06 Jan 2018 10:30:56 GMT) ==
> 
> Improvements:
> 
>  31%  tscrollx linux64 opt e10s     6.52 -> 4.47
>  30%  tscrollx linux64 pgo e10s     6.30 -> 4.39
>   4%  tsvgx linux64 pgo e10s        354.92 -> 339.44
>   3%  tsvgx linux64 opt e10s        376.24 -> 363.36
> 
> For up to date results, see:
> https://treeherder.mozilla.org/perf.html#/alerts?id=11104

Interesting -- I'm guessing that's a result of the first patch here, as the others should have little impact on performance for existing tests. However, I'm afraid we may need to revert that patch (at least partly/temporarily) due to bug 1428826, so we may lose these improvements in the process of fixing that. :(

Comment 14

[Ionuț Goldan [:igoldan]]

(In reply to Jonathan Kew (:jfkthame) from comment #13)
> However, I'm afraid we may need to revert that patch (at least partly/temporarily)
> due to bug 1428826, so we may lose these improvements in the process of
> fixing that. :(

Thanks for sharing this! Will be on the lookout.

Comment 15

[Ionuț Goldan [:igoldan]]

:jfkthame I'm following up your notice with the expected regression from bug 1428826. Performance baseline has returned to previous values; nothing strange noticed otherwise.

== Change summary for alert #11149 (as of Thu, 11 Jan 2018 10:20:13 GMT) ==

Regressions:

 45%  tscrollx linux64 opt e10s     4.50 -> 6.52
 43%  tscrollx linux64 pgo e10s     4.40 -> 6.28
  4%  tsvgx linux64 pgo e10s        340.82 -> 355.49
  4%  tsvgx linux64 opt e10s        362.21 -> 377.11

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=11149

Comment 16

[Landry Breuil (:gaston)]

Fwiw, this causes a build breakage with system-cairo, using 1.14.12:



/usr/obj/ports/firefox-59.0beta3/mozilla-beta-c94e1fd9fe7a47c51c421db2a7052292ac411e3a/gfx/2d/ScaledFontFontconfig.cpp:400:29: error:     
      no matching function for call to 'cairo_ft_font_face_create_for_pattern'
  cairo_font_face_t* font = cairo_ft_font_face_create_for_pattern(pattern, nullptr, 0);
                            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/local/include/cairo/cairo-ft.h:104:1: note: candidate function not viable: requires single argument 'pattern', but 3
      arguments were provided
cairo_ft_font_face_create_for_pattern (FcPattern *pattern);

Comment 17

[Landry Breuil (:gaston)]

Afaict cairo_ft_font_face_create_for_ft_face and cairo_ft_font_face_create_for_pattern that you modify in the internal cairo copy are part of the public exposed api of cairo, why not providing alternatives with different names ? Or providing a wrapper/compat with SYSTEM_CAIRO case ?

Comment 18

[Landry Breuil (:gaston)]

It also blows on gfxFcPlatformFontList.cpp, unsurprisingly:

/usr/obj/ports/firefox-59.0beta3/mozilla-beta-c94e1fd9fe7a47c51c421db2a7052292ac411e3a/gfx/thebes/gfxFcPlatformFontList.cpp:731:9: error: 
      no matching function for call to 'cairo_ft_font_face_create_for_pattern'                                                            
        cairo_ft_font_face_create_for_pattern(aRenderPattern,
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/local/include/cairo/cairo-ft.h:104:1: note: candidate function not viable: requires single argument 'pattern', but 3 arguments were  
      provided
cairo_ft_font_face_create_for_pattern (FcPattern *pattern);


The whole https://hg.mozilla.org/integration/mozilla-inbound/rev/54896137200d cset itself is problematic.. should i file a separate bug ?

Comment 19

[Jonathan Kew [:jfkthame]]

Yes, a separate bug would be best, thanks. We don't build with system cairo, so it's not a tested or well-supported option, but let's see if we can fix things up to keep it working.