bug#34001: 26.1.90; :distant-foreground face property fails to work in most cases

[Federico Tedin <federicotedin@gmail.com>]

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

Tina Russell <tinakellyrussell@gmail.com> writes:

> :distant-foreground is a very useful concept for a face property: text
> will be rendered with the :foreground color, unless it’s too close to
> the current background color, in which case :distant-foreground kicks
> in. Like, try this in Eshell or IELM:
>
> (propertize "Greetings, esteemed Emacs developers!" 'face '(:foreground
> "yellow" :background "yellow"))
>
> You’ll get a solid band of yellow, of course. But, with
> :distant-foreground…
>
> (propertize "Greetings, esteemed Emacs developers!" 'face '(:foreground
> "yellow" :background "yellow" :distant-foreground "black"))
>
> Now it is a friendly greeting. (Naturally, you wouldn’t normally set
> :background and :distant-foreground in the same face, this is just an
> example.)
>
> But, try this:
>
> (propertize "Greetings, esteemed Emacs developers!" 'face '(:foreground
> "yellow" :background "white" :distant-foreground "black"))
>
> :distant-foreground doesn’t kick in—and you’re left with yellow-on-white
> text that’s impossible to read, the exact scenario that
> :distant-foreground was quite rightly designed to avoid.
>
> I’m not the only one who’s noticed this; there’s a good StackExchange
> thread from 2015 here:
> https://emacs.stackexchange.com/questions/7982/ The author notes that
> there should be a user option to set the amount of “distance” (between
> foreground and background colors) that is required for
> distant-foreground to kick in, and adds that a good way to measure
> color distance in real-world circumstances is described here:
> http://web.mst.edu/~rhall/web_design/color_readability.html A commenter
> suggested that he file a bug, but I can’t find an Emacs bug report
> related to this issue, so I’m filing this now.
>
> I really hope this gets addressed, because :distant-foreground is a
> really nice and clever feature. Thanks!
>
> In GNU Emacs 26.1.90 (build 2, x86_64-pc-linux-gnu, GTK+ Version 3.22.30)
>  of 2019-01-02 built on pannychis
> Repository revision: 08840f2f7bfc6144bd163dd85efe87d28541e425
> Windowing system distributor 'The X.Org Foundation', version 11.0.11906000
> System Description: Ubuntu 18.04.1 LTS
>
> Configured using:
>  'configure --with-xaw3d --with-modules --with-xwidgets'

I've created a patch that introduces a new variable
`face-near-same-color-threshold', with a default value of 30000 (as was
defined in NEAR_SAME_COLOR_THRESHOLD in xfaces.c). Changing this
variable's value will make certain color combinations be considered as
'same', which will lead to :distant-foreground being used. For example:

M-: (setq face-near-same-color-threshold 200000)
M-: (clear-font-cache)

Then, using your third example, the text will be displayed with a black
foregound.

Is this a reasonable fix, or should we consider implementing another way
of measuring distance between colors like Tina mentioned?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch --]
[-- Type: text/x-diff, Size: 1636 bytes --]

diff --git a/src/xfaces.c b/src/xfaces.c
index cffa89e1f3..eeea165187 100644
--- a/src/xfaces.c
+++ b/src/xfaces.c
@@ -1157,8 +1157,6 @@ load_color (struct frame *f, struct face *face, Lisp_Object name,
 
 #ifdef HAVE_WINDOW_SYSTEM
 
-#define NEAR_SAME_COLOR_THRESHOLD 30000
-
 /* Load colors for face FACE which is used on frame F.  Colors are
    specified by slots LFACE_BACKGROUND_INDEX and LFACE_FOREGROUND_INDEX
    of ATTRS.  If the background color specified is not supported on F,
@@ -1198,8 +1196,9 @@ load_face_colors (struct frame *f, struct face *face,
   face->foreground = load_color2 (f, face, fg, LFACE_FOREGROUND_INDEX, &xfg);
 
   dfg = attrs[LFACE_DISTANT_FOREGROUND_INDEX];
+  int distance = color_distance (&xbg, &xfg);
   if (!NILP (dfg) && !UNSPECIFIEDP (dfg)
-      && color_distance (&xbg, &xfg) < NEAR_SAME_COLOR_THRESHOLD)
+      && distance < face_near_same_color_threshold)
     {
       if (EQ (attrs[LFACE_INVERSE_INDEX], Qt))
         face->background = load_color (f, face, dfg, LFACE_BACKGROUND_INDEX);
@@ -6768,6 +6767,12 @@ RESCALE-RATIO is a floating point number to specify how much larger
 a font of 10 point, we actually use a font of 10 * RESCALE-RATIO point.  */);
   Vface_font_rescale_alist = Qnil;
 
+  DEFVAR_INT ("face-near-same-color-threshold", face_near_same_color_threshold,
+	      doc: /* Number representing the minimum tolerated distance
+between face background and foreground colors before distant-foreground is
+used instead (when present). */);
+  face_near_same_color_threshold = 30000;
+
 #ifdef HAVE_WINDOW_SYSTEM
   defsubr (&Sbitmap_spec_p);
   defsubr (&Sx_list_fonts);