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

[Dima Kogan <dima@secretsauce.net>]

[-- 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