bug#75291: Redisplay not updating fringe when face filter changes

[Daniel Colascione <dancol@dancol.org>]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Daniel Colascione <dancol@dancol.org>
>> Cc: 75291@debbugs.gnu.org,  mina86@mina86.com
>> Date: Fri, 03 Jan 2025 14:46:20 -0500
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> >> From: Daniel Colascione <dancol@dancol.org>
>> >> Cc: 75291@debbugs.gnu.org,  mina86@mina86.com
>> >> Date: Fri, 03 Jan 2025 12:25:05 -0500
>> >> 
>> >> > 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.
>> >
>> > People still report flickering from time to time, so I don't think
>> > this never happens.
>> >
>> >> > 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?
>> >
>> > Doesn't that only support face remapping with :filtered attribute?
>> > What about the more general case where the fringe face is remapped in
>> > a way that's independent of the windows?
>> 
>> That seems to work already. It's only in the fringe that I see problems
>> --- it just doesn't seem worth it to limit the redraw to the fringe.
>
> Sorry, I don't understand.  I _was_ talking about the fringe face.

I misread your question. To answer it: what are the circumstances in
which the effective fringe face can change behind our backs?  Any change
to a face attribute proper will cause a redraw anyway.
The face-remap.el functions call force-mode-line-update to make sure
face remapping lists take effect, so we should have non-window-specific
remapping changes covered.  What's left is the window parameters.
Any other cases?

> But if redraw_frame solves the issue, and doesn't cause unpleasant or
> expensive redraws, feel free to install on the master branch.  But
> please change this:
>
> +  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));
>
> to say this instead:
>
> +  if (SYMBOLP (parameter) &&
> +      WINDOW_LIVE_P (window) &&
> +      FRAME_WINDOW_P (WINDOW_XFRAME (w)) &&
> +      EQ (Fget (parameter, Qface_filter), Qt) && <<<<<<<<<<<<<<<<<
> +      !EQ (CDR_SAFE (old_alist_elt), value))
> +    redraw_frame (WINDOW_XFRAME (w));
>
> (A stylistic comment: our conventions is to put the && operator at the
> beginning of a line, not at the end of a line.)

Thanks for the reminder!