set-window-vscroll causes infinite loop in redisplay

set-window-vscroll causes infinite loop in redisplay

[Kim F. Storm]

This is with CVS head on GNU/Linux under X.

1) start emacs

emacs -q

2) get rid of splash screen

RET

3) In scratch buffer, enter:

(set-window-vscroll nil 0.5) C-j

4) (try to) move cursor to the first (partially visible) line.

C-p C-p C-p C-p C-p C-p C-p C-p

5) emacs loops.



The following patch fixes this.  I'll install it tomorrow unless
someone finds a better fix.

Index: xdisp.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/xdisp.c,v
retrieving revision 1.806
diff -c -r1.806 xdisp.c
*** xdisp.c	18 Feb 2003 00:17:17 -0000	1.806
--- xdisp.c	24 Feb 2003 00:31:05 -0000
***************
*** 10821,10826 ****
--- 10821,10834 ----
  
    if (!make_cursor_line_fully_visible (w))
      {
+       /* If vscroll is non-zero, and the cursor is on the top line,
+ 	 disable vscroll, and retry. */
+       if (w->vscroll < 0 && w->cursor.y < 0)
+ 	{
+ 	  w->vscroll = 0;
+ 	  clear_glyph_matrix (w->desired_matrix);
+ 	}
+ 
        /* If centering point failed to make the whole line visible,
  	 put point at the top instead.  That has to make the whole line
  	 visible, if it can be done.  */

Re: set-window-vscroll causes infinite loop in redisplay

[Kim F. Storm]

storm@cua.dk (Kim F. Storm) writes:

> This is with CVS head on GNU/Linux under X.
> 
> 1) start emacs
> 
> emacs -q
> 
> 2) get rid of splash screen
> 
> RET
> 
> 3) In scratch buffer, enter:
> 
> (set-window-vscroll nil 0.5) C-j
> 
> 4) (try to) move cursor to the first (partially visible) line.
> 
> C-p C-p C-p C-p C-p C-p C-p C-p
> 
> 5) emacs loops.
> 
> 
> 
> The following patch fixes this.  I'll install it tomorrow unless
> someone finds a better fix.

I think the following fix is slightly better -- it retries the
recenter if vscroll was enabled.  But it probably doesn't make much
visible difference...

Index: xdisp.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/xdisp.c,v
retrieving revision 1.807
diff -c -r1.807 xdisp.c
*** xdisp.c	24 Feb 2003 07:57:55 -0000	1.807
--- xdisp.c	24 Feb 2003 14:46:05 -0000
***************
*** 10822,10827 ****
--- 10822,10835 ----
  
    if (!make_cursor_line_fully_visible (w))
      {
+       /* If vscroll is enabled, disable it and try again.  */
+       if (w->vscroll)
+ 	{
+ 	  w->vscroll = 0;
+ 	  clear_glyph_matrix (w->desired_matrix);
+ 	  goto recenter;
+ 	}
+       
        /* If centering point failed to make the whole line visible,
  	 put point at the top instead.  That has to make the whole line
  	 visible, if it can be done.  */


-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: set-window-vscroll causes infinite loop in redisplay

[Kim F. Storm]

storm@cua.dk (Kim F. Storm) writes:

> I think the following fix is slightly better -- it retries the
> recenter if vscroll was enabled.  But it probably doesn't make much
> visible difference...

I just committed the fix.

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: set-window-vscroll causes infinite loop in redisplay

[Kim F. Storm]

"D. Goel" <deego@gnufans.org> writes:


> hello
> 
> I can get set-window-vscroll to work from M-: or from inside functions
> containing that one single command.
> 
> Example:
> 
> (defun vel-scroll-up-fractional (acc)
>     (set-window-vscroll nil acc))
> 
> works just fine when acc is 0.5
> 
> However, 
> try
> 
> (defun vel-scroll-up (amount acc)	
>   (scroll-up amount)	
>   (set-window-vscroll nil acc))
> 
> where amount is 0 and acc is 0.5.  (the first one is a dummy line
> supposed to do nothing here..)
> 
> And the second line seems to have no effect at all.
> 

I can confirm this (not) happening here too.

The problem seems to that after a scroll (even a zero scroll), the
function window_scroll_pixel_based sets w->force_start = Qt, which
later causes the redisplay engine to set w->vscroll to 0.

Since this happens during redisplay, the set-window-vscroll
function has already been called, and the explicit setting of
w->vscroll is lost.

The following version is one way to work-around this problem:
(defun vel-scroll-up (amount acc)	
  (scroll-up amount)	
  (sit-for 0)
  (set-window-vscroll nil acc))

I think the proper fix is to define a separate w->vscroll_extra member
(controlled via set-window-vscroll) which specifies the smooth scroll
offset the user wants ... and let the automatic vscroll adjustment
take this into account (except at the top and bottom of the buffer).

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: set-window-vscroll causes infinite loop in redisplay

[Richard Stallman]

    The following patch fixes this.  I'll install it tomorrow unless
    someone finds a better fix.

I don't see anything obviously wrong with it.