* 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 public inbox https://git.savannah.gnu.org/cgit/emacs.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).