bug#57555: 29.0.50; variable-pitch font issue with medium weight default font

bug#57555: 29.0.50; variable-pitch font issue with medium weight default font

[Amin Bandali]

As of the recent 6b1ed2f2c99a1c2da56c5f434570c438cad6576d, I've been
experiencing an issue where Emacs chooses / falls back on 'misc-fixed'
instead of the correct sans-serif variable-pitch font ('GNU FreeSans',
for me) when I use a custom default font with medium weight.

Screenshots from before (no issue) and after (with issue) the commit:

https://kelar.org/~bandali/tmp/20220903/before-6b1ed2f.png
https://kelar.org/~bandali/tmp/20220903/after-6b1ed2f.png

This happens regardless of whether I set the font via X resources:

    Emacs.font: Source Code Pro Medium-10.5

or with Emacs Lisp:

    (set-frame-font "Source Code Pro-10.5:weight=medium" nil t)

This happens only if I use the medium weight variant, and if I leave
that out and just use Source Code Pro (regular) this doesn't happen.
Also, perhaps strangely enough, the issue seems to only affect the
regular variable-pitch font: e.g. italic variable-pitch is correctly
displayed in sans-serif.


In GNU Emacs 29.0.50 (build 86, x86_64-pc-linux-gnu, X toolkit, cairo
 version 1.16.0, Xaw3d scroll bars) of 2022-09-02 built on langa
Repository revision: 63ee620700b1d85f7069a0b0f4232815e5374b4c
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12013000
System Description: Trisquel GNU/Linux Nabia (10.0.1)

Configured using:
 'configure --without-gconf --without-gsettings --without-gpm
 --with-x-toolkit=lucid --with-xaw3d --with-mailutils'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS HARFBUZZ JPEG JSON LCMS2
LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 M17N_FLT MODULES NOTIFY INOTIFY
PDUMPER PNG RSVG SECCOMP SOUND THREADS TIFF TOOLKIT_SCROLL_BARS WEBP
X11 XAW3D XDBE XIM XPM LUCID ZLIB

Important settings:
  value of $LANG: en_CA.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Group

Minor modes in effect:
  gnus-topic-mode: t
  gnus-undo-mode: t
  display-battery-mode: t
  yas-global-mode: t
  yas-minor-mode: t
  server-mode: t
  winner-mode: t
  windmove-mode: t
  global-hl-todo-mode: t
  pixel-scroll-mode: t
  shell-dirtrack-mode: t
  recentf-mode: t
  savehist-mode: t
  save-place-mode: t
  global-display-fill-column-indicator-mode: t
  global-auto-revert-mode: t
  display-time-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  global-prettify-symbols-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  buffer-read-only: t
  column-number-mode: t
  line-number-mode: t
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  temp-buffer-resize-mode: t

Load-path shadows:
None found.

Features:
(shadow sort gnus-cite flyspell ispell mail-extr emacsbug gnus-topic
nndoc nndraft nnmh utf-7 epa-file network-stream nsm nnfolder nnnil
gnus-agent gnus-srvr gnus-score score-mode nnvirtual gnus-msg gnus-art
mm-uu mml2015 mm-view mml-smime smime gnutls dig nntp gnus-cache
gnus-sum shr pixel-fill kinsoku url-file svg dom gnus-group gnus-undo
gnus-start gnus-dbus gnus-cloud nnimap nnmail mail-source utf7 nnoo
gnus-spec gnus-int gnus-range message sendmail yank-media puny dired-x
dired dired-loaddefs rfc822 mml mml-sec epa epg rfc6068 epg-config
mm-decode mm-bodies mm-encode mail-parse rfc2231 rfc2047 rfc2045
ietf-drums mailabbrev gmm-utils mailheader gnus-win gnus nnheader
gnus-util mail-utils range mm-util mail-prsvr battery dbus
cmake-font-lock boxquote rect cl-extra yasnippet help-mode server
winner windmove cmake-mode thingatpt derived rst compile
text-property-search hl-todo pcase delight ffs pixel-scroll cua-base
tramp-cache time-stamp tramp-sh tramp rx tramp-loaddefs trampver
tramp-integration cus-edit pp cus-start cus-load icons files-x
tramp-compat shell pcomplete comint ansi-color ring parse-time iso8601
time-date ls-lisp format-spec recentf tree-widget wid-edit savehist
saveplace easy-mmode display-fill-column-indicator autorevert
filenotify time xml finder-inf boxquote-autoloads
modus-themes-autoloads info package browse-url url url-proxy
url-privacy url-expand url-methods url-history url-cookie
generate-lisp-file url-domsuf url-util mailcap url-handlers url-parse
auth-source cl-seq eieio eieio-core cl-macs password-cache json subr-x
map byte-opt gv bytecomp byte-compile cconv url-vars cl-loaddefs
cl-lib view bandali-po bandali-erc bandali-message bandali-gnus
bandali-dired bandali-ibuffer ibuf-macs bandali-eshell bandali-org
advice rmc iso-transl tooltip eldoc paren electric uniquify ediff-hook
vc-hooks lisp-float-type elisp-mode mwheel term/x-win x-win
term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe
tabulated-list replace newcomment text-mode lisp-mode prog-mode
register page tab-bar menu-bar rfn-eshadow isearch easymenu timer
select scroll-bar mouse jit-lock font-lock syntax font-core
term/tty-colors frame minibuffer nadvice seq simple cl-generic
indonesian philippine cham georgian utf-8-lang misc-lang vietnamese
tibetan thai tai-viet lao korean japanese eucjp-ms cp51932 hebrew
greek romanian slovak czech european ethiopic indian cyrillic chinese
composite emoji-zwj charscript charprop case-table epa-hook
jka-cmpr-hook help abbrev obarray oclosure cl-preloaded button
loaddefs faces cus-face macroexp files window text-properties overlay
sha1 md5 base64 format env code-pages mule custom widget keymap
hashtable-print-readable backquote threads dbusbind inotify lcms2
dynamic-setting font-render-setting cairo x-toolkit x multi-tty
make-network-process emacs)

Memory information:
((conses 16 272607 55417)
 (symbols 48 21970 2)
 (strings 32 80614 6279)
 (string-bytes 1 2420189)
 (vectors 16 37593)
 (vector-slots 8 437507 11528)
 (floats 8 264 106)
 (intervals 56 1444 0)
 (buffers 1000 24))

bug#57555: 29.0.50; variable-pitch font issue with medium weight default font

[Eli Zaretskii]

> From: Amin Bandali <bandali@gnu.org>
> Date: Fri, 02 Sep 2022 21:35:24 -0400
> 
> As of the recent 6b1ed2f2c99a1c2da56c5f434570c438cad6576d, I've been
> experiencing an issue where Emacs chooses / falls back on 'misc-fixed'
> instead of the correct sans-serif variable-pitch font ('GNU FreeSans',
> for me) when I use a custom default font with medium weight.
> 
> Screenshots from before (no issue) and after (with issue) the commit:
> 
> https://kelar.org/~bandali/tmp/20220903/before-6b1ed2f.png
> https://kelar.org/~bandali/tmp/20220903/after-6b1ed2f.png
> 
> This happens regardless of whether I set the font via X resources:
> 
>     Emacs.font: Source Code Pro Medium-10.5
> 
> or with Emacs Lisp:
> 
>     (set-frame-font "Source Code Pro-10.5:weight=medium" nil t)
> 
> This happens only if I use the medium weight variant, and if I leave
> that out and just use Source Code Pro (regular) this doesn't happen.
> Also, perhaps strangely enough, the issue seems to only affect the
> regular variable-pitch font: e.g. italic variable-pitch is correctly
> displayed in sans-serif.

Thanks, but please show the font setup you are using; without that, the
"instead of the correct sans-serif variable-pitch font" part is
impossible to interpret.

Also, do you have the medium (not regular) variant of the GNU FreeSans
font installed?  If not, can you install it and try again?

What seems to happen here is that Emacs decides that the font you
don't like is a better match than the one you do like.  Which is not a
serious problem from my POV (as long as the text is legible), just a
minor aesthetic annoyance.  To fix such annoyances, one must tune
one's font configuration.  But even if the annoyance stays, it's
definitely less serious than the original problem which that commit
attempted to fix.

Thanks.

bug#57555: 29.0.50; variable-pitch font issue with medium weight default font

[Amin Bandali]

Eli Zaretskii writes:

[...]
>
> Thanks, but please show the font setup you are using; without that, the
> "instead of the correct sans-serif variable-pitch font" part is
> impossible to interpret.

Thanks for your reply, Eli.  Globally, I'm basically using the default
font setup that Trisquel GNU/Linux comes with.  Here is a tarball of
the /etc/fonts/ directory on my system:

https://kelar.org/~bandali/tmp/20220903/etc-fonts.tar.gz

Beyond that, I have a few additional local (user-specific) settings in
in my ~/.config/fontconfig/fonts.conf, but this is the main relevant
excerpt, where I specify that sans-serif should be GNU FreeSans (and
this works basically in all applications, including Firefox-based
browsers, and until recently Emacs as well):

  <alias binding="strong">
   <family>sans-serif</family>
   <prefer>
    <family>FreeSans</family>
    <family>Sahel WOL</family>
    <family>Liberation Sans</family>
    <family>Roboto</family>
   </prefer>
  </alias>

The complete file is available from:

https://kelar.org/~bandali/tmp/20220903/fonts.conf

> Also, do you have the medium (not regular) variant of the GNU FreeSans
> font installed?  If not, can you install it and try again?

As far as I can tell, GNU FreeSans does not have a medium variant;
only regular, bold, oblique, and bold oblique:

$ ls /usr/share/fonts/truetype/freefont/FreeSans*.ttf
/usr/share/fonts/truetype/freefont/FreeSansBoldOblique.ttf
/usr/share/fonts/truetype/freefont/FreeSansBold.ttf
/usr/share/fonts/truetype/freefont/FreeSansOblique.ttf
/usr/share/fonts/truetype/freefont/FreeSans.ttf

I suppose what might be happening here is that Emacs now tries to use
a medium variant for the variable-pitch GNU FreeSans as well (even
though I'd expect that 'medium' to only apply to the font I explicitly
mentioned it for, i.e. Source Code Pro), which does not have such a
variant, and then it falls all the way back to "misc-fixed".  What is
a bit curious, though, is that this doesn't seem to affect italic
variable-pitch text, and Emacs correctly picks GNU FreeSans oblique
for that just fine.

> What seems to happen here is that Emacs decides that the font you
> don't like is a better match than the one you do like.  Which is not a
> serious problem from my POV (as long as the text is legible), just a
> minor aesthetic annoyance.  To fix such annoyances, one must tune
> one's font configuration.  But even if the annoyance stays, it's
> definitely less serious than the original problem which that commit
> attempted to fix.
>
> Thanks.
>

Right.  For what it's worth, I find Emacs picking misc-fixed for
variable-pitch text more than just a minor annoyance: for long hours
of use, I find misc-fixed it considerably less comfortable to my eyes
than GNU FreeSans (for proportional/variable-pitch) or Source Code Pro
medium (for monospace).


Thanks again,
-a

bug#57555: 29.0.50; variable-pitch font issue with medium weight default font

[Eli Zaretskii]

> From: Amin Bandali <bandali@gnu.org>
> Cc: 57555@debbugs.gnu.org
> Date: Mon, 05 Sep 2022 03:06:48 -0400
> 
> Eli Zaretskii writes:
> 
> [...]
> >
> > Thanks, but please show the font setup you are using; without that, the
> > "instead of the correct sans-serif variable-pitch font" part is
> > impossible to interpret.
> 
> Thanks for your reply, Eli.  Globally, I'm basically using the default
> font setup that Trisquel GNU/Linux comes with.  Here is a tarball of
> the /etc/fonts/ directory on my system:

Thanks, but I meant the font setup specific to Emacs, i.e.:

  . any set-fontest-font calls in your init files
  . any set-frame-font calls or any other face-related calls in your
    init files that could affect the font selection

Sorry for my imprecise wording.

> > Also, do you have the medium (not regular) variant of the GNU FreeSans
> > font installed?  If not, can you install it and try again?
> 
> As far as I can tell, GNU FreeSans does not have a medium variant;
> only regular, bold, oblique, and bold oblique:
> 
> $ ls /usr/share/fonts/truetype/freefont/FreeSans*.ttf
> /usr/share/fonts/truetype/freefont/FreeSansBoldOblique.ttf
> /usr/share/fonts/truetype/freefont/FreeSansBold.ttf
> /usr/share/fonts/truetype/freefont/FreeSansOblique.ttf
> /usr/share/fonts/truetype/freefont/FreeSans.ttf
> 
> I suppose what might be happening here is that Emacs now tries to use
> a medium variant for the variable-pitch GNU FreeSans as well (even
> though I'd expect that 'medium' to only apply to the font I explicitly
> mentioned it for, i.e. Source Code Pro), which does not have such a
> variant, and then it falls all the way back to "misc-fixed".  What is
> a bit curious, though, is that this doesn't seem to affect italic
> variable-pitch text, and Emacs correctly picks GNU FreeSans oblique
> for that just fine.

I think this means that Emacs gives greater importance to consistency
in weight than to consistency of the family.  Not sure if that is TRT.

> Right.  For what it's worth, I find Emacs picking misc-fixed for
> variable-pitch text more than just a minor annoyance: for long hours
> of use, I find misc-fixed it considerably less comfortable to my eyes
> than GNU FreeSans (for proportional/variable-pitch) or Source Code Pro
> medium (for monospace).

Can't you customize the variable-pitch face to specify the FreeSans
family, or even to specify the exact font you want Emacs to use?  That
should allow you to override any built-in heuristics that Emacs uses
when it has no other restrictions.

bug#57555: 29.0.50; variable-pitch font issue with medium weight default font

[Amin Bandali]

Eli Zaretskii writes:

>> From: Amin Bandali <bandali@gnu.org>
>> Cc: 57555@debbugs.gnu.org
>> Date: Mon, 05 Sep 2022 03:06:48 -0400
>> 
>> Eli Zaretskii writes:
>> 
>> [...]
>> >
>> > Thanks, but please show the font setup you are using; without that, the
>> > "instead of the correct sans-serif variable-pitch font" part is
>> > impossible to interpret.
>> 
>> Thanks for your reply, Eli.  Globally, I'm basically using the default
>> font setup that Trisquel GNU/Linux comes with.  Here is a tarball of
>> the /etc/fonts/ directory on my system:
>
> Thanks, but I meant the font setup specific to Emacs, i.e.:
>
>   . any set-fontest-font calls in your init files
>   . any set-frame-font calls or any other face-related calls in your
>     init files that could affect the font selection
>
> Sorry for my imprecise wording.

Ah, right.  No problem. :-)  Other than setting the Emacs font from
Xresources (though I also did try with set-frame-font per my initial
email, with the same results), this is pretty much all I have in my
init file that's related to fonts:

(when (display-graphic-p)
  (set-fontset-font t 'arabic "Vazir"))

And further down:

(when (display-graphic-p)
  (with-eval-after-load 'faces
    (let ((grey "#e7e7e7"))
      (set-face-attribute 'fixed-pitch nil :family "Source Code Pro")
      (set-face-attribute 'mode-line nil
                          :background grey
                          :inherit 'fixed-pitch))))

[...]
>
> I think this means that Emacs gives greater importance to consistency
> in weight than to consistency of the family.  Not sure if that is TRT.

Hmm, right.  That may indeed not be TRT: at least in this case (and
with various other fonts I've tried/used in the past), their weights
don't necessarily match.  For instance, I find Source Code Pro's
regular weight way too thin, and I prefer to use its medium weight
wherever possible.  It's more readable and much more closely matches
the regular weight of GNU FreeSans and most other sans fonts on my
machine.

>> Right.  For what it's worth, I find Emacs picking misc-fixed for
>> variable-pitch text more than just a minor annoyance: for long hours
>> of use, I find misc-fixed it considerably less comfortable to my eyes
>> than GNU FreeSans (for proportional/variable-pitch) or Source Code Pro
>> medium (for monospace).
>
> Can't you customize the variable-pitch face to specify the FreeSans
> family, or even to specify the exact font you want Emacs to use?  That
> should allow you to override any built-in heuristics that Emacs uses
> when it has no other restrictions.
>

This indeed doesn't seem to work anymore.  With the following,

    (set-face-attribute 'variable-pitch nil :family "FreeSans")

Emacs picks/uses Source Code Pro medium for variable-pitch (confirmed
visually and with describe-char), even though describe-face for
variable-pitch shows the family set as "FreeSans".  This used to work
before, but not anymore.

bug#57555: 29.0.50; variable-pitch font issue with medium weight default font

[Eli Zaretskii]

> From: Amin Bandali <bandali@gnu.org>
> Cc: 57555@debbugs.gnu.org
> Date: Mon, 05 Sep 2022 11:08:10 -0400
> 
> (when (display-graphic-p)
>   (set-fontset-font t 'arabic "Vazir"))
> 
> And further down:
> 
> (when (display-graphic-p)
>   (with-eval-after-load 'faces
>     (let ((grey "#e7e7e7"))
>       (set-face-attribute 'fixed-pitch nil :family "Source Code Pro")
>       (set-face-attribute 'mode-line nil
>                           :background grey
>                           :inherit 'fixed-pitch))))

Does it change anything if you remove the customization of fixed-pitch
face?

> > Can't you customize the variable-pitch face to specify the FreeSans
> > family, or even to specify the exact font you want Emacs to use?  That
> > should allow you to override any built-in heuristics that Emacs uses
> > when it has no other restrictions.
> >
> 
> This indeed doesn't seem to work anymore.  With the following,
> 
>     (set-face-attribute 'variable-pitch nil :family "FreeSans")
> 
> Emacs picks/uses Source Code Pro medium for variable-pitch (confirmed
> visually and with describe-char), even though describe-face for
> variable-pitch shows the family set as "FreeSans".  This used to work
> before, but not anymore.

Even if you set the face's font explicitly by its full XLFD name, not
implicitly via :family?

bug#57555: 29.0.50; variable-pitch font issue with medium weight default font

[Amin Bandali]

Eli Zaretskii writes:

>> From: Amin Bandali <bandali@gnu.org>
>> Cc: 57555@debbugs.gnu.org
>> Date: Mon, 05 Sep 2022 11:08:10 -0400
>> 
>> (when (display-graphic-p)
>>   (set-fontset-font t 'arabic "Vazir"))
>> 
>> And further down:
>> 
>> (when (display-graphic-p)
>>   (with-eval-after-load 'faces
>>     (let ((grey "#e7e7e7"))
>>       (set-face-attribute 'fixed-pitch nil :family "Source Code Pro")
>>       (set-face-attribute 'mode-line nil
>>                           :background grey
>>                           :inherit 'fixed-pitch))))
>
> Does it change anything if you remove the customization of fixed-pitch
> face?

Removing that did not change anything.

[...]
>
> Even if you set the face's font explicitly by its full XLFD name, not
> implicitly via :family?
>

Ha, so I just tried with the following XLFD:

    (set-face-font 'variable-pitch
                   "-GNU -FreeSans-*-*-*-*-14-*-*-*-*-0-iso10646-1")

And it works, somewhat.  Emacs now uses GNU FreeSans, but seemingly
only its regular variant.  Based on the '*' I specified for the third
and fourth fields (WEIGHT_NAME and SLANT), I would expect it to pick
GNU FreeSans's other variants (bold, oblique, or bold oblique) where
needed, but it doesn't seem to.

Here's a screenshot from Gnus, using the above set-face-font call to
set the above XLFD:

https://kelar.org/~bandali/tmp/20220903/gnus-after-xlfd.png

And here's a screenshot from Gnus without that call to set-face-font,
without the 6b1ed2f2c99a1c2da56c5f434570c438cad6576d emacs.git commit
that introduced this change:

https://kelar.org/~bandali/tmp/20220903/gnus-before.png

Notice how Emacs used to pick the bold and bold oblique variants for
those headers (gnus-before.png), but doesn't do so anymore
(gnus-after-xlfd.png).

bug#57555: 29.0.50; variable-pitch font issue with medium weight default font

[Eli Zaretskii]

> From: Amin Bandali <bandali@gnu.org>
> Cc: 57555@debbugs.gnu.org
> Date: Mon, 05 Sep 2022 14:19:14 -0400
> 
> > Even if you set the face's font explicitly by its full XLFD name, not
> > implicitly via :family?
> >
> 
> Ha, so I just tried with the following XLFD:
> 
>     (set-face-font 'variable-pitch
>                    "-GNU -FreeSans-*-*-*-*-14-*-*-*-*-0-iso10646-1")
> 
> And it works, somewhat.  Emacs now uses GNU FreeSans, but seemingly
> only its regular variant.  Based on the '*' I specified for the third
> and fourth fields (WEIGHT_NAME and SLANT), I would expect it to pick
> GNU FreeSans's other variants (bold, oblique, or bold oblique) where
> needed, but it doesn't seem to.

No, it can't: you specified a particular font.  The requirement
regarding the slant variant wasn't in your original complaint ;-)

Anyway, feel free to step through font_load_for_lface and its
subroutines, to figure out why your FreeSans font is rejected for
variable-pitch face.  Maybe you will find something that could be
improved there.

bug#57555: 29.0.50; variable-pitch font issue with medium weight default font

[Gregory Heytings]

Amin, can you try the following patch, and tell us if it fixes your 
problem?

diff --git a/src/xfaces.c b/src/xfaces.c
index 5e3a47d7f8..08f37cf0b2 100644
--- a/src/xfaces.c
+++ b/src/xfaces.c
@@ -6058,7 +6058,7 @@ realize_gui_face (struct face_cache *cache, 
Lisp_Object attrs[LFACE_VECTOR_SIZE]
         }
        if (! FONT_OBJECT_P (attrs[LFACE_FONT_INDEX]))
         attrs[LFACE_FONT_INDEX]
-         = font_load_for_lface (f, attrs, attrs[LFACE_FONT_INDEX]);
+         = font_load_for_lface (f, attrs, Ffont_spec (0, NULL));
        if (FONT_OBJECT_P (attrs[LFACE_FONT_INDEX]))
         {
           face->font = XFONT_OBJECT (attrs[LFACE_FONT_INDEX]);

bug#57555: 29.0.50; variable-pitch font issue with medium weight default font

[Eli Zaretskii]

> Date: Mon, 05 Sep 2022 21:17:22 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: Amin Bandali <bandali@gnu.org>, 57555@debbugs.gnu.org
> 
> 
> Amin, can you try the following patch, and tell us if it fixes your 
> problem?

Why is this TRT, even if it does solve this particular problem?
Forgetting the other attributes is problematic, and would quite
certainly lead to much more serious problems.

Commit 6b1ed2f2c99a1c2da56c5f434570c438cad6576d, which allegedly
caused this problem, made the inverse change, for those reasons.

bug#57555: 29.0.50; variable-pitch font issue with medium weight default font

[Amin Bandali]

Gregory Heytings writes:

> Amin, can you try the following patch, and tell us if it fixes your
> problem?
>
> diff --git a/src/xfaces.c b/src/xfaces.c
> index 5e3a47d7f8..08f37cf0b2 100644
> --- a/src/xfaces.c
> +++ b/src/xfaces.c
> @@ -6058,7 +6058,7 @@ realize_gui_face (struct face_cache *cache,
> Lisp_Object attrs[LFACE_VECTOR_SIZE]
>         }
>        if (! FONT_OBJECT_P (attrs[LFACE_FONT_INDEX]))
>         attrs[LFACE_FONT_INDEX]
> -         = font_load_for_lface (f, attrs, attrs[LFACE_FONT_INDEX]);
> +         = font_load_for_lface (f, attrs, Ffont_spec (0, NULL));
>        if (FONT_OBJECT_P (attrs[LFACE_FONT_INDEX]))
>         {
>           face->font = XFONT_OBJECT (attrs[LFACE_FONT_INDEX]);
>

Thank you both.  Gregory, your proposed patch indeed seems to help fix
this and restore the expected behaviour for my use-case as far as
I can tell; thanks!  Would you mind elaborating on how you suspected
this might be a potential fix?

Thanks again,
-a

bug#57555: 29.0.50; variable-pitch font issue with medium weight default font

[Gregory Heytings]

>> Amin, can you try the following patch, and tell us if it fixes your 
>> problem?
>
> Why is this TRT, even if it does solve this particular problem?
>

I don't claim it's TRT, I was exploring that bug and wanted to 
double-check with Amin that I'm not on a wrong path.

>
> Forgetting the other attributes is problematic, and would quite 
> certainly lead to much more serious problems.
>

I don't know.  That's what that code had been doing during the last eight 
years or so, so it can't be completely wrong.

bug#57555: 29.0.50; variable-pitch font issue with medium weight default font

[Eli Zaretskii]

> Date: Tue, 06 Sep 2022 08:08:22 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: 57555@debbugs.gnu.org, bandali@gnu.org
> 
> >> Amin, can you try the following patch, and tell us if it fixes your 
> >> problem?
> >
> > Why is this TRT, even if it does solve this particular problem?
> 
> I don't claim it's TRT, I was exploring that bug and wanted to 
> double-check with Amin that I'm not on a wrong path.

Well, you reverted the important part of commit 6b1ed2f, which Amin
said caused his problem.  So you are not on a wrong path wrt that...

> > Forgetting the other attributes is problematic, and would quite 
> > certainly lead to much more serious problems.
> 
> I don't know.  That's what that code had been doing during the last eight 
> years or so, so it can't be completely wrong.

It also caused bug#37473, which IMO is much more serious than what
Amin describes, and which I attempted to fix in commit 6b1ed2f with
exactly the part you now suggest to remove.

Did you read the discussion of bug#37473?  We've been through the
issue of whether ignoring some of the font attributes is a good
strategy; my conclusion was that it wasn't, and thus the change I made
in 6b1ed2f simply went back to the code we had for many years before
commit bf0d3f7, which did what you are suggesting to do now, and
caused bug#37473.

If, after reading those discussions, and studying the changes we did
in that area of the code, you still think the change you propose is
the right way, please elaborate why you think so, given all the
negative feedback we have for it.

Thanks.

bug#57555: 29.0.50; variable-pitch font issue with medium weight default font

[Gregory Heytings]

>>> Why is this TRT, even if it does solve this particular problem?
>>
>> I don't claim it's TRT, I was exploring that bug and wanted to 
>> double-check with Amin that I'm not on a wrong path.
>
> Well, you reverted the important part of commit 6b1ed2f, which Amin said 
> caused his problem.  So you are not on a wrong path wrt that...
>

???  I didn't revert anything, I only asked Amin to try a patch.

>>> Forgetting the other attributes is problematic, and would quite 
>>> certainly lead to much more serious problems.
>>
>> I don't know.  That's what that code had been doing during the last 
>> eight years or so, so it can't be completely wrong.
>
> It also caused bug#37473, which IMO is much more serious than what Amin 
> describes, and which I attempted to fix in commit 6b1ed2f with exactly 
> the part you now suggest to remove.
>

I don't suggest anything or want to remove anything, sorry if that was 
unclear in my previous post.  FWIW, I don't think I agree that bug#37473 
is much more serious than what Amin describes, but I'm not yet sure about 
that either.

bug#57555: 29.0.50; variable-pitch font issue with medium weight default font

[Eli Zaretskii]

> Date: Tue, 06 Sep 2022 13:09:46 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: 57555@debbugs.gnu.org, bandali@gnu.org
> 
> FWIW, I don't think I agree that bug#37473 is much more serious than
> what Amin describes, but I'm not yet sure about that either.

Losing the antialias attribute of the font? it's serious.

bug#57555: 29.0.50; variable-pitch font issue with medium weight default font

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> Well, you reverted the important part of commit 6b1ed2f, which Amin
> said caused his problem.  So you are not on a wrong path wrt that...

As far as I can tell, your patch just reverted the code back to the
state before this:

commit bf0d3f76dcfe7881cb3058169b51cf6602fdcdcb
Author:     Kenichi Handa <handa@gnu.org>
AuthorDate: Sun Jul 20 00:18:23 2014 +0900

    2014-07-19  Kenichi Handa  <handa@gnu.org>
    
            * xfaces.c (realize_x_face): Call font_load_for_lface with no
            mandatory font spec (Bug#17973).
    
    2014-07-19  Stefan Monnier  <monnier@iro.umontreal.ca>
    
            * font.c (font_score): Return the worst score if the size of
            ENTITY is wrong by more than a factor 2 (Bug#17973).

So we've come full circle.

bug#57555: 29.0.50; variable-pitch font issue with medium weight default font

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: Gregory Heytings <gregory@heytings.org>,  bandali@gnu.org,
>   57555@debbugs.gnu.org
> Date: Tue, 06 Sep 2022 16:28:21 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Well, you reverted the important part of commit 6b1ed2f, which Amin
> > said caused his problem.  So you are not on a wrong path wrt that...
> 
> As far as I can tell, your patch just reverted the code back to the
> state before this:
> 
> commit bf0d3f76dcfe7881cb3058169b51cf6602fdcdcb
> Author:     Kenichi Handa <handa@gnu.org>
> AuthorDate: Sun Jul 20 00:18:23 2014 +0900
> 
>     2014-07-19  Kenichi Handa  <handa@gnu.org>
>     
>             * xfaces.c (realize_x_face): Call font_load_for_lface with no
>             mandatory font spec (Bug#17973).
>     
>     2014-07-19  Stefan Monnier  <monnier@iro.umontreal.ca>
>     
>             * font.c (font_score): Return the worst score if the size of
>             ENTITY is wrong by more than a factor 2 (Bug#17973).

Yes.  As I explained back then.

> So we've come full circle.

No, we haven't.  Emacs works as expected: it requests a medium-weight
font for the variable-pitch face, because the fixed-pitch face was
customized to use a medium weight.  Since the font Amin wants to use
for variable-pitch doesn't have a medium variant, Emacs selects
another one.  There's no real problem I see here, except an issue with
user expectations not being satisfied.

"Fixing" this by disregarding all the attributes of the face's font is
throwing the baby with bathwater.

bug#57555: 29.0.50; variable-pitch font issue with medium weight default font

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

>> As far as I can tell, your patch just reverted the code back to the
>> state before this:
>> 
>> commit bf0d3f76dcfe7881cb3058169b51cf6602fdcdcb
>> Author:     Kenichi Handa <handa@gnu.org>
>> AuthorDate: Sun Jul 20 00:18:23 2014 +0900
>> 
>>     2014-07-19  Kenichi Handa  <handa@gnu.org>
>>     
>>             * xfaces.c (realize_x_face): Call font_load_for_lface with no
>>             mandatory font spec (Bug#17973).
>>     
>>     2014-07-19  Stefan Monnier  <monnier@iro.umontreal.ca>
>>     
>>             * font.c (font_score): Return the worst score if the size of
>>             ENTITY is wrong by more than a factor 2 (Bug#17973).
>
> Yes.  As I explained back then.
>
>> So we've come full circle.
>
> No, we haven't.

In what way isn't that "full circle"?

> Emacs works as expected: it requests a medium-weight
> font for the variable-pitch face, because the fixed-pitch face was
> customized to use a medium weight.  Since the font Amin wants to use
> for variable-pitch doesn't have a medium variant, Emacs selects
> another one.  There's no real problem I see here, except an issue with
> user expectations not being satisfied.

I think that the vast majority of users would want Emacs to choose a
different weight, but use the same font, which is what we had before we
went full circle.

bug#57555: 29.0.50; variable-pitch font issue with medium weight default font

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: gregory@heytings.org,  bandali@gnu.org,  57555@debbugs.gnu.org
> Date: Wed, 07 Sep 2022 14:44:01 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> So we've come full circle.
> >
> > No, we haven't.
> 
> In what way isn't that "full circle"?

In the way I explained below:

> > Emacs works as expected: it requests a medium-weight
> > font for the variable-pitch face, because the fixed-pitch face was
> > customized to use a medium weight.  Since the font Amin wants to use
> > for variable-pitch doesn't have a medium variant, Emacs selects
> > another one.  There's no real problem I see here, except an issue with
> > user expectations not being satisfied.
> 
> I think that the vast majority of users would want Emacs to choose a
> different weight, but use the same font, which is what we had before we
> went full circle.

So maybe we should consider resetting just the weight attribute,
perhaps only if its value is 'medium' (assuming that solves this
particular issue).  That's a far cry from resetting all of them.

bug#57555: 29.0.50; variable-pitch font issue with medium weight default font

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> So maybe we should consider resetting just the weight attribute,
> perhaps only if its value is 'medium' (assuming that solves this
> particular issue).  That's a far cry from resetting all of them.

Yes -- what we had before the last change was "reset all except a
couple", and I suspect we should just keep going that way instead.  And
include the weight in that list.

But I'm not at all sure about anything in this area.

bug#57555: 29.0.50; variable-pitch font issue with medium weight default font

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: gregory@heytings.org,  bandali@gnu.org,  57555@debbugs.gnu.org
> Date: Thu, 08 Sep 2022 13:42:10 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > So maybe we should consider resetting just the weight attribute,
> > perhaps only if its value is 'medium' (assuming that solves this
> > particular issue).  That's a far cry from resetting all of them.
> 
> Yes -- what we had before the last change was "reset all except a
> couple", and I suspect we should just keep going that way instead.  And
> include the weight in that list.
> 
> But I'm not at all sure about anything in this area.

"All except a couple" is too many, and the couple that was exempted
was rather arbitrary, as the discussion has shown.  So I'd rather we
added attributes to the list of those we reset one by one, and only
after careful consideration.

Right now, I'm not even sure resetting the weight if it's value is
'medium' will solve Amin's problem, because Amin didn't respond.  Once
it says his problem is solved by that, I think we should reset only
the 'medium' weight, and see whether this causes problems elsewhere.

bug#57555: 29.0.50; variable-pitch font issue with medium weight default font

[Gregory Heytings]

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


Amin, could you please try the attached patch and tell us if it fixes your 
problem?  (It does here, with your recipe.)

Thanks.

[-- Attachment #2: Also-try-normal-weight-when-searching-a-font-with-me.patch --]
[-- Type: text/x-diff, Size: 4135 bytes --]

From ab7090e055b7c2043f9fdb07b760ae8b304fe02c Mon Sep 17 00:00:00 2001
From: Gregory Heytings <gregory@heytings.org>
Date: Sun, 20 Nov 2022 13:50:47 +0000
Subject: [PATCH] Also try 'normal' weight when searching a font with 'medium'
 weight.

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.
In the meantime, commit 65fd3ca84f added support for the 'medium' font
weight, which was previously synonymous to 'normal'.

Together, the two commits 6b1ed2f2c9 and 65fd3ca84f lead to suboptimal
font choices.  When the font chosen for the default face has its
weight set to 'medium' and actually supports that weight,
font_load_for_lface will be called with a weight attribute set to
'medium' in spec for other faces.  However, fonts with an explicit
'medium' weight are much less common than fonts with an explicit
'normal' weight, which means that fonts that only support a 'normal'
weight are rejected, although they are close to the desired font.

Therefore, font_find_for_lface should also try the 'normal' weight
when the weight in spec is 'medium', after trying the 'medium' weight.

* src/font.c (font_find_for_lface): When the weight in SPEC is
'medium', also try the 'normal' weight.
---
 src/font.c | 39 +++++++++++++++++++++++++++------------
 1 file changed, 27 insertions(+), 12 deletions(-)

diff --git a/src/font.c b/src/font.c
index 6e720bc285..4222d60231 100644
--- a/src/font.c
+++ b/src/font.c
@@ -2959,9 +2959,9 @@ font_find_for_lface (struct frame *f, Lisp_Object *attrs, Lisp_Object spec, int
 {
   Lisp_Object work;
   Lisp_Object entities, val;
-  Lisp_Object foundry[3], *family, registry[3], adstyle[3];
+  Lisp_Object foundry[3], *family, registry[3], adstyle[3], weight[3];
   int pixel_size;
-  int i, j, k, l;
+  int i, j, k, l, m;
   USE_SAFE_ALLOCA;
 
   /* Registry specification alternatives: from the most specific to
@@ -3081,6 +3081,17 @@ font_find_for_lface (struct frame *f, Lisp_Object *attrs, Lisp_Object spec, int
 	}
     }
 
+  /* If weight is "medium" in SPEC, also try "normal".  Fonts with an
+     explicit "medium" weight are much less common than fonts with an
+     explicit "normal" weight, and for a long time "medium" and
+     "normal" (a.k.a. "regular" a.k.a. "book") were synonymous in
+     Emacs.  See e.g. bug#59347 and bug#57555.  */
+  weight[0] = AREF (spec, FONT_WEIGHT_INDEX);
+  if (EQ (weight[0], Qmedium))
+    weight[1] = Qnormal, weight[2] = zero_vector;
+  else
+    weight[1] = zero_vector;
+
   /* Now look up suitable fonts, from the most specific spec to the
      least specific spec.  Accept the first one that matches.  */
   for (i = 0; SYMBOLP (family[i]); i++)
@@ -3095,18 +3106,22 @@ font_find_for_lface (struct frame *f, Lisp_Object *attrs, Lisp_Object spec, int
 	      for (l = 0; SYMBOLP (adstyle[l]); l++)
 		{
 		  ASET (work, FONT_ADSTYLE_INDEX, adstyle[l]);
-		  /* Produce the list of candidates for the spec in WORK.  */
-		  entities = font_list_entities (f, work);
-		  if (! NILP (entities))
+		  for (m = 0; SYMBOLP (weight[m]); m++)
 		    {
-		      /* If there are several candidates, select the
-			 best match for PIXEL_SIZE and attributes in ATTRS.  */
-		      val = font_select_entity (f, entities,
-						attrs, pixel_size, c);
-		      if (! NILP (val))
+		      ASET (work, FONT_WEIGHT_INDEX, weight[m]);
+		      /* Produce the list of candidates for the spec in WORK.  */
+		      entities = font_list_entities (f, work);
+		      if (! NILP (entities))
 			{
-			  SAFE_FREE ();
-			  return val;
+			  /* If there are several candidates, select the
+			     best match for PIXEL_SIZE and attributes in ATTRS.  */
+			  val = font_select_entity (f, entities,
+						    attrs, pixel_size, c);
+			  if (! NILP (val))
+			    {
+			      SAFE_FREE ();
+			      return val;
+			    }
 			}
 		    }
 		}
-- 
2.35.1

bug#57555: 29.0.50; variable-pitch font issue with medium weight default font

[Amin Bandali]

Hi Gregory, (all,)

Gregory Heytings writes:

> Amin, could you please try the attached patch and tell us if it fixes
> your problem?  (It does here, with your recipe.)
>
> Thanks.
>
>

Sorry for my unintended long absence -- I meant to get back to this
bug report much earlier -- and thanks very much for the patch.

Your patch nicely remedies the original issue that I'd reported, but
with it applied, at least the following doesn't seem to have an effect
anymore:

    (set-face-attribute 'fixed-pitch nil :family "Source Code Pro")

I use that in conjunction with the following to have my mode-line
typeset in Source Code Pro:

    (set-face-attribute 'mode-line nil
                        :background "#e7e7e7"
                        :inherit 'fixed-pitch)

With your patch, my mode-line (and fixed-pitch itself) are typeset in
my system's sans-serif font, aka GNU FreeSans (even though
describe-face reports "Family: Source Code Pro" for fixed-pitch).

Sorry if this is not a lot to go by; I'll see if I can set aside some
time in the coming days to have a closer look at the underlying code.

bug#57555: 29.0.50; variable-pitch font issue with medium weight default font

[Gregory Heytings]

>
> Sorry for my unintended long absence -- I meant to get back to this bug 
> report much earlier -- and thanks very much for the patch.
>

No worries, the patch was sent only a few hours ago!

>
> Your patch nicely remedies the original issue that I'd reported,
>

Great!

>
> but with it applied, at least the following doesn't seem to have an 
> effect anymore:
>
> (set-face-attribute 'fixed-pitch nil :family "Source Code Pro")
>

Are you sure :family is the right attribute here?  Using :font instead of 
:family seems to do TRT.  Can you please try that and see if it does what 
you expect?

bug#57555: 29.0.50; variable-pitch font issue with medium weight default font

[Amin Bandali]

Gregory Heytings writes:

>>
>> Sorry for my unintended long absence -- I meant to get back to this
>> bug report much earlier -- and thanks very much for the patch.
>>
>
> No worries, the patch was sent only a few hours ago!

Thanks.  I meant more in terms of my earlier absence and not getting
around to debug and try putting together a patch for it myself. :-)

[...]
>
> Are you sure :family is the right attribute here?  Using :font instead
> of :family seems to do TRT.  Can you please try that and see if it
> does what you expect?
>

Hmm, not sure indeed: I always used :family and it seemed to work fine
so I never really questioned it.  I had a quick look at the relevant
parts of the manual(s) and I think their difference is a bit clearer
for me now.  That said, you're quite right, the following works nicely
with your patch, thanks!

    (set-face-attribute 'fixed-pitch nil
                        :font "Source Code Pro"
                        :weight 'medium)

bug#57555: 29.0.50; variable-pitch font issue with medium weight default font

[Gregory Heytings]

>> Are you sure :family is the right attribute here?  Using :font instead 
>> of :family seems to do TRT.  Can you please try that and see if it does 
>> what you expect?
>
> Hmm, not sure indeed: I always used :family and it seemed to work fine 
> so I never really questioned it.  I had a quick look at the relevant 
> parts of the manual(s) and I think their difference is a bit clearer for 
> me now.  That said, you're quite right, the following works nicely with 
> your patch, thanks!
>

Great, thanks!  I'll close this bug after applying my patch (which might 
take a few days, it's being discussed in bug#59347).

bug#57555: 29.0.50; variable-pitch font issue with medium weight default font

[Gregory Heytings]

>
> I'll close this bug after applying my patch (which might take a few 
> days, it's being discussed in bug#59347).
>

The discussion was a bit longer than anticipated, but a variant of the 
patch has now been installed (30e3cb2135).

Closing.