Bug 164689 – [FreeType] FontPlatformData::fallbacks() returns unprepared FcPatterns

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Michael Catanzaro]

Behdad warned me about this at the Web Engines Hackfest. From the documentation of FcFontSort():

"""The returned FcFontSet references FcPattern structures which may be shared by the return value from multiple FcFontSort calls, applications must not modify these patterns. Instead, they should be passed, along with p to FcFontRenderPrepare which combines them into a complete pattern."""

That means each call to FcFontSort() must be followed up by a call to FcFontRenderPrepare(), else the patterns will lack information about how to draw the font properly.

[Michael Catanzaro]

Created attachment 294643 [details]
[FreeType] FontPlatformData::fallbacks() returns unprepared FcPatterns

[Michael Catanzaro]

Same caveat here as in bug #82889: it's surely going to affect layout tests, so has to be landed once we've cleaned up our existing expectations.

[Myles C. Maxfield]

I would review this but I don't understand enough about FontConfig.

[Michael Catanzaro]

Comment on attachment 294643 [details]
[FreeType] FontPlatformData::fallbacks() returns unprepared FcPatterns

Seems like a good time to try landing this. Carlos?

[Carlos Garcia Campos]

Comment on attachment 294643 [details]
[FreeType] FontPlatformData::fallbacks() returns unprepared FcPatterns

View in context: https://bugs.webkit.org/attachment.cgi?id=294643&action=review

> Source/WebCore/platform/graphics/freetype/FontPlatformDataFreeType.cpp:265
> +        for (int i = 0; i < unpreparedFallbacks.get()->nfont; i++) {

unsigned

> Source/WebCore/platform/graphics/freetype/FontPlatformDataFreeType.cpp:267
> +            FcPattern* pattern = FcFontRenderPrepare(nullptr, m_pattern.get(), unpreparedFallbacks.get()->fonts[i]);
> +            FcFontSetAdd(m_fallbacks.get(), pattern);

This could probably be just one line

[Michael Catanzaro]

(In reply to comment #5)
> > Source/WebCore/platform/graphics/freetype/FontPlatformDataFreeType.cpp:265
> > +        for (int i = 0; i < unpreparedFallbacks.get()->nfont; i++) {
> 
> unsigned

Nope, because FcFontSet::nfont is an int.

> > Source/WebCore/platform/graphics/freetype/FontPlatformDataFreeType.cpp:267
> > +            FcPattern* pattern = FcFontRenderPrepare(nullptr, m_pattern.get(), unpreparedFallbacks.get()->fonts[i]);
> > +            FcFontSetAdd(m_fallbacks.get(), pattern);
> 
> This could probably be just one line

OK.

[Michael Catanzaro]

Committed r213532: <http://trac.webkit.org/changeset/213532>