bug#42406: Mouse-wheel scrolling can be flickering

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: alan@idiocy.org,  konrad.podczeck@univie.ac.at,  42406@debbugs.gnu.org
> Date: Thu, 17 Dec 2020 16:07:19 -0500
> 
> > That probably just means abbrev-mode should be added to the list at
> > the end of frame.el.  Or maybe that we need some new mechanism to
> > trigger update of the lighter on the mode line when a mode is turned
> > on or off.
> 
> Don't know about "new" but the old mechanism is that the standard
> minor-mode code ends up calling `force-mode-line-update` (this now
> mostly comes from `define-minor-mode`, but in the past it was present in
> most "manual" definitions as well).

IMO, that shouldn't be needed.  And force-mode-line-update has its own
problems, see the many questions about why it doesn't do what the
caller thought it would.

> > And while we are talking about force-mode-line-update: can you explain
> > why we need to set the prevent_redisplay_optimizations_p flag of the
> > buffer, in addition to setting update_mode_lines to a magic value?
> 
> I wish I could, but that bit predates me, and I have no idea what
> `prevent_redisplay_optimizations_p` means or does, really.
> 
> I just removed it from my local Emacs, to see if I notice any difference.

My point is that we are dealing with a bunch of global and local flags
with overlapping functionality about which we have no clear idea what
each one is doing.  Removing a flag will not solve that basic problem,
because I don't doubt that some of these flags is needed in some,
perhaps subtle and rare, situation.  We won't find flags that could be
removed without unwanted consequences.

> > And btw, redisplaying the mode line in general could mean you need to
> > redisplay the text area as well, for example when the mode line
> > changes its height.  So setting update_mode_lines to REDISPLAY_SOME
> > under the assumption that only the mode line needs to be considered is
> > not necessarily true and can cause redisplay bugs.
> 
> I don't see why you relate this problem to REDISPLAY_SOME: when setting
> update_mode_lines to other values, xdisp.c should suffer from the same
> problem (it presumably updates the mode-lines of all windows without
> updating the corresponding window's contents).

My point is that the documentation says REDISPLAY_SOME causes only the
mode line(s) to be updated, but the idea that changes which affect the
mode line could be handled by using REDISPLAY_SOME is incorrect
because it assumes the windows above and below will never be affected
by such changes.  So the idea itself is flawed, albeit in very rare
and somewhat unusual use cases.

I hoped that this will lead to the conclusion that the current
partition of possible use cases and its translation into specific sets
of values of the flags and variables we have is at least inaccurate
and incomplete, if not worse.  With the implied realization that we
need to rethink this and then reimplement it.

> >   consider_all_windows_p = (update_mode_lines
> >                         || windows_or_buffers_changed);
> >   [...]
> >   if (consider_all_windows_p)
> >     {
> >       FOR_EACH_FRAME (tail, frame)
> >     XFRAME (frame)->updated_p = false;
> >
> >       propagate_buffer_redisplay ();
> >
> >       FOR_EACH_FRAME (tail, frame)
> >     {
> >    [...]
> >
> > If the redisplay flag is all we need, how come we must also set
> > update_mode_lines or windows_or_buffers_changed to get Emacs to
> > consider anything beyond the selected window?
> 
> The `redisplay` bits were designed to reduce the set of windows that we
> consider at each redisplay.

Then why do we need the consider_all_windows_p condition, which is
based on 2 other variables?  It should have been enough to simply go
over all the redisplay flags of all the frames and windows and
buffers, and see if any of them are set for any window other than the
selected window of the selected frame.  Right?

> > Why does it have to be so complicated to say "this frame needs to have
> > all of its windows reconsidered for redisplay"?
> 
> Is it?  AFAIK `fset_redisplay (f)` is all it takes, which doesn't seem
> particularly complex (and neither does its code).

So you are saying that wset_update_mode_line should only set the
frame's redisplay flag?  If so, why didn't your patch to do just that
work?

> >> The main problems I see with my suggested patch are:
> >> - I don't know if it actually fixes the original problem.
> > And this is exactly my problem: this is the "heuristic" part I was
> > talking about.  Instead of knowing exactly which flag does what and
> > why, we have a combination of flags and global variables, and try
> > tweaking them until we get the desired result.  This can only work up
> > to a point, and I think we are well beyond that point.
> 
> Not sure what you're suggesting here.

I suggest that we should have a system for selectively redisplaying
parts of the Emacs display where we do know which change will fix what
problem.  IOW, we should rethink all of this, remove the flags that
shouldn't be needed, maybe add flags that are missing, and augment/fix
the existing ones which do make sense (that definitely includes the
various redisplay flags we have now, but they are not enough and
perhaps should not be simple booleans).

> [ At least I know what the `redisplay` bits are *supposed* to do.

Yes, I also think I know what each of those variables is _supposed_ to
do.  The problem is, they don't quite do what I think they should, not
always anyway.

> > See, we have a single set of conditions that controls when we consider
> > the frame title, when we consider the mode line, the header-line, the
> > tab-line, the tool bar, and the menu bar.  It makes very little sense
> > to me to use the same condition for all of these.
> 
> I think it makes a lot of sense from the point of view of managing
> code complexity.  But indeed, it leaves open optimization opportunities,
> so we could refine the info used to keep track of what needs to
> be redisplayed.

And that's because the flags we use, however numerous, are too blunt
for selectively specifying which parts to redisplay.  Which AFAIU is
the crux of this bug report.

> >> > I'm not against experimenting with replacing 42 by 32 or by
> >> > REDISPLAY_SOME etc., but I don't think we should install anything
> >> > along these lines, except if we need to fix a clear bug (i.e. a
> >> > redisplay glitch), which this one isn't.
> >> I don't know what you mean by "along these lines".
> > "Along these lines" means playing more games with "special" values of
> > update_mode_lines and windows_or_buffers_changed.
> 
> I don't know what you mean by "special values".
> And I'm not playing any games here.

This is not useful: you are responding to the specific words I used
instead of responding to what I meant (which I think is fairly
obvious).

> The meaning of those vars is as follows:
> 
> - update_mode_lines == 0 means that none of the mode lines (and
>   relatives) needs to be updated.
> - update_mode_lines > 2 means that all the mode lines in all windows
>   need to be updated.
> - update_mode_lines == 2 means that all the mode lines need to be
>   updated in the set designated by the `redisplay` bits (where the
>   `redisplay` on a frame means that all of its windows are also part opf
>   the set, and where the `redisplay` bit of a buffer means that all the
>   windows that display this buffer are also part of the set).
> 
> - windows_or_buffers_changed == 0 means that only the selected window's
>   content may need to be updated.

Yes, I know.  The comments you provided tell this much.  The problem
is, the reality is subtly and annoyingly different, and that is not
good for maintainability.

> - update_mode_lines > 2 means that the contents in all windows
>   may need to be updated.
> - update_mode_lines == 2 means that the contents in all windows in the
>   set designated by the `redisplay` bits may need to be updated.

Copy/paste? did you mean windows_or_buffers_changed?