Closed Bug 1393467 Opened 7 years ago Closed 7 years ago

Imminent breakage of subpixel font rendering (due to skia/FreeType interaction)

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox-esr52 --- fixed
firefox55 --- wontfix
firefox56 --- fixed
firefox57 --- fixed

People

(Reporter: heftig, Assigned: lsalzman)

References

(Blocks 1 open bug)

Details

(Keywords: regression, Whiteboard: [gfx-noted])

Attachments

(2 files)

Firefox is still affected by this Skia bug: https://bugs.chromium.org/p/skia/issues/detail?id=6663

With the next release of FreeType, Firefox's subpixel font rendering will be broken and cut off glyphs due to a violation of FreeType's API.

Please backport Skia commit d1f2d15b: https://skia.googlesource.com/skia/+/d1f2d15b36f6a6a9d199581b998a7ca924a1f1a8%5E%21/
Severity: normal → major
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(lsalzman)
Summary: Imminent brakage of subpixel font rendering → Imminent breakage of subpixel font rendering (due to skia/FreeType interaction)
You've made this block 1364714 (upgrade to FT 2.8), but the problematic change is actually post-2.8.

Shouldn't this affect all versions since Firefox uses the system FreeType on Linux? MOZ_TREE_FREETYPE is only enabled on Android and Windows.
(In reply to Jan Alexander Steffens from comment #1)
> You've made this block 1364714 (upgrade to FT 2.8), but the problematic
> change is actually post-2.8.

Ah, sorry about that. I misunderstood comment 0.
No longer blocks: 1364714
OS: Linux → All
Unfortunately, as mentioned in https://lists.nongnu.org/archive/html/freetype-devel/2017-08/msg00063.html, this patch isn't sufficient. When the new (patent-free, enabled by default) "Harmony" LCD rendering is used, various glyphs are still shifted to the right and cut off. It's not quite as bad as completely unpatched, though.
This is a backport of this upstream Skia fix: https://skia.googlesource.com/skia/+/d1f2d15b36f6a6a9d199581b998a7ca924a1f1a8%5E%21/

Further details in upstream Skia bug: https://bugs.chromium.org/p/skia/issues/detail?id=6663

They have already seemingly deployed this as an emergency fix in Chrome.
Assignee: nobody → lsalzman
Status: NEW → ASSIGNED
Flags: needinfo?(lsalzman)
Attachment #8902859 - Flags: review?(jmuizelaar)
While this is in some sense a "regression", that happened in bug 1287552 when we made SkFontHost_cairo add the LCD filter padding that Skia's native FreeType font host. That was done back in 50, which means all release channels are affected by this bug. Unfortunately, despite it being a "regression" of a sort, the fix here is not to revert, but just to backport Skia's provisional fix for now, until they come up with a better fix. At a minimum we would likely need to backport this to 52 ESR.
Attachment #8902859 - Flags: review?(jmuizelaar) → review+
Pushed by lsalzman@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a1b9aaa4f798
clip FreeType glyph bitmap to mask in Skia. r=jrmuizel
https://hg.mozilla.org/mozilla-central/rev/a1b9aaa4f798
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Please request Beta/ESR52 approval on this when you get a chance. Note that it'll need a rebased patch for ESR52 as well.
Flags: needinfo?(lsalzman)
Comment on attachment 8902859 [details] [diff] [review]
clip FreeType glyph bitmap to mask in Skia

Approval Request Comment
[Feature/Bug causing the regression]: This is a pre-existing issue in Skia that affects all release channels.
[User impact if declined]: Broken font rendering when subpixel AA is used on Linux.
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: no 
[List of other uplifts needed for the feature/fix]:
[Is the change risky?]: no
[Why is the change risky/not risky?]: This is a backport of an upstream Skia fix already successfully deployed in Chrome.
[String changes made/needed]: none
Flags: needinfo?(lsalzman)
Attachment #8902859 - Flags: approval-mozilla-beta?
This is just a simple backport of the fix to 52 ESR.

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
User impact if declined: Broken font rendering with subpixel AA on Linux.
Fix Landed on Version: 57
Risk to taking this patch (and alternatives if risky): none 
String or UUID changes made by this patch: none

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.
Attachment #8904705 - Flags: review+
Attachment #8904705 - Flags: approval-mozilla-esr52?
Comment on attachment 8902859 [details] [diff] [review]
clip FreeType glyph bitmap to mask in Skia

Fix a broken font rendering issue. Beta56+.
Attachment #8902859 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment on attachment 8904705 [details] [diff] [review]
clip FreeType glyph bitmap to mask in Skia (52 ESR)

avoid a font issue with newer freetype, esr52.4+
Attachment #8904705 - Flags: approval-mozilla-esr52? → approval-mozilla-esr52+
You need to log in before you can comment on or make changes to this bug.