Bug 20255

Summary:	cairo_scaled_font_glyph_extents breaks with invalid glyph id
Product:	cairo	Reporter:	Elmar Braun <elmar.braun>
Component:	win32 backend	Assignee:	cairo-bugs mailing list <cairo-bugs>
Status:	RESOLVED FIXED	QA Contact:	cairo-bugs mailing list <cairo-bugs>
Severity:	normal
Priority:	medium
Version:	1.8.6
Hardware:	All
OS:	Windows (All)
Whiteboard:
i915 platform:		i915 features:
Attachments:	Testcase Patch to remove broken error handling Alternate patch

Description Elmar Braun 2009-02-22 12:47:14 UTC

Using cairo_scaled_font_glyph_extents to get the extents for non-existing glyphs puts a cairo_scaled_font_t permanently into an unusable state. This did not happen in Cairo 1.6.

The attached test case requests three glyph extents: first for the character 'a', then for a non-existing glyph, then again for the character 'a'. In Cairo 1.6, unlike Cairo 1.8, the third cairo_scaled_font_glyph_extents returns the same result as the first.

This is the output for the attached testcase with Cairo 1.6.4:

x/y advance for char 'a' -> glyph '0x44': 5.000000/0.000000
status of cairo_scaled_font_t: success
x/y advance for '0xffff': 0.000000/0.000000
status of cairo_scaled_font_t: success
x/y advance for char 'a' -> glyph '0x44': 5.000000/0.000000

This is the output for the attached testcase with Cairo 1.8.6:

x/y advance for char 'a' -> glyph '0x44': 5.000000/0.000000
status of cairo_scaled_font_t: success
x/y advance for non-existing glyph '0xffff': 0.000000/0.000000
status of cairo_scaled_font_t: out of memory
x/y advance for char 'a' -> glyph '0x44': 0.000000/0.000000

Comment 1 Elmar Braun 2009-02-22 12:48:00 UTC

Created attachment 23180 [details]
Testcase

Comment 2 Chris Wilson 2009-05-20 09:20:25 UTC

I think this should have been fixed by Adrian's commit:

commit 2a34992cccfd77c2acf30fe851311f16137ba32f
Author: Adrian Johnson <ajohnson@redneon.com>
Date:   Sun May 17 18:12:39 2009 +0930

    Ensure win32 font index_to_ucs4() sets ucs4 to -1 if lookup fails


Could you please try cairo from git (or apply that relatively simple patch) and see if it does indeed fix this problem? Thanks.

Comment 3 Elmar Braun 2009-05-24 08:00:23 UTC

Created attachment 26170 [details] [review]
Patch to remove broken error handling

Adrian's commit doesn't solve the problem.

The problem is overzealous error checking introduced in commit 47824dee31acdaf5ad6488dd2714e9f28c936aa2. The problem can be fixed by removing it (see patch).

GetGlyphOutlineW returns GDI_ERROR when called with a glyph id that doesn't exist in the font. In Cairo 1.6 _cairo_win32_scaled_font_init_glyph_metrics would memset the glyph metrics to 0 and then return CAIRO_STATUS_SUCCESS. In 1.8 it returns an error, which renders the scaled font permanently unusable.

Comment 4 Elmar Braun 2009-05-24 08:23:26 UTC

Created attachment 26171 [details] [review]
Alternate patch

An alternate solution that leaves some of the error checking in place is this: check GetLastError after GetGlyphOutlineW, because it returns a success if GDI_ERROR was caused by an invalid glyph id.

Comment 5 Elmar Braun 2009-05-24 08:27:33 UTC

Quick note about the patches: _cairo_win32_scaled_font_init_glyph_metrics has another call to GetGlyphOutlineW somewhat further down in the function body; the suggested changes should probably be applied there too.

Comment 6 Elmar Braun 2009-05-24 08:34:46 UTC

In comment #3 I gave a wrong commit number, I was talking about 6808174e72c923ebabe61846617496e25df92363

Comment 7 Chris Wilson 2009-06-10 05:05:58 UTC

Thank you Elmar for your diagnosis of this bug, and your timely reminder on cairo@.

commit 610da573e91810c53305b6bfe02eb7b714e3d08e
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Wed Jun 10 12:59:36 2009 +0100

    [win32-font] Non-fatal error from GetGlyphOutlineW(GGO_METRICS)
    
    If GetGlyphOutlineW(GGO_METRICS) fails to retreive the metrics for the
    specified glyph it returns GDI_ERROR. Like ft, do not interpret this as a
    fatal error but just mark the glyph as empty.
    
    Fixes http://bugs.freedesktop.org/show_bug.cgi?id=20255
    Bug 20255 -- cairo_scaled_font_glyph_extents breaks with invalid glyph id

And cherry-picked into 1.8.

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.