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

merge 75291 74876
thanks

> From: Daniel Colascione <dancol@dancol.org>
> Cc: 75291@debbugs.gnu.org, Michal Nazarewicz <mina86@mina86.com>
> Date: Thu, 02 Jan 2025 13:36:34 -0500
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > 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.

Yes, redrawing everything will work, but will also cause flicker, and
is generally expensive, thus undesirable.

> 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.

AFAIR, the current implementation is the other way around: when some
glyph row changes, we consider redrawing its fringes.  E.g.,
draw_window_fringes only considers glyph rows that are enabled in the
glyph matrix, which means redisplay found that glyph row has changed.
Clearly, someone didn't think the fringe face will change during a
session!

Regarding your idea about Lisp function that would invalidate GUI
parts: it is not very easy, since a Lisp program cannot easily know
where on the screen a given region of buffer positions will be.
There is posn-at-point, of course, but (a) it is quite expensive, and
(b) when Lisp runs, display could be outdated, so what posn-at-point
returns could be inaccurate.

In any case, I don't think this will be needed in the case in point,
because when the fringe face changes, we'd want to redraw the fringes
of the entire window, right?  So redisplay_window would need to notice
the change in the face, and invoke update_window_fringes in a special
way, such that update_window_fringes marks fringes to be redrawn not
only when the glyph row is enabled.  Maybe that would work.

The way to "notice the change in the face" is not a simple problem to
solve, btw.  We currently don't know which faces change when some face
is modified.  So we have a frame-global flag that is set when any face
changes its attributes.  If we use that flag for detecting potential
changes in the fringe face, we'd start redrawing fringes
unnecessarily.  We could add a special flag for the fringe face, but
then we'd need to add a special mechanism in xfaces.c which would
analyze each change in some face's attribute and determine whether it
could possibly affect the fringe face.  Or maybe I'm missing some more
elegant/easier solution.





  parent reply	other threads:[~2025-01-02 19:26 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
2025-01-02 19:24     ` Michal Nazarewicz
2025-01-02 19:26     ` Eli Zaretskii [this message]
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=86v7uxhv9c.fsf@gnu.org \
    --to=eliz@gnu.org \
    --cc=75291@debbugs.gnu.org \
    --cc=dancol@dancol.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).