bug#37473: 27.0.50; antialias setting is not preserved by inheriting

bug#37473: 27.0.50; antialias setting is not preserved by inheriting

[YAMAMOTO Mitsuharu]

Steps to Reproduce:

1. % emacs -Q -fn monospace:antialias=0 &

   As expected, the normal text in the *scratch* buffer is not
   antialiased.  But the bold text in the mode line is unexpectedly
   antialiased.

2. C-x C-+

   Observe that the scaled text in the *scratch* buffer is
   unexpectedly antialiased.

I found that it used to work (i.e., antialias setting was preserved in
bold or scaled text) before the following change:

commit bf0d3f76dcfe7881cb3058169b51cf6602fdcdcb (HEAD)
Author: Kenichi Handa <handa@gnu.org>
Date:   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).

				     YAMAMOTO Mitsuharu
				mituharu@math.s.chiba-u.ac.jp

In GNU Emacs 27.0.50 (build 1, x86_64-apple-darwin16.7.0, X toolkit, Xaw3d scroll bars)
 of 2019-09-21 built on YAMAMOTO-no-iMac-5K.local
Repository revision: 87b7c069583ddccae89791b2389bd872a49bc1b0
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.11804000
System Description:  Mac OS X 10.12.6

Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.

Configured using:
 'configure --without-ns --with-jpeg=no --with-gif=no --with-tiff=no
 --with-gnutls=no PKG_CONFIG=/opt/local/bin/pkg-config
 PKG_CONFIG_PATH=/usr/lib/pkgconfig:/opt/X11/lib/pkgconfig:/usr/local/lib/pkgconfig
 PKG_CONFIG_LIBDIR= CFLAGS=-g'

Configured features:
XAW3D XPM PNG NOTIFY KQUEUE ACL LIBXML2 FREETYPE M17N_FLT LIBOTF XFT
ZLIB TOOLKIT_SCROLL_BARS LUCID X11 XDBE XIM THREADS PDUMPER

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

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message rmc puny dired dired-loaddefs
format-spec rfc822 mml easymenu mml-sec password-cache epa derived epg
epg-config gnus-util rmail rmail-loaddefs text-property-search time-date
subr-x seq byte-opt gv bytecomp byte-compile cconv mm-decode mm-bodies
mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader cl-loaddefs
cl-lib sendmail rfc2047 rfc2045 ietf-drums mm-util mail-prsvr mail-utils
japan-util tooltip eldoc electric uniquify ediff-hook vc-hooks
lisp-float-type 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 elisp-mode lisp-mode prog-mode register page menu-bar
rfn-eshadow isearch timer select scroll-bar mouse jit-lock font-lock
syntax facemenu font-core term/tty-colors frame cl-generic 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 charscript charprop
case-table epa-hook jka-cmpr-hook help simple abbrev obarray minibuffer
cl-preloaded nadvice loaddefs button faces cus-face macroexp files
text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget hashtable-print-readable backquote threads kqueue
dynamic-setting font-render-setting x-toolkit x multi-tty
make-network-process emacs)

Memory information:
((conses 16 46279 5949)
 (symbols 48 5916 1)
 (strings 32 16004 1573)
 (string-bytes 1 507230)
 (vectors 16 9815)
 (vector-slots 8 200586 9608)
 (floats 8 19 39)
 (intervals 56 216 0)
 (buffers 992 11))

bug#37473: 27.0.50; antialias setting is not preserved by inheriting

[Lars Ingebrigtsen]

YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp> writes:

> 1. % emacs -Q -fn monospace:antialias=0 &
>
>    As expected, the normal text in the *scratch* buffer is not
>    antialiased.  But the bold text in the mode line is unexpectedly
>    antialiased.
>
> 2. C-x C-+
>
>    Observe that the scaled text in the *scratch* buffer is
>    unexpectedly antialiased.

I can reproduce this in Emacs 29, too, and

diff --git a/src/xfaces.c b/src/xfaces.c
index 7395ce157e..1939a73d5e 100644
--- a/src/xfaces.c
+++ b/src/xfaces.c
@@ -5950,7 +5950,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, Ffont_spec (0, NULL));
+	  = font_load_for_lface (f, attrs, attrs[LFACE_FONT_INDEX]);
       if (FONT_OBJECT_P (attrs[LFACE_FONT_INDEX]))
 	{
 	  face->font = XFONT_OBJECT (attrs[LFACE_FONT_INDEX]);

Fixes the issue.  But that would bring bug#17973 back, I think.

What we want to happen here is, I think, that we want the "initial" font
spec (which would here just be family monospace + antialias, and not the
entire font spec that we initially realised?  I think?

Is it immediately obvious to somebody how to achieve that here?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#37473: 27.0.50; antialias setting is not preserved by inheriting

[Lars Ingebrigtsen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> What we want to happen here is, I think, that we want the "initial" font
> spec (which would here just be family monospace + antialias, and not the
> entire font spec that we initially realised?  I think?
>
> Is it immediately obvious to somebody how to achieve that here?

I've now fixed this in a rather cumbersome way in Emacs 29, but if
anybody has a better idea here, please go ahead and re-fix.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#37473: 27.0.50; antialias setting is not preserved by, inheriting

[David Ponce]

Hello,

The below related today's commit to xface.c breaks antialiasing:

 From b2d11d69dd49864874f8fe53669b4049e83bfce9 Mon Sep 17 00:00:00 2001
From: Po Lu <luangruo@yahoo.com>
Date: Sun, 19 Jun 2022 21:57:11 +0800
Subject: More conservative fix for bug#37473

* src/xfaces.c (realize_gui_face): Add more conservative fix,
since the last change makes C-x C-+ lead to weight weirdness on
my machine.
---
  src/xfaces.c | 23 ++++++++++++-----------
  1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/src/xfaces.c b/src/xfaces.c
index 4242205..25b5e4d 100644
--- a/src/xfaces.c
+++ b/src/xfaces.c
@@ -5909,7 +5909,7 @@ realize_gui_face (struct face_cache *cache, 
Lisp_Object attrs[LFACE_VECTOR_SIZE]
  #ifdef HAVE_WINDOW_SYSTEM
    struct face *default_face;
    struct frame *f;
-  Lisp_Object stipple, underline, overline, strike_through, box;
+  Lisp_Object stipple, underline, overline, strike_through, box, temp_spec;

    eassert (FRAME_WINDOW_P (cache->f));

@@ -5953,17 +5953,18 @@ realize_gui_face (struct face_cache *cache, 
Lisp_Object attrs[LFACE_VECTOR_SIZE]
        if (! FONT_OBJECT_P (attrs[LFACE_FONT_INDEX]))
  	{
  	  /* We want attrs to allow overriding most elements in the
-	     spec, but we don't want to start with an all-nil font,
-	     either, because then we lose attributes like
-	     antialiasing.  This should probably be fixed in a
-	     different way, see bug#17973 and bug#37473.  */
-	  Lisp_Object spec = copy_font_spec (attrs[LFACE_FONT_INDEX]);
-	  Ffont_put (spec, QCfoundry, Qnil);
-	  Ffont_put (spec, QCfamily, Qnil);
-	  Ffont_put (spec, QCregistry, Qnil);
-	  Ffont_put (spec, QCadstyle, Qnil);
+	     spec (IOW, to start out as an empty font spec), but
+	     preserve the antialiasing attribute.  (bug#17973,
+	     bug#37473).  */
+	  temp_spec = Ffont_spec (0, NULL);
+
+	  if (FONTP (attrs[LFACE_FONT_INDEX]))
+	    Ffont_put (temp_spec, QCantialias,
+		       Ffont_get (attrs[LFACE_FONT_INDEX],
+				  QCantialias));
+
  	  attrs[LFACE_FONT_INDEX]
-	    = font_load_for_lface (f, attrs, spec);
+	    = font_load_for_lface (f, attrs, temp_spec);
  	}
        if (FONT_OBJECT_P (attrs[LFACE_FONT_INDEX]))
  	{
-- 
cgit v1.1


The below marked line seems not correct. According to the previous 
condition "if (! FONT_OBJECT_P (attrs[LFACE_FONT_INDEX]))", it should 
always be false, so the antialising attibute is never preserved:


	  if (FONTP (attrs[LFACE_FONT_INDEX])) <=====================
	    Ffont_put (temp_spec, QCantialias,
		       Ffont_get (attrs[LFACE_FONT_INDEX],
				  QCantialias));

Thanks,
David Ponce

bug#37473: 27.0.50; antialias setting is not preserved by, inheriting

[Colin Baxter]

>>>>> David Ponce <da_vid@orange.fr> writes:

    > Hello, The below related today's commit to xface.c breaks
    > antialiasing:

    >  From b2d11d69dd49864874f8fe53669b4049e83bfce9 Mon Sep 17 00:00:00
    > 2001 From: Po Lu <luangruo@yahoo.com> Date: Sun, 19 Jun 2022
    > 21:57:11 +0800 Subject: More conservative fix for bug#37473

    > * src/xfaces.c (realize_gui_face): Add more conservative fix,
    > since the last change makes C-x C-+ lead to weight weirdness on my
    > machine.  --- src/xfaces.c | 23 ++++++++++++----------- 1 file
    > changed, 12 insertions(+), 11 deletions(-)

    > diff --git a/src/xfaces.c b/src/xfaces.c index 4242205..25b5e4d
    > 100644 --- a/src/xfaces.c +++ b/src/xfaces.c @@ -5909,7 +5909,7 @@
    > realize_gui_face (struct face_cache *cache, Lisp_Object
    > attrs[LFACE_VECTOR_SIZE] #ifdef HAVE_WINDOW_SYSTEM struct face
    > *default_face; struct frame *f; - Lisp_Object stipple, underline,
    > overline, strike_through, box; + Lisp_Object stipple, underline,
    > overline, strike_through, box, temp_spec;

    >     eassert (FRAME_WINDOW_P (cache->f));

    > @@ -5953,17 +5953,18 @@ realize_gui_face (struct face_cache
    > *cache, Lisp_Object attrs[LFACE_VECTOR_SIZE] if (! FONT_OBJECT_P
    > (attrs[LFACE_FONT_INDEX])) { /* We want attrs to allow overriding
    > most elements in the - spec, but we don't want to start with an
    > all-nil font, - either, because then we lose attributes like -
    > antialiasing.  This should probably be fixed in a - different way,
    > see bug#17973 and bug#37473.  */ - Lisp_Object spec =
    > copy_font_spec (attrs[LFACE_FONT_INDEX]); - Ffont_put (spec,
    > QCfoundry, Qnil); - Ffont_put (spec, QCfamily, Qnil); - Ffont_put
    > (spec, QCregistry, Qnil); - Ffont_put (spec, QCadstyle, Qnil); +
    > spec (IOW, to start out as an empty font spec), but + preserve the
    > antialiasing attribute.  (bug#17973, + bug#37473).  */ + temp_spec
    > = Ffont_spec (0, NULL); + + if (FONTP (attrs[LFACE_FONT_INDEX])) +
    > Ffont_put (temp_spec, QCantialias, + Ffont_get
    > (attrs[LFACE_FONT_INDEX], + QCantialias)); +
    > attrs[LFACE_FONT_INDEX] - = font_load_for_lface (f, attrs, spec);
    > + = font_load_for_lface (f, attrs, temp_spec); } if (FONT_OBJECT_P
    > (attrs[LFACE_FONT_INDEX])) { --

    > cgit v1.1


    > The below marked line seems not correct. According to the previous
    > condition "if (! FONT_OBJECT_P (attrs[LFACE_FONT_INDEX]))", it
    > should always be false, so the antialising attibute is never
    > preserved:


    > 	  if (FONTP (attrs[LFACE_FONT_INDEX])) <=====================
    > Ffont_put (temp_spec, QCantialias, Ffont_get
    > (attrs[LFACE_FONT_INDEX], QCantialias));

    > Thanks, David Ponce


Yes, I see exactly the same breakage. I reported this to emacs-dev -
wrong site!

Best wishes,

Colin.

bug#37473: 27.0.50; antialias setting is not preserved by, inheriting

[Eli Zaretskii]

> Cc: Lars Ingebrigtsen <larsi@gnus.org>
> Date: Sun, 19 Jun 2022 18:43:33 +0200
> From: David Ponce <da_vid@orange.fr>
> 
> The below related today's commit to xface.c breaks antialiasing:
> 
>  From b2d11d69dd49864874f8fe53669b4049e83bfce9 Mon Sep 17 00:00:00 2001
> From: Po Lu <luangruo@yahoo.com>
> Date: Sun, 19 Jun 2022 21:57:11 +0800
> Subject: More conservative fix for bug#37473
> 
> * src/xfaces.c (realize_gui_face): Add more conservative fix,
> since the last change makes C-x C-+ lead to weight weirdness on
> my machine.

FWIW, I understand neither that commit nor the one it attempted to
fix.

First, why is :antialiasing being singled out? won't the same happen
for any attribute in FONT_EXTRA_INDEX, like :hinting, :hintstyle, and
whatnot?

And if indeed the issue is with FONT_EXTRA_INDEX, then I'd first ask
why the right value is not in attrs[] in the first place?  Because if
it is there, why not just use it directly, instead of via Ffont_put?

And in addition, why does it make sense to reset foundry, family,
registry, and adstyle of the font in attrs[]? don't we want the same
font, for example, when resizing the text?

IOW, do we really understand the reason for bug#37473?  Not the commit
which introduced it, but the reason why :antialiasing is being reset
and doesn't appear in attrs[] and in the realized face?  If so, could
someone please spell that out?

bug#37473: 27.0.50; antialias setting is not preserved by, inheriting

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> First, why is :antialiasing being singled out? won't the same happen
> for any attribute in FONT_EXTRA_INDEX, like :hinting, :hintstyle, and
> whatnot?

Yes, that's a bug -- there's all kinds of extra stuff, and :antialiasing
isn't special.

> And in addition, why does it make sense to reset foundry, family,
> registry, and adstyle of the font in attrs[]? don't we want the same
> font, for example, when resizing the text?
>
> IOW, do we really understand the reason for bug#37473?  Not the commit
> which introduced it, but the reason why :antialiasing is being reset
> and doesn't appear in attrs[] and in the realized face?  If so, could
> someone please spell that out?

bug#17973 was fixed by disregarding the font spec completely so that
font_load_for_lface would be free to recompute those bits:

> The patch below expresses the first part, but it looks like the second
> part doesn't exit: Emacs just doesn't find any font to use for the "thin
> space" of C-x SPC and indicates it to me with one of those big squares
> that say "0020", which is a lot more intrusive than the problem I'm
> trying to fix.

But that gets rid of all the data, which is what I attempted to fix.  I
think a better fix would just be to start with a blank spec and copy
:extra -- but I wasn't able to make that work.  That is, I needed to
copy more, but I wasn't able to determine what.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#37473: 27.0.50; antialias setting is not preserved by, inheriting

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

>> Cc: Lars Ingebrigtsen <larsi@gnus.org>
>> Date: Sun, 19 Jun 2022 18:43:33 +0200
>> From: David Ponce <da_vid@orange.fr>
>> 
>> The below related today's commit to xface.c breaks antialiasing:
>> 
>>  From b2d11d69dd49864874f8fe53669b4049e83bfce9 Mon Sep 17 00:00:00 2001
>> From: Po Lu <luangruo@yahoo.com>
>> Date: Sun, 19 Jun 2022 21:57:11 +0800
>> Subject: More conservative fix for bug#37473
>> 
>> * src/xfaces.c (realize_gui_face): Add more conservative fix,
>> since the last change makes C-x C-+ lead to weight weirdness on
>> my machine.
>
> FWIW, I understand neither that commit nor the one it attempted to
> fix.

I didn't either.  My goal was to fix C-x C-+ causing a font with an
incorrect weight to be selected, by only preserving :antialias (as
opposed to everything aside from foundry, family, registry and adstyle.)

The fix should definitely be rewritten.

> IOW, do we really understand the reason for bug#37473?  Not the commit
> which introduced it, but the reason why :antialiasing is being reset
> and doesn't appear in attrs[] and in the realized face?  If so, could
> someone please spell that out?

I think the person who wrote the font code is the only person who really
understands it.  Is Kenchi Handa still around?

bug#37473: 27.0.50; antialias setting is not preserved by, inheriting

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: David Ponce <da_vid@orange.fr>,  37473@debbugs.gnu.org,  larsi@gnus.org
> Date: Mon, 20 Jun 2022 08:49:19 +0800
> 
> My goal was to fix C-x C-+ causing a font with an incorrect weight
> to be selected, by only preserving :antialias (as opposed to
> everything aside from foundry, family, registry and adstyle.)

What did you need to fix? what did you see that didn't work correctly,
and how to reproduce that?

> The fix should definitely be rewritten.

The latest code still singles out :antialiasing, and I don't
understand why.

> > IOW, do we really understand the reason for bug#37473?  Not the commit
> > which introduced it, but the reason why :antialiasing is being reset
> > and doesn't appear in attrs[] and in the realized face?  If so, could
> > someone please spell that out?
> 
> I think the person who wrote the font code is the only person who really
> understands it.  Is Kenchi Handa still around?

I added Handa-san to the CC, hopefully he will comment.

bug#37473: 27.0.50; antialias setting is not preserved by, inheriting

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

>> My goal was to fix C-x C-+ causing a font with an incorrect weight
>> to be selected, by only preserving :antialias (as opposed to
>> everything aside from foundry, family, registry and adstyle.)
>
> What did you need to fix? what did you see that didn't work correctly,
> and how to reproduce that?

The recipe is in bug#37473 -- start Emacs with antialiasing off, hit
`C-x C-+', and the antialiasing would be on.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#37473: 27.0.50; antialias setting is not preserved by, inheriting

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: Po Lu <luangruo@yahoo.com>,  Kenichi Handa <handa@gnu.org>,
>   da_vid@orange.fr,  37473@debbugs.gnu.org
> Date: Mon, 20 Jun 2022 14:04:04 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> My goal was to fix C-x C-+ causing a font with an incorrect weight
> >> to be selected, by only preserving :antialias (as opposed to
> >> everything aside from foundry, family, registry and adstyle.)
> >
> > What did you need to fix? what did you see that didn't work correctly,
> > and how to reproduce that?
> 
> The recipe is in bug#37473 -- start Emacs with antialiasing off, hit
> `C-x C-+', and the antialiasing would be on.

I know, but Po Lu said he had some problems with your fix on his
machine, and that's what I was asking about.

bug#37473: 27.0.50; antialias setting is not preserved by, inheriting

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

> What did you need to fix? what did you see that didn't work correctly,
> and how to reproduce that?

The bold variant of Source Code Pro would be used after C-x C-+, instead
of the regular one.

> The latest code still singles out :antialiasing, and I don't
> understand why.

See the original bug report.

> I added Handa-san to the CC, hopefully he will comment.

Thanks.

bug#37473: 27.0.50; antialias setting is not preserved by, inheriting

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: Kenichi Handa <handa@gnu.org>,  da_vid@orange.fr,
>   37473@debbugs.gnu.org,  larsi@gnus.org
> Date: Mon, 20 Jun 2022 20:20:06 +0800
> 
> > The latest code still singles out :antialiasing, and I don't
> > understand why.
> 
> See the original bug report.

??? The original bug report talked about antialiasing, but it didn't
say that other attributes in the :extra property don't exhibit the
same issue.

bug#37473: 27.0.50; antialias setting is not preserved by, inheriting

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

> ??? The original bug report talked about antialiasing, but it didn't
> say that other attributes in the :extra property don't exhibit the
> same issue.

They might, but apparently :antialias is the only one to have caused
enough of a problem for someone to open a bug report.

I guess copying the entire extras alist would work too, if that's what
you would prefer.

Thanks.

bug#37473: 27.0.50; antialias setting is not preserved by, inheriting

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: handa@gnu.org,  da_vid@orange.fr,  37473@debbugs.gnu.org,  larsi@gnus.org
> Date: Mon, 20 Jun 2022 21:20:09 +0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > ??? The original bug report talked about antialiasing, but it didn't
> > say that other attributes in the :extra property don't exhibit the
> > same issue.
> 
> They might, but apparently :antialias is the only one to have caused
> enough of a problem for someone to open a bug report.
> 
> I guess copying the entire extras alist would work too, if that's what
> you would prefer.

Yes, I think that's what we should do, at least as long as the
solution is based on this technique.

bug#37473: 27.0.50; antialias setting is not preserved by, inheriting

[Eli Zaretskii]

> Cc: handa@gnu.org, larsi@gnus.org, 37473@debbugs.gnu.org, da_vid@orange.fr
> Date: Mon, 20 Jun 2022 16:37:32 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> 
> > From: Po Lu <luangruo@yahoo.com>
> > Cc: handa@gnu.org,  da_vid@orange.fr,  37473@debbugs.gnu.org,  larsi@gnus.org
> > Date: Mon, 20 Jun 2022 21:20:09 +0800
> > 
> > Eli Zaretskii <eliz@gnu.org> writes:
> > 
> > > ??? The original bug report talked about antialiasing, but it didn't
> > > say that other attributes in the :extra property don't exhibit the
> > > same issue.
> > 
> > They might, but apparently :antialias is the only one to have caused
> > enough of a problem for someone to open a bug report.
> > 
> > I guess copying the entire extras alist would work too, if that's what
> > you would prefer.
> 
> Yes, I think that's what we should do, at least as long as the
> solution is based on this technique.

Actually, why not go to the code we had in realize_gui_face before
commit bf0d3f7?  The problem described in bug#1793, which led to that
commit, only happens if one uses a specific (misc-fixed) font family,
not for the usual default fonts used by Emacs, and I'm not sure what's
described there is a bug at all.  At least for the purposes of
text-scale-adjust, it makes no sense to ignore the
foundry/family/adstyle of the original font, because we _want_ the
same (or very similar) font, just of a different size.

And likely in other use cases: if the :font attribute of a face
specifies some font properties, we want to keep them all, not
arbitrarily to ignore some of them.  And definitely catering to an
obscure use case such as the one cited in bug#17973 is NOT a good
reason to make such radical changes in face-realization behavior.

So I think we should just back out that part of commit bf0d3f7, and if
we decide that this causes bug#17973 in too many situations we care
about, I'd rather find a kludge for that specific case than do that
for every face realization.

Specifically, I propose the change for the master branch.  Any
objections?

diff --git a/src/xfaces.c b/src/xfaces.c
index f70fe87..10bde40 100644
--- a/src/xfaces.c
+++ b/src/xfaces.c
@@ -5952,28 +5952,8 @@ realize_gui_face (struct face_cache *cache, Lisp_Object attrs[LFACE_VECTOR_SIZE]
 	    emacs_abort ();
 	}
       if (! FONT_OBJECT_P (attrs[LFACE_FONT_INDEX]))
-	{
-	  /* We want attrs to allow overriding most elements in the
-	     spec (IOW, to start out as an empty font spec), but
-	     preserve the antialiasing attribute.  (bug#17973,
-	     bug#37473).  */
-	  temp_spec = Ffont_spec (0, NULL);
-	  temp_extra = AREF (attrs[LFACE_FONT_INDEX],
-			     FONT_EXTRA_INDEX);
-	  /* If `:antialias' wasn't specified, keep it unspecified
-	     instead of changing it to nil.  */
-
-	  if (CONSP (temp_extra))
-	    antialias = Fassq (QCantialias, temp_extra);
-	  else
-	    antialias = Qnil;
-
-	  if (FONTP (attrs[LFACE_FONT_INDEX]) && !NILP (antialias))
-	    Ffont_put (temp_spec, QCantialias, Fcdr (antialias));
-
-	  attrs[LFACE_FONT_INDEX]
-	    = font_load_for_lface (f, attrs, temp_spec);
-	}
+	attrs[LFACE_FONT_INDEX]
+	  = font_load_for_lface (f, attrs, attrs[LFACE_FONT_INDEX]);
       if (FONT_OBJECT_P (attrs[LFACE_FONT_INDEX]))
 	{
 	  face->font = XFONT_OBJECT (attrs[LFACE_FONT_INDEX]);

bug#37473: 27.0.50; antialias setting is not preserved by, inheriting

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> And likely in other use cases: if the :font attribute of a face
> specifies some font properties, we want to keep them all, not
> arbitrarily to ignore some of them.

Yup.

> Specifically, I propose the change for the master branch.  Any
> objections?

Not from me, but perhaps Handa-san has some comments.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#37473: 27.0.50; antialias setting is not preserved by, inheriting

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: luangruo@yahoo.com,  handa@gnu.org,  37473@debbugs.gnu.org,
>   da_vid@orange.fr,  Stefan Monnier <monnier@iro.umontreal.ca>
> Date: Mon, 20 Jun 2022 17:43:06 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > And likely in other use cases: if the :font attribute of a face
> > specifies some font properties, we want to keep them all, not
> > arbitrarily to ignore some of them.
> 
> Yup.
> 
> > Specifically, I propose the change for the master branch.  Any
> > objections?
> 
> Not from me, but perhaps Handa-san has some comments.

Sure, there's no rush.

My main problem with that change is that I don't understand why would
it make sense to "forget" all the attributes of a font when realizing
a face whose :font attribute is non-nil.  It's like ignoring the
foreground color when a face specifies it.

If there are special situations where some font attributes should be
"forgotten", we should perhaps have separate functions for them, which
we should call only in those special situations.

bug#37473: 27.0.50; antialias setting is not preserved by, inheriting

[Eli Zaretskii]

Ping!  Kenichi, could you please chime in on this issue and help us
resolve it?

> Date: Mon, 20 Jun 2022 17:11:49 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: da_vid@orange.fr, 37473@debbugs.gnu.org,
>  Stefan Monnier <monnier@iro.umontreal.ca>
> 
> > Cc: handa@gnu.org, larsi@gnus.org, 37473@debbugs.gnu.org, da_vid@orange.fr
> > Date: Mon, 20 Jun 2022 16:37:32 +0300
> > From: Eli Zaretskii <eliz@gnu.org>
> > 
> > > From: Po Lu <luangruo@yahoo.com>
> > > Cc: handa@gnu.org,  da_vid@orange.fr,  37473@debbugs.gnu.org,  larsi@gnus.org
> > > Date: Mon, 20 Jun 2022 21:20:09 +0800
> > > 
> > > Eli Zaretskii <eliz@gnu.org> writes:
> > > 
> > > > ??? The original bug report talked about antialiasing, but it didn't
> > > > say that other attributes in the :extra property don't exhibit the
> > > > same issue.
> > > 
> > > They might, but apparently :antialias is the only one to have caused
> > > enough of a problem for someone to open a bug report.
> > > 
> > > I guess copying the entire extras alist would work too, if that's what
> > > you would prefer.
> > 
> > Yes, I think that's what we should do, at least as long as the
> > solution is based on this technique.
> 
> Actually, why not go to the code we had in realize_gui_face before
> commit bf0d3f7?  The problem described in bug#1793, which led to that
> commit, only happens if one uses a specific (misc-fixed) font family,
> not for the usual default fonts used by Emacs, and I'm not sure what's
> described there is a bug at all.  At least for the purposes of
> text-scale-adjust, it makes no sense to ignore the
> foundry/family/adstyle of the original font, because we _want_ the
> same (or very similar) font, just of a different size.
> 
> And likely in other use cases: if the :font attribute of a face
> specifies some font properties, we want to keep them all, not
> arbitrarily to ignore some of them.  And definitely catering to an
> obscure use case such as the one cited in bug#17973 is NOT a good
> reason to make such radical changes in face-realization behavior.
> 
> So I think we should just back out that part of commit bf0d3f7, and if
> we decide that this causes bug#17973 in too many situations we care
> about, I'd rather find a kludge for that specific case than do that
> for every face realization.
> 
> Specifically, I propose the change for the master branch.  Any
> objections?
> 
> diff --git a/src/xfaces.c b/src/xfaces.c
> index f70fe87..10bde40 100644
> --- a/src/xfaces.c
> +++ b/src/xfaces.c
> @@ -5952,28 +5952,8 @@ realize_gui_face (struct face_cache *cache, Lisp_Object attrs[LFACE_VECTOR_SIZE]
>  	    emacs_abort ();
>  	}
>        if (! FONT_OBJECT_P (attrs[LFACE_FONT_INDEX]))
> -	{
> -	  /* We want attrs to allow overriding most elements in the
> -	     spec (IOW, to start out as an empty font spec), but
> -	     preserve the antialiasing attribute.  (bug#17973,
> -	     bug#37473).  */
> -	  temp_spec = Ffont_spec (0, NULL);
> -	  temp_extra = AREF (attrs[LFACE_FONT_INDEX],
> -			     FONT_EXTRA_INDEX);
> -	  /* If `:antialias' wasn't specified, keep it unspecified
> -	     instead of changing it to nil.  */
> -
> -	  if (CONSP (temp_extra))
> -	    antialias = Fassq (QCantialias, temp_extra);
> -	  else
> -	    antialias = Qnil;
> -
> -	  if (FONTP (attrs[LFACE_FONT_INDEX]) && !NILP (antialias))
> -	    Ffont_put (temp_spec, QCantialias, Fcdr (antialias));
> -
> -	  attrs[LFACE_FONT_INDEX]
> -	    = font_load_for_lface (f, attrs, temp_spec);
> -	}
> +	attrs[LFACE_FONT_INDEX]
> +	  = font_load_for_lface (f, attrs, attrs[LFACE_FONT_INDEX]);
>        if (FONT_OBJECT_P (attrs[LFACE_FONT_INDEX]))
>  	{
>  	  face->font = XFONT_OBJECT (attrs[LFACE_FONT_INDEX]);
> 
> 
> 
>