432411 - Avoid GetGlyphOutlineW for GDI "vector" fonts in cairo

Status: NEW

(Core :: Graphics: Text, defect, P3)

Description

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

Loading a testcase using old-style Win32 "vector" fonts such as 'Modern' can cause warnings about GetGlyphOutlineW failing. These come when we try to measure glyph extents. The current code takes the non-GetGlyphOutlineW path for bitmap fonts, but GetGlyphOutlineW fails for vector fonts as well.

This patch fixes that, and refactors the information about font format into an enumeration. I think this is easier to follow.

Maybe I should just send this directly upstream to cairo? But who would review it there?

Comment 1

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Adrian Johnson -- I've just Cc'd him to this bug.

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Should I just send this patch to the cairo list then?

Comment 3

[Stuart Parmenter]

Comment on attachment 319533 [details] [diff] [review]
fix

this looks OK

Comment 4

[u285178]

The patch has missed OpenType/CFF fonts. I suggest changing the code for getting the font type to:

+   /* The TMPF_TRUETYPE test is not required (both TrueType and OpenType/CFF
+    * fonts have a CMAP table . However it is a free optimization given that 
+    * TrueType fonts are the most common fonts on Windows.
+    */
+    if (metrics.tmPitchAndFamily & TMPF_TRUETYPE ||
+        GetFontData (hdc, CMAP_TAG, 0, NULL, 0) != GDI_ERROR) {
+        scaled_font->type = CAIRO_WIN32_FONT_TRUETYPE;
+    } else if (!(metrics.tmPitchAndFamily & TMPF_VECTOR)) {
+        scaled_font->type = CAIRO_WIN32_FONT_BITMAP;
+    } else {
+         * Calling GetFontData() with tag = 0 on a Type 1 font will retrieve
+         * the Type 1 font. GetFontData() will fail on a stroke fonts such
+         * as "Script" and "Modern".
+         if (GetFontData (hdc, 0, 0, NULL, 0) != GDI_ERROR) {
+             scaled_font->type = CAIRO_WIN32_FONT_TYPE1;
+         } else {
+             scaled_font->type = CAIRO_WIN32_FONT_VECTOR;
+         }
     }


I've committed some recent changes to cairo-win32-font.c so you will probably need to rebase. There is a new _cairo_win32_font_is_bitmap() function.

I suggest adding a comment to the enum explaining that CAIRO_WIN32_FONT_TRUETYPE also includes OpenType/CFF fonts.

Microsoft calls fonts like Modern and Script "stroke fonts" [1]. A comment explaining that CAIRO_WIN32_FONT_VECTOR means stroke fonts would be good. Or maybe just change VECTOR to STROKE as vector can imply any outline font and the TMPF_VECTOR flag is set to true for any non bitamp font.

[1] http://www.microsoft.com/mspress/books/sampchap/2344.aspx#606

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Good idea, I'll change it to stroke.

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Adrian, do you want me to send the new patch to the cairo list, or just attach it here, or what?

Comment 7

[u285178]

(In reply to comment #6)
> Adrian, do you want me to send the new patch to the cairo list, or just attach
> it here, or what?
> 

Either is fine. Attaching here is probably easiest.

Comment 8

[Joe Drew (not getting mail)]

This never got committed upstream, but I don't know if we care anymore.

Comment 9

[Jeff Muizelaar [:jrmuizel]]

We seem to be the only recent browser that supports vector fonts. We could probably drop support for them. Still it probably makes sense to try to land something like this sometime.

Comment 10

[Sylvestre Ledru [:Sylvestre]]

Moving to p3 because no activity for at least 1 year(s).
See https://github.com/mozilla/bug-handling/blob/master/policy/triage-bugzilla.md#how-do-you-triage for more information