On Fri, January 31, 2014 16:53, Behdad Esfahbod wrote: > On 14-01-30 09:26 PM, Akira TAGOH wrote: >> On Fri, Jan 31, 2014 at 4:27 AM, Behdad Esfahbod <behdad@xxxxxxxxxx> wrote: >>> The problem is that in fcfreetype.c, we only set FC_SPACING on the font >>> if >>> it's not FC_PROPORTIONAL. Then when matching, if the font pattern doesn't >>> have FC_SPACING, it is an implicit match. Ouch! I think we should fix >>> the >>> first part. I'm sure there was a reason for it (broken fonts, etc?). But I >>> think we should fix it now. Here's the relevant part of fcfreetype.c: >>> if (spacing != FC_PROPORTIONAL) >>> if (!FcPatternAddInteger (pat, FC_SPACING, spacing)) >>> goto bail2; >> The reason may be true. but I have no idea what the font it was. we may need to figure out the side-effect on this change in advance. Aside from that, we should add a test case to check if targeted parameters on matcher is surely available after scanning. or having a trick for those missing parameters on matcher and ignore it. later one might be more robust for incompatibility on matcher in the future perhaps. > Sure that would be good long term. But in this case, not setting FC_PROPORTIONAL totally breaks any use of this property. > Indeed, we want to be able to match monospace fonts. We should do that in > vte > for example. If I look at: http://cgit.freedesktop.org/fontconfig/commit/?id=148656ed8b3755f0634be14ae60996a1ad493836 and: http://cgit.freedesktop.org/fontconfig/commit/?id=848d32bd3f141f0c14abfec38d4cf27eedd1f0a5 it appears to me that finding a reliable way to set FC_MONO (without the use of buggy font metadata) was the main concern back then. It seems that for font formats other than PCF it is done by actually comparing all the glyph widths (in FcFreeTypeCharSetAndSpacingForSize). Maybe the problem with this heuristic approach wasn't how to detect FC_MONO but how to distinguish between the other values, which could explain why the latter weren't actually used in patterns, particularly as the first version from the above commit didn't support detecting FC_DUAL. When that was added later maybe this could have been changed though, since FC_CHARCELL is probably somewhat of a special case for PCF anyway (no expert on this though). As it stands now the code is: [...] if (fixed_advance) *spacing = FC_MONO; else if (dual_advance && APPROXIMATELY_EQUAL (2 * FC_MIN (advance_one, advance_two), FC_MAX (advance_one, advance_two))) *spacing = FC_DUAL; else *spacing = FC_PROPORTIONAL; return fcs; [...] NB. While I agree that it makes no sense having SPACING in the matcher in its current state, the situation for terminal apps etc. is not that bad I think. They can always use the "monospace" family as fallback value in the pattern, or use FcFontList with spacing=mono or spacing=dual to get available mono/dual-space families before matching, which should be the natural choice for GUI pickers anyway... Raimund -- Worringer Str 31 Duesseldorf 40211 DE home: <rs@xxxxxxxx> +49-179-2981632 icq 16845346 work: <rs@xxxxxxxxxxxxxxx> _______________________________________________ Fontconfig mailing list Fontconfig@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/fontconfig
