1547063 - Remove Cairo usage from Skia FreeType font host

Status: RESOLVED FIXED

(Core :: Graphics: Text, enhancement, P5)

Description

[Lee Salzman [:lsalzman]]

SkFontHost_cairo depends on the interposition of Cairo for dealing with creating/loading FreeType faces. This imposes annoying limits on our Skia text rasterization such as a lack of subpixel text positioning if desired on Android and Linux. It also forces us to build and maintain FcPattern structures that cause memory bloat and have performance overhead to interpret.

We need to move the creation of FreeType faces into the Moz2D/thebes code. We will probably need to modify our tree Cairo if necessary to accept the raw FreeType faces and any other parameters necessary to interpret them without having to rely on the FcPatterns.

It might then be subsequently possible to remove SkFontHost_cairo entirely and rely on a small set of modifications to the provided SkFontHost_FreeType instead.

Comment 1

[Ivan Molodetskikh]

About 2 months ago subpixel positioning was added to Cairo: https://gitlab.freedesktop.org/cairo/cairo/commits/master

Does that perhaps remove the need to drop Cairo? Or maybe some of the current Linux text rendering issues can be fixed using that until Cairo is removed?

Comment 2

[Lee Salzman [:lsalzman]]

(In reply to Ivan Molodetskikh from comment #1)

About 2 months ago subpixel positioning was added to Cairo: https://gitlab.freedesktop.org/cairo/cairo/commits/master

Does that perhaps remove the need to drop Cairo? Or maybe some of the current Linux text rendering issues can be fixed using that until Cairo is removed?

Getting Cairo out of the way of things it shouldn't be involved in is still the right thing to do. Though, this might help in other cases where we can't once I finish this bug.

Comment 3

[Jonathan Kew [:jfkthame]]

It's also worth mentioning that AFAIK we haven't updated mozilla's copy of cairo in a long time, and have done various local patches -- it's basically a forked version at this point -- so merging significant changes from upstream cairo may be non-trivial...

Comment 4

[Lee Salzman [:lsalzman]]

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

It's also worth mentioning that AFAIK we haven't updated mozilla's copy of cairo in a long time, and have done various local patches -- it's basically a forked version at this point -- so merging significant changes from upstream cairo may be non-trivial...

The patches here seem small enough that a hand-merge/manual graft seem easily feasible, just that there is no real excuse for Cairo to still be interposed here in the first place. So likely both of these are things we want to accomplish.

Comment 5

[Lee Salzman [:lsalzman]]

Attachment: Bug 1547063 - add SharedFTFace abstraction of FT_Face. r?jfkthame

Comment 6

[Lee Salzman [:lsalzman]]

Attachment: Bug 1547063 - Cairo support for SharedFTFace. r?jfkthame

Depends on D44491

Comment 7

[Lee Salzman [:lsalzman]]

Attachment: Bug 1547063 - Remove FcPattern usage from Skia. r?jfkthame

Depends on D44492

Comment 8

[Lee Salzman [:lsalzman]]

Attachment: Bug 1547063 - Use SharedFTFace in thebes. r?jfkthame

Depends on D44493

Comment 9

[Lee Salzman [:lsalzman]]

Attachment: Bug 1547063 - implement recording instance data for ScaledFontMac. r?jfkthame

Depends on D44494

Comment 10

[Lee Salzman [:lsalzman]]

Attachment: Bug 1547063 - Use SharedFTFace locking instead of Cairo locking. r?jfkthame

Depends on D44495

Comment 11

[Lee Salzman [:lsalzman]]

Attachment: Bug 1547063 - Use FreeType metrics directly instead of querying Cairo. r?jfkthame

Depends on D44496

Comment 12

[Lee Salzman [:lsalzman]]

Attachment: Bug 1547063 - fuzz for SharedFTFace. r?jfkthame

Depends on D44497

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e74a67e2afe3
add SharedFTFace abstraction of FT_Face. r=jfkthame
https://hg.mozilla.org/integration/autoland/rev/772c3427c791
Cairo support for SharedFTFace. r=jfkthame
https://hg.mozilla.org/integration/autoland/rev/ebc71e607938
Remove FcPattern usage from Skia. r=jfkthame
https://hg.mozilla.org/integration/autoland/rev/af6e6807ece7
Use SharedFTFace in thebes. r=jfkthame
https://hg.mozilla.org/integration/autoland/rev/23892ecc6ef8
implement recording instance data for ScaledFontMac. r=jfkthame
https://hg.mozilla.org/integration/autoland/rev/2c7032b4d022
Use SharedFTFace locking instead of Cairo locking. r=jfkthame
https://hg.mozilla.org/integration/autoland/rev/003f5a79c6a7
Use FreeType metrics directly instead of querying Cairo. r=jfkthame
https://hg.mozilla.org/integration/autoland/rev/c969a93b0ca7
fuzz for SharedFTFace. r=jfkthame

Comment 14

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/e74a67e2afe3
https://hg.mozilla.org/mozilla-central/rev/772c3427c791
https://hg.mozilla.org/mozilla-central/rev/ebc71e607938
https://hg.mozilla.org/mozilla-central/rev/af6e6807ece7
https://hg.mozilla.org/mozilla-central/rev/23892ecc6ef8
https://hg.mozilla.org/mozilla-central/rev/2c7032b4d022
https://hg.mozilla.org/mozilla-central/rev/003f5a79c6a7
https://hg.mozilla.org/mozilla-central/rev/c969a93b0ca7

Comment 15

[Lee Salzman [:lsalzman]]

Attachment: Bug 1547063 - fix advance scaling for emoji. r?jfkthame

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7b54bb7c31f6
fix advance scaling for emoji. r=karlt

Comment 17

[Cosmin Sabou [:CosminS]]

Backed out changes in comment 14 for causing bug 1581466: https://hg.mozilla.org/mozilla-central/rev/ece24a197d500be1d4f48838c6f4a12a2fb1861c

Comment 18

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/mozilla-central/rev/be65ff9860e7
Backed out changeset 7b54bb7c31f6 for backing out also the other changes in Bug 1547063.

Comment 19

[Lee Salzman [:lsalzman]]

Fixed the printing crash. Should be fixed when I reland. Doing a try push now to verify.

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fe7f211ad6a3
add SharedFTFace abstraction of FT_Face. r=jfkthame
https://hg.mozilla.org/integration/autoland/rev/0b008480f41c
Cairo support for SharedFTFace. r=jfkthame
https://hg.mozilla.org/integration/autoland/rev/e15c477972e6
Remove FcPattern usage from Skia. r=jfkthame
https://hg.mozilla.org/integration/autoland/rev/542af2a68a49
Use SharedFTFace in thebes. r=jfkthame
https://hg.mozilla.org/integration/autoland/rev/880865a950e5
implement recording instance data for ScaledFontMac. r=jfkthame
https://hg.mozilla.org/integration/autoland/rev/99b28f31b550
Use SharedFTFace locking instead of Cairo locking. r=jfkthame
https://hg.mozilla.org/integration/autoland/rev/699f10cc0658
Use FreeType metrics directly instead of querying Cairo. r=jfkthame
https://hg.mozilla.org/integration/autoland/rev/e2324b6a2b33
fuzz for SharedFTFace. r=jfkthame

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c0ff29989db1
fix advance scaling for emoji. r=karlt

Comment 22

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/fe7f211ad6a3
https://hg.mozilla.org/mozilla-central/rev/0b008480f41c
https://hg.mozilla.org/mozilla-central/rev/e15c477972e6
https://hg.mozilla.org/mozilla-central/rev/542af2a68a49
https://hg.mozilla.org/mozilla-central/rev/880865a950e5
https://hg.mozilla.org/mozilla-central/rev/99b28f31b550
https://hg.mozilla.org/mozilla-central/rev/699f10cc0658
https://hg.mozilla.org/mozilla-central/rev/e2324b6a2b33
https://hg.mozilla.org/mozilla-central/rev/c0ff29989db1

Comment 23

[Marian Raiciof [:marauder]]

== Change summary for alert #23211 (as of Tue, 24 Sep 2019 07:40:20 GMT) ==

Improvements:

5% Heap Unclassified linux64 opt 82,886,558.17 -> 79,008,195.00
4% Heap Unclassified linux64 opt 82,543,552.35 -> 79,023,892.13

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