bug#59347: 29.0.50; `:family` face setting ignored

[Gregory Heytings <gregory@heytings.org>]

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


I did some further testing to answer your previous email, and I now think 
that my previous patch is a half-measure, and that TRT would be to simply 
ignore the font weight specified in the default face while searching for 
other fonts.  Consider the following recipe:

emacs -Q
M-: (fancy-startup-screen) RET

and now evaluate the following lines in turn:

(set-face-attribute 'default nil :font "Source Code Pro" :weight 'ultra-heavy) ;; 1
(set-face-attribute 'default nil :font "Source Code Pro" :weight 'heavy) ;; 2
(set-face-attribute 'default nil :font "Source Code Pro" :weight 'ultra-bold) ;; 3
(set-face-attribute 'default nil :font "Source Code Pro" :weight 'bold) ;; 4
(set-face-attribute 'default nil :font "Source Code Pro" :weight 'semi-bold) ;; 5
(set-face-attribute 'default nil :font "Source Code Pro" :weight 'medium) ;; 6
(set-face-attribute 'default nil :font "Source Code Pro" :weight 'normal) ;; 7
(set-face-attribute 'default nil :font "Source Code Pro" :weight 'semi-light) ;; 8
(set-face-attribute 'default nil :font "Source Code Pro" :weight 'light) ;; 9
(set-face-attribute 'default nil :font "Source Code Pro" :weight 'ultra-light) ;; 10
(set-face-attribute 'default nil :font "Source Code Pro" :weight 'thin) ;; 11

(If you don't have the Source Code Pro font on your system, I'm sure you 
can find another font with more weight variants with which you will 
observe a similar effect.)

With current master, the variable-pitch face is realized as follows:

- with 1-3: -ADBO-Source Code Pro-black-normal-normal-*-29-*-*-*-m-0-iso10646-1, which is a monospace font

- with 4: -PfEd-DejaVu Sans-bold-normal-normal-*-29-*-*-*-*-0-iso10646-1, which is a variable pitch font

- with 5: -ADBO-Source Code Pro-semibold-normal-normal-*-29-*-*-*-m-0-iso10646-1, which is again a monospace font

- with 6: -urw-nimbus sans l-regular-r-normal--29-210-100-100-p-158-iso8859-1, which is a variable pitch font but without anti-aliasing

- with 7: -PfEd-DejaVu Sans-regular-normal-normal-*-29-*-*-*-*-0-iso10646-1, which is a variable pitch font

- with 8-9: -ADBO-Source Code Pro-light-normal-normal-*-29-*-*-*-m-0-iso10646-1, which is again a monospace font

- with 10-11: -PfEd-DejaVu Sans-ultralight-normal-normal-*-29-*-*-*-*-0-iso10646-1, which is a variable pitch font

That can't be right.  Only 4, 7, and 10-11 correspond to what is expected 
for that face, namely a variable pitch font.  Fixing the 'medium' case (as 
my patch does) only improves 7.  Fixing the other cases would amount to do 
something similar for each possible weight (e.g. "also try 'bold' after 
trying 'ultra-bold'"), which in fact amounts to ignoring the weight in 
spec.

When the weight is ignored in font_find_for_lface, the variable-pitch face 
is realized as follows:

- with 1-5: -PfEd-DejaVu Sans-bold-normal-normal-*-29-*-*-*-*-0-iso10646-1

- with 6-7: -PfEd-DejaVu Sans-regular-normal-normal-*-29-*-*-*-*-0-iso10646-1

- with 8-11: -PfEd-DejaVu Sans-ultralight-normal-normal-*-29-*-*-*-*-0-iso10646-1

Which is clearly TRT.

New patch attached.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Unset-the-weight-in-font-specs-when-searching-for-a-.patch --]
[-- Type: text/x-diff; name=Unset-the-weight-in-font-specs-when-searching-for-a-.patch, Size: 1577 bytes --]

From 7e8fb5cebd4031d947940ae8eeef0a2b72fb4cba Mon Sep 17 00:00:00 2001
From: Gregory Heytings <gregory@heytings.org>
Date: Mon, 21 Nov 2022 23:26:07 +0000
Subject: [PATCH] Unset the weight in font specs when searching for a font.

Between commits bf0d3f76dc (2014) and 6b1ed2f2c9 (2022),
realize_gui_face called font_load_for_lface with an empty or partly
emptied font spec, i.e. it ignored a part of its attrs argument.  The
rationale given in bug#17973, which led to bf0d3f76dc, is not clear.
However, 6b1ed2f2c9 leads to suboptimal font choices when the font
chosen for the default face has a weight that is not supported by
other available fonts on the system, such as 'medium' or 'heavy'.
Therefore, the weight in the spec argument to font_find_for_lface
must be unset.

* src/font.c (font_find_for_lface): Unset the weight of the font spec.
Fixes bug#57555 and bug#59347.
---
 src/font.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/src/font.c b/src/font.c
index 6e720bc285..fc8019fe13 100644
--- a/src/font.c
+++ b/src/font.c
@@ -3003,6 +3003,10 @@ font_find_for_lface (struct frame *f, Lisp_Object *attrs, Lisp_Object spec, int
     }
   ASET (work, FONT_SIZE_INDEX, Qnil);
 
+  /* Also ignore the font weight, which when set leads to suboptimal
+     font choices.  See bug#59347.  */
+  ASET (work, FONT_WEIGHT_INDEX, Qnil);
+
   /* Foundry specification alternatives: from the most specific to the
      least specific and finally an unspecified one.  */
   foundry[0] = AREF (work, FONT_FOUNDRY_INDEX);
-- 
2.35.1