[cairo] Error handling in cairo_type1_font_subset_get_glyph_names_and_widths

[Peter Weilbacher]

On 30.10.08 11:16, Adrian Johnson wrote:
> Peter Weilbacher wrote:
>> I was investigating (full, mostly irrelevant details at
>> https://bugzilla.mozilla.org/show_bug.cgi?id=446290) why I saw
>> problems creating a PDF when I noticed an error message
>> could not load glyph 499
>> from cairo.
>>
>> It turns out that the font used is broken (or FreeType is broken
>> that it cannot load glyph 499 from it), but I wonder if the
>> error handling in cairo_type1_font_subset_get_glyph_names_and_widths()
>> makes sense.[1][2] To me (someone without any PS knowledge) it seems
>> that this function gets data for many glyphs; at least in my case
>> most of them won't be used. So my workaround is to only access the
>> glyph width for success and otherwise set width=0 and just continue.
>> Patch appended below.
>>
>> Not sure how wrong this is but at least the PDF created with this
>> workaround seems to be OK while the original one was so garbled
>> that all viewers had trouble displaying it...
>>
>> So, I guess I'm asking what the best way would be to make cairo
>> more robust against slightly broken fonts for this case.
>>
>> 1: It's probably not optimal for a library to print errors to
>> stdout, but in this case I am happy about that because it made
>> it easier for me to locate the problem. :-)
>> 2: Returning CAIRO_STATUS_NO_MEMORY is probably also not optimal
>> for an FT_Load_Glyph() failure, the error I get is really
>> "opcode syntax error"...
>>
>> Cheers,
>> Peter.
>>
>
> Thanks for the bug report and determining the cause of the bug. The fix
> that I would like to implement (when the 1.9.x branch opens) is to:
>
> - Use _cairo_scaled_glyph_lookup() to get the glyph advance instead of
> using FreeType directly. And only get the widths for the glyphs that are
> actually used.
>
> - The names of all the glyphs need to be be read as Type 1 subset needs
> to be able to lookup glyphs by name. If FreeType fails while getting a
> name we should leave the name as NULL. If a name lookup fails (due to
> the name being NULL) we should return CAIRO_INT_STATUS_UNSUPPORTED. This
> will avoid failing for glyphs that are not used.
>
> - Any other FreeType errors that are returning CAIRO_STATUS_NO_MEMORY
> and printing to stdout should instead return
> CAIRO_INT_STATUS_UNSUPPORTED. Returning CAIRO_INT_STATUS_UNSUPPORTED
> will fallback to embedding a Type 1 Fallback font which should always work.

OK, all this sounds nice. But for the first item I'm wondering how you are
going to get the advance for that glyph if you cannot load it. Is there
some other code in cairo that magically derives the metrics without asking
FreeType?

> If you need a quick fix now it would be safer to return
> CAIRO_INT_STATUS_UNSUPPORTED when an error is detected. The risk in
> setting the glyph advance to 0 is the possibility that a glyph that is
> used will have the glyph advance set to 0. The PS and PDF output require
> the glyphs advances in the embedded subset to be correct in order for
> the glyphs to be correctly positioned.

Yeah, so if they were used there would be other characters on top of the
ones with width=0, right? Will try the workaround you suggest.

    Peter.