Assertion failure: !ftError, at gfx/thebes/gfxFT2FontBase.cpp:543

RESOLVED FIXED in Firefox 59

Status

()

defect
RESOLVED FIXED
Last year
Last year

People

(Reporter: bc, Assigned: jfkthame)

Tracking

(Blocks 1 bug, {assertion, regression})

59 Branch
mozilla60
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox58 unaffected, firefox59 fixed, firefox60 fixed)

Details

(Whiteboard: gfx-noted, )

Attachments

(2 attachments)

1. https://www.wr.de/
   Nightly/60, Beta/59 Linux

2. Assertion failure: !ftError, at /mozilla/builds/nightly/mozilla/gfx/thebes/gfxFT2FontBase.cpp:543
#01: gfxFT2FontBase::GetFTGlyphAdvance(unsigned short) (/mozilla/builds/nightly/mozilla/gfx/thebes/gfxFT2FontBase.cpp:543 (discriminator 1))
#02: gfxFT2FontBase::GetCharWidth(char, double*) (/mozilla/builds/nightly/mozilla/gfx/thebes/gfxFT2FontBase.cpp:177)
#03: gfxFT2FontBase::InitMetrics() (/mozilla/builds/nightly/mozilla/gfx/thebes/gfxFT2FontBase.cpp:415)
#04: gfxFT2FontBase::gfxFT2FontBase(RefPtr<mozilla::gfx::UnscaledFontFreeType> const&, _cairo_scaled_font*, gfxFontEntry*, gfxFontStyle const*, bool) (/mozilla/builds/nightly/mozilla/gfx/thebes/gfxFT2FontBase.cpp:38)
#05: RefPtr<mozilla::gfx::UnscaledFontFreeType>::~RefPtr() (/mozilla/builds/nightly/mozilla/firefox-debug/dist/include/mozilla/RefPtr.h:78)
#06: gfxFontconfigFontEntry::CreateFontInstance(gfxFontStyle const*, bool) (/mozilla/builds/nightly/mozilla/gfx/thebes/gfxFcPlatformFontList.cpp:1048)

test.html

<style>
@font-face{
    font-family:"FiraSansRegular";
    src:
    url("fira-sans-regular.ttf")
}
body{
    font-family:"FiraSansRegular"
</style>
Whiteboard: gfx-noted
I was able to reproduce this, and observed that what is happening is that with this particular Fira Sans font (which is a subsetted version, not the original), the FT_Load_Glyph function is failing when the no-hinting flags are _not_ passed to it. Specifically, it is returning FT_Err_Execution_Too_Long "execution context too long".

What's rather puzzling at the moment is that FreeType nevertheless _can_ render the glyphs; the failure only happens when we're calling FT_Load_Glyph to get the glyph advance, but apparently not during painting. I haven't traced enough to figure out why that is.

The FT_Err_Execution_Too_Long error sounds -- without having traced through the FreeType code to see exactly why it arises -- as though there may be an issue with the TrueType instructions (hinting) in this font, perhaps as a result of how it was subset? Or it could actually be a FreeType bug. I'll try to look into this further.

Anyhow, to avoid this issue, and (I hope) also resolve the glyph-spacing issues some users have been reporting (bug 1435234), I'm inclined to revert the implementation of glyph advances in gfxFT2FontBase to something like what we had before bug 1427641 patch 1, except in the case of variation fonts (where we have to use the linearHoriAdvance field). For non-variation fonts, this should return us to the pre-1427641 behavior.
I'm not really happy about this, as I wanted to bypass cairo as much as possible, but for the time being it seems the simplest and safest way to resolve the current problems. I'm expecting this to fix the current glyph-spacing issues (because it effectively reverts to the old implementation of glyph widths) in addition to the failure here.
Attachment #8955090 - Flags: review?(lsalzman)
Assignee: nobody → jfkthame
Status: NEW → ASSIGNED
Attachment #8955090 - Flags: review?(lsalzman) → review+
Pushed by jkew@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e3aa74bcd495
Fall back to cairo's glyph metrics API if FreeType fails in some way, or if we're not using a variation font. r=lsalzman
Pushed by archaeopteryx@coole-files.de:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ed0c321f39cc
Fall back to cairo's glyph metrics API if FreeType fails in some way, or if we're not using a variation font: Add back annotations for failing Android reftests. r=test-fix CLOSED TREE
https://hg.mozilla.org/mozilla-central/rev/e3aa74bcd495
https://hg.mozilla.org/mozilla-central/rev/ed0c321f39cc
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
Comment on attachment 8955090 [details] [diff] [review]
Fall back to cairo's glyph metrics API if FreeType fails in some way, or if we're not using a variation font

Approval Request Comment
[Feature/Bug causing the regression]: glyph-metrics changes from bug 1427641
[User impact if declined]: possible broken rendering of some fonts (& a fatal assertion in debug builds)
[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]: the patch depends on bug 1430446 (which has already been uplifted to mozilla-59)
[Is the change risky?]: no
[Why is the change risky/not risky?]: reverts to pre-1427641 glyph-metrics API for existing fonts
[String changes made/needed]: none
Attachment #8955090 - Flags: approval-mozilla-beta?
Comment on attachment 8955090 [details] [diff] [review]
Fall back to cairo's glyph metrics API if FreeType fails in some way, or if we're not using a variation font

Partially revert a patch that landed in 59 to avoid font rendering regressions for users under some circumstances. Approved for Fx59rc1.
Attachment #8955090 - Flags: approval-mozilla-beta? → approval-mozilla-release+
See Also: → 1438919
You need to log in before you can comment on or make changes to this bug.