* bug#25792: 26.0.50; Scroll-margin working incorrectly when scrolling down if show-trailing-whitespace is t
@ 2017-02-19 15:34 Alexander Miller
2017-02-19 16:59 ` npostavs
0 siblings, 1 reply; 9+ messages in thread
From: Alexander Miller @ 2017-02-19 15:34 UTC (permalink / raw)
To: 25792
Only scrolling down is affected. The issue first appeared when I
recompiled emacs roughly 1 week ago and has persisted over multiple
recompilations on 2 different laptops (both running Antergos & Xfce), so
my guess is that it's an issue in the current emacs master.
The bug can be reproduced as follows:
1) emacs -q
2) Create enough newlines to scroll down
3) (setq show-trailing-whitespace t scroll-margin 10
scroll-conservatively 101)
4) Go to the top of the buffer and scroll down with C-n
5) When scrolling the cursor will first move one line less than the
margin away from the window bottom.
6) Press C-n again and the margin is used correclty, resulting in the
cursor visually jumping back a line.
7) The end result is that scrolling downward this way is a jittery mess
with the cursor constantly jumping back and forth.
In GNU Emacs 26.0.50 (build 1, x86_64-unknown-linux-gnu, GTK+ Version
3.22.8)
of 2017-02-19 built on a-laptop
Repository revision: 938426d1ca0930b859c3e37b26513f5d74761284
Windowing system distributor 'The X.Org Foundation
^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#25792: 26.0.50; Scroll-margin working incorrectly when scrolling down if show-trailing-whitespace is t
2017-02-19 15:34 bug#25792: 26.0.50; Scroll-margin working incorrectly when scrolling down if show-trailing-whitespace is t Alexander Miller
@ 2017-02-19 16:59 ` npostavs
2017-02-19 17:20 ` Eli Zaretskii
0 siblings, 1 reply; 9+ messages in thread
From: npostavs @ 2017-02-19 16:59 UTC (permalink / raw)
To: Alexander Miller; +Cc: 25792
severity 25792 minor
tags 25792 confirmed
quit
Alexander Miller <alexanderm@web.de> writes:
> Only scrolling down is affected. The issue first appeared when I
> recompiled emacs roughly 1 week ago and has persisted over multiple
> recompilations on 2 different laptops (both running Antergos & Xfce), so
> my guess is that it's an issue in the current emacs master.
This seems to be caused by [1: b9be4c14e8], it only happens when the
window has partial lines (i.e., (zerop (cadr (cl-floor
(window-screen-lines)))) is non-nil). Not sure how
show-trailing-whitespace affects this, but it does.
1: 2017-02-02 21:21:18 -0500 b9be4c14e89f5cec08a7a0f0d24033e0e6ff5ef0
Fix scrolling with partial lines
^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#25792: 26.0.50; Scroll-margin working incorrectly when scrolling down if show-trailing-whitespace is t
2017-02-19 16:59 ` npostavs
@ 2017-02-19 17:20 ` Eli Zaretskii
2017-02-19 20:56 ` Eli Zaretskii
0 siblings, 1 reply; 9+ messages in thread
From: Eli Zaretskii @ 2017-02-19 17:20 UTC (permalink / raw)
To: npostavs; +Cc: alexanderm, 25792
> From: npostavs@users.sourceforge.net
> Date: Sun, 19 Feb 2017 11:59:22 -0500
> Cc: 25792@debbugs.gnu.org
>
> > Only scrolling down is affected. The issue first appeared when I
> > recompiled emacs roughly 1 week ago and has persisted over multiple
> > recompilations on 2 different laptops (both running Antergos & Xfce), so
> > my guess is that it's an issue in the current emacs master.
>
> This seems to be caused by [1: b9be4c14e8], it only happens when the
> window has partial lines (i.e., (zerop (cadr (cl-floor
> (window-screen-lines)))) is non-nil). Not sure how
> show-trailing-whitespace affects this, but it does.
My guess would be that show-trailing-whitespace causes an additional
redisplay cycle, due to its post-command-hook.
^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#25792: 26.0.50; Scroll-margin working incorrectly when scrolling down if show-trailing-whitespace is t
2017-02-19 17:20 ` Eli Zaretskii
@ 2017-02-19 20:56 ` Eli Zaretskii
2017-02-20 17:42 ` npostavs
0 siblings, 1 reply; 9+ messages in thread
From: Eli Zaretskii @ 2017-02-19 20:56 UTC (permalink / raw)
To: npostavs; +Cc: alexanderm, 25792
> Date: Sun, 19 Feb 2017 19:20:40 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: alexanderm@web.de, 25792@debbugs.gnu.org
>
> > From: npostavs@users.sourceforge.net
> > Date: Sun, 19 Feb 2017 11:59:22 -0500
> > Cc: 25792@debbugs.gnu.org
> >
> > > Only scrolling down is affected. The issue first appeared when I
> > > recompiled emacs roughly 1 week ago and has persisted over multiple
> > > recompilations on 2 different laptops (both running Antergos & Xfce), so
> > > my guess is that it's an issue in the current emacs master.
> >
> > This seems to be caused by [1: b9be4c14e8], it only happens when the
> > window has partial lines (i.e., (zerop (cadr (cl-floor
> > (window-screen-lines)))) is non-nil). Not sure how
> > show-trailing-whitespace affects this, but it does.
>
> My guess would be that show-trailing-whitespace causes an additional
> redisplay cycle, due to its post-command-hook.
If you invoke trace-redisplay and then run the recipe, you will see
that there are two kinds of redisplay cycles: one that calls
try_scrolling, the other that doesn't. So there are 2 different paths
through redisplay, and they don't behave the same under these
conditions.
HTH
^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#25792: 26.0.50; Scroll-margin working incorrectly when scrolling down if show-trailing-whitespace is t
2017-02-19 20:56 ` Eli Zaretskii
@ 2017-02-20 17:42 ` npostavs
2017-02-20 18:02 ` Eli Zaretskii
0 siblings, 1 reply; 9+ messages in thread
From: npostavs @ 2017-02-20 17:42 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: alexanderm, 25792
Eli Zaretskii <eliz@gnu.org> writes:
>> Date: Sun, 19 Feb 2017 19:20:40 +0200
>> From: Eli Zaretskii <eliz@gnu.org>
>> Cc: alexanderm@web.de, 25792@debbugs.gnu.org
>>
>> > From: npostavs@users.sourceforge.net
>> > Date: Sun, 19 Feb 2017 11:59:22 -0500
>> > Cc: 25792@debbugs.gnu.org
>> >
>> > > Only scrolling down is affected. The issue first appeared when I
>> > > recompiled emacs roughly 1 week ago and has persisted over multiple
>> > > recompilations on 2 different laptops (both running Antergos & Xfce), so
>> > > my guess is that it's an issue in the current emacs master.
>> >
>> > This seems to be caused by [1: b9be4c14e8], it only happens when the
>> > window has partial lines (i.e., (zerop (cadr (cl-floor
>> > (window-screen-lines)))) is non-nil). Not sure how
>> > show-trailing-whitespace affects this, but it does.
>>
>> My guess would be that show-trailing-whitespace causes an additional
>> redisplay cycle, due to its post-command-hook.
>
> If you invoke trace-redisplay and then run the recipe, you will see
> that there are two kinds of redisplay cycles: one that calls
> try_scrolling, the other that doesn't. So there are 2 different paths
> through redisplay, and they don't behave the same under these
> conditions.
The difference seems to be that try_window returns -1 in the case where
we correctly stay out of the margin, but 1 in the case where we cross
into it. I tried adding a partial_line_height call, but it's not giving
the right answer. The move_it_to (&it, ZV, -1, it.last_visible_y, -1,
MOVE_TO_POS | MOVE_TO_Y) call doesn't stop at last_visible_y, it goes to
the end of buffer, so the returned partial height is a very large number
(approx 6500 in my test).
modified src/xdisp.c
@@ -17389,9 +17389,10 @@ try_window (Lisp_Object window, struct text_pos pos, int flags)
/* rms: considering make_cursor_line_fully_visible_p here
seems to give wrong results. We don't want to recenter
when the last line is partly visible, we want to allow
that case to be handled in the usual way. */
- || w->cursor.y > it.last_visible_y - this_scroll_margin - 1)
+ || w->cursor.y > (it.last_visible_y - partial_line_height (&it)
+ - this_scroll_margin - 1))
{
w->cursor.vpos = -1;
clear_glyph_matrix (w->desired_matrix);
return -1;
^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#25792: 26.0.50; Scroll-margin working incorrectly when scrolling down if show-trailing-whitespace is t
2017-02-20 17:42 ` npostavs
@ 2017-02-20 18:02 ` Eli Zaretskii
2017-02-20 18:46 ` npostavs
0 siblings, 1 reply; 9+ messages in thread
From: Eli Zaretskii @ 2017-02-20 18:02 UTC (permalink / raw)
To: npostavs; +Cc: alexanderm, 25792
> From: npostavs@users.sourceforge.net
> Cc: 25792@debbugs.gnu.org, alexanderm@web.de
> Date: Mon, 20 Feb 2017 12:42:58 -0500
>
> > If you invoke trace-redisplay and then run the recipe, you will see
> > that there are two kinds of redisplay cycles: one that calls
> > try_scrolling, the other that doesn't. So there are 2 different paths
> > through redisplay, and they don't behave the same under these
> > conditions.
>
> The difference seems to be that try_window returns -1 in the case where
> we correctly stay out of the margin, but 1 in the case where we cross
> into it. I tried adding a partial_line_height call, but it's not giving
> the right answer. The move_it_to (&it, ZV, -1, it.last_visible_y, -1,
> MOVE_TO_POS | MOVE_TO_Y) call doesn't stop at last_visible_y, it goes to
> the end of buffer, so the returned partial height is a very large number
> (approx 6500 in my test).
Probably because it.current_y is already beyond it.last_visible_y, due
to the loop before the call:
/* Display all lines of W. */
while (it.current_y < it.last_visible_y)
{
if (display_line (&it))
last_text_row = it.glyph_row - 1;
if (f->fonts_changed && !(flags & TRY_WINDOW_IGNORE_FONTS_CHANGE))
return 0;
}
Does it work to call start_display again, before the call to
partial_line_height?
^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#25792: 26.0.50; Scroll-margin working incorrectly when scrolling down if show-trailing-whitespace is t
2017-02-20 18:02 ` Eli Zaretskii
@ 2017-02-20 18:46 ` npostavs
2017-02-20 19:36 ` Eli Zaretskii
0 siblings, 1 reply; 9+ messages in thread
From: npostavs @ 2017-02-20 18:46 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: alexanderm, 25792
[-- Attachment #1: Type: text/plain, Size: 902 bytes --]
tags 25792 patch
quit
Eli Zaretskii <eliz@gnu.org> writes:
>> From: npostavs@users.sourceforge.net
>> Cc: 25792@debbugs.gnu.org, alexanderm@web.de
>> Date: Mon, 20 Feb 2017 12:42:58 -0500
>>
>> The difference seems to be that try_window returns -1 in the case where
>> we correctly stay out of the margin, but 1 in the case where we cross
>> into it. I tried adding a partial_line_height call, but it's not giving
>> the right answer. The move_it_to (&it, ZV, -1, it.last_visible_y, -1,
>> MOVE_TO_POS | MOVE_TO_Y) call doesn't stop at last_visible_y, it goes to
>> the end of buffer, so the returned partial height is a very large number
>> (approx 6500 in my test).
>
> Probably because it.current_y is already beyond it.last_visible_y, due
> to the loop before the call:
Ah, right.
> Does it work to call start_display again, before the call to
> partial_line_height?
Yes, here's a patch:
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch --]
[-- Type: text/x-diff, Size: 2124 bytes --]
From f3f948057ae78a551a2f83d89f7918623bed3e6e Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Mon, 20 Feb 2017 13:34:39 -0500
Subject: [PATCH v1] Fix scrolling with partial line corner case (Bug#25792)
* src/xdisp.c (try_window): Take partial line height into account when
comparing cursor position against scroll margin.
---
src/xdisp.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/src/xdisp.c b/src/xdisp.c
index e59934d2d5..6c94147844 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -17376,21 +17376,27 @@ try_window (Lisp_Object window, struct text_pos pos, int flags)
return 0;
}
+ /* Save the character position of 'it' before we call
+ 'start_display' again. */
+ ptrdiff_t it_charpos = IT_CHARPOS (it);
+
/* Don't let the cursor end in the scroll margins. */
if ((flags & TRY_WINDOW_CHECK_MARGINS)
&& !MINI_WINDOW_P (w))
{
int this_scroll_margin = window_scroll_margin (w, MARGIN_IN_PIXELS);
+ start_display (&it, w, pos);
if ((w->cursor.y >= 0 /* not vscrolled */
&& w->cursor.y < this_scroll_margin
&& CHARPOS (pos) > BEGV
- && IT_CHARPOS (it) < ZV)
+ && it_charpos < ZV)
/* rms: considering make_cursor_line_fully_visible_p here
seems to give wrong results. We don't want to recenter
when the last line is partly visible, we want to allow
that case to be handled in the usual way. */
- || w->cursor.y > it.last_visible_y - this_scroll_margin - 1)
+ || w->cursor.y > (it.last_visible_y - partial_line_height (&it)
+ - this_scroll_margin - 1))
{
w->cursor.vpos = -1;
clear_glyph_matrix (w->desired_matrix);
@@ -17399,7 +17405,7 @@ try_window (Lisp_Object window, struct text_pos pos, int flags)
}
/* If bottom moved off end of frame, change mode line percentage. */
- if (w->window_end_pos <= 0 && Z != IT_CHARPOS (it))
+ if (w->window_end_pos <= 0 && Z != it_charpos)
w->update_mode_line = true;
/* Set window_end_pos to the offset of the last character displayed
--
2.11.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* bug#25792: 26.0.50; Scroll-margin working incorrectly when scrolling down if show-trailing-whitespace is t
2017-02-20 18:46 ` npostavs
@ 2017-02-20 19:36 ` Eli Zaretskii
2017-02-25 4:37 ` npostavs
0 siblings, 1 reply; 9+ messages in thread
From: Eli Zaretskii @ 2017-02-20 19:36 UTC (permalink / raw)
To: npostavs; +Cc: alexanderm, 25792
> From: npostavs@users.sourceforge.net
> Cc: 25792@debbugs.gnu.org, alexanderm@web.de
> Date: Mon, 20 Feb 2017 13:46:02 -0500
>
> > Does it work to call start_display again, before the call to
> > partial_line_height?
>
> Yes, here's a patch:
LGTM, thanks.
^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#25792: 26.0.50; Scroll-margin working incorrectly when scrolling down if show-trailing-whitespace is t
2017-02-20 19:36 ` Eli Zaretskii
@ 2017-02-25 4:37 ` npostavs
0 siblings, 0 replies; 9+ messages in thread
From: npostavs @ 2017-02-25 4:37 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: alexanderm, 25792
tags 25792 fixed
close 25792
quit
Eli Zaretskii <eliz@gnu.org> writes:
>> > Does it work to call start_display again, before the call to
>> > partial_line_height?
>>
>> Yes, here's a patch:
>
> LGTM, thanks.
Pushed to master [1: f0e7f39e0b].
1: 2017-02-24 23:15:40 -0500 f0e7f39e0b4026a3d613416ad8ffc84e6b74242b
Fix scrolling with partial line corner case (Bug#25792)
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2017-02-25 4:37 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-02-19 15:34 bug#25792: 26.0.50; Scroll-margin working incorrectly when scrolling down if show-trailing-whitespace is t Alexander Miller
2017-02-19 16:59 ` npostavs
2017-02-19 17:20 ` Eli Zaretskii
2017-02-19 20:56 ` Eli Zaretskii
2017-02-20 17:42 ` npostavs
2017-02-20 18:02 ` Eli Zaretskii
2017-02-20 18:46 ` npostavs
2017-02-20 19:36 ` Eli Zaretskii
2017-02-25 4:37 ` npostavs
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.