* Avoid recentering when user says so
@ 2011-03-26 18:48 Eli Zaretskii
2011-03-26 19:52 ` Eli Zaretskii
` (4 more replies)
0 siblings, 5 replies; 8+ messages in thread
From: Eli Zaretskii @ 2011-03-26 18:48 UTC (permalink / raw
To: emacs-devel
This is related to bug #6671, read there if you want to know the gory
details.
In a nutshell, Emacs currently doesn't honor meticulously enough user
settings such as scroll-conservatively and scroll-up/down-aggressively.
Sometimes, especially if redisplay cannot keep up with high-rate
keyboard input, it falls back on recentering, which these variables
are supposed to avoid. The last attempt to solve this introduced a
bug that is the subject of bug #6671.
Following the latest discussions in that bug report, I came up with a
way to solve this problem without incurring unduly slow movement far
away in the buffer. The patch below seems to give good results, but
as I don't use the above settings (I like the default recentering),
I'm not a good candidate for testing the patch.
Would people who use these variables please try the patch below, and
report any findings, both positive and negative? Please report your
findings to the bug tracker at 6671@debbugs.gnu.org, so that the
details get filed there.
Given positive feedback, I will commit this to the trunk.
Thanks.
=== modified file 'src/xdisp.c'
--- src/xdisp.c 2011-03-26 12:20:20 +0000
+++ src/xdisp.c 2011-03-26 18:07:58 +0000
@@ -13003,6 +13003,9 @@ enum
SCROLLING_NEED_LARGER_MATRICES
};
+/* If scroll-conservatively is more than this, never recenter. */
+#define SCROLL_LIMIT 100
+
static int
try_scrolling (Lisp_Object window, int just_this_one_p,
EMACS_INT arg_scroll_conservatively, EMACS_INT scroll_step,
@@ -13016,7 +13019,8 @@ try_scrolling (Lisp_Object window, int j
int dy = 0, amount_to_scroll = 0, scroll_down_p = 0;
int extra_scroll_margin_lines = last_line_misfit ? 1 : 0;
Lisp_Object aggressive;
- int scroll_limit = INT_MAX / FRAME_LINE_HEIGHT (f);
+ /* We will never try scrolling more than this number of lines. */
+ int scroll_limit = SCROLL_LIMIT;
#if GLYPH_DEBUG
debug_method_add (w, "try_scrolling");
@@ -13032,14 +13036,14 @@ try_scrolling (Lisp_Object window, int j
else
this_scroll_margin = 0;
- /* Force arg_scroll_conservatively to have a reasonable value, to avoid
- overflow while computing how much to scroll. Note that the user
- can supply scroll-conservatively equal to `most-positive-fixnum',
- which can be larger than INT_MAX. */
+ /* Force arg_scroll_conservatively to have a reasonable value, to
+ avoid scrolling too far away with slow move_it_* functions. Note
+ that the user can supply scroll-conservatively equal to
+ `most-positive-fixnum', which can be larger than INT_MAX. */
if (arg_scroll_conservatively > scroll_limit)
{
- arg_scroll_conservatively = scroll_limit;
- scroll_max = INT_MAX;
+ arg_scroll_conservatively = scroll_limit + 1;
+ scroll_max = scroll_limit * FRAME_LINE_HEIGHT (f);
}
else if (scroll_step || arg_scroll_conservatively || temp_scroll_step)
/* Compute how much we should try to scroll maximally to bring
@@ -13076,7 +13080,7 @@ try_scrolling (Lisp_Object window, int j
/* Compute how many pixels below window bottom to stop searching
for PT. This avoids costly search for PT that is far away if
the user limited scrolling by a small number of lines, but
- always finds PT if arg_scroll_conservatively is set to a large
+ always finds PT if scroll_conservatively is set to a large
number, such as most-positive-fixnum. */
int slack = max (scroll_max, 10 * FRAME_LINE_HEIGHT (f));
int y_to_move =
@@ -13128,12 +13132,12 @@ try_scrolling (Lisp_Object window, int j
return SCROLLING_FAILED;
start_display (&it, w, startp);
- if (scroll_max < INT_MAX)
+ if (arg_scroll_conservatively <= scroll_limit)
move_it_vertically (&it, amount_to_scroll);
else
{
/* Extra precision for users who set scroll-conservatively
- to most-positive-fixnum: make sure the amount we scroll
+ to a large number: make sure the amount we scroll
the window start is never less than amount_to_scroll,
which was computed as distance from window bottom to
point. This matters when lines at window top and lines
@@ -14201,11 +14205,10 @@ redisplay_window (Lisp_Object window, in
}
}
- /* Finally, just choose place to start which centers point */
+ /* Finally, just choose a place to start which positions point
+ according to user preferences. */
recenter:
- if (centering_position < 0)
- centering_position = window_box_height (w) / 2;
#if GLYPH_DEBUG
debug_method_add (w, "recenter");
@@ -14217,10 +14220,62 @@ redisplay_window (Lisp_Object window, in
if (!buffer_unchanged_p)
w->base_line_number = Qnil;
- /* Move backward half the height of the window. */
+ /* Determine the window start relative to point. */
init_iterator (&it, w, PT, PT_BYTE, NULL, DEFAULT_FACE_ID);
it.current_y = it.last_visible_y;
+ if (centering_position < 0)
+ {
+ int margin =
+ scroll_margin > 0
+ ? min (scroll_margin, WINDOW_TOTAL_LINES (w) / 4)
+ : 0;
+ int scrolling_up = PT > CHARPOS (startp) + margin;
+ Lisp_Object aggressive =
+ scrolling_up
+ ? BVAR (current_buffer, scroll_up_aggressively)
+ : BVAR (current_buffer, scroll_down_aggressively);
+
+ if (!MINI_WINDOW_P (w)
+ && scroll_conservatively > SCROLL_LIMIT || NUMBERP (aggressive))
+ {
+ int pt_offset = 0;
+
+ /* Setting scroll-conservatively overrides
+ scroll-*-aggressively. */
+ if (!scroll_conservatively && NUMBERP (aggressive))
+ {
+ double float_amount = XFLOATINT (aggressive);
+
+ pt_offset = float_amount * WINDOW_BOX_TEXT_HEIGHT (w);
+ if (pt_offset == 0 && float_amount > 0)
+ pt_offset = 1;
+ if (pt_offset)
+ margin -= 1;
+ }
+ /* Compute how much to move the window start backward from
+ point so that point will be displayed where the user
+ wants it. */
+ if (scrolling_up)
+ {
+ centering_position = it.last_visible_y;
+ if (pt_offset)
+ centering_position -= pt_offset;
+ centering_position -=
+ FRAME_LINE_HEIGHT (f) * (1 + margin + (last_line_misfit != 0));
+ /* Don't let point enter the scroll margin near top of
+ the window. */
+ if (centering_position < margin * FRAME_LINE_HEIGHT (f))
+ centering_position = margin * FRAME_LINE_HEIGHT (f);
+ }
+ else
+ centering_position = margin * FRAME_LINE_HEIGHT (f) + pt_offset;
+ }
+ else
+ /* Move backward from point half the height of the window. */
+ centering_position = window_box_height (w) / 2;
+ }
move_it_vertically_backward (&it, centering_position);
+
xassert (IT_CHARPOS (it) >= BEGV);
/* The function move_it_vertically_backward may move over more
@@ -14237,8 +14292,9 @@ redisplay_window (Lisp_Object window, in
it.current_x = it.hpos = 0;
- /* Set startp here explicitly in case that helps avoid an infinite loop
- in case the window-scroll-functions functions get errors. */
+ /* Set the window start position here explicitly, to avoid an
+ infinite loop in case the functions in window-scroll-functions
+ get errors. */
set_marker_both (w->start, Qnil, IT_CHARPOS (it), IT_BYTEPOS (it));
/* Run scroll hooks. */
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Avoid recentering when user says so
2011-03-26 18:48 Avoid recentering when user says so Eli Zaretskii
@ 2011-03-26 19:52 ` Eli Zaretskii
2011-03-27 2:58 ` Juanma Barranquero
2011-03-27 3:57 ` Eli Zaretskii
` (3 subsequent siblings)
4 siblings, 1 reply; 8+ messages in thread
From: Eli Zaretskii @ 2011-03-26 19:52 UTC (permalink / raw
To: emacs-devel
> Date: Sat, 26 Mar 2011 20:48:35 +0200
> From: Eli Zaretskii <eliz@gnu.org>
>
> Following the latest discussions in that bug report, I came up with a
> way to solve this problem without incurring unduly slow movement far
> away in the buffer. The patch below seems to give good results
Btw, in the discussions related to bug #6671, some people said that
Emacs should pay attention to the value of another variable,
scroll-preserve-screen-position, when Emacs decides where to put point
after moving it to a far away location.
I would like to point out that this option has nothing to do with the
issue at hand, although its name might suggest that it does. This
option is only examined by the scroll-up and scroll-down commands,
which are just one way of scrolling. No other command ever looks at
this option. (Documentation seem to suggest that any command that has
non-nil scroll-command property will also behave according to this
option, but unless I'm missing something, I cannot see how that could
be true.)
Therefore, the patch I proposed does not consult this option at all.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Avoid recentering when user says so
2011-03-26 19:52 ` Eli Zaretskii
@ 2011-03-27 2:58 ` Juanma Barranquero
0 siblings, 0 replies; 8+ messages in thread
From: Juanma Barranquero @ 2011-03-27 2:58 UTC (permalink / raw
To: Eli Zaretskii; +Cc: emacs-devel
On Sat, Mar 26, 2011 at 20:52, Eli Zaretskii <eliz@gnu.org> wrote:
> Btw, in the discussions related to bug #6671, some people said that
> Emacs should pay attention to the value of another variable,
> scroll-preserve-screen-position, when Emacs decides where to put point
> after moving it to a far away location.
He, not exactly ;-)
Chong asked if the unwanted recentering affected other scrolling
commands, and I said that scroll-(up|down)-command do preserve the
screen position and don't do any recentering.
I think the documentation makes reasonably clear that
`scroll-preserve-screen-position' is not defined for non-scrolling
movements (like goto-line), and as per line-by-line scrolling
(assuming you've got track-eol == t) you can already combine
pgup/pgdown and up/down and get the cursor back to the same screen
position, so it's well defined and well-behaved. It's true that
(next|previous)-line do not really obey
scroll-preserve-screen-position, but as they try to move vertically
and are smart with respect to end of line (with track-eol), the net
effect is the same.
Juanma
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Avoid recentering when user says so
2011-03-26 18:48 Avoid recentering when user says so Eli Zaretskii
2011-03-26 19:52 ` Eli Zaretskii
@ 2011-03-27 3:57 ` Eli Zaretskii
2011-03-27 7:58 ` bug#6671: " David Engster
` (2 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Eli Zaretskii @ 2011-03-27 3:57 UTC (permalink / raw
To: emacs-devel
> Date: Sat, 26 Mar 2011 20:48:35 +0200
> From: Eli Zaretskii <eliz@gnu.org>
>
> + if (!MINI_WINDOW_P (w)
> + && scroll_conservatively > SCROLL_LIMIT || NUMBERP (aggressive))
Please fix a stupid mistake here, and use this instead:
+ if (!MINI_WINDOW_P (w)
+ && (scroll_conservatively > SCROLL_LIMIT || NUMBERP (aggressive)))
^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#6671: Avoid recentering when user says so
2011-03-26 18:48 Avoid recentering when user says so Eli Zaretskii
2011-03-26 19:52 ` Eli Zaretskii
2011-03-27 3:57 ` Eli Zaretskii
@ 2011-03-27 7:58 ` David Engster
2011-03-27 7:58 ` David Engster
2011-03-31 19:20 ` Eli Zaretskii
4 siblings, 0 replies; 8+ messages in thread
From: David Engster @ 2011-03-27 7:58 UTC (permalink / raw
To: Eli Zaretskii; +Cc: 6671, emacs-devel
Eli Zaretskii writes:
> Would people who use these variables please try the patch below, and
> report any findings, both positive and negative? Please report your
> findings to the bug tracker at 6671@debbugs.gnu.org, so that the
> details get filed there.
>
> Given positive feedback, I will commit this to the trunk.
Oh, please do! Using this patch, scrolling is definitely *much* improved
with my setup. No more annoying recenterings - Thank you!
-David
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Avoid recentering when user says so
2011-03-26 18:48 Avoid recentering when user says so Eli Zaretskii
` (2 preceding siblings ...)
2011-03-27 7:58 ` bug#6671: " David Engster
@ 2011-03-27 7:58 ` David Engster
2011-03-27 10:46 ` Eli Zaretskii
2011-03-31 19:20 ` Eli Zaretskii
4 siblings, 1 reply; 8+ messages in thread
From: David Engster @ 2011-03-27 7:58 UTC (permalink / raw
To: Eli Zaretskii; +Cc: 6671, emacs-devel
Eli Zaretskii writes:
> Would people who use these variables please try the patch below, and
> report any findings, both positive and negative? Please report your
> findings to the bug tracker at 6671@debbugs.gnu.org, so that the
> details get filed there.
>
> Given positive feedback, I will commit this to the trunk.
Oh, please do! Using this patch, scrolling is definitely *much* improved
with my setup. No more annoying recenterings - Thank you!
-David
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Avoid recentering when user says so
2011-03-27 7:58 ` David Engster
@ 2011-03-27 10:46 ` Eli Zaretskii
0 siblings, 0 replies; 8+ messages in thread
From: Eli Zaretskii @ 2011-03-27 10:46 UTC (permalink / raw
To: David Engster; +Cc: emacs-devel
> From: David Engster <deng@randomsample.de>
> Date: Sun, 27 Mar 2011 09:58:51 +0200
> Cc: 6671@debbugs.gnu.org, emacs-devel@gnu.org
>
> Eli Zaretskii writes:
> > Would people who use these variables please try the patch below, and
> > report any findings, both positive and negative? Please report your
> > findings to the bug tracker at 6671@debbugs.gnu.org, so that the
> > details get filed there.
> >
> > Given positive feedback, I will commit this to the trunk.
>
> Oh, please do! Using this patch, scrolling is definitely *much* improved
> with my setup. No more annoying recenterings - Thank you!
Thanks for the feedback.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Avoid recentering when user says so
2011-03-26 18:48 Avoid recentering when user says so Eli Zaretskii
` (3 preceding siblings ...)
2011-03-27 7:58 ` David Engster
@ 2011-03-31 19:20 ` Eli Zaretskii
4 siblings, 0 replies; 8+ messages in thread
From: Eli Zaretskii @ 2011-03-31 19:20 UTC (permalink / raw
To: emacs-devel
> Date: Sat, 26 Mar 2011 20:48:35 +0200
> From: Eli Zaretskii <eliz@gnu.org>
>
> This is related to bug #6671, read there if you want to know the gory
> details.
>
> In a nutshell, Emacs currently doesn't honor meticulously enough user
> settings such as scroll-conservatively and scroll-up/down-aggressively.
> Sometimes, especially if redisplay cannot keep up with high-rate
> keyboard input, it falls back on recentering, which these variables
> are supposed to avoid. The last attempt to solve this introduced a
> bug that is the subject of bug #6671.
>
> Following the latest discussions in that bug report, I came up with a
> way to solve this problem without incurring unduly slow movement far
> away in the buffer. The patch below seems to give good results, but
> as I don't use the above settings (I like the default recentering),
> I'm not a good candidate for testing the patch.
>
> Would people who use these variables please try the patch below, and
> report any findings, both positive and negative? Please report your
> findings to the bug tracker at 6671@debbugs.gnu.org, so that the
> details get filed there.
>
> Given positive feedback, I will commit this to the trunk.
Done. Thanks to all who tried the patch and provided feedback.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2011-03-31 19:20 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-03-26 18:48 Avoid recentering when user says so Eli Zaretskii
2011-03-26 19:52 ` Eli Zaretskii
2011-03-27 2:58 ` Juanma Barranquero
2011-03-27 3:57 ` Eli Zaretskii
2011-03-27 7:58 ` bug#6671: " David Engster
2011-03-27 7:58 ` David Engster
2011-03-27 10:46 ` Eli Zaretskii
2011-03-31 19:20 ` Eli Zaretskii
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.