bug#6671: moving point and scroll-conservatively

bug#6671: moving point and scroll-conservatively

[Juanma Barranquero]

Package: emacs
Severity: important
Version: 24.0.50


---------- Forwarded message ----------
From: Eli Zaretskii <eliz@gnu.org>
Date: Sat, Jun 26, 2010 at 13:34
Subject: moving point and scroll-conservatively
To: emacs-devel@gnu.org


Did any of you who set scroll-conservatively to most-positive-fixnum
notice that moving around an unmodified buffer became much slower,
since revno 100620, when point moves far away?

For example, with this recipe:

 emacs -Q
 M-: (setq scroll-conservatively most-positive-fixnum scroll-step 0) RET
 C-x C-f xdisp.c
 C-u 25000 M-g M-g

it takes Emacs 17 seconds on my 3GHz machine to display xdisp.c around
line 25000, whereas it's instantaneous in Emacs 23.2.

This happens because the try_scrolling method would previously give up
when point was more than 10 screen lines below the current end of the
window.  Emacs would then perform a complete redisplay of the window,
with point located on the middle line.  This "recentering" annoyed
users who set scroll-conservatively to most-positive-fixnum, so revno
100620 modified try_scrolling to _never_ give up due to point being
too far away, when scroll-conservatively is set to such a large value.
However, try_scrolling is used not only for scroll commands such as
the C-n or C-v, but for _any_ motion in a buffer that didn't change.
Thus the unintended effect described above.

I guess we should limit try_scrolling to situations where at least one
screen line from the previous display is left on the screen.
Otherwise, we are going to redisplay the entire window anyway, and
this optimization does not make sense.  Note that the documentation of
scroll-conservatively explicitly says "set it to some small number N";
i.e. it was never the intent that it will be set to such large values.
For the same reason the value of the scroll-*-aggressively variables
is limited to 1.

The resulting recentering when Emacs cannot keep up with the keyboard
auto-repeat rate caused by user leaning on the down arrow should be
handled in some different way (see my suggestion in the "The
unwarranted scrolling assumption" thread).

Btw, what do users of scroll-conservatively = most-positive-fixnum
want from C-v and PageDown keys?  Is it okay to recenter in that case,
or do you want to see the cursor on the last screen line in that case
as well?  (Emacs does not currently distinguish between these two
cases.  In fact, it does not care at all which command caused point to
move.)

bug#6671: moving point and scroll-conservatively

[Juanma Barranquero]

Relevant theads:

http://lists.gnu.org/archive/html/emacs-devel/2010-06/msg01011.html
http://lists.gnu.org/archive/html/emacs-devel/2010-06/msg00554.html

bug#6671: moving point and scroll-conservatively

[Chong Yidong]

> Did any of you who set scroll-conservatively to most-positive-fixnum
> notice that moving around an unmodified buffer became much slower,
> since revno 100620, when point moves far away?
>
> This happens because the try_scrolling method would previously give up
> when point was more than 10 screen lines below the current end of the
> window.  Emacs would then perform a complete redisplay of the window,
> with point located on the middle line.  This "recentering" annoyed
> users who set scroll-conservatively to most-positive-fixnum, so revno
> 100620 modified try_scrolling to _never_ give up due to point being
> too far away, when scroll-conservatively is set to such a large value.
> However, try_scrolling is used not only for scroll commands such as
> the C-n or C-v, but for _any_ motion in a buffer that didn't change.
> Thus the unintended effect described above.

I don't think it's reasonable to make the iterator slog through
thousands of lines in an attempt to "scroll" general point motions.

Is the "unintended recentering" problem purely associated with the
commands next-line and previous-line?  If so, maybe we should
investigate ways to prevent point motion from "running ahead" of
redisplay in those specific instances.

bug#6671: moving point and scroll-conservatively

[Juanma Barranquero]

On Thu, Mar 24, 2011 at 01:42, Chong Yidong <cyd@stupidchicken.com> wrote:

> Is the "unintended recentering" problem purely associated with the
> commands next-line and previous-line?

Other scrolling commands, like scroll-up-command, do respect
scroll-preserve-scroll-position and do not cause unexpected
recentering, though their performance isn't currently very good (try
keeping PgDn pressed in xdisp.c...).

    Juanma

bug#6671: moving point and scroll-conservatively

[Chong Yidong]

Juanma Barranquero <lekktu@gmail.com> writes:

>> Is the "unintended recentering" problem purely associated with the
>> commands next-line and previous-line?
>
> Other scrolling commands, like scroll-up-command, do respect
> scroll-preserve-scroll-position and do not cause unexpected
> recentering, though their performance isn't currently very good (try
> keeping PgDn pressed in xdisp.c...).

OK.

After playing around with the "C-n in xdisp.c" testcase with the changes
in revision 100619/100620 reverted, I think I know the problem.

In this test case, recentering seems to be triggered by redisplay-time
fontification.  To demonstrat this, set fontification-functions to nil
in xdisp.c, before initiating the C-n testcase.  For me, at least, the
anomalous recentering then does not occur.

The precise mechanism by which the fontification functions screw things
up is by narrowing the buffer.  This is why, in last year's thread, the
buffer's clip_changed flag was found to be set.  That forces recentering
in redisplay_window (xdisp.c:14159).

So, a good fix might be to check whether redisplay-time fontification
changes the buffer's restrictions, and, if so, reset the clip_changed
flag.  That should make the 100619/100620 changes unnecessary.

bug#6671: moving point and scroll-conservatively

[Eli Zaretskii]

> From: Chong Yidong <cyd@stupidchicken.com>
> Date: Wed, 23 Mar 2011 23:58:27 -0400
> Cc: 6671@debbugs.gnu.org
> 
> After playing around with the "C-n in xdisp.c" testcase with the changes
> in revision 100619/100620 reverted, I think I know the problem.
> 
> In this test case, recentering seems to be triggered by redisplay-time
> fontification.  To demonstrat this, set fontification-functions to nil
> in xdisp.c, before initiating the C-n testcase.  For me, at least, the
> anomalous recentering then does not occur.

Sorry, but phenomenological explanations shouldn't be accepted in this
case.  We need specific evidence, and the only way to show that is by
adding the necessary tracing code and showing the results.

For example, I suggest to try introducing artificial delays (with
`nanosleep' or some such) into redisplay, and trying then, with
fontification-functions disabled.  You may find out (as I did at the
time) that fontification is just a trigger, not the cause itself; the
cause is more computations done during redisplay, which cause the
display engine bail out prematurely (unless you turn on
redisplay-dont-pause), which is yet another cause of recentering.

> The precise mechanism by which the fontification functions screw things
> up is by narrowing the buffer.  This is why, in last year's thread, the
> buffer's clip_changed flag was found to be set.  That forces recentering
> in redisplay_window (xdisp.c:14159).

Well, it shouldn't.  Instead of "fixing" the code by preventing this
flag to be set based on some heuristics, we should make the code DTRT
even if the flag is set (assuming that this indeed is the root cause
of the recentering in its all incarnations, which I'm not sure about).

Please note that I'm not convinced that the issue of fontification and
the clip_changed flag is relevant to the issue at hand.  Recentering
has more than one reason, so you could be seeing one of the other
ones.  We shouldn't mix them, if we want to remain sane and leave our
display code maintainable.

> So, a good fix might be to check whether redisplay-time fontification
> changes the buffer's restrictions, and, if so, reset the clip_changed
> flag.  That should make the 100619/100620 changes unnecessary.

Please don't revert these changes.  They took a lot of effort to
arrive at, and generally DTRT in a way that is easy to understand and
maintain.

The problem with performance for large moves of point is IMO
straightforward to fix: when point is "far away" (which could be set
back to those proverbial 10 screen lines), then, instead of moving one
line at a time, move to point in one go, and then compute the window
start so that point is at the proper place relative to window start.
That proper place depends on the variables that try_scrolling
considers already, like scroll-conservatively etc.  The problem is, it
considers them too early, before we realize that point is too far
away.  When we do realize that, we are already past the code that
computes the scroll according to those variables, and all we do is
punt and recenter.

So the fix would be to re-arrange the code in try_scrolling so that
the decision to move far away and the move itself are done _before_ we
consider the adjustments of window start according to the
scroll-related variables.

IOW, we are suffering from historical legacy in redisplay here.
Originally, Emacs always recentered.  Then the various scrolling
options were introduced, but the code is still structured so that it
eventually falls back on recentering.  We should change that so that
whatever happens, after catching up with point we compute the window
start according to user expectations expressed through the scroll-*
options.  That would eliminate the recentering fallback completely,
and finally put this issue to rest.

bug#6671: moving point and scroll-conservatively

[Eli Zaretskii]

> From: Chong Yidong <cyd@stupidchicken.com>
> Date: Wed, 23 Mar 2011 20:42:49 -0400
> Cc: 
> 
> I don't think it's reasonable to make the iterator slog through
> thousands of lines in an attempt to "scroll" general point motions.

Well, if it were reasonable, we wouldn't have this bug report, would
we?

bug#6671: moving point and scroll-conservatively

[Chong Yidong]

Eli Zaretskii <eliz@gnu.org> writes:

>> That should make the 100619/100620 changes unnecessary.
>
> Please don't revert these changes.  They took a lot of effort to
> arrive at, and generally DTRT in a way that is easy to understand and
> maintain.
>
> The problem with performance for large moves of point is IMO
> straightforward to fix: when point is "far away" (which could be set
> back to those proverbial 10 screen lines), then, instead of moving one
> line at a time, move to point in one go, and then compute the window
> start so that point is at the proper place relative to window start.

These change you propose would also necessitate reverting 100619/100620.
Anytime the iterator is forced to traverse thousands (or in this case
tens of thousands) of lines, that's clearly the wrong thing.

bug#6671: moving point and scroll-conservatively

[Chong Yidong]

Chong Yidong <cyd@stupidchicken.com> writes:

> These change you propose would also necessitate reverting 100619/100620.
> Anytime the iterator is forced to traverse thousands (or in this case
> tens of thousands) of lines, that's clearly the wrong thing.

Well, "the wrong thing" in the sense of "obviously not what the user
intended", anyway.  I agree that the changes you made cause the
redisplay engine to literally follow the documented meaning of
scroll-conservatively.

Here's a more concrete proposal.  We change scroll-conservatively to
accept a new value, t, which means "scroll as far as you need".  Then
try_scrolling can use the "try scrolling for 10 lines" heuristic before
failing.  In this specific case, a failure changes centering_position,
adjusting the window start as though we had really scrolled the full
amount.

However, it's become idiomatic to set scroll-conservatively to a large
number.  Users of that idiom, who haven't changed scroll-conservatively
to t, would remain affected by the performance hit.  To cope with this,
let's change the meaning of numeric values of scroll-conservatively: if
it's larger than (say) 300, that is equivalent to t (infinity).

What do you think?

bug#6671: moving point and scroll-conservatively

[Eli Zaretskii]

> From: Chong Yidong <cyd@stupidchicken.com>
> Cc: lekktu@gmail.com, 6671@debbugs.gnu.org
> Date: Thu, 24 Mar 2011 16:01:46 -0400
> 
> Well, "the wrong thing" in the sense of "obviously not what the user
> intended", anyway.  I agree that the changes you made cause the
> redisplay engine to literally follow the documented meaning of
> scroll-conservatively.

I didn't mean for the code to do what it does now, and certainly
didn't interpret scroll-conservatively to mean that.  It was an
accident: I simply didn't realize that arbitrary movement of point
will invoke try_scrolling.  I thought we only get there for scrolling
commands, like next-line and scroll-up/down.  I intend to fix this.

> Here's a more concrete proposal.  We change scroll-conservatively to
> accept a new value, t, which means "scroll as far as you need".  Then
> try_scrolling can use the "try scrolling for 10 lines" heuristic before
> failing.  In this specific case, a failure changes centering_position,
> adjusting the window start as though we had really scrolled the full
> amount.
> 
> However, it's become idiomatic to set scroll-conservatively to a large
> number.  Users of that idiom, who haven't changed scroll-conservatively
> to t, would remain affected by the performance hit.  To cope with this,
> let's change the meaning of numeric values of scroll-conservatively: if
> it's larger than (say) 300, that is equivalent to t (infinity).
> 
> What do you think?

I would like to try fixing that code along the lines I suggested in a
previous mail.  That looks like the best alternative, since it doesn't
have any user-level changes (except for the bug that it should fix).
Maybe after I try that, I will come to a conclusion that what I
suggest is unworkable, but until then, I don't think we should
consider such radical changes.

Please let me work on this for a few days.  This bug is present on the
trunk for a very long time, and it was never an issue until now (in
fact, its first report came from me, even though I don't use
scroll-conservatively).  Given that, I don't see any urgency today, it
can well wait for a few more days.

bug#6671: moving point and scroll-conservatively

[Juanma Barranquero]

On Thu, Mar 24, 2011 at 21:01, Chong Yidong <cyd@stupidchicken.com> wrote:

> Here's a more concrete proposal.  We change scroll-conservatively to
> accept a new value, t, which means "scroll as far as you need".  Then
> try_scrolling can use the "try scrolling for 10 lines" heuristic before
> failing.  In this specific case, a failure changes centering_position,
> adjusting the window start as though we had really scrolled the full
> amount.

What does that mean, in terms of behavior?

> However, it's become idiomatic to set scroll-conservatively to a large
> number.

Because the doc suggested it.

> To cope with this,
> let's change the meaning of numeric values of scroll-conservatively: if
> it's larger than (say) 300, that is equivalent to t (infinity).

That's a bit hackish. IMHO, if you do change `scroll-conservatively'
as suggested, just document the new values and let people change it.

    Juanma

bug#6671: moving point and scroll-conservatively

[Juanma Barranquero]

On Thu, Mar 24, 2011 at 21:25, Eli Zaretskii <eliz@gnu.org> wrote:

> Maybe after I try that, I will come to a conclusion that what I
> suggest is unworkable, but until then, I don't think we should
> consider such radical changes.

Agreed.

    Juanma

bug#6671: moving point and scroll-conservatively

[Chong Yidong]

Eli Zaretskii <eliz@gnu.org> writes:

> I would like to try fixing that code along the lines I suggested in a
> previous mail.  That looks like the best alternative, since it doesn't
> have any user-level changes (except for the bug that it should fix).

The solution you suggested was:

> when point is "far away" (which could be set back to those proverbial
> 10 screen lines), then, instead of moving one line at a time, move to
> point in one go, and then compute the window start so that point is at
> the proper place relative to window start.

If the current literal meaning of scroll-conservatively is retained, we
need to count the number of lines involved in performing this "far-away
scroll", by iterating.  That brings us back to square one.

Think about it this way: suppose scroll-conservatively is 200000.  Taken
literally, this means that if we move point 200000 (screen) lines down,
redisplay should leave the cursor at the bottom of the window; if we
move point 200001 (screen) lines down, redisplay should recenter.  There
is only one way to determine the number of screen lines between the
bottom of our current window and the new value of point: a big expensive
redisplay iteration.

If you want to avoid this pitfall, your solution and mine become the
same, i.e. special handling of "sufficiently large" values of
scroll-conservatively.

bug#6671: moving point and scroll-conservatively

[Chong Yidong]

Juanma Barranquero <lekktu@gmail.com> writes:

>> Here's a more concrete proposal.  We change scroll-conservatively to
>> accept a new value, t, which means "scroll as far as you need".  Then
>> try_scrolling can use the "try scrolling for 10 lines" heuristic before
>> failing.  In this specific case, a failure changes centering_position,
>> adjusting the window start as though we had really scrolled the full
>> amount.
>
> What does that mean, in terms of behavior?

This means that any time point moves below the bottom of the window, the
resulting scroll will leave the cursor at the bottom of the window.  Any
time point moves above the top of the window, the resulting scroll will
leave the cursor at the top of the window.  Unlike numerical values of
scroll-conservatively, there is no "scrolling limit" to check.

>> To cope with this, let's change the meaning of numeric values of
>> scroll-conservatively: if it's larger than (say) 300, that is
>> equivalent to t (infinity).
>
> That's a bit hackish. IMHO, if you do change `scroll-conservatively'
> as suggested, just document the new values and let people change it.

Hackish yes, but if there is no real reason to specify such big
numerical values, I don't see any way this could have bad effects, in
practice.  Otherwise, users who miss that NEWS entry might get
frustrated by the sluggish behavior.

Another possibility is to introduce a "scroll-conservatively-scan-limit"
variable, instead of hardcoding 300 lines.

bug#6671: moving point and scroll-conservatively

[Juanma Barranquero]

On Thu, Mar 24, 2011 at 22:47, Chong Yidong <cyd@stupidchicken.com> wrote:

> This means that any time point moves below the bottom of the window, the
> resulting scroll will leave the cursor at the bottom of the window.  Any
> time point moves above the top of the window, the resulting scroll will
> leave the cursor at the top of the window.  Unlike numerical values of
> scroll-conservatively, there is no "scrolling limit" to check.

In other words, `scroll-conservatively' set to t means "scroll
line-by-line", or "scroll but never recenter". I'm OK with that (well,
I'm much more than OK; that's what I consider the only sane behavior
of scrolling, and what many other text editors do, though I readily
admit that most Emacs users seem to prefer the recentering behavior).

But the trick is making it fast enough, isn't? IIRC, the current
sluggish performance and some other changes were brought in to cope
with redisplay pausing for a while and then suddenly "jumping ahead"
in the buffer because it wasn't fast enough to keep up with typematic
repeat during scrolling (pre-Unicode2 merge it always was,
post-Unicode2 scrolling line-by-line is jumpy the best of days,
specially when moving up the beginning of the buffer).

    Juanma

bug#6671: moving point and scroll-conservatively

[Chong Yidong]

Juanma Barranquero <lekktu@gmail.com> writes:

> But the trick is making it fast enough, isn't? IIRC, the current
> sluggish performance and some other changes were brought in to cope
> with redisplay pausing for a while and then suddenly "jumping ahead"
> in the buffer because it wasn't fast enough to keep up with typematic
> repeat during scrolling (pre-Unicode2 merge it always was,
> post-Unicode2 scrolling line-by-line is jumpy the best of days,
> specially when moving up the beginning of the buffer).

I don't think the problem is keyboard-triggered commands changing the
buffer too fast between redisplays.  I've done a bit of tracing, and
redisplay is called once for every one, some times two, calls to
command-execute.  Certainly not the ten or twenty times that would cause
window scrolling to fall behind.

Instead, the recentering is caused by the situation I described in an
earlier email, where the action of fontification functions causes
redisplay to become confused.  I don't know whether there are other
causes of anomalous recentering, but that is the only one I have
encountered while experimenting with the code.

bug#6671: moving point and scroll-conservatively

[Eli Zaretskii]

> From: Chong Yidong <cyd@stupidchicken.com>
> Cc: Eli Zaretskii <eliz@gnu.org>, 6671@debbugs.gnu.org
> Date: Thu, 24 Mar 2011 22:09:28 -0400
> 
> I don't think the problem is keyboard-triggered commands changing the
> buffer too fast between redisplays.  I've done a bit of tracing, and
> redisplay is called once for every one, some times two, calls to
> command-execute.  Certainly not the ten or twenty times that would cause
> window scrolling to fall behind.

It could easily fall behind with high enough rate of keyboard
auto-repeat operation.  There are 2 possible situations: when
redisplay-dont-pause is set and when it is nil.

When redisplay-dont-pause is set, we don't check for available input
until we finish one redisplay cycle completely.  When the user leans
on a key and high-rate auto-repeat is at work and, we can easily have
many keystrokes in the buffer by the time we finish a full redisplay,
especially if you have many large frames.  Processing many <down>
strokes, for example, can easily get you several screenfuls away.

When redisplay-dont-pause is nil, we check for input in several places
during redisplay, and bail out if there's input.  In that case, with
high-rate keyboard input, we quickly create a situation where
important data structures and flags are out of sync and cannot be
relied upon, and try_scrolling cannot be used at all, which again
means recentering (because we only try not to recenter inside
try_scrolling).

> Instead, the recentering is caused by the situation I described in an
> earlier email, where the action of fontification functions causes
> redisplay to become confused.  I don't know whether there are other
> causes of anomalous recentering, but that is the only one I have
> encountered while experimenting with the code.

Try leaning on <down> in etc/HELLO, after turning off font-lock.

bug#6671: moving point and scroll-conservatively

[Eli Zaretskii]

> From: Chong Yidong <cyd@stupidchicken.com>
> Cc: lekktu@gmail.com, 6671@debbugs.gnu.org
> Date: Thu, 24 Mar 2011 17:36:36 -0400
> 
> The solution you suggested was:
> 
> > when point is "far away" (which could be set back to those proverbial
> > 10 screen lines), then, instead of moving one line at a time, move to
> > point in one go, and then compute the window start so that point is at
> > the proper place relative to window start.
> 
> If the current literal meaning of scroll-conservatively is retained, we
> need to count the number of lines involved in performing this "far-away
> scroll", by iterating.  That brings us back to square one.

Granted, I didn't mean to get back to square one, i.e. I didn't mean
to retain the literal meaning of scroll-conservatively.  (It wasn't
interpreted literally before those changes, anyway.  The manual
describes the effect of scroll-conservatively only for small
arguments, so the effect of setting it to a large value like 200000 is
undefined, and we can make it do anything that is reasonable.)

What I mean to do is make it so redisplay always positions point on
the last or first line of the window when scroll-conservatively is a
large number, so it would never-ever recenter in that case, not even
if try_scrolling fails.  (Currently, scroll-conservatively is only
checked inside try_scrolling, and the net effect is that it is
sometimes ignored, under conditions that are not well-defined.  IOW,
its effect is unpredictable for users who want predictable behavior in
this regard.)

Whether the threshold value of scroll-conservatively for never
recentering is 300 or not is not important, as long as we document
that threshold.  (FWIW, I'd go with a lower number, like 100.)  I
don't see a reason for having a variable that gives users control on
that threshold: if the threshold is documented, users can always get
the desired effect by changing the value of scroll-conservatively.

> If you want to avoid this pitfall, your solution and mine become the
> same, i.e. special handling of "sufficiently large" values of
> scroll-conservatively.

Yes, but with 2 important differences:

 . my proposal doesn't introduce a new value for scroll-conservatively

 . it makes sure we never recenter with a large value of that option,
   not even if try_scrolling cannot be used

bug#6671: moving point and scroll-conservatively

[Eli Zaretskii]

> From: Chong Yidong <cyd@stupidchicken.com>
> Cc: lekktu@gmail.com, 6671@debbugs.gnu.org
> Date: Thu, 24 Mar 2011 11:14:25 -0400
> 
> These change you propose would also necessitate reverting 100619/100620.

100620, perhaps.  I don't see any need to revert 100619, because it
just limited the search to values smaller than 10 screen lines.

bug#6671: moving point and scroll-conservatively

[Eli Zaretskii]

> From: Chong Yidong <cyd@stupidchicken.com>
> Cc: Eli Zaretskii <eliz@gnu.org>, 6671@debbugs.gnu.org
> Date: Thu, 24 Mar 2011 22:09:28 -0400
> 
> Instead, the recentering is caused by the situation I described in an
> earlier email, where the action of fontification functions causes
> redisplay to become confused.

If you can prevent this confusion in important use cases, please do.

But I really think that we need to be able to position point at the
first/last line of the window regardless of any "confusion".  This is
what users who set scroll-conservatively want.  They don't care how
"confused" can the display engine become.  And I think you cannot
prevent confusion altogether, because with input that comes in at high
enough rate, Emacs will eventually fall behind and will be unable to
use try_scrolling.  With today's code, if try_scrolling fails, we will
always recenter.

bug#6671: moving point and scroll-conservatively

[Eli Zaretskii]

> Date: Fri, 25 Mar 2011 10:48:09 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: lekktu@gmail.com, 6671@debbugs.gnu.org
> 
> When redisplay-dont-pause is set, we don't check for available input
> until we finish one redisplay cycle completely.  When the user leans
> on a key and high-rate auto-repeat is at work and, we can easily have
                                                ^^^
Strike out that "and".

> many keystrokes in the buffer by the time we finish a full redisplay,
> especially if you have many large frames.  Processing many <down>
> strokes, for example, can easily get you several screenfuls away.

bug#6671: moving point and scroll-conservatively

[Juanma Barranquero]

On Fri, Mar 25, 2011 at 09:48, Eli Zaretskii <eliz@gnu.org> wrote:

> When redisplay-dont-pause is nil, we check for input in several places
> during redisplay, and bail out if there's input.  In that case, with
> high-rate keyboard input, we quickly create a situation where
> important data structures and flags are out of sync and cannot be
> relied upon, and try_scrolling cannot be used at all, which again
> means recentering (because we only try not to recenter inside
> try_scrolling).

Indeed, I have `redisplay-dont-pause' set to t to avoid that kind of
recentering.

    Juanma

bug#6671: moving point and scroll-conservatively

[Chong Yidong]

Eli Zaretskii <eliz@gnu.org> writes:

> Whether the threshold value of scroll-conservatively for never
> recentering is 300 or not is not important, as long as we document
> that threshold.  (FWIW, I'd go with a lower number, like 100.)

Sounds fine by me.

> What I mean to do is make it so redisplay always positions point on
> the last or first line of the window when scroll-conservatively is a
> large number, so it would never-ever recenter in that case, not even
> if try_scrolling fails.

Yes, we agree.  OTOH, the easiest way to do this is probably by choosing
an appropriate value of centering_position in redisplay_window, but it's
up to you.  Just remember to take scroll-margin into account.

bug#6671: moving point and scroll-conservatively

[Eli Zaretskii]

> From: Chong Yidong <cyd@stupidchicken.com>
> Date: Wed, 23 Mar 2011 23:58:27 -0400
> Cc: 6671@debbugs.gnu.org
> 
> The precise mechanism by which the fontification functions screw things
> up is by narrowing the buffer.  This is why, in last year's thread, the
> buffer's clip_changed flag was found to be set.  That forces recentering
> in redisplay_window (xdisp.c:14159).
> 
> So, a good fix might be to check whether redisplay-time fontification
> changes the buffer's restrictions, and, if so, reset the clip_changed
> flag.  That should make the 100619/100620 changes unnecessary.

As result of that last year's thread (which, to recap, started with
http://lists.gnu.org/archive/html/emacs-devel/2010-06/msg00554.html),
I looked closer at the clip_changed flag, and discussed my findings
with Gerd Möllmann.  Our conclusion was that it shouldn't be necessary
to test this flag when deciding whether try_scrolling can be called.
So I now committed a change to that effect (revno 103756 on the
trunk); please see if recentering as result of fontification functions
is now avoided.

This does not yet constitute resolution of this bug, I'm still working
on that.  Stay tuned.

bug#6671: moving point and scroll-conservatively

[Juanma Barranquero]

On Sat, Mar 26, 2011 at 19:48, Eli Zaretskii <eliz@gnu.org> wrote:

> 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.

On first tests, it seems to work great. I see no unwanted recentering,
line-by-line scrolling is smooth and reasonably fast, and goto-line
moves around the file blindlingly fast again.

Scrolling via scroll-up-command is still bad if you lean on the key,
but that is not related to this patch, I think.

    Juanma

bug#6671: moving point and scroll-conservatively

[Eli Zaretskii]

> From: Juanma Barranquero <lekktu@gmail.com>
> Date: Sun, 27 Mar 2011 04:39:36 +0200
> Cc: Chong Yidong <cyd@stupidchicken.com>, 6671@debbugs.gnu.org
> 
> On Sat, Mar 26, 2011 at 19:48, Eli Zaretskii <eliz@gnu.org> wrote:
> 
> > 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.
> 
> On first tests, it seems to work great. I see no unwanted recentering,
> line-by-line scrolling is smooth and reasonably fast, and goto-line
> moves around the file blindlingly fast again.

That's good to hear.  Please see if it still works great after fixing
a stupid mistake with parentheses (see my other mail this morning).

> Scrolling via scroll-up-command is still bad if you lean on the key,
> but that is not related to this patch, I think.

In what sense is that "bad"?

And by "scroll-up-command" do you mean `scroll-up' and `scroll-down'?

bug#6671: moving point and scroll-conservatively

[Juanma Barranquero]

[Sorry, I somehow missed your answer yesterday]

On Sun, Mar 27, 2011 at 05:59, Eli Zaretskii <eliz@gnu.org> wrote:

> That's good to hear.  Please see if it still works great after fixing
> a stupid mistake with parentheses (see my other mail this morning).

Yes, it works great. Please install it.

> In what sense is that "bad"?

emacs xdisp.c    ;; not -Q, but with my setup
PgDown             ;; keep pressed

It scrolls for a while, then stops for a while, then jumps ahead
(sometimes, hundreds or thousands of lines).

> And by "scroll-up-command" do you mean `scroll-up' and `scroll-down'?

No.

 emacs -Q
 C-h k PgDown

<next> (translated from <kp-next>) runs the command scroll-up-command,
which is an interactive compiled Lisp function in `window.el'.


    Juanma

bug#6671: moving point and scroll-conservatively

[Eli Zaretskii]

> From: Juanma Barranquero <lekktu@gmail.com>
> Date: Tue, 29 Mar 2011 01:00:00 +0200
> Cc: cyd@stupidchicken.com, 6671@debbugs.gnu.org
> 
> [Sorry, I somehow missed your answer yesterday]
> 
> On Sun, Mar 27, 2011 at 05:59, Eli Zaretskii <eliz@gnu.org> wrote:
> 
> > That's good to hear.  Please see if it still works great after fixing
> > a stupid mistake with parentheses (see my other mail this morning).
> 
> Yes, it works great. Please install it.

I will in a couple of days.  I'm still working on minor issues related
to scrolling due to cursor motion (e.g., it was totally screwed up
when scroll-*-aggressively is set) and on code in try_scrolling that
cannot be anything but wrong, like this one:

	  if (arg_scroll_conservatively)
	    amount_to_scroll
	      = max (dy, FRAME_LINE_HEIGHT (f) * max (scroll_step, temp_scroll_step));

Huh? we're under scroll-conservatively, but don't consider its value
for scrolling?

> > In what sense is that "bad"?
> 
> emacs xdisp.c    ;; not -Q, but with my setup
> PgDown             ;; keep pressed
> 
> It scrolls for a while, then stops for a while, then jumps ahead
> (sometimes, hundreds or thousands of lines).

Ah, that one.  Yes, I see it.  I think that's because scroll-up uses a
heavy algorithm.  But its implementation is entirely separate from
what I was trying to fix, so it's expected that scroll-up and
scroll-down won't be affected.

> > And by "scroll-up-command" do you mean `scroll-up' and `scroll-down'?
> 
> No.
> 
>  emacs -Q
>  C-h k PgDown
> 
> <next> (translated from <kp-next>) runs the command scroll-up-command,
> which is an interactive compiled Lisp function in `window.el'.

I was talking from Emacs 23.  But anyway, the difference is small,
because scroll-up-command just calls scroll-up.

bug#6671: moving point and scroll-conservatively

[Juanma Barranquero]

On Tue, Mar 29, 2011 at 06:03, Eli Zaretskii <eliz@gnu.org> wrote:

> I will in a couple of days.  I'm still working on minor issues related
> to scrolling due to cursor motion (e.g., it was totally screwed up
> when scroll-*-aggressively is set)

Ah, I don't use that, so didn't test it.

> Huh? we're under scroll-conservatively, but don't consider its value
> for scrolling?

:-)

> But its implementation is entirely separate from
> what I was trying to fix, so it's expected that scroll-up and
> scroll-down won't be affected.

Right. I wonder whether there's a bug report for that... I'll take a look.

> I was talking from Emacs 23.  But anyway, the difference is small,
> because scroll-up-command just calls scroll-up.

Well, no. `scroll-up-command' does respect `scroll-error-top-bottom',
and that's all the difference in the world, to me. I don't want PgUp
from the first few lines in the buffer to move the point to
(point-min). Ever.

    Juanma

bug#6671: moving point and scroll-conservatively

[Eli Zaretskii]

> From: Juanma Barranquero <lekktu@gmail.com>
> Date: Tue, 29 Mar 2011 12:54:52 +0200
> Cc: cyd@stupidchicken.com, 6671@debbugs.gnu.org
> 
> > I was talking from Emacs 23.  But anyway, the difference is small,
> > because scroll-up-command just calls scroll-up.
> 
> Well, no. `scroll-up-command' does respect `scroll-error-top-bottom',
> and that's all the difference in the world, to me.

I meant no difference performance-wise, of course.

bug#6671: moving point and scroll-conservatively

[Eli Zaretskii]

> From: Juanma Barranquero <lekktu@gmail.com>
> Date: Tue, 29 Mar 2011 01:00:00 +0200
> Cc: cyd@stupidchicken.com, 6671@debbugs.gnu.org
> 
> On Sun, Mar 27, 2011 at 05:59, Eli Zaretskii <eliz@gnu.org> wrote:
> 
> > That's good to hear.  Please see if it still works great after fixing
> > a stupid mistake with parentheses (see my other mail this morning).
> 
> Yes, it works great. Please install it.

Done (in revision 103792 on the trunk).

I'm closing this bug, because I think it has been beaten to death.  If
there are any left-overs, or new bugs I introduced, please file new
bug reports.