Re: The unwarranted scrolling assumption

[Lennart Borgman <lennart.borgman@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1227 bytes --]

On Fri, Jun 18, 2010 at 6:39 PM, Lennart Borgman
<lennart.borgman@gmail.com> wrote:
> On Fri, Jun 18, 2010 at 5:35 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>>> From: Lennart Borgman <lennart.borgman@gmail.com>
>>> Date: Fri, 18 Jun 2010 17:08:18 +0200
>>> Cc: emacs-devel@gnu.org
>>>
>>> You are looking for the reason why reconsider_clip_changes does not do
>>> its job. I have said (though maybe not explicitly enough, I don't
>>> know) that it can't because it does not have the information it needs.
>>>
>>> It does not have that information because narrow_to_region has erased
>>> it by setting clip_changed to 1.
>>
>> The information is not erased, it is still present in
>> w->current_matrix->zv and w->current_matrix->begv.  You will see that
>> reconsider_clip_changes uses those to decide whether to reset the
>> clip_changed flag.
>
>
> Oh, fine, I missed that. Then we do not need the new variables I introduced.

I have switched to using those variables instead. Seems to work just
as well as my old patch.

I have attached the patches for the current version of xdisp.c I am
using now. Beside using those variable I have moved the check into
reconsider_clip_changes where it belongs.

[-- Attachment #2: xdisp-jumpscroll-1.diff --]
[-- Type: text/x-patch, Size: 5826 bytes --]

=== modified file 'src/xdisp.c'
--- trunk/src/xdisp.c	2010-06-01 02:34:49 +0000
+++ patched/src/xdisp.c	2010-06-18 17:21:15 +0000
@@ -11711,12 +11711,33 @@
      struct window *w;
      struct buffer *b;
 {
+  /* if ((b->clip_begv != b->begv) || (b->clip_zv != b->zv)) */
+  /*   b->clip_changed = 1; */
+
+  if ((w->current_matrix->begv != b->begv) || (w->current_matrix->zv != b->zv))
+    b->clip_changed = 1;
+
+  if (b->clip_changed
+      && !(!NILP (w->window_end_valid)
+	   && w->current_matrix->buffer == b
+	   && w->current_matrix->zv == BUF_ZV (b)
+	   && w->current_matrix->begv == BUF_BEGV (b)))
+    DebPrint (("+++reconsider: end_valid=%d, b=%d, ZV=%d, BEGV=%d",
+               !NILP (w->window_end_valid),
+               w->current_matrix->buffer == b,
+               w->current_matrix->zv == BUF_ZV (b),
+               w->current_matrix->begv == BUF_BEGV (b)));
+
   if (b->clip_changed
 	   && !NILP (w->window_end_valid)
 	   && w->current_matrix->buffer == b
 	   && w->current_matrix->zv == BUF_ZV (b)
 	   && w->current_matrix->begv == BUF_BEGV (b))
+    {
     b->clip_changed = 0;
+      b->clip_begv = b->begv;
+      b->clip_zv = b->zv;
+    }
 
   /* If display wasn't paused, and W is not a tool bar window, see if
      point has been moved into or out of a composition.  In that case,
@@ -13443,7 +13464,10 @@
 	  dy = line_bottom_y (&it) - y0;
 
 	  if (dy > scroll_max)
+            {
+              DebPrint (("try_scrolling SCROLLING_FAILED dy > scroll_max A"));
 	    return SCROLLING_FAILED;
+            }
 
 	  scroll_down_p = 1;
 	}
@@ -13475,7 +13499,10 @@
 	}
 
       if (amount_to_scroll <= 0)
+        {
+          DebPrint (("try_scrolling SCROLLING_FAILED amount_to_scroll <= 0 A"));
 	return SCROLLING_FAILED;
+        }
 
       start_display (&it, w, startp);
       move_it_vertically (&it, amount_to_scroll);
@@ -13515,7 +13542,10 @@
 		      MOVE_TO_POS | MOVE_TO_X | MOVE_TO_Y);
 	  dy = it.current_y - y0;
 	  if (dy > scroll_max)
+            {
+              DebPrint (("try_scrolling SCROLLING_FAILED dy > scroll_max B"));
 	    return SCROLLING_FAILED;
+            }
 
 	  /* Compute new window start.  */
 	  start_display (&it, w, startp);
@@ -13539,7 +13569,10 @@
 	    }
 
 	  if (amount_to_scroll <= 0)
+            {
+              DebPrint (("try_scrolling SCROLLING_FAILED amount_to_scroll <= 0 B"));
 	    return SCROLLING_FAILED;
+            }
 
 	  move_it_vertically_backward (&it, amount_to_scroll);
 	  startp = it.current.pos;
@@ -13556,6 +13589,7 @@
   else if (w->cursor.vpos < 0)
     {
       clear_glyph_matrix (w->desired_matrix);
+      DebPrint (("try_scrolling SCROLLING_FAILED clear_glyph"));
       rc = SCROLLING_FAILED;
     }
   else
@@ -14235,7 +14269,10 @@
 
   /* If window-start is screwed up, choose a new one.  */
   if (XMARKER (w->start)->buffer != current_buffer)
+    {
+      DebPrint (("redisplay_window.recenter window-start screwed up"));
     goto recenter;
+    }
 
   SET_TEXT_POS_FROM_MARKER (startp, w->start);
 
@@ -14389,6 +14426,7 @@
 #if GLYPH_DEBUG
       debug_method_add (w, "recenter 1");
 #endif
+      DebPrint (("redisplay_window.recenter orig beg of line, but not now"));
       goto recenter;
     }
 
@@ -14515,6 +14553,14 @@
       && CHARPOS (startp) >= BEGV
       && CHARPOS (startp) <= ZV)
     {
+      DebPrint (("redisplay_window b try_scr, clip_changed=%d, >= BEGV: %d, <= ZV: %d, jtop=%d, llmf=%d",
+                 current_buffer->clip_changed,
+                 CHARPOS (startp) >= BEGV,
+                 CHARPOS (startp) <= ZV,
+                 just_this_one_p,
+                 last_line_misfit
+                 ));
+
       /* The function returns -1 if new fonts were loaded, 1 if
 	 successful, 0 if not successful.  */
       int rc = try_scrolling (window, just_this_one_p,
@@ -14530,6 +14576,7 @@
 	  goto need_larger_matrices;
 
 	case SCROLLING_FAILED:
+          DebPrint (("redisplay_window.SCROLLING_FAILED"));
 	  break;
 
 	default:
@@ -14539,6 +14586,7 @@
 
   /* Finally, just choose place to start which centers point */
 
+  DebPrint (("redisplay_window.recenter centering_position=%d", centering_position));
  recenter:
   if (centering_position < 0)
     centering_position = window_box_height (w) / 2;
@@ -14644,6 +14692,7 @@
 	{
 	  w->vscroll = 0;
 	  clear_glyph_matrix (w->desired_matrix);
+          DebPrint (("redisplay_window.recenter disable vscroll and try again"));
 	  goto recenter;
 	}
 
@@ -14655,6 +14704,7 @@
 
       clear_glyph_matrix (w->desired_matrix);
       centering_position = 0;
+      DebPrint (("redisplay_window.recenter failed to make whole line visible"));
       goto recenter;
     }
 
@@ -14852,8 +14902,11 @@
       if (display_line (&it))
 	last_text_row = it.glyph_row - 1;
       if (fonts_changed_p && !(flags & TRY_WINDOW_IGNORE_FONTS_CHANGE))
+        {
+          DebPrint (("try_window: fonts_changed_p=%d", fonts_changed_p));
 	return 0;
     }
+    }
 
   /* Don't let the cursor end in the scroll margins.  */
   if ((flags & TRY_WINDOW_CHECK_MARGINS)
@@ -14881,6 +14934,7 @@
 	{
 	  w->cursor.vpos = -1;
 	  clear_glyph_matrix (w->desired_matrix);
+          DebPrint (("try_window returning -1"));
 	  return -1;
 	}
     }
@@ -26279,7 +26333,8 @@
 recenters point as usual.
 
 A value of zero means always recenter point if it moves off screen.  */);
-  scroll_conservatively = 0;
+  // scroll_conservatively = Vmost_positive_fixnum;
+  scroll_conservatively = make_number (MOST_POSITIVE_FIXNUM);
 
   DEFVAR_INT ("scroll-margin", &scroll_margin,
     doc: /* *Number of lines of margin at the top and bottom of a window.