bug#12600: 24.2.50; linum-mode: line numbers in fringe do not refresh when resizing frame

[Eli Zaretskii]

> Date: Mon, 15 Oct 2012 11:41:05 +0200
> From: martin rudalics <rudalics@gmx.at>
> CC: monnier@iro.umontreal.ca, 12600@debbugs.gnu.org
> 
>  > And current_matrix_up_to_date_p is computed thusly:
>  >
>  >   current_matrix_up_to_date_p
>  >     = (!NILP (w->window_end_valid)
>  >        && !current_buffer->clip_changed
>  >        && !current_buffer->prevent_redisplay_optimizations_p
>  >        && w->last_modified >= MODIFF
>  >        && w->last_overlay_modified >= OVERLAY_MODIFF);
>  >
>  >>  > then
>  >>  > "goto done".  try_window_id is called only if there's some change that
>  >>  > requires redisplaying some of the text, not just moving the cursor.
> 
> I got the impression that the big conjunct below
> 
>    if (/* Point may be in this window.  */
>        PT >= CHARPOS (startp)
>        /* Selective display hasn't changed.  */
>        && !current_buffer->clip_changed
>        /* Function force-mode-line-update is used to force a thorough
>        redisplay.  It sets either windows_or_buffers_changed or
>        update_mode_lines.  So don't take a shortcut here for these
>        cases.  */
>        && !update_mode_lines
>        && !windows_or_buffers_changed
>        && !cursor_type_changed
>        /* Can't use this case if highlighting a region.  When a
>           region exists, cursor movement has to do more than just
>           set the cursor.  */
>        && !(!NILP (Vtransient_mark_mode)
>          && !NILP (BVAR (current_buffer, mark_active)))
>        && NILP (w->region_showing)
>        && NILP (Vshow_trailing_whitespace)
>        /* This code is not used for mini-buffer for the sake of the case
>        of redisplaying to replace an echo area message; since in
>        that case the mini-buffer contents per se are usually
>        unchanged.  This code is of no real use in the mini-buffer
>        since the handling of this_line_start_pos, etc., in redisplay
>        handles the same cases.  */
>        && !EQ (window, minibuf_window)
>        /* When splitting windows or for new windows, it happens that
>        redisplay is called with a nil window_end_vpos or one being
>        larger than the window.  This should really be fixed in
>        window.c.  I don't have this on my list, now, so we do
>        approximately the same as the old redisplay code.  --gerd.  */
>        && INTEGERP (w->window_end_vpos)
>        && XFASTINT (w->window_end_vpos) < w->current_matrix->nrows
>        && (FRAME_WINDOW_P (f)
>         || !overlay_arrow_in_current_buffer_p ()))
> 
> indicates all the cases where try_cursor_movement fails immediately.

This is inside try_cursor_movement.  Some simple checks (which are
also useful for considering other optimizations) are done before
try_cursor_movement is called.

>  > I'm not sure I understand where you are getting, but to facilitate
>  > this discussion, let me describe the logic of redisplay_window.  It
>  > goes like this:
>  >
>  >   . if nothing changed except possibly point, call
>  >     try_cursor_movement; if that succeeds, we are done
> 
> But you clumsily check there whether "nothing changed".

The name of the variable, current_matrix_up_to_date_p, is revealing.
If these conditions are true, the current glyph matrix is reusable.
There's nothing clumsy about that.  This variable is used further down
the code.

> So I'd move this to the end.  I'd start checking conditions like
> windows_or_buffers_changed and do the through redisplay immediately.

Conceptually, the order doesn't matter.  Practically, it is better to
first check the feasibility of optimizations that allow us to get away
with cheaper redisplay, because (a) thorough redisplay anyway takes
significantly more time, so there's no need to rush doing it, and 
(b) the tests for the best optimization are faster and cheaper than
those for slower alternatives.

>  >   . otherwise, if the buffer is unchanged, try reusing some of the
>  >     current glyph matrix, assuming that just the window-start has
>  >     changed -- this is what try_window_reusing_current_matrix does
> 
> What happens when the window width has changed?  Is the glyph matrix
> still OK?

No, the glyph matrix needs to be reallocated in that case, which
causes a thorough redisplay (AFAIR).

>  >   . if that fails, call try_window_id, which tries reusing of the
>  >     current glyph matrix, assuming that only some lines at the
>  >     beginning or the end of the window have changed
>  >
>  >   . if that fails, too, call try_window to redisplay the entire window
>  >     using the previous window-start point
> 
> Provided the window-start position is still the same, I assume.

If this assumptions proves false, we call try_scrolling, see below.

> Do we assume that the buffer has changed from here on?

Yes.

>  >   . if try_window finds that point ends up outside the window,
>  >     "scroll" the window, i.e. find a better window-start point such
>  >     that point enters the window -- this is what try_scrolling does
> 
> I suppose this could be called by the next as well?
> 
>  >   . if that fails as well, compute the new window-start and redisplay
>  >     the entire window starting from there

Not normally.  The new window-start is computed so as to make sure
point will appear.  But there are a couple of obscure fallbacks if it
doesn't (e.g., if the whole window shows one very high line, or
there's a lot of invisible text at window-start, etc.).

> OK.  I suppose the order is due to the fact that if applying step N
> fails, you continue with step N + 1.

Right.  And the flags are designed so that testing for potential
applicability of step N is cheaper than for step N+1.

> I thought there are enough cases where one decided statically (that
> is, when redisplay starts) that the last step doing the full
> redisplay is needed anyway and it doesn't pay to try another step
> first.

Some tests are expensive, so we postpone them as much as possible.

>  >> This would mean that try_cursor_movement had instead of
>  >>
>  >>        && !windows_or_buffers_changed
>  >>
>  >> check
>  >>
>  >>        && !w->window_modified
>  >>
>  >> and the respective MODIFF conjuncts for w's buffer.
>  >
>  > What would we gain by this change?
> 
> That for certain windows we could exclude that a redisplay is needed
> because the window's size or the buffer's start position in the window
> changed.  But if you can't use this information there's obviously no
> need providing it.

You can't use it easily.  There are just too many conditions when
redisplay, or at least some part of it, cannot be avoided.

> BTW, I meanwhile discovered that the update_mode_line field (with a
> misleading comment IIUC) is the canonical approach to ask for
> redisplaying a specific window.

That's not my reading of the code.  The w->update_mode_line flag, if
non-zero, forces redisplay of the mode line, and also of the menu bar
and the tool bar, of that window.  Where did you see that its effect
is to redisplay the whole window?

> Or am I wrong again and `force-window-update' never discriminates
> between updating a single or all windows of a frame.

force-window-update with a nil argument sets update_mode_lines and
also windows_or_buffers_changed.  The latter prevents redisplay
optimizations; the former just causes redisplay to consider all
windows, but does not prevent optimizations for each and every window,
if these optimizations are otherwise possible.

force-window-update with a window as its argument just marks that one
window's display "inaccurate", requests redisplay of its mode-line,
and prevents optimizations of any other window displaying the same
buffer.  Windows that don't display this buffer can still avoid some
or all of redisplay through optimizations.

>  > It depends on how you reason about logic.  To me, the condition
>  >
>  >     !NILP (Vtransient_mark_mode)
>  >      && !NILP (BVAR (current_buffer, mark_active))
>  >
>  > is clear, whereas its reverse is less so.
> 
> ... doubling the negation is even less clear ;-)

It's not doubling the negation, it negates a condition with a clear
and easy interpretation.

> Ayway, I now checked in a fix for this bug based on the assumption that
> all involved routines set windows_or_buffers_changed.  Some, like the
> frame resizing functions, never did so and I changed that as well.

Thanks.  I think resizing requests a thorough redisplay anyway, but
setting windows_or_buffers_changed cannot hurt.