GNU bug report logs - #75291
Redisplay not updating fringe when face filter changes

Previous Next

Full log

Message #28 received at 75291 <at> debbugs.gnu.org (full text, mbox):

From: Daniel Colascione <dancol <at> dancol.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 75291 <at> debbugs.gnu.org, mina86 <at> mina86.com
Subject: Re: bug#75291: Redisplay not updating fringe when face filter changes
Date: Fri, 03 Jan 2025 12:25:05 -0500

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Daniel Colascione <dancol <at> dancol.org>
>> Cc: 75291 <at> debbugs.gnu.org,  mina86 <at> mina86.com
>> Date: Thu, 02 Jan 2025 14:50:54 -0500
>> 
>> Eli Zaretskii <eliz <at> gnu.org> writes:
>> 
>> > Yes, redrawing everything will work, but will also cause flicker, and
>> > is generally expensive, thus undesirable.
>> 
>> FWIW, it doesn't seem to cause flicker in practice.  I see flicker only
>> when walking through messages in mu4e --- we do redisplay and draw only
>> the background, and I haven't figured out why yet.  But in general, on a
>> modern window system, turning a given redisplay into a full redisplay
>> shouldn't cause flicker, even if it's inefficient.
>
> I think it depends on whether you use double-buffering (some people
> don't or cannot) and whether you have the mouse pointer over an Emacs
> frame.  Also, depending on the GUI toolkit, the decorations might
> flicker.

TTY windows don't have fringes, and the most commonly-used window
systems all do atomic updates nowadays.

>> I came across overlay_arrows_changed_p --- isn't this function trying to
>> deal with exactly the case of something in the fringe changing outside
>> the changed text region?
>
> So you want to add to display_line code that sets each glyph_row's
> redraw_fringe_bitmaps_p flag when the fringe face changes?  That could
> probably work, provided that we disable redisplay optimizations which
> might avoid calling display_line (you will see that we already disable
> such optimizations when overlay_arrows_changed_p returns non-zero).
> We might actually need to disable more of the optimizations, because
> the overlay-arrow thing doesn't contradict the optimizations that
> scroll the pixels, something that reaction to changes in the fringe
> face cannot tolerate.

That might work, but I don't think we even need anything that
complicated or low-level.  Not many are using :filtered now, and those
that do big redraws anyway.  How about this simpler code that gets us
correctness, albeit more conservatively?

diff --git a/src/window.c b/src/window.c
index 5a10c381eaf..6d135a67a66 100644
--- a/src/window.c
+++ b/src/window.c
@@ -2396,11 +2396,29 @@ DEFUN ("set-window-parameter", Fset_window_parameter,
   Lisp_Object old_alist_elt;
 
   old_alist_elt = Fassq (parameter, w->window_parameters);
+
+  /* If this window parameter has been used in a face remapping filter
+     expression and we changed its value, force a from-scratch redisplay
+     to make sure that everything that depends on the window parameter
+     value is up-to-date.  FIXME: instead of taking a sledgehammer to
+     redisplay, we could be more precise in tracking what display bits
+     depend on which remapped faces.  Skip redrawing TTY frames: they
+     don't have fringes or other graphical bits we might have to redraw
+     here.  */
+  if (SYMBOLP (parameter) &&
+      WINDOW_LIVE_P (window) &&
+      FRAME_WINDOW_P (WINDOW_XFRAME (w)) &&
+      !NILP (Fget (parameter, Qface_filter)) &&
+      !EQ (CDR_SAFE (old_alist_elt), value))
+    redraw_frame (WINDOW_XFRAME (w));
+
   if (NILP (old_alist_elt))
     wset_window_parameters
       (w, Fcons (Fcons (parameter, value), w->window_parameters));
   else
     Fsetcdr (old_alist_elt, value);
+
+
   return value;
 }
 
diff --git a/src/xfaces.c b/src/xfaces.c
index d1ca2e0d5d4..1f58bdbf6ae 100644
--- a/src/xfaces.c
+++ b/src/xfaces.c
@@ -2512,6 +2512,9 @@ evaluate_face_filter (Lisp_Object filter, struct window *w,
     if (!NILP (filter))
       goto err;
 
+    if (NILP (Fget (parameter, Qface_filter)))
+      Fput (parameter, Qface_filter, Qt);
+
     bool match = false;
     if (w)
       {
@@ -7623,6 +7626,8 @@ syms_of_xfaces (void)
   Vface_remapping_alist = Qnil;
   DEFSYM (Qface_remapping_alist,"face-remapping-alist");
 
+  DEFSYM (Qface_filter, "face-filter");
+
   DEFVAR_LISP ("face-font-rescale-alist", Vface_font_rescale_alist,
 	       doc: /* Alist of fonts vs the rescaling factors.
 Each element is a cons (FONT-PATTERN . RESCALE-RATIO), where

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.