Improve accuracy of scrolling commands

* src/xdisp.c (move_it_vertically_backward): Don't rely on
line_bottom_y for accurate calculation of the next screen line's Y
coordinate: it doesn't work when the current screen line was not
yet traversed.  Instead, record the previous Y coordinate and
reseat there if overshoot is detected.
* src/window.c (window_scroll_pixel_based): Calculate the new
window-start point more accurately when screen lines have uneven
height.  (Bug#8355)

diff --git a/src/window.c b/src/window.c
index 8e75e460b2b1e1489cde3f38c120785405b02b79..4eab786958f17d2e3a97abd6f3a78f522d3fffa0 100644 (file)
--- a/src/window.c
+++ b/src/window.c
@@ -5686,27 +5686,20 @@ window_scroll_pixel_based (Lisp_Object window, int n, bool whole, bool noerror)
         we would end up at the start of the line ending at ZV.  */
       if (dy <= 0)
        {
-         goal_y = it.current_y - dy;
+         goal_y = it.current_y + dy;
          move_it_vertically_backward (&it, -dy);
-         /* Extra precision for people who want us to preserve the
-            screen position of the cursor: effectively round DY to the
-            nearest screen line, instead of rounding to zero; the latter
-            causes point to move by one line after C-v followed by M-v,
-            if the buffer has lines of different height.  */
-         if (!NILP (Vscroll_preserve_screen_position)
-             && it.current_y - goal_y > 0.5 * flh)
+         /* move_it_vertically_backward above always overshoots if DY
+            cannot be reached exactly, i.e. if it falls in the middle
+            of a screen line.  But if that screen line is large
+            (e.g., a tall image), it might make more sense to
+            undershoot instead.  */
+         if (goal_y - it.current_y > 0.5 * flh)
            {
              it_data = bidi_shelve_cache ();
-             struct it it2 = it;
-
-             move_it_by_lines (&it, -1);
-             if (it.current_y < goal_y - 0.5 * flh)
-               {
-                 it = it2;
-                 bidi_unshelve_cache (it_data, false);
-               }
-             else
-               bidi_unshelve_cache (it_data, true);
+             struct it it1 = it;
+             if (line_bottom_y (&it1) - goal_y < goal_y - it.current_y)
+               move_it_by_lines (&it, 1);
+             bidi_unshelve_cache (it_data, true);
            }
          /* Ensure we actually do move, e.g. in case we are currently
             looking at an image that is taller that the window height.  */
@@ -5718,8 +5711,11 @@ window_scroll_pixel_based (Lisp_Object window, int n, bool whole, bool noerror)
        {
          goal_y = it.current_y + dy;
          move_it_to (&it, ZV, -1, goal_y, -1, MOVE_TO_POS | MOVE_TO_Y);
-         /* See the comment above, for the reasons of this
-            extra-precision.  */
+         /* Extra precision for people who want us to preserve the
+            screen position of the cursor: effectively round DY to the
+            nearest screen line, instead of rounding to zero; the latter
+            causes point to move by one line after C-v followed by M-v,
+            if the buffer has lines of different height.  */
          if (!NILP (Vscroll_preserve_screen_position)
              && goal_y - it.current_y  > 0.5 * flh)
            {

diff --git a/src/xdisp.c b/src/xdisp.c
index 96dd4fade25090850aa6b897007f8296bf0da25f..699183f3f59d5744fa0771f47f9ea0a0579f5c2b 100644 (file)
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -10301,11 +10301,22 @@ move_it_vertically_backward (struct it *it, int dy)
            move_it_vertically (it, target_y - it->current_y);
          else
            {
+             struct text_pos last_pos;
+             int last_y, last_vpos;
              do
                {
+                 last_pos = it->current.pos;
+                 last_y = it->current_y;
+                 last_vpos = it->vpos;
                  move_it_by_lines (it, 1);
                }
-             while (target_y >= line_bottom_y (it) && IT_CHARPOS (*it) < ZV);
+             while (target_y > it->current_y && IT_CHARPOS (*it) < ZV);
+             if (it->current_y > target_y)
+               {
+                 reseat (it, last_pos, true);
+                 it->current_y = last_y;
+                 it->vpos = last_vpos;
+               }
            }
        }
     }