1558375 - Some fonts render incorrectly on Android

Status: RESOLVED FIXED

(Core :: Graphics: Text, defect, P3)

Description

[Glenn Linderman]

Attachment: test-font-lines.html

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:67.0) Gecko/20100101 Firefox/67.0

Steps to reproduce:

Referenced a font in my .html file. (Dump the two .ttf and the .html attachments in a test directory, open the .html file)

Actual results:

Rendered incorrectly. Both lines should look similar; and do on Windows (but not identical, they are different fonts). But on Android Firefox, the DFKai-SB font is goofy looking.

Expected results:

Should have rendered correctly. Works on Android Chrome, Windows Firefox.

Two different font files are supplied, with a similar style of Chinese fonts (Both are "Kai Ti" fonts: sort of like Roman and Times-Roman, versus Helvetica being quite different). The one font renders fine everywhere I've tried it, and the DFKai-SB font renders on Windows in all browsers, and on Android Chrome, but not on Android Firefox. A year ago, it didn't render properly on Android Chrome, either, but it does now, so maybe there was a platform enhancement that Chrome jumped on and Firefox hasn't yet? Or maybe Chrome got enhanced, meanwhile? I can't tell.

I''m not sure how to tell what version of FF is on Android, but it is ever-fresh, released version.

Comment 1

[Glenn Linderman]

Attachment: kaiu.ttf

Comment 2

[Glenn Linderman]

Attachment: KaiTi.zip

Have to decompress this one before testing

Comment 3

[Jonathan Kew [:jfkthame]]

Could you attach a screenshot showing the broken rendering you see? Thanks.

Comment 4

[Glenn Linderman]

Attachment: Screenshot_20190613-103133-from-Firefox-Android.png

Comment 5

[Glenn Linderman]

Attachment: Screenshot_20190613-103341-from-Chrome-Android.png

Comment 6

[Jonathan Kew [:jfkthame]]

That looks like a font that's doing weird things with TrueType instructions. I believe FreeType has some kind of special handling for this, but it may depend on build settings of some kind.... I don't recall details exactly.

Ah, yes... see tt_check_trickyness_family() in freetype2/src/truetype/ttobjs.c. It may be that we're not using the right FreeType options to allow its "trickiness" handling to work; there's some interaction with hinting etc, iirc.

Comment 7

[Glenn Linderman]

Not familiar with FreeType, but I agree that the font is somewhat different than most fonts. Most of the font subset tools online produce garbage with this font, so I had to use Python fonttools module to make subset fonts from it. I was glad to learn about fonttools, and that it did the job, looks like a comprehensive package, but that probably won't help Firefox if it uses FreeType.

On the other hand, you make it sound like FreeType can handle the font, if the settings are checked/used properly, which might be a relatively easy fix? Mostly in testing, to be sure existing cases continue to work?

Comment 8

[Jonathan Kew [:jfkthame]]

Attachment: testcase for the dfkai font only

Comment 9

[Jonathan Kew [:jfkthame]]

Attachment: testcase for the dfkai font only

Comment 10

[Jonathan Kew [:jfkthame]]

One point to note for testing this is that the kaiu.ttf font will not load via @font-face in Nightly builds unless the gfx.downloadable_fonts.otl_validation pref is toggled to false, because of issues with its OpenType layout tables.

Comment 11

[Jonathan Kew [:jfkthame]]

Looking at the FT2 font backend, I think the problem occurs because we disable hinting on Android (for smoother pinch-zoom etc), but using the FT_LOAD_NO_AUTOHINT flag in particular breaks the rendering of "tricky" faces like this where the "hinting" instructions are required not just for better grid alignment but for basic rendering of the glyph components.

We probably need to ensure we avoid setting that load flag for fonts with the TRICKY face flag. There are a bunch of places where we set up loadFlags for FreeType; I'm not sure how many of these are strictly necessary to patch, but it's probably best to keep them all consistent.

Comment 12

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1558375 - Don't set the FT_LOAD_NO_AUTOHINT flag for fonts that have FT_FACE_FLAG_TRICKY, to avoid broken rendering. r=lsalzman

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6153e6bb58e1
Don't set the FT_LOAD_NO_AUTOHINT flag for fonts that have FT_FACE_FLAG_TRICKY, to avoid broken rendering. r=lsalzman

Comment 14

[Andrei Ciure[:aciure]]

Backed out changeset 6153e6bb58e1 (bug 1558375) for causing 1229972.html to perma crash

push that caused the backout: https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedJob=252022561&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&revision=6153e6bb58e1fb7015c89722eaa1830850e165f5

failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&searchStr=android%2C7.0%2Cx86-64%2Copt%2Creftests%2Ctest-android-em-7.0-x86_64%2Fopt-crashtest-e10s%2Cr%28c%29

backout: https://hg.mozilla.org/integration/autoland/rev/b0b3679a34b86481185c65f4d2b2e00659d9f948

Comment 15

[Jonathan Kew [:jfkthame]]

Looks like CairoLockedFTFace() in SkFontHost_cairo can fail, so we need to null-check the face before using it.

https://treeherder.mozilla.org/#/jobs?repo=try&tier=1&revision=b9355c948961ed38e2a74fb5ccebd50f77bc55ee

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7d79dcab14bd
Don't set the FT_LOAD_NO_AUTOHINT flag for fonts that have FT_FACE_FLAG_TRICKY, to avoid broken rendering. r=lsalzman

Comment 17

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/7d79dcab14bd