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

Status: RESOLVED FIXED

(Core :: Graphics: Text, defect)

Description

[Bob Clary [:bc] (inactive)]

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>

Comment 1

[Bob Clary [:bc] (inactive)]

Attachment: fira-sans-regular.ttf

Comment 2

[Jonathan Kew [:jfkthame]]

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.

Comment 3

[Jonathan Kew [:jfkthame]]

Attachment: Fall back to cairo's glyph metrics API if FreeType fails in some way, or if we're not using a variation font

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.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
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

Comment 5

[Pulsebot]

Pushed by [e-mail]:
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

Comment 6

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/e3aa74bcd495
https://hg.mozilla.org/mozilla-central/rev/ed0c321f39cc

Comment 7

[Jonathan Kew [:jfkthame]]

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

Comment 8

[Ryan VanderMeulen [:RyanVM]]

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.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/55dd9c21061a (FIREFOX_59b_RELBRANCH)
https://hg.mozilla.org/releases/mozilla-release/rev/745e458677e3