bug#19117: 25.0.50; emacs on x11 chooses different fonts for the same face sometimes

bug#19117: 25.0.50; emacs on x11 chooses different fonts for the same face sometimes

[Dima Kogan]

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

Hi. I'm observing an issue when running the latest GNU Emacs built from
source using the lucid X toolkit; this is on a Debian machine running
X11 (build details at the end).

Starting a few months ago (this is a regression) I'm seeing that
sometimes Emacs renders some faces with an unrequested (and ugly) font.
I can usually make it do the right thing by moving Emacs windows around
or resizing them, or maybe opening a new Emacs frame. It's not
consistent, and I don't know exactly what triggers it breaking or how I
can reliably fix it.

I'm attaching two screenshots of 'M-x describe-face italic' when it's
broken and right after I moved the windows around a bit, and re-invoked
describe-face. Note the very different font. Using what-cursor-position
I can see that in the wrong case the font is

 -urw-nimbus mono l-regular-o-normal--11-79-100-100-p-70-iso8859-1 (#x70)

and in the right case the font is

 -adobe-courier-medium-o-normal--11-80-100-100-m-60-iso8859-1 (#x6D)

The latter is correct since I customized my default-frame-alist to contain

 (font . "-adobe-courier-medium-r-*-*-*-80-*-*-m-*-iso8859-1")

Can somebody give me some pointers on how to debug this to make this bug
report useful? I see the breakage many times a day every day.

Thanks.



[-- Attachment #2: good.png --]
[-- Type: image/png, Size: 2729 bytes --]

[-- Attachment #3: bad.png --]
[-- Type: image/png, Size: 2760 bytes --]

[-- Attachment #4: Type: text/plain, Size: 1216 bytes --]



In GNU Emacs 25.0.50.1 (x86_64-pc-linux-gnu, X toolkit, Xaw scroll bars)
 of 2014-11-02 on lilbastard, modified by Debian
 (emacs-snapshot package, version 2:20141102+emacs-24.4-1596-gb8f900f-1)
Windowing system distributor `The X.Org Foundation', version 11.0.11600000
System Description:	Debian GNU/Linux unstable (sid)

Configured using:
 `configure --build x86_64-linux-gnu --host x86_64-linux-gnu
 --prefix=/usr --sharedstatedir=/var/lib --libexecdir=/usr/lib
 --localstatedir=/var --infodir=/usr/share/info/emacs-snapshot
 --mandir=/usr/share/man --with-pop=yes
 --enable-locallisppath=/etc/emacs-snapshot:/etc/emacs:/usr/local/share/emacs/25.0.50/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/25.0.50/site-lisp:/usr/share/emacs/site-lisp
 --with-crt-dir=/usr/lib/x86_64-linux-gnu/ --with-x=yes
 --with-x-toolkit=lucid 'CFLAGS=-DDEBIAN -DSITELOAD_PURESIZE_EXTRA=5000
 -g -O2' CPPFLAGS=-D_FORTIFY_SOURCE=2 'LDFLAGS=-g -Wl,--as-needed
 -znocombreloc''

Configured features:
XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND GPM DBUS GSETTINGS NOTIFY
LIBSELINUX GNUTLS LIBXML2 FREETYPE M17N_FLT LIBOTF XFT ZLIB

Important settings:
  value of $LC_ALL: C
  value of $LANG: C
  locale-coding-system: nil

bug#19117: 25.0.50; emacs on x11 chooses different fonts for the same face sometimes

[Eli Zaretskii]

> From: Dima Kogan <dima@secretsauce.net>
> Date: Wed, 19 Nov 2014 22:13:47 -0800
> 
> Starting a few months ago (this is a regression) I'm seeing that
> sometimes Emacs renders some faces with an unrequested (and ugly) font.

"Sometimes"?  Can it be that it happens when Emacs needs to display
some character not available in the "non-ugly" font?

> I can usually make it do the right thing by moving Emacs windows around
> or resizing them, or maybe opening a new Emacs frame.

??? Incredible.  You mean, you do all that magic, and the font used
for some text suddenly changes?  Without killing the buffer with that
text?

> Can somebody give me some pointers on how to debug this to make this bug
> report useful? I see the breakage many times a day every day.

Put a breakpoint where Emacs loads new fonts, and see who calls that
code.

bug#19117: 25.0.50; emacs on x11 chooses different fonts for the same face sometimes

[Dima Kogan]

Eli Zaretskii <eliz@gnu.org> writes:

>> Starting a few months ago (this is a regression) I'm seeing that
>> sometimes Emacs renders some faces with an unrequested (and ugly) font.
>
> "Sometimes"?  Can it be that it happens when Emacs needs to display
> some character not available in the "non-ugly" font?

No, it's independent of displaying any particular characters, as far as
I can tell. On my machine I can start emacs (with all my .emacs stuff
sadly) and issue

 (face-font "italic")

and it reports the wrong font. Moving windows around fixes it, and makes
it report the correct font.



>> I can usually make it do the right thing by moving Emacs windows around
>> or resizing them, or maybe opening a new Emacs frame.
>
> ??? Incredible.  You mean, you do all that magic, and the font used
> for some text suddenly changes?  Without killing the buffer with that
> text?

Not 100% sure. The exact sequence to make it recover eludes me, so I'm
not sure if buffers need to be killed or refreshed in any way. Once it
recovers, things look good and (face-font "italic") reports the right
thing.


>> Can somebody give me some pointers on how to debug this to make this bug
>> report useful? I see the breakage many times a day every day.
>
> Put a breakpoint where Emacs loads new fonts, and see who calls that
> code.

I'm digging through the code. It's slow going so far, but I'm getting
more familiar with it. In my init.el I have in my default-frame-alist

 (font . "-adobe-courier-medium-r-*-*-*-80-*-*-m-*-iso8859-1")

I'm looking at realize_x_face() in xfaces.c. When I create a new frame
by pressing C-x 5 2, all the faces get re-realized, and I can see which
fonts are chosen. That function takes in a list of face attributes, and
associates a font with each face. I'm looking at attrs[LFACE_FONT_INDEX]
on input and face->font on output. For most faces, these two values are

 #<font-object "-adobe-courier-medium-r-normal--11-80-100-100-m-60-iso8859-1">
 "-adobe-courier-medium-r-normal--11-80-100-100-m-60-iso8859-1"

which is correct: we're asking for a particular font and we're getting
it. For many faces, however these values are

 #<font-spec nil adobe courier ## iso8859-1 medium nil normal 11 100 100 60 ((user-spec . "-adobe-courier-medium-r-*-*-*-80-*-*-m-*-iso8859-1"))>
 "-urw-nimbus mono l-regular-o-normal--11-79-100-100-p-70-iso8859-1"

or

 #<font-spec nil adobe courier ## iso8859-1 medium nil normal 11 100 100 60 ((user-spec . "-adobe-courier-medium-r-*-*-*-80-*-*-m-*-iso8859-1"))>
 "-adobe-courier-medium-o-normal--11-80-100-100-m-60-iso8859-1"

Here we're requesting a slanted version of the default font. Sometimes
emacs picks the right one (-adobe-courier-medium-o-...), but at other
times it picks the wrong one (-urw-....). The exact set of faces that
get the wrong font varies with each invocation of realize_x_face(), but
there's always at least one wrong one.

That's the only conclusive evidence I have so far, but I'm still looking
into it. Any suggestions welcome.

bug#19117: 25.0.50; emacs on x11 chooses different fonts for the same face sometimes

[Eli Zaretskii]

> From: Dima Kogan <dima@secretsauce.net>
> Date: Sat, 06 Dec 2014 23:28:34 -0800
> 
> > Put a breakpoint where Emacs loads new fonts, and see who calls that
> > code.
> 
> I'm digging through the code. It's slow going so far, but I'm getting
> more familiar with it. In my init.el I have in my default-frame-alist
> 
>  (font . "-adobe-courier-medium-r-*-*-*-80-*-*-m-*-iso8859-1")
> 
> I'm looking at realize_x_face() in xfaces.c. When I create a new frame
> by pressing C-x 5 2, all the faces get re-realized, and I can see which
> fonts are chosen. That function takes in a list of face attributes, and
> associates a font with each face. I'm looking at attrs[LFACE_FONT_INDEX]
> on input and face->font on output. For most faces, these two values are
> 
>  #<font-object "-adobe-courier-medium-r-normal--11-80-100-100-m-60-iso8859-1">
>  "-adobe-courier-medium-r-normal--11-80-100-100-m-60-iso8859-1"
> 
> which is correct: we're asking for a particular font and we're getting
> it.

the above is a font object.

> For many faces, however these values are
> 
>  #<font-spec nil adobe courier ## iso8859-1 medium nil normal 11 100 100 60 ((user-spec . "-adobe-courier-medium-r-*-*-*-80-*-*-m-*-iso8859-1"))>
>  "-urw-nimbus mono l-regular-o-normal--11-79-100-100-p-70-iso8859-1"
> 
> or
> 
>  #<font-spec nil adobe courier ## iso8859-1 medium nil normal 11 100 100 60 ((user-spec . "-adobe-courier-medium-r-*-*-*-80-*-*-m-*-iso8859-1"))>
>  "-adobe-courier-medium-o-normal--11-80-100-100-m-60-iso8859-1"

These two aren't font objects, they are font specs, a different
object.  See font.h for their respective descriptions (unless you
already read that).

> Here we're requesting a slanted version of the default font. Sometimes
> emacs picks the right one (-adobe-courier-medium-o-...), but at other
> times it picks the wrong one (-urw-....). The exact set of faces that
> get the wrong font varies with each invocation of realize_x_face(), but
> there's always at least one wrong one.

Does the backtrace tell something about the reason(s) for the
differences?

bug#19117: 25.0.50; emacs on x11 chooses different fonts for the same face sometimes

[Dima Kogan]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Dima Kogan <dima@secretsauce.net>
>> Date: Sat, 06 Dec 2014 23:28:34 -0800
>> 
>> > Put a breakpoint where Emacs loads new fonts, and see who calls that
>> > code.
>> 
>> I'm digging through the code. It's slow going so far, but I'm getting
>> more familiar with it. In my init.el I have in my default-frame-alist
>> 
>>  (font . "-adobe-courier-medium-r-*-*-*-80-*-*-m-*-iso8859-1")

I just dumped more time into this, and have more information, some of it
(hopefully) actionable.

I just built a optimization-free copy of emacs from the latest git:
e2815bf. This uses the lucid UI. I'm on a recent Debian/sid box.

This allows me to use gdb effectively. I also have a mostly reproducible
way to both break it and to fix it with emacs -Q. I suspect this recipe
is dependent on my window manager and font setup, so it probably
wouldn't trigger failures on other boxes, but it's useful to illustrate
what I'm doing. Recipe:

1. emacs -Q --eval "(progn (custom-set-variables '(default-frame-alist '( (font . \"-adobe-courier-medium-r-*-*-*-80-*-*-m-*-iso8859-1\")))) (kill-new \"(face-font \\\"italic\\\")\"))"

2. M-: (face-font "italic")

   The commandline eval places this string into the kill-ring so you can
   simply M-: C-y, but this is a detail. I expect it to give me an
   -adobe-courier font, but at this point it gives me an (ugly)
   -urw-.... font. Happens about 80% of the time

3. C-x 5 2

   New frame

4. Kill that new frame with my window manager

5. M-: (face-font "italic")

   Same font lookup as before. This time it gives me the correct font.


I traced various parts of the data flow. Internally emacs generates a
list of candidate fonts, scores each one against the requested font
spec, and picks the font with the best score. The issue is two-fold:

- The list of candidate fonts changes over time. This is why the issue
  self-corrects

- In the above recipe, the "right" font is in the list both times,
  however the first (wrong) time there's another candidate in the list
  that scores highly, and produces an ugly result


The top-level code flow is

 font_find_for_lface()      calls
   font_list_entities()     to get the candidate font list and
   font_select_entity()     to pick the best candidate; this calls
     font_sort_entities()   which calls
       font_score()         to evaluate a specific candidate

The "reference" font that font_score() is asked to match appears to
always be

 #<font-spec nil nil nil nil nil medium italic normal 11 nil nil nil nil>

In font_list_entities(), need_filtering = 0 is always true.



When the incorrect font is chosen at the start I see that

font_list_entities() gets a list of fonts from the cache (not from the
driver; driver_list->driver->list() is not invoked here).
font_list_entities() returns this candidate list (a vector):

#<font-entity x adobe courier ## iso8859-1 medium r normal 8 75 100 50 nil>
#<font-entity x adobe courier ## iso8859-1 medium r normal 34 100 100 200 nil>
#<font-entity x adobe courier ## iso8859-1 medium r normal 25 100 100 150 nil>
#<font-entity x adobe courier ## iso8859-1 medium r normal 24 75 100 150 nil>
#<font-entity x adobe courier ## iso8859-1 medium r normal 20 100 100 110 nil>
#<font-entity x adobe courier ## iso8859-1 medium r normal 18 75 100 110 nil>
#<font-entity x adobe courier ## iso8859-1 medium r normal 17 100 100 100 nil>
#<font-entity x adobe courier ## iso8859-1 medium r normal 14 75 100 90 nil>
#<font-entity x adobe courier ## iso8859-1 medium r normal 14 100 100 90 nil>
#<font-entity x adobe courier ## iso8859-1 medium r normal 12 75 100 70 nil>
#<font-entity x adobe courier ## iso8859-1 medium r normal 11 100 100 60 nil>
#<font-entity x adobe courier ## iso8859-1 medium r normal 10 75 100 60 nil>
#<font-entity x adobe courier ## iso8859-1 medium r normal 0 0 0 0 nil>
#<font-entity x adobe courier ## iso8859-1 medium o normal 8 75 100 50 nil>
#<font-entity x adobe courier ## iso8859-1 medium o normal 34 100 100 200 nil>
#<font-entity x adobe courier ## iso8859-1 medium o normal 25 100 100 150 nil>
#<font-entity x adobe courier ## iso8859-1 medium o normal 24 75 100 150 nil>
#<font-entity x adobe courier ## iso8859-1 medium o normal 20 100 100 110 nil>
#<font-entity x adobe courier ## iso8859-1 medium o normal 18 75 100 110 nil>
#<font-entity x adobe courier ## iso8859-1 medium o normal 17 100 100 100 nil>
#<font-entity x adobe courier ## iso8859-1 medium o normal 14 75 100 90 nil>
#<font-entity x adobe courier ## iso8859-1 medium o normal 14 100 100 90 nil>
#<font-entity x adobe courier ## iso8859-1 medium o normal 12 75 100 70 nil>
#<font-entity x adobe courier ## iso8859-1 medium o normal 11 100 100 60 nil>
#<font-entity x adobe courier ## iso8859-1 medium o normal 10 75 100 60 nil>
#<font-entity x adobe courier ## iso8859-1 medium o normal 0 0 0 0 nil>
#<font-entity x adobe courier ## iso8859-1 medium i normal 0 0 0 0 nil>
#<font-entity x adobe courier ## iso8859-1 bold r normal 8 75 100 50 nil>
#<font-entity x adobe courier ## iso8859-1 bold r normal 34 100 100 200 nil>
#<font-entity x adobe courier ## iso8859-1 bold r normal 25 100 100 150 nil>
#<font-entity x adobe courier ## iso8859-1 bold r normal 24 75 100 150 nil>
#<font-entity x adobe courier ## iso8859-1 bold r normal 20 100 100 110 nil>
#<font-entity x adobe courier ## iso8859-1 bold r normal 18 75 100 110 nil>
#<font-entity x adobe courier ## iso8859-1 bold r normal 17 100 100 100 nil>
#<font-entity x adobe courier ## iso8859-1 bold r normal 14 75 100 90 nil>
#<font-entity x adobe courier ## iso8859-1 bold r normal 14 100 100 90 nil>
#<font-entity x adobe courier ## iso8859-1 bold r normal 12 75 100 70 nil>
#<font-entity x adobe courier ## iso8859-1 bold r normal 11 100 100 60 nil>
#<font-entity x adobe courier ## iso8859-1 bold r normal 10 75 100 60 nil>
#<font-entity x adobe courier ## iso8859-1 bold r normal 0 0 0 0 nil>
#<font-entity x adobe courier ## iso8859-1 bold o normal 8 75 100 50 nil>
#<font-entity x adobe courier ## iso8859-1 bold o normal 34 100 100 200 nil>
#<font-entity x adobe courier ## iso8859-1 bold o normal 25 100 100 150 nil>
#<font-entity x adobe courier ## iso8859-1 bold o normal 24 75 100 150 nil>
#<font-entity x adobe courier ## iso8859-1 bold o normal 20 100 100 110 nil>
#<font-entity x adobe courier ## iso8859-1 bold o normal 18 75 100 110 nil>
#<font-entity x adobe courier ## iso8859-1 bold o normal 17 100 100 100 nil>
#<font-entity x adobe courier ## iso8859-1 bold o normal 14 75 100 90 nil>
#<font-entity x adobe courier ## iso8859-1 bold o normal 14 100 100 90 nil>
#<font-entity x adobe courier ## iso8859-1 bold o normal 12 75 100 70 nil>
#<font-entity x adobe courier ## iso8859-1 bold o normal 11 100 100 60 nil>
#<font-entity x adobe courier ## iso8859-1 bold o normal 10 75 100 60 nil>
#<font-entity x adobe courier ## iso8859-1 bold o normal 0 0 0 0 nil>
#<font-entity x adobe courier ## iso8859-1 bold i normal 0 0 0 0 nil>

font_score() is given a subset of these. Candidates and returned scores:

| font                                                                           |   score |
|--------------------------------------------------------------------------------+---------|
| #<font-entity x adobe courier ## iso8859-1 medium r normal 8 75 100 50 nil>    |  393616 |
| #<font-entity x adobe courier ## iso8859-1 medium r normal 34 100 100 200 nil> |         |
| #<font-entity x adobe courier ## iso8859-1 medium r normal 25 100 100 150 nil> |         |
| #<font-entity x adobe courier ## iso8859-1 medium r normal 24 75 100 150 nil>  |         |
| #<font-entity x adobe courier ## iso8859-1 medium r normal 20 100 100 110 nil> | 1180048 |
| #<font-entity x adobe courier ## iso8859-1 medium r normal 18 75 100 110 nil>  |  917904 |
| #<font-entity x adobe courier ## iso8859-1 medium r normal 17 100 100 100 nil> |  786832 |
| #<font-entity x adobe courier ## iso8859-1 medium r normal 14 75 100 90 nil>   |  393616 |
| #<font-entity x adobe courier ## iso8859-1 medium r normal 14 100 100 90 nil>  |  393616 |
| #<font-entity x adobe courier ## iso8859-1 medium r normal 12 75 100 70 nil>   |  131472 |
| #<font-entity x adobe courier ## iso8859-1 medium r normal 11 100 100 60 nil>  |     400 |
| #<font-entity x adobe courier ## iso8859-1 medium r normal 10 75 100 60 nil>   |  131472 |
| #<font-entity x adobe courier ## iso8859-1 medium r normal 0 0 0 0 nil>        |     400 |
| #<font-entity x adobe courier ## iso8859-1 medium o normal 8 75 100 50 nil>    |  393256 |
| #<font-entity x adobe courier ## iso8859-1 medium o normal 34 100 100 200 nil> |         |
| #<font-entity x adobe courier ## iso8859-1 medium o normal 25 100 100 150 nil> |         |
| #<font-entity x adobe courier ## iso8859-1 medium o normal 24 75 100 150 nil>  |         |
| #<font-entity x adobe courier ## iso8859-1 medium o normal 20 100 100 110 nil> | 1179688 |
| #<font-entity x adobe courier ## iso8859-1 medium o normal 18 75 100 110 nil>  |  917544 |
| #<font-entity x adobe courier ## iso8859-1 medium o normal 17 100 100 100 nil> |  786472 |
| #<font-entity x adobe courier ## iso8859-1 medium o normal 14 75 100 90 nil>   |  393256 |
| #<font-entity x adobe courier ## iso8859-1 medium o normal 14 100 100 90 nil>  |  393256 |
| #<font-entity x adobe courier ## iso8859-1 medium o normal 12 75 100 70 nil>   |  131112 |
| #<font-entity x adobe courier ## iso8859-1 medium o normal 11 100 100 60 nil>  |      40 |
| #<font-entity x adobe courier ## iso8859-1 medium o normal 10 75 100 60 nil>   |  131112 |
| #<font-entity x adobe courier ## iso8859-1 medium o normal 0 0 0 0 nil>        |      40 |
| #<font-entity x adobe courier ## iso8859-1 medium i normal 0 0 0 0 nil>        |       0 |

Fonts with blank scores are disqualified. Lower scores are better, so
the last font with a score of 0 is chosen. This, unfortunately produces
poor results.



When the correct font is chosen further down in the recipe, I see that

font_list_entities() again gets a list of fonts from the cache:

#<font-entity x adobe courier ## iso8859-1 medium r normal 8 75 100 50 nil>
#<font-entity x adobe courier ## iso8859-1 medium r normal 34 100 100 200 nil>
#<font-entity x adobe courier ## iso8859-1 medium r normal 25 100 100 150 nil>
#<font-entity x adobe courier ## iso8859-1 medium r normal 24 75 100 150 nil>
#<font-entity x adobe courier ## iso8859-1 medium r normal 20 100 100 110 nil>
#<font-entity x adobe courier ## iso8859-1 medium r normal 18 75 100 110 nil>
#<font-entity x adobe courier ## iso8859-1 medium r normal 17 100 100 100 nil>
#<font-entity x adobe courier ## iso8859-1 medium r normal 14 75 100 90 nil>
#<font-entity x adobe courier ## iso8859-1 medium r normal 14 100 100 90 nil>
#<font-entity x adobe courier ## iso8859-1 medium r normal 12 75 100 70 nil>
#<font-entity x adobe courier ## iso8859-1 medium r normal 11 100 100 60 nil>
#<font-entity x adobe courier ## iso8859-1 medium r normal 10 75 100 60 nil>
#<font-entity x adobe courier ## iso8859-1 medium o normal 8 75 100 50 nil>
#<font-entity x adobe courier ## iso8859-1 medium o normal 34 100 100 200 nil>
#<font-entity x adobe courier ## iso8859-1 medium o normal 25 100 100 150 nil>
#<font-entity x adobe courier ## iso8859-1 medium o normal 24 75 100 150 nil>
#<font-entity x adobe courier ## iso8859-1 medium o normal 20 100 100 110 nil>
#<font-entity x adobe courier ## iso8859-1 medium o normal 18 75 100 110 nil>
#<font-entity x adobe courier ## iso8859-1 medium o normal 17 100 100 100 nil>
#<font-entity x adobe courier ## iso8859-1 medium o normal 14 75 100 90 nil>
#<font-entity x adobe courier ## iso8859-1 medium o normal 14 100 100 90 nil>
#<font-entity x adobe courier ## iso8859-1 medium o normal 12 75 100 70 nil>
#<font-entity x adobe courier ## iso8859-1 medium o normal 11 100 100 60 nil>
#<font-entity x adobe courier ## iso8859-1 medium o normal 10 75 100 60 nil>
#<font-entity x adobe courier ## iso8859-1 bold r normal 8 75 100 50 nil>
#<font-entity x adobe courier ## iso8859-1 bold r normal 34 100 100 200 nil>
#<font-entity x adobe courier ## iso8859-1 bold r normal 25 100 100 150 nil>
#<font-entity x adobe courier ## iso8859-1 bold r normal 24 75 100 150 nil>
#<font-entity x adobe courier ## iso8859-1 bold r normal 20 100 100 110 nil>
#<font-entity x adobe courier ## iso8859-1 bold r normal 18 75 100 110 nil>
#<font-entity x adobe courier ## iso8859-1 bold r normal 17 100 100 100 nil>
#<font-entity x adobe courier ## iso8859-1 bold r normal 14 75 100 90 nil>
#<font-entity x adobe courier ## iso8859-1 bold r normal 14 100 100 90 nil>
#<font-entity x adobe courier ## iso8859-1 bold r normal 12 75 100 70 nil>
#<font-entity x adobe courier ## iso8859-1 bold r normal 11 100 100 60 nil>
#<font-entity x adobe courier ## iso8859-1 bold r normal 10 75 100 60 nil>
#<font-entity x adobe courier ## iso8859-1 bold o normal 8 75 100 50 nil>
#<font-entity x adobe courier ## iso8859-1 bold o normal 34 100 100 200 nil>
#<font-entity x adobe courier ## iso8859-1 bold o normal 25 100 100 150 nil>
#<font-entity x adobe courier ## iso8859-1 bold o normal 24 75 100 150 nil>
#<font-entity x adobe courier ## iso8859-1 bold o normal 20 100 100 110 nil>
#<font-entity x adobe courier ## iso8859-1 bold o normal 18 75 100 110 nil>
#<font-entity x adobe courier ## iso8859-1 bold o normal 17 100 100 100 nil>
#<font-entity x adobe courier ## iso8859-1 bold o normal 14 75 100 90 nil>
#<font-entity x adobe courier ## iso8859-1 bold o normal 14 100 100 90 nil>
#<font-entity x adobe courier ## iso8859-1 bold o normal 12 75 100 70 nil>
#<font-entity x adobe courier ## iso8859-1 bold o normal 11 100 100 60 nil>
#<font-entity x adobe courier ## iso8859-1 bold o normal 10 75 100 60 nil>

and the corresponding scores:

| font                                                                           |   score |
|--------------------------------------------------------------------------------+---------|
| #<font-entity x adobe courier ## iso8859-1 medium r normal 8 75 100 50 nil>    |  393616 |
| #<font-entity x adobe courier ## iso8859-1 medium r normal 34 100 100 200 nil> |         |
| #<font-entity x adobe courier ## iso8859-1 medium r normal 25 100 100 150 nil> |         |
| #<font-entity x adobe courier ## iso8859-1 medium r normal 24 75 100 150 nil>  |         |
| #<font-entity x adobe courier ## iso8859-1 medium r normal 20 100 100 110 nil> | 1180048 |
| #<font-entity x adobe courier ## iso8859-1 medium r normal 18 75 100 110 nil>  |  917904 |
| #<font-entity x adobe courier ## iso8859-1 medium r normal 17 100 100 100 nil> |  786832 |
| #<font-entity x adobe courier ## iso8859-1 medium r normal 14 75 100 90 nil>   |  393616 |
| #<font-entity x adobe courier ## iso8859-1 medium r normal 14 100 100 90 nil>  |  393616 |
| #<font-entity x adobe courier ## iso8859-1 medium r normal 12 75 100 70 nil>   |  131472 |
| #<font-entity x adobe courier ## iso8859-1 medium r normal 11 100 100 60 nil>  |     400 |
| #<font-entity x adobe courier ## iso8859-1 medium r normal 10 75 100 60 nil>   |  131472 |
| #<ont-entity x adobe courier ## iso8859-1 medium o normal 8 75 100 50 nil>     |  393256 |
| #<font-entity x adobe courier ## iso8859-1 medium o normal 34 100 100 200 nil> |         |
| #<font-entity x adobe courier ## iso8859-1 medium o normal 25 100 100 150 nil> |         |
| #<font-entity x adobe courier ## iso8859-1 medium o normal 24 75 100 150 nil>  |         |
| #<font-entity x adobe courier ## iso8859-1 medium o normal 20 100 100 110 nil> | 1179688 |
| #<font-entity x adobe courier ## iso8859-1 medium o normal 18 75 100 110 nil>  |  917544 |
| #<font-entity x adobe courier ## iso8859-1 medium o normal 17 100 100 100 nil> |  786472 |
| #<font-entity x adobe courier ## iso8859-1 medium o normal 14 75 100 90 nil>   |  393256 |
| #<font-entity x adobe courier ## iso8859-1 medium o normal 14 100 100 90 nil>  |  393256 |
| #<font-entity x adobe courier ## iso8859-1 medium o normal 12 75 100 70 nil>   |  131112 |
| #<font-entity x adobe courier ## iso8859-1 medium o normal 11 100 100 60 nil>  |      40 |
| #<font-entity x adobe courier ## iso8859-1 medium o normal 10 75 100 60 nil>   |  131112 |
| #<font-entity x adobe courier ## iso8859-1 bold r normal 8 75 100 50 nil>      |  444816 |
| #<font-entity x adobe courier ## iso8859-1 bold r normal 34 100 100 200 nil>   |         |
| #<font-entity x adobe courier ## iso8859-1 bold r normal 25 100 100 150 nil>   |         |
| #<font-entity x adobe courier ## iso8859-1 bold r normal 24 75 100 150 nil>    |         |
| #<font-entity x adobe courier ## iso8859-1 bold r normal 20 100 100 110 nil>   | 1231248 |
| #<font-entity x adobe courier ## iso8859-1 bold r normal 18 75 100 110 nil>    |  969104 |
| #<font-entity x adobe courier ## iso8859-1 bold r normal 17 100 100 100 nil>   |  838032 |
| #<font-entity x adobe courier ## iso8859-1 bold r normal 14 75 100 90 nil>     |  444816 |
| #<font-entity x adobe courier ## iso8859-1 bold r normal 14 100 100 90 nil>    |  444816 |
| #<font-entity x adobe courier ## iso8859-1 bold r normal 12 75 100 70 nil>     |  182672 |
| #<font-entity x adobe courier ## iso8859-1 bold r normal 11 100 100 60 nil>    |   51600 |
| #<font-entity x adobe courier ## iso8859-1 bold r normal 10 75 100 60 nil>     |  182672 |
| #<font-entity x adobe courier ## iso8859-1 bold o normal 8 75 100 50 nil>      |  444456 |
| #<font-entity x adobe courier ## iso8859-1 bold o normal 34 100 100 200 nil>   |         |
| #<font-entity x adobe courier ## iso8859-1 bold o normal 25 100 100 150 nil>   |         |
| #<font-entity x adobe courier ## iso8859-1 bold o normal 24 75 100 150 nil>    |         |
| #<font-entity x adobe courier ## iso8859-1 bold o normal 20 100 100 110 nil>   | 1230888 |
| #<font-entity x adobe courier ## iso8859-1 bold o normal 18 75 100 110 nil>    |  968744 |
| #<font-entity x adobe courier ## iso8859-1 bold o normal 17 100 100 100 nil>   |  837672 |
| #<font-entity x adobe courier ## iso8859-1 bold o normal 14 75 100 90 nil>     |  444456 |
| #<font-entity x adobe courier ## iso8859-1 bold o normal 14 100 100 90 nil>    |  444456 |
| #<font-entity x adobe courier ## iso8859-1 bold o normal 12 75 100 70 nil>     |  182312 |
| #<font-entity x adobe courier ## iso8859-1 bold o normal 11 100 100 60 nil>    |   51240 |
| #<font-entity x adobe courier ## iso8859-1 bold o normal 10 75 100 60 nil>     |  182312 |

The best font has a score of 40, and it is the "right" answer.



I'm not an expert, but it seems wrong to me that the candidate list does
not stay constant. Furthermore, it's odd that the bold fonts are not
passed to font_score() during the "bad" pass, but ARE passed to it during
the "good" pass. Furthermore, the fonts that have "0 0 0 0" all score
relatively well in the "bad" pass, and the best match is one of those.
In the "good" pass those "0 0 0 0" fonts aren't there at all.

I'm going to stop investigating for today. If this is enough for
somebody knowledgeable to investigate, then that's great. Otherwise I'll
keep digging myself later. What's the "right" behavior? I suspect that
blocking out the "0 0 0 0" fonts would be sufficient to solve my issue,
but wouldn't address the other things that look wrong and possibly are
an issue for others.

bug#19117: 25.0.50; emacs on x11 chooses different fonts for the same face sometimes

[Dima Kogan]

I just looked a bit more.

As I mentioned in the last message, the font description that results in
ugly rendering is

 #<font-entity x adobe courier ## iso8859-1 medium i normal 0 0 0 0 nil>

Apparently the "0 0 0 0" represents a "scalable" font. Not 100% sure
what this means in the context of X11. In Emacs, we load this font with
XLoadQueryFont() in xfont_open(). I wrote a small program to see what
this means on my box:

 #include <stdio.h>
 #include <stdlib.h>
 #include <X11/Xlib.h>
 #include <X11/Xatom.h>
 int main(void)
 {
     Display* dpy = XOpenDisplay(":0.0");

     const char* font_input = "-adobe-courier-medium-i-normal-*-0-0-0-0-p-0-iso8859-1";
     const char* font_output;
     unsigned long value;

     XFontStruct *font = XLoadQueryFont(dpy, font_input);
     XGetFontProperty (font, XA_FONT, &value);
     font_output = XGetAtomName (dpy, (Atom) value);

     fprintf(stderr, "font '%s' loaded as '%s'\n", font_input, font_output);
     return 0;
 }

Here I load the scalable font that Emacs is choosing (and that xlsfonts
says exists on my machine). Then after it is loaded, I ask what the font
is called. The output is this:

font '-adobe-courier-medium-i-normal-*-0-0-0-0-p-0-iso8859-1' loaded as '-urw-nimbus mono l-regular-o-normal--17-120-100-100-p-100-iso8859-1'

So this completely explains why I'm getting ugly rendering. Emacs is
picking a scalable adobe font, which XLoadQueryFont() interprets as a
17-pixel urw bitmap font. Since I was asking for an 11-pixel font to
begin with, emacs is down-scaling this font by a factor of 11/17, which
does not look so good.

bug#19117: 25.0.50; emacs on x11 chooses different fonts for the same face sometimes

[Eli Zaretskii]

> From: Dima Kogan <dima@secretsauce.net>
> Cc: 19117@debbugs.gnu.org
> Date: Tue, 16 Dec 2014 22:57:41 -0800
> 
> I just looked a bit more.

Dmitry, Jan, could you please help Dima here?

bug#19117: 25.0.50; emacs on x11 chooses different fonts for the same face sometimes

[Dmitry Antipov]

On 12/17/2014 09:57 AM, Dima Kogan wrote:

> Apparently the "0 0 0 0" represents a "scalable" font. Not 100% sure
> what this means in the context of X11. In Emacs, we load this font with
> XLoadQueryFont() in xfont_open(). I wrote a small program to see what
> this means on my box:
>
>   #include <stdio.h>
>   #include <stdlib.h>
>   #include <X11/Xlib.h>
>   #include <X11/Xatom.h>
>   int main(void)
>   {
>       Display* dpy = XOpenDisplay(":0.0");
>
>       const char* font_input = "-adobe-courier-medium-i-normal-*-0-0-0-0-p-0-iso8859-1";
>       const char* font_output;
>       unsigned long value;
>
>       XFontStruct *font = XLoadQueryFont(dpy, font_input);
>       XGetFontProperty (font, XA_FONT, &value);
>       font_output = XGetAtomName (dpy, (Atom) value);
>
>       fprintf(stderr, "font '%s' loaded as '%s'\n", font_input, font_output);
>       return 0;
>   }
>
> Here I load the scalable font that Emacs is choosing (and that xlsfonts
> says exists on my machine). Then after it is loaded, I ask what the font
> is called. The output is this:
>
> font '-adobe-courier-medium-i-normal-*-0-0-0-0-p-0-iso8859-1' loaded as
> '-urw-nimbus mono l-regular-o-normal--17-120-100-100-p-100-iso8859-1'

Hm. On my system (Fedora 21), there are no -adobe-courier-medium-i-normal-*
fonts but:

$ xlsfonts | grep -- -adobe-utopia-bold-i-normal-
-adobe-utopia-bold-i-normal--0-0-0-0-p-0-iso10646-1
-adobe-utopia-bold-i-normal--0-0-0-0-p-0-iso10646-1
-adobe-utopia-bold-i-normal--0-0-0-0-p-0-iso8859-1
-adobe-utopia-bold-i-normal--0-0-0-0-p-0-iso8859-1
-adobe-utopia-bold-i-normal--10-100-75-75-p-58-iso10646-1
-adobe-utopia-bold-i-normal--12-120-75-75-p-70-iso10646-1
-adobe-utopia-bold-i-normal--14-100-100-100-p-78-iso10646-1
-adobe-utopia-bold-i-normal--14-100-100-100-p-78-iso8859-1
-adobe-utopia-bold-i-normal--15-140-75-75-p-82-iso10646-1
-adobe-utopia-bold-i-normal--17-120-100-100-p-93-iso10646-1
-adobe-utopia-bold-i-normal--17-120-100-100-p-93-iso8859-1
-adobe-utopia-bold-i-normal--19-140-100-100-p-109-iso10646-1
-adobe-utopia-bold-i-normal--19-140-100-100-p-109-iso8859-1
-adobe-utopia-bold-i-normal--19-180-75-75-p-105-iso10646-1
-adobe-utopia-bold-i-normal--25-180-100-100-p-139-iso10646-1
-adobe-utopia-bold-i-normal--25-180-100-100-p-139-iso8859-1
-adobe-utopia-bold-i-normal--25-240-75-75-p-140-iso10646-1
-adobe-utopia-bold-i-normal--33-240-100-100-p-186-iso10646-1
-adobe-utopia-bold-i-normal--33-240-100-100-p-186-iso8859-1

And running your program with -adobe-utopia-bold-i-normal--0-0-0-0-p-0-iso8859-1
produces:

font '-adobe-utopia-bold-i-normal--0-0-0-0-p-0-iso8859-1' loaded as '-adobe-utopia-bold-i-normal--17-120-100-100-p-94-iso8859-1'

Your X behaves pretty strange; I can't explain this just now.

Also, what happens if you specify default font via ~/.Xdefaults
and run with emacs -q (small 'q', not 'Q')?

Dmitry

bug#19117: 25.0.50; emacs on x11 chooses different fonts for the same face sometimes

[Dima Kogan]

Dmitry Antipov <dmantipov@yandex.ru> writes:

> Hm. On my system (Fedora 21), there are no -adobe-courier-medium-i-normal-*
> fonts but:
>
> $ xlsfonts | grep -- -adobe-utopia-bold-i-normal-
> -adobe-utopia-bold-i-normal--0-0-0-0-p-0-iso10646-1
> -adobe-utopia-bold-i-normal--0-0-0-0-p-0-iso10646-1
> -adobe-utopia-bold-i-normal--0-0-0-0-p-0-iso8859-1
> -adobe-utopia-bold-i-normal--0-0-0-0-p-0-iso8859-1
> -adobe-utopia-bold-i-normal--10-100-75-75-p-58-iso10646-1
> -adobe-utopia-bold-i-normal--12-120-75-75-p-70-iso10646-1
> -adobe-utopia-bold-i-normal--14-100-100-100-p-78-iso10646-1
> -adobe-utopia-bold-i-normal--14-100-100-100-p-78-iso8859-1
> -adobe-utopia-bold-i-normal--15-140-75-75-p-82-iso10646-1
> -adobe-utopia-bold-i-normal--17-120-100-100-p-93-iso10646-1
> -adobe-utopia-bold-i-normal--17-120-100-100-p-93-iso8859-1
> -adobe-utopia-bold-i-normal--19-140-100-100-p-109-iso10646-1
> -adobe-utopia-bold-i-normal--19-140-100-100-p-109-iso8859-1
> -adobe-utopia-bold-i-normal--19-180-75-75-p-105-iso10646-1
> -adobe-utopia-bold-i-normal--25-180-100-100-p-139-iso10646-1
> -adobe-utopia-bold-i-normal--25-180-100-100-p-139-iso8859-1
> -adobe-utopia-bold-i-normal--25-240-75-75-p-140-iso10646-1
> -adobe-utopia-bold-i-normal--33-240-100-100-p-186-iso10646-1
> -adobe-utopia-bold-i-normal--33-240-100-100-p-186-iso8859-1
>
> And running your program with -adobe-utopia-bold-i-normal--0-0-0-0-p-0-iso8859-1
> produces:
>
> font '-adobe-utopia-bold-i-normal--0-0-0-0-p-0-iso8859-1' loaded as '-adobe-utopia-bold-i-normal--17-120-100-100-p-94-iso8859-1'
>
> Your X behaves pretty strange; I can't explain this just now.
>
> Also, what happens if you specify default font via ~/.Xdefaults
> and run with emacs -q (small 'q', not 'Q')?

Hi. Thank you for looking.

First off, setting the font in the .Xresources has the same effect as
doing it with the lisp snippet on the commandline.

My font configuration is stock from Debian; I didn't mess with it. The
adobe->urw conversion probably comes from
/usr/share/fonts/X11/Type1/fonts.alias which on my machine contains many
lines such as

 -adobe-courier-medium-o-normal--0-0-0-0-p-0-iso8859-2 "-urw-nimbus mono l-regular-o-normal--0-0-0-0-p-0-iso8859-2"

That file contains only aliases from scalable fonts to other scalable
fonts, but it's probably the cause in any case.

I do suspect that the adobe->urw conversion is not important here. What
IS important is that we ask Emacs for a particular size 11 font, it
gives back a size 0 (scalable) font, and when you ask X for that size 0
font, you get back a size 17 font. You are seeing this too, apparently.
You asked X for a scalable font

 -adobe-utopia-bold-i-normal--0-0-0-0-p-0-iso8859-1

and X gave you a size-17 font

 -adobe-utopia-bold-i-normal--17-120-100-100-p-94-iso8859-1

So in your case, if you ask Emacs for a size-11 -adobe-utopia-... font
then does Emacs pick a scalable font for you? If so, I would expect
things to not look very good, since you'll get a 17->11 scaling.

The questions for Emacs are:

- Should the candidate font list stay consistent as Emacs runs?
- Should the candidate font list contain any scalable fonts?

The answer to the 1st question is probably "yes". Do you know the answer
to the 2nd? I can keep probing if you tell me which observed behavior is
wrong.

My suspicion is that there's some race condition here that is tickled by
my window manager. I'm using notion, which is a niche WM, so this
wouldn't be widely reported. But even so, it has been working fine for
many years, only regressing 6 months ago or so.

Thanks again

bug#19117: 25.0.50; emacs on x11 chooses different fonts for the same face sometimes

[Jan Djärv]

I'm travelling, so it will be some days before I can look at this. 

      Jan D. 


18 dec 2014 kl. 17:24 skrev Eli Zaretskii <eliz@gnu.org>:

>> From: Dima Kogan <dima@secretsauce.net>
>> Cc: 19117@debbugs.gnu.org
>> Date: Tue, 16 Dec 2014 22:57:41 -0800
>> 
>> I just looked a bit more.
> 
> Dmitry, Jan, could you please help Dima here?

bug#19117: 25.0.50; emacs on x11 chooses different fonts for the same face sometimes

[Dmitry Antipov]

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

On 12/20/2014 01:46 AM, Dima Kogan wrote:

> So in your case, if you ask Emacs for a size-11 -adobe-utopia-... font
> then does Emacs pick a scalable font for you? If so, I would expect
> things to not look very good, since you'll get a 17->11 scaling.

Hm...below are my results of running:

emacs -Q --font "-adobe-utopia-bold-i-normal--11-120-100-100-p-94-iso8859-1"

and

emacs -Q --font "-adobe-utopia-bold-i-normal--17-120-100-100-p-94-iso8859-1"

respectively. May be *-11-* is just too small to see glitches on my screen,
but I can't complain about ugly rendering.

> - Should the candidate font list stay consistent as Emacs runs?

IIUC yes, unless you install/remove new/existing fonts and
do 'xset +/-fp /path/to/new|old/fonts' as Emacs runs.

> - Should the candidate font list contain any scalable fonts?

I'll try to check.

> My suspicion is that there's some race condition here that is tickled by
> my window manager. I'm using notion, which is a niche WM, so this
> wouldn't be widely reported. But even so, it has been working fine for
> many years, only regressing 6 months ago or so.

If you suspect the WM-side tricks, try to investigate this issue under twm
as well.

Dmitry


[-- Attachment #2: emacs-adobe-11.png --]
[-- Type: image/png, Size: 39342 bytes --]

[-- Attachment #3: emacs-adobe-17.png --]
[-- Type: image/png, Size: 78463 bytes --]

bug#19117: 25.0.50; emacs on x11 chooses different fonts for the same face sometimes

[Jan Djärv]

Den 2014-12-19 23:46, Dima Kogan skrev:

>
> The questions for Emacs are:
>
> - Should the candidate font list stay consistent as Emacs runs?
> - Should the candidate font list contain any scalable fonts?
>
> The answer to the 1st question is probably "yes". Do you know the answer
> to the 2nd? I can keep probing if you tell me which observed behavior is
> wrong.

The answer to the first one is no.  Or rather, it is out of Emacs control.
Since you are using non-client fonts, the list is determined by the X- or 
font-server.  The fact that you can get two matches suggests that your font 
spec is underspecified, if your goal is to get just one specific font.

I see no bug here.

	Jan D.

bug#19117: 25.0.50; emacs on x11 chooses different fonts for the same face sometimes

[Dima Kogan]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=utf-8, Size: 3305 bytes --]


Jan Djärv <jan.h.d@swipnet.se> writes:

> Den 2014-12-19 23:46, Dima Kogan skrev:
>
>> The questions for Emacs are:
>>
>> - Should the candidate font list stay consistent as Emacs runs?
>> - Should the candidate font list contain any scalable fonts?
>>
>> The answer to the 1st question is probably "yes". Do you know the answer
>> to the 2nd? I can keep probing if you tell me which observed behavior is
>> wrong.
>
> The answer to the first one is no.  Or rather, it is out of Emacs control.
> Since you are using non-client fonts, the list is determined by the X- or 
> font-server.  The fact that you can get two matches suggests that your font 
> spec is underspecified, if your goal is to get just one specific font.
>
> I see no bug here.

Hi. The observed behavior (same face getting different fonts of varying
ugliness over time) cannot be considered correct, but I cannot yet say
where the bad behavior lies. It could be in Emacs or X or the Debian
font configuration.

Looking at this some more, I'm now more confused than when I started.
Any enlightenment is welcome.


To summarize the earlier findings:

- Emacs is asked for a font that matches the spec

 #<font-spec nil nil nil nil nil medium italic normal 11 nil nil nil nil>

- The font chosen by emacs can vary here. When it chooses the "ugly"
  path, it picks

 #<font-entity x adobe courier ## iso8859-1 medium i normal 0 0 0 0 nil>

- When X is asked for a corresponding font

 -adobe-courier-medium-i-normal-*-0-0-0-0-p-0-iso8859-1

it actually loads

 -urw-nimbus mono l-regular-o-normal--17-120-100-100-p-100-iso8859-1

Note that we originally sought an 11-px font, but got a 17-px one
instead.


OK. So the hypothesis is that the 11-px version of that 17-px urw font
does not exist, hence it's down-scaling the 17-px version. However,
apparently I can do

 xfontsel -pattern "-urw-nimbus mono l-regular-o-normal--11-*-*-*-p-70-iso8859-1"

and it works. I.e. xfontsel doesn't complain that the font doesn't
exist, it simply shows me the ugly 11-px font I'm complaining about.
I don't think this font actually exists (it's ugly; a human would not
draw it). However if I ask the font system whether it exists or not, it
unambiguously says yes:

 $ xlsfonts -fn "-urw-nimbus mono l-regular-o-normal--11-*-*-*-p-70-iso8859-1"

 -urw-nimbus mono l-regular-o-normal--11-79-100-100-p-70-iso8859-1


However, when I use a wildcard for the font size, (so * instead of 11),
it reports that the 11-px font does NOT exist, but a 17-px one does:

 $ xlsfonts -fn "-urw-nimbus mono l-regular-o-normal--*-*-*-*-p-70-iso8859-1"

 -urw-nimbus mono l-regular-o-normal--17-120-100-100-p-70-iso8859-1

This 17-px font looks good in xfontsel, but it's obviously bigger than
the 11-px one.

It looks like the font specs have a pixel-size and a point-size, (11-79)
and (17-120), with the ratio staying roughly constant in either case, so
the 11-79 is almost certainly a scaled version of 17-120. Problem is
that the rendering is based on the px-size only, so it looks like crap.
Is this how X fonts are supposed to work? What if Emacs treated scalable
fonts by looking at the actual px-sizes that were available (17 in this
case) and only evaluating those? Would that be a reasonable thing to do?
I can work on an implementation if it is.

Thanks

bug#19117: 25.0.50; emacs on x11 chooses different fonts for the same face sometimes

[Stefan Monnier]

>  $ xlsfonts -fn "-urw-nimbus mono l-regular-o-normal--11-*-*-*-p-70-iso8859-1"
>  -urw-nimbus mono l-regular-o-normal--11-79-100-100-p-70-iso8859-1
> However, when I use a wildcard for the font size, (so * instead of 11),
> it reports that the 11-px font does NOT exist, but a 17-px one does:
>  $ xlsfonts -fn "-urw-nimbus mono l-regular-o-normal--*-*-*-*-p-70-iso8859-1"
>  -urw-nimbus mono l-regular-o-normal--17-120-100-100-p-70-iso8859-1

That's indeed the expected behavior for server-side scaled bitmap fonts.

> What if Emacs treated scalable fonts by looking at the actual px-sizes
> that were available (17 in this case) and only evaluating those?

IIUC if `scalable-fonts-allowed' is set to nil, that's what should happen.


        Stefan

bug#19117: 25.0.50; emacs on x11 chooses different fonts for the same face sometimes

[Dima Kogan]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> IIUC if `scalable-fonts-allowed' is set to nil, that's what should happen.

This was a VERY useful comment. I do not touch scalable-fonts-allowed,
so it should be the default (i.e. nil), meaning that I should already be
disallowing these fonts. Looking at it deeper I see that the font list
is obtained by xfont_list_pattern() in xfont.c. This is an XListFonts()
call, with emacs then filtering the resulting list in various ways.

I see XListFonts() return consistent results, but the post-filtering of
the list is inconsistent. Specifically, the inconsistent piece is
exactly the scalable_fonts_allowed check in xfont_list_pattern():

https://github.com/emacs-mirror/emacs/blob/162660c3639a68a7b71439bdd713d54f940274b8/src/xfont.c#L416

When we get here, scalable_fonts_allowed is sometimes non-nil, which is
the direct cause of my problem. Forcefully removing that check and
always assuming that it IS nil makes this bug go away.

I haven't verified this yet, but presumably scalable_fonts_allowed is
getting set to t in realize_basic_faces() in xfaces.c:

https://github.com/emacs-mirror/emacs/blob/162660c3639a68a7b71439bdd713d54f940274b8/src/xfaces.c#L5236

Any comment on that? That bit of code was there since 2001, so
presumably it's reasonable. Anybody know more?

bug#19117: 25.0.50; emacs on x11 chooses different fonts for the same face sometimes

[Dima Kogan]

Hi. I looked at this some more. The scalable-fonts-allowed is indeed
being set to t in realize_basic_faces().

As emacs runs, the font lists are generated (xfont_list() called)
multiple times, and there are many code paths that end up doing that.
Some code paths go through realize_basic_faces(), and thus allow
scalable fonts, and some code paths do NOT go through that function.
Each time a new emacs frame is created, xfont_list() is called multiple
times, and if the last time it is called, the code path went through
realize_basic_faces(), then I get the wrong fonts chosen for that frame.
This is not consistent, so sometimes I see the bad behavior.

What's the best route from here? Options:

1. Look at all the (very numerous, and complicated) code paths, and set
it up such that realize_basic_faces() is never the last one. I don't
know enough to do this (yet), and can't tell if this is even appropriate

2. Do not set scalable-fonts-allowed to t in realize_basic_faces(). Does
anybody know why this is desirable?

3. Never choose scalable X fonts, regardless of scalable-fonts-allowed.
If the X scalable fonts are simply scaled bitmaps, then the scaled
result will never look particularly good, so choosing these is not
useful. This almost certainly doesn't apply to the other font backends
(xft, etc)

bug#19117: 25.0.50; emacs on x11 chooses different fonts for the same face sometimes

[Stefan Monnier]

> 2. Do not set scalable-fonts-allowed to t in realize_basic_faces(). Does
> anybody know why this is desirable?

Have you looked at the "git blame" output to try and see when/why we do that?


        Stefan

bug#19117: 25.0.50; emacs on x11 chooses different fonts for the same face sometimes

[Dima Kogan]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> 2. Do not set scalable-fonts-allowed to t in realize_basic_faces(). Does
>> anybody know why this is desirable?
>
> Have you looked at the "git blame" output to try and see when/why we do that?

I did look at earlier it without any particular enlightenment. But I
just looked at it again, and it IS helpful, actually. This is all from
2001, written by Gerd Moellmann. Here are the changes in chronological
order

https://github.com/emacs-mirror/emacs/commit/702a1e8e
https://github.com/emacs-mirror/emacs/commit/441bf856
https://github.com/emacs-mirror/emacs/commit/eeffb293

The logic was using a realizing_default_face_p variable (changed in the
last commit), anad roughtly it did this:

x_face_list_fonts()
{
  get_all_fonts();
  for(fonts)
  {
      if(!font_scalable)
          add to list;
  }

  if (realizing_default_face_p
      && no fonts in list
      && some were scalable)
  {
      for(fonts)
      {
          if(!font_scalable)
              add to list;
      }
  }
  ...
}

So we only added the scalable fonts if no other fonts were found. The
new logic does not do this; it always adds the scalable fonts. The
docstring of scalable-fonts-allowed further indicates that the original
intent was a last-resort like before:

 Note that if value is nil, a scalable font might still be used, if no
 other font of the appropriate family and registry is available.

I will patch xfont_list_pattern() to accept a 3rd type of
scalable-fonts-allowed: 'fallback where we add these fonts only if no
others are available, and in realize_basic_faces() I'll use THAT value.
Does that sound reasonable?

bug#19117: 25.0.50; emacs on x11 chooses different fonts for the same face sometimes

[Eli Zaretskii]

> From: Dima Kogan <dima@secretsauce.net>
> Date: Tue, 30 Dec 2014 10:33:17 -0800
> Cc: 19117@debbugs.gnu.org
> 
> I will patch xfont_list_pattern() to accept a 3rd type of
> scalable-fonts-allowed: 'fallback where we add these fonts only if no
> others are available, and in realize_basic_faces() I'll use THAT value.
> Does that sound reasonable?

Please note that on MS-Windows the scalable fonts were always used in
this scenario, back with that code, and there was a comment explaining
that otherwise too few fonts will be available on Windows.

bug#19117: 25.0.50; emacs on x11 chooses different fonts for the same face sometimes

[Dima Kogan]

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

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Dima Kogan <dima@secretsauce.net>
>> Date: Tue, 30 Dec 2014 10:33:17 -0800
>> Cc: 19117@debbugs.gnu.org
>> 
>> I will patch xfont_list_pattern() to accept a 3rd type of
>> scalable-fonts-allowed: 'fallback where we add these fonts only if no
>> others are available, and in realize_basic_faces() I'll use THAT value.
>> Does that sound reasonable?
>
> Please note that on MS-Windows the scalable fonts were always used in
> this scenario, back with that code, and there was a comment explaining
> that otherwise too few fonts will be available on Windows.

Hi. I'm attaching patches. The first one does this:

  scalable font logic now matches the docs and is more consistent

  A nil value of scalable-fonts-allowed allows scalable fonts if no others
  were found.  This is the previously-documented behavior.

  realize_basic_faces() no longer forces scalable-fonts-allowed to t.  If
  nil, some fonts will always be found even if only scalable fonts are
  available, as set by the other part of this patch.

This is simpler than what I proposed earlier, since it does not
introduce a third setting to scalable-fonts-allowed: the default nil
value is sufficient. The patch mimics the older behavior to loop through
the whole font list a second time if no non-scalable fonts were found
and some scalable fonts WERE found, but were skipped the first time
around due to scalable-fonts-allowed. This fixes my bug. The patch
touches many lines, but almost all of it is indentation.


The second patch is mostly unrelated. It removes some redundant code.

I don't believe the behavior on Windows should be affected at all, but I
have not verified this.



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-scalable-font-logic-now-matches-the-docs-and-is-more.patch --]
[-- Type: text/x-diff, Size: 8924 bytes --]

From af0ec4452c5157e526af3c1190c3e8ba6f72c982 Mon Sep 17 00:00:00 2001
From: Dima Kogan <dima@secretsauce.net>
Date: Tue, 30 Dec 2014 20:05:11 -0800
Subject: [PATCH 1/2] scalable font logic now matches the docs and is more
 consistent

A nil value of scalable-fonts-allowed allows scalable fonts if no others
were found.  This is the previously-documented behavior.

realize_basic_faces() no longer forces scalable-fonts-allowed to t.  If
nil, some fonts will always be found even if only scalable fonts are
available, as set by the other part of this patch.
---
 src/xfaces.c |   3 --
 src/xfont.c  | 172 +++++++++++++++++++++++++++++++++--------------------------
 2 files changed, 95 insertions(+), 80 deletions(-)

diff --git a/src/xfaces.c b/src/xfaces.c
index 446107e..0aa9968 100644
--- a/src/xfaces.c
+++ b/src/xfaces.c
@@ -5228,12 +5228,10 @@ static bool
 realize_basic_faces (struct frame *f)
 {
   bool success_p = 0;
-  ptrdiff_t count = SPECPDL_INDEX ();
 
   /* Block input here so that we won't be surprised by an X expose
      event, for instance, without having the faces set up.  */
   block_input ();
-  specbind (Qscalable_fonts_allowed, Qt);
 
   if (realize_default_face (f))
     {
@@ -5267,7 +5265,6 @@ realize_basic_faces (struct frame *f)
       success_p = 1;
     }
 
-  unbind_to (count, Qnil);
   unblock_input ();
   return success_p;
 }
diff --git a/src/xfont.c b/src/xfont.c
index 06a4479..c6a3f2b 100644
--- a/src/xfont.c
+++ b/src/xfont.c
@@ -388,94 +388,112 @@ xfont_list_pattern (Display *display, const char *pattern,
       Lisp_Object *props = XVECTOR (xfont_scratch_props)->contents;
       Lisp_Object scripts = Qnil, entity = Qnil;
 
+      /* We take two passes over the font list.  The second pass is
+         taken only if scalable-fonts-allowed is nil, and only
+         scalable fonts were found.
+       */
+      int i_pass;
+      bool skipped_some_scalable_fonts = false;
+
       for (i = 0; i < ASIZE (xfont_scratch_props); i++)
 	ASET (xfont_scratch_props, i, Qnil);
       for (i = 0; i < num_fonts; i++)
 	indices[i] = names[i];
       qsort (indices, num_fonts, sizeof (char *), compare_font_names);
 
-      for (i = 0; i < num_fonts; i++)
-	{
-	  ptrdiff_t len;
+      for (i_pass = 0; i_pass < 2; i_pass++)
+        {
+          for (i = 0; i < num_fonts; i++)
+            {
+              ptrdiff_t len;
 
-	  if (i > 0 && xstrcasecmp (indices[i - 1], indices[i]) == 0)
-	    continue;
-	  if (NILP (entity))
-	    entity = font_make_entity ();
-	  len = xfont_decode_coding_xlfd (indices[i], -1, buf);
-	  if (font_parse_xlfd (buf, len, entity) < 0)
-	    continue;
-	  ASET (entity, FONT_TYPE_INDEX, Qx);
-	  /* Avoid auto-scaled fonts.  */
-	  if (INTEGERP (AREF (entity, FONT_DPI_INDEX))
-	      && INTEGERP (AREF (entity, FONT_AVGWIDTH_INDEX))
-	      && XINT (AREF (entity, FONT_DPI_INDEX)) != 0
-	      && XINT (AREF (entity, FONT_AVGWIDTH_INDEX)) == 0)
-	    continue;
-	  /* Avoid not-allowed scalable fonts.  */
-	  if (NILP (Vscalable_fonts_allowed))
-	    {
-	      int size = 0;
+              if (i > 0 && xstrcasecmp (indices[i - 1], indices[i]) == 0)
+                continue;
+              if (NILP (entity))
+                entity = font_make_entity ();
+              len = xfont_decode_coding_xlfd (indices[i], -1, buf);
+              if (font_parse_xlfd (buf, len, entity) < 0)
+                continue;
+              ASET (entity, FONT_TYPE_INDEX, Qx);
+              /* Avoid auto-scaled fonts.  */
+              if (INTEGERP (AREF (entity, FONT_DPI_INDEX))
+                  && INTEGERP (AREF (entity, FONT_AVGWIDTH_INDEX))
+                  && XINT (AREF (entity, FONT_DPI_INDEX)) != 0
+                  && XINT (AREF (entity, FONT_AVGWIDTH_INDEX)) == 0)
+                continue;
+              /* Avoid not-allowed scalable fonts.  */
+              if (NILP (Vscalable_fonts_allowed))
+                {
+                  int size = 0;
 
-	      if (INTEGERP (AREF (entity, FONT_SIZE_INDEX)))
-		size = XINT (AREF (entity, FONT_SIZE_INDEX));
-	      else if (FLOATP (AREF (entity, FONT_SIZE_INDEX)))
-		size = XFLOAT_DATA (AREF (entity, FONT_SIZE_INDEX));
-	      if (size == 0)
-		continue;
-	    }
-	  else if (CONSP (Vscalable_fonts_allowed))
-	    {
-	      Lisp_Object tail, elt;
+                  if (INTEGERP (AREF (entity, FONT_SIZE_INDEX)))
+                    size = XINT (AREF (entity, FONT_SIZE_INDEX));
+                  else if (FLOATP (AREF (entity, FONT_SIZE_INDEX)))
+                    size = XFLOAT_DATA (AREF (entity, FONT_SIZE_INDEX));
+                  if (size == 0 && i_pass == 0)
+                    {
+                      skipped_some_scalable_fonts = true;
+                      continue;
+                    }
+                }
+              else if (CONSP (Vscalable_fonts_allowed))
+                {
+                  Lisp_Object tail, elt;
 
-	      for (tail = Vscalable_fonts_allowed; CONSP (tail);
-		   tail = XCDR (tail))
-		{
-		  elt = XCAR (tail);
-		  if (STRINGP (elt)
-		      && fast_c_string_match_ignore_case (elt, indices[i],
-							  len) >= 0)
-		    break;
-		}
-	      if (! CONSP (tail))
-		continue;
-	    }
+                  for (tail = Vscalable_fonts_allowed; CONSP (tail);
+                       tail = XCDR (tail))
+                    {
+                      elt = XCAR (tail);
+                      if (STRINGP (elt)
+                          && fast_c_string_match_ignore_case (elt, indices[i],
+                                                              len) >= 0)
+                        break;
+                    }
+                  if (! CONSP (tail))
+                    continue;
+                }
 
-	  /* Avoid fonts of invalid registry.  */
-	  if (NILP (AREF (entity, FONT_REGISTRY_INDEX)))
-	    continue;
+              /* Avoid fonts of invalid registry.  */
+              if (NILP (AREF (entity, FONT_REGISTRY_INDEX)))
+                continue;
 
-	  /* Update encoding and repertory if necessary.  */
-	  if (! EQ (registry, AREF (entity, FONT_REGISTRY_INDEX)))
-	    {
-	      registry = AREF (entity, FONT_REGISTRY_INDEX);
-	      if (font_registry_charsets (registry, &encoding, &repertory) < 0)
-		encoding = NULL;
-	    }
-	  if (! encoding)
-	    /* Unknown REGISTRY, not supported.  */
-	    continue;
-	  if (repertory)
-	    {
-	      if (NILP (script)
-		  || xfont_chars_supported (chars, NULL, encoding, repertory))
-		list = Fcons (entity, list), entity = Qnil;
-	      continue;
-	    }
-	  if (memcmp (props, aref_addr (entity, FONT_FOUNDRY_INDEX),
-		      word_size * 7)
-	      || ! EQ (AREF (entity, FONT_SPACING_INDEX), props[7]))
-	    {
-	      vcopy (xfont_scratch_props, 0,
-		     aref_addr (entity, FONT_FOUNDRY_INDEX), 7);
-	      ASET (xfont_scratch_props, 7, AREF (entity, FONT_SPACING_INDEX));
-	      scripts = xfont_supported_scripts (display, indices[i],
-						 xfont_scratch_props, encoding);
-	    }
-	  if (NILP (script)
-	      || ! NILP (Fmemq (script, scripts)))
-	    list = Fcons (entity, list), entity = Qnil;
-	}
+              /* Update encoding and repertory if necessary.  */
+              if (! EQ (registry, AREF (entity, FONT_REGISTRY_INDEX)))
+                {
+                  registry = AREF (entity, FONT_REGISTRY_INDEX);
+                  if (font_registry_charsets (registry, &encoding, &repertory) < 0)
+                    encoding = NULL;
+                }
+              if (! encoding)
+                /* Unknown REGISTRY, not supported.  */
+                continue;
+              if (repertory)
+                {
+                  if (NILP (script)
+                      || xfont_chars_supported (chars, NULL, encoding, repertory))
+                    list = Fcons (entity, list), entity = Qnil;
+                  continue;
+                }
+              if (memcmp (props, aref_addr (entity, FONT_FOUNDRY_INDEX),
+                          word_size * 7)
+                  || ! EQ (AREF (entity, FONT_SPACING_INDEX), props[7]))
+                {
+                  vcopy (xfont_scratch_props, 0,
+                         aref_addr (entity, FONT_FOUNDRY_INDEX), 7);
+                  ASET (xfont_scratch_props, 7, AREF (entity, FONT_SPACING_INDEX));
+                  scripts = xfont_supported_scripts (display, indices[i],
+                                                     xfont_scratch_props, encoding);
+                }
+              if (NILP (script)
+                  || ! NILP (Fmemq (script, scripts)))
+                list = Fcons (entity, list), entity = Qnil;
+            }
+
+          /* We skip the second pass unless we really need it.  */
+          if (!(NILP (list)                      /* No fonts found on the first pass */
+                && skipped_some_scalable_fonts)) /* and we skipped some scalable ones.  */
+            break;
+        }
       XFreeFontNames (names);
     }
 
-- 
2.1.3


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-removed-unused-variable-assignment.patch --]
[-- Type: text/x-diff, Size: 642 bytes --]

From 282bee4cfe6775063358c616e966593501620b54 Mon Sep 17 00:00:00 2001
From: Dima Kogan <dima@secretsauce.net>
Date: Tue, 30 Dec 2014 20:05:58 -0800
Subject: [PATCH 2/2] removed unused variable assignment

---
 src/font.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/font.c b/src/font.c
index 70e6316..dc821dd 100644
--- a/src/font.c
+++ b/src/font.c
@@ -2160,7 +2160,6 @@ font_score (Lisp_Object entity, Lisp_Object *spec_prop)
       }
 
   /* Score the size.  Maximum difference is 127.  */
-  i = FONT_SIZE_INDEX;
   if (! NILP (spec_prop[FONT_SIZE_INDEX])
       && XINT (AREF (entity, FONT_SIZE_INDEX)) > 0)
     {
-- 
2.1.3

bug#19117: 25.0.50; emacs on x11 chooses different fonts for the same face sometimes

[Eli Zaretskii]

> From: Dima Kogan <dima@secretsauce.net>
> Cc: monnier@iro.umontreal.ca, 19117@debbugs.gnu.org
> Date: Tue, 30 Dec 2014 20:06:38 -0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> From: Dima Kogan <dima@secretsauce.net>
> >> Date: Tue, 30 Dec 2014 10:33:17 -0800
> >> Cc: 19117@debbugs.gnu.org
> >> 
> >> I will patch xfont_list_pattern() to accept a 3rd type of
> >> scalable-fonts-allowed: 'fallback where we add these fonts only if no
> >> others are available, and in realize_basic_faces() I'll use THAT value.
> >> Does that sound reasonable?
> >
> > Please note that on MS-Windows the scalable fonts were always used in
> > this scenario, back with that code, and there was a comment explaining
> > that otherwise too few fonts will be available on Windows.
> 
> Hi. I'm attaching patches. The first one does this:
> 
>   scalable font logic now matches the docs and is more consistent
> 
>   A nil value of scalable-fonts-allowed allows scalable fonts if no others
>   were found.  This is the previously-documented behavior.
> 
>   realize_basic_faces() no longer forces scalable-fonts-allowed to t.  If
>   nil, some fonts will always be found even if only scalable fonts are
>   available, as set by the other part of this patch.
> 
> This is simpler than what I proposed earlier, since it does not
> introduce a third setting to scalable-fonts-allowed: the default nil
> value is sufficient. The patch mimics the older behavior to loop through
> the whole font list a second time if no non-scalable fonts were found
> and some scalable fonts WERE found, but were skipped the first time
> around due to scalable-fonts-allowed. This fixes my bug. The patch
> touches many lines, but almost all of it is indentation.
> 
> 
> The second patch is mostly unrelated. It removes some redundant code.
> 
> I don't believe the behavior on Windows should be affected at all, but I
> have not verified this.

Thanks.

I don't really know how to test these changes on Windows, since most
of the changes are in xfont.c, and the equivalent w32font.c has no
similar code that I could identify.  And since I'm not an expert on
fonts, I'm stuck.

I've built Emacs with your changes and verified that x-list-fonts
still returns the same number of fonts, and the buffer font selection
dialog still seems to offer the same list of fonts AFAICS.

Can you suggest any further way of testing the patch?

bug#19117: 25.0.50; emacs on x11 chooses different fonts for the same face sometimes

[Dima Kogan]

Eli Zaretskii <eliz@gnu.org> writes:

> I don't really know how to test these changes on Windows, since most
> of the changes are in xfont.c, and the equivalent w32font.c has no
> similar code that I could identify.  And since I'm not an expert on
> fonts, I'm stuck.
>
> I've built Emacs with your changes and verified that x-list-fonts
> still returns the same number of fonts, and the buffer font selection
> dialog still seems to offer the same list of fonts AFAICS.
>
> Can you suggest any further way of testing the patch?

Hi. The patch changes xfont.c (X11-only I believe) and xfaces.c (applies
to Windows as well, I believe).

In xfaces.c it changes the value of scalable-fonts-allowed in some
cases, but that variable is ONLY used in xfont.c. Thus any system that
does not use xfont.c should be unaffected by scalable-fonts-allowed, and
thus this patch.

So I guess the test you already ran is the best you can do: the patch
shouldn't change anything, and you have observed this under nominal
conditions.

bug#19117: 25.0.50; emacs on x11 chooses different fonts for the same face sometimes

[Dima Kogan]

This is a gentle ping. Is there anything more I can do to get this
merged?

Thanks!

bug#19117: 25.0.50; emacs on x11 chooses different fonts for the same face sometimes

[Stefan Monnier]

> This is a gentle ping. Is there anything more I can do to get this
> merged?

I could install it, but I really have no idea whether your patch does
the right thing, because I'm too unfamiliar with this code.
Jan?  Handa?


        Stefan

bug#19117: 25.0.50; emacs on x11 chooses different fonts for the same face sometimes

[handa]

In article <jwvfvamhoh2.fsf-monnier+emacsbugs@gnu.org>, Stefan Monnier <monnier@iro.umontreal.ca> writes:

> > This is a gentle ping. Is there anything more I can do to get this
> > merged?

> I could install it, but I really have no idea whether your patch does
> the right thing, because I'm too unfamiliar with this code.
> Jan?  Handa?

I'm currently on a trip, and can't study the patch until
next week, sorry.

---
K. Handa
handa@gnu.org

bug#19117: 25.0.50; emacs on x11 chooses different fonts for the same face sometimes

[Jan D.]

Stefan Monnier skrev den 2015-02-03 18:53:
>> This is a gentle ping. Is there anything more I can do to get this
>> merged?
>
> I could install it, but I really have no idea whether your patch does
> the right thing, because I'm too unfamiliar with this code.
> Jan?  Handa?
>

The patch looks bigger than it is because lot of the diff is just 
increased indentation.  I guess it looks OK.  These old fonts are 
deprecated anyway, so if it does the right thing for the OP, I guess we 
can install it.

But the comment here:

+          /* We skip the second pass unless we really need it.  */
+          if (!(NILP (list)                      /* No fonts found on 
the first pass */
+                && skipped_some_scalable_fonts)) /* and we skipped some 
scalable ones.  */
+            break;


looks like it really should be  /* Fonts found on the first pass */

	Jan D.

bug#19117: 25.0.50; emacs on x11 chooses different fonts for the same face sometimes

[Dima Kogan]

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

Jan D. <jan.h.d@swipnet.se> writes:

>
> But the comment here:
>
> +          /* We skip the second pass unless we really need it.  */
> +          if (!(NILP (list)                      /* No fonts found on 
> the first pass */
> +                && skipped_some_scalable_fonts)) /* and we skipped some 
> scalable ones.  */
> +            break;
>
>
> looks like it really should be  /* Fonts found on the first pass */

There was an ! there, so it made it unclear what the comment is
referring to. Attaching new patches that apply against the current
master, and have a clarified comment in that place


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-scalable-font-logic-now-matches-the-docs-and-is-more.patch --]
[-- Type: text/x-diff, Size: 9005 bytes --]

From 4e895fb38c84342021b602dd4a9830b55fe56a58 Mon Sep 17 00:00:00 2001
From: Dima Kogan <dima@secretsauce.net>
Date: Tue, 30 Dec 2014 20:05:11 -0800
Subject: [PATCH 1/2] scalable font logic now matches the docs and is more
 consistent

A nil value of scalable-fonts-allowed allows scalable fonts if no others
were found.  This is the previously-documented behavior.

realize_basic_faces() no longer forces scalable-fonts-allowed to t.  If
nil, some fonts will always be found even if only scalable fonts are
available, as set by the other part of this patch.
---
 src/xfaces.c |   3 --
 src/xfont.c  | 173 +++++++++++++++++++++++++++++++++--------------------------
 2 files changed, 96 insertions(+), 80 deletions(-)

diff --git a/src/xfaces.c b/src/xfaces.c
index 6e01ab0..d0fe872 100644
--- a/src/xfaces.c
+++ b/src/xfaces.c
@@ -5141,12 +5141,10 @@ static bool
 realize_basic_faces (struct frame *f)
 {
   bool success_p = false;
-  ptrdiff_t count = SPECPDL_INDEX ();
 
   /* Block input here so that we won't be surprised by an X expose
      event, for instance, without having the faces set up.  */
   block_input ();
-  specbind (Qscalable_fonts_allowed, Qt);
 
   if (realize_default_face (f))
     {
@@ -5180,7 +5178,6 @@ realize_basic_faces (struct frame *f)
       success_p = true;
     }
 
-  unbind_to (count, Qnil);
   unblock_input ();
   return success_p;
 }
diff --git a/src/xfont.c b/src/xfont.c
index 3a0f5e3..5f1c61c 100644
--- a/src/xfont.c
+++ b/src/xfont.c
@@ -388,94 +388,113 @@ xfont_list_pattern (Display *display, const char *pattern,
       Lisp_Object *props = XVECTOR (xfont_scratch_props)->contents;
       Lisp_Object scripts = Qnil, entity = Qnil;
 
+      /* We take two passes over the font list.  The second pass is
+         taken only if scalable-fonts-allowed is nil, and only
+         scalable fonts were found.
+       */
+      int i_pass;
+      bool skipped_some_scalable_fonts = false;
+
       for (i = 0; i < ASIZE (xfont_scratch_props); i++)
 	ASET (xfont_scratch_props, i, Qnil);
       for (i = 0; i < num_fonts; i++)
 	indices[i] = names[i];
       qsort (indices, num_fonts, sizeof (char *), compare_font_names);
 
-      for (i = 0; i < num_fonts; i++)
-	{
-	  ptrdiff_t len;
+      for (i_pass = 0; i_pass < 2; i_pass++)
+        {
+          for (i = 0; i < num_fonts; i++)
+            {
+              ptrdiff_t len;
 
-	  if (i > 0 && xstrcasecmp (indices[i - 1], indices[i]) == 0)
-	    continue;
-	  if (NILP (entity))
-	    entity = font_make_entity ();
-	  len = xfont_decode_coding_xlfd (indices[i], -1, buf);
-	  if (font_parse_xlfd (buf, len, entity) < 0)
-	    continue;
-	  ASET (entity, FONT_TYPE_INDEX, Qx);
-	  /* Avoid auto-scaled fonts.  */
-	  if (INTEGERP (AREF (entity, FONT_DPI_INDEX))
-	      && INTEGERP (AREF (entity, FONT_AVGWIDTH_INDEX))
-	      && XINT (AREF (entity, FONT_DPI_INDEX)) != 0
-	      && XINT (AREF (entity, FONT_AVGWIDTH_INDEX)) == 0)
-	    continue;
-	  /* Avoid not-allowed scalable fonts.  */
-	  if (NILP (Vscalable_fonts_allowed))
-	    {
-	      int size = 0;
+              if (i > 0 && xstrcasecmp (indices[i - 1], indices[i]) == 0)
+                continue;
+              if (NILP (entity))
+                entity = font_make_entity ();
+              len = xfont_decode_coding_xlfd (indices[i], -1, buf);
+              if (font_parse_xlfd (buf, len, entity) < 0)
+                continue;
+              ASET (entity, FONT_TYPE_INDEX, Qx);
+              /* Avoid auto-scaled fonts.  */
+              if (INTEGERP (AREF (entity, FONT_DPI_INDEX))
+                  && INTEGERP (AREF (entity, FONT_AVGWIDTH_INDEX))
+                  && XINT (AREF (entity, FONT_DPI_INDEX)) != 0
+                  && XINT (AREF (entity, FONT_AVGWIDTH_INDEX)) == 0)
+                continue;
+              /* Avoid not-allowed scalable fonts.  */
+              if (NILP (Vscalable_fonts_allowed))
+                {
+                  int size = 0;
 
-	      if (INTEGERP (AREF (entity, FONT_SIZE_INDEX)))
-		size = XINT (AREF (entity, FONT_SIZE_INDEX));
-	      else if (FLOATP (AREF (entity, FONT_SIZE_INDEX)))
-		size = XFLOAT_DATA (AREF (entity, FONT_SIZE_INDEX));
-	      if (size == 0)
-		continue;
-	    }
-	  else if (CONSP (Vscalable_fonts_allowed))
-	    {
-	      Lisp_Object tail, elt;
+                  if (INTEGERP (AREF (entity, FONT_SIZE_INDEX)))
+                    size = XINT (AREF (entity, FONT_SIZE_INDEX));
+                  else if (FLOATP (AREF (entity, FONT_SIZE_INDEX)))
+                    size = XFLOAT_DATA (AREF (entity, FONT_SIZE_INDEX));
+                  if (size == 0 && i_pass == 0)
+                    {
+                      skipped_some_scalable_fonts = true;
+                      continue;
+                    }
+                }
+              else if (CONSP (Vscalable_fonts_allowed))
+                {
+                  Lisp_Object tail, elt;
 
-	      for (tail = Vscalable_fonts_allowed; CONSP (tail);
-		   tail = XCDR (tail))
-		{
-		  elt = XCAR (tail);
-		  if (STRINGP (elt)
-		      && fast_c_string_match_ignore_case (elt, indices[i],
-							  len) >= 0)
-		    break;
-		}
-	      if (! CONSP (tail))
-		continue;
-	    }
+                  for (tail = Vscalable_fonts_allowed; CONSP (tail);
+                       tail = XCDR (tail))
+                    {
+                      elt = XCAR (tail);
+                      if (STRINGP (elt)
+                          && fast_c_string_match_ignore_case (elt, indices[i],
+                                                              len) >= 0)
+                        break;
+                    }
+                  if (! CONSP (tail))
+                    continue;
+                }
 
-	  /* Avoid fonts of invalid registry.  */
-	  if (NILP (AREF (entity, FONT_REGISTRY_INDEX)))
-	    continue;
+              /* Avoid fonts of invalid registry.  */
+              if (NILP (AREF (entity, FONT_REGISTRY_INDEX)))
+                continue;
 
-	  /* Update encoding and repertory if necessary.  */
-	  if (! EQ (registry, AREF (entity, FONT_REGISTRY_INDEX)))
-	    {
-	      registry = AREF (entity, FONT_REGISTRY_INDEX);
-	      if (font_registry_charsets (registry, &encoding, &repertory) < 0)
-		encoding = NULL;
-	    }
-	  if (! encoding)
-	    /* Unknown REGISTRY, not supported.  */
-	    continue;
-	  if (repertory)
-	    {
-	      if (NILP (script)
-		  || xfont_chars_supported (chars, NULL, encoding, repertory))
-		list = Fcons (entity, list), entity = Qnil;
-	      continue;
-	    }
-	  if (memcmp (props, aref_addr (entity, FONT_FOUNDRY_INDEX),
-		      word_size * 7)
-	      || ! EQ (AREF (entity, FONT_SPACING_INDEX), props[7]))
-	    {
-	      vcopy (xfont_scratch_props, 0,
-		     aref_addr (entity, FONT_FOUNDRY_INDEX), 7);
-	      ASET (xfont_scratch_props, 7, AREF (entity, FONT_SPACING_INDEX));
-	      scripts = xfont_supported_scripts (display, indices[i],
-						 xfont_scratch_props, encoding);
-	    }
-	  if (NILP (script)
-	      || ! NILP (Fmemq (script, scripts)))
-	    list = Fcons (entity, list), entity = Qnil;
-	}
+              /* Update encoding and repertory if necessary.  */
+              if (! EQ (registry, AREF (entity, FONT_REGISTRY_INDEX)))
+                {
+                  registry = AREF (entity, FONT_REGISTRY_INDEX);
+                  if (font_registry_charsets (registry, &encoding, &repertory) < 0)
+                    encoding = NULL;
+                }
+              if (! encoding)
+                /* Unknown REGISTRY, not supported.  */
+                continue;
+              if (repertory)
+                {
+                  if (NILP (script)
+                      || xfont_chars_supported (chars, NULL, encoding, repertory))
+                    list = Fcons (entity, list), entity = Qnil;
+                  continue;
+                }
+              if (memcmp (props, aref_addr (entity, FONT_FOUNDRY_INDEX),
+                          word_size * 7)
+                  || ! EQ (AREF (entity, FONT_SPACING_INDEX), props[7]))
+                {
+                  vcopy (xfont_scratch_props, 0,
+                         aref_addr (entity, FONT_FOUNDRY_INDEX), 7);
+                  ASET (xfont_scratch_props, 7, AREF (entity, FONT_SPACING_INDEX));
+                  scripts = xfont_supported_scripts (display, indices[i],
+                                                     xfont_scratch_props, encoding);
+                }
+              if (NILP (script)
+                  || ! NILP (Fmemq (script, scripts)))
+                list = Fcons (entity, list), entity = Qnil;
+            }
+
+          /* We skip the second pass unless we really need it.  */
+          if (!                                 /* Loop again if... */
+              (NILP (list)                      /*   No fonts found on the first pass */
+               && skipped_some_scalable_fonts)) /*   and we skipped some scalable ones.  */
+            break;
+        }
       XFreeFontNames (names);
     }
 
-- 
2.1.4


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-removed-unused-variable-assignment.patch --]
[-- Type: text/x-diff, Size: 642 bytes --]

From 80bdfb936f9dd24fd8bb0ce0aecc10f796dd4f6f Mon Sep 17 00:00:00 2001
From: Dima Kogan <dima@secretsauce.net>
Date: Tue, 30 Dec 2014 20:05:58 -0800
Subject: [PATCH 2/2] removed unused variable assignment

---
 src/font.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/font.c b/src/font.c
index 9ea43cd..b2b43c7 100644
--- a/src/font.c
+++ b/src/font.c
@@ -2136,7 +2136,6 @@ font_score (Lisp_Object entity, Lisp_Object *spec_prop)
       }
 
   /* Score the size.  Maximum difference is 127.  */
-  i = FONT_SIZE_INDEX;
   if (! NILP (spec_prop[FONT_SIZE_INDEX])
       && XINT (AREF (entity, FONT_SIZE_INDEX)) > 0)
     {
-- 
2.1.4

bug#19117: 25.0.50; emacs on x11 chooses different fonts for the same face sometimes

[Jan D.]

Hi.

There is a Dmitriy Kogan in the copyright list.  Is that you?

	Jan D.

> 5 feb 2015 kl. 21:41 skrev Dima Kogan <dima@secretsauce.net>:
> 
> Jan D. <jan.h.d@swipnet.se> writes:
> 
>> 
>> But the comment here:
>> 
>> +          /* We skip the second pass unless we really need it.  */
>> +          if (!(NILP (list)                      /* No fonts found on 
>> the first pass */
>> +                && skipped_some_scalable_fonts)) /* and we skipped some 
>> scalable ones.  */
>> +            break;
>> 
>> 
>> looks like it really should be  /* Fonts found on the first pass */
> 
> There was an ! there, so it made it unclear what the comment is
> referring to. Attaching new patches that apply against the current
> master, and have a clarified comment in that place
> 
>> From 4e895fb38c84342021b602dd4a9830b55fe56a58 Mon Sep 17 00:00:00 2001
> From: Dima Kogan <dima@secretsauce.net>
> Date: Tue, 30 Dec 2014 20:05:11 -0800
> Subject: [PATCH 1/2] scalable font logic now matches the docs and is more
> consistent
> 
> A nil value of scalable-fonts-allowed allows scalable fonts if no others
> were found.  This is the previously-documented behavior.
> 
> realize_basic_faces() no longer forces scalable-fonts-allowed to t.  If
> nil, some fonts will always be found even if only scalable fonts are
> available, as set by the other part of this patch.
> ---
> src/xfaces.c |   3 --
> src/xfont.c  | 173 +++++++++++++++++++++++++++++++++--------------------------
> 2 files changed, 96 insertions(+), 80 deletions(-)
> 
> diff --git a/src/xfaces.c b/src/xfaces.c
> index 6e01ab0..d0fe872 100644
> --- a/src/xfaces.c
> +++ b/src/xfaces.c
> @@ -5141,12 +5141,10 @@ static bool
> realize_basic_faces (struct frame *f)
> {
>   bool success_p = false;
> -  ptrdiff_t count = SPECPDL_INDEX ();
> 
>   /* Block input here so that we won't be surprised by an X expose
>      event, for instance, without having the faces set up.  */
>   block_input ();
> -  specbind (Qscalable_fonts_allowed, Qt);
> 
>   if (realize_default_face (f))
>     {
> @@ -5180,7 +5178,6 @@ realize_basic_faces (struct frame *f)
>       success_p = true;
>     }
> 
> -  unbind_to (count, Qnil);
>   unblock_input ();
>   return success_p;
> }
> diff --git a/src/xfont.c b/src/xfont.c
> index 3a0f5e3..5f1c61c 100644
> --- a/src/xfont.c
> +++ b/src/xfont.c
> @@ -388,94 +388,113 @@ xfont_list_pattern (Display *display, const char *pattern,
>       Lisp_Object *props = XVECTOR (xfont_scratch_props)->contents;
>       Lisp_Object scripts = Qnil, entity = Qnil;
> 
> +      /* We take two passes over the font list.  The second pass is
> +         taken only if scalable-fonts-allowed is nil, and only
> +         scalable fonts were found.
> +       */
> +      int i_pass;
> +      bool skipped_some_scalable_fonts = false;
> +
>       for (i = 0; i < ASIZE (xfont_scratch_props); i++)
> 	ASET (xfont_scratch_props, i, Qnil);
>       for (i = 0; i < num_fonts; i++)
> 	indices[i] = names[i];
>       qsort (indices, num_fonts, sizeof (char *), compare_font_names);
> 
> -      for (i = 0; i < num_fonts; i++)
> -	{
> -	  ptrdiff_t len;
> +      for (i_pass = 0; i_pass < 2; i_pass++)
> +        {
> +          for (i = 0; i < num_fonts; i++)
> +            {
> +              ptrdiff_t len;
> 
> -	  if (i > 0 && xstrcasecmp (indices[i - 1], indices[i]) == 0)
> -	    continue;
> -	  if (NILP (entity))
> -	    entity = font_make_entity ();
> -	  len = xfont_decode_coding_xlfd (indices[i], -1, buf);
> -	  if (font_parse_xlfd (buf, len, entity) < 0)
> -	    continue;
> -	  ASET (entity, FONT_TYPE_INDEX, Qx);
> -	  /* Avoid auto-scaled fonts.  */
> -	  if (INTEGERP (AREF (entity, FONT_DPI_INDEX))
> -	      && INTEGERP (AREF (entity, FONT_AVGWIDTH_INDEX))
> -	      && XINT (AREF (entity, FONT_DPI_INDEX)) != 0
> -	      && XINT (AREF (entity, FONT_AVGWIDTH_INDEX)) == 0)
> -	    continue;
> -	  /* Avoid not-allowed scalable fonts.  */
> -	  if (NILP (Vscalable_fonts_allowed))
> -	    {
> -	      int size = 0;
> +              if (i > 0 && xstrcasecmp (indices[i - 1], indices[i]) == 0)
> +                continue;
> +              if (NILP (entity))
> +                entity = font_make_entity ();
> +              len = xfont_decode_coding_xlfd (indices[i], -1, buf);
> +              if (font_parse_xlfd (buf, len, entity) < 0)
> +                continue;
> +              ASET (entity, FONT_TYPE_INDEX, Qx);
> +              /* Avoid auto-scaled fonts.  */
> +              if (INTEGERP (AREF (entity, FONT_DPI_INDEX))
> +                  && INTEGERP (AREF (entity, FONT_AVGWIDTH_INDEX))
> +                  && XINT (AREF (entity, FONT_DPI_INDEX)) != 0
> +                  && XINT (AREF (entity, FONT_AVGWIDTH_INDEX)) == 0)
> +                continue;
> +              /* Avoid not-allowed scalable fonts.  */
> +              if (NILP (Vscalable_fonts_allowed))
> +                {
> +                  int size = 0;
> 
> -	      if (INTEGERP (AREF (entity, FONT_SIZE_INDEX)))
> -		size = XINT (AREF (entity, FONT_SIZE_INDEX));
> -	      else if (FLOATP (AREF (entity, FONT_SIZE_INDEX)))
> -		size = XFLOAT_DATA (AREF (entity, FONT_SIZE_INDEX));
> -	      if (size == 0)
> -		continue;
> -	    }
> -	  else if (CONSP (Vscalable_fonts_allowed))
> -	    {
> -	      Lisp_Object tail, elt;
> +                  if (INTEGERP (AREF (entity, FONT_SIZE_INDEX)))
> +                    size = XINT (AREF (entity, FONT_SIZE_INDEX));
> +                  else if (FLOATP (AREF (entity, FONT_SIZE_INDEX)))
> +                    size = XFLOAT_DATA (AREF (entity, FONT_SIZE_INDEX));
> +                  if (size == 0 && i_pass == 0)
> +                    {
> +                      skipped_some_scalable_fonts = true;
> +                      continue;
> +                    }
> +                }
> +              else if (CONSP (Vscalable_fonts_allowed))
> +                {
> +                  Lisp_Object tail, elt;
> 
> -	      for (tail = Vscalable_fonts_allowed; CONSP (tail);
> -		   tail = XCDR (tail))
> -		{
> -		  elt = XCAR (tail);
> -		  if (STRINGP (elt)
> -		      && fast_c_string_match_ignore_case (elt, indices[i],
> -							  len) >= 0)
> -		    break;
> -		}
> -	      if (! CONSP (tail))
> -		continue;
> -	    }
> +                  for (tail = Vscalable_fonts_allowed; CONSP (tail);
> +                       tail = XCDR (tail))
> +                    {
> +                      elt = XCAR (tail);
> +                      if (STRINGP (elt)
> +                          && fast_c_string_match_ignore_case (elt, indices[i],
> +                                                              len) >= 0)
> +                        break;
> +                    }
> +                  if (! CONSP (tail))
> +                    continue;
> +                }
> 
> -	  /* Avoid fonts of invalid registry.  */
> -	  if (NILP (AREF (entity, FONT_REGISTRY_INDEX)))
> -	    continue;
> +              /* Avoid fonts of invalid registry.  */
> +              if (NILP (AREF (entity, FONT_REGISTRY_INDEX)))
> +                continue;
> 
> -	  /* Update encoding and repertory if necessary.  */
> -	  if (! EQ (registry, AREF (entity, FONT_REGISTRY_INDEX)))
> -	    {
> -	      registry = AREF (entity, FONT_REGISTRY_INDEX);
> -	      if (font_registry_charsets (registry, &encoding, &repertory) < 0)
> -		encoding = NULL;
> -	    }
> -	  if (! encoding)
> -	    /* Unknown REGISTRY, not supported.  */
> -	    continue;
> -	  if (repertory)
> -	    {
> -	      if (NILP (script)
> -		  || xfont_chars_supported (chars, NULL, encoding, repertory))
> -		list = Fcons (entity, list), entity = Qnil;
> -	      continue;
> -	    }
> -	  if (memcmp (props, aref_addr (entity, FONT_FOUNDRY_INDEX),
> -		      word_size * 7)
> -	      || ! EQ (AREF (entity, FONT_SPACING_INDEX), props[7]))
> -	    {
> -	      vcopy (xfont_scratch_props, 0,
> -		     aref_addr (entity, FONT_FOUNDRY_INDEX), 7);
> -	      ASET (xfont_scratch_props, 7, AREF (entity, FONT_SPACING_INDEX));
> -	      scripts = xfont_supported_scripts (display, indices[i],
> -						 xfont_scratch_props, encoding);
> -	    }
> -	  if (NILP (script)
> -	      || ! NILP (Fmemq (script, scripts)))
> -	    list = Fcons (entity, list), entity = Qnil;
> -	}
> +              /* Update encoding and repertory if necessary.  */
> +              if (! EQ (registry, AREF (entity, FONT_REGISTRY_INDEX)))
> +                {
> +                  registry = AREF (entity, FONT_REGISTRY_INDEX);
> +                  if (font_registry_charsets (registry, &encoding, &repertory) < 0)
> +                    encoding = NULL;
> +                }
> +              if (! encoding)
> +                /* Unknown REGISTRY, not supported.  */
> +                continue;
> +              if (repertory)
> +                {
> +                  if (NILP (script)
> +                      || xfont_chars_supported (chars, NULL, encoding, repertory))
> +                    list = Fcons (entity, list), entity = Qnil;
> +                  continue;
> +                }
> +              if (memcmp (props, aref_addr (entity, FONT_FOUNDRY_INDEX),
> +                          word_size * 7)
> +                  || ! EQ (AREF (entity, FONT_SPACING_INDEX), props[7]))
> +                {
> +                  vcopy (xfont_scratch_props, 0,
> +                         aref_addr (entity, FONT_FOUNDRY_INDEX), 7);
> +                  ASET (xfont_scratch_props, 7, AREF (entity, FONT_SPACING_INDEX));
> +                  scripts = xfont_supported_scripts (display, indices[i],
> +                                                     xfont_scratch_props, encoding);
> +                }
> +              if (NILP (script)
> +                  || ! NILP (Fmemq (script, scripts)))
> +                list = Fcons (entity, list), entity = Qnil;
> +            }
> +
> +          /* We skip the second pass unless we really need it.  */
> +          if (!                                 /* Loop again if... */
> +              (NILP (list)                      /*   No fonts found on the first pass */
> +               && skipped_some_scalable_fonts)) /*   and we skipped some scalable ones.  */
> +            break;
> +        }
>       XFreeFontNames (names);
>     }
> 
> -- 
> 2.1.4
> 
>> From 80bdfb936f9dd24fd8bb0ce0aecc10f796dd4f6f Mon Sep 17 00:00:00 2001
> From: Dima Kogan <dima@secretsauce.net>
> Date: Tue, 30 Dec 2014 20:05:58 -0800
> Subject: [PATCH 2/2] removed unused variable assignment
> 
> ---
> src/font.c | 1 -
> 1 file changed, 1 deletion(-)
> 
> diff --git a/src/font.c b/src/font.c
> index 9ea43cd..b2b43c7 100644
> --- a/src/font.c
> +++ b/src/font.c
> @@ -2136,7 +2136,6 @@ font_score (Lisp_Object entity, Lisp_Object *spec_prop)
>       }
> 
>   /* Score the size.  Maximum difference is 127.  */
> -  i = FONT_SIZE_INDEX;
>   if (! NILP (spec_prop[FONT_SIZE_INDEX])
>       && XINT (AREF (entity, FONT_SIZE_INDEX)) > 0)
>     {
> -- 
> 2.1.4
> 

bug#19117: 25.0.50; emacs on x11 chooses different fonts for the same face sometimes

[Jan D.]

Can you add ChangeLog entries for this so we can install it?

Thanks,

	Jan D.

> 5 feb 2015 kl. 21:41 skrev Dima Kogan <dima@secretsauce.net>:
> 
> Jan D. <jan.h.d@swipnet.se> writes:
> 
>> 
>> But the comment here:
>> 
>> +          /* We skip the second pass unless we really need it.  */
>> +          if (!(NILP (list)                      /* No fonts found on 
>> the first pass */
>> +                && skipped_some_scalable_fonts)) /* and we skipped some 
>> scalable ones.  */
>> +            break;
>> 
>> 
>> looks like it really should be  /* Fonts found on the first pass */
> 
> There was an ! there, so it made it unclear what the comment is
> referring to. Attaching new patches that apply against the current
> master, and have a clarified comment in that place
> 
>> From 4e895fb38c84342021b602dd4a9830b55fe56a58 Mon Sep 17 00:00:00 2001
> From: Dima Kogan <dima@secretsauce.net>
> Date: Tue, 30 Dec 2014 20:05:11 -0800
> Subject: [PATCH 1/2] scalable font logic now matches the docs and is more
> consistent
> 
> A nil value of scalable-fonts-allowed allows scalable fonts if no others
> were found.  This is the previously-documented behavior.
> 
> realize_basic_faces() no longer forces scalable-fonts-allowed to t.  If
> nil, some fonts will always be found even if only scalable fonts are
> available, as set by the other part of this patch.
> ---
> src/xfaces.c |   3 --
> src/xfont.c  | 173 +++++++++++++++++++++++++++++++++--------------------------
> 2 files changed, 96 insertions(+), 80 deletions(-)
> 
> diff --git a/src/xfaces.c b/src/xfaces.c
> index 6e01ab0..d0fe872 100644
> --- a/src/xfaces.c
> +++ b/src/xfaces.c
> @@ -5141,12 +5141,10 @@ static bool
> realize_basic_faces (struct frame *f)
> {
>   bool success_p = false;
> -  ptrdiff_t count = SPECPDL_INDEX ();
> 
>   /* Block input here so that we won't be surprised by an X expose
>      event, for instance, without having the faces set up.  */
>   block_input ();
> -  specbind (Qscalable_fonts_allowed, Qt);
> 
>   if (realize_default_face (f))
>     {
> @@ -5180,7 +5178,6 @@ realize_basic_faces (struct frame *f)
>       success_p = true;
>     }
> 
> -  unbind_to (count, Qnil);
>   unblock_input ();
>   return success_p;
> }
> diff --git a/src/xfont.c b/src/xfont.c
> index 3a0f5e3..5f1c61c 100644
> --- a/src/xfont.c
> +++ b/src/xfont.c
> @@ -388,94 +388,113 @@ xfont_list_pattern (Display *display, const char *pattern,
>       Lisp_Object *props = XVECTOR (xfont_scratch_props)->contents;
>       Lisp_Object scripts = Qnil, entity = Qnil;
> 
> +      /* We take two passes over the font list.  The second pass is
> +         taken only if scalable-fonts-allowed is nil, and only
> +         scalable fonts were found.
> +       */
> +      int i_pass;
> +      bool skipped_some_scalable_fonts = false;
> +
>       for (i = 0; i < ASIZE (xfont_scratch_props); i++)
> 	ASET (xfont_scratch_props, i, Qnil);
>       for (i = 0; i < num_fonts; i++)
> 	indices[i] = names[i];
>       qsort (indices, num_fonts, sizeof (char *), compare_font_names);
> 
> -      for (i = 0; i < num_fonts; i++)
> -	{
> -	  ptrdiff_t len;
> +      for (i_pass = 0; i_pass < 2; i_pass++)
> +        {
> +          for (i = 0; i < num_fonts; i++)
> +            {
> +              ptrdiff_t len;
> 
> -	  if (i > 0 && xstrcasecmp (indices[i - 1], indices[i]) == 0)
> -	    continue;
> -	  if (NILP (entity))
> -	    entity = font_make_entity ();
> -	  len = xfont_decode_coding_xlfd (indices[i], -1, buf);
> -	  if (font_parse_xlfd (buf, len, entity) < 0)
> -	    continue;
> -	  ASET (entity, FONT_TYPE_INDEX, Qx);
> -	  /* Avoid auto-scaled fonts.  */
> -	  if (INTEGERP (AREF (entity, FONT_DPI_INDEX))
> -	      && INTEGERP (AREF (entity, FONT_AVGWIDTH_INDEX))
> -	      && XINT (AREF (entity, FONT_DPI_INDEX)) != 0
> -	      && XINT (AREF (entity, FONT_AVGWIDTH_INDEX)) == 0)
> -	    continue;
> -	  /* Avoid not-allowed scalable fonts.  */
> -	  if (NILP (Vscalable_fonts_allowed))
> -	    {
> -	      int size = 0;
> +              if (i > 0 && xstrcasecmp (indices[i - 1], indices[i]) == 0)
> +                continue;
> +              if (NILP (entity))
> +                entity = font_make_entity ();
> +              len = xfont_decode_coding_xlfd (indices[i], -1, buf);
> +              if (font_parse_xlfd (buf, len, entity) < 0)
> +                continue;
> +              ASET (entity, FONT_TYPE_INDEX, Qx);
> +              /* Avoid auto-scaled fonts.  */
> +              if (INTEGERP (AREF (entity, FONT_DPI_INDEX))
> +                  && INTEGERP (AREF (entity, FONT_AVGWIDTH_INDEX))
> +                  && XINT (AREF (entity, FONT_DPI_INDEX)) != 0
> +                  && XINT (AREF (entity, FONT_AVGWIDTH_INDEX)) == 0)
> +                continue;
> +              /* Avoid not-allowed scalable fonts.  */
> +              if (NILP (Vscalable_fonts_allowed))
> +                {
> +                  int size = 0;
> 
> -	      if (INTEGERP (AREF (entity, FONT_SIZE_INDEX)))
> -		size = XINT (AREF (entity, FONT_SIZE_INDEX));
> -	      else if (FLOATP (AREF (entity, FONT_SIZE_INDEX)))
> -		size = XFLOAT_DATA (AREF (entity, FONT_SIZE_INDEX));
> -	      if (size == 0)
> -		continue;
> -	    }
> -	  else if (CONSP (Vscalable_fonts_allowed))
> -	    {
> -	      Lisp_Object tail, elt;
> +                  if (INTEGERP (AREF (entity, FONT_SIZE_INDEX)))
> +                    size = XINT (AREF (entity, FONT_SIZE_INDEX));
> +                  else if (FLOATP (AREF (entity, FONT_SIZE_INDEX)))
> +                    size = XFLOAT_DATA (AREF (entity, FONT_SIZE_INDEX));
> +                  if (size == 0 && i_pass == 0)
> +                    {
> +                      skipped_some_scalable_fonts = true;
> +                      continue;
> +                    }
> +                }
> +              else if (CONSP (Vscalable_fonts_allowed))
> +                {
> +                  Lisp_Object tail, elt;
> 
> -	      for (tail = Vscalable_fonts_allowed; CONSP (tail);
> -		   tail = XCDR (tail))
> -		{
> -		  elt = XCAR (tail);
> -		  if (STRINGP (elt)
> -		      && fast_c_string_match_ignore_case (elt, indices[i],
> -							  len) >= 0)
> -		    break;
> -		}
> -	      if (! CONSP (tail))
> -		continue;
> -	    }
> +                  for (tail = Vscalable_fonts_allowed; CONSP (tail);
> +                       tail = XCDR (tail))
> +                    {
> +                      elt = XCAR (tail);
> +                      if (STRINGP (elt)
> +                          && fast_c_string_match_ignore_case (elt, indices[i],
> +                                                              len) >= 0)
> +                        break;
> +                    }
> +                  if (! CONSP (tail))
> +                    continue;
> +                }
> 
> -	  /* Avoid fonts of invalid registry.  */
> -	  if (NILP (AREF (entity, FONT_REGISTRY_INDEX)))
> -	    continue;
> +              /* Avoid fonts of invalid registry.  */
> +              if (NILP (AREF (entity, FONT_REGISTRY_INDEX)))
> +                continue;
> 
> -	  /* Update encoding and repertory if necessary.  */
> -	  if (! EQ (registry, AREF (entity, FONT_REGISTRY_INDEX)))
> -	    {
> -	      registry = AREF (entity, FONT_REGISTRY_INDEX);
> -	      if (font_registry_charsets (registry, &encoding, &repertory) < 0)
> -		encoding = NULL;
> -	    }
> -	  if (! encoding)
> -	    /* Unknown REGISTRY, not supported.  */
> -	    continue;
> -	  if (repertory)
> -	    {
> -	      if (NILP (script)
> -		  || xfont_chars_supported (chars, NULL, encoding, repertory))
> -		list = Fcons (entity, list), entity = Qnil;
> -	      continue;
> -	    }
> -	  if (memcmp (props, aref_addr (entity, FONT_FOUNDRY_INDEX),
> -		      word_size * 7)
> -	      || ! EQ (AREF (entity, FONT_SPACING_INDEX), props[7]))
> -	    {
> -	      vcopy (xfont_scratch_props, 0,
> -		     aref_addr (entity, FONT_FOUNDRY_INDEX), 7);
> -	      ASET (xfont_scratch_props, 7, AREF (entity, FONT_SPACING_INDEX));
> -	      scripts = xfont_supported_scripts (display, indices[i],
> -						 xfont_scratch_props, encoding);
> -	    }
> -	  if (NILP (script)
> -	      || ! NILP (Fmemq (script, scripts)))
> -	    list = Fcons (entity, list), entity = Qnil;
> -	}
> +              /* Update encoding and repertory if necessary.  */
> +              if (! EQ (registry, AREF (entity, FONT_REGISTRY_INDEX)))
> +                {
> +                  registry = AREF (entity, FONT_REGISTRY_INDEX);
> +                  if (font_registry_charsets (registry, &encoding, &repertory) < 0)
> +                    encoding = NULL;
> +                }
> +              if (! encoding)
> +                /* Unknown REGISTRY, not supported.  */
> +                continue;
> +              if (repertory)
> +                {
> +                  if (NILP (script)
> +                      || xfont_chars_supported (chars, NULL, encoding, repertory))
> +                    list = Fcons (entity, list), entity = Qnil;
> +                  continue;
> +                }
> +              if (memcmp (props, aref_addr (entity, FONT_FOUNDRY_INDEX),
> +                          word_size * 7)
> +                  || ! EQ (AREF (entity, FONT_SPACING_INDEX), props[7]))
> +                {
> +                  vcopy (xfont_scratch_props, 0,
> +                         aref_addr (entity, FONT_FOUNDRY_INDEX), 7);
> +                  ASET (xfont_scratch_props, 7, AREF (entity, FONT_SPACING_INDEX));
> +                  scripts = xfont_supported_scripts (display, indices[i],
> +                                                     xfont_scratch_props, encoding);
> +                }
> +              if (NILP (script)
> +                  || ! NILP (Fmemq (script, scripts)))
> +                list = Fcons (entity, list), entity = Qnil;
> +            }
> +
> +          /* We skip the second pass unless we really need it.  */
> +          if (!                                 /* Loop again if... */
> +              (NILP (list)                      /*   No fonts found on the first pass */
> +               && skipped_some_scalable_fonts)) /*   and we skipped some scalable ones.  */
> +            break;
> +        }
>       XFreeFontNames (names);
>     }
> 
> -- 
> 2.1.4
> 
>> From 80bdfb936f9dd24fd8bb0ce0aecc10f796dd4f6f Mon Sep 17 00:00:00 2001
> From: Dima Kogan <dima@secretsauce.net>
> Date: Tue, 30 Dec 2014 20:05:58 -0800
> Subject: [PATCH 2/2] removed unused variable assignment
> 
> ---
> src/font.c | 1 -
> 1 file changed, 1 deletion(-)
> 
> diff --git a/src/font.c b/src/font.c
> index 9ea43cd..b2b43c7 100644
> --- a/src/font.c
> +++ b/src/font.c
> @@ -2136,7 +2136,6 @@ font_score (Lisp_Object entity, Lisp_Object *spec_prop)
>       }
> 
>   /* Score the size.  Maximum difference is 127.  */
> -  i = FONT_SIZE_INDEX;
>   if (! NILP (spec_prop[FONT_SIZE_INDEX])
>       && XINT (AREF (entity, FONT_SIZE_INDEX)) > 0)
>     {
> -- 
> 2.1.4
> 

bug#19117: 25.0.50; emacs on x11 chooses different fonts for the same face sometimes

[Dima Kogan]

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

Jan D. <jan.h.d@swipnet.se> writes:

> Can you add ChangeLog entries for this so we can install it?

Attached. Let me know if there are issues with the changelog style.
Also, I'm including everything in one patch. It was mostly like that
already.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-scalable-font-logic-now-matches-the-docs-and-is-more.patch --]
[-- Type: text/x-diff, Size: 9881 bytes --]

From 2380816e2160236b35f06eed63839f5e71280686 Mon Sep 17 00:00:00 2001
From: Dima Kogan <dima@secretsauce.net>
Date: Tue, 30 Dec 2014 20:05:11 -0800
Subject: [PATCH] scalable font logic now matches the docs and is more
 consistent

A nil value of scalable-fonts-allowed allows scalable fonts if no others
were found.  This is the previously-documented behavior.

realize_basic_faces() no longer forces scalable-fonts-allowed to t.  If
nil, some fonts will always be found even if only scalable fonts are
available, as set by the other part of this patch.
---
 src/ChangeLog |   8 +++
 src/font.c    |   1 -
 src/xfaces.c  |   3 -
 src/xfont.c   | 173 ++++++++++++++++++++++++++++++++--------------------------
 4 files changed, 104 insertions(+), 81 deletions(-)

diff --git a/src/ChangeLog b/src/ChangeLog
index cd72f98..d189b7e 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,11 @@
+2015-02-07  Dima Kogan  <dima@secretsauce.net>
+
+	* xfont.c (xfont_list_pattern):
+	* xfaces.c (realize_basic_faces):
+	* font.c (font_score): scalable font logic now matches the docs
+	and is more consistent
+	Fixes: bug#19117
+
 2015-02-07  Paul Eggert  <eggert@cs.ucla.edu>
 
 	Remove no-longer-used cursor_in_echo_area code
diff --git a/src/font.c b/src/font.c
index 9ea43cd..b2b43c7 100644
--- a/src/font.c
+++ b/src/font.c
@@ -2136,7 +2136,6 @@ font_score (Lisp_Object entity, Lisp_Object *spec_prop)
       }
 
   /* Score the size.  Maximum difference is 127.  */
-  i = FONT_SIZE_INDEX;
   if (! NILP (spec_prop[FONT_SIZE_INDEX])
       && XINT (AREF (entity, FONT_SIZE_INDEX)) > 0)
     {
diff --git a/src/xfaces.c b/src/xfaces.c
index 6e01ab0..d0fe872 100644
--- a/src/xfaces.c
+++ b/src/xfaces.c
@@ -5141,12 +5141,10 @@ static bool
 realize_basic_faces (struct frame *f)
 {
   bool success_p = false;
-  ptrdiff_t count = SPECPDL_INDEX ();
 
   /* Block input here so that we won't be surprised by an X expose
      event, for instance, without having the faces set up.  */
   block_input ();
-  specbind (Qscalable_fonts_allowed, Qt);
 
   if (realize_default_face (f))
     {
@@ -5180,7 +5178,6 @@ realize_basic_faces (struct frame *f)
       success_p = true;
     }
 
-  unbind_to (count, Qnil);
   unblock_input ();
   return success_p;
 }
diff --git a/src/xfont.c b/src/xfont.c
index 3a0f5e3..5f1c61c 100644
--- a/src/xfont.c
+++ b/src/xfont.c
@@ -388,94 +388,113 @@ xfont_list_pattern (Display *display, const char *pattern,
       Lisp_Object *props = XVECTOR (xfont_scratch_props)->contents;
       Lisp_Object scripts = Qnil, entity = Qnil;
 
+      /* We take two passes over the font list.  The second pass is
+         taken only if scalable-fonts-allowed is nil, and only
+         scalable fonts were found.
+       */
+      int i_pass;
+      bool skipped_some_scalable_fonts = false;
+
       for (i = 0; i < ASIZE (xfont_scratch_props); i++)
 	ASET (xfont_scratch_props, i, Qnil);
       for (i = 0; i < num_fonts; i++)
 	indices[i] = names[i];
       qsort (indices, num_fonts, sizeof (char *), compare_font_names);
 
-      for (i = 0; i < num_fonts; i++)
-	{
-	  ptrdiff_t len;
+      for (i_pass = 0; i_pass < 2; i_pass++)
+        {
+          for (i = 0; i < num_fonts; i++)
+            {
+              ptrdiff_t len;
 
-	  if (i > 0 && xstrcasecmp (indices[i - 1], indices[i]) == 0)
-	    continue;
-	  if (NILP (entity))
-	    entity = font_make_entity ();
-	  len = xfont_decode_coding_xlfd (indices[i], -1, buf);
-	  if (font_parse_xlfd (buf, len, entity) < 0)
-	    continue;
-	  ASET (entity, FONT_TYPE_INDEX, Qx);
-	  /* Avoid auto-scaled fonts.  */
-	  if (INTEGERP (AREF (entity, FONT_DPI_INDEX))
-	      && INTEGERP (AREF (entity, FONT_AVGWIDTH_INDEX))
-	      && XINT (AREF (entity, FONT_DPI_INDEX)) != 0
-	      && XINT (AREF (entity, FONT_AVGWIDTH_INDEX)) == 0)
-	    continue;
-	  /* Avoid not-allowed scalable fonts.  */
-	  if (NILP (Vscalable_fonts_allowed))
-	    {
-	      int size = 0;
+              if (i > 0 && xstrcasecmp (indices[i - 1], indices[i]) == 0)
+                continue;
+              if (NILP (entity))
+                entity = font_make_entity ();
+              len = xfont_decode_coding_xlfd (indices[i], -1, buf);
+              if (font_parse_xlfd (buf, len, entity) < 0)
+                continue;
+              ASET (entity, FONT_TYPE_INDEX, Qx);
+              /* Avoid auto-scaled fonts.  */
+              if (INTEGERP (AREF (entity, FONT_DPI_INDEX))
+                  && INTEGERP (AREF (entity, FONT_AVGWIDTH_INDEX))
+                  && XINT (AREF (entity, FONT_DPI_INDEX)) != 0
+                  && XINT (AREF (entity, FONT_AVGWIDTH_INDEX)) == 0)
+                continue;
+              /* Avoid not-allowed scalable fonts.  */
+              if (NILP (Vscalable_fonts_allowed))
+                {
+                  int size = 0;
 
-	      if (INTEGERP (AREF (entity, FONT_SIZE_INDEX)))
-		size = XINT (AREF (entity, FONT_SIZE_INDEX));
-	      else if (FLOATP (AREF (entity, FONT_SIZE_INDEX)))
-		size = XFLOAT_DATA (AREF (entity, FONT_SIZE_INDEX));
-	      if (size == 0)
-		continue;
-	    }
-	  else if (CONSP (Vscalable_fonts_allowed))
-	    {
-	      Lisp_Object tail, elt;
+                  if (INTEGERP (AREF (entity, FONT_SIZE_INDEX)))
+                    size = XINT (AREF (entity, FONT_SIZE_INDEX));
+                  else if (FLOATP (AREF (entity, FONT_SIZE_INDEX)))
+                    size = XFLOAT_DATA (AREF (entity, FONT_SIZE_INDEX));
+                  if (size == 0 && i_pass == 0)
+                    {
+                      skipped_some_scalable_fonts = true;
+                      continue;
+                    }
+                }
+              else if (CONSP (Vscalable_fonts_allowed))
+                {
+                  Lisp_Object tail, elt;
 
-	      for (tail = Vscalable_fonts_allowed; CONSP (tail);
-		   tail = XCDR (tail))
-		{
-		  elt = XCAR (tail);
-		  if (STRINGP (elt)
-		      && fast_c_string_match_ignore_case (elt, indices[i],
-							  len) >= 0)
-		    break;
-		}
-	      if (! CONSP (tail))
-		continue;
-	    }
+                  for (tail = Vscalable_fonts_allowed; CONSP (tail);
+                       tail = XCDR (tail))
+                    {
+                      elt = XCAR (tail);
+                      if (STRINGP (elt)
+                          && fast_c_string_match_ignore_case (elt, indices[i],
+                                                              len) >= 0)
+                        break;
+                    }
+                  if (! CONSP (tail))
+                    continue;
+                }
 
-	  /* Avoid fonts of invalid registry.  */
-	  if (NILP (AREF (entity, FONT_REGISTRY_INDEX)))
-	    continue;
+              /* Avoid fonts of invalid registry.  */
+              if (NILP (AREF (entity, FONT_REGISTRY_INDEX)))
+                continue;
 
-	  /* Update encoding and repertory if necessary.  */
-	  if (! EQ (registry, AREF (entity, FONT_REGISTRY_INDEX)))
-	    {
-	      registry = AREF (entity, FONT_REGISTRY_INDEX);
-	      if (font_registry_charsets (registry, &encoding, &repertory) < 0)
-		encoding = NULL;
-	    }
-	  if (! encoding)
-	    /* Unknown REGISTRY, not supported.  */
-	    continue;
-	  if (repertory)
-	    {
-	      if (NILP (script)
-		  || xfont_chars_supported (chars, NULL, encoding, repertory))
-		list = Fcons (entity, list), entity = Qnil;
-	      continue;
-	    }
-	  if (memcmp (props, aref_addr (entity, FONT_FOUNDRY_INDEX),
-		      word_size * 7)
-	      || ! EQ (AREF (entity, FONT_SPACING_INDEX), props[7]))
-	    {
-	      vcopy (xfont_scratch_props, 0,
-		     aref_addr (entity, FONT_FOUNDRY_INDEX), 7);
-	      ASET (xfont_scratch_props, 7, AREF (entity, FONT_SPACING_INDEX));
-	      scripts = xfont_supported_scripts (display, indices[i],
-						 xfont_scratch_props, encoding);
-	    }
-	  if (NILP (script)
-	      || ! NILP (Fmemq (script, scripts)))
-	    list = Fcons (entity, list), entity = Qnil;
-	}
+              /* Update encoding and repertory if necessary.  */
+              if (! EQ (registry, AREF (entity, FONT_REGISTRY_INDEX)))
+                {
+                  registry = AREF (entity, FONT_REGISTRY_INDEX);
+                  if (font_registry_charsets (registry, &encoding, &repertory) < 0)
+                    encoding = NULL;
+                }
+              if (! encoding)
+                /* Unknown REGISTRY, not supported.  */
+                continue;
+              if (repertory)
+                {
+                  if (NILP (script)
+                      || xfont_chars_supported (chars, NULL, encoding, repertory))
+                    list = Fcons (entity, list), entity = Qnil;
+                  continue;
+                }
+              if (memcmp (props, aref_addr (entity, FONT_FOUNDRY_INDEX),
+                          word_size * 7)
+                  || ! EQ (AREF (entity, FONT_SPACING_INDEX), props[7]))
+                {
+                  vcopy (xfont_scratch_props, 0,
+                         aref_addr (entity, FONT_FOUNDRY_INDEX), 7);
+                  ASET (xfont_scratch_props, 7, AREF (entity, FONT_SPACING_INDEX));
+                  scripts = xfont_supported_scripts (display, indices[i],
+                                                     xfont_scratch_props, encoding);
+                }
+              if (NILP (script)
+                  || ! NILP (Fmemq (script, scripts)))
+                list = Fcons (entity, list), entity = Qnil;
+            }
+
+          /* We skip the second pass unless we really need it.  */
+          if (!                                 /* Loop again if... */
+              (NILP (list)                      /*   No fonts found on the first pass */
+               && skipped_some_scalable_fonts)) /*   and we skipped some scalable ones.  */
+            break;
+        }
       XFreeFontNames (names);
     }
 
-- 
2.1.4

bug#19117: 25.0.50; emacs on x11 chooses different fonts for the same face sometimes

[Jan D.]

Commited.

> 7 feb 2015 kl. 09:28 skrev Dima Kogan <dima@secretsauce.net>:
> 
> Jan D. <jan.h.d@swipnet.se> writes:
> 
>> Can you add ChangeLog entries for this so we can install it?
> 
> Attached. Let me know if there are issues with the changelog style.

Many, I rewrote them.

	Jan D.

> Also, I'm including everything in one patch. It was mostly like that
> already.
> 
>> From 2380816e2160236b35f06eed63839f5e71280686 Mon Sep 17 00:00:00 2001
> From: Dima Kogan <dima@secretsauce.net>
> Date: Tue, 30 Dec 2014 20:05:11 -0800
> Subject: [PATCH] scalable font logic now matches the docs and is more
> consistent
> 
> A nil value of scalable-fonts-allowed allows scalable fonts if no others
> were found.  This is the previously-documented behavior.
> 
> realize_basic_faces() no longer forces scalable-fonts-allowed to t.  If
> nil, some fonts will always be found even if only scalable fonts are
> available, as set by the other part of this patch.
> ---
> src/ChangeLog |   8 +++
> src/font.c    |   1 -
> src/xfaces.c  |   3 -
> src/xfont.c   | 173 ++++++++++++++++++++++++++++++++--------------------------
> 4 files changed, 104 insertions(+), 81 deletions(-)
> 
> diff --git a/src/ChangeLog b/src/ChangeLog
> index cd72f98..d189b7e 100644
> --- a/src/ChangeLog
> +++ b/src/ChangeLog
> @@ -1,3 +1,11 @@
> +2015-02-07  Dima Kogan  <dima@secretsauce.net>
> +
> +	* xfont.c (xfont_list_pattern):
> +	* xfaces.c (realize_basic_faces):
> +	* font.c (font_score): scalable font logic now matches the docs
> +	and is more consistent
> +	Fixes: bug#19117
> +
> 2015-02-07  Paul Eggert  <eggert@cs.ucla.edu>
> 
> 	Remove no-longer-used cursor_in_echo_area code
> diff --git a/src/font.c b/src/font.c
> index 9ea43cd..b2b43c7 100644
> --- a/src/font.c
> +++ b/src/font.c
> @@ -2136,7 +2136,6 @@ font_score (Lisp_Object entity, Lisp_Object *spec_prop)
>       }
> 
>   /* Score the size.  Maximum difference is 127.  */
> -  i = FONT_SIZE_INDEX;
>   if (! NILP (spec_prop[FONT_SIZE_INDEX])
>       && XINT (AREF (entity, FONT_SIZE_INDEX)) > 0)
>     {
> diff --git a/src/xfaces.c b/src/xfaces.c
> index 6e01ab0..d0fe872 100644
> --- a/src/xfaces.c
> +++ b/src/xfaces.c
> @@ -5141,12 +5141,10 @@ static bool
> realize_basic_faces (struct frame *f)
> {
>   bool success_p = false;
> -  ptrdiff_t count = SPECPDL_INDEX ();
> 
>   /* Block input here so that we won't be surprised by an X expose
>      event, for instance, without having the faces set up.  */
>   block_input ();
> -  specbind (Qscalable_fonts_allowed, Qt);
> 
>   if (realize_default_face (f))
>     {
> @@ -5180,7 +5178,6 @@ realize_basic_faces (struct frame *f)
>       success_p = true;
>     }
> 
> -  unbind_to (count, Qnil);
>   unblock_input ();
>   return success_p;
> }
> diff --git a/src/xfont.c b/src/xfont.c
> index 3a0f5e3..5f1c61c 100644
> --- a/src/xfont.c
> +++ b/src/xfont.c
> @@ -388,94 +388,113 @@ xfont_list_pattern (Display *display, const char *pattern,
>       Lisp_Object *props = XVECTOR (xfont_scratch_props)->contents;
>       Lisp_Object scripts = Qnil, entity = Qnil;
> 
> +      /* We take two passes over the font list.  The second pass is
> +         taken only if scalable-fonts-allowed is nil, and only
> +         scalable fonts were found.
> +       */
> +      int i_pass;
> +      bool skipped_some_scalable_fonts = false;
> +
>       for (i = 0; i < ASIZE (xfont_scratch_props); i++)
> 	ASET (xfont_scratch_props, i, Qnil);
>       for (i = 0; i < num_fonts; i++)
> 	indices[i] = names[i];
>       qsort (indices, num_fonts, sizeof (char *), compare_font_names);
> 
> -      for (i = 0; i < num_fonts; i++)
> -	{
> -	  ptrdiff_t len;
> +      for (i_pass = 0; i_pass < 2; i_pass++)
> +        {
> +          for (i = 0; i < num_fonts; i++)
> +            {
> +              ptrdiff_t len;
> 
> -	  if (i > 0 && xstrcasecmp (indices[i - 1], indices[i]) == 0)
> -	    continue;
> -	  if (NILP (entity))
> -	    entity = font_make_entity ();
> -	  len = xfont_decode_coding_xlfd (indices[i], -1, buf);
> -	  if (font_parse_xlfd (buf, len, entity) < 0)
> -	    continue;
> -	  ASET (entity, FONT_TYPE_INDEX, Qx);
> -	  /* Avoid auto-scaled fonts.  */
> -	  if (INTEGERP (AREF (entity, FONT_DPI_INDEX))
> -	      && INTEGERP (AREF (entity, FONT_AVGWIDTH_INDEX))
> -	      && XINT (AREF (entity, FONT_DPI_INDEX)) != 0
> -	      && XINT (AREF (entity, FONT_AVGWIDTH_INDEX)) == 0)
> -	    continue;
> -	  /* Avoid not-allowed scalable fonts.  */
> -	  if (NILP (Vscalable_fonts_allowed))
> -	    {
> -	      int size = 0;
> +              if (i > 0 && xstrcasecmp (indices[i - 1], indices[i]) == 0)
> +                continue;
> +              if (NILP (entity))
> +                entity = font_make_entity ();
> +              len = xfont_decode_coding_xlfd (indices[i], -1, buf);
> +              if (font_parse_xlfd (buf, len, entity) < 0)
> +                continue;
> +              ASET (entity, FONT_TYPE_INDEX, Qx);
> +              /* Avoid auto-scaled fonts.  */
> +              if (INTEGERP (AREF (entity, FONT_DPI_INDEX))
> +                  && INTEGERP (AREF (entity, FONT_AVGWIDTH_INDEX))
> +                  && XINT (AREF (entity, FONT_DPI_INDEX)) != 0
> +                  && XINT (AREF (entity, FONT_AVGWIDTH_INDEX)) == 0)
> +                continue;
> +              /* Avoid not-allowed scalable fonts.  */
> +              if (NILP (Vscalable_fonts_allowed))
> +                {
> +                  int size = 0;
> 
> -	      if (INTEGERP (AREF (entity, FONT_SIZE_INDEX)))
> -		size = XINT (AREF (entity, FONT_SIZE_INDEX));
> -	      else if (FLOATP (AREF (entity, FONT_SIZE_INDEX)))
> -		size = XFLOAT_DATA (AREF (entity, FONT_SIZE_INDEX));
> -	      if (size == 0)
> -		continue;
> -	    }
> -	  else if (CONSP (Vscalable_fonts_allowed))
> -	    {
> -	      Lisp_Object tail, elt;
> +                  if (INTEGERP (AREF (entity, FONT_SIZE_INDEX)))
> +                    size = XINT (AREF (entity, FONT_SIZE_INDEX));
> +                  else if (FLOATP (AREF (entity, FONT_SIZE_INDEX)))
> +                    size = XFLOAT_DATA (AREF (entity, FONT_SIZE_INDEX));
> +                  if (size == 0 && i_pass == 0)
> +                    {
> +                      skipped_some_scalable_fonts = true;
> +                      continue;
> +                    }
> +                }
> +              else if (CONSP (Vscalable_fonts_allowed))
> +                {
> +                  Lisp_Object tail, elt;
> 
> -	      for (tail = Vscalable_fonts_allowed; CONSP (tail);
> -		   tail = XCDR (tail))
> -		{
> -		  elt = XCAR (tail);
> -		  if (STRINGP (elt)
> -		      && fast_c_string_match_ignore_case (elt, indices[i],
> -							  len) >= 0)
> -		    break;
> -		}
> -	      if (! CONSP (tail))
> -		continue;
> -	    }
> +                  for (tail = Vscalable_fonts_allowed; CONSP (tail);
> +                       tail = XCDR (tail))
> +                    {
> +                      elt = XCAR (tail);
> +                      if (STRINGP (elt)
> +                          && fast_c_string_match_ignore_case (elt, indices[i],
> +                                                              len) >= 0)
> +                        break;
> +                    }
> +                  if (! CONSP (tail))
> +                    continue;
> +                }
> 
> -	  /* Avoid fonts of invalid registry.  */
> -	  if (NILP (AREF (entity, FONT_REGISTRY_INDEX)))
> -	    continue;
> +              /* Avoid fonts of invalid registry.  */
> +              if (NILP (AREF (entity, FONT_REGISTRY_INDEX)))
> +                continue;
> 
> -	  /* Update encoding and repertory if necessary.  */
> -	  if (! EQ (registry, AREF (entity, FONT_REGISTRY_INDEX)))
> -	    {
> -	      registry = AREF (entity, FONT_REGISTRY_INDEX);
> -	      if (font_registry_charsets (registry, &encoding, &repertory) < 0)
> -		encoding = NULL;
> -	    }
> -	  if (! encoding)
> -	    /* Unknown REGISTRY, not supported.  */
> -	    continue;
> -	  if (repertory)
> -	    {
> -	      if (NILP (script)
> -		  || xfont_chars_supported (chars, NULL, encoding, repertory))
> -		list = Fcons (entity, list), entity = Qnil;
> -	      continue;
> -	    }
> -	  if (memcmp (props, aref_addr (entity, FONT_FOUNDRY_INDEX),
> -		      word_size * 7)
> -	      || ! EQ (AREF (entity, FONT_SPACING_INDEX), props[7]))
> -	    {
> -	      vcopy (xfont_scratch_props, 0,
> -		     aref_addr (entity, FONT_FOUNDRY_INDEX), 7);
> -	      ASET (xfont_scratch_props, 7, AREF (entity, FONT_SPACING_INDEX));
> -	      scripts = xfont_supported_scripts (display, indices[i],
> -						 xfont_scratch_props, encoding);
> -	    }
> -	  if (NILP (script)
> -	      || ! NILP (Fmemq (script, scripts)))
> -	    list = Fcons (entity, list), entity = Qnil;
> -	}
> +              /* Update encoding and repertory if necessary.  */
> +              if (! EQ (registry, AREF (entity, FONT_REGISTRY_INDEX)))
> +                {
> +                  registry = AREF (entity, FONT_REGISTRY_INDEX);
> +                  if (font_registry_charsets (registry, &encoding, &repertory) < 0)
> +                    encoding = NULL;
> +                }
> +              if (! encoding)
> +                /* Unknown REGISTRY, not supported.  */
> +                continue;
> +              if (repertory)
> +                {
> +                  if (NILP (script)
> +                      || xfont_chars_supported (chars, NULL, encoding, repertory))
> +                    list = Fcons (entity, list), entity = Qnil;
> +                  continue;
> +                }
> +              if (memcmp (props, aref_addr (entity, FONT_FOUNDRY_INDEX),
> +                          word_size * 7)
> +                  || ! EQ (AREF (entity, FONT_SPACING_INDEX), props[7]))
> +                {
> +                  vcopy (xfont_scratch_props, 0,
> +                         aref_addr (entity, FONT_FOUNDRY_INDEX), 7);
> +                  ASET (xfont_scratch_props, 7, AREF (entity, FONT_SPACING_INDEX));
> +                  scripts = xfont_supported_scripts (display, indices[i],
> +                                                     xfont_scratch_props, encoding);
> +                }
> +              if (NILP (script)
> +                  || ! NILP (Fmemq (script, scripts)))
> +                list = Fcons (entity, list), entity = Qnil;
> +            }
> +
> +          /* We skip the second pass unless we really need it.  */
> +          if (!                                 /* Loop again if... */
> +              (NILP (list)                      /*   No fonts found on the first pass */
> +               && skipped_some_scalable_fonts)) /*   and we skipped some scalable ones.  */
> +            break;
> +        }
>       XFreeFontNames (names);
>     }
> 
> -- 
> 2.1.4
> 

bug#19117: 25.0.50; emacs on x11 chooses different fonts for the same face sometimes

[K. Handa]

In article <87bnl9nkqe.fsf@gnu.org>, handa <handa@gnu.org> writes:

> In article <jwvfvamhoh2.fsf-monnier+emacsbugs@gnu.org>, Stefan Monnier <monnier@iro.umontreal.ca> writes:
> > > This is a gentle ping. Is there anything more I can do to get this
> > > merged?

> > I could install it, but I really have no idea whether your patch does
> > the right thing, because I'm too unfamiliar with this code.
> > Jan?  Handa?

Sorry for the late response.  I agree that the patch is
doing the correct thing, but it seems that it overlooks the
case of scalable-fonts-allowed being a cons:


	    else if (CONSP (Vscalable_fonts_allowed))
	      {
		Lisp_Object tail;

		for (tail = Vscalable_fonts_allowed; CONSP (tail);
		     tail = XCDR (tail))
		  {
		    Lisp_Object elt = XCAR (tail);
		    if (STRINGP (elt)
			&& (fast_c_string_match_ignore_case (elt, indices[i],
							     len)
			    >= 0))
		      break;
		  }
		if (! CONSP (tail))
		  continue;
	      }

Here too, skipped_some_scalable_fonts must be set to true
before "continue;".

And, one trivial comment on ChangeLog entry :-p

	* xfaces.c (realize_basic_faces): Don't set Qscalable_fonts_allowed to
	t.

Isn't it better to use the pair Qscalable_fonts_allowed and
Qt, or the pair scalable-fonts-allowed and t?

---
Kenichi Handa
handa@gnu.org