Re: scroll-preserve-screen-position and tall lines

[Chong Yidong]

An old bug:

Ralf Angeli <address@hidden> wrote:

> When `scroll-preserve-screen-position' is set to `always' Emacs
> doesn't manage to keep the vertical position of point relative to
> window boundaries if tall lines are involved.  Here is a testcase:
>
> (progn
>   (setq scroll-preserve-screen-position 'always)
>   (pop-to-buffer (get-buffer-create "*test*"))
>   (dotimes (i 50) (insert (concat (number-to-string i) "\n")))
>   (insert "TALL\n")
>   (put-text-property (line-beginning-position 0) (line-end-position 0)
>                      'face '(:height 2.0 :inherit variable-pitch))
>   (dotimes (i 50) (insert (concat (number-to-string (+ i 50)) "\n")))
>   (search-backward "TALL"))
>
> After executing it, scroll the buffer by one line with `C-u 1 C-v'.
> Notice that point does not keep its vertical position but moves
> upwards together with the string "TALL".  (`scroll-down' does not seem
> to be affected.)

How about making window_scroll_pixel_based preserve the value of vpos
instead of current_y?  This patch removes the above bug.

*** emacs/src/window.c.~1.535.~ 2006-02-18 10:23:42.000000000 -0500
--- emacs/src/window.c  2006-02-21 10:41:23.000000000 -0500
***************
*** 4720,4726 ****
    struct text_pos start;
    Lisp_Object tem;
    int this_scroll_margin;
!   int preserve_y;
    /* True if we fiddled the window vscroll field without really scrolling.   
*/
    int vscrolled = 0;
  
--- 4720,4726 ----
    struct text_pos start;
    Lisp_Object tem;
    int this_scroll_margin;
!   int old_vpos;
    /* True if we fiddled the window vscroll field without really scrolling.   
*/
    int vscrolled = 0;
  
***************
*** 4788,4797 ****
      {
        start_display (&it, w, start);
        move_it_to (&it, PT, -1, -1, -1, MOVE_TO_POS);
!       preserve_y = it.current_y;
      }
    else
!     preserve_y = -1;
  
    /* Move iterator it from start the specified distance forward or
       backward.  The result is the new window start.  */
--- 4788,4797 ----
      {
        start_display (&it, w, start);
        move_it_to (&it, PT, -1, -1, -1, MOVE_TO_POS);
!       old_vpos = it.vpos;
      }
    else
!     old_vpos = -1;
  
    /* Move iterator it from start the specified distance forward or
       backward.  The result is the new window start.  */
***************
*** 4921,4934 ****
              || EQ (Vscroll_preserve_screen_position, Qt)))
        /* We found PT at a legitimate height.  Leave it alone.  */
        ;
!       else if (preserve_y >= 0)
        {
!         /* If we have a header line, take account of it.
!            This is necessary because we set it.current_y to 0, above.  */
!         if (WINDOW_WANTS_HEADER_LINE_P (w))
!           preserve_y -= CURRENT_HEADER_LINE_HEIGHT (w);
! 
!         move_it_to (&it, -1, -1, preserve_y, -1, MOVE_TO_Y);
          SET_PT_BOTH (IT_CHARPOS (it), IT_BYTEPOS (it));
        }
        else
--- 4921,4929 ----
              || EQ (Vscroll_preserve_screen_position, Qt)))
        /* We found PT at a legitimate height.  Leave it alone.  */
        ;
!       else if (old_vpos >= 0)
        {
!         move_it_to (&it, -1, -1, -1, old_vpos, MOVE_TO_VPOS);
          SET_PT_BOTH (IT_CHARPOS (it), IT_BYTEPOS (it));
        }
        else
***************
*** 4948,4954 ****
        int charpos, bytepos;
        int partial_p;
  
!       /* Save our position, for the preserve_y case.  */
        charpos = IT_CHARPOS (it);
        bytepos = IT_BYTEPOS (it);
  
--- 4943,4949 ----
        int charpos, bytepos;
        int partial_p;
  
!       /* Save our position, for the old_vpos case.  */
        charpos = IT_CHARPOS (it);
        bytepos = IT_BYTEPOS (it);
  
***************
*** 4978,4997 ****
              || EQ (Vscroll_preserve_screen_position, Qt)))
        /* We found PT before we found the display margin, so PT is ok.  */
        ;
!       else if (preserve_y >= 0)
        {
          SET_TEXT_POS_FROM_MARKER (start, w->start);
          start_display (&it, w, start);
! #if 0  /* It's wrong to subtract this here
!         because we called start_display again
!         and did not alter it.current_y this time.  */
! 
!         /* If we have a header line, take account of it.  */
!         if (WINDOW_WANTS_HEADER_LINE_P (w))
!           preserve_y -= CURRENT_HEADER_LINE_HEIGHT (w);
! #endif
! 
!         move_it_to (&it, -1, -1, preserve_y, -1, MOVE_TO_Y);
          SET_PT_BOTH (IT_CHARPOS (it), IT_BYTEPOS (it));
        }
        else
--- 4973,4983 ----
              || EQ (Vscroll_preserve_screen_position, Qt)))
        /* We found PT before we found the display margin, so PT is ok.  */
        ;
!       else if (old_vpos >= 0)
        {
          SET_TEXT_POS_FROM_MARKER (start, w->start);
          start_display (&it, w, start);
!         move_it_to (&it, -1, -1, -1, old_vpos, MOVE_TO_VPOS);
          SET_PT_BOTH (IT_CHARPOS (it), IT_BYTEPOS (it));
        }
        else