bug#5718: scroll-margin in buffer with small line count.

[npostavs]

Eli Zaretskii <eliz@gnu.org> writes:

>
>> The issues with `scroll-down-command' (and `next-line', below) remain.
>> I find the following change fixes the problem for `scroll-down-command',
>> though I'm not sure whether it's the right thing to do.
>> 
>> --- a/src/window.c
>> +++ b/src/window.c
>> @@ -5148,7 +5148,7 @@ window_scroll_pixel_based (Lisp_Object window, int n, 
>> bool whole, bool noerror)
>>          in the scroll margin at the bottom.  */
>>        move_it_to (&it, PT, -1,
>>                   (it.last_visible_y - WINDOW_HEADER_LINE_HEIGHT (w)
>> -                  - this_scroll_margin - 1),
>> +                  - this_scroll_margin - 1 - frame_line_height),
>>                   -1,
>>                   MOVE_TO_POS | MOVE_TO_Y);
>
> Can you explain why this fixes the problem?  IOW, what was the problem
> with the original code, and why moving to the previous screen line
> here solves the problem?
>
> Also, does this correction need to be conditional on the new variable
> in some way, or is it correct in the general case as well?

Actually, this is also a problem with partial lines.  I didn't notice at
first, because Emacs produces a window with a partial line by default
when I do `C-x 2'.  AFAICT, it's not related with the new variable,
except that the problem becomes more obvious with maximal margins.  In
Emacs 25.1, using a window where (window-screen-lines) => 20.22, and a
scroll-margin of 100, doing `scroll-down-command' leaves 5 full lines
above point, while `scroll-up-command' leaves 4 full lines (and the
partial one) below point.

So in the code, I think the problem is that it.last_visible_y includes
the partial line, while this_scroll_margin is just the integer
scroll_margin multiplied by pixels per line.  I guess to fix it,
subtracting the partial line height would be more correct than
frame_line_height.

>> 
>> The problem involves partial lines.  In a window where
>> (window-screen-lines) returns 7.222, doing M-: (vertical-motion '(0.0
>> . 1)) does not scroll the window, which lets point end up 1 line away
>> from the center.  Doing M-: (vertical-motion '(0.0 . -1)) does scroll
>> the window, keeping the point in the center (as expected).
>> 
>> Adjusting the window so that (window-screen-lines) returns 7.0, there is
>> no discrepancy between up and down motion.
>> 
>> I guess there is some incorrect boundary condition in try_scrolling,
>> though I haven't worked out where.

Looks like the same kind of problem as the other case, I can fix it
again by subtracting frame_line_height, though again, subtracting the
partial height is probably more correct.

---   i/src/xdisp.c
+++   w/src/xdisp.c
@@ -15369,7 +15369,7 @@ try_scrolling (Lisp_Object window, bool just_this_one_p,
         either that ypos or PT, whichever comes first.  */
       start_display (&it, w, startp);
       scroll_margin_y = it.last_visible_y - this_scroll_margin
-        - frame_line_height * extra_scroll_margin_lines;
+        - frame_line_height * (1 + extra_scroll_margin_lines);
       move_it_to (&it, PT, -1, scroll_margin_y - 1, -1,
                  (MOVE_TO_POS | MOVE_TO_Y));