bug#52888: 29.0.50; font_{delete_unmatched,score} do not handle nil FONT_WEIGHT_INDEX

[Sean Whitton <spwhitton@spwhitton.name>]

[-- Attachment #1: Type: text/plain, Size: 1896 bytes --]

Hello,

On Wed 05 Jan 2022 at 02:37PM +02, Eli Zaretskii wrote:

> According to what I see in the documentation, FcPatternGetRange exists
> only since version 2.11.91 of Fontconfig, whereas we support 2.2.0 and
> up.

Ah, right.  Is this version bound written down anywhere other than the
conditionals in configure.ac?  Sorry I didn't think to look there.

> So we cannot unconditionally use that API, and some part of the
> temporary solution will have to be left in the source.  I'd need to
> see the final patch with that in mind, before I can decide whether it
> is simple/safe enough for the release branch.

My patch requires FC_VARIABLE which was not added until 2.12.91, so in
the attached revision I've set up preprocessor conditionals based on
FC_VARIABLE.  Checking that FC_VARIABLE is true is how we know it's one
of the virtual entries.

It looks like fontconfig commit 83b41611 introduced the meta patterns
under discussion.  The first tagged release including that commit is
2.12.91.  So AFAICT nothing of the old fix need remain.

> I'm far from being an expert on Fontconfig programming, but the above
> use of 'range' looks strange: it's a pointer that starts pointing to
> some random (potentially invalid) address, and you pass its address to
> FcPatternGetRange, which presumably assigns to it a valid point.  But
> doesn't that valid pointer need to be released somehow after we use
> it?  Or does it point to static area(s)?  I cannot find anything in
> the Fontconfig documentation about the memory-management protocols for
> FcValue objects, but maybe we should call FcValueDestroy on it after
> we are done with it?  Or maybe this is not needed, as this passage
> from the docs says near the end:

Yes, I was surprised too, but as has been mentioned, the FcPatternGet*
functions are documented as supplying pointers to storage which must not
be freed.

-- 
Sean Whitton

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: v2-0001-Skip-virtual-FcPattern-entries-for-variable-weigh.patch --]
[-- Type: text/x-patch, Size: 1789 bytes --]

From d88f23882b66243b42d79e0fe897f99f8125c7c4 Mon Sep 17 00:00:00 2001
From: Sean Whitton <spwhitton@spwhitton.name>
Date: Tue, 4 Jan 2022 19:07:29 -0700
Subject: [PATCH v2] Skip virtual FcPattern entries for variable weight fonts

* src/ftfont.c (ftfont_list): Pass FC_VARIABLE to FcObjectSetBuild.
* src/ftfont.c (ftfont_pattern_entity): Skip meta/virtual FcPattern
entries for variable weight fonts (Bug#52888).
---
 src/ftfont.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/src/ftfont.c b/src/ftfont.c
index 2bdcce306b..6bfbc50ffa 100644
--- a/src/ftfont.c
+++ b/src/ftfont.c
@@ -189,6 +189,19 @@ ftfont_pattern_entity (FcPattern *p, Lisp_Object extra)
     return Qnil;
   if (FcPatternGetInteger (p, FC_INDEX, 0, &idx) != FcResultMatch)
     return Qnil;
+#ifdef FC_VARIABLE
+  FcRange *range;
+  if (FcPatternGetRange (p, FC_WEIGHT, 0, &range) == FcResultMatch
+      && FcPatternGetBool (p, FC_VARIABLE, 0, &b) == FcResultMatch
+      && b == FcTrue)
+    /* This is a virtual/meta FcPattern for a variable weight font,
+       from which it is possible to extract an FcRange value
+       specifying the minimum and maximum weights available in this
+       file.  We don't need to know that information explicitly, so
+       skip it.  We will be called with an FcPattern for each actually
+       available, non-virtual weight.  */
+    return Qnil;
+#endif	/* FC_VARIABLE */
 
   file = (char *) str;
   key = Fcons (build_unibyte_string (file), make_fixnum (idx));
@@ -863,6 +876,9 @@ ftfont_list (struct frame *f, Lisp_Object spec)
 #if defined HAVE_XFT && defined FC_COLOR
                              FC_COLOR,
 #endif
+#ifdef FC_VARIABLE
+			     FC_VARIABLE,
+#endif	/* FC_VARIABLE */
 			     NULL);
   if (! objset)
     goto err;
-- 
2.30.2