bug#42406: Mouse-wheel scrolling can be flickering

[Stefan Monnier]

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

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

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

>> > We should stop lumping heuristics one on top another, and instead
>> > redesign this from scratch and make sure that every flag we set is
>> > acted upon as intended, and only in situations we intend them to be
>> > acted upon.  E.g., we should be able to set f->redisplay to a value
>> > that means "update only the frame title".
>> 
>> The `redisplay` bit is not supposed to be a heuristic at all.  It just
>> tried to keep track more precisely of which part of the redisplay may
>> have changed.  `fset_redisplay` marks the frame to be redisplayed at the
>> next redisplay, setting `update_mode_lines` to a non-zero value means
>> that when redisplaying a window we also redisplay its mode line, so
>> the suggested hunk definitely doesn't rely on any kind of heuristic.
>
> Then what is this bit of redisplay_internal about:
>
>   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.  Before them, there were the 2 modes you
described in the TODO: either only consider the selected window or
consider all windows.  The `redisplay` bits only come into play when we
get to the "all windows" case.

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

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

[ At least I know what the `redisplay` bits are *supposed* to do.
  What I meant by "I don't know if it actually fixes the original
  problem" is that I can't reproduce it locally so I need someone else
  to confirm that it fixes the original problem, and this is not because
  I don't understand what the code I changed does, but because I don't
  know enough about the problem to be sure that my fix addresses the
  actual problem.  ]

>> - It can cause *more* redisplay work because it will force redisplay of
>>   all the windows in the current frame (rather than only their mode
>>   lines).
>
> 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.

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

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


        Stefan