[cairo] Error handling in cairo_type1_font_subset_get_glyph_names_and_widths

[Adrian Johnson]

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.

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.