Avoid recentering when user says so

[Eli Zaretskii]

This is related to bug #6671, read there if you want to know the gory
details.

In a nutshell, Emacs currently doesn't honor meticulously enough user
settings such as scroll-conservatively and scroll-up/down-aggressively.
Sometimes, especially if redisplay cannot keep up with high-rate
keyboard input, it falls back on recentering, which these variables
are supposed to avoid.  The last attempt to solve this introduced a
bug that is the subject of bug #6671.

Following the latest discussions in that bug report, I came up with a
way to solve this problem without incurring unduly slow movement far
away in the buffer.  The patch below seems to give good results, but
as I don't use the above settings (I like the default recentering),
I'm not a good candidate for testing the patch.

Would people who use these variables please try the patch below, and
report any findings, both positive and negative?  Please report your
findings to the bug tracker at address@hidden, so that the
details get filed there.

Given positive feedback, I will commit this to the trunk.

Thanks.

=== modified file 'src/xdisp.c'
--- src/xdisp.c 2011-03-26 12:20:20 +0000
+++ src/xdisp.c 2011-03-26 18:07:58 +0000
@@ -13003,6 +13003,9 @@ enum
   SCROLLING_NEED_LARGER_MATRICES
 };
 
+/* If scroll-conservatively is more than this, never recenter.  */
+#define SCROLL_LIMIT 100
+
 static int
 try_scrolling (Lisp_Object window, int just_this_one_p,
               EMACS_INT arg_scroll_conservatively, EMACS_INT scroll_step,
@@ -13016,7 +13019,8 @@ try_scrolling (Lisp_Object window, int j
   int dy = 0, amount_to_scroll = 0, scroll_down_p = 0;
   int extra_scroll_margin_lines = last_line_misfit ? 1 : 0;
   Lisp_Object aggressive;
-  int scroll_limit = INT_MAX / FRAME_LINE_HEIGHT (f);
+  /* We will never try scrolling more than this number of lines.  */
+  int scroll_limit = SCROLL_LIMIT;
 
 #if GLYPH_DEBUG
   debug_method_add (w, "try_scrolling");
@@ -13032,14 +13036,14 @@ try_scrolling (Lisp_Object window, int j
   else
     this_scroll_margin = 0;
 
-  /* Force arg_scroll_conservatively to have a reasonable value, to avoid
-     overflow while computing how much to scroll.  Note that the user
-     can supply scroll-conservatively equal to `most-positive-fixnum',
-     which can be larger than INT_MAX.  */
+  /* Force arg_scroll_conservatively to have a reasonable value, to
+     avoid scrolling too far away with slow move_it_* functions.  Note
+     that the user can supply scroll-conservatively equal to
+     `most-positive-fixnum', which can be larger than INT_MAX.  */
   if (arg_scroll_conservatively > scroll_limit)
     {
-      arg_scroll_conservatively = scroll_limit;
-      scroll_max = INT_MAX;
+      arg_scroll_conservatively = scroll_limit + 1;
+      scroll_max = scroll_limit * FRAME_LINE_HEIGHT (f);
     }
   else if (scroll_step || arg_scroll_conservatively || temp_scroll_step)
     /* Compute how much we should try to scroll maximally to bring
@@ -13076,7 +13080,7 @@ try_scrolling (Lisp_Object window, int j
          /* Compute how many pixels below window bottom to stop searching
             for PT.  This avoids costly search for PT that is far away if
             the user limited scrolling by a small number of lines, but
-            always finds PT if arg_scroll_conservatively is set to a large
+            always finds PT if scroll_conservatively is set to a large
             number, such as most-positive-fixnum.  */
          int slack = max (scroll_max, 10 * FRAME_LINE_HEIGHT (f));
          int y_to_move =
@@ -13128,12 +13132,12 @@ try_scrolling (Lisp_Object window, int j
        return SCROLLING_FAILED;
 
       start_display (&it, w, startp);
-      if (scroll_max < INT_MAX)
+      if (arg_scroll_conservatively <= scroll_limit)
        move_it_vertically (&it, amount_to_scroll);
       else
        {
          /* Extra precision for users who set scroll-conservatively
-            to most-positive-fixnum: make sure the amount we scroll
+            to a large number: make sure the amount we scroll
             the window start is never less than amount_to_scroll,
             which was computed as distance from window bottom to
             point.  This matters when lines at window top and lines
@@ -14201,11 +14205,10 @@ redisplay_window (Lisp_Object window, in
        }
     }
 
-  /* Finally, just choose place to start which centers point */
+  /* Finally, just choose a place to start which positions point
+     according to user preferences.  */
 
  recenter:
-  if (centering_position < 0)
-    centering_position = window_box_height (w) / 2;
 
 #if GLYPH_DEBUG
   debug_method_add (w, "recenter");
@@ -14217,10 +14220,62 @@ redisplay_window (Lisp_Object window, in
   if (!buffer_unchanged_p)
     w->base_line_number = Qnil;
 
-  /* Move backward half the height of the window.  */
+  /* Determine the window start relative to point.  */
   init_iterator (&it, w, PT, PT_BYTE, NULL, DEFAULT_FACE_ID);
   it.current_y = it.last_visible_y;
+  if (centering_position < 0)
+    {
+      int margin =
+       scroll_margin > 0
+       ? min (scroll_margin, WINDOW_TOTAL_LINES (w) / 4)
+       : 0;
+      int scrolling_up = PT > CHARPOS (startp) + margin;
+      Lisp_Object aggressive =
+       scrolling_up
+       ? BVAR (current_buffer, scroll_up_aggressively)
+       : BVAR (current_buffer, scroll_down_aggressively);
+
+      if (!MINI_WINDOW_P (w)
+         && scroll_conservatively > SCROLL_LIMIT || NUMBERP (aggressive))
+       {
+         int pt_offset = 0;
+
+         /* Setting scroll-conservatively overrides
+            scroll-*-aggressively.  */
+         if (!scroll_conservatively && NUMBERP (aggressive))
+           {
+             double float_amount = XFLOATINT (aggressive);
+
+             pt_offset = float_amount * WINDOW_BOX_TEXT_HEIGHT (w);
+             if (pt_offset == 0 && float_amount > 0)
+               pt_offset = 1;
+             if (pt_offset)
+               margin -= 1;
+           }
+         /* Compute how much to move the window start backward from
+            point so that point will be displayed where the user
+            wants it.  */
+         if (scrolling_up)
+           {
+             centering_position = it.last_visible_y;
+             if (pt_offset)
+               centering_position -= pt_offset;
+             centering_position -=
+               FRAME_LINE_HEIGHT (f) * (1 + margin + (last_line_misfit != 0));
+             /* Don't let point enter the scroll margin near top of
+                the window.  */
+             if (centering_position < margin * FRAME_LINE_HEIGHT (f))
+               centering_position = margin * FRAME_LINE_HEIGHT (f);
+           }
+         else
+           centering_position = margin * FRAME_LINE_HEIGHT (f) + pt_offset;
+       }
+      else
+       /* Move backward from point half the height of the window.  */
+       centering_position = window_box_height (w) / 2;
+    }
   move_it_vertically_backward (&it, centering_position);
+
   xassert (IT_CHARPOS (it) >= BEGV);
 
   /* The function move_it_vertically_backward may move over more
@@ -14237,8 +14292,9 @@ redisplay_window (Lisp_Object window, in
 
   it.current_x = it.hpos = 0;
 
-  /* Set startp here explicitly in case that helps avoid an infinite loop
-     in case the window-scroll-functions functions get errors.  */
+  /* Set the window start position here explicitly, to avoid an
+     infinite loop in case the functions in window-scroll-functions
+     get errors.  */
   set_marker_both (w->start, Qnil, IT_CHARPOS (it), IT_BYTEPOS (it));
 
   /* Run scroll hooks.  */