GNOME Bugzilla – Bug 310700
Support for aliases in win32 load_font() implementation
Last modified: 2008-05-27 01:15:27 UTC
The function pango_layout_line_get_empty_extents() is called for instance with a layout that has a font description that uses the name "sans". With a fontconfig-based Pango backend, "sans" apparently works also as a font name, but isn't a name like that supposed to be used to load a fontset? Or am I confused? Currently there is a rather odd piece of code in pangowin32-fontmap.c that makes pango_layout_line_get_empty_extents() find a font "sans" also on Win32. (Look for the comment "Thought pango.aliases would make this code unnecessart, but no".) There is an entry in ChangeLog.pre-1-2 that says: * pango/pangowin32-fontmap.c (pango_win32_insert_font): Revert change from 2002-09-21: Don't bypass the code that automatically adds fonts to the families "monospace", "serif" and "sans". I thought it would be unnecessary if you have a pango.aliases that sets up aliases for these family names, but apparently not. Without this code, pango_layout_line_get_empty_extents() thinks empty lines have zero height, as it tries to use a font called "sans" for instance, and no aliases get used. If I change pango_layoyt_line_get_empty_extents() to use a fontset instead of a font, the odd piece of crap in pangowin32-fontmap.c can be removed, which is nice. Patch attached.
Created attachment 49340 [details] [review] Suggested patch
I think we really do want font metrics here not fontset metrics ... fontset metrics tend to be bigger (all scripts that could be used), while here we really want the extents of an empty line to be the same of a line with just a single space in them. What I'd suggest is that you make the win32 backend perform similarly to what the fontconfig backend does and make load_font() return the first font that load_fontset() would return. The code I have in pangofc-fontmap.c isn't immediately reusable, since it has load_font() call load_fontset(), and the default implementation of that in turn calls load_font()... you may need to cut-and-paste pango_fontmap_real_load_fontset() into pangowin32-fontmap.c, but perhaps there is some other way it could be organized.
Marking patches...
How about changing pango_font_map_load_font to return the first font of the fontset, and load_fontset calling .->load_font directly?
Another related issue is that the font picker on Windows does show the "sans", "serif" and "monospace" pseudo-fonts, as it presumably should. *But*, it shows several copies of "Normal", "Italic", "Bold" and "Bold Italic" in the Style widget for these pseudo-fonts. And the Preview widget indeed shows a different actual font for each "Normal"... Apparently they correspond to the fonts added to the font families for these pseudo-font names by the "odd code" in pango_win32_insert_font() mentioned in the initial comment. (If I ifdef out that "odd code" snippet, then the "sans" etc pseudo-fonts don't show up at all in the font selector, and also I get critical errors "_pango_engine_shape_covers: assertion `PANGO_IS_FONT (font)' failed". Sigh.)
The issue mentioned in comment #5 is fixed now. The odd piece of code mentioned in the initial description is now gone, and the "sans", "serif" and "monospace" font families are now instead added in what I think is a somewhat cleaner way, and they refer to only the first font of the corresponding alias list (fontset) that exists on the machine. I think this bug can be closed. There are still a couple of other bugs open against pangowin32 that describe valid issue.