unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: Daniel Colascione <dancol@dancol.org>
To: Eli Zaretskii <eliz@gnu.org>
Cc: 75291@debbugs.gnu.org, Michal Nazarewicz <mina86@mina86.com>
Subject: bug#75291: Redisplay not updating fringe when face filter changes
Date: Thu, 02 Jan 2025 13:36:34 -0500	[thread overview]
Message-ID: <87ttah2hcd.fsf@dancol.org> (raw)
In-Reply-To: <8634i1jeai.fsf@gnu.org> (Eli Zaretskii's message of "Thu, 02 Jan 2025 19:50:29 +0200")

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Daniel Colascione <dancol@dancol.org>
>> Date: Thu, 02 Jan 2025 12:30:16 -0500
>> 
>> Sometimes we don't redraw when its effective face changes indirectly due
>> to a face filter condition evaluating differently.
>> 
>> An explicit redraw-display works around the problem, but it shouldn't be
>> necessary and is an efficiency hit.
>> 
>> See the code below:
>> 
>> ;; -*- lexical-binding: t -*-
>> 
>> ;; emacs -Q
>> 
>> (setf my-window (selected-window))
>> 
>> (dolist (face '(default fringe))
>>   (face-remap-add-relative
>>    face `(:filtered (:window foo t) (:background "green"))))
>> 
>> (set-window-parameter my-window 'foo t)
>> 
>> ;; During this sit-for, background and fringe should be green
>> (sit-for 1)
>> (set-window-parameter my-window 'foo nil)
>> 
>> ;; Enable to work around bug
>> (when nil
>>   (redraw-display))
>> 
>> ;; During this sit-for, background and fringe should be their original
>> ;; color (probably white), but only the background changes.
>> ;; Without redraw-display, the fringe remains green.
>> (sit-for 1)
>
> I think this is bug#74876 again.  I tried to explain there why the way
> the fringe drawing is implemented doesn't work well with
> face-remapping (or with changes in the fringe face in general).
>
> Maybe I'm missing something, but supporting what this and that bug
> want would need a rewrite of update_window_fringes (and possibly also
> the way we record fringes' attributes in the glyph row).

That's amazing. Seven years ago, I implemented
https://github.com/dcolascione/emacs-window-highlight/blob/master/window-highlight.el.
I worked around the bug we're discussing using redraw-display. I see
that Michal (++) implemented the same feature independently and reported
the same bug just a few weeks before I got around to reporting the same
bug from seven years ago!  Pure serendipity.

(BTW: my code uses pre-redisplay-function, not a command hook. I've
tried it both ways and found the redisplay hook to be more reliable.)

Anyway, given that the feature has been implemented twice now, maybe we
can find some way to make it work efficiently?  I haven't looked into
how exactly we do fringe invalidation, but isn't there a way we can
limit the blast radius of the redraw by providing a lisp-level function
to invalidate extra GUI parts of specific windows rather than forcing a
redraw-frame?  It's not clear to me why we couldn't skip redrawing every
row's content but redraw every row's fringe anyway.

Ideally, a change to a face in which the change couldn't possibly affect
layout (e.g. changing a background color) would be pretty efficient from
a redisplay POV, since we wouldn't have to even try to reflow any text.





  reply	other threads:[~2025-01-02 18:36 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-01-02 17:30 bug#75291: Redisplay not updating fringe when face filter changes Daniel Colascione
2025-01-02 17:50 ` Eli Zaretskii
2025-01-02 18:36   ` Daniel Colascione [this message]
2025-01-02 19:24     ` Michal Nazarewicz
2025-01-02 19:26     ` Eli Zaretskii
2025-01-02 19:50       ` Daniel Colascione
2025-01-02 20:56         ` Eli Zaretskii
2025-01-03 17:25           ` Daniel Colascione
2025-01-03 19:31             ` Eli Zaretskii
2025-01-03 19:46               ` Daniel Colascione
2025-01-03 20:10                 ` Eli Zaretskii
2025-01-03 20:27                   ` Eli Zaretskii
2025-01-03 20:57                   ` Daniel Colascione
2025-01-04  7:12                     ` Eli Zaretskii

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87ttah2hcd.fsf@dancol.org \
    --to=dancol@dancol.org \
    --cc=75291@debbugs.gnu.org \
    --cc=eliz@gnu.org \
    --cc=mina86@mina86.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).