bug#70038: 29.3.50; Shift up/down in buffer with images on M-x other-window with some fonts

[Eli Zaretskii <eliz@gnu.org>]

> Date: Sun, 14 Apr 2024 10:31:15 +0200
> Cc: luangruo@yahoo.com, 70038@debbugs.gnu.org
> From: martin rudalics <rudalics@gmx.at>
> 
>  > The (slight) harm this does is that it might make redisplay of the
>  > window slightly more expensive, because it disables some
>  > optimizations, like if nothing changed except the position of point.
>  > Another kind of harm is what triggered this bug report: it could cause
>  > us to reset the window's vscroll for now good reason, because when
>  > start positions are frozen, we set the window's force_start flag, and
>  > that sometimes forces us to reset vscroll.
> 
>  From looking at the code it's hard to understand what you say here.  It
> might be a good idea to add this as a comment.

If the window is frozen, we do this:

  /* If someone specified a new starting point but did not insist,
     check whether it can be used.  */
  if ((w->optional_new_start || window_frozen_p (w))
      && CHARPOS (startp) >= BEGV
      && CHARPOS (startp) <= ZV)
    {
      ptrdiff_t it_charpos;

      w->optional_new_start = false;
      if (!w->force_start)
	{
	[...]
	  /* Make sure we set the force_start flag only if the cursor
	     row will be fully visible.  Otherwise, the code under
	     force_start label below will try to move point back into
	     view, which is not what the code which sets
	     optional_new_start wants.  */
	  if (it.current_y == 0 || line_bottom_y (&it) < it.last_visible_y)
	    {
	      if (it_charpos == PT)
		w->force_start = true;
	      /* IT may overshoot PT if text at PT is invisible.  */
	      else if (it_charpos > PT && CHARPOS (startp) <= PT)
		w->force_start = true;

So this sets w->force_start.  Then the code under this condition will
be executed:

  /* Handle case where place to start displaying has been specified,
     unless the specified location is outside the accessible range.  */
  if (w->force_start)

That code calls try_window, so it's more expensive than the
optimization under this condition:

 ignore_start:

  /* Handle case where text has not changed, only point, and it has
     not moved off the frame, and we are not retrying after hscroll.
     (current_matrix_up_to_date_p is true when retrying.)  */
  if (current_matrix_up_to_date_p
      && (rc = try_cursor_movement (window, startp, &temp_scroll_step),
	  rc != CURSOR_MOVEMENT_CANNOT_BE_USED))

which is what we want to do when text has not changed and point didn't
move far away.

Did I succeed explaining the issue?

>  > So the patch below is the only change we need to make sure the frame's
>  > frozen_window_starts is reset when the mini-window is resized back?
> 
> Probably not.  If 'resize-mini-windows' is t, we never unfreeze window
> starts because we don't call shrink_mini_window in that case.
> 
>  > Should it matter whether, if the minibuffer is active, we do that only
>  > at the outer level of minibuffer?
> 
> It shouldn't.  Window starts would be frozen immediately as soon as the
> mini window grows again.  I think that to cover most cases we should
> unfreeze window starts every time the mini window gets smaller as in the
> patch below.

Is that in addition to what I suggested to do in shrink_mini_window?

Also, shouldn't we do this instead:

> -      FRAME_WINDOWS_FROZEN (f) = true;
> +      FRAME_WINDOWS_FROZEN (f) = (old_height + delta > min_height) ? true : false;

IOW, shouldn't we unfreeze only when resizing to the default one-line
height?