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