trunk patch: font.c: font_matching_entity()

trunk patch: font.c: font_matching_entity()

[Adrian Robert]

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

Hi,

The function font_matching_entity() in font.c ignores its ATTRS  
argument.  If, in the sequence

xfaces.c: realize_x_face()
-> font.c: font_load_for_lface()
-> font.c: font_find_for_lface

the last call fails to find anything, font_matching_entity() will be  
called, and, as far as I can tell, returns incorrect results.  The  
attached patch causes ATTRS to be used in both the cache check and the  
call to driver->match(), and allows a better-matching font to be found  
in the above use case.

(The patch also makes a case-insensitive family name comparison in  
font_find_for_lface() case-sensitive when compiling under HAVE_NS.)


-Adrian


[-- Attachment #2: fontMatch_20080929.patch --]
[-- Type: application/octet-stream, Size: 2320 bytes --]

Index: font.c
===================================================================
RCS file: /sources/emacs/emacs/src/font.c,v
retrieving revision 1.83
diff -u -p -r1.83 font.c
--- font.c	10 Sep 2008 05:54:09 -0000	1.83
+++ font.c	30 Sep 2008 00:51:43 -0000
@@ -2765,12 +2765,18 @@ font_matching_entity (f, attrs, spec)
   struct font_driver_list *driver_list = f->font_driver_list;
   Lisp_Object ftype, size, entity;
   Lisp_Object frame;
+  Lisp_Object work = Fcopy_font_spec (spec);
 
   XSETFRAME (frame, f);
   ftype = AREF (spec, FONT_TYPE_INDEX);
   size = AREF (spec, FONT_SIZE_INDEX);
   if (FLOATP (size))
     ASET (spec, FONT_SIZE_INDEX, make_number (font_pixel_size (f, spec)));
+
+  FONT_SET_STYLE (work, FONT_WEIGHT_INDEX, attrs[LFACE_WEIGHT_INDEX]);
+  FONT_SET_STYLE (work, FONT_SLANT_INDEX, attrs[LFACE_SLANT_INDEX]);
+  FONT_SET_STYLE (work, FONT_WIDTH_INDEX, attrs[LFACE_SWIDTH_INDEX]);
+
   entity = Qnil;
   for (; driver_list; driver_list = driver_list->next)
     if (driver_list->on
@@ -2779,14 +2785,14 @@ font_matching_entity (f, attrs, spec)
 	Lisp_Object cache = font_get_cache (f, driver_list->driver);
 	Lisp_Object copy;
 
-	ASET (spec, FONT_TYPE_INDEX, driver_list->driver->type);
-	entity = assoc_no_quit (spec, XCDR (cache));
+	ASET (work, FONT_TYPE_INDEX, driver_list->driver->type);
+	entity = assoc_no_quit (work, XCDR (cache));
 	if (CONSP (entity))
 	  entity = XCDR (entity);
 	else
 	  {
-	    entity = driver_list->driver->match (frame, spec);
-	    copy = Fcopy_font_spec (spec);
+	    entity = driver_list->driver->match (frame, work);
+	    copy = Fcopy_font_spec (work);
 	    ASET (copy, FONT_TYPE_INDEX, driver_list->driver->type);
 	    XSETCDR (cache, Fcons (Fcons (copy, entity), XCDR (cache)));
 	  }
@@ -2795,7 +2801,7 @@ font_matching_entity (f, attrs, spec)
       }
   ASET (spec, FONT_TYPE_INDEX, ftype);
   ASET (spec, FONT_SIZE_INDEX, size);
-  font_add_log ("match", spec, entity);
+  font_add_log ("match", work, entity);
   return entity;
 }
 
@@ -3166,7 +3172,12 @@ font_find_for_lface (f, attrs, spec, c)
   else
     {
       Lisp_Object alters
-	= Fassoc_string (val, Vface_alternative_font_family_alist, Qt);
+	= Fassoc_string (val, Vface_alternative_font_family_alist,
+#ifndef HAVE_NS
+ Qt);
+#else
+ Qnil);
+#endif
 
       if (! NILP (alters))
 	{

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

Re: trunk patch: font.c: font_matching_entity()

[Kenichi Handa]

In article <717CC6DC-7294-4815-9B61-748AEF59D0FB@gmail.com>, Adrian Robert <adrian.b.robert@gmail.com> writes:

> [1  <text/plain; US-ASCII (7bit)>]
> Hi,

> The function font_matching_entity() in font.c ignores its ATTRS  
> argument.  If, in the sequence

> xfaces.c: realize_x_face()
> -> font.c: font_load_for_lface()
> -> font.c: font_find_for_lface

> the last call fails to find anything, font_matching_entity() will be  
> called, and, as far as I can tell, returns incorrect results.  The  
> attached patch causes ATTRS to be used in both the cache check and the  
> call to driver->match(), and allows a better-matching font to be found  
> in the above use case.

> (The patch also makes a case-insensitive family name comparison in  
> font_find_for_lface() case-sensitive when compiling under HAVE_NS.)

Thank you for the patch.  I've just installed it.

---
Kenichi Handa
handa@ni.aist.go.jp


> -Adrian

> [2 fontMatch_20080929.patch <application/octet-stream (7bit)>]
> Index: font.c
> ===================================================================
> RCS file: /sources/emacs/emacs/src/font.c,v
> retrieving revision 1.83
> diff -u -p -r1.83 font.c
> --- font.c	10 Sep 2008 05:54:09 -0000	1.83
> +++ font.c	30 Sep 2008 00:51:43 -0000
> @@ -2765,12 +2765,18 @@ font_matching_entity (f, attrs, spec)
>    struct font_driver_list *driver_list = f->font_driver_list;
>    Lisp_Object ftype, size, entity;
>    Lisp_Object frame;
> +  Lisp_Object work = Fcopy_font_spec (spec);
 
>    XSETFRAME (frame, f);
>    ftype = AREF (spec, FONT_TYPE_INDEX);
>    size = AREF (spec, FONT_SIZE_INDEX);
>    if (FLOATP (size))
>      ASET (spec, FONT_SIZE_INDEX, make_number (font_pixel_size (f, spec)));
> +
> +  FONT_SET_STYLE (work, FONT_WEIGHT_INDEX, attrs[LFACE_WEIGHT_INDEX]);
> +  FONT_SET_STYLE (work, FONT_SLANT_INDEX, attrs[LFACE_SLANT_INDEX]);
> +  FONT_SET_STYLE (work, FONT_WIDTH_INDEX, attrs[LFACE_SWIDTH_INDEX]);
> +
>    entity = Qnil;
>    for (; driver_list; driver_list = driver_list->next)
>      if (driver_list->on
> @@ -2779,14 +2785,14 @@ font_matching_entity (f, attrs, spec)
>  	Lisp_Object cache = font_get_cache (f, driver_list->driver);
>  	Lisp_Object copy;
 
> -	ASET (spec, FONT_TYPE_INDEX, driver_list->driver->type);
> -	entity = assoc_no_quit (spec, XCDR (cache));
> +	ASET (work, FONT_TYPE_INDEX, driver_list->driver->type);
> +	entity = assoc_no_quit (work, XCDR (cache));
>  	if (CONSP (entity))
>  	  entity = XCDR (entity);
>  	else
>  	  {
> -	    entity = driver_list->driver->match (frame, spec);
> -	    copy = Fcopy_font_spec (spec);
> +	    entity = driver_list->driver->match (frame, work);
> +	    copy = Fcopy_font_spec (work);
>  	    ASET (copy, FONT_TYPE_INDEX, driver_list->driver->type);
>  	    XSETCDR (cache, Fcons (Fcons (copy, entity), XCDR (cache)));
>  	  }
> @@ -2795,7 +2801,7 @@ font_matching_entity (f, attrs, spec)
>        }
>    ASET (spec, FONT_TYPE_INDEX, ftype);
>    ASET (spec, FONT_SIZE_INDEX, size);
> -  font_add_log ("match", spec, entity);
> +  font_add_log ("match", work, entity);
>    return entity;
>  }
 
> @@ -3166,7 +3172,12 @@ font_find_for_lface (f, attrs, spec, c)
>    else
>      {
>        Lisp_Object alters
> -	= Fassoc_string (val, Vface_alternative_font_family_alist, Qt);
> +	= Fassoc_string (val, Vface_alternative_font_family_alist,
> +#ifndef HAVE_NS
> + Qt);
> +#else
> + Qnil);
> +#endif
 
>        if (! NILP (alters))
>  	{
> [3  <text/plain; US-ASCII (7bit)>]