The unwarranted scrolling assumption

The unwarranted scrolling assumption

[Lennart Borgman]

I get mad now and then at Emacs peculiar scrolling behavior. Now and
then I get told that it is fixed, but it is not as far as I can see.

It is a quite serious problem, but old Emacs users might oversee it
because of habits.

Here is what I think is the bad assumption:

* Many times you are looking at the point where the cursor is. If a
jump scroll happens then you just have to look for the cursor again. I
think the bad assumption is that you are always looking where the
cursor is when Emacs does it jumping scrolling.

So you get the advice to use packages that hilights the cursor. But it
does not help at all since you were not looking at that point.

Maybe the assumption that you are looking where the cursor is comes
from that there are other ways to scroll than to move the cursor to
the end of the window.

Perhaps there are such commands butt that does not help those that are
used to this kind of getting the window to scroll.


What is the problem with implementing a sane scroll in Emacs? I don't
understand it.

Re: The unwarranted scrolling assumption

[Juri Linkov]

> What is the problem with implementing a sane scroll in Emacs? I don't
> understand it.

Perhaps you need to customize `scroll-preserve-screen-position'.

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: The unwarranted scrolling assumption

[Juanma Barranquero]

On Tue, Jun 15, 2010 at 23:39, Juri Linkov <juri@jurta.org> wrote:

> Perhaps you need to customize `scroll-preserve-screen-position'.

And set line-by-line scrolling to deactivate the automatic scroll
recentering. Both are needed to get sane scrolling behavior (for
values of "sane" that exactly match my preferences, of course... ;-)

    Juanma

Re: The unwarranted scrolling assumption

[Lennart Borgman]

On Wed, Jun 16, 2010 at 12:41 AM, Juanma Barranquero <lekktu@gmail.com> wrote:
> On Tue, Jun 15, 2010 at 23:39, Juri Linkov <juri@jurta.org> wrote:
>
>> Perhaps you need to customize `scroll-preserve-screen-position'.

Thanks. Perhaps. I just set it to 'always but Emacs still do the
"jumping scrolling".

However not always. And that makes it even more frustrating. I feel
like a prey. Suddenly it jumps - and I do not know why and when.

Is that jumping expected?

> And set line-by-line scrolling to deactivate the automatic scroll
> recentering. Both are needed to get sane scrolling behavior (for
> values of "sane" that exactly match my preferences, of course... ;-)

I am not sure which option you mean. Could you please tell me?

>    Juanma
>

Re: The unwarranted scrolling assumption

[Sean Sieger]

    >> Perhaps you need to customize `scroll-preserve-screen-position'.

    Thanks. Perhaps. I just set it to 'always but Emacs still do the
    "jumping scrolling".

    However not always. And that makes it even more frustrating. I feel
    like a prey. Suddenly it jumps - and I do not know why and when.

    Is that jumping expected?

    > And set line-by-line scrolling to deactivate the automatic scroll
    > recentering. Both are needed to get sane scrolling behavior (for
    > values of "sane" that exactly match my preferences, of course... ;-)

    I am not sure which option you mean. Could you please tell me?

This seems to work well, Lennart:

(setq scroll-preserve-screen-position 'always
      scroll-conservatively           most-positive-fixnum
      scroll-step                     0)

Re: The unwarranted scrolling assumption

[Juanma Barranquero]

On Wed, Jun 16, 2010 at 01:24, Sean Sieger <sean.sieger@gmail.com> wrote:

> This seems to work well, Lennart:
>
> (setq scroll-preserve-screen-position 'always
>      scroll-conservatively           most-positive-fixnum
>      scroll-step                     0)

That is exactly what I do.

    Juanma

Re: The unwarranted scrolling assumption

[Lennart Borgman]

On Wed, Jun 16, 2010 at 1:46 AM, Juanma Barranquero <lekktu@gmail.com> wrote:
> On Wed, Jun 16, 2010 at 01:24, Sean Sieger <sean.sieger@gmail.com> wrote:
>
>> This seems to work well, Lennart:
>>
>> (setq scroll-preserve-screen-position 'always
>>      scroll-conservatively           most-positive-fixnum
>>      scroll-step                     0)
>
> That is exactly what I do.

Oh, I did not read your answer before I tested this. As I said in the
answer to Sean this does not work for me. I still get "jumping
scrolling".

All I do to check this is open window.c and then hit/hold down the
down arrow key (or "j" in viper).

Does this work without "jumping scrolling" for you? (You have to test
for a while, the jumping does not happen always. I is a bit like
playing a computer game.)

Re: The unwarranted scrolling assumption

[Juanma Barranquero]

On Wed, Jun 16, 2010 at 01:59, Lennart Borgman
<lennart.borgman@gmail.com> wrote:

> Does this work without "jumping scrolling" for you?

Yes.

    Juanma

Re: The unwarranted scrolling assumption

[Chad Brown]

[-- Attachment #1: Type: text/plain, Size: 994 bytes --]


On Jun 15, 2010, at 4:59 PM, Lennart Borgman wrote:
> 
> All I do to check this is open window.c and then hit/hold down the
> down arrow key (or "j" in viper).
> 
> Does this work without "jumping scrolling" for you? (You have to test
> for a while, the jumping does not happen always. I is a bit like
> playing a computer game.)

This can be a function of computer speed -- the settings make emacs try to keep up the scrolling, but if the cursor motion gets ahead of the scrolling, it chooses to jump-scroll rather than silently eating the scrolling.

Caveat: this is my `basic' understanding; I haven't tested exactly these settings.

What would you like Emacs to do, conceptually, if you move down lines faster than the display engine can redraw them?  I assume `scroll less' and `error' are both obvious non-starters, so I'm guessing that you would want emacs to remain syncronous, even if it meant being unresponsive in a scroll-redraw-repeat loop; is that correct?

*Chad

[-- Attachment #2: Type: text/html, Size: 1446 bytes --]

Re: The unwarranted scrolling assumption

[Lennart Borgman]

On Wed, Jun 16, 2010 at 2:29 AM, Chad Brown <yandros@mit.edu> wrote:
>
> On Jun 15, 2010, at 4:59 PM, Lennart Borgman wrote:
>
> All I do to check this is open window.c and then hit/hold down the
> down arrow key (or "j" in viper).
>
> Does this work without "jumping scrolling" for you? (You have to test
> for a while, the jumping does not happen always. I is a bit like
> playing a computer game.)
>
> This can be a function of computer speed -- the settings make emacs try to
> keep up the scrolling, but if the cursor motion gets ahead of the scrolling,
> it chooses to jump-scroll rather than silently eating the scrolling.
> Caveat: this is my `basic' understanding; I haven't tested exactly these
> settings.


I think you are right, it seems to be working that way.


> What would you like Emacs to do, conceptually, if you move down lines faster
> than the display engine can redraw them?


I think it would be better if Emacs just skipped redrawing and then
come back to correct output as it would look if it had been able to
redraw everything in time.

If it worked that way you could much more easily understand what is
happening (especially for the case when you are not looking at the
point where the cursor is). You could then just pause and wait if you
noticed that Emacs was behind. There would be no big surprises.

Is there perhaps already an option for this type of scrolling behavior?


> I assume `scroll less' and `error'
> are both obvious non-starters, so I'm guessing that you would want emacs to
> remain syncronous, even if it meant being unresponsive in a
> scroll-redraw-repeat loop; is that correct?


Conceptually synchronous, but it could skip to redraw some pictures.

Looking at it this way I wonder if there is a bug/weakness in the
display engine that prevents it from interrupting a display that has
not been finished. Anyone knows about that?


> *Chad

Re: The unwarranted scrolling assumption

[Stefan Monnier]

>> What would you like Emacs to do, conceptually, if you move down lines faster
>> than the display engine can redraw them?
> I think it would be better if Emacs just skipped redrawing and then
> come back to correct output as it would look if it had been able to
> redraw everything in time.

You might be right, here, but until someone dedicated to this attacks
the problem, I don't see it happening.  In the mean time, you may want
to try (setq redisplay-dont-pause t).


        Stefan

Re: The unwarranted scrolling assumption

[David De La Harpe Golden]

On 16/06/10 00:59, Lennart Borgman wrote:

> All I do to check this is open window.c and then hit/hold down the
> down arrow key (or "j" in viper).
>
> Does this work without "jumping scrolling" for you? (You have to test
> for a while, the jumping does not happen always. I is a bit like
> playing a computer game.)
>

I see this sometimes, with the recipe you outline.

Sometimes emacs seems to decide to recenter, when it has "too much" 
input while display is taking its time (which I guess it might for e.g. 
font-locked window.c or a bunch of foreign scripts on view-hello-file)

I found the faster I set keyboard autorepeat the easier it is to make 
happen.  e.g. with my usual "xset r rate 150 60" it happens very 
occasionally, but with "xset r rate 150 180" it happens a lot.

Re: The unwarranted scrolling assumption

[Lennart Borgman]

On Wed, Jun 16, 2010 at 2:55 AM, David De La Harpe Golden
<david@harpegolden.net> wrote:
> On 16/06/10 00:59, Lennart Borgman wrote:
>
>> All I do to check this is open window.c and then hit/hold down the
>> down arrow key (or "j" in viper).
>>
>> Does this work without "jumping scrolling" for you? (You have to test
>> for a while, the jumping does not happen always. I is a bit like
>> playing a computer game.)
>>
>
> I see this sometimes, with the recipe you outline.
>
> Sometimes emacs seems to decide to recenter, when it has "too much" input
> while display is taking its time (which I guess it might for e.g.
> font-locked window.c or a bunch of foreign scripts on view-hello-file)
>
> I found the faster I set keyboard autorepeat the easier it is to make
> happen.  e.g. with my usual "xset r rate 150 60" it happens very
> occasionally, but with "xset r rate 150 180" it happens a lot.


Just a somewhat wild guess:

I wonder if this happens in redisplay_window in xdisp.c. This
functions has a labeled called "recenter":

  /* Finally, just choose place to start which centers point */

 recenter:
  if (centering_position < 0)
    centering_position = window_box_height (w) / 2;

The intent of that label seems to be just trying to came up to speed
with the input. But that is exactly the problem here.

However I dislike the way the problem is solved. I would rather wish
that the display engine instead calculated a new window start point
from the input command queue. I have no idea if that is actually
possible.

What I can see is that redisplay_window can restart through
redisplay_internal (the comment at the label "need_larger_matrices"
says that).

Perhaps this possibility to restart can be used instead.

But this is over the head for me.


Kim, are you there? Help!

Re: The unwarranted scrolling assumption

[Kim F. Storm]

Lennart Borgman <lennart.borgman@gmail.com> writes:

> But this is over the head for me.
>
> Kim, are you there? Help!
>

I'm here, but I think at this time it is way over my head too.

Unless you work on xdisp almost every day, you wont know what's up or down
and it's been two years since I last looked.

-- 
Kim F. Storm  http://www.cua.dk

Re: The unwarranted scrolling assumption

[Lennart Borgman]

On Wed, Jun 16, 2010 at 11:36 AM, Kim F. Storm <no-spam@cua.dk> wrote:
> Lennart Borgman <lennart.borgman@gmail.com> writes:
>
>> But this is over the head for me.
>>
>> Kim, are you there? Help!
>>
>
> I'm here, but I think at this time it is way over my head too.
>
> Unless you work on xdisp almost every day, you wont know what's up or down
> and it's been two years since I last looked.


I see. Perhaps you can comment on my thoughts below.

I looked around a bit and it looks like perhaps clip_changed is the
problem, but I am not sure. Save-restriction and friends sets this to
1.

redisplay_window checks this somewhere around line 14500 where the
comment says "Try to scroll by specified few lines". If clip_changed
is set then the if arg will be 0 and the "jumping scrolling" will
happen. At least that is how I understand it at the moment.

The comment for clip_changed says

  /* Set nonzero whenever the narrowing is changed in this buffer.  */
  int clip_changed;

This does not really tell what it is used for. Is it used for the
display engine or something else (or several things)?

If it is meant for the display engine only then I think that
save-restriction and friends should be a bit more careful with setting
it since there is no need to set it unless the widening state has
changed when next redisplay is done.

Re: The unwarranted scrolling assumption

[Eli Zaretskii]

> From: Lennart Borgman <lennart.borgman@gmail.com>
> Date: Wed, 16 Jun 2010 19:35:53 +0200
> Cc: "Kim F. Storm" <storm@cua.dk>, emacs-devel@gnu.org,
> 	David De La Harpe Golden <david@harpegolden.net>
> 
> I looked around a bit and it looks like perhaps clip_changed is the
> problem, but I am not sure. Save-restriction and friends sets this to
> 1.
> 
> redisplay_window checks this somewhere around line 14500 where the
> comment says "Try to scroll by specified few lines". If clip_changed
> is set then the if arg will be 0 and the "jumping scrolling" will
> happen. At least that is how I understand it at the moment.

I'm not sure this is the problem.  Can you step through try_scrolling
in the case that "jumping scrolling" happens, and tell what it does in
that case and what value it returns?

Re: The unwarranted scrolling assumption

[Lennart Borgman]

On Wed, Jun 16, 2010 at 11:22 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Lennart Borgman <lennart.borgman@gmail.com>
>> Date: Wed, 16 Jun 2010 19:35:53 +0200
>> Cc: "Kim F. Storm" <storm@cua.dk>, emacs-devel@gnu.org,
>>       David De La Harpe Golden <david@harpegolden.net>
>>
>> I looked around a bit and it looks like perhaps clip_changed is the
>> problem, but I am not sure. Save-restriction and friends sets this to
>> 1.
>>
>> redisplay_window checks this somewhere around line 14500 where the
>> comment says "Try to scroll by specified few lines". If clip_changed
>> is set then the if arg will be 0 and the "jumping scrolling" will
>> happen. At least that is how I understand it at the moment.
>
> I'm not sure this is the problem.  Can you step through try_scrolling
> in the case that "jumping scrolling" happens, and tell what it does in
> that case and what value it returns?

It is a bit diffucult to step through a problem like this. First you
have to guess what is happening and then you can add a little bit code
that let you just break when it happens for those reasons you guessed
... ;-)

And in this case it does not pass try_scroll when the "jumping
scrolling" happens.

I think the problem is in the handling of clip_changed as I said
before, but I am not sure it can be cured. A lot of calls are made to
narrow_to_region and I thought that the problem was the bugs in the
handling of clip_changed there, but that does not seem to be the whole
story.

But thanks for the tip. Is there anything else that comes up in your head here?

Re: The unwarranted scrolling assumption

[Eli Zaretskii]

> From: Lennart Borgman <lennart.borgman@gmail.com>
> Date: Thu, 17 Jun 2010 00:17:11 +0200
> Cc: david@harpegolden.net, storm@cua.dk, emacs-devel@gnu.org, no-spam@cua.dk
> 
> And in this case it does not pass try_scroll when the "jumping
> scrolling" happens.

So which parts of the following condition prevent it from calling
try_scrolling?  Please test all of them, not just the first one which
causes the condition to become false.

  if ((scroll_conservatively
       || scroll_step
       || temp_scroll_step
       || NUMBERP (current_buffer->scroll_up_aggressively)
       || NUMBERP (current_buffer->scroll_down_aggressively))
      && !current_buffer->clip_changed
      && CHARPOS (startp) >= BEGV
      && CHARPOS (startp) <= ZV)

> Is there anything else that comes up in your head here?

I need more data to have more ideas.

Re: The unwarranted scrolling assumption

[Lennart Borgman]

Maybe I should have sent this to you before. I saved it as a draft
(this is part of it), but thought it was unnecessary when I later
understood the root of the problem.

This is what I saw (with some debug tracing):

- The path try_scrolling is not taken.
- This is because clip_changed is 1.
- narrow_to_region set it to 1.
- reconsider_clip_changes gets a chance to reset it to 0, but it does
not in this case.
- reconsider_clip_changes does not however set it to 1.

The reason that reconsider_clip_changes does not set clip_changed to 0 is that

   !NILP (w->window_end_valid),

which looks like window_end_valid is non-nil.

This is and the code logic itself is the basis for my patches.

Re: The unwarranted scrolling assumption

[Eli Zaretskii]

> From: Lennart Borgman <lennart.borgman@gmail.com>
> Date: Thu, 17 Jun 2010 20:44:26 +0200
> Cc: emacs-devel@gnu.org
> 
> - The path try_scrolling is not taken.
> - This is because clip_changed is 1.
> - narrow_to_region set it to 1.
> - reconsider_clip_changes gets a chance to reset it to 0, but it does
> not in this case.
> - reconsider_clip_changes does not however set it to 1.
> 
> The reason that reconsider_clip_changes does not set clip_changed to 0 is that
> 
>    !NILP (w->window_end_valid),
> 
> which looks like window_end_valid is non-nil.

Thanks, this is a start, but something is still missing.  We have this
code at the beginning of reconsider_clip_changes:

  if (b->clip_changed
	   && !NILP (w->window_end_valid)
	   && w->current_matrix->buffer == b
	   && w->current_matrix->zv == BUF_ZV (b)
	   && w->current_matrix->begv == BUF_BEGV (b))
    b->clip_changed = 0;

So, if b->clip_changed is non-zero at entry to reconsider_clip_changes,
and w->window_end_valid is non-nil, as you say, this code should have
reset b->clip_changed to zero.  And yet you say it didn't.  Why not?
what prevented it from doing so?

Re: The unwarranted scrolling assumption

[Lennart Borgman]

On Thu, Jun 17, 2010 at 9:08 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Lennart Borgman <lennart.borgman@gmail.com>
>> Date: Thu, 17 Jun 2010 20:44:26 +0200
>> Cc: emacs-devel@gnu.org
>>
>> - The path try_scrolling is not taken.
>> - This is because clip_changed is 1.
>> - narrow_to_region set it to 1.
>> - reconsider_clip_changes gets a chance to reset it to 0, but it does
>> not in this case.
>> - reconsider_clip_changes does not however set it to 1.
>>
>> The reason that reconsider_clip_changes does not set clip_changed to 0 is that
>>
>>    !NILP (w->window_end_valid),
>>
>> which looks like window_end_valid is non-nil.
>
> Thanks, this is a start, but something is still missing.  We have this
> code at the beginning of reconsider_clip_changes:
>
>  if (b->clip_changed
>           && !NILP (w->window_end_valid)
>           && w->current_matrix->buffer == b
>           && w->current_matrix->zv == BUF_ZV (b)
>           && w->current_matrix->begv == BUF_BEGV (b))
>    b->clip_changed = 0;
>
> So, if b->clip_changed is non-zero at entry to reconsider_clip_changes,
> and w->window_end_valid is non-nil, as you say, this code should have
> reset b->clip_changed to zero.  And yet you say it didn't.  Why not?
> what prevented it from doing so?

I thought so first too, but please see the message I just sent with an
explanation of the patches I sent.

There might be some problem in the part above too, but my patches are
for another part of the problem. However the first question I gave to
you in the message I mention above may be a bit related to what you
ask here.

Re: The unwarranted scrolling assumption

[Eli Zaretskii]

> From: Lennart Borgman <lennart.borgman@gmail.com>
> Date: Thu, 17 Jun 2010 21:14:04 +0200
> Cc: emacs-devel@gnu.org
> 
> >  if (b->clip_changed
> >           && !NILP (w->window_end_valid)
> >           && w->current_matrix->buffer == b
> >           && w->current_matrix->zv == BUF_ZV (b)
> >           && w->current_matrix->begv == BUF_BEGV (b))
> >    b->clip_changed = 0;
> >
> > So, if b->clip_changed is non-zero at entry to reconsider_clip_changes,
> > and w->window_end_valid is non-nil, as you say, this code should have
> > reset b->clip_changed to zero.  And yet you say it didn't.  Why not?
> > what prevented it from doing so?
> 
> I thought so first too, but please see the message I just sent with an
> explanation of the patches I sent.

Sorry, that doesn't explain it to me, because narrowing is not part of
the recipe at all.

Again, PLEASE answer my questions, if you really want my help in
understanding this issue.  What parts of the above condition prevented
reconsider_clip_changes from resetting b->clip_changed to zero?  If
needed, please re-run under GDB (without your patches) and see what
other factors are at work here.

Re: The unwarranted scrolling assumption

[Lennart Borgman]

On Thu, Jun 17, 2010 at 9:26 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Lennart Borgman <lennart.borgman@gmail.com>
>> Date: Thu, 17 Jun 2010 21:14:04 +0200
>> Cc: emacs-devel@gnu.org
>>
>> >  if (b->clip_changed
>> >           && !NILP (w->window_end_valid)
>> >           && w->current_matrix->buffer == b
>> >           && w->current_matrix->zv == BUF_ZV (b)
>> >           && w->current_matrix->begv == BUF_BEGV (b))
>> >    b->clip_changed = 0;
>> >
>> > So, if b->clip_changed is non-zero at entry to reconsider_clip_changes,
>> > and w->window_end_valid is non-nil, as you say, this code should have
>> > reset b->clip_changed to zero.  And yet you say it didn't.  Why not?
>> > what prevented it from doing so?
>>
>> I thought so first too, but please see the message I just sent with an
>> explanation of the patches I sent.
>
> Sorry, that doesn't explain it to me, because narrowing is not part of
> the recipe at all.

You are making a bad assumption. Narrowing is happening during scrolling.

> Again, PLEASE answer my questions, if you really want my help in
> understanding this issue.  What parts of the above condition prevented
> reconsider_clip_changes from resetting b->clip_changed to zero?

Sorry, I thought I did tell that, but maybe I deleted that from some message.

It is !NILP (w->window_end_valid). There might be an error there too.

But it is not this that normally gives the "jumping scrolling". I am
not sure it is a problem at all.

> needed, please re-run under GDB (without your patches) and see what
> other factors are at work here.

Why should I do that?

Re: The unwarranted scrolling assumption

[Eli Zaretskii]

> From: Lennart Borgman <lennart.borgman@gmail.com>
> Date: Thu, 17 Jun 2010 21:35:51 +0200
> Cc: emacs-devel@gnu.org
> 
> > What parts of the above condition prevented
> > reconsider_clip_changes from resetting b->clip_changed to zero?
> 
> Sorry, I thought I did tell that, but maybe I deleted that from some message.
> 
> It is !NILP (w->window_end_valid).

??? How can this be?  The code says:

  if (b->clip_changed
	   && !NILP (w->window_end_valid)
	   && w->current_matrix->buffer == b
	   && w->current_matrix->zv == BUF_ZV (b)
	   && w->current_matrix->begv == BUF_BEGV (b))
    b->clip_changed = 0;

So if w->window_end_valid is non-nil, this code should reset
b->clip_changed to zero.  And you say it does _not_ reset it to zero.
So what prevents b->clip_changed from being reset to zero?

> But it is not this that normally gives the "jumping scrolling". I am
> not sure it is a problem at all.

Then what _is_ the problem?

Re: The unwarranted scrolling assumption

[Lennart Borgman]

On Fri, Jun 18, 2010 at 9:39 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Lennart Borgman <lennart.borgman@gmail.com>
>> Date: Thu, 17 Jun 2010 21:35:51 +0200
>> Cc: emacs-devel@gnu.org
>>
>> > What parts of the above condition prevented
>> > reconsider_clip_changes from resetting b->clip_changed to zero?
>>
>> Sorry, I thought I did tell that, but maybe I deleted that from some message.
>>
>> It is !NILP (w->window_end_valid).
>
> ??? How can this be?  The code says:

A misunderstanding, sorry. I meant that in this case that line/code
gave the value 0.

>  if (b->clip_changed
>           && !NILP (w->window_end_valid)
>           && w->current_matrix->buffer == b
>           && w->current_matrix->zv == BUF_ZV (b)
>           && w->current_matrix->begv == BUF_BEGV (b))
>    b->clip_changed = 0;
>
> So if w->window_end_valid is non-nil, this code should reset
> b->clip_changed to zero.  And you say it does _not_ reset it to zero.
> So what prevents b->clip_changed from being reset to zero?
>
>> But it is not this that normally gives the "jumping scrolling". I am
>> not sure it is a problem at all.
>
> Then what _is_ the problem?


So far there are two (mabye three) distinct bugs I have seen:

1) The handling of clip_changed, which I have sent a patch for.
(Please remember it is not quite finished. Some comments on that part
would be nice.)

2) The problems I described with visual-line-mode around line 702 in window.c.

Re: The unwarranted scrolling assumption

[Eli Zaretskii]

> From: Lennart Borgman <lennart.borgman@gmail.com>
> Date: Fri, 18 Jun 2010 15:30:37 +0200
> Cc: emacs-devel@gnu.org
> 
> >> It is !NILP (w->window_end_valid).
> >
> > ??? How can this be?  The code says:
> 
> A misunderstanding, sorry. I meant that in this case that line/code
> gave the value 0.
> 
> >  if (b->clip_changed
> >           && !NILP (w->window_end_valid)
> >           && w->current_matrix->buffer == b
> >           && w->current_matrix->zv == BUF_ZV (b)
> >           && w->current_matrix->begv == BUF_BEGV (b))
> >    b->clip_changed = 0;

So what is the value of w->window_end_valid when you enter
reconsider_clip_changes?  Is it nil?

> So far there are two (mabye three) distinct bugs I have seen:
> 
> 1) The handling of clip_changed, which I have sent a patch for.

I explained in another message why I think your patch is wrong.

The clip_changed flag should be reset to zero in order to enable the
scrolling optimization.  This is done by reconsider_clip_changes.  So
I think we should focus on understanding why it does not, in your
case.

> 2) The problems I described with visual-line-mode around line 702 in window.c.

That's an entirely different problem.  Let's handle this one problem
at a time.

Re: The unwarranted scrolling assumption

[Lennart Borgman]

On Fri, Jun 18, 2010 at 3:52 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Lennart Borgman <lennart.borgman@gmail.com>
>> Date: Fri, 18 Jun 2010 15:30:37 +0200
>> Cc: emacs-devel@gnu.org
>>
>> >> It is !NILP (w->window_end_valid).
>> >
>> > ??? How can this be?  The code says:
>>
>> A misunderstanding, sorry. I meant that in this case that line/code
>> gave the value 0.
>>
>> >  if (b->clip_changed
>> >           && !NILP (w->window_end_valid)
>> >           && w->current_matrix->buffer == b
>> >           && w->current_matrix->zv == BUF_ZV (b)
>> >           && w->current_matrix->begv == BUF_BEGV (b))
>> >    b->clip_changed = 0;
>
> So what is the value of w->window_end_valid when you enter
> reconsider_clip_changes?  Is it nil?

This is the what I used to get the output I sent:

  if (b->clip_changed)
    DebPrint (("+++reconsider: end_valid=%d, b=%d, ZV=%d, BEGV=%d",
               !NILP (w->window_end_valid),
               w->current_matrix->buffer == b,
               w->current_matrix->zv == BUF_ZV (b),
               w->current_matrix->begv == BUF_BEGV (b)));



But I think you are on the wrong track here. Can we please leave this
until my patch has been investigated a bit more?

The problem is how clip_changed is handled by narrow_to_region etc.
That is what my patch is about.


>> So far there are two (mabye three) distinct bugs I have seen:
>>
>> 1) The handling of clip_changed, which I have sent a patch for.
>
> I explained in another message why I think your patch is wrong.
>
> The clip_changed flag should be reset to zero in order to enable the
> scrolling optimization.  This is done by reconsider_clip_changes.  So
> I think we should focus on understanding why it does not, in your
> case.


As I have said several times I think the problem is that clip_changed
is set by narrow_to_region etc. Please look into this first.


>> 2) The problems I described with visual-line-mode around line 702 in window.c.
>
> That's an entirely different problem.  Let's handle this one problem
> at a time.


Yes, let us not confuse them.

Re: The unwarranted scrolling assumption

[Eli Zaretskii]

> From: Lennart Borgman <lennart.borgman@gmail.com>
> Date: Fri, 18 Jun 2010 16:07:17 +0200
> Cc: emacs-devel@gnu.org
> 
> On Fri, Jun 18, 2010 at 3:52 PM, Eli Zaretskii <eliz@gnu.org> wrote:
> >> From: Lennart Borgman <lennart.borgman@gmail.com>
> >> Date: Fri, 18 Jun 2010 15:30:37 +0200
> >> Cc: emacs-devel@gnu.org
> >>
> >> >> It is !NILP (w->window_end_valid).
> >> >
> >> > ??? How can this be?  The code says:
> >>
> >> A misunderstanding, sorry. I meant that in this case that line/code
> >> gave the value 0.
> >>
> >> >  if (b->clip_changed
> >> >           && !NILP (w->window_end_valid)
> >> >           && w->current_matrix->buffer == b
> >> >           && w->current_matrix->zv == BUF_ZV (b)
> >> >           && w->current_matrix->begv == BUF_BEGV (b))
> >> >    b->clip_changed = 0;
> >
> > So what is the value of w->window_end_valid when you enter
> > reconsider_clip_changes?  Is it nil?
> 
> This is the what I used to get the output I sent:
> 
>   if (b->clip_changed)
>     DebPrint (("+++reconsider: end_valid=%d, b=%d, ZV=%d, BEGV=%d",
>                !NILP (w->window_end_valid),
>                w->current_matrix->buffer == b,
>                w->current_matrix->zv == BUF_ZV (b),
>                w->current_matrix->begv == BUF_BEGV (b)));

And what was the output?  I don't see it in this thread.

> The problem is how clip_changed is handled by narrow_to_region etc.
> That is what my patch is about.

I think your patch is wrong.  I think the current code handles the
case you are trying to "fix" just fine.  It tried to recenter for some
reason, and I'm trying to find out what that reason was.  I'm guessing
that the reason was the window_end_valid flag was nil.  If you confirm
that, we could then start looking for why that flag was nil.  Reusing
the current matrix when that flag is nil is unsafe.

> As I have said several times I think the problem is that clip_changed
> is set by narrow_to_region etc. Please look into this first.

I already did, and I think that code is correct.  Obviously,
clip_changed needs to be set whenever the narrowing changes.

Re: The unwarranted scrolling assumption

[Lennart Borgman]

On Fri, Jun 18, 2010 at 4:29 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>>
>> This is the what I used to get the output I sent:
>>
>>   if (b->clip_changed)
>>     DebPrint (("+++reconsider: end_valid=%d, b=%d, ZV=%d, BEGV=%d",
>>                !NILP (w->window_end_valid),
>>                w->current_matrix->buffer == b,
>>                w->current_matrix->zv == BUF_ZV (b),
>>                w->current_matrix->begv == BUF_BEGV (b)));
>
> And what was the output?  I don't see it in this thread.


Sorry, I do not have it currently. If necessary I will recreate it,
but not until you have convinced me that my patch is wrong.


>> The problem is how clip_changed is handled by narrow_to_region etc.
>> That is what my patch is about.
>
> I think your patch is wrong.


Yes I know. But let us look into this. So far you have not convinced
me. Please try to.


> I think the current code handles the
> case you are trying to "fix" just fine.


It looks like you are totally missing the point of my patch.


> It tried to recenter for some
> reason, and I'm trying to find out what that reason was.  I'm guessing
> that the reason was the window_end_valid flag was nil.  If you confirm
> that, we could then start looking for why that flag was nil.  Reusing
> the current matrix when that flag is nil is unsafe.


Sure I will later if it is necessary. But this is not where we are now.


>> As I have said several times I think the problem is that clip_changed
>> is set by narrow_to_region etc. Please look into this first.
>
> I already did, and I think that code is correct.  Obviously,
> clip_changed needs to be set whenever the narrowing changes.


Ah, finally. Good.

And why do you think that?

Re: The unwarranted scrolling assumption

[Eli Zaretskii]

> From: Lennart Borgman <lennart.borgman@gmail.com>
> Date: Fri, 18 Jun 2010 16:37:55 +0200
> Cc: emacs-devel@gnu.org
> 
> >> As I have said several times I think the problem is that clip_changed
> >> is set by narrow_to_region etc. Please look into this first.
> >
> > I already did, and I think that code is correct.  Obviously,
> > clip_changed needs to be set whenever the narrowing changes.
> 
> 
> Ah, finally. Good.
> 
> And why do you think that?

Because that's how it is documented.

Re: The unwarranted scrolling assumption

[Lennart Borgman]

On Fri, Jun 18, 2010 at 4:58 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Lennart Borgman <lennart.borgman@gmail.com>
>> Date: Fri, 18 Jun 2010 16:37:55 +0200
>> Cc: emacs-devel@gnu.org
>>
>> >> As I have said several times I think the problem is that clip_changed
>> >> is set by narrow_to_region etc. Please look into this first.
>> >
>> > I already did, and I think that code is correct.  Obviously,
>> > clip_changed needs to be set whenever the narrowing changes.
>>
>>
>> Ah, finally. Good.
>>
>> And why do you think that?
>
> Because that's how it is documented.


That is a surprising answer to me. Do you want to explain what you really mean?

I feel quite frustrated and wonder why my message does not reach you.

You are looking for the reason why reconsider_clip_changes does not do
its job. I have said (though maybe not explicitly enough, I don't
know) that it can't because it does not have the information it needs.

It does not have that information because narrow_to_region has erased
it by setting clip_changed to 1.

(And yes, the lines where I check if clip_changed need to be set to 1
should of course go into reconsider_clip_changes.)

Re: The unwarranted scrolling assumption

[Eli Zaretskii]

> From: Lennart Borgman <lennart.borgman@gmail.com>
> Date: Fri, 18 Jun 2010 17:08:18 +0200
> Cc: emacs-devel@gnu.org
> 
> You are looking for the reason why reconsider_clip_changes does not do
> its job. I have said (though maybe not explicitly enough, I don't
> know) that it can't because it does not have the information it needs.
> 
> It does not have that information because narrow_to_region has erased
> it by setting clip_changed to 1.

The information is not erased, it is still present in
w->current_matrix->zv and w->current_matrix->begv.  You will see that
reconsider_clip_changes uses those to decide whether to reset the
clip_changed flag.

Re: The unwarranted scrolling assumption

[Lennart Borgman]

On Fri, Jun 18, 2010 at 5:35 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Lennart Borgman <lennart.borgman@gmail.com>
>> Date: Fri, 18 Jun 2010 17:08:18 +0200
>> Cc: emacs-devel@gnu.org
>>
>> You are looking for the reason why reconsider_clip_changes does not do
>> its job. I have said (though maybe not explicitly enough, I don't
>> know) that it can't because it does not have the information it needs.
>>
>> It does not have that information because narrow_to_region has erased
>> it by setting clip_changed to 1.
>
> The information is not erased, it is still present in
> w->current_matrix->zv and w->current_matrix->begv.  You will see that
> reconsider_clip_changes uses those to decide whether to reset the
> clip_changed flag.


Oh, fine, I missed that. Then we do not need the new variables I introduced.

But that does not change the logic.

What I did was setting clip_changed to 1 if the clipping had changed
since last redisplay instead of doing it in narrow_to_region etc.

Can you see anything wrong in that?

You seem to say that window_valid_end should be considered too. I
can't see why since setting it here is instead of setting it in
narrow_... etc. Do you really see a reason why window_valid_end shoudl
be considered in this step? (It will be considered later in
reconsider_clip_changes but that is a different thing.)


So sofar I am not aware of any valid objections to the logic of my
patch. Please tell me if I am missing some.

Re: The unwarranted scrolling assumption

[Lennart Borgman]

[-- Attachment #1: Type: text/plain, Size: 1227 bytes --]

On Fri, Jun 18, 2010 at 6:39 PM, Lennart Borgman
<lennart.borgman@gmail.com> wrote:
> On Fri, Jun 18, 2010 at 5:35 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>>> From: Lennart Borgman <lennart.borgman@gmail.com>
>>> Date: Fri, 18 Jun 2010 17:08:18 +0200
>>> Cc: emacs-devel@gnu.org
>>>
>>> You are looking for the reason why reconsider_clip_changes does not do
>>> its job. I have said (though maybe not explicitly enough, I don't
>>> know) that it can't because it does not have the information it needs.
>>>
>>> It does not have that information because narrow_to_region has erased
>>> it by setting clip_changed to 1.
>>
>> The information is not erased, it is still present in
>> w->current_matrix->zv and w->current_matrix->begv.  You will see that
>> reconsider_clip_changes uses those to decide whether to reset the
>> clip_changed flag.
>
>
> Oh, fine, I missed that. Then we do not need the new variables I introduced.

I have switched to using those variables instead. Seems to work just
as well as my old patch.

I have attached the patches for the current version of xdisp.c I am
using now. Beside using those variable I have moved the check into
reconsider_clip_changes where it belongs.

[-- Attachment #2: xdisp-jumpscroll-1.diff --]
[-- Type: text/x-patch, Size: 5826 bytes --]

=== modified file 'src/xdisp.c'
--- trunk/src/xdisp.c	2010-06-01 02:34:49 +0000
+++ patched/src/xdisp.c	2010-06-18 17:21:15 +0000
@@ -11711,12 +11711,33 @@
      struct window *w;
      struct buffer *b;
 {
+  /* if ((b->clip_begv != b->begv) || (b->clip_zv != b->zv)) */
+  /*   b->clip_changed = 1; */
+
+  if ((w->current_matrix->begv != b->begv) || (w->current_matrix->zv != b->zv))
+    b->clip_changed = 1;
+
+  if (b->clip_changed
+      && !(!NILP (w->window_end_valid)
+	   && w->current_matrix->buffer == b
+	   && w->current_matrix->zv == BUF_ZV (b)
+	   && w->current_matrix->begv == BUF_BEGV (b)))
+    DebPrint (("+++reconsider: end_valid=%d, b=%d, ZV=%d, BEGV=%d",
+               !NILP (w->window_end_valid),
+               w->current_matrix->buffer == b,
+               w->current_matrix->zv == BUF_ZV (b),
+               w->current_matrix->begv == BUF_BEGV (b)));
+
   if (b->clip_changed
 	   && !NILP (w->window_end_valid)
 	   && w->current_matrix->buffer == b
 	   && w->current_matrix->zv == BUF_ZV (b)
 	   && w->current_matrix->begv == BUF_BEGV (b))
+    {
     b->clip_changed = 0;
+      b->clip_begv = b->begv;
+      b->clip_zv = b->zv;
+    }
 
   /* If display wasn't paused, and W is not a tool bar window, see if
      point has been moved into or out of a composition.  In that case,
@@ -13443,7 +13464,10 @@
 	  dy = line_bottom_y (&it) - y0;
 
 	  if (dy > scroll_max)
+            {
+              DebPrint (("try_scrolling SCROLLING_FAILED dy > scroll_max A"));
 	    return SCROLLING_FAILED;
+            }
 
 	  scroll_down_p = 1;
 	}
@@ -13475,7 +13499,10 @@
 	}
 
       if (amount_to_scroll <= 0)
+        {
+          DebPrint (("try_scrolling SCROLLING_FAILED amount_to_scroll <= 0 A"));
 	return SCROLLING_FAILED;
+        }
 
       start_display (&it, w, startp);
       move_it_vertically (&it, amount_to_scroll);
@@ -13515,7 +13542,10 @@
 		      MOVE_TO_POS | MOVE_TO_X | MOVE_TO_Y);
 	  dy = it.current_y - y0;
 	  if (dy > scroll_max)
+            {
+              DebPrint (("try_scrolling SCROLLING_FAILED dy > scroll_max B"));
 	    return SCROLLING_FAILED;
+            }
 
 	  /* Compute new window start.  */
 	  start_display (&it, w, startp);
@@ -13539,7 +13569,10 @@
 	    }
 
 	  if (amount_to_scroll <= 0)
+            {
+              DebPrint (("try_scrolling SCROLLING_FAILED amount_to_scroll <= 0 B"));
 	    return SCROLLING_FAILED;
+            }
 
 	  move_it_vertically_backward (&it, amount_to_scroll);
 	  startp = it.current.pos;
@@ -13556,6 +13589,7 @@
   else if (w->cursor.vpos < 0)
     {
       clear_glyph_matrix (w->desired_matrix);
+      DebPrint (("try_scrolling SCROLLING_FAILED clear_glyph"));
       rc = SCROLLING_FAILED;
     }
   else
@@ -14235,7 +14269,10 @@
 
   /* If window-start is screwed up, choose a new one.  */
   if (XMARKER (w->start)->buffer != current_buffer)
+    {
+      DebPrint (("redisplay_window.recenter window-start screwed up"));
     goto recenter;
+    }
 
   SET_TEXT_POS_FROM_MARKER (startp, w->start);
 
@@ -14389,6 +14426,7 @@
 #if GLYPH_DEBUG
       debug_method_add (w, "recenter 1");
 #endif
+      DebPrint (("redisplay_window.recenter orig beg of line, but not now"));
       goto recenter;
     }
 
@@ -14515,6 +14553,14 @@
       && CHARPOS (startp) >= BEGV
       && CHARPOS (startp) <= ZV)
     {
+      DebPrint (("redisplay_window b try_scr, clip_changed=%d, >= BEGV: %d, <= ZV: %d, jtop=%d, llmf=%d",
+                 current_buffer->clip_changed,
+                 CHARPOS (startp) >= BEGV,
+                 CHARPOS (startp) <= ZV,
+                 just_this_one_p,
+                 last_line_misfit
+                 ));
+
       /* The function returns -1 if new fonts were loaded, 1 if
 	 successful, 0 if not successful.  */
       int rc = try_scrolling (window, just_this_one_p,
@@ -14530,6 +14576,7 @@
 	  goto need_larger_matrices;
 
 	case SCROLLING_FAILED:
+          DebPrint (("redisplay_window.SCROLLING_FAILED"));
 	  break;
 
 	default:
@@ -14539,6 +14586,7 @@
 
   /* Finally, just choose place to start which centers point */
 
+  DebPrint (("redisplay_window.recenter centering_position=%d", centering_position));
  recenter:
   if (centering_position < 0)
     centering_position = window_box_height (w) / 2;
@@ -14644,6 +14692,7 @@
 	{
 	  w->vscroll = 0;
 	  clear_glyph_matrix (w->desired_matrix);
+          DebPrint (("redisplay_window.recenter disable vscroll and try again"));
 	  goto recenter;
 	}
 
@@ -14655,6 +14704,7 @@
 
       clear_glyph_matrix (w->desired_matrix);
       centering_position = 0;
+      DebPrint (("redisplay_window.recenter failed to make whole line visible"));
       goto recenter;
     }
 
@@ -14852,8 +14902,11 @@
       if (display_line (&it))
 	last_text_row = it.glyph_row - 1;
       if (fonts_changed_p && !(flags & TRY_WINDOW_IGNORE_FONTS_CHANGE))
+        {
+          DebPrint (("try_window: fonts_changed_p=%d", fonts_changed_p));
 	return 0;
     }
+    }
 
   /* Don't let the cursor end in the scroll margins.  */
   if ((flags & TRY_WINDOW_CHECK_MARGINS)
@@ -14881,6 +14934,7 @@
 	{
 	  w->cursor.vpos = -1;
 	  clear_glyph_matrix (w->desired_matrix);
+          DebPrint (("try_window returning -1"));
 	  return -1;
 	}
     }
@@ -26279,7 +26333,8 @@
 recenters point as usual.
 
 A value of zero means always recenter point if it moves off screen.  */);
-  scroll_conservatively = 0;
+  // scroll_conservatively = Vmost_positive_fixnum;
+  scroll_conservatively = make_number (MOST_POSITIVE_FIXNUM);
 
   DEFVAR_INT ("scroll-margin", &scroll_margin,
     doc: /* *Number of lines of margin at the top and bottom of a window.

Re: The unwarranted scrolling assumption

[Eli Zaretskii]

> From: Lennart Borgman <lennart.borgman@gmail.com>
> Date: Fri, 18 Jun 2010 19:31:30 +0200
> Cc: emacs-devel@gnu.org
> 
> +  if ((w->current_matrix->begv != b->begv) || (w->current_matrix->zv != b->zv))
> +    b->clip_changed = 1;

You cannot safely use w->current_matrix if w->window_end_valid is nil.

And if you add that test, you will end up with exactly the same code
that was there originally.

IOW, we still need to find out why reconsider_clip_changes fails to
reset the clip_changed flag.

Re: The unwarranted scrolling assumption

[Lennart Borgman]

On Fri, Jun 18, 2010 at 9:00 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Lennart Borgman <lennart.borgman@gmail.com>
>> Date: Fri, 18 Jun 2010 19:31:30 +0200
>> Cc: emacs-devel@gnu.org
>>
>> +  if ((w->current_matrix->begv != b->begv) || (w->current_matrix->zv != b->zv))
>> +    b->clip_changed = 1;
>
> You cannot safely use w->current_matrix if w->window_end_valid is nil.


So you say that w->current_matrix->begv is not valid unless
window_end_valid is non-nil.

All the above test does is comparing the current clipping with that
recorded in w->current_matrix->begv/zv. In that case we must set
clip_changed to 1.

Are you saying that w->current_matrix->begv/zv is set before the
redisplay is finished? (I do not have the code here so I can not check
at the moment.)

If you are saying that then what you are saying above seems correct to
me. But on the other hand then I think what you said before about that
the redisplay engine had enough information for setting clip_changed
is false.

Can you explain more exactly how you are thinking?


> And if you add that test, you will end up with exactly the same code
> that was there originally.
>
> IOW, we still need to find out why reconsider_clip_changes fails to
> reset the clip_changed flag.
>
>

Re: The unwarranted scrolling assumption

[Lennart Borgman]

On Fri, Jun 18, 2010 at 11:25 PM, Lennart Borgman
<lennart.borgman@gmail.com> wrote:
> On Fri, Jun 18, 2010 at 9:00 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>>> From: Lennart Borgman <lennart.borgman@gmail.com>
>>> Date: Fri, 18 Jun 2010 19:31:30 +0200
>>> Cc: emacs-devel@gnu.org
>>>
>>> +  if ((w->current_matrix->begv != b->begv) || (w->current_matrix->zv != b->zv))
>>> +    b->clip_changed = 1;
>>
>> You cannot safely use w->current_matrix if w->window_end_valid is nil.
>
>
> So you say that w->current_matrix->begv is not valid unless
> window_end_valid is non-nil.
>
> All the above test does is comparing the current clipping with that
> recorded in w->current_matrix->begv/zv. In that case we must set
> clip_changed to 1.
>
> Are you saying that w->current_matrix->begv/zv is set before the
> redisplay is finished? (I do not have the code here so I can not check
> at the moment.)


I just checked the code. w->current_matrix->begv/zv is only set in

  mark_window_display_accurate_1 (w, accurate_p)

That means that it has the value needed to set clip_changed as I suggested.

(So your first claim that w already had the necessary information
seems correct, i.e. with my patch, and the second is false. Not that I
care, but for clearness.)


I can't think of any reason that it would be terribly bad to set
clip_changed to 1 when the clipping has changed since last redisplay.
So I do not think there is any danger with my patch.

However I can think of reasons that it is not needed. If the clipping
change is outside w->current_matrix->begv/zv then obviously then I
believe the displayed region can't be influenced. Can anyone see any
way it could be influenced?


> If you are saying that then what you are saying above seems correct to
> me. But on the other hand then I think what you said before about that
> the redisplay engine had enough information for setting clip_changed
> is false.
>
> Can you explain more exactly how you are thinking?
>
>
>> And if you add that test, you will end up with exactly the same code
>> that was there originally.
>>
>> IOW, we still need to find out why reconsider_clip_changes fails to
>> reset the clip_changed flag.


I think I have explained that several times. narrow_to_region etc sets
clip_changed to 1 without ever caring about the state redisplay is in.

I could go into more details, but is that necessary?

Re: The unwarranted scrolling assumption

[Eli Zaretskii]

> From: Lennart Borgman <lennart.borgman@gmail.com>
> Date: Sat, 19 Jun 2010 02:18:13 +0200
> Cc: emacs-devel@gnu.org
> 
> I just checked the code. w->current_matrix->begv/zv is only set in
> 
>   mark_window_display_accurate_1 (w, accurate_p)
> 
> That means that it has the value needed to set clip_changed as I suggested.

You cannot base such general conclusions on what the code does now.
Each API and each member of the structures we use _must_ live up to
their contract, as documented in the source.  Any new code _must_
follow those contracts, or else it will break Emacs some day, if not
today.

> >> IOW, we still need to find out why reconsider_clip_changes fails to
> >> reset the clip_changed flag.
> 
> I think I have explained that several times. narrow_to_region etc sets
> clip_changed to 1 without ever caring about the state redisplay is in.

That is true, but then reconsider_clip_changes is supposed to fix the
value for redisplay's purposes.  The question is: why it seemingly
fails in your case, when you lean on the <down> key, in the original
recipe you posted?

> I could go into more details, but is that necessary?

Yes, it's necessary.  Without understanding the reasons of why
reconsider_clip_changes fails, your patch cannot be accepted, because
the need in such a patch and its correctness cannot be established.

But please do not "go into details" of how narrow_to_region is wrong
in setting the clip_changed flag.  That is not the issue here.  The
issue is what happens in reconsider_clip_changes, that prevents it
from resetting the clip_changed flag.

Re: The unwarranted scrolling assumption

[Lennart Borgman]

On Sat, Jun 19, 2010 at 10:44 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Lennart Borgman <lennart.borgman@gmail.com>
>> Date: Sat, 19 Jun 2010 02:18:13 +0200
>> Cc: emacs-devel@gnu.org
>>
>> I just checked the code. w->current_matrix->begv/zv is only set in
>>
>>   mark_window_display_accurate_1 (w, accurate_p)
>>
>> That means that it has the value needed to set clip_changed as I suggested.
>
> You cannot base such general conclusions on what the code does now.
> Each API and each member of the structures we use _must_ live up to
> their contract, as documented in the source.  Any new code _must_
> follow those contracts, or else it will break Emacs some day, if not
> today.


Why do you assume that I have not checked this? Please see the
description of the begv/zv members.


>> >> IOW, we still need to find out why reconsider_clip_changes fails to
>> >> reset the clip_changed flag.
>>
>> I think I have explained that several times. narrow_to_region etc sets
>> clip_changed to 1 without ever caring about the state redisplay is in.
>
> That is true, but then reconsider_clip_changes is supposed to fix the
> value for redisplay's purposes.  The question is: why it seemingly
> fails in your case, when you lean on the <down> key, in the original
> recipe you posted?


I have explained that this happens because of the bad assumption that
narrow_to_region can set clip_changed and that redisplay later can
correct this. It is not true because redisplay does not have the
needed information (i.e. it does not know if the change was made after
last successful redisplay.)


>> I could go into more details, but is that necessary?
>
> Yes, it's necessary.  Without understanding the reasons of why
> reconsider_clip_changes fails, your patch cannot be accepted, because
> the need in such a patch and its correctness cannot be established.


Please see above.


> But please do not "go into details" of how narrow_to_region is wrong
> in setting the clip_changed flag.  That is not the issue here.  The
> issue is what happens in reconsider_clip_changes, that prevents it
> from resetting the clip_changed flag.


I am afraid if you do not want to look into those details ()that I
have told too many times already) you will not be able to understand
the problem.

Re: The unwarranted scrolling assumption

[Eli Zaretskii]

> From: Lennart Borgman <lennart.borgman@gmail.com>
> Date: Fri, 18 Jun 2010 23:25:21 +0200
> Cc: emacs-devel@gnu.org
> 
> On Fri, Jun 18, 2010 at 9:00 PM, Eli Zaretskii <eliz@gnu.org> wrote:
> >> From: Lennart Borgman <lennart.borgman@gmail.com>
> >> Date: Fri, 18 Jun 2010 19:31:30 +0200
> >> Cc: emacs-devel@gnu.org
> >>
> >> +  if ((w->current_matrix->begv != b->begv) || (w->current_matrix->zv != b->zv))
> >> +    b->clip_changed = 1;
> >
> > You cannot safely use w->current_matrix if w->window_end_valid is nil.
> 
> 
> So you say that w->current_matrix->begv is not valid unless
> window_end_valid is non-nil.

I'm saying that _none_ of the members of w->current_matrix is safe to
use when the window_end_valid flag is nil.  This flag being nil means
that the current matrix is not yet up to date, so its various members
are not completely computed and do not reflect what's on the screen.

> Are you saying that w->current_matrix->begv/zv is set before the
> redisplay is finished?

The definition of ``redisplay is finished'' for each window is
precisely that this flag is set.  Obviously, the other members of the
current_matrix struct are computed before that, and not in a single
place.  So yes, the begv/zv members are computed before redisplay is
finished.

> If you are saying that then what you are saying above seems correct to
> me. But on the other hand then I think what you said before about that
> the redisplay engine had enough information for setting clip_changed
> is false.

Yes, because it makes sure the window_end_valid flag is non-nil.  Then
the information is valid and can be used.

Re: The unwarranted scrolling assumption

[Lennart Borgman]

On Sat, Jun 19, 2010 at 10:36 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Lennart Borgman <lennart.borgman@gmail.com>
>> Date: Fri, 18 Jun 2010 23:25:21 +0200
>> Cc: emacs-devel@gnu.org
>>
>> On Fri, Jun 18, 2010 at 9:00 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> >> From: Lennart Borgman <lennart.borgman@gmail.com>
>> >> Date: Fri, 18 Jun 2010 19:31:30 +0200
>> >> Cc: emacs-devel@gnu.org
>> >>
>> >> +  if ((w->current_matrix->begv != b->begv) || (w->current_matrix->zv != b->zv))
>> >> +    b->clip_changed = 1;
>> >
>> > You cannot safely use w->current_matrix if w->window_end_valid is nil.
>>
>>
>> So you say that w->current_matrix->begv is not valid unless
>> window_end_valid is non-nil.
>
> I'm saying that _none_ of the members of w->current_matrix is safe to
> use when the window_end_valid flag is nil.


I am glad that you pointed me to the variables, but what you are
saying here looks maybe more like an assumption you make. It not then
please explain exactly why you think the begv/zv members and not valid
at the point where I use them. (See below first.)


> This flag being nil means
> that the current matrix is not yet up to date, so its various members
> are not completely computed and do not reflect what's on the screen.
>
>> Are you saying that w->current_matrix->begv/zv is set before the
>> redisplay is finished?
>
> The definition of ``redisplay is finished'' for each window is
> precisely that this flag is set.  Obviously, the other members of the
> current_matrix struct are computed before that, and not in a single
> place.  So yes, the begv/zv members are computed before redisplay is
> finished.


No. begv/zv members are set when redisplay is finished and nowhere
else. This is also in accordance with what the documentation for them
says:

  /* Values of BEGV and ZV as of last redisplay.  Set in
     mark_window_display_accurate_1.  */
  int begv, zv;


> Yes, because it makes sure the window_end_valid flag is non-nil.  Then
> the information is valid and can be used.


Since they those begv/zv members are only set in
mark_window_display_accurate_1 and reconsider_clip_changes is always
called (or at least should have been called) before any redisplay
activity the information in w->current_matrix->begv/zv is precisely
what is needed to set clip_changed to 1. (Note that it would have been
different if I instead wanted to clip_changed to 0. For that case I
guess that the check of window_end_valid is correct, but it is a
completely different situation.)

However there is one (important) assumption I make here: Clip changes
can not occur during redisplay, i.e. between after
reconsider_clip_changes and mark_window_display_accurate_1.

Is this assumption valid?


If not then the patch needed to coordinate narrow_to_region etc with
redisplay is quite a bit more complicated. (I will explain why if
needed and there is a good reason to believe that the assumption above
is not valid.)

Re: The unwarranted scrolling assumption

[Stefan Monnier]

>> I already did, and I think that code is correct.  Obviously,
>> clip_changed needs to be set whenever the narrowing changes.
> Ah, finally. Good.
> And why do you think that?

because src/buffer.h says:

  /* Set nonzero whenever the narrowing is changed in this buffer.  */
  int clip_changed;

"clipping" and "narrowing" are the same thing.  So, changing the
narrowing should say that clipping has changed.  All it says is
"narrowing has been changed at some point, don't forget to check it".
So it's the redisplay's responsability to determine whether narrowing is
indeed different from what it was during the last redisplay (and
clip_changed is just a variable that can save us from performing this
check).  If setting clip_changed to 1 in the main loop has some other
effect than slowing things down, that means we have a bug in redisplay.



        Stefan

Re: The unwarranted scrolling assumption

[Lennart Borgman]

On Fri, Jun 18, 2010 at 5:22 PM, Stefan Monnier
<monnier@iro.umontreal.ca> wrote:
>>> I already did, and I think that code is correct.  Obviously,
>>> clip_changed needs to be set whenever the narrowing changes.
>> Ah, finally. Good.
>> And why do you think that?
>
> because src/buffer.h says:
>
>  /* Set nonzero whenever the narrowing is changed in this buffer.  */
>  int clip_changed;
>
> "clipping" and "narrowing" are the same thing.  So, changing the
> narrowing should say that clipping has changed.  All it says is
> "narrowing has been changed at some point, don't forget to check it".
> So it's the redisplay's responsability to determine whether narrowing is
> indeed different from what it was during the last redisplay (and
> clip_changed is just a variable that can save us from performing this
> check).  If setting clip_changed to 1 in the main loop has some other
> effect than slowing things down, that means we have a bug in redisplay.


Yes, clipping and narrowing is the same things, but clip_changed is
just something the display engine uses to see whether a redisplay is
necessary.

Currently redisplay is mislead by narrow_to_region etc setting
clip_changed to 1. The problem as I understands it is that redisplay
has not saved enough information to know whether the clip_changed
should be reset to 0 after the narrowing routines have set it to 1.

My patch fixes this by letting redisplay have the whole responsibility
for clip_changed. There are of course other ways to do it, but from a
logic point that seems the best and I think it gives much cleaner
code.

Re: The unwarranted scrolling assumption

[Lennart Borgman]

On Fri, Jun 18, 2010 at 5:29 PM, Lennart Borgman
<lennart.borgman@gmail.com> wrote:
>
> My patch fixes this by letting redisplay have the whole responsibility
> for clip_changed. There are of course other ways to do it, but from a
> logic point that seems the best and I think it gives much cleaner
> code.

A little bit more explicit: If I am not misreading/misremember the
code currently this will lead to a redisplay in some cases:

  (save-excursion (widen))

Why would redisplay have to investigate cases like that?

Re: The unwarranted scrolling assumption

[Lennart Borgman]

On Thu, Jun 17, 2010 at 9:26 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Lennart Borgman <lennart.borgman@gmail.com>
>> Date: Thu, 17 Jun 2010 21:14:04 +0200
>> Cc: emacs-devel@gnu.org
>>
>> >  if (b->clip_changed
>> >           && !NILP (w->window_end_valid)
>> >           && w->current_matrix->buffer == b
>> >           && w->current_matrix->zv == BUF_ZV (b)
>> >           && w->current_matrix->begv == BUF_BEGV (b))
>> >    b->clip_changed = 0;
>> >
>> > So, if b->clip_changed is non-zero at entry to reconsider_clip_changes,
>> > and w->window_end_valid is non-nil, as you say, this code should have
>> > reset b->clip_changed to zero.  And yet you say it didn't.  Why not?
>> > what prevented it from doing so?
>>
>> I thought so first too, but please see the message I just sent with an
>> explanation of the patches I sent.
>
> Sorry, that doesn't explain it to me, because narrowing is not part of
> the recipe at all.
>
> Again, PLEASE answer my questions, if you really want my help in
> understanding this issue.  What parts of the above condition prevented
> reconsider_clip_changes from resetting b->clip_changed to zero?  If
> needed, please re-run under GDB (without your patches) and see what
> other factors are at work here.

I can now answer you question again since I have found a new way to
make "jumping scrolling" appear. However the situation above is not
involved, at least not directly, since ->clip_changed is 0. (I think
there is an unused global variable clip_changed that is not used.)

The situation is now this:
- I am using my patch (in the state I sent it here).
- I have seen no problem with the patch (but as I said I think there
is one problem though it has not shown up).
- It is quite hard to make Emacs do the "jumping scrolling".

So it is better, still there are problems.

- It is possible to still get Emacs to do the "jumping scrolling".
- To do that I narrow the buffer to 45 c, turn on visual-line mode and
add a wrap-prefix on the line.
- Then if I just hold down the down arrow it can happen after a while.
- My output shows that it is because try_scrolling failed.

Some variables as I see them now at the recenter: label in redisplay_window:
  scroll_step == 1
  temp_scroll_step == 0
  current_buffer->clip_changed ==0   Why current_buffer, there is a
variable buffer here?
  current_buffer->scroll_up_aggressively == 45459482, same down

BTW, the use of current_buffer here seems to me to be a bug.

And it is because clip_changed is 0 that try_scrolling is tested, see
the output here from the DebPrint below and some statement later:

  redisplay_window b try_scr, clip_changed=0, >= BEGV: 1, <= ZV: 1,
jtop=1, llmf=0
  try_scrolling SCROLLING_FAILED clear_glyph <---------------- This
seem to be the reason why try_scrolling failed.
  redisplay_window.SCROLLING_FAILED (This is the return value from
try_scrolling)
  redisplay_window.recenter centering_position=-1 (This is at the
"recenter:" label)

      DebPrint (("redisplay_window b try_scr, clip_changed=%d, >=
BEGV: %d, <= ZV: %d, jtop=%d, llmf=%d",
                 current_buffer->clip_changed,
                 CHARPOS (startp) >= BEGV,
                 CHARPOS (startp) <= ZV,
                 just_this_one_p,
                 last_line_misfit
                 ));

The line in the buffer it stopped at is wrapped once and is indented.

I guess an interesting part may be why clear_glyph_matrix failed. Do
you think there is something interesting there?

Re: The unwarranted scrolling assumption

[Lennart Borgman]

On Fri, Jun 18, 2010 at 12:16 AM, Lennart Borgman
<lennart.borgman@gmail.com> wrote:
>
> I guess an interesting part may be why clear_glyph_matrix failed. Do
> you think there is something interesting there?

Eh, sorry, my confusion. It is not clear_glyph_matrix that failed, of
course. I just meant the reason we came to that line in try_scrolling.
What lead to the condition

  w->cursor.vpos < 0

BTW, looking at the code there are a couple of places where
w->cursor.vpos is set to -1. I guess the tests are just for if it is <
0. Wouldn't it be better to set different negative values so it would
be easier to see where it failed? (That is normal error tracing.)

Re: The unwarranted scrolling assumption

[Lennart Borgman]

On Fri, Jun 18, 2010 at 12:50 AM, Lennart Borgman
<lennart.borgman@gmail.com> wrote:
> On Fri, Jun 18, 2010 at 12:16 AM, Lennart Borgman
> <lennart.borgman@gmail.com> wrote:
>>
>> I guess an interesting part may be why clear_glyph_matrix failed. Do
>> you think there is something interesting there?
>
> Eh, sorry, my confusion. It is not clear_glyph_matrix that failed, of
> course. I just meant the reason we came to that line in try_scrolling.
> What lead to the condition
>
>  w->cursor.vpos < 0
>
> BTW, looking at the code there are a couple of places where
> w->cursor.vpos is set to -1. I guess the tests are just for if it is <
> 0. Wouldn't it be better to set different negative values so it would
> be easier to see where it failed? (That is normal error tracing.)

I notice now that this is clearly a separate issue, not related to
speed. It is another bug.

Recentering happens whenever you are at the bottom of the screen if
some conditions are met. I can reproduce it with this:

- Open window.c
- Goto line 702:

			     + WINDOW_HEADER_LINE_HEIGHT (w)),

- Turn on wrap-to-fill-column-mode. Turn it off again.
- Turn on visual-line-mode
- I have (window-width) = 55, height 34.
- Position to W in WINDOW (it is on a wrapped line)
- Use C-L to get this to the bottom.
- Press down arrow.

(Note: wrap-to-fill-column-mode justs adds add wrap-prefix here. Which
is for some reason does not remove. Bug ;-( --
wrap-to-fill-column-mode is of course part of nXhtml, but probably you
can just add a fill prefix on the whole line that is equal to what the
line indentation have.)

For me this recenters. And try_scrolling fails where I said before,
around line 13581:

  /* Display the window.  Give up if new fonts are loaded, or if point
     doesn't appear.  */
  if (!try_window (window, startp, 0))
    rc = SCROLLING_NEED_LARGER_MATRICES;
  else if (w->cursor.vpos < 0)
    {
      clear_glyph_matrix (w->desired_matrix);
      DebPrint (("try_scrolling SCROLLING_FAILED clear_glyph"));
      rc = SCROLLING_FAILED;
    }


Playing with this I see some other bugs to. Which bug I see depends on
the window width. 55 gives this one, 54 another one where point jumps
instead.

But I get no output in the message buffer or in the debugger telling
what went wrong. That is a bit strange.

Re: The unwarranted scrolling assumption

[Lennart Borgman]

On Fri, Jun 18, 2010 at 1:49 AM, Lennart Borgman
<lennart.borgman@gmail.com> wrote:
>
> Playing with this I see some other bugs to. Which bug I see depends on
> the window width. 55 gives this one, 54 another one where point jumps
> instead.
>
> But I get no output in the message buffer or in the debugger telling
> what went wrong. That is a bit strange.

The 54 width bug was a bit different, it happens during C-l, when it
should have positioned point at the bottom of the window. The old
point is now below the window and points get "recentered". (Please
when writing things like this put some output in the debugger about
what goes wrong! The recentering here is done by some part of Emacs
that discovers that point is below window bottom. Emacs should not
just panic. After all it has its own psychologist.)

For width 55 there is the bug I described before.

For width 56 things seems to work better.

Re: The unwarranted scrolling assumption

[Lennart Borgman]

On Fri, Jun 18, 2010 at 1:59 AM, Lennart Borgman
<lennart.borgman@gmail.com> wrote:
>
> For width 55 there is the bug I described before.
>
> For width 56 things seems to work better.

Forgot to say:

- In the 55 case line 702 is wrapped in 3 parts and you are in the
middle part when you are on W.

- In the 56 case it is wrapped in 2 parts and you are at the bottom
part when you are on W.

Re: The unwarranted scrolling assumption

[Eli Zaretskii]

> From: Lennart Borgman <lennart.borgman@gmail.com>
> Date: Fri, 18 Jun 2010 01:49:52 +0200
> Cc: emacs-devel@gnu.org
> 
> > Eh, sorry, my confusion. It is not clear_glyph_matrix that failed, of
> > course. I just meant the reason we came to that line in try_scrolling.
> > What lead to the condition
> >
> >  w->cursor.vpos < 0
> >
> > BTW, looking at the code there are a couple of places where
> > w->cursor.vpos is set to -1. I guess the tests are just for if it is <
> > 0. Wouldn't it be better to set different negative values so it would
> > be easier to see where it failed? (That is normal error tracing.)
> 
> I notice now that this is clearly a separate issue, not related to
> speed. It is another bug.

As I tried to explain, this is not a bug, but expected behavior, at
least in general: when scrolling fails to bring point into the window,
the try_scrolling optimization fails, and other methods are tried
instead.

> Recentering happens whenever you are at the bottom of the screen if
> some conditions are met. I can reproduce it with this:
> 
> - Open window.c
> - Goto line 702:
> 
> 			     + WINDOW_HEADER_LINE_HEIGHT (w)),
> 
> - Turn on wrap-to-fill-column-mode. Turn it off again.
> - Turn on visual-line-mode
> - I have (window-width) = 55, height 34.
> - Position to W in WINDOW (it is on a wrapped line)
> - Use C-L to get this to the bottom.
> - Press down arrow.
> 
> (Note: wrap-to-fill-column-mode justs adds add wrap-prefix here. Which
> is for some reason does not remove. Bug ;-( --
> wrap-to-fill-column-mode is of course part of nXhtml, but probably you
> can just add a fill prefix on the whole line that is equal to what the
> line indentation have.)
> 
> For me this recenters. And try_scrolling fails where I said before,
> around line 13581:
> 
>   /* Display the window.  Give up if new fonts are loaded, or if point
>      doesn't appear.  */
>   if (!try_window (window, startp, 0))
>     rc = SCROLLING_NEED_LARGER_MATRICES;
>   else if (w->cursor.vpos < 0)
>     {
>       clear_glyph_matrix (w->desired_matrix);
>       DebPrint (("try_scrolling SCROLLING_FAILED clear_glyph"));
>       rc = SCROLLING_FAILED;
>     }

Again, this is normal.  You cannot prevent recentering from kicking in
in some cases.

Re: The unwarranted scrolling assumption

[Lennart Borgman]

On Fri, Jun 18, 2010 at 9:58 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Lennart Borgman <lennart.borgman@gmail.com>
>> Date: Fri, 18 Jun 2010 01:49:52 +0200
>> Cc: emacs-devel@gnu.org
>>
>> > Eh, sorry, my confusion. It is not clear_glyph_matrix that failed, of
>> > course. I just meant the reason we came to that line in try_scrolling.
>> > What lead to the condition
>> >
>> >  w->cursor.vpos < 0
>> >
>> > BTW, looking at the code there are a couple of places where
>> > w->cursor.vpos is set to -1. I guess the tests are just for if it is <
>> > 0. Wouldn't it be better to set different negative values so it would
>> > be easier to see where it failed? (That is normal error tracing.)
>>
>> I notice now that this is clearly a separate issue, not related to
>> speed. It is another bug.
>
> As I tried to explain, this is not a bug, but expected behavior, at
> least in general: when scrolling fails to bring point into the window,
> the try_scrolling optimization fails, and other methods are tried
> instead.


I know but in this case it looks like a clear bug.


>> Recentering happens whenever you are at the bottom of the screen if
>> some conditions are met. I can reproduce it with this:
>>
>> - Open window.c
>> - Goto line 702:
>>
>>                            + WINDOW_HEADER_LINE_HEIGHT (w)),
>>
>> - Turn on wrap-to-fill-column-mode. Turn it off again.
>> - Turn on visual-line-mode
>> - I have (window-width) = 55, height 34.
>> - Position to W in WINDOW (it is on a wrapped line)
>> - Use C-L to get this to the bottom.
>> - Press down arrow.
>>
>> (Note: wrap-to-fill-column-mode justs adds add wrap-prefix here. Which
>> is for some reason does not remove. Bug ;-( --
>> wrap--fill-column-mode is of course part of nXhtml, but probably you
>> can just add a fill prefix on the whole line that is equal to what the
>> line indentation have.)
>>
>> For me this recenters. And try_scrolling fails where I said before,
>> around line 13581:
>>
>>   /* Display the window.  Give up if new fonts are loaded, or if point
>>      doesn't appear.  */
>>   if (!try_window (window, startp, 0))
>>     rc = SCROLLING_NEED_LARGER_MATRICES;
>>   else if (w->cursor.vpos < 0)
>>     {
>>       clear_glyph_matrix (w->desired_matrix);
>>       DebPrint (("try_scrolling SCROLLING_FAILED clear_glyph"));
>>       rc = SCROLLING_FAILED;
>>     }
>
> Again, this is normal.  You cannot prevent recentering from kicking in
> in some cases.


I do not think so. I see several small bugs around this. Please see
the later messages I sent for this example. (Point jumps etc. It is
not only scrolling.)

Re: The unwarranted scrolling assumption

[Eli Zaretskii]

> From: Lennart Borgman <lennart.borgman@gmail.com>
> Date: Fri, 18 Jun 2010 01:49:52 +0200
> Cc: emacs-devel@gnu.org
> 
> Recentering happens whenever you are at the bottom of the screen if
> some conditions are met. I can reproduce it with this:
> 
> - Open window.c
> - Goto line 702:
> 
> 			     + WINDOW_HEADER_LINE_HEIGHT (w)),
> 
> - Turn on wrap-to-fill-column-mode. Turn it off again.
> - Turn on visual-line-mode
> - I have (window-width) = 55, height 34.
> - Position to W in WINDOW (it is on a wrapped line)
> - Use C-L to get this to the bottom.
> - Press down arrow.

With the latest trunk, I cannot reproduce this: pressing the down
arrow just scrolls by a single line, as expected.  However, I'm not
sure I did all the necessary steps.

First, how did you get to window-width of 55?  I just resized the
Emacs frame.

Next, there's this:

> (Note: wrap-to-fill-column-mode justs adds add wrap-prefix here. Which
> is for some reason does not remove. Bug ;-( --
> wrap-to-fill-column-mode is of course part of nXhtml, but probably you
> can just add a fill prefix on the whole line that is equal to what the
> line indentation have.)

wrap-to-fill-column-mode is complicated.  If you still see the problem
with the current trunk, please show a recipe that does not use
wrap-to-fill-column-mode, but instead sets the fill prefix and/or
whatever else is needed to reproduce this problem in "emacs -Q".

Re: The unwarranted scrolling assumption

[Lennart Borgman]

On Sat, Jun 19, 2010 at 3:27 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Lennart Borgman <lennart.borgman@gmail.com>
>> Date: Fri, 18 Jun 2010 01:49:52 +0200
>> Cc: emacs-devel@gnu.org
>>
>> Recentering happens whenever you are at the bottom of the screen if
>> some conditions are met. I can reproduce it with this:
>>
>> - Open window.c
>> - Goto line 702:
>>
>>                            + WINDOW_HEADER_LINE_HEIGHT (w)),
>>
>> - Turn on wrap-to-fill-column-mode. Turn it off again.
>> - Turn on visual-line-mode
>> - I have (window-width) = 55, height 34.
>> - Position to W in WINDOW (it is on a wrapped line)
>> - Use C-L to get this to the bottom.
>> - Press down arrow.
>
> With the latest trunk, I cannot reproduce this: pressing the down
> arrow just scrolls by a single line, as expected.  However, I'm not
> sure I did all the necessary steps.
>
> First, how did you get to window-width of 55?  I just resized the
> Emacs frame.

Same.

> Next, there's this:
>
>> (Note: wrap-to-fill-column-mode justs adds add wrap-prefix here. Which
>> is for some reason does not remove. Bug ;-( --
>> wrap-to-fill-column-mode is of course part of nXhtml, but probably you
>> can just add a fill prefix on the whole line that is equal to what the
>> line indentation have.)
>
> wrap-to-fill-column-mode is complicated.  If you still see the problem
> with the current trunk, please show a recipe that does not use
> wrap-to-fill-column-mode, but instead sets the fill prefix and/or
> whatever else is needed to reproduce this problem in "emacs -Q".

I told that recipe when I mentioned wrap-to-fill-column-mode.
Mentioning that mode was just a way to accomplish that much more easy.


Unfortunately I can not test now for these reasons:

- It takes some time for the changes to reach Launpad.
- You have said you implemented another way to avoid the scrolling bug
and that will clash with my way.

I found it a bit disturbing if you just implemented another way. I
can't say if it is right or wrong until I have seen your patch but
from our discussion it looks to me like you at least before
misunderstood the problem.

Re: The unwarranted scrolling assumption

[Eli Zaretskii]

> From: Lennart Borgman <lennart.borgman@gmail.com>
> Date: Sat, 19 Jun 2010 15:36:16 +0200
> Cc: monnier@iro.umontreal.ca, emacs-devel@gnu.org
> 
> >> (Note: wrap-to-fill-column-mode justs adds add wrap-prefix here. Which
> >> is for some reason does not remove. Bug ;-( --
> >> wrap-to-fill-column-mode is of course part of nXhtml, but probably you
> >> can just add a fill prefix on the whole line that is equal to what the
> >> line indentation have.)
> >
> > wrap-to-fill-column-mode is complicated.  If you still see the problem
> > with the current trunk, please show a recipe that does not use
> > wrap-to-fill-column-mode, but instead sets the fill prefix and/or
> > whatever else is needed to reproduce this problem in "emacs -Q".
> 
> I told that recipe when I mentioned wrap-to-fill-column-mode.

No, you told just part of it:

> >> wrap-to-fill-column-mode is of course part of nXhtml, but probably you
> >> can just add a fill prefix on the whole line that is equal to what the
> >> line indentation have.)

Can you please make this more explicit?  Please show what I need to
type in Emacs to accomplish what you say here.

> Unfortunately I can not test now for these reasons:

If you tell me how to define the fill prefix, I can test myself.

> I found it a bit disturbing if you just implemented another way. I
> can't say if it is right or wrong until I have seen your patch but
> from our discussion it looks to me like you at least before
> misunderstood the problem.

What I implemented came out of the sub-thread of this thread where I
explained to Stefan why I think it sometimes recenters, and Stefan
agreed that fixing that is a good idea.  See

    http://lists.gnu.org/archive/html/emacs-devel/2010-06/msg00766.html
    http://lists.gnu.org/archive/html/emacs-devel/2010-06/msg00773.html

for details.  In a nutshell, the problem I fixed was that Emacs didn't
look for point far enough beyond the end of the window.

Re: The unwarranted scrolling assumption

[Lennart Borgman]

On Sat, Jun 19, 2010 at 4:10 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Lennart Borgman <lennart.borgman@gmail.com>
>> Date: Sat, 19 Jun 2010 15:36:16 +0200
>> Cc: monnier@iro.umontreal.ca, emacs-devel@gnu.org
>>
>> >> (Note: wrap-to-fill-column-mode justs adds add wrap-prefix here. Which
>> >> is for some reason does not remove. Bug ;-( --
>> >> wrap-to-fill-column-mode is of course part of nXhtml, but probably you
>> >> can just add a fill prefix on the whole line that is equal to what the
>> >> line indentation have.)
>> >
>> > wrap-to-fill-column-mode is complicated.  If you still see the problem
>> > with the current trunk, please show a recipe that does not use
>> > wrap-to-fill-column-mode, but instead sets the fill prefix and/or
>> > whatever else is needed to reproduce this problem in "emacs -Q".
>>
>> I told that recipe when I mentioned wrap-to-fill-column-mode.
>
> No, you told just part of it:
>
>> >> wrap-to-fill-column-mode is of course part of nXhtml, but probably you
>> >> can just add a fill prefix on the whole line that is equal to what the
>> >> line indentation have.)
>
> Can you please make this more explicit?  Please show what I need to
> type in Emacs to accomplish what you say here.
>
>> Unfortunately I can not test now for these reasons:
>
> If you tell me how to define the fill prefix, I can test myself.


Just take the string that is the indentation of the current line and
put it as text property 'wrap-prefix. I have put that string in
ind-str-fill and do just this:

                (put-text-property beg-pos end-pos 'wrap-prefix ind-str-fill)


>> I found it a bit disturbing if you just implemented another way. I
>> can't say if it is right or wrong until I have seen your patch but
>> from our discussion it looks to me like you at least before
>> misunderstood the problem.
>
> What I implemented came out of the sub-thread of this thread where I
> explained to Stefan why I think it sometimes recenters, and Stefan
> agreed that fixing that is a good idea.  See
>
>    http://lists.gnu.org/archive/html/emacs-devel/2010-06/msg00766.html
>    http://lists.gnu.org/archive/html/emacs-devel/2010-06/msg00773.html
>
> for details.  In a nutshell, the problem I fixed was that Emacs didn't
> look for point far enough beyond the end of the window.


OK, I seem to have misunderstood. It looks like that is for another
part of the problem.

Re: The unwarranted scrolling assumption

[Eli Zaretskii]

> From: Lennart Borgman <lennart.borgman@gmail.com>
> Date: Sat, 19 Jun 2010 16:20:16 +0200
> Cc: monnier@iro.umontreal.ca, emacs-devel@gnu.org
> 
> Just take the string that is the indentation of the current line and
> put it as text property 'wrap-prefix. I have put that string in
> ind-str-fill and do just this:
> 
>                 (put-text-property beg-pos end-pos 'wrap-prefix ind-str-fill)

And the value of ind-str-fill is the whitespace preceding the first
non-white character of the line?  That is, a string containing blanks
and TABs?

Re: The unwarranted scrolling assumption

[Lennart Borgman]

Yes, exactly.

On Sat, Jun 19, 2010 at 6:07 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Lennart Borgman <lennart.borgman@gmail.com>
>> Date: Sat, 19 Jun 2010 16:20:16 +0200
>> Cc: monnier@iro.umontreal.ca, emacs-devel@gnu.org
>>
>> Just take the string that is the indentation of the current line and
>> put it as text property 'wrap-prefix. I have put that string in
>> ind-str-fill and do just this:
>>
>>                 (put-text-property beg-pos end-pos 'wrap-prefix ind-str-fill)
>
> And the value of ind-str-fill is the whitespace preceding the first
> non-white character of the line?  That is, a string containing blanks
> and TABs?
>
>

Re: The unwarranted scrolling assumption

[Eli Zaretskii]

> From: Lennart Borgman <lennart.borgman@gmail.com>
> Date: Sat, 19 Jun 2010 19:21:10 +0200
> Cc: monnier@iro.umontreal.ca, emacs-devel@gnu.org
> 
> Yes, exactly.

Thanks.  I cannot reproduce the problem, with the current trunk: I get
a 1-line scroll, not recentering.

Re: The unwarranted scrolling assumption

[Lennart Borgman]

On Sat, Jun 19, 2010 at 8:02 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Lennart Borgman <lennart.borgman@gmail.com>
>> Date: Sat, 19 Jun 2010 19:21:10 +0200
>> Cc: monnier@iro.umontreal.ca, emacs-devel@gnu.org
>>
>> Yes, exactly.
>
> Thanks.  I cannot reproduce the problem, with the current trunk: I get
> a 1-line scroll, not recentering.


So new the visual-line-mode problem is fixed. Very good indeed.

But the original problem with "jumping scrolling" is of course still there.

Re: The unwarranted scrolling assumption

[Eli Zaretskii]

> From: Lennart Borgman <lennart.borgman@gmail.com>
> Date: Sat, 19 Jun 2010 20:07:20 +0200
> Cc: monnier@iro.umontreal.ca, emacs-devel@gnu.org
> 
> But the original problem with "jumping scrolling" is of course still there.

No, it is gone as well.

Re: The unwarranted scrolling assumption

[Lennart Borgman]

On Sat, Jun 19, 2010 at 8:31 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Lennart Borgman <lennart.borgman@gmail.com>
>> Date: Sat, 19 Jun 2010 20:07:20 +0200
>> Cc: monnier@iro.umontreal.ca, emacs-devel@gnu.org
>>
>> But the original problem with "jumping scrolling" is of course still there.
>
> No, it is gone as well.

How could it be?

Re: The unwarranted scrolling assumption

[Eli Zaretskii]

> From: Lennart Borgman <lennart.borgman@gmail.com>
> Date: Fri, 18 Jun 2010 00:16:33 +0200
> Cc: emacs-devel@gnu.org
> 
> > Again, PLEASE answer my questions, if you really want my help in
> > understanding this issue.  What parts of the above condition prevented
> > reconsider_clip_changes from resetting b->clip_changed to zero?  If
> > needed, please re-run under GDB (without your patches) and see what
> > other factors are at work here.
> 
> I can now answer you question again since I have found a new way to
> make "jumping scrolling" appear. However the situation above is not
> involved, at least not directly, since ->clip_changed is 0. (I think
> there is an unused global variable clip_changed that is not used.)

So what you describe is a different situation altogether, and doesn't
help to understand your original report and the patch that is supposed
to fix it.

Emacs uses the recenter as the last resort, and it does this in many
different situations.  Discovering another situation where it happens
does not help to understand the one you discovered before, and the
patch to correct it (assuming we want that) might be entirely
different.  This just adds confusion.

> The situation is now this:
> - I am using my patch (in the state I sent it here).
> - I have seen no problem with the patch (but as I said I think there
> is one problem though it has not shown up).
> - It is quite hard to make Emacs do the "jumping scrolling".
> 
> So it is better, still there are problems.

It may be "better" in the use-cases you tried.  But without
understanding the original problem and why your patch fixes it, it is
quite likely that the patch will introduce other redisplay problems.
So I don't think we should accept this patch without understanding
what problem causes the recentering in the original situation you
reported.

> - Then if I just hold down the down arrow it can happen after a while.
> - My output shows that it is because try_scrolling failed.

try_scrolling may legitimately fail in some cases.

> Some variables as I see them now at the recenter: label in redisplay_window:
>   scroll_step == 1
>   temp_scroll_step == 0
>   current_buffer->clip_changed ==0   Why current_buffer, there is a
> variable buffer here?
>   current_buffer->scroll_up_aggressively == 45459482, same down
> 
> BTW, the use of current_buffer here seems to me to be a bug.

No, it isn't a bug.  When redisplay works on a window, current_buffer
is bound to the buffer displayed by the window.  See the call to
set_buffer_internal_1 near the beginning of redisplay_window.

> I guess an interesting part may be why clear_glyph_matrix failed. Do
> you think there is something interesting there?

I just see normal operation of the display engine.  The condition

  w->cursor.vpos < 0

means that try_window failed to find the cursor position after
redisplaying the window, because point is not inside the window.

IOW, try_scrolling tried to redisplay the window by reusing some of
the results of the previous redisplay, but found that doing so would
produce a window that does not include point.  So it gives up.  This
is normal.

Re: The unwarranted scrolling assumption

[Stefan Monnier]

>> - Then if I just hold down the down arrow it can happen after a while.
>> - My output shows that it is because try_scrolling failed.
> try_scrolling may legitimately fail in some cases.

Maybe that's the problem.  What are those legitimate cases and what
could we do about them.


        Stefan

Re: The unwarranted scrolling assumption

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Lennart Borgman <lennart.borgman@gmail.com>,  emacs-devel@gnu.org
> Date: Fri, 18 Jun 2010 09:30:29 -0400
> 
> >> - Then if I just hold down the down arrow it can happen after a while.
> >> - My output shows that it is because try_scrolling failed.
> > try_scrolling may legitimately fail in some cases.
> 
> Maybe that's the problem.  What are those legitimate cases and what
> could we do about them.

Here are the reasons I see:

 . When the distance from the scroll margin to point is larger than
   the maximum scroll amount we allow in try_scrolling.  The maximum
   scroll amount depends on various options; see the code around line
   13400 in xdisp.c.

 . When we fail to compute the amount to scroll (see computation of
   amount_to_scroll in try_scrolling; I think this happens only if
   scroll-*-aggressively is nil, but maybe I'm mistaken).

 . When try_window, called by try_scrolling, fails to find a proper
   place for cursor, after scrolling the window.  This means that the
   computed amount of scroll was incorrect: it didn't bring point into
   the view.  Not sure when this could happen.

Re: The unwarranted scrolling assumption

[Lennart Borgman]

On Fri, Jun 18, 2010 at 4:09 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>
>  . When try_window, called by try_scrolling, fails to find a proper
>   place for cursor, after scrolling the window.  This means that the
>   computed amount of scroll was incorrect: it didn't bring point into
>   the view.  Not sure when this could happen.

The bug in visual-line-mode in the example I gave (around line 702 in
window.c) is an example where try_scrolling gives up for wrong
reasons.

Maybe it would be good to at least output some debug messages when
try_scrolling gives up?

Re: The unwarranted scrolling assumption

[Stefan Monnier]

> Here are the reasons I see:

>  . When the distance from the scroll margin to point is larger than
>    the maximum scroll amount we allow in try_scrolling.  The maximum
>    scroll amount depends on various options; see the code around line
>    13400 in xdisp.c.

If scroll-conservatively is very large, then this should never happen.

>  . When try_window, called by try_scrolling, fails to find a proper
>    place for cursor, after scrolling the window.  This means that the
>    computed amount of scroll was incorrect: it didn't bring point into
>    the view.  Not sure when this could happen.

This case sounds like the culprit.


        Stefan

Re: The unwarranted scrolling assumption

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: lennart.borgman@gmail.com,  emacs-devel@gnu.org
> Date: Fri, 18 Jun 2010 11:11:27 -0400
> 
> >  . When try_window, called by try_scrolling, fails to find a proper
> >    place for cursor, after scrolling the window.  This means that the
> >    computed amount of scroll was incorrect: it didn't bring point into
> >    the view.  Not sure when this could happen.
> 
> This case sounds like the culprit.

I don't think so; this is just a generic "fire escape".

I think the place to look is here:

	  /* Compute the distance from the scroll margin to PT
	     (including the height of the cursor line).  Moving the
	     iterator unconditionally to PT can be slow if PT is far
	     away, so stop 10 lines past the window bottom (is there a
	     way to do the right thing quickly?).  */
	  move_it_to (&it, PT, -1,
	  	      it.last_visible_y + 10 * FRAME_LINE_HEIGHT (f),
	  	      -1, MOVE_TO_POS | MOVE_TO_Y);
	  dy = line_bottom_y (&it) - y0;

According to my reading, it stops the search for point if it is more
than 10 lines below (resp above) the scroll margin.  The reason is
speed, as the comment says.  Can we safely assume that whoever sets
scroll-conservatively to the maximum value gives up this speed
consideration?  Or maybe we should have another option to replace the
hard-coded value of 10?

Re: The unwarranted scrolling assumption

[Stefan Monnier]

> According to my reading, it stops the search for point if it is more
> than 10 lines below (resp above) the scroll margin.  The reason is
> speed, as the comment says.  Can we safely assume that whoever sets
> scroll-conservatively to the maximum value gives up this speed
> consideration?

I think so, yes.


        Stefan

Re: The unwarranted scrolling assumption

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: emacs-devel@gnu.org
> Date: Fri, 18 Jun 2010 14:30:11 -0400
> 
> > According to my reading, it stops the search for point if it is more
> > than 10 lines below (resp above) the scroll margin.  The reason is
> > speed, as the comment says.  Can we safely assume that whoever sets
> > scroll-conservatively to the maximum value gives up this speed
> > consideration?
> 
> I think so, yes.

OK.  I installed a change along these lines (revno 100619).  It
actually goes a tad farther, in that it computes the limit for
searching for point from the values of scroll-conservatively and
scroll-step, even when scroll-conservatively is less than
most-positive-fixnum, instead of using an arbitrary limit of 10 screen
lines.

Would people who customize these variables (and those who use
scroll-*-aggressively) please see if the new behavior is to their
liking?

Btw, the limit of 10 screen lines comes from this change:

  2008-10-27  Chong Yidong  <cyd@stupidchicken.com>

	  * xdisp.c (try_scrolling): When computing the distance from the
	  scroll margin to PT, try moving some distance past the window
	  bottom before giving up.

Interestingly, this change introduced an asymmetry between scrolling
down and up: there's no such limitation in the latter case.

The archives of emacs-devel do not reveal any details beyond the fact
that this was done to prevent some jumpy scrolling.  Perhaps Yidong
can shed more light on why this was done back then.  (The reason I
care is that I'd like to make sure my change now doesn't break
anything, since the emacs-devel archives don't show any test cases.)

Re: The unwarranted scrolling assumption

[Eli Zaretskii]

> Date: Sat, 19 Jun 2010 12:51:20 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: Chong Yidong <cyd@stupidchicken.com>, emacs-devel@gnu.org
> 
> > From: Stefan Monnier <monnier@iro.umontreal.ca>
> > Cc: emacs-devel@gnu.org
> > Date: Fri, 18 Jun 2010 14:30:11 -0400
> > 
> > > According to my reading, it stops the search for point if it is more
> > > than 10 lines below (resp above) the scroll margin.  The reason is
> > > speed, as the comment says.  Can we safely assume that whoever sets
> > > scroll-conservatively to the maximum value gives up this speed
> > > consideration?
> > 
> > I think so, yes.
> 
> OK.  I installed a change along these lines (revno 100619).  It
> actually goes a tad farther, in that it computes the limit for
> searching for point from the values of scroll-conservatively and
> scroll-step, even when scroll-conservatively is less than
> most-positive-fixnum, instead of using an arbitrary limit of 10 screen
> lines.

With this change, there are still situations where Emacs occasionally
recenters.  Disabling font-lock-mode does not help.

I've found a file with which I can reproduce this reliably by just
leaning on the <down> arrow.  I'm working on trying to find out why
this happens.

Re: The unwarranted scrolling assumption

[Eli Zaretskii]

> Date: Sat, 19 Jun 2010 14:36:17 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: emacs-devel@gnu.org
> 
> > OK.  I installed a change along these lines (revno 100619).  It
> > actually goes a tad farther, in that it computes the limit for
> > searching for point from the values of scroll-conservatively and
> > scroll-step, even when scroll-conservatively is less than
> > most-positive-fixnum, instead of using an arbitrary limit of 10 screen
> > lines.
> 
> With this change, there are still situations where Emacs occasionally
> recenters.  Disabling font-lock-mode does not help.
> 
> I've found a file with which I can reproduce this reliably by just
> leaning on the <down> arrow.  I'm working on trying to find out why
> this happens.

Update: with revno 100620, I can no longer get Emacs to recenter by
pressing and holding the <down> arrow key, unless bidi-display-reordering
is non-nil.  The latter is probably due to some bug in cursor positioning
in set_cursor_from_row and display_line, under bidirectional display; I
will look into that next.

So please try the latest trunk and see if Emacs still recenters sometimes.

Re: The unwarranted scrolling assumption

[Lennart Borgman]

On Sat, Jun 19, 2010 at 2:54 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>
> Update: with revno 100620, I can no longer get Emacs to recenter by
> pressing and holding the <down> arrow key, unless bidi-display-reordering
> is non-nil.  The latter is probably due to some bug in cursor positioning
> in set_cursor_from_row and display_line, under bidirectional display; I
> will look into that next.
>
> So please try the latest trunk and see if Emacs still recenters sometimes.

Can you please instead show your patch here?

Re: The unwarranted scrolling assumption

[Eli Zaretskii]

> From: Lennart Borgman <lennart.borgman@gmail.com>
> Date: Sat, 19 Jun 2010 15:00:37 +0200
> Cc: monnier@iro.umontreal.ca, cyd@stupidchicken.com, emacs-devel@gnu.org
> 
> On Sat, Jun 19, 2010 at 2:54 PM, Eli Zaretskii <eliz@gnu.org> wrote:
> >
> > Update: with revno 100620, I can no longer get Emacs to recenter by
> > pressing and holding the <down> arrow key, unless bidi-display-reordering
> > is non-nil.  The latter is probably due to some bug in cursor positioning
> > in set_cursor_from_row and display_line, under bidirectional display; I
> > will look into that next.
> >
> > So please try the latest trunk and see if Emacs still recenters sometimes.
> 
> Can you please instead show your patch here?

Here:

=== modified file 'src/ChangeLog'
--- src/ChangeLog	2010-06-16 20:08:41 +0000
+++ src/ChangeLog	2010-06-19 09:39:37 +0000
@@ -1,3 +1,13 @@
+2010-06-19  Eli Zaretskii  <eliz@gnu.org>
+
+	* xdisp.c (try_scrolling): Compute the limit for searching point
+	in forward scroll from scroll_max, instead of an arbitrary limit
+	of 10 screen lines.  See
+	http://lists.gnu.org/archive/html/emacs-devel/2010-06/msg00766.html
+	and
+	http://lists.gnu.org/archive/html/emacs-devel/2010-06/msg00773.html
+	for details.
+
 2010-06-16  Glenn Morris  <rgm@gnu.org>
 
 	* editfns.c (Fbyte_to_string): Pacify compiler.

=== modified file 'src/xdisp.c'
--- src/xdisp.c	2010-06-01 02:34:49 +0000
+++ src/xdisp.c	2010-06-19 11:55:00 +0000
@@ -13431,14 +13431,22 @@ try_scrolling (window, just_this_one_p, 
       if (PT > CHARPOS (it.current.pos))
 	{
 	  int y0 = line_bottom_y (&it);
-
-	  /* Compute the distance from the scroll margin to PT
-	     (including the height of the cursor line).  Moving the
-	     iterator unconditionally to PT can be slow if PT is far
-	     away, so stop 10 lines past the window bottom (is there a
-	     way to do the right thing quickly?).  */
-	  move_it_to (&it, PT, -1,
-	  	      it.last_visible_y + 10 * FRAME_LINE_HEIGHT (f),
+	  /* 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 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 =
+	    slack >= INT_MAX - it.last_visible_y
+	    ? INT_MAX
+	    : it.last_visible_y + slack;
+
+	  /* Compute the distance from the scroll margin to PT or to
+	     the scroll limit, whichever comes first.  This should
+	     include the height of the cursor line, to make that line
+	     fully visible.  */
+	  move_it_to (&it, PT, -1, y_to_move,
 	  	      -1, MOVE_TO_POS | MOVE_TO_Y);
 	  dy = line_bottom_y (&it) - y0;
 

Re: The unwarranted scrolling assumption

[Lennart Borgman]

On Sat, Jun 19, 2010 at 4:03 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Lennart Borgman <lennart.borgman@gmail.com>
>> Date: Sat, 19 Jun 2010 15:00:37 +0200
>> Cc: monnier@iro.umontreal.ca, cyd@stupidchicken.com, emacs-devel@gnu.org
>>
>> On Sat, Jun 19, 2010 at 2:54 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> >
>> > Update: with revno 100620, I can no longer get Emacs to recenter by
>> > pressing and holding the <down> arrow key, unless bidi-display-reordering
>> > is non-nil.  The latter is probably due to some bug in cursor positioning
>> > in set_cursor_from_row and display_line, under bidirectional display; I
>> > will look into that next.
>> >
>> > So please try the latest trunk and see if Emacs still recenters sometimes.
>>
>> Can you please instead show your patch here?
>
> Here:
>
> === modified file 'src/ChangeLog'
> --- src/ChangeLog       2010-06-16 20:08:41 +0000
> +++ src/ChangeLog       2010-06-19 09:39:37 +0000
> @@ -1,3 +1,13 @@
> +2010-06-19  Eli Zaretskii  <eliz@gnu.org>
> +
> +       * xdisp.c (try_scrolling): Compute the limit for searching point
> +       in forward scroll from scroll_max, instead of an arbitrary limit
> +       of 10 screen lines.  See


Thanks.

I have no objections or comments to this part. I guess this is good,
but it is not about the problem with the handling of clip_changed and
I have not looked into it.

If needed I can do that later.

Re: The unwarranted scrolling assumption

[Eli Zaretskii]

> From: Lennart Borgman <lennart.borgman@gmail.com>
> Date: Sat, 19 Jun 2010 16:11:16 +0200
> Cc: monnier@iro.umontreal.ca, cyd@stupidchicken.com, emacs-devel@gnu.org
> 
> > +2010-06-19  Eli Zaretskii  <eliz@gnu.org>
> > +
> > +       * xdisp.c (try_scrolling): Compute the limit for searching point
> > +       in forward scroll from scroll_max, instead of an arbitrary limit
> > +       of 10 screen lines.  See
> 
> 
> Thanks.
> 
> I have no objections or comments to this part. I guess this is good,
> but it is not about the problem with the handling of clip_changed and
> I have not looked into it.
> 
> If needed I can do that later.

Please try this patch after reverting yours, and see if the problem
you were trying to solve is gone.

Re: The unwarranted scrolling assumption

[Lennart Borgman]

On Sat, Jun 19, 2010 at 4:16 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Lennart Borgman <lennart.borgman@gmail.com>
>> Date: Sat, 19 Jun 2010 16:11:16 +0200
>> Cc: monnier@iro.umontreal.ca, cyd@stupidchicken.com, emacs-devel@gnu.org
>>
>> > +2010-06-19  Eli Zaretskii  <eliz@gnu.org>
>> > +
>> > +       * xdisp.c (try_scrolling): Compute the limit for searching point
>> > +       in forward scroll from scroll_max, instead of an arbitrary limit
>> > +       of 10 screen lines.  See
>>
>>
>> Thanks.
>>
>> I have no objections or comments to this part. I guess this is good,
>> but it is not about the problem with the handling of clip_changed and
>> I have not looked into it.
>>
>> If needed I can do that later.
>
> Please try this patch after reverting yours, and see if the problem
> you were trying to solve is gone.


I am sure it does not.

Re: The unwarranted scrolling assumption

[Eli Zaretskii]

> From: Lennart Borgman <lennart.borgman@gmail.com>
> Date: Sat, 19 Jun 2010 16:20:58 +0200
> Cc: cyd@stupidchicken.com, monnier@iro.umontreal.ca, emacs-devel@gnu.org
> 
> > Please try this patch after reverting yours, and see if the problem
> > you were trying to solve is gone.
> 
> 
> I am sure it does not.

Could you please humor me by trying?

Re: The unwarranted scrolling assumption

[Lennart Borgman]

On Sat, Jun 19, 2010 at 6:05 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Lennart Borgman <lennart.borgman@gmail.com>
>> Date: Sat, 19 Jun 2010 16:20:58 +0200
>> Cc: cyd@stupidchicken.com, monnier@iro.umontreal.ca, emacs-devel@gnu.org
>>
>> > Please try this patch after reverting yours, and see if the problem
>> > you were trying to solve is gone.
>>
>>
>> I am sure it does not.
>
> Could you please humor me by trying?


I think misread this as if you tried to solve the original problem
with this patch. Did you try to solve the original problem or another
problem? What exactly what you want me to test?

Re: The unwarranted scrolling assumption

[Eli Zaretskii]

> From: Lennart Borgman <lennart.borgman@gmail.com>
> Date: Sat, 19 Jun 2010 19:22:43 +0200
> Cc: cyd@stupidchicken.com, monnier@iro.umontreal.ca, emacs-devel@gnu.org
> 
> On Sat, Jun 19, 2010 at 6:05 PM, Eli Zaretskii <eliz@gnu.org> wrote:
> >> From: Lennart Borgman <lennart.borgman@gmail.com>
> >> Date: Sat, 19 Jun 2010 16:20:58 +0200
> >> Cc: cyd@stupidchicken.com, monnier@iro.umontreal.ca, emacs-devel@gnu.org
> >>
> >> > Please try this patch after reverting yours, and see if the problem
> >> > you were trying to solve is gone.
> >>
> >>
> >> I am sure it does not.
> >
> > Could you please humor me by trying?
> 
> 
> I think misread this as if you tried to solve the original problem
> with this patch. Did you try to solve the original problem or another
> problem?

The problem I tried to solve was the one you reported at the beginning
of this thread: evaluate this:

    (setq scroll-preserve-screen-position 'always
	  scroll-conservatively           most-positive-fixnum
	  scroll-step                     0)

and then visit window.c and press and hold the <down> arrow key.  You
said that sometimes you see display being recentered, i.e. you see the
cursor near the middle of the window, instead of it being at the last
screen line.

David Engster reported the same symptoms with the above settings.
(David also used redisplay-dont-pause set to t, but I don't see
recentering with that customization, either, when I use the current
trunk.)

> What exactly what you want me to test?

I would like you to try the above recipe in "emacs -Q", and tell me if
you still see recentering.

TIA

Re: The unwarranted scrolling assumption

[Lennart Borgman]

On Sat, Jun 19, 2010 at 7:54 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Lennart Borgman <lennart.borgman@gmail.com>
>> Date: Sat, 19 Jun 2010 19:22:43 +0200
>> Cc: cyd@stupidchicken.com, monnier@iro.umontreal.ca, emacs-devel@gnu.org
>>
>> On Sat, Jun 19, 2010 at 6:05 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> >> From: Lennart Borgman <lennart.borgman@gmail.com>
>> >> Date: Sat, 19 Jun 2010 16:20:58 +0200
>> >> Cc: cyd@stupidchicken.com, monnier@iro.umontreal.ca, emacs-devel@gnu.org
>> >>
>> >> > Please try this patch after reverting yours, and see if the problem
>> >> > you were trying to solve is gone.
>> >>
>> >>
>> >> I am sure it does not.
>> >
>> > Could you please humor me by trying?
>>
>>
>> I think misread this as if you tried to solve the original problem
>> with this patch. Did you try to solve the original problem or another
>> problem?
>
> The problem I tried to solve was the one you reported at the beginning
> of this thread: evaluate this:
>
>    (setq scroll-preserve-screen-position 'always
>          scroll-conservatively           most-positive-fixnum
>          scroll-step                     0)
>
> and then visit window.c and press and hold the <down> arrow key.  You
> said that sometimes you see display being recentered, i.e. you see the
> cursor near the middle of the window, instead of it being at the last
> screen line.


Then I am afraid the patch is plain wrong.

Could you please try my patch instead?

Re: The unwarranted scrolling assumption

[Eli Zaretskii]

> From: Lennart Borgman <lennart.borgman@gmail.com>
> Date: Sat, 19 Jun 2010 19:58:39 +0200
> Cc: cyd@stupidchicken.com, monnier@iro.umontreal.ca, emacs-devel@gnu.org
> 
> > The problem I tried to solve was the one you reported at the beginning
> > of this thread: evaluate this:
> >
> >    (setq scroll-preserve-screen-position 'always
> >          scroll-conservatively           most-positive-fixnum
> >          scroll-step                     0)
> >
> > and then visit window.c and press and hold the <down> arrow key.  You
> > said that sometimes you see display being recentered, i.e. you see the
> > cursor near the middle of the window, instead of it being at the last
> > screen line.
> 
> 
> Then I am afraid the patch is plain wrong.

Did you try it?

Re: The unwarranted scrolling assumption

[Lennart Borgman]

On Sat, Jun 19, 2010 at 8:30 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Lennart Borgman <lennart.borgman@gmail.com>
>> Date: Sat, 19 Jun 2010 19:58:39 +0200
>> Cc: cyd@stupidchicken.com, monnier@iro.umontreal.ca, emacs-devel@gnu.org
>>
>> > The problem I tried to solve was the one you reported at the beginning
>> > of this thread: evaluate this:
>> >
>> >    (setq scroll-preserve-screen-position 'always
>> >          scroll-conservatively           most-positive-fixnum
>> >          scroll-step                     0)
>> >
>> > and then visit window.c and press and hold the <down> arrow key.  You
>> > said that sometimes you see display being recentered, i.e. you see the
>> > cursor near the middle of the window, instead of it being at the last
>> > screen line.
>>
>>
>> Then I am afraid the patch is plain wrong.
>
> Did you try it?

Why do you ask? It does not at all address the original problem.

Re: The unwarranted scrolling assumption

[Eli Zaretskii]

> From: Lennart Borgman <lennart.borgman@gmail.com>
> Date: Sat, 19 Jun 2010 21:10:28 +0200
> Cc: cyd@stupidchicken.com, monnier@iro.umontreal.ca, emacs-devel@gnu.org
> 
> On Sat, Jun 19, 2010 at 8:30 PM, Eli Zaretskii <eliz@gnu.org> wrote:
> >> From: Lennart Borgman <lennart.borgman@gmail.com>
> >> Date: Sat, 19 Jun 2010 19:58:39 +0200
> >> Cc: cyd@stupidchicken.com, monnier@iro.umontreal.ca, emacs-devel@gnu.org
> >>
> >> > The problem I tried to solve was the one you reported at the beginning
> >> > of this thread: evaluate this:
> >> >
> >> >    (setq scroll-preserve-screen-position 'always
> >> >          scroll-conservatively           most-positive-fixnum
> >> >          scroll-step                     0)
> >> >
> >> > and then visit window.c and press and hold the <down> arrow key.  You
> >> > said that sometimes you see display being recentered, i.e. you see the
> >> > cursor near the middle of the window, instead of it being at the last
> >> > screen line.
> >>
> >>
> >> Then I am afraid the patch is plain wrong.
> >
> > Did you try it?
> 
> Why do you ask?

Because only by trying and reporting the results you can prove that
the changes I installed do not fix the problem.

> It does not at all address the original problem.

I think it does, except that you interpret the ``original problem''
incorrectly.

Re: The unwarranted scrolling assumption

[Lennart Borgman]

On Sat, Jun 19, 2010 at 11:18 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Lennart Borgman <lennart.borgman@gmail.com>
>> Date: Sat, 19 Jun 2010 21:10:28 +0200
>> Cc: cyd@stupidchicken.com, monnier@iro.umontreal.ca, emacs-devel@gnu.org
>>
>> On Sat, Jun 19, 2010 at 8:30 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> >> From: Lennart Borgman <lennart.borgman@gmail.com>
>> >> Date: Sat, 19 Jun 2010 19:58:39 +0200
>> >> Cc: cyd@stupidchicken.com, monnier@iro.umontreal.ca, emacs-devel@gnu.org
>> >>
>> >> > The problem I tried to solve was the one you reported at the beginning
>> >> > of this thread: evaluate this:
>> >> >
>> >> >    (setq scroll-preserve-screen-position 'always
>> >> >          scroll-conservatively           most-positive-fixnum
>> >> >          scroll-step                     0)
>> >> >
>> >> > and then visit window.c and press and hold the <down> arrow key.  You
>> >> > said that sometimes you see display being recentered, i.e. you see the
>> >> > cursor near the middle of the window, instead of it being at the last
>> >> > screen line.
>> >>
>> >>
>> >> Then I am afraid the patch is plain wrong.
>> >
>> > Did you try it?
>>
>> Why do you ask?
>
> Because only by trying and reporting the results you can prove that
> the changes I installed do not fix the problem.

For me this is not about checking if the patch works but more about
convincing you that it does not.

Or, I would be very surprised if it did ;-)

Anyway the easiest way now seems that I take this trouble. I tried to
apply your patch as sent, but I could not get it to work. Maybe you
can just send me that function so I can replace that?

>> It does not at all address the original problem.
>
> I think it does, except that you interpret the ``original problem''
> incorrectly.

Yes, let us see.

Re: The unwarranted scrolling assumption

[Eli Zaretskii]

> From: Lennart Borgman <lennart.borgman@gmail.com>
> Date: Sat, 19 Jun 2010 23:58:09 +0200
> Cc: cyd@stupidchicken.com, monnier@iro.umontreal.ca, emacs-devel@gnu.org
> 
> Anyway the easiest way now seems that I take this trouble. I tried to
> apply your patch as sent, but I could not get it to work. Maybe you
> can just send me that function so I can replace that?

It's a bit silly to do that, when we have a dVCS, but here you go
anyway:

------------------------------------------------------------------
static int
try_scrolling (window, just_this_one_p, scroll_conservatively,
	       scroll_step, temp_scroll_step, last_line_misfit)
     Lisp_Object window;
     int just_this_one_p;
     EMACS_INT scroll_conservatively, scroll_step;
     int temp_scroll_step;
     int last_line_misfit;
{
  struct window *w = XWINDOW (window);
  struct frame *f = XFRAME (w->frame);
  struct text_pos pos, startp;
  struct it it;
  int this_scroll_margin, scroll_max, rc, height;
  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);

#if GLYPH_DEBUG
  debug_method_add (w, "try_scrolling");
#endif

  SET_TEXT_POS_FROM_MARKER (startp, w->start);

  /* Compute scroll margin height in pixels.  We scroll when point is
     within this distance from the top or bottom of the window.  */
  if (scroll_margin > 0)
    this_scroll_margin = min (scroll_margin, WINDOW_TOTAL_LINES (w) / 4)
      * FRAME_LINE_HEIGHT (f);
  else
    this_scroll_margin = 0;

  /* Force 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.  */
  if (scroll_conservatively > scroll_limit)
    {
      scroll_conservatively = scroll_limit;
      scroll_max = INT_MAX;
    }
  else if (scroll_step || scroll_conservatively || temp_scroll_step)
    /* Compute how much we should try to scroll maximally to bring
       point into view.  */
    scroll_max = (max (scroll_step,
		       max (scroll_conservatively, temp_scroll_step))
		  * FRAME_LINE_HEIGHT (f));
  else if (NUMBERP (current_buffer->scroll_down_aggressively)
	   || NUMBERP (current_buffer->scroll_up_aggressively))
    /* We're trying to scroll because of aggressive scrolling but no
       scroll_step is set.  Choose an arbitrary one.  */
    scroll_max = 10 * FRAME_LINE_HEIGHT (f);
  else
    scroll_max = 0;

 too_near_end:

  /* Decide whether to scroll down.  */
  if (PT > CHARPOS (startp))
    {
      int scroll_margin_y;

      /* Compute the pixel ypos of the scroll margin, then move it to
	 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 (f) * extra_scroll_margin_lines;
      move_it_to (&it, PT, -1, scroll_margin_y - 1, -1,
		  (MOVE_TO_POS | MOVE_TO_Y));

      if (PT > CHARPOS (it.current.pos))
	{
	  int y0 = line_bottom_y (&it);
	  /* 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 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 =
	    slack >= INT_MAX - it.last_visible_y
	    ? INT_MAX
	    : it.last_visible_y + slack;

	  /* Compute the distance from the scroll margin to PT or to
	     the scroll limit, whichever comes first.  This should
	     include the height of the cursor line, to make that line
	     fully visible.  */
	  move_it_to (&it, PT, -1, y_to_move,
	  	      -1, MOVE_TO_POS | MOVE_TO_Y);
	  dy = line_bottom_y (&it) - y0;

	  if (dy > scroll_max)
	    return SCROLLING_FAILED;

	  scroll_down_p = 1;
	}
    }

  if (scroll_down_p)
    {
      /* Point is in or below the bottom scroll margin, so move the
	 window start down.  If scrolling conservatively, move it just
	 enough down to make point visible.  If scroll_step is set,
	 move it down by scroll_step.  */
      if (scroll_conservatively)
	amount_to_scroll
	  = min (max (dy, FRAME_LINE_HEIGHT (f)),
		 FRAME_LINE_HEIGHT (f) * scroll_conservatively);
      else if (scroll_step || temp_scroll_step)
	amount_to_scroll = scroll_max;
      else
	{
	  aggressive = current_buffer->scroll_up_aggressively;
	  height = WINDOW_BOX_TEXT_HEIGHT (w);
	  if (NUMBERP (aggressive))
	    {
	      double float_amount = XFLOATINT (aggressive) * height;
	      amount_to_scroll = float_amount;
	      if (amount_to_scroll == 0 && float_amount > 0)
		amount_to_scroll = 1;
	    }
	}

      if (amount_to_scroll <= 0)
	return SCROLLING_FAILED;

      start_display (&it, w, startp);
      move_it_vertically (&it, amount_to_scroll);

      /* If STARTP is unchanged, move it down another screen line.  */
      if (CHARPOS (it.current.pos) == CHARPOS (startp))
	move_it_by_lines (&it, 1, 1);
      startp = it.current.pos;
    }
  else
    {
      struct text_pos scroll_margin_pos = startp;

      /* See if point is inside the scroll margin at the top of the
         window.  */
      if (this_scroll_margin)
	{
	  start_display (&it, w, startp);
	  move_it_vertically (&it, this_scroll_margin);
	  scroll_margin_pos = it.current.pos;
	}

      if (PT < CHARPOS (scroll_margin_pos))
	{
	  /* Point is in the scroll margin at the top of the window or
	     above what is displayed in the window.  */
	  int y0;

	  /* Compute the vertical distance from PT to the scroll
	     margin position.  Give up if distance is greater than
	     scroll_max.  */
	  SET_TEXT_POS (pos, PT, PT_BYTE);
	  start_display (&it, w, pos);
	  y0 = it.current_y;
	  move_it_to (&it, CHARPOS (scroll_margin_pos), 0,
		      it.last_visible_y, -1,
		      MOVE_TO_POS | MOVE_TO_X | MOVE_TO_Y);
	  dy = it.current_y - y0;
	  if (dy > scroll_max)
	    return SCROLLING_FAILED;

	  /* Compute new window start.  */
	  start_display (&it, w, startp);

	  if (scroll_conservatively)
	    amount_to_scroll
	      = max (dy, FRAME_LINE_HEIGHT (f) * max (scroll_step, temp_scroll_step));
	  else if (scroll_step || temp_scroll_step)
	    amount_to_scroll = scroll_max;
	  else
	    {
	      aggressive = current_buffer->scroll_down_aggressively;
	      height = WINDOW_BOX_TEXT_HEIGHT (w);
	      if (NUMBERP (aggressive))
		{
		  double float_amount = XFLOATINT (aggressive) * height;
		  amount_to_scroll = float_amount;
		  if (amount_to_scroll == 0 && float_amount > 0)
		    amount_to_scroll = 1;
		}
	    }

	  if (amount_to_scroll <= 0)
	    return SCROLLING_FAILED;

	  move_it_vertically_backward (&it, amount_to_scroll);
	  startp = it.current.pos;
	}
    }

  /* Run window scroll functions.  */
  startp = run_window_scroll_functions (window, startp);

  /* Display the window.  Give up if new fonts are loaded, or if point
     doesn't appear.  */
  if (!try_window (window, startp, 0))
    rc = SCROLLING_NEED_LARGER_MATRICES;
  else if (w->cursor.vpos < 0)
    {
      clear_glyph_matrix (w->desired_matrix);
      rc = SCROLLING_FAILED;
    }
  else
    {
      /* Maybe forget recorded base line for line number display.  */
      if (!just_this_one_p
	  || current_buffer->clip_changed
	  || BEG_UNCHANGED < CHARPOS (startp))
	w->base_line_number = Qnil;

      /* If cursor ends up on a partially visible line,
	 treat that as being off the bottom of the screen.  */
      if (! cursor_row_fully_visible_p (w, extra_scroll_margin_lines <= 1, 0))
	{
	  clear_glyph_matrix (w->desired_matrix);
	  ++extra_scroll_margin_lines;
	  goto too_near_end;
	}
      rc = SCROLLING_SUCCESS;
    }

  return rc;
}

Re: The unwarranted scrolling assumption

[Eli Zaretskii]

> Date: Sun, 20 Jun 2010 18:24:18 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: cyd@stupidchicken.com, monnier@iro.umontreal.ca, emacs-devel@gnu.org
> 
> It's a bit silly to do that, when we have a dVCS, but here you go
> anyway:

Here's an updated version:

----------------------------------------------------------------------
static int
try_scrolling (window, just_this_one_p, scroll_conservatively,
	       scroll_step, temp_scroll_step, last_line_misfit)
     Lisp_Object window;
     int just_this_one_p;
     EMACS_INT scroll_conservatively, scroll_step;
     int temp_scroll_step;
     int last_line_misfit;
{
  struct window *w = XWINDOW (window);
  struct frame *f = XFRAME (w->frame);
  struct text_pos pos, startp;
  struct it it;
  int this_scroll_margin, scroll_max, rc, height;
  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);

#if GLYPH_DEBUG
  debug_method_add (w, "try_scrolling");
#endif

  SET_TEXT_POS_FROM_MARKER (startp, w->start);

  /* Compute scroll margin height in pixels.  We scroll when point is
     within this distance from the top or bottom of the window.  */
  if (scroll_margin > 0)
    this_scroll_margin = min (scroll_margin, WINDOW_TOTAL_LINES (w) / 4)
      * FRAME_LINE_HEIGHT (f);
  else
    this_scroll_margin = 0;

  /* Force 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.  */
  if (scroll_conservatively > scroll_limit)
    {
      scroll_conservatively = scroll_limit;
      scroll_max = INT_MAX;
    }
  else if (scroll_step || scroll_conservatively || temp_scroll_step)
    /* Compute how much we should try to scroll maximally to bring
       point into view.  */
    scroll_max = (max (scroll_step,
		       max (scroll_conservatively, temp_scroll_step))
		  * FRAME_LINE_HEIGHT (f));
  else if (NUMBERP (current_buffer->scroll_down_aggressively)
	   || NUMBERP (current_buffer->scroll_up_aggressively))
    /* We're trying to scroll because of aggressive scrolling but no
       scroll_step is set.  Choose an arbitrary one.  */
    scroll_max = 10 * FRAME_LINE_HEIGHT (f);
  else
    scroll_max = 0;

 too_near_end:

  /* Decide whether to scroll down.  */
  if (PT > CHARPOS (startp))
    {
      int scroll_margin_y;

      /* Compute the pixel ypos of the scroll margin, then move it to
	 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 (f) * extra_scroll_margin_lines;
      move_it_to (&it, PT, -1, scroll_margin_y - 1, -1,
		  (MOVE_TO_POS | MOVE_TO_Y));

      if (PT > CHARPOS (it.current.pos))
	{
	  int y0 = line_bottom_y (&it);
	  /* 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 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 =
	    slack >= INT_MAX - it.last_visible_y
	    ? INT_MAX
	    : it.last_visible_y + slack;

	  /* Compute the distance from the scroll margin to PT or to
	     the scroll limit, whichever comes first.  This should
	     include the height of the cursor line, to make that line
	     fully visible.  */
	  move_it_to (&it, PT, -1, y_to_move,
	  	      -1, MOVE_TO_POS | MOVE_TO_Y);
	  dy = line_bottom_y (&it) - y0;

	  if (dy > scroll_max)
	    return SCROLLING_FAILED;

	  scroll_down_p = 1;
	}
    }

  if (scroll_down_p)
    {
      /* Point is in or below the bottom scroll margin, so move the
	 window start down.  If scrolling conservatively, move it just
	 enough down to make point visible.  If scroll_step is set,
	 move it down by scroll_step.  */
      if (scroll_conservatively)
	amount_to_scroll
	  = min (max (dy, FRAME_LINE_HEIGHT (f)),
		 FRAME_LINE_HEIGHT (f) * scroll_conservatively);
      else if (scroll_step || temp_scroll_step)
	amount_to_scroll = scroll_max;
      else
	{
	  aggressive = current_buffer->scroll_up_aggressively;
	  height = WINDOW_BOX_TEXT_HEIGHT (w);
	  if (NUMBERP (aggressive))
	    {
	      double float_amount = XFLOATINT (aggressive) * height;
	      amount_to_scroll = float_amount;
	      if (amount_to_scroll == 0 && float_amount > 0)
		amount_to_scroll = 1;
	    }
	}

      if (amount_to_scroll <= 0)
	return SCROLLING_FAILED;

      start_display (&it, w, startp);
      if (scroll_max < INT_MAX)
	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
	     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
	     below window bottom have different height.  */
	  struct it it1 = it;
	  /* We use a temporary it1 because line_bottom_y can modify
	     its argument, if it moves one line down; see there.  */
	  int start_y = line_bottom_y (&it1);

	  do {
	    move_it_by_lines (&it, 1, 1);
	    it1 = it;
	  } while (line_bottom_y (&it1) - start_y < amount_to_scroll);
	}

      /* If STARTP is unchanged, move it down another screen line.  */
      if (CHARPOS (it.current.pos) == CHARPOS (startp))
	move_it_by_lines (&it, 1, 1);
      startp = it.current.pos;
    }
  else
    {
      struct text_pos scroll_margin_pos = startp;

      /* See if point is inside the scroll margin at the top of the
         window.  */
      if (this_scroll_margin)
	{
	  start_display (&it, w, startp);
	  move_it_vertically (&it, this_scroll_margin);
	  scroll_margin_pos = it.current.pos;
	}

      if (PT < CHARPOS (scroll_margin_pos))
	{
	  /* Point is in the scroll margin at the top of the window or
	     above what is displayed in the window.  */
	  int y0;

	  /* Compute the vertical distance from PT to the scroll
	     margin position.  Give up if distance is greater than
	     scroll_max.  */
	  SET_TEXT_POS (pos, PT, PT_BYTE);
	  start_display (&it, w, pos);
	  y0 = it.current_y;
	  move_it_to (&it, CHARPOS (scroll_margin_pos), 0,
		      it.last_visible_y, -1,
		      MOVE_TO_POS | MOVE_TO_X | MOVE_TO_Y);
	  dy = it.current_y - y0;
	  if (dy > scroll_max)
	    return SCROLLING_FAILED;

	  /* Compute new window start.  */
	  start_display (&it, w, startp);

	  if (scroll_conservatively)
	    amount_to_scroll
	      = max (dy, FRAME_LINE_HEIGHT (f) * max (scroll_step, temp_scroll_step));
	  else if (scroll_step || temp_scroll_step)
	    amount_to_scroll = scroll_max;
	  else
	    {
	      aggressive = current_buffer->scroll_down_aggressively;
	      height = WINDOW_BOX_TEXT_HEIGHT (w);
	      if (NUMBERP (aggressive))
		{
		  double float_amount = XFLOATINT (aggressive) * height;
		  amount_to_scroll = float_amount;
		  if (amount_to_scroll == 0 && float_amount > 0)
		    amount_to_scroll = 1;
		}
	    }

	  if (amount_to_scroll <= 0)
	    return SCROLLING_FAILED;

	  move_it_vertically_backward (&it, amount_to_scroll);
	  startp = it.current.pos;
	}
    }

  /* Run window scroll functions.  */
  startp = run_window_scroll_functions (window, startp);

  /* Display the window.  Give up if new fonts are loaded, or if point
     doesn't appear.  */
  if (!try_window (window, startp, 0))
    rc = SCROLLING_NEED_LARGER_MATRICES;
  else if (w->cursor.vpos < 0)
    {
      clear_glyph_matrix (w->desired_matrix);
      rc = SCROLLING_FAILED;
    }
  else
    {
      /* Maybe forget recorded base line for line number display.  */
      if (!just_this_one_p
	  || current_buffer->clip_changed
	  || BEG_UNCHANGED < CHARPOS (startp))
	w->base_line_number = Qnil;

      /* If cursor ends up on a partially visible line,
	 treat that as being off the bottom of the screen.  */
      if (! cursor_row_fully_visible_p (w, extra_scroll_margin_lines <= 1, 0))
	{
	  clear_glyph_matrix (w->desired_matrix);
	  ++extra_scroll_margin_lines;
	  goto too_near_end;
	}
      rc = SCROLLING_SUCCESS;
    }

  return rc;
}

Re: The unwarranted scrolling assumption

[Lennart Borgman]

On Sun, Jun 20, 2010 at 8:17 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> Date: Sun, 20 Jun 2010 18:24:18 +0300
>> From: Eli Zaretskii <eliz@gnu.org>
>> Cc: cyd@stupidchicken.com, monnier@iro.umontreal.ca, emacs-devel@gnu.org
>>
>> It's a bit silly to do that, when we have a dVCS, but here you go
>> anyway:

Thanks.

Yes, it is perhaps a bit silly, but I am using Launchpad and I could
not apply the patch for some reason (not the usual CRLF problem
because I took care of that, I do not have time to look into what
happened now). Beside that I do not want to update my sources to make
it easier for me if there are some merge problems in the end.

However please send things like this as attachment. There seem to be a
good chance it gets mangled by mail otherwise. (If you do not hear
from me I have not had time to test yet and would be glad for a
private mail with this as an attachment if it is not too much job for
you.)

Re: The unwarranted scrolling assumption

[Eli Zaretskii]

> From: Lennart Borgman <lennart.borgman@gmail.com>
> Date: Mon, 21 Jun 2010 03:13:07 +0200
> Cc: emacs-devel@gnu.org
> 
> However please send things like this as attachment. There seem to be a
> good chance it gets mangled by mail otherwise.

I'm using Emacs to send mail.  Emacs doesn't mangle anything.

Re: The unwarranted scrolling assumption

[Lennart Borgman]

On Mon, Jun 21, 2010 at 4:55 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Lennart Borgman <lennart.borgman@gmail.com>
>> Date: Mon, 21 Jun 2010 03:13:07 +0200
>> Cc: emacs-devel@gnu.org
>>
>> However please send things like this as attachment. There seem to be a
>> good chance it gets mangled by mail otherwise.
>
> I'm using Emacs to send mail.  Emacs doesn't mangle anything.


OK, if Emacs did not mange anything (or some other mail software in
the route) then the result is this of your latest patch:

- The original "jumping scrolling" is still there (this was expected by me).

- The problems around line 702 in window.c (concerning
visual-line-mode), which I thought you have tried to address are also
still there.


So now we are in this situation:

I have repeatedly asked for any kind of feed back from you about my
patch (which I think is correct). I am not aware that you have sent
any.

Juanma said that I should explain my patch more carefully to you. I
did, but I do not think I have seen any answer to that at all from
anyone.

Stephen has implied that you are afraid that redisplay will be broken
somehow by my patch. You have not said this as far as I know. I think
that might be the case, but I would like you to tell more exactly why
then.


Can you please tell me what you think is the best to do now?
What do others think we can/should do?

Re: The unwarranted scrolling assumption

[Juanma Barranquero]

On Mon, Jun 21, 2010 at 11:31, Lennart Borgman
<lennart.borgman@gmail.com> wrote:

> Juanma said that I should explain my patch more carefully to you. I
> did, but I do not think I have seen any answer to that at all from
> anyone.

Mine was a general comment (and/or recommendation) about the process
of sending patches up for discussion, trying to explain my *personal*
view of why your proposed changes are sometimes overlooked (at least,
from your point of view; I would substitute for "not well
understood").

Eli is not bound by my requests, or anyone else's. So please stop
saying "Juanma asked me to do X, I did and I got no answer from person
P" (for values of P different of "Juanma").

    Juanma

Re: The unwarranted scrolling assumption

[Lennart Borgman]

On Mon, Jun 21, 2010 at 11:59 AM, Juanma Barranquero <lekktu@gmail.com> wrote:
> On Mon, Jun 21, 2010 at 11:31, Lennart Borgman
> <lennart.borgman@gmail.com> wrote:
>
>> Juanma said that I should explain my patch more carefully to you. I
>> did, but I do not think I have seen any answer to that at all from
>> anyone.
>
> Mine was a general comment (and/or recommendation) about the process
> of sending patches up for discussion, trying to explain my *personal*
> view of why your proposed changes are sometimes overlooked (at least,
> from your point of view; I would substitute for "not well
> understood").
>
> Eli is not bound by my requests, or anyone else's. So please stop
> saying "Juanma asked me to do X, I did and I got no answer from person
> P" (for values of P different of "Juanma").


Sorry, I was not trying to imply that you did support my view. I just
followed your advice to see if it could help and noted the result.

Re: The unwarranted scrolling assumption

[Juanma Barranquero]

On Mon, Jun 21, 2010 at 12:35, Lennart Borgman
<lennart.borgman@gmail.com> wrote:

> Sorry, I was not trying to imply that you did support my view.

I am not talking about support.

> I just followed your advice to see if it could help and noted the result.

If you have already reached conclusions, perhaps you're going too fast.

    Juanma

Re: The unwarranted scrolling assumption

[Lennart Borgman]

On Mon, Jun 21, 2010 at 1:09 PM, Juanma Barranquero <lekktu@gmail.com> wrote:
> On Mon, Jun 21, 2010 at 12:35, Lennart Borgman
> <lennart.borgman@gmail.com> wrote:
>
>> Sorry, I was not trying to imply that you did support my view.
>
> I am not talking about support.

So how could I mention what you said? Or should I never mention it?

>> I just followed your advice to see if it could help and noted the result.
>
> If you have already reached conclusions, perhaps you're going too fast.

Yes, it is possible that I am going too fast. The possibilities for
misunderstandings are very huge since we mostly do not know each other
except for what we read and write here. We do not know much about each
others situation.

I glad for what we can achieve. Complexity is hard to handle within
the limits we have, but even though I tend to think it is the biggest
problem I also often forget it.

Re: The unwarranted scrolling assumption

[Juanma Barranquero]

On Mon, Jun 21, 2010 at 13:21, Lennart Borgman
<lennart.borgman@gmail.com> wrote:

> So how could I mention what you said? Or should I never mention it?

No, of course I don't mean that (it wasn't private communication,
after all). It's just that it seems like, because *I* recommended a
way, Eli *should* have answered. And I'm stressing that my
recommendation and anyone else's participation on this (or any other)
conversation are mutually independent events.

> Yes, it is possible that I am going too fast. The possibilities for
> misunderstandings are very huge since we mostly do not know each other
> except for what we read and write here. We do not know much about each
> others situation.

Agreed. But it is hard to imagine a situation that requires solving
this scrolling problem in the next few hours ;-)

    Juanma

Re: The unwarranted scrolling assumption

[Lennart Borgman]

On Mon, Jun 21, 2010 at 1:29 PM, Juanma Barranquero <lekktu@gmail.com> wrote:
> On Mon, Jun 21, 2010 at 13:21, Lennart Borgman
> <lennart.borgman@gmail.com> wrote:
>
>> So how could I mention what you said? Or should I never mention it?
>
> No, of course I don't mean that (it wasn't private communication,
> after all). It's just that it seems like, because *I* recommended a
> way, Eli *should* have answered. And I'm stressing that my
> recommendation and anyone else's participation on this (or any other)
> conversation are mutually independent events.

It was a hard decision for me to make whether to mention your
recommendation to explain more carefully or not. If I did not mention
it it would have been as if I did not care to explain because your
message somehow implied that too. And when I did mention what you
wrote I might put more pressure on Eli which was not what I wanted.

And if I did not mention it it would have been like I was hopelessly
begging for someone to care about what I have suggested.

So I just did what I did to see if this could move things further
without anyone got too much feelings of being insulted.

>> Yes, it is possible that I am going too fast. The possibilities for
>> misunderstandings are very huge since we mostly do not know each other
>> except for what we read and write here. We do not know much about each
>> others situation.
>
> Agreed. But it is hard to imagine a situation that requires solving
> this scrolling problem in the next few hours ;-)

Yes, if we can get back to that.

Re: The unwarranted scrolling assumption

[Juanma Barranquero]

On Mon, Jun 21, 2010 at 13:41, Lennart Borgman
<lennart.borgman@gmail.com> wrote:

> So I just did what I did to see if this could move things further
> without anyone got too much feelings of being insulted.

OK, no big deal after all.

> Yes, if we can get back to that.

That'd be good.

    Juanma

Re: The unwarranted scrolling assumption

[Eli Zaretskii]

> From: Lennart Borgman <lennart.borgman@gmail.com>
> Date: Mon, 21 Jun 2010 11:31:17 +0200
> Cc: emacs-devel@gnu.org
> 
> - The original "jumping scrolling" is still there (this was expected by me).

What exactly is this "jumping scrolling"?  The problem that I'm trying
to solve is that Emacs recenters the display, i.e. displays the line
with the cursor in the middle of the window.  If that is not what you
see, we are talking about two different issues; in that case, please
describe in more detail what you see.

> - The problems around line 702 in window.c (concerning
> visual-line-mode), which I thought you have tried to address are also
> still there.

Maybe I didn't do everything that's needed to reproduce this.  It
would help if you could find an easier recipe, one that didn't involve
loading nxhtml, resizing the frame, etc.

> Can you please tell me what you think is the best to do now?

I'm trying to reproduce the problem on my machine.  That seems to be
the only efficient way for me to study this problem.  Please be
patient.

Re: The unwarranted scrolling assumption

[Lennart Borgman]

On Mon, Jun 21, 2010 at 7:01 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Lennart Borgman <lennart.borgman@gmail.com>
>> Date: Mon, 21 Jun 2010 11:31:17 +0200
>> Cc: emacs-devel@gnu.org
>>
>> - The original "jumping scrolling" is still there (this was expected by me).
>
> What exactly is this "jumping scrolling"?  The problem that I'm trying
> to solve is that Emacs recenters the display, i.e. displays the line
> with the cursor in the middle of the window.  If that is not what you
> see, we are talking about two different issues; in that case, please
> describe in more detail what you see.


This is exactly what I mean when I say "jumping scrolling".


>> - The problems around line 702 in window.c (concerning
>> visual-line-mode), which I thought you have tried to address are also
>> still there.
> Maybe I didn't do everything that's needed to reproduce this.  It
> would help if you could find an easier recipe, one that didn't involve
> loading nxhtml, resizing the frame, etc.


If you do not want to test with nxhtml I can try to find another way
and check before sending it, but I do not have time right now.

What is the problem with using nXhtml for the test? You load it with
just one line. (You have to download it to and unzip it.)

Finding a clear example of this situation took me quite some time, but
I believe I understand what is happening. If I am wrong it might be
difficult to find it again if some more changes are mixed in. That is
why I sent you the exact width for the two errors I investigated
there. (I am sure more can go wrong there, but I thought that what I
sent was enough to start digging into it.)


>> Can you please tell me what you think is the best to do now?
>
> I'm trying to reproduce the problem on my machine.  That seems to be
> the only efficient way for me to study this problem.  Please be
> patient.


I understand from Stephen's message that we do indeed work very
differently. I think that is good if we find a way to cooperate.

You say that the only efficient way is for you to reproduce the
problem on your machine. I can understand it is the way you mostly
work and I do that too.

However here I do think I have a piece of information that you can not
discover easily that way. That is why I am trying to communicate my
understanding of the problem to you. I think it could save time
starting there. (But there are surely more problems.)

Maybe you are too involved in other parts of the problem at the moment
to be able to take that communication, but as soon as you can and want
please tell me.

Re: The unwarranted scrolling assumption

[David De La Harpe Golden]

On 19/06/10 18:54, Eli Zaretskii wrote:

>
> I would like you to try the above recipe in "emacs -Q", and tell me if
> you still see recentering.
>


Testing rev 100620:

recentering still happening, though possibly less often (or my system is 
just having a better day, I should have kept an old build about).

I can still get it to happen easily by ramping up keyboard repeat rate, 
but it happens quite often on HELLO even at reasonable rates.

Re: The unwarranted scrolling assumption

[Lennart Borgman]

On Sat, Jun 19, 2010 at 10:13 PM, David De La Harpe Golden
<david@harpegolden.net> wrote:
> On 19/06/10 18:54, Eli Zaretskii wrote:
>
>>
>> I would like you to try the above recipe in "emacs -Q", and tell me if
>> you still see recentering.
>>
>
>
> Testing rev 100620:
>
> recentering still happening, though possibly less often (or my system is
> just having a better day, I should have kept an old build about).
>
> I can still get it to happen easily by ramping up keyboard repeat rate, but
> it happens quite often on HELLO even at reasonable rates.


This is expected. The original problem is still only addressed in my patch.

And that is not applied.

Re: The unwarranted scrolling assumption

[Juanma Barranquero]

On Sat, Jun 19, 2010 at 22:26, Lennart Borgman
<lennart.borgman@gmail.com> wrote:

> This is expected. The original problem is still only addressed in my patch.
>
> And that is not applied.

Then, help Eli understand why your patch is the right fix.

    Juanma

Re: The unwarranted scrolling assumption

[Lennart Borgman]

On Sat, Jun 19, 2010 at 10:30 PM, Juanma Barranquero <lekktu@gmail.com> wrote:
> On Sat, Jun 19, 2010 at 22:26, Lennart Borgman
> <lennart.borgman@gmail.com> wrote:
>
>> This is expected. The original problem is still only addressed in my patch.
>>
>> And that is not applied.
>
> Then, help Eli understand why your patch is the right fix.


I have done the best I can. I do not know what to do.

The basic assumption that clip_changed can be set in narrow_to_region
etc and that redisplay later can decide when it can be reset is false.
Redisplay does not have the necessary information since it does not
know exactly why clip_changed was set to 1.

I have said that there is no reason to let narrow_to_region set
clip_changed (even if it really seems intuitively right to do so). It
is much easier to let redisplay do that instead in
reconsider_clip_changes because then all needed information for the
specific window is available.

Is this clear?

Re: The unwarranted scrolling assumption

[Juanma Barranquero]

On Sat, Jun 19, 2010 at 22:38, Lennart Borgman
<lennart.borgman@gmail.com> wrote:

> Is this clear?

Perhaps. Let's hear the redisplay experts.

    Juanma

Re: The unwarranted scrolling assumption

[Eli Zaretskii]

> From: Lennart Borgman <lennart.borgman@gmail.com>
> Date: Sat, 19 Jun 2010 22:38:24 +0200
> Cc: emacs-devel@gnu.org, David De La Harpe Golden <david@harpegolden.net>
> 
> On Sat, Jun 19, 2010 at 10:30 PM, Juanma Barranquero <lekktu@gmail.com> wrote:
> > On Sat, Jun 19, 2010 at 22:26, Lennart Borgman
> > <lennart.borgman@gmail.com> wrote:
> >
> >> This is expected. The original problem is still only addressed in my patch.
> >>
> >> And that is not applied.
> >
> > Then, help Eli understand why your patch is the right fix.
> 
> 
> I have done the best I can. I do not know what to do.
> 
> The basic assumption that clip_changed can be set in narrow_to_region
> etc and that redisplay later can decide when it can be reset is false.
> Redisplay does not have the necessary information since it does not
> know exactly why clip_changed was set to 1.
> 
> I have said that there is no reason to let narrow_to_region set
> clip_changed (even if it really seems intuitively right to do so). It
> is much easier to let redisplay do that instead in
> reconsider_clip_changes because then all needed information for the
> specific window is available.

Lennart, I need _evidence_, not ideas, assumptions, and theories.

We are not trying to rewrite the display code from scratch, we are
trying to fix a relatively minor problem in an otherwise working code.
The way to fix minor problems is not to find a totally different way
of doing something, but to find specific reason(s) why existing code
doesn't do what it was supposed to do.

IOW, it doesn't matter to me in how many different ways you can set
the clip_changed flag.  What I want to understand first and foremost
is why the _existing_ method fails.  Until I understand that, I cannot
reason about your suggested patch, and I therefore will not consider
it for inclusion in Emacs.

Now, I asked you repeatedly to give me values of variables inside
reconsider_clip_changes, which would explain why clip_changed is not
reset by it.  You didn't give those values to me, for some reason that
I cannot understand.  If you persist in your refusal to cooperate by
showing those values, and any others that I may need for this
investigation, I don't see any reason to continue talking to you in
this thread.

Re: The unwarranted scrolling assumption

[Lennart Borgman]

On Sat, Jun 19, 2010 at 11:33 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Lennart Borgman <lennart.borgman@gmail.com>
>> Date: Sat, 19 Jun 2010 22:38:24 +0200
>> Cc: emacs-devel@gnu.org, David De La Harpe Golden <david@harpegolden.net>
>>
>> On Sat, Jun 19, 2010 at 10:30 PM, Juanma Barranquero <lekktu@gmail.com> wrote:
>> > On Sat, Jun 19, 2010 at 22:26, Lennart Borgman
>> > <lennart.borgman@gmail.com> wrote:
>> >
>> >> This is expected. The original problem is still only addressed in my patch.
>> >>
>> >> And that is not applied.
>> >
>> > Then, help Eli understand why your patch is the right fix.
>>
>>
>> I have done the best I can. I do not know what to do.
>>
>> The basic assumption that clip_changed can be set in narrow_to_region
>> etc and that redisplay later can decide when it can be reset is false.
>> Redisplay does not have the necessary information since it does not
>> know exactly why clip_changed was set to 1.
>>
>> I have said that there is no reason to let narrow_to_region set
>> clip_changed (even if it really seems intuitively right to do so). It
>> is much easier to let redisplay do that instead in
>> reconsider_clip_changes because then all needed information for the
>> specific window is available.
>
> Lennart, I need _evidence_, not ideas, assumptions, and theories.


Hm, you are talking to someone who is quite theoretical in those
areas. I can see we look upon code quite differently.


> We are not trying to rewrite the display code from scratch, we are
> trying to fix a relatively minor problem in an otherwise working code.
> The way to fix minor problems is not to find a totally different way
> of doing something, but to find specific reason(s) why existing code
> doesn't do what it was supposed to do.


The involvement of the display engine is quite minor. This is merely a
logical problem with updating clip_changed.

I am only talking about replacing the logic used there. The so to say
inner parts of redisplay is not at all touch by my patch. (How could
they be that, the patch will only a few lines in its final state?)


> IOW, it doesn't matter to me in how many different ways you can set
> the clip_changed flag.  What I want to understand first and foremost
> is why the _existing_ method fails.  Until I understand that, I cannot
> reason about your suggested patch, and I therefore will not consider
> it for inclusion in Emacs.


And I am trying to explain this, but I apparently fail. And this
thread has become just too long because of our problems to
communicate. It is a bit complex, indeed, and I guess that is why we
have problems.

So just let us take it again and let me be more expressive this time.
This will allow you to more clearly see if I have missed something. I
will try to take the whole picture (with some parts I have avoided to
mention to not stir up more confusion).


First, clip_changed is currently a property of the buffer. This in
itself problematic since it supposed to be used to tell if a specific
window needs to care about clip changes. Different windows might have
been displayed at different times so one of them may have the clipping
correct while another does not have it correct. Since clip_changed
belongs to the buffer you can not distinguish those cases. So it
should maybe be moved to the windows instead.

This of course makes it impossible to update it from the narrow_to_region etc.

However I can see no problem updating it at the beginning of
reconsider_clip_changes as I suggested. If we set clip_changed to 1
there as soon as current clipping does not match the clipping recorded
at last finished redisplay we can not come from the start of a new
redisplay to a finished redisplay without setting clip_changed to 1 if
the buffer clipping has changed.

NOTE: At least not if buffer clipping can not change between the last
call to reconsider_clip_changes and a finished redisplay. If this is
not true then my suggestion will fail if clip_changed is 0 when the
buffer clipping is changed.


You can also see in the code that clip_changed is not just about
buffer clipping, look in reconsider_clip_changes. There clip_changed
is set to 1 to reflect something quite different than clipping
changes. It is set to 1 to force a redisplay. And in the same function
the code checks (for reasons currently beyond me but I do not doubt
they are correct) that window_valid_end is not nil before it resets
clip_changed to 0.

That window_valid_end is 0 is the reason resetting clip_changed to 0
is not done. It could have been 0 before the clipping change occurred
and this might mean that we could reset clip_changed (but it need not
mean that). We do not have the slightest idea at this point because we
do not have enough information.

Therefore it looks much easier to me to just set clip_changed to 1
when it seems to be needed in redisplay instead. (But please see the
NOTE above.) It also allows clipping to be really handled by window
(which is a corner case but it is nice to fix it if we can).


To make the picture complete we must also of course be aware of what
the narrowing routines does. But that is not very complicated
regarding this. For example save-restriction-restore just sets
clip_changed to 1 when restoring clipping if some clipping change has
occurred. Those routines can not actually work in any other way when
setting clip_changed since they do not know at all what has happened
with redisplay.


> Now, I asked you repeatedly to give me values of variables inside
> reconsider_clip_changes, which would explain why clip_changed is not
> reset by it.  You didn't give those values to me, for some reason that
> I cannot understand.  If you persist in your refusal to cooperate by
> showing those values, and any others that I may need for this
> investigation, I don't see any reason to continue talking to you in
> this thread.


Sorry, some misunderstanding. I think I told, but it looks like I did
send you the actual trace output. (I have been waiting for some more
serious feedback from you about the solution I suggested. There has
been very many messages here.) The output shows that it is because
window_valid_end is 0 that clip_changed is not reset:

   warning: +++reconsider: end_valid=0, b=1, ZV=1, BEGV=1

And this is how I get it:

static INLINE void
reconsider_clip_changes (w, b)
     struct window *w;
     struct buffer *b;
{
  if (b->clip_changed
      && !(!NILP (w->window_end_valid)
	   && w->current_matrix->buffer == b
	   && w->current_matrix->zv == BUF_ZV (b)
	   && w->current_matrix->begv == BUF_BEGV (b)))
    DebPrint (("+++reconsider: end_valid=%d, b=%d, ZV=%d, BEGV=%d",
               !NILP (w->window_end_valid),
               w->current_matrix->buffer == b,
               w->current_matrix->zv == BUF_ZV (b),
               w->current_matrix->begv == BUF_BEGV (b)));

  if (b->clip_changed
	   && !NILP (w->window_end_valid)
	   && w->current_matrix->buffer == b
	   && w->current_matrix->zv == BUF_ZV (b)
	   && w->current_matrix->begv == BUF_BEGV (b))
    b->clip_changed = 0;

...

Re: The unwarranted scrolling assumption

[Stephen J. Turnbull]

Lennart Borgman writes:

 > I have done the best I can. I do not know what to do.

Why don't you simply do what Eli asked, and test his patch?

You seem to be terrified that it might work, but if it does, it will
go it and make life better for you.  If it doesn't, you have no
guarantee that your patch will go in.

I agree from what you say that redisplay seems to be broken by design
here, and a more comprehensive patch should be considered.  But I also
agree with Eli that your proposed changes are scary (he might phrase
that differently) because redisplay is really complex, and any
substantial changes are likely to break something that is important to
somebody.  By Murphy's law, this will only manifest itself the day
before release.  You wouldn't want that, would you?

So "fixing the jumping scroll" and "fixing redisplay" are two
different things.  It is possible that Eli's patch will fix the
particular bug(s) that this thread treats, while leaving the
underlying design problem for later.

Re: The unwarranted scrolling assumption

[Lennart Borgman]

On Sun, Jun 20, 2010 at 6:54 PM, Stephen J. Turnbull <stephen@xemacs.org> wrote:
> Lennart Borgman writes:
>
>  > I have done the best I can. I do not know what to do.
>
> Why don't you simply do what Eli asked, and test his patch?

I am going to do that but as far as I understand Eli has not in his
patch addressed the logic problems behind the "jumping scrolling".
However I gave a clear recipe for another scrolling bug and that is
what I assume Eli fixed so I am going to test that carefully.

> You seem to be terrified that it might work,

No, but I am clearly a bit upset because no one seem to be interested
in the logic. Juanma asked me to explain again more clearly to Eli. I
did. Where is the answer to my more detailed explanation?

> If it doesn't, you have no
> guarantee that your patch will go in.

I do not care about if my patch goes in. I just care about if the
problem gets fixed. And from David's testing it seems like it is not
fixed. But I will test anyway since I do not want to be unfair to Eli.
I do care that he tried to fix the problem.

> I agree from what you say that redisplay seems to be broken by design
> here, and a more comprehensive patch should be considered.

In my opinion the patch I sent is very simple.

Why don't you take a look at it? I mean the logic. I think you should
have no difficulties to see how the patch will look. Otherwise I can
make a clean version of the patch. This is anyway what I will do since
I will put it in my patched version for those who care.

> But I also
> agree with Eli that your proposed changes are scary (he might phrase
> that differently) because redisplay is really complex, and any
> substantial changes are likely to break something that is important to
> somebody.

I can understand that. I did look into some part of redisplay before
(because I was annoyed bye some part of the overlay handling and
invisibility).

The logic of my patch does however not change the inner logic of
redisplay in any way. It does change the interaction between clipping
and redisplay. It moves the whole responsibility to redisplay. There
is a possibility for one glitch (i.e. if my logic is correct): That
clipping changes occur between the last check of redisplay and display
finish. I think that fixing this kind of problem will not be too hard.

> By Murphy's law, this will only manifest itself the day
> before release.  You wouldn't want that, would you?

That is ok. Just fix it then before release ;-)

Or, to be more serious. Now is the time to fix it. We are far form the
version 24 release. (Just do not include it in next 23 series bug
fixes if any doubt remains.)

> So "fixing the jumping scroll" and "fixing redisplay" are two
> different things.  It is possible that Eli's patch will fix the
> particular bug(s) that this thread treats, while leaving the
> underlying design problem for later.

Yes of course. I may be wrong however I believe rather strongly the
efforts Eli has made can not cure the problem. (I think what Eli has
done may still be very good. It may of course also break redisplay, we
all make mistakes since human beings are unfortunately not very good
at logic, but I am not very worried and no one else seems worried
about it either.)

Re: The unwarranted scrolling assumption

[Stephen J. Turnbull]

Lennart Borgman writes:
 > On Sun, Jun 20, 2010 at 6:54 PM, Stephen J. Turnbull <stephen@xemacs.org> wrote:
 > > Lennart Borgman writes:
 > >
 > >  > I have done the best I can. I do not know what to do.
 > >
 > > Why don't you simply do what Eli asked, and test his patch?
 > 
 > I am going to do that but as far as I understand Eli has not in his
 > patch addressed the logic problems behind the "jumping scrolling".

Perhaps not, but testing it and saying "it doesn't work" gives you
much stronger claim on his attention to your patch.

 > No, but I am clearly a bit upset because no one seem to be interested
 > in the logic. Juanma asked me to explain again more clearly to Eli. I
 > did. Where is the answer to my more detailed explanation?

I've been working with Eli for about two decades.  He has never
trusted logic unaccompanied by data.  I happen to like that attitude,
but even if it doesn't work for you, he's unlikely to change.

 > I do not care about if my patch goes in. I just care about if the
 > problem gets fixed. And from David's testing it seems like it is not
 > fixed.

But David's testing used HELLO, and HELLO triggers bidi, and bidi has
its own problems IIUC; certainly Eli produced a patch for it.  There's
some evidence that they are different problems.  So you need to test
too.

 > Why don't you take a look at it? I mean the logic. I think you should
 > have no difficulties to see how the patch will look.

Because I don't understand the relevant redisplay invariants, and
don't have time to study them.

Re: The unwarranted scrolling assumption

[Lennart Borgman]

On Mon, Jun 21, 2010 at 1:38 PM, Stephen J. Turnbull
<turnbull@sk.tsukuba.ac.jp> wrote:
>
> Perhaps not, but testing it and saying "it doesn't work" gives you
> much stronger claim on his attention to your patch.

Yes, I just did and sent that here.

>  > No, but I am clearly a bit upset because no one seem to be interested
>  > in the logic. Juanma asked me to explain again more clearly to Eli. I
>  > did. Where is the answer to my more detailed explanation?
>
> I've been working with Eli for about two decades.  He has never
> trusted logic unaccompanied by data.  I happen to like that attitude,
> but even if it doesn't work for you, he's unlikely to change.

Thanks. Yes, Eli and I are then working very differently.

For me if I want (or need) data depends very much on what I know about
the person I am working together with. If we can communicate on the
logic level I prefer that since it is much quicker.

I of course also appreciate that Eli want things tested. It is very
good. It is more about when we should test. (And in some situations
logic have to come first, but not here.)

Anyway I have tried to accommodate to what Eli wants and I hope we can
start communicating again.

>  > I do not care about if my patch goes in. I just care about if the
>  > problem gets fixed. And from David's testing it seems like it is not
>  > fixed.
>
> But David's testing used HELLO, and HELLO triggers bidi, and bidi has
> its own problems IIUC; certainly Eli produced a patch for it.  There's
> some evidence that they are different problems.  So you need to test
> too.

Yes. I tested and I seem to have misunderstood what Eli's patch was
for. Those problems I have mentioned ("jumping scrolling" and the
problems that includes visual-line-mode/word-wrap) are still there.
However David seems to have tested this so I think it is ok.

>  > Why don't you take a look at it? I mean the logic. I think you should
>  > have no difficulties to see how the patch will look.
>
> Because I don't understand the relevant redisplay invariants, and
> don't have time to study them.

OK.

Re: The unwarranted scrolling assumption

[David Engster]

Stephen J. Turnbull writes:
> But David's testing used HELLO, and HELLO triggers bidi, and bidi has
> its own problems IIUC; certainly Eli produced a patch for it.  There's
> some evidence that they are different problems. 

Yes, I wasn't aware that some problems with HELLO were due to triggering
bidi on Emacs 24. I actually also tested my recipe with Emacs 23.2, and
I have recentering there, too, since my main problems pretty clearly
come from Emacs' redisplay not keeping up with keyboard input while
leaning on the <down> key. This is supported by the fact that when I
rebind <down> to '(progn (next-line) (redisplay t))', the recentering
practically disappears. As far as I understood from this thread, neither
Eli's nor Lennart's patches will be able to fix this, since this is a
result of the basic design of the display engine. Maybe it would be
possible to deal with this better than the current recentering, but this
is a separate issue and hence should be discussed elsewhere.

-David

Re: The unwarranted scrolling assumption

[Lennart Borgman]

On Mon, Jun 21, 2010 at 2:11 PM, David Engster <deng@randomsample.de> wrote:
>
> I have recentering there, too, since my main problems pretty clearly
> come from Emacs' redisplay not keeping up with keyboard input while
> leaning on the <down> key. This is supported by the fact that when I
> rebind <down> to '(progn (next-line) (redisplay t))', the recentering
> practically disappears.

I think that might be a misunderstanding. What is probably happening
is rather that redisplay somehow resets the state of clip_changed.

And that is needed. My patch does that without calling (redisplay t)
which would be time consuming.

> As far as I understood from this thread, neither
> Eli's nor Lennart's patches will be able to fix this, since this is a
> result of the basic design of the display engine.

I am not sure my patch fixes this, but I have seen no problem with
"jumping scrolling" when using it (but I have seen a problem with
visual-line-mode that looks a bit similar on the surface, but is
totally different). Something more may be hitting you but I believe my
patch is any way needed.

Since I do not have any clue if anything else hits you unless you test
my patch I would be glad if you did. That could make some progress
here and I could look into further problems too.

Do you want to test my patch? In that case I will try to make a clean
patch and send it here.

Re: The unwarranted scrolling assumption

[David Engster]

Lennart Borgman writes:
> On Mon, Jun 21, 2010 at 2:11 PM, David Engster <deng@randomsample.de> wrote:
>>
>> I have recentering there, too, since my main problems pretty clearly
>> come from Emacs' redisplay not keeping up with keyboard input while
>> leaning on the <down> key. This is supported by the fact that when I
>> rebind <down> to '(progn (next-line) (redisplay t))', the recentering
>> practically disappears.
>
> I think that might be a misunderstanding. What is probably happening
> is rather that redisplay somehow resets the state of clip_changed.
> And that is needed. My patch does that without calling (redisplay t)
> which would be time consuming.

Well yes, which is why my rebinding to enforce redisplay isn't a real
solution but merely a circumvention of the problem.

I'm not sure if this is a misunderstanding; I think Juanma pretty much
summarized the problem with "Emacs should stop reading input until it
catches his breath", and Stefan's as well as Eli's response to that was
pretty clear that this cannot be done without redesigning
redisplay. This is why I thought you're actually working on another,
more subtle bug, which might very well be there but accounts only for a
small fraction of recenterings I'm witnessing. Please also note that I
really only see those recenterings while leaning on <down>, not when
scrolling one single line.

I actually did test the patch you first send, but I couldn't notice any
difference while scrolling; recentering did still happen when leaning on
<down>. However, you also said that the patches were not complete, so
I'd be happy to test anything you send me (feel free to send them via
private mail, no need to clean anything up).

-David

Re: The unwarranted scrolling assumption

[Lennart Borgman]

On Mon, Jun 21, 2010 at 2:45 PM, David Engster <deng@randomsample.de> wrote:
>
> I'm not sure if this is a misunderstanding; I think Juanma pretty much
> summarized the problem with "Emacs should stop reading input until it
> catches his breath", and Stefan's as well as Eli's response to that was
> pretty clear that this cannot be done without redesigning
> redisplay.

I think we also have made different guesses of what is most important.

> This is why I thought you're actually working on another,
> more subtle bug,

The bug with clip_changed is not very subtle in my view. It is just a
bug in the logic, a bad assumption that was not noticed before.

> I actually did test the patch you first send, but I couldn't notice any
> difference while scrolling; recentering did still happen when leaning on
> <down>.

There seems to be something interesting there then. Fine.

I can't see any recentering at all (except for those connect with the
visual-line-mode/word-wrap bug I also told about) but Emacs was
recentering very often for me when holding down <down> before I wrote
my patch for clip_changed.

> However, you also said that the patches were not complete, so
> I'd be happy to test anything you send me (feel free to send them via
> private mail, no need to clean anything up).

Thanks. I will send them to you privately then.

Re: The unwarranted scrolling assumption

[Eli Zaretskii]

> From: David Engster <deng@randomsample.de>
> Date: Mon, 21 Jun 2010 14:45:54 +0200
> Cc: emacs-devel@gnu.org
> 
> I actually did test the patch you first send, but I couldn't notice any
> difference while scrolling; recentering did still happen when leaning on
> <down>.

Did you try the latest trunk (revno 100623 or later)?  If not, please
do.

Re: The unwarranted scrolling assumption

[David Engster]

Eli Zaretskii writes:
>> From: David Engster <deng@randomsample.de>
>> Date: Mon, 21 Jun 2010 14:45:54 +0200
>> Cc: emacs-devel@gnu.org
>> 
>> I actually did test the patch you first send, but I couldn't notice any
>> difference while scrolling; recentering did still happen when leaning on
>> <down>.
>
> Did you try the latest trunk (revno 100623 or later)?  If not, please
> do.

I did test it, and Emacs is still recentering.

-David

Re: The unwarranted scrolling assumption

[Eli Zaretskii]

> From: David Engster <deng@randomsample.de>
> Cc: emacs-devel@gnu.org
> Date: Tue, 22 Jun 2010 09:48:39 +0200
> 
> Eli Zaretskii writes:
> >> From: David Engster <deng@randomsample.de>
> >> Date: Mon, 21 Jun 2010 14:45:54 +0200
> >> Cc: emacs-devel@gnu.org
> >> 
> >> I actually did test the patch you first send, but I couldn't notice any
> >> difference while scrolling; recentering did still happen when leaning on
> >> <down>.
> >
> > Did you try the latest trunk (revno 100623 or later)?  If not, please
> > do.
> 
> I did test it, and Emacs is still recentering.

Thanks.

Re: The unwarranted scrolling assumption

[Eli Zaretskii]

> From: David Engster <deng@randomsample.de>
> Date: Mon, 21 Jun 2010 14:11:41 +0200
> 
> As far as I understood from this thread, neither Eli's nor Lennart's
> patches will be able to fix this, since this is a result of the
> basic design of the display engine.

That's not true.  The only thing that's basic in the design is that
Emacs always consumes all input events before it enters redisplay.
But if you set scroll-conservatively to a large number, the worst that
should happen to you is that Emacs seemingly stops responding when you
lean on the <down> key.  Once you release the key, you still should
see the cursor on the last screen line (if the last screen line is not
fully visible, it is okay to see the cursor on the next-to-last
line).  That's it; you should never see the cursor in the middle of
the screen -- that is a bug (or maybe misfeature).

In the last few days I fixed a couple of bugs that caused incorrect
recentering; evidently there are more bugs lurking there.  I will try
to hunt them down.  Unfortunately, I cannot easily reproduce these
problems on my machine, probably because it's faster than yours, which
doesn't make the job easier.

Re: The unwarranted scrolling assumption

[Eli Zaretskii]

> Date: Sat, 19 Jun 2010 21:13:23 +0100
> From: David De La Harpe Golden <david@harpegolden.net>
> 
> I can still get it to happen easily by ramping up keyboard repeat rate, 
> but it happens quite often on HELLO even at reasonable rates.

Thanks for testing.

HELLO turns on bidi-display-reordering, which affects scrolling
through problems that are specific to the bidi redisplay.  Can you
still see recentering if you turn bidi-display-reordering off in
HELLO, like this:

  M-: (setq bidi-display-reordering nil)

?

Re: The unwarranted scrolling assumption

[Eli Zaretskii]

> Date: Sun, 20 Jun 2010 00:17:08 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: emacs-devel@gnu.org
> 
> > Date: Sat, 19 Jun 2010 21:13:23 +0100
> > From: David De La Harpe Golden <david@harpegolden.net>
> > 
> > I can still get it to happen easily by ramping up keyboard repeat rate, 
> > but it happens quite often on HELLO even at reasonable rates.
> 
> Thanks for testing.
> 
> HELLO turns on bidi-display-reordering, which affects scrolling
> through problems that are specific to the bidi redisplay.  Can you
> still see recentering if you turn bidi-display-reordering off in
> HELLO, like this:
> 
>   M-: (setq bidi-display-reordering nil)
> 
> ?

Also, do you see the recentering only the first time you scroll
through HELLO, or also after that?  That is, if you first lean on the
<down> arrow key and scroll through the entire file, then type C-Home
and lean again on <down>, do you see recentering again on the second
scroll through the file?

TIA

Re: The unwarranted scrolling assumption

[David De La Harpe Golden]

On 19/06/10 22:38, Eli Zaretskii wrote:


> Also, do you see the recentering only the first time you scroll
> through HELLO, or also after that?  That is, if you first lean on the
> <down>  arrow key and scroll through the entire file, then type C-Home
> and lean again on<down>, do you see recentering again on the second
> scroll through the file?
>

Yep. At least sometimes. At keyboard repeat rates high enough to trigger
it nearly every time, nearly every time.

Re: The unwarranted scrolling assumption

[Eli Zaretskii]

> Date: Sat, 19 Jun 2010 23:19:41 +0100
> From: David De La Harpe Golden <david@harpegolden.net>
> CC: emacs-devel@gnu.org
> 
> On 19/06/10 22:38, Eli Zaretskii wrote:
> 
> 
> > Also, do you see the recentering only the first time you scroll
> > through HELLO, or also after that?  That is, if you first lean on the
> > <down>  arrow key and scroll through the entire file, then type C-Home
> > and lean again on<down>, do you see recentering again on the second
> > scroll through the file?
> >
> 
> Yep. At least sometimes. At keyboard repeat rates high enough to trigger
> it nearly every time, nearly every time.

I succeeded to reproduce recentering in HELLO, and fixed the problem
which caused that on my machine.  Please see if revno 100623 fixes the
problem for you as well, i.e. there's no more recentering in HELLO (or
elsewhere), with these settings:

    (setq scroll-preserve-screen-position 'always
	  scroll-conservatively           most-positive-fixnum
	  scroll-step                     0)

(You can also set redisplay-dont-pause, if you want.)

Re: The unwarranted scrolling assumption

[David De La Harpe Golden]

On 20/06/10 19:13, Eli Zaretskii wrote:

> I succeeded to reproduce recentering in HELLO, and fixed the problem
> which caused that on my machine.  Please see if revno 100623 fixes the
> problem for you as well, i.e. there's no more recentering in HELLO (or
> elsewhere), with these settings:
>
>      (setq scroll-preserve-screen-position 'always
> 	  scroll-conservatively           most-positive-fixnum
> 	  scroll-step                     0)
>
> (You can also set redisplay-dont-pause, if you want.)
>


window.c: Still recentering sometimes.

I guess you've fixed _something_, just not the issue Lennart is talking 
about: HELLO is definitely recentering less often than it was (making 
the window narrower so that lines wrap makes it happen more repeatably).

However, when redisplay-dont-pause is nil, sometimes it
stops updating for a bit but with scroll bar still moving, only 
eventually updating. Maybe that was previously masked.

Re: The unwarranted scrolling assumption

[Eli Zaretskii]

> Date: Sun, 20 Jun 2010 22:40:44 +0100
> From: David De La Harpe Golden <david@harpegolden.net>
> CC: emacs-devel@gnu.org
> 
> window.c: Still recentering sometimes.

Does the problem go away if you disable font-lock-mode?

> I guess you've fixed _something_, just not the issue Lennart is talking 
> about: HELLO is definitely recentering less often than it was (making 
> the window narrower so that lines wrap makes it happen more repeatably).

Again, does it happen if you disable font-lock-mode and
bidi-display-reordering?

> However, when redisplay-dont-pause is nil, sometimes it
> stops updating for a bit but with scroll bar still moving, only 
> eventually updating.

That's expected, when Emacs cannot keep up with keyboard auto-repeat
rate.

Thanks.

Re: The unwarranted scrolling assumption

[David De La Harpe Golden]

On 21/06/10 03:59, Eli Zaretskii wrote:
>> Date: Sun, 20 Jun 2010 22:40:44 +0100
>> From: David De La Harpe Golden<david@harpegolden.net>
>> CC: emacs-devel@gnu.org
>>
>> window.c: Still recentering sometimes.
>
> Does the problem go away if you disable font-lock-mode?
>

I could not reproduce today without font-lock-mode on two complete 
scrollthroughs (one with redisplay-dont-pause and one without).


font-lock-mode on:

redisplay-dont-pause nil (that's nil):

I counted about 114 visible recenters on one complete scrollthrough 
(N.B. definitely recenters i.e. cursor restarting from the middle, not 
just update lags),

redisplay-dont-pause t (that's t):

No visible recenters on one complete scrollthough.


>> I guess you've fixed _something_, just not the issue Lennart is talking
>> about: HELLO is definitely recentering less often than it was (making
>> the window narrower so that lines wrap makes it happen more repeatably).
>
> Again, does it happen if you disable font-lock-mode and
> bidi-display-reordering?

Yes (font-lock-mode doesn't appear to do anything visible at a glance on 
HELLO, but I turned it off anyway).

redisplay-dont-pause nil (that's nil):

Only visibly once in 20 whole buffer scrollthroughs (HELLO being a lot 
shorter than window.c...), though with at least one of those update lags 
every time.

redisplay-dont-pause t (that's t):

11 times in 20 whole buffer scrollthroughs.

Note there were still some wrapped lines during that test, which I have 
some difficulty eliminating with emacs -Q at present with typical
window sizes owing to #6471.

Re: The unwarranted scrolling assumption

[Lennart Borgman]

On Tue, Jun 22, 2010 at 12:06 AM, David De La Harpe Golden
<david@harpegolden.net> wrote:
> On 21/06/10 03:59, Eli Zaretskii wrote:
>>>
>>> Date: Sun, 20 Jun 2010 22:40:44 +0100
>>> From: David De La Harpe Golden<david@harpegolden.net>
>>> CC: emacs-devel@gnu.org
>>>
>>> window.c: Still recentering sometimes.
>>
>> Does the problem go away if you disable font-lock-mode?
>>
>
> I could not reproduce today without font-lock-mode on two complete
> scrollthroughs (one with redisplay-dont-pause and one without).
>
>
> font-lock-mode on:
>
> redisplay-dont-pause nil (that's nil):
>
> I counted about 114 visible recenters on one complete scrollthrough (N.B.
> definitely recenters i.e. cursor restarting from the middle, not just update
> lags),
>
> redisplay-dont-pause t (that's t):
>
> No visible recenters on one complete scrollthough.


I think you must close and open the file before each such test.

The reason is that font lock calls some of the narrowing functions and
that will cause recentering (because clip_changed will be set to 1).
And when you have scrolled through the whole file it is fontified so
the next time you test you will not see this particular problem.

Re: The unwarranted scrolling assumption

[David De La Harpe Golden]

On 22/06/10 00:04, Lennart Borgman wrote:
> On Tue, Jun 22, 2010 at 12:06 AM, David De La Harpe Golden

>
> I think you must close and open the file before each such test.
>

I restarted emacs between runs for window.c, actually (though I suppose 
I could have said...).

I just scrolled through window.c twice in one session with
font-lock-mode on and redisplay-dont-pause nil and it indeed recentered 
a lot less the second time through, if at all (I think it may have, or 
at least hiccoughed a little, close to a couple of long lines, e/g. 1686 
or so)

For HELLO, I didn't restart emacs between the (much shorter) runs, but 
it doesn't seem to make such difference.

Re: The unwarranted scrolling assumption

[Eli Zaretskii]

> Date: Mon, 21 Jun 2010 23:06:26 +0100
> From: David De La Harpe Golden <david@harpegolden.net>
> CC: emacs-devel@gnu.org
> 
> On 21/06/10 03:59, Eli Zaretskii wrote:
> >> Date: Sun, 20 Jun 2010 22:40:44 +0100
> >> From: David De La Harpe Golden<david@harpegolden.net>
> >> CC: emacs-devel@gnu.org
> >>
> >> window.c: Still recentering sometimes.
> >
> > Does the problem go away if you disable font-lock-mode?
> >
> 
> I could not reproduce today without font-lock-mode on two complete 
> scrollthroughs (one with redisplay-dont-pause and one without).
> 
> 
> font-lock-mode on:
> 
> redisplay-dont-pause nil (that's nil):
> 
> I counted about 114 visible recenters on one complete scrollthrough 
> (N.B. definitely recenters i.e. cursor restarting from the middle, not 
> just update lags),
> 
> redisplay-dont-pause t (that's t):
> 
> No visible recenters on one complete scrollthough.

Is this with the default dimensions of the Emacs frame and with the
default font you get with "emacs -Q"?

And what are the frame dimensions (in text line units)?

Re: The unwarranted scrolling assumption

[David De La Harpe Golden]

On 22/06/10 19:27, Eli Zaretskii wrote:

> Is this with the default dimensions of the Emacs frame and with the
> default font you get with "emacs -Q"?
>

Yes

xft:-bitstream-Bitstream Vera Sans 
Mono-normal-normal-normal-*-13-*-*-*-m-0-iso10646-1

  > And what are the frame dimensions (in text line units)?

Seems to be 80 x 33(+1 modeline+ 1 minibuf),
excluding menubar, toolbar.

I'm unclear how emacs picks its default size.

Re: The unwarranted scrolling assumption

[Eli Zaretskii]

> Date: Mon, 21 Jun 2010 23:06:26 +0100
> From: David De La Harpe Golden <david@harpegolden.net>
> CC: emacs-devel@gnu.org
> 
> font-lock-mode on:
> 
> redisplay-dont-pause nil (that's nil):
> 
> I counted about 114 visible recenters on one complete scrollthrough 
> (N.B. definitely recenters i.e. cursor restarting from the middle, not 
> just update lags),
> 
> redisplay-dont-pause t (that's t):
> 
> No visible recenters on one complete scrollthough.

After looking at the code and experimenting a bit, I think there's no
other way to prevent recentering on slow machines except set
redisplay-dont-pause non-nil.  (You could also try playing with
redisplay-preemption-period, which basically gives you more fine
granularity than just the ON/OFF you get with redisplay-dont-pause.)

I could perhaps make things a bit better by marking each frame's
windows, or even each window up to date as soon as it is redisplayed.
(By contrast, the current code only does that after _all_ frames were
completely redisplayed without being preempted by input that arrived.)
But that would hardly help in this simple use-case, where there's a
single frame and a single window on that frame.

> redisplay-dont-pause nil (that's nil):
> 
> Only visibly once in 20 whole buffer scrollthroughs (HELLO being a lot 
> shorter than window.c...), though with at least one of those update lags 
> every time.
> 
> redisplay-dont-pause t (that's t):
> 
> 11 times in 20 whole buffer scrollthroughs.

This is due to some other problem.  Do you really see Emacs
recentering the window with HELLO?  What I see is that the current
line is not recentered, but scrolled all the way to the top of the
window.  IOW, the cursor jumps to the first screen line, and then
moves down until it hits the end of the window.  Could it be that you
see this, instead of recentering?

Re: The unwarranted scrolling assumption

[David De La Harpe Golden]

On 23/06/10 20:11, Eli Zaretskii wrote:


> After looking at the code and experimenting a bit, I think there's no
> other way to prevent recentering on slow machines except set
> redisplay-dont-pause non-nil.

[FWIW, my current machine, while not totally new, is a multicore opteron 
and not especially slow either]

>> redisplay-dont-pause t (that's t):
>>
>> 11 times in 20 whole buffer scrollthroughs.
>
> This is due to some other problem.  Do you really see Emacs
> recentering the window with HELLO?  What I see is that the current
> line is not recentered, but scrolled all the way to the top of the
> window.  IOW, the cursor jumps to the first screen line, and then
> moves down until it hits the end of the window.  Could it be that you
> see this, instead of recentering?


Hmm.  I am fallible, so yes its possible.

Trying to _stop_ as soon as possible after the event occurs rather than 
scrolling through, I have now made that happen - managed to stop with 
with tigrigna at first screen line and the point nearby, in the upper 
half of the window. Which couldn't happen with recenter.

But I've also stopped with Lao  ending up at the top of the screen soon 
after hitting Slovenian or so at the bottom, and Oriya
at the top soon after hitting Tigrigna or so, with the point in the 
lower half of the window.

So that could be two seperate problems.  Or I'm just mistaken about the
second, unlucky blinks and saccades.  Either way, I'm on the way to a 
headache now.

Both do seem to be correlated with areas of HELLO with some of the more 
complicated looking scripts, though.

Re: The unwarranted scrolling assumption

[Eli Zaretskii]

> Date: Wed, 23 Jun 2010 22:11:47 +0100
> From: David De La Harpe Golden <david@harpegolden.net>
> CC: emacs-devel@gnu.org
> 
> On 23/06/10 20:11, Eli Zaretskii wrote:
> 
> 
> > After looking at the code and experimenting a bit, I think there's no
> > other way to prevent recentering on slow machines except set
> > redisplay-dont-pause non-nil.
> 
> [FWIW, my current machine, while not totally new, is a multicore opteron 
> and not especially slow either]

That's strange: I cannot reproduce these problems unless I
artificially slow down redisplay (by adding a few msec of sleep each
time Emacs displays a screen line).  What is the clock speed of your
machine?

> Both do seem to be correlated with areas of HELLO with some of the more 
> complicated looking scripts, though.

It could be that problems in HELLO are due to the actual drawing of
characters, which slows down Emacs even more than I did.

Re: The unwarranted scrolling assumption

[David De La Harpe Golden]

On 25/06/10 16:15, Eli Zaretskii wrote:


> That's strange: I cannot reproduce these problems unless I
> artificially slow down redisplay (by adding a few msec of sleep each
> time Emacs displays a screen line).  What is the clock speed of your
> machine?

2.2GHz. Also bear in mind that it is probably keyboard repeat rate 
dependent - I don't know your keyboard repeat rate.

It's also possible the relative perf between cpu and gfx is different on 
my system.

 > It could be that problems in HELLO are due to the actual drawing of
 > characters, which slows down Emacs even more than I did.

And were you testing on w32 or x11? After all, while the redisplay code 
path is partially shared, the font rendering layer is different between 
the two. I was using the latter, I didn't think to stress that in later 
posts.

Re: The unwarranted scrolling assumption

[Eli Zaretskii]

> Date: Fri, 25 Jun 2010 20:42:05 +0100
> From: David De La Harpe Golden <david@harpegolden.net>
> CC: emacs-devel@gnu.org
> 
> Also bear in mind that it is probably keyboard repeat rate 
> dependent - I don't know your keyboard repeat rate.

I don't know it either, except that it's the highest repeat rate I can
get on Windows XP.

> And were you testing on w32 or x11?

w32.

Re: The unwarranted scrolling assumption

[David De La Harpe Golden]

On 25/06/10 22:07, Eli Zaretskii wrote:
>> Date: Fri, 25 Jun 2010 20:42:05 +0100
>> From: David De La Harpe Golden<david@harpegolden.net>
>> CC: emacs-devel@gnu.org
>>
>> Also bear in mind that it is probably keyboard repeat rate
>> dependent - I don't know your keyboard repeat rate.
>
> I don't know it either, except that it's the highest repeat rate I can
> get on Windows XP.
>

Searching, I see some reports online that the Windows max keyboard 
repeat rate setting is capped low in the GUI and you need to edit a 
registry key:

HKEY_CURRENT_USER\Control Panel\Keyboard\KeyboardSpeed

http://www.pbdr.com/ostips/keyrrate.htm

I don't know if the value is on the exact same scale as the x11 value, 
but if so, note that 30 is a half to a third the speed I tend to use!

Re: The unwarranted scrolling assumption

[Stefan Monnier]

> After looking at the code and experimenting a bit, I think there's no
> other way to prevent recentering on slow machines except set
> redisplay-dont-pause non-nil.  (You could also try playing with
> redisplay-preemption-period, which basically gives you more fine
> granularity than just the ON/OFF you get with redisplay-dont-pause.)

Could explain the reason why you think it can't be done?


        Stefan

Re: The unwarranted scrolling assumption

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: David De La Harpe Golden <david@harpegolden.net>,  emacs-devel@gnu.org
> Date: Thu, 24 Jun 2010 00:18:33 +0200
> 
> > After looking at the code and experimenting a bit, I think there's no
> > other way to prevent recentering on slow machines except set
> > redisplay-dont-pause non-nil.  (You could also try playing with
> > redisplay-preemption-period, which basically gives you more fine
> > granularity than just the ON/OFF you get with redisplay-dont-pause.)
> 
> Could explain the reason why you think it can't be done?

Sorry, my wording was inaccurate and thus misleading.  What I meant to
say was that I see no way of using the try_scrolling optimization
under the conditioned described above, except by setting
redisplay-dont-pause non-nil.  When the window_end_valid flag is nil,
we cannot reuse the current glyph matrix, so we fall back to full
redisplay of the window, and that means displaying point in the middle
of the window.

If avoiding recentering is really important (is it?), I guess we could
try tweaking the full window redisplay code so that it would put point
not in the middle of the window, but rather in the scroll margin.

Is this worth pursuing, and if it is, what values of the various
scroll-* variables should trigger this new behavior?

Re: The unwarranted scrolling assumption

[David De La Harpe Golden]

On 19/06/10 22:17, Eli Zaretskii wrote:
>> Date: Sat, 19 Jun 2010 21:13:23 +0100
>> From: David De La Harpe Golden<david@harpegolden.net>
>>
>> I can still get it to happen easily by ramping up keyboard repeat rate,
>> but it happens quite often on HELLO even at reasonable rates.
>
> Thanks for testing.
>
> HELLO turns on bidi-display-reordering, which affects scrolling
> through problems that are specific to the bidi redisplay.  Can you
> still see recentering if you turn bidi-display-reordering off in
> HELLO, like this:
>
>    M-: (setq bidi-display-reordering nil)
>
> ?

Yes.  And to be clear: it happens on files other than HELLO too, 
including window.c. It just happens particularly often on HELLO.

Aside: HELLO looks distinctly odd around the arabic lines in emacs -Q 
but not with my normal config. Probably independent from the immediate 
scrolling issue and possibly font-dependent, I'll file a separate bug.

bug#6471: Arabic display by emacs -Q on HELLO

[David De La Harpe Golden]

[-- Attachment #1: Type: text/plain, Size: 1648 bytes --]

On 19/06/10 23:16, David De La Harpe Golden wrote:

>
> Aside: HELLO looks distinctly odd around the arabic lines in emacs -Q
> but not with my normal config. Probably independent from the immediate
> scrolling issue and possibly font-dependent, I'll file a separate bug.
>


Yes, emacs -Q is picking this font:

xft:-unknown-Metal-normal-normal-normal-*-13-*-*-*-*-0-iso10646-1

from debian package ttf-arabeyes:

/usr/share/fonts/truetype/ttf-arabeyes/ae_Metal.ttf

and "emacs" this font (in accord with my default font customisation):

xft:-unknown-DejaVu Sans Mono-normal-normal-normal-*-15-*-*-*-m-0-iso10646-1


Actually, I suspect (not that I know arabic) both cases are displaying 
it incorrectly (not composed), but it looks particularly off with the 
former, with the character cell for the character taking up half the 
screen.  This might be partially a problem with the font in question.


[1]
character: ّ (1617, #o3121, #x651)
preferred charset: arabic-iso8859-6 (Right-Hand Part of ISO/IEC 8859/6 
(Latin/Arabic): ISO-IR-127)
        code point: 0x71
            syntax: w 	which means: word
          category: b:Arabic
       buffer code: #xD9 #x91
         file code: ESC #x2C #x47 #x71 (encoded by coding system 
iso-2022-7bit-unix)
           display: by this font (glyph code)
     xft:-unknown-Metal-normal-normal-normal-*-13-*-*-*-*-0-iso10646-1 
(#x21C)

Character code properties: customize what to show
   name: ARABIC SHADDA
   old-name: ARABIC SHADDAH
   general-category: Mn (Mark, Nonspacing)

There are text properties here:
   charset              arabic-iso8859-6

[-- Attachment #2: emacs_q_hello_arabic.png --]
[-- Type: image/png, Size: 62691 bytes --]

[-- Attachment #3: emacs_hello_arabic.png --]
[-- Type: image/png, Size: 79901 bytes --]

bug#6471: Arabic display by emacs -Q on HELLO

[Lars Ingebrigtsen]

David De La Harpe Golden <david@harpegolden.net> writes:

>> Aside: HELLO looks distinctly odd around the arabic lines in emacs -Q
>> but not with my normal config. Probably independent from the immediate
>> scrolling issue and possibly font-dependent, I'll file a separate bug.
>>
>
> Yes, emacs -Q is picking this font:
>
> xft:-unknown-Metal-normal-normal-normal-*-13-*-*-*-*-0-iso10646-1
>
> from debian package ttf-arabeyes:
>
> /usr/share/fonts/truetype/ttf-arabeyes/ae_Metal.ttf
>
> and "emacs" this font (in accord with my default font customisation):
>
> xft:-unknown-DejaVu Sans Mono-normal-normal-normal-*-15-*-*-*-m-0-iso10646-1
>
> Actually, I suspect (not that I know arabic) both cases are displaying
> it incorrectly (not composed), but it looks particularly off with the 
> former, with the character cell for the character taking up half the
> screen.  This might be partially a problem with the font in question.

(I'm going through old bug reports that have unfortunately gotten no
attention yet.)

I tried reproducing this in Emacs 27, but as far as I can tell, the
Arabic text shows up composed for me (emacs -Q is using "Ubuntu Mono")
on my GNU/Linux system.

Are you still seeing this problem, or has it been fixed in the
intervening years?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#6471: Arabic display by emacs -Q on HELLO

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: 6471@debbugs.gnu.org,  Eli Zaretskii <eliz@gnu.org>
> Date: Wed, 21 Aug 2019 15:57:10 -0700
> 
> David De La Harpe Golden <david@harpegolden.net> writes:
> 
> >> Aside: HELLO looks distinctly odd around the arabic lines in emacs -Q
> >> but not with my normal config. Probably independent from the immediate
> >> scrolling issue and possibly font-dependent, I'll file a separate bug.
> >>
> >
> > Yes, emacs -Q is picking this font:
> >
> > xft:-unknown-Metal-normal-normal-normal-*-13-*-*-*-*-0-iso10646-1
> >
> > from debian package ttf-arabeyes:
> >
> > /usr/share/fonts/truetype/ttf-arabeyes/ae_Metal.ttf
> >
> > and "emacs" this font (in accord with my default font customisation):
> >
> > xft:-unknown-DejaVu Sans Mono-normal-normal-normal-*-15-*-*-*-m-0-iso10646-1
> >
> > Actually, I suspect (not that I know arabic) both cases are displaying
> > it incorrectly (not composed), but it looks particularly off with the 
> > former, with the character cell for the character taking up half the
> > screen.  This might be partially a problem with the font in question.
> 
> (I'm going through old bug reports that have unfortunately gotten no
> attention yet.)
> 
> I tried reproducing this in Emacs 27, but as far as I can tell, the
> Arabic text shows up composed for me (emacs -Q is using "Ubuntu Mono")
> on my GNU/Linux system.
> 
> Are you still seeing this problem, or has it been fixed in the
> intervening years?

FWIW, I cannot see any problems, and am almost positive they were
either fixed or were due to a faulty font.

bug#6471: Arabic display by emacs -Q on HELLO

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> FWIW, I cannot see any problems, and am almost positive they were
> either fixed or were due to a faulty font.

OK; closing the bug report.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: The unwarranted scrolling assumption

[Eli Zaretskii]

> Date: Sat, 19 Jun 2010 12:51:20 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: Chong Yidong <cyd@stupidchicken.com>, emacs-devel@gnu.org
> 
> OK.  I installed a change along these lines (revno 100619).

Please use 100620 instead, which fixes a stupid mistake in 100619.

Re: The unwarranted scrolling assumption

[Juanma Barranquero]

On Sat, Jun 19, 2010 at 11:51, Eli Zaretskii <eliz@gnu.org> wrote:

> Would people who customize these variables (and those who use
> scroll-*-aggressively) please see if the new behavior is to their
> liking?

It works for me, but that's not very informative because it was working before.

> Interestingly, this change introduced an asymmetry between scrolling
> down and up: there's no such limitation in the latter case.

IIRC, for some reason scrolling up didn't cause recentering.

> The archives of emacs-devel do not reveal any details beyond the fact
> that this was done to prevent some jumpy scrolling.  Perhaps Yidong
> can shed more light on why this was done back then.

What do you mean? That is the reason for the change: scrolling was
much slower and jumpier than before (this was soon after the unicode2
merge and the new font backend was still quite slow).

    Juanma

Re: The unwarranted scrolling assumption

[Eli Zaretskii]

> From: Juanma Barranquero <lekktu@gmail.com>
> Date: Sat, 19 Jun 2010 21:13:26 +0200
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>, Chong Yidong <cyd@stupidchicken.com>, emacs-devel@gnu.org
> 
> On Sat, Jun 19, 2010 at 11:51, Eli Zaretskii <eliz@gnu.org> wrote:
> 
> > Would people who customize these variables (and those who use
> > scroll-*-aggressively) please see if the new behavior is to their
> > liking?
> 
> It works for me, but that's not very informative because it was working before.

Thanks for testing.

> > The archives of emacs-devel do not reveal any details beyond the fact
> > that this was done to prevent some jumpy scrolling.  Perhaps Yidong
> > can shed more light on why this was done back then.
> 
> What do you mean? That is the reason for the change: scrolling was
> much slower and jumpier than before (this was soon after the unicode2
> merge and the new font backend was still quite slow).

Does the current trunk feel slower and jumpier than the one before the
changes I installed today?

Re: The unwarranted scrolling assumption

[Juanma Barranquero]

On Sat, Jun 19, 2010 at 23:20, Eli Zaretskii <eliz@gnu.org> wrote:

> Does the current trunk feel slower and jumpier than the one before the
> changes I installed today?

No.

    Juanma

Re: The unwarranted scrolling assumption

[Lennart Borgman]

On Fri, Jun 18, 2010 at 9:54 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Lennart Borgman <lennart.borgman@gmail.com>
>> Date: Fri, 18 Jun 2010 00:16:33 +0200
>> Cc: emacs-devel@gnu.org
>>
>> > Again, PLEASE answer my questions, if you really want my help in
>> > understanding this issue.  What parts of the above condition prevented
>> > reconsider_clip_changes from resetting b->clip_changed to zero?  If
>> > needed, please re-run under GDB (without your patches) and see what
>> > other factors are at work here.
>>
>> I can now answer you question again since I have found a new way to
>> make "jumping scrolling" appear. However the situation above is not
>> involved, at least not directly, since ->clip_changed is 0. (I think
>> there is an unused global variable clip_changed that is not used.)
>
> So what you describe is a different situation altogether, and doesn't
> help to understand your original report and the patch that is supposed
> to fix it.


Yes and no.

If you look at my other messages you can see there are two distinct
bugs that I have seen that leads to the "jumping scrolling" (the
problem with clip_changed and the bug regarding visual-line-mode).

To clear things out I think it would be good if you (and anyone
interested) looked at the patch I have sent first. I have tried to
explain why I did this patch so please reread my messages first if
something is unclear.


> Emacs uses the recenter as the last resort, and it does this in many
> different situations.  Discovering another situation where it happens
> does not help to understand the one you discovered before, and the
> patch to correct it (assuming we want that) might be entirely
> different.  This just adds confusion.


I can see that this is confusing if still find my patch unclear.

So at least now it is clear that I think there are two distinct bugs
that leads to "jumping scrolling" (which I think is a better
description than "recentering").


>> The situation is now this:
>> - I am using my patch (in the state I sent it here).
>> - I have seen no problem with the patch (but as I said I think there
>> is one problem though it has not shown up).
>> - It is quite hard to make Emacs do the "jumping scrolling".
>>
>> So it is better, still there are problems.
>
> It may be "better" in the use-cases you tried.  But without
> understanding the original problem and why your patch fixes it, it is
> quite likely that the patch will introduce other redisplay problems.


So you think that the patch may introduce redisplay problems? That
concern is good, but can you seen something in the patch that you
think will cause problems?

Maybe it helps if I explain that the patch is not about how the screen
is updated. It is only about when it is updated. At least that was my
intention.


> try_scrolling may legitimately fail in some cases.


Maybe, but I do not think that is the problem here.


>> BTW, the use of current_buffer here seems to me to be a bug.
>
> No, it isn't a bug.  When redisplay works on a window, current_buffer
> is bound to the buffer displayed by the window.  See the call to
> set_buffer_internal_1 near the beginning of redisplay_window.


Thanks. I missed that. Then it is just a matter of style. I really
think that using current_buffer there is not the best.


>> I guess an interesting part may be why clear_glyph_matrix failed. Do
>> you think there is something interesting there?
>
> I just see normal operation of the display engine.  The condition
>
>  w->cursor.vpos < 0
>
> means that try_window failed to find the cursor position after
> redisplaying the window, because point is not inside the window.
>
> IOW, try_scrolling tried to redisplay the window by reusing some of
> the results of the previous redisplay, but found that doing so would
> produce a window that does not include point.  So it gives up.  This
> is normal.


Thanks, that is good to know, but I think there is a bug in this case.
Can you please reread the message where I talked about how to
reproduce this around line 702 in window.c?

Re: The unwarranted scrolling assumption

[Eli Zaretskii]

> From: Lennart Borgman <lennart.borgman@gmail.com>
> Date: Wed, 16 Jun 2010 19:35:53 +0200
> Cc: "Kim F. Storm" <storm@cua.dk>, emacs-devel@gnu.org,
> 	David De La Harpe Golden <david@harpegolden.net>
> 
> redisplay_window checks this somewhere around line 14500 where the
> comment says "Try to scroll by specified few lines". If clip_changed
> is set then the if arg will be 0 and the "jumping scrolling" will
> happen. At least that is how I understand it at the moment.

Oh, and btw, does the same problem happens in Emacs 23.2?

Re: The unwarranted scrolling assumption

[Lennart Borgman]

On Wed, Jun 16, 2010 at 11:23 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Lennart Borgman <lennart.borgman@gmail.com>
>> Date: Wed, 16 Jun 2010 19:35:53 +0200
>> Cc: "Kim F. Storm" <storm@cua.dk>, emacs-devel@gnu.org,
>>       David De La Harpe Golden <david@harpegolden.net>
>>
>> redisplay_window checks this somewhere around line 14500 where the
>> comment says "Try to scroll by specified few lines". If clip_changed
>> is set then the if arg will be 0 and the "jumping scrolling" will
>> happen. At least that is how I understand it at the moment.
>
> Oh, and btw, does the same problem happens in Emacs 23.2?

I am not sure, but I think so  since I always have been irritated by it.

And I have hardly touched Emacs 23.2 so I do not know, but maybe
someone else can test.

Re: The unwarranted scrolling assumption

[Eli Zaretskii]

> From: Lennart Borgman <lennart.borgman@gmail.com>
> Date: Wed, 16 Jun 2010 03:40:11 +0200
> Cc: emacs-devel@gnu.org
> 
> I wonder if this happens in redisplay_window in xdisp.c. This
> functions has a labeled called "recenter":
> 
>   /* Finally, just choose place to start which centers point */
> 
>  recenter:
>   if (centering_position < 0)
>     centering_position = window_box_height (w) / 2;
> 
> The intent of that label seems to be just trying to came up to speed
> with the input.

No, it's not just to keep up.  Recentering is the generic redisplay
method, used as the last resort if Emacs is unable to be smarter than
that.

> However I dislike the way the problem is solved. I would rather wish
> that the display engine instead calculated a new window start point
> from the input command queue. I have no idea if that is actually
> possible.

I think it's impossible.  How can you guess the effect of list of
arbitrary input events on the display, without actually redisplaying?

> What I can see is that redisplay_window can restart through
> redisplay_internal (the comment at the label "need_larger_matrices"
> says that).

need_larger_matrices means that the dimensions of the window changed
drastically, e.g. if the text uses smaller fonts.  In that case, the
current redisplay cycle is aborted, and Emacs reallocates the glyph
matrices to fit the new dimensions.

> Perhaps this possibility to restart can be used instead.

You will end up with the same recenter anyway, after redisplay is
re-entered.

Re: The unwarranted scrolling assumption

[Lennart Borgman]

On Wed, Jun 16, 2010 at 11:17 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Lennart Borgman <lennart.borgman@gmail.com>
>> Date: Wed, 16 Jun 2010 03:40:11 +0200
>> Cc: emacs-devel@gnu.org
>>
>> I wonder if this happens in redisplay_window in xdisp.c. This
>> functions has a labeled called "recenter":
>>
>>   /* Finally, just choose place to start which centers point */
>>
>>  recenter:
>>   if (centering_position < 0)
>>     centering_position = window_box_height (w) / 2;
>>
>> The intent of that label seems to be just trying to came up to speed
>> with the input.
>
> No, it's not just to keep up.  Recentering is the generic redisplay
> method, used as the last resort if Emacs is unable to be smarter than
> that.

Is not that the same thing here?

>> However I dislike the way the problem is solved. I would rather wish
>> that the display engine instead calculated a new window start point
>> from the input command queue. I have no idea if that is actually
>> possible.
>
> I think it's impossible.  How can you guess the effect of list of
> arbitrary input events on the display, without actually redisplaying?

In a general case this is impossible, but we know more here.

>> What I can see is that redisplay_window can restart through
>> redisplay_internal (the comment at the label "need_larger_matrices"
>> says that).
>
> need_larger_matrices means that the dimensions of the window changed
> drastically, e.g. if the text uses smaller fonts.  In that case, the
> current redisplay cycle is aborted, and Emacs reallocates the glyph
> matrices to fit the new dimensions.

Thanks, yes I saw that. I just wondered if the state was interrupt-able.

>> Perhaps this possibility to restart can be used instead.
>
> You will end up with the same recenter anyway, after redisplay is
> re-entered.

Hm, yes, that is what I wonder. ;-)

I am not sure. I am trying to fix it some way, but there is still
something I do not understand.

Re: The unwarranted scrolling assumption

[Eli Zaretskii]

> From: Lennart Borgman <lennart.borgman@gmail.com>
> Date: Wed, 16 Jun 2010 23:27:33 +0200
> Cc: storm@cua.dk, emacs-devel@gnu.org, david@harpegolden.net
> 
> > No, it's not just to keep up.  Recentering is the generic redisplay
> > method, used as the last resort if Emacs is unable to be smarter than
> > that.
> 
> Is not that the same thing here?

Sorry, I don't understand.

> >> However I dislike the way the problem is solved. I would rather wish
> >> that the display engine instead calculated a new window start point
> >> from the input command queue. I have no idea if that is actually
> >> possible.
> >
> > I think it's impossible.  How can you guess the effect of list of
> > arbitrary input events on the display, without actually redisplaying?
> 
> In a general case this is impossible, but we know more here.

What do we know here?

Re: The unwarranted scrolling assumption

[Stefan Monnier]

>> However I dislike the way the problem is solved. I would rather wish
>> that the display engine instead calculated a new window start point
>> from the input command queue. I have no idea if that is actually
>> possible.
> I think it's impossible.  How can you guess the effect of list of
> arbitrary input events on the display, without actually redisplaying?

Indeed what he suggests is probably impossible.  But the end result he's
after is definitely possible: move the window-start as little as possible
(tho still, line by line) to keep point displayed.


        Stefan

Re: The unwarranted scrolling assumption

[Lennart Borgman]

On Thu, Jun 17, 2010 at 2:54 AM, Stefan Monnier
<monnier@iro.umontreal.ca> wrote:
>>> However I dislike the way the problem is solved. I would rather wish
>>> that the display engine instead calculated a new window start point
>>> from the input command queue. I have no idea if that is actually
>>> possible.
>> I think it's impossible.  How can you guess the effect of list of
>> arbitrary input events on the display, without actually redisplaying?
>
> Indeed what he suggests is probably impossible.  But the end result he's
> after is definitely possible: move the window-start as little as possible
> (tho still, line by line) to keep point displayed.


Though it is not easy to understand what is happening.

I believed I safely could assume that save_restriction_save and
save_restriction_restore always was called in pair. Is not that the
case?

I do not understand the output I see. I added a counter, initialized
to 0, that is incremented when entering save_restriction_save and
decremented when exiting ..._restore. It looks like it becomes -1
right at the end before "jumping scrolling". Maybe this is just my bad
C fu. Can I do

  DebPrint (("%d", current_buffer));

where current_buffer is a Lisp_Object? Or does that result in some overflow?

Re: The unwarranted scrolling assumption

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Lennart Borgman <lennart.borgman@gmail.com>,  storm@cua.dk,  emacs-devel@gnu.org,  david@harpegolden.net
> Date: Wed, 16 Jun 2010 20:54:52 -0400
> 
> Indeed what he suggests is probably impossible.  But the end result he's
> after is definitely possible: move the window-start as little as possible
> (tho still, line by line) to keep point displayed.

xdisp.c already tries to do that, we just need to find out why it
fails in this particular case.

Re: The unwarranted scrolling assumption

[Lennart Borgman]

On Thu, Jun 17, 2010 at 5:05 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Stefan Monnier <monnier@iro.umontreal.ca>
>> Cc: Lennart Borgman <lennart.borgman@gmail.com>,  storm@cua.dk,  emacs-devel@gnu.org,  david@harpegolden.net
>> Date: Wed, 16 Jun 2010 20:54:52 -0400
>>
>> Indeed what he suggests is probably impossible.  But the end result he's
>> after is definitely possible: move the window-start as little as possible
>> (tho still, line by line) to keep point displayed.
>
> xdisp.c already tries to do that, we just need to find out why it
> fails in this particular case.

After trying to restore the clip_changed value when save-restriction,
widen etc unnecessarily changed I suddenly wonder why they changed it.
Is not this something that only redisplay should handle? redisplay_can
easily save the clipping when it sets clip_changed to 0 and compare
with that when it enters redisplay_window and set clip_changed to 1 if
the saved clipping does not match.

Does that sound reasonable? (I do not know if I am missing something.)

Re: The unwarranted scrolling assumption

[Lennart Borgman]

On Thu, Jun 17, 2010 at 5:10 AM, Lennart Borgman
<lennart.borgman@gmail.com> wrote:
> On Thu, Jun 17, 2010 at 5:05 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>>> From: Stefan Monnier <monnier@iro.umontreal.ca>
>>> Cc: Lennart Borgman <lennart.borgman@gmail.com>,  storm@cua.dk,  emacs-devel@gnu.org,  david@harpegolden.net
>>> Date: Wed, 16 Jun 2010 20:54:52 -0400
>>>
>>> Indeed what he suggests is probably impossible.  But the end result he's
>>> after is definitely possible: move the window-start as little as possible
>>> (tho still, line by line) to keep point displayed.
>>
>> xdisp.c already tries to do that, we just need to find out why it
>> fails in this particular case.
>
> After trying to restore the clip_changed value when save-restriction,
> widen etc unnecessarily changed I suddenly wonder why they changed it.
> Is not this something that only redisplay should handle? redisplay_can
> easily save the clipping when it sets clip_changed to 0 and compare
> with that when it enters redisplay_window and set clip_changed to 1 if
> the saved clipping does not match.
>
> Does that sound reasonable? (I do not know if I am missing something.)

At least it works. I just tested. First time I am using Emacs without
"jumping scrolling"! :-)

If you think this is the right way to fix it I will send patches later
when I cleaned up a bit.

There is one more similar problem which I do not know how to handle,
in fileio.c. In the function Finsert_file_contents clip_changed is set
to 1. I am not quite sure of the impact of this but it looks wrong to
me to do this. I think the display engine should be told, but it must
decide what to do with redisplay, not fileio.

Re: The unwarranted scrolling assumption

[Lennart Borgman]

[-- Attachment #1: Type: text/plain, Size: 1937 bytes --]

On Thu, Jun 17, 2010 at 5:59 AM, Lennart Borgman
<lennart.borgman@gmail.com> wrote:
> On Thu, Jun 17, 2010 at 5:10 AM, Lennart Borgman
> <lennart.borgman@gmail.com> wrote:
>> On Thu, Jun 17, 2010 at 5:05 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>>>> From: Stefan Monnier <monnier@iro.umontreal.ca>
>>>> Cc: Lennart Borgman <lennart.borgman@gmail.com>,  storm@cua.dk,  emacs-devel@gnu.org,  david@harpegolden.net
>>>> Date: Wed, 16 Jun 2010 20:54:52 -0400
>>>>
>>>> Indeed what he suggests is probably impossible.  But the end result he's
>>>> after is definitely possible: move the window-start as little as possible
>>>> (tho still, line by line) to keep point displayed.
>>>
>>> xdisp.c already tries to do that, we just need to find out why it
>>> fails in this particular case.
>>
>> After trying to restore the clip_changed value when save-restriction,
>> widen etc unnecessarily changed I suddenly wonder why they changed it.
>> Is not this something that only redisplay should handle? redisplay_can
>> easily save the clipping when it sets clip_changed to 0 and compare
>> with that when it enters redisplay_window and set clip_changed to 1 if
>> the saved clipping does not match.
>>
>> Does that sound reasonable? (I do not know if I am missing something.)
>
> At least it works. I just tested. First time I am using Emacs without
> "jumping scrolling"! :-)
>
> If you think this is the right way to fix it I will send patches later
> when I cleaned up a bit.
>
> There is one more similar problem which I do not know how to handle,
> in fileio.c. In the function Finsert_file_contents clip_changed is set
> to 1. I am not quite sure of the impact of this but it looks wrong to
> me to do this. I think the display engine should be told, but it must
> decide what to do with redisplay, not fileio.

Here is the patch (without the fileio.c part which I leave until I got
some comments).

[-- Attachment #2: buffer-h-jumpscroll-0.diff --]
[-- Type: application/octet-stream, Size: 514 bytes --]

=== modified file 'src/buffer.h'
--- trunk/src/buffer.h	2010-04-20 17:01:39 +0000
+++ patched/src/buffer.h	2010-06-17 04:48:01 +0000
@@ -541,6 +541,9 @@
 
   /* Set nonzero whenever the narrowing is changed in this buffer.  */
   int clip_changed;
+  /* Set to the narrowing in effect when clip_changed is set to 0. */
+  EMACS_INT clip_begv;
+  EMACS_INT clip_zv;
 
   /* If the long line scan cache is enabled (i.e. the buffer-local
      variable cache-long-line-scans is non-nil), newline_cache


[-- Attachment #3: editfns-jumpscroll-0.diff --]
[-- Type: application/octet-stream, Size: 1508 bytes --]

=== modified file 'src/editfns.c'
--- trunk/src/editfns.c	2010-06-10 05:31:36 +0000
+++ patched/src/editfns.c	2010-06-17 04:07:10 +0000
@@ -3215,12 +3221,13 @@
      ()
 {
   if (BEG != BEGV || Z != ZV)
-    current_buffer->clip_changed = 1;
+    {
   BEGV = BEG;
   BEGV_BYTE = BEG_BYTE;
   SET_BUF_ZV_BOTH (current_buffer, Z, Z_BYTE);
   /* Changing the buffer bounds invalidates any recorded current column.  */
   invalidate_current_column ();
+    }
   return Qnil;
 }
 
@@ -3249,8 +3256,7 @@
     args_out_of_range (start, end);
 
   if (BEGV != XFASTINT (start) || ZV != XFASTINT (end))
-    current_buffer->clip_changed = 1;
-
+    {
   SET_BUF_BEGV (current_buffer, XFASTINT (start));
   SET_BUF_ZV (current_buffer, XFASTINT (end));
   if (PT < XFASTINT (start))
@@ -3259,6 +3265,7 @@
     SET_PT (XFASTINT (end));
   /* Changing the buffer bounds invalidates any recorded current column.  */
   invalidate_current_column ();
+    }
   return Qnil;
 }
 
@@ -3327,8 +3334,6 @@
 			     clip_to_bounds (beg->charpos, pt, end->charpos),
 			     clip_to_bounds (beg->bytepos, BUF_PT_BYTE (buf),
 					     end->bytepos));
-
-	  buf->clip_changed = 1; /* Remember that the narrowing changed. */
 	}
     }
   else
@@ -3340,8 +3345,6 @@
 	{
 	  SET_BUF_BEGV_BOTH (buf, BUF_BEG (buf), BUF_BEG_BYTE (buf));
 	  SET_BUF_ZV_BOTH (buf, BUF_Z (buf), BUF_Z_BYTE (buf));
-
-	  buf->clip_changed = 1; /* Remember that the narrowing changed. */
 	}
     }
 


[-- Attachment #4: xdisp-jumpscroll-0.diff --]
[-- Type: application/octet-stream, Size: 822 bytes --]

=== modified file 'src/xdisp.c'
--- trunk/src/xdisp.c	2010-06-01 02:34:49 +0000
+++ patched/src/xdisp.c	2010-06-17 04:14:21 +0000
@@ -11716,7 +11716,11 @@
 	   && w->current_matrix->buffer == b
 	   && w->current_matrix->zv == BUF_ZV (b)
 	   && w->current_matrix->begv == BUF_BEGV (b))
+    {
     b->clip_changed = 0;
+      b->clip_begv = b->begv;
+      b->clip_zv = b->zv;
+    }
 
   /* If display wasn't paused, and W is not a tool bar window, see if
      point has been moved into or out of a composition.  In that case,
@@ -14055,6 +14059,9 @@
   int last_line_misfit = 0;
   int beg_unchanged, end_unchanged;
 
+  if ((buffer->clip_begv != buffer->begv) || (buffer->clip_zv != buffer->zv))
+    buffer->clip_changed = 1;
+
   SET_TEXT_POS (lpoint, PT, PT_BYTE);
   opoint = lpoint;
 

Re: The unwarranted scrolling assumption

[Eli Zaretskii]

> From: Lennart Borgman <lennart.borgman@gmail.com>
> Date: Thu, 17 Jun 2010 06:55:17 +0200
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>, storm@cua.dk, emacs-devel@gnu.org, 
> 	david@harpegolden.net
> 
> > At least it works. I just tested. First time I am using Emacs without
> > "jumping scrolling"! :-)
> >
> > If you think this is the right way to fix it I will send patches later
> > when I cleaned up a bit.
> >
> > There is one more similar problem which I do not know how to handle,
> > in fileio.c. In the function Finsert_file_contents clip_changed is set
> > to 1. I am not quite sure of the impact of this but it looks wrong to
> > me to do this. I think the display engine should be told, but it must
> > decide what to do with redisplay, not fileio.
> 
> Here is the patch (without the fileio.c part which I leave until I got
> some comments).

This patch is not correct, IMO, because it does not consider the value
of the window's window_end_valid flag.  The function
reconsider_clip_changes, which you try to side-step, does the same as
this test in your patch:

> +  if ((buffer->clip_begv != buffer->begv) || (buffer->clip_zv != buffer->zv))
> +    buffer->clip_changed = 1;

but it also considers the window_end_valid flag, without which your
patched version could try reusing portions of the current glyph matrix
when the matrix is not guaranteed to be up to date.

Re: The unwarranted scrolling assumption

[Lennart Borgman]

On Fri, Jun 18, 2010 at 3:45 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Lennart Borgman <lennart.borgman@gmail.com>
>> Date: Thu, 17 Jun 2010 06:55:17 +0200
>> Cc: Stefan Monnier <monnier@iro.umontreal.ca>, storm@cua.dk, emacs-devel@gnu.org,
>>       david@harpegolden.net
>>
>> > At least it works. I just tested. First time I am using Emacs without
>> > "jumping scrolling"! :-)
>> >
>> > If you think this is the right way to fix it I will send patches later
>> > when I cleaned up a bit.
>> >
>> > There is one more similar problem which I do not know how to handle,
>> > in fileio.c. In the function Finsert_file_contents clip_changed is set
>> > to 1. I am not quite sure of the impact of this but it looks wrong to
>> > me to do this. I think the display engine should be told, but it must
>> > decide what to do with redisplay, not fileio.
>>
>> Here is the patch (without the fileio.c part which I leave until I got
>> some comments).
>
> This patch is not correct, IMO, because it does not consider the value
> of the window's window_end_valid flag.  The function
> reconsider_clip_changes, which you try to side-step, does the same as
> this test in your patch:
>
>> +  if ((buffer->clip_begv != buffer->begv) || (buffer->clip_zv != buffer->zv))
>> +    buffer->clip_changed = 1;
>
> but it also considers the window_end_valid flag, without which your
> patched version could try reusing portions of the current glyph matrix
> when the matrix is not guaranteed to be up to date.


I think you are wrong here, but I am not sure.

What I have done in the patch is trying to avoid setting clip_changed
whenever `narrow_to_region' or `widen' is called. I do this by
comparing clipping the next time redisplay happens with the clipping
that was in effect the last redisplay happened.

Can you please explain why window_end_valid flag should be involved?

Can you please also comment on the point where to check if clipping
has changed? Is not the entry of redisplay_window the right point for
this?

Re: The unwarranted scrolling assumption

[Eli Zaretskii]

> From: Lennart Borgman <lennart.borgman@gmail.com>
> Date: Fri, 18 Jun 2010 15:56:38 +0200
> Cc: david@harpegolden.net, monnier@iro.umontreal.ca, emacs-devel@gnu.org
> 
> Can you please explain why window_end_valid flag should be involved?

This flag indicates that the redisplay finished and the current glyph
matrix is up to date.  It is imperative to know that, because
try_scrolling, the function you are trying to get called, reuses
portions of the current glyph matrix (to avoid redrawing the entire
window).  It is unsafe to reuse the current matrix if it is not
guaranteed to be up to date.

> Can you please also comment on the point where to check if clipping
> has changed? Is not the entry of redisplay_window the right point for
> this?

What do you mean by ``where to check if clipping has changed''?  The
clip_changed flag is checked in many places during redisplay.

Re: The unwarranted scrolling assumption

[Lennart Borgman]

On Fri, Jun 18, 2010 at 4:20 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Lennart Borgman <lennart.borgman@gmail.com>
>> Date: Fri, 18 Jun 2010 15:56:38 +0200
>> Cc: david@harpegolden.net, monnier@iro.umontreal.ca, emacs-devel@gnu.org
>>
>> Can you please explain why window_end_valid flag should be involved?
>
> This flag indicates that the redisplay finished and the current glyph
> matrix is up to date.  It is imperative to know that, because
> try_scrolling, the function you are trying to get called, reuses
> portions of the current glyph matrix (to avoid redrawing the entire
> window).  It is unsafe to reuse the current matrix if it is not
> guaranteed to be up to date.


Thanks, that is a clear description.

But how is that involved here? I save the old clipping in
reconsider_clip_changes when window_end_valid is non-nil. (Or do I
misread the code there because of the bans I have that forbid me to
read C code corretly?)


>> Can you please also comment on the point where to check if clipping
>> has changed? Is not the entry of redisplay_window the right point for
>> this?
>
> What do you mean by ``where to check if clipping has changed''?  The
> clip_changed flag is checked in many places during redisplay.


Sorry, I was unclear. I meant where it is set to 1. narrow_to_region
etc set it to 1 and that is not something they should do since they do
not (and should not) know about the redisplay state.

Re: The unwarranted scrolling assumption

[Eli Zaretskii]

> From: Lennart Borgman <lennart.borgman@gmail.com>
> Date: Fri, 18 Jun 2010 16:32:07 +0200
> Cc: emacs-devel@gnu.org
> 
> On Fri, Jun 18, 2010 at 4:20 PM, Eli Zaretskii <eliz@gnu.org> wrote:
> >> From: Lennart Borgman <lennart.borgman@gmail.com>
> >> Date: Fri, 18 Jun 2010 15:56:38 +0200
> >> Cc: david@harpegolden.net, monnier@iro.umontreal.ca, emacs-devel@gnu.org
> >>
> >> Can you please explain why window_end_valid flag should be involved?
> >
> > This flag indicates that the redisplay finished and the current glyph
> > matrix is up to date.  It is imperative to know that, because
> > try_scrolling, the function you are trying to get called, reuses
> > portions of the current glyph matrix (to avoid redrawing the entire
> > window).  It is unsafe to reuse the current matrix if it is not
> > guaranteed to be up to date.
> 
> 
> Thanks, that is a clear description.
> 
> But how is that involved here? I save the old clipping in
> reconsider_clip_changes when window_end_valid is non-nil.

You save it when window_end_valid is non-nil, but you use it without
testing window_end_valid.

> Sorry, I was unclear. I meant where it is set to 1. narrow_to_region
> etc set it to 1 and that is not something they should do since they do
> not (and should not) know about the redisplay state.

This flag means that narrowing was changed since the last redisplay.
So yes, setting it in narrow_to_region is the right thing.

Re: The unwarranted scrolling assumption

[Eli Zaretskii]

> From: Lennart Borgman <lennart.borgman@gmail.com>
> Date: Thu, 17 Jun 2010 05:59:12 +0200
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>, storm@cua.dk, emacs-devel@gnu.org, 
> 	david@harpegolden.net
> 
> > After trying to restore the clip_changed value when save-restriction,
> > widen etc unnecessarily changed I suddenly wonder why they changed it.
> > Is not this something that only redisplay should handle? redisplay_can
> > easily save the clipping when it sets clip_changed to 0 and compare
> > with that when it enters redisplay_window and set clip_changed to 1 if
> > the saved clipping does not match.
> >
> > Does that sound reasonable? (I do not know if I am missing something.)
> 
> At least it works. I just tested. First time I am using Emacs without
> "jumping scrolling"! :-)
> 
> If you think this is the right way to fix it I will send patches later
> when I cleaned up a bit.

You are posting conclusions without supporting data, and patches based
on those conclusions.  You also didn't answer the questions I asked.
Sorry, I cannot help you like that.

At the very least, please post the shortest way of reproducing the
original problem starting with "emacs -Q".  Then, maybe, I will be
able to figure this out myself.

Re: The unwarranted scrolling assumption

[Lennart Borgman]

On Thu, Jun 17, 2010 at 8:25 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Lennart Borgman <lennart.borgman@gmail.com>
>> Date: Thu, 17 Jun 2010 05:59:12 +0200
>> Cc: Stefan Monnier <monnier@iro.umontreal.ca>, storm@cua.dk, emacs-devel@gnu.org,
>>       david@harpegolden.net
>>
>> > After trying to restore the clip_changed value when save-restriction,
>> > widen etc unnecessarily changed I suddenly wonder why they changed it.
>> > Is not this something that only redisplay should handle? redisplay_can
>> > easily save the clipping when it sets clip_changed to 0 and compare
>> > with that when it enters redisplay_window and set clip_changed to 1 if
>> > the saved clipping does not match.
>> >
>> > Does that sound reasonable? (I do not know if I am missing something.)
>>
>> At least it works. I just tested. First time I am using Emacs without
>> "jumping scrolling"! :-)
>>
>> If you think this is the right way to fix it I will send patches later
>> when I cleaned up a bit.
>
> You are posting conclusions without supporting data, and patches based
> on those conclusions.  You also didn't answer the questions I asked.
> Sorry, I cannot help you like that.
>
> At the very least, please post the shortest way of reproducing the
> original problem starting with "emacs -Q".  Then, maybe, I will be
> able to figure this out myself.

Thanks, but I do not understand what you mean. Several of us has said
that the problem is easily reproduceable.

The conclusions are based on the logic of the source code. But you
seem to think they are unwarranted. Can you please explain what part
of the patches you think are unwarranted or unexplained?

Re: The unwarranted scrolling assumption

[Eli Zaretskii]

> From: Lennart Borgman <lennart.borgman@gmail.com>
> Date: Thu, 17 Jun 2010 20:32:27 +0200
> Cc: emacs-devel@gnu.org, david@harpegolden.net
> 
> Several of us has said that the problem is easily reproduceable.

Please show a recipe starting from "emacs -Q".

> The conclusions are based on the logic of the source code. But you
> seem to think they are unwarranted. Can you please explain what part
> of the patches you think are unwarranted or unexplained?

I cannot explain anything yet, because I cannot follow your reasoning
and thus cannot assess the changes you suggest.  That's why I asked
the questions you didn't answer.

Re: The unwarranted scrolling assumption

[Lennart Borgman]

On Thu, Jun 17, 2010 at 8:43 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Lennart Borgman <lennart.borgman@gmail.com>
>> Date: Thu, 17 Jun 2010 20:32:27 +0200
>> Cc: emacs-devel@gnu.org, david@harpegolden.net
>>
>> Several of us has said that the problem is easily reproduceable.
>
> Please show a recipe starting from "emacs -Q".

I am sorry, I thought that was clear. Just open a large C file for
example, like window.c. Set the variables as suggested earlier in this
thread that should prevent "jumping scrolling".

Then just hold down or rapidly press the down arrow. You will see (if
your pc is not too fast) that Emacs does the "jumping scrolling" now
and then. Not every time, but now and then, probably when it gets
behind in screen updating.

>> The conclusions are based on the logic of the source code. But you
>> seem to think they are unwarranted. Can you please explain what part
>> of the patches you think are unwarranted or unexplained?
>
> I cannot explain anything yet, because I cannot follow your reasoning
> and thus cannot assess the changes you suggest.  That's why I asked
> the questions you didn't answer.

Hi Eli,

I will try to explain, but a problem for me is that I do not know
where to start when explaining. You did not comment on the explanation
I gave with the patches. So I just do it the best I can. Please just
tell me what you do not understand.


I said that clip_changed is used only by the display routines. Only
those routines knows exactly what data was used when redisplay was
done.

Clipping is part of the data that might influence redisplay. Changing
clipping might invalidate what is displayed or it may not.

In the current code (without my patch) there is a very rough guess
about this: "if clipping is change we should invalidate the display of
every window".

So this is what `narrow-to-region' and `widen' did by setting clip_changed to 1.

I removed that guess and made another, a little bit less rough guess:
"if clipping is not the same as when last redisplay was done we should
invalidate the display".

This is (nearly) on the safe side. It invalidate the display even if
the changes in clipping actually does not need to invalidate the
display.


If something above is unclear then please ask. I now have questions to you.

As you can see above there are two a bit weak points in the last
paragraph of my explanation. The last part is a bit more easy so I
start with that. Is there an easy way to find out if the change in the
clipping data did not invalidate the display? That could be great.

Now to the more problematic part. This patch is not yet finished
because there is one thing that I have forgotten to fix after testing:
I have implemented this in redisplay_window, i.e. the cashing of the
clipping should have been per window, but it is per buffer at the
moment.

I guess that this is quite easy to fix by moving the cashing of the
clipping data to the window instead. However when I came to think
about this I wondered if the display of a buffer in several windows is
in some (magic) way connected. I don't think they are, but anyway I
ask you: are they?

Re: The unwarranted scrolling assumption

[Eli Zaretskii]

> From: Lennart Borgman <lennart.borgman@gmail.com>
> Date: Thu, 17 Jun 2010 21:10:55 +0200
> Cc: emacs-devel@gnu.org, david@harpegolden.net
> 
> > Please show a recipe starting from "emacs -Q".
> 
> I am sorry, I thought that was clear. Just open a large C file for
> example, like window.c. Set the variables as suggested earlier in this
> thread that should prevent "jumping scrolling".
> 
> Then just hold down or rapidly press the down arrow. You will see (if
> your pc is not too fast) that Emacs does the "jumping scrolling" now
> and then. Not every time, but now and then, probably when it gets
> behind in screen updating.

It doesn't jump for me.  And, if the machine is too slow and cannot
keep up with the keyboard's autorepeat rate, then what's wrong with
the "jumps"?  What would you want Emacs to do instead, if it cannot
keep up with the input?

> I said that clip_changed is used only by the display routines. Only
> those routines knows exactly what data was used when redisplay was
> done.
> 
> Clipping is part of the data that might influence redisplay. Changing
> clipping might invalidate what is displayed or it may not.
> 
> In the current code (without my patch) there is a very rough guess
> about this: "if clipping is change we should invalidate the display of
> every window".
> 
> So this is what `narrow-to-region' and `widen' did by setting clip_changed to 1.

But narrow-to-region and widen are not part of the recipe, so how can
they influence scrolling in this case?

> However when I came to think about this I wondered if the display of
> a buffer in several windows is in some (magic) way connected. I
> don't think they are, but anyway I ask you: are they?

It is connected via the buffer-local variables.  I'm not sure this is
what you wanted to hear, but your question is too broad.

Anyway, I don't think it's an efficient way of debugging this -- by
you asking me questions based on some assumptions which I'm not sure
are correct.  Let's stick to trying to understand why scrolling is
rejected by redisplay in favor of recentering (see my other mail).

Re: The unwarranted scrolling assumption

[Lennart Borgman]

On Thu, Jun 17, 2010 at 9:23 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Lennart Borgman <lennart.borgman@gmail.com>
>> Date: Thu, 17 Jun 2010 21:10:55 +0200
>> Cc: emacs-devel@gnu.org, david@harpegolden.net
>>
>> > Please show a recipe starting from "emacs -Q".
>>
>> I am sorry, I thought that was clear. Just open a large C file for
>> example, like window.c. Set the variables as suggested earlier in this
>> thread that should prevent "jumping scrolling".
>>
>> Then just hold down or rapidly press the down arrow. You will see (if
>> your pc is not too fast) that Emacs does the "jumping scrolling" now
>> and then. Not every time, but now and then, probably when it gets
>> behind in screen updating.
>
> It doesn't jump for me.

Good for you, but why do you say that? Why is that interesting here???

> And, if the machine is too slow and cannot
> keep up with the keyboard's autorepeat rate, then what's wrong with
> the "jumps"?  What would you want Emacs to do instead, if it cannot
> keep up with the input?

Just do as it does with my patch. The same as for all other apps I
know. Continue scrolling without jumping. (At the speed it can of
course.)

>> I said that clip_changed is used only by the display routines. Only
>> those routines knows exactly what data was used when redisplay was
>> done.
>>
>> Clipping is part of the data that might influence redisplay. Changing
>> clipping might invalidate what is displayed or it may not.
>>
>> In the current code (without my patch) there is a very rough guess
>> about this: "if clipping is change we should invalidate the display of
>> every window".
>>
>> So this is what `narrow-to-region' and `widen' did by setting clip_changed to 1.
>
> But narrow-to-region and widen are not part of the recipe, so how can
> they influence scrolling in this case?

What do you mean? They are both used in Emacs could and there is a lot
of calls to them during scrolling.

Probably that is from font locking.

>> However when I came to think about this I wondered if the display of
>> a buffer in several windows is in some (magic) way connected. I
>> don't think they are, but anyway I ask you: are they?
>
> It is connected via the buffer-local variables.  I'm not sure this is
> what you wanted to hear, but your question is too broad.

Sorry. I meant if there was something special in the display routines
themselves for the connection.

I doubt that there are, but there could be some optimization.

> Anyway, I don't think it's an efficient way of debugging this -- by
> you asking me questions based on some assumptions which I'm not sure
> are correct.  Let's stick to trying to understand why scrolling is
> rejected by redisplay in favor of recentering (see my other mail).

I have already explained that. Can you please before you suggest new
things try to understand what I have written. Just ask if you do not
understand.

Re: The unwarranted scrolling assumption

[Eli Zaretskii]

> From: Lennart Borgman <lennart.borgman@gmail.com>
> Date: Thu, 17 Jun 2010 21:31:07 +0200
> Cc: storm@cua.dk, emacs-devel@gnu.org, david@harpegolden.net
> 
> > It doesn't jump for me.
> 
> Good for you, but why do you say that? Why is that interesting here???

Because it means I cannot reproduce the problem myself and must rely
on your help to understand what is wrong.

> > But narrow-to-region and widen are not part of the recipe, so how can
> > they influence scrolling in this case?
> 
> What do you mean? They are both used in Emacs could and there is a lot
> of calls to them during scrolling.
> 
> Probably that is from font locking.

If you disable font lock, does the problem go away?

Re: The unwarranted scrolling assumption

[Lennart Borgman]

On Fri, Jun 18, 2010 at 9:36 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>>
>> > It doesn't jump for me.
>>
>> Good for you, but why do you say that? Why is that interesting here???
>
> Because it means I cannot reproduce the problem myself and must rely
> on your help to understand what is wrong.


OK, fine. It just looked as you thought it was not a problem. My
excuse if that was not the case.


>> > But narrow-to-region and widen are not part of the recipe, so how can
>> > they influence scrolling in this case?
>>
>> What do you mean? They are both used in Emacs could and there is a lot
>> of calls to them during scrolling.
>>
>> Probably that is from font locking.
>
> If you disable font lock, does the problem go away?


Yes.

That is expected and that is what the patch I sent so far is about.

Re: The unwarranted scrolling assumption

[David Engster]

Eli Zaretskii writes:
>> From: Lennart Borgman <lennart.borgman@gmail.com>
>> Date: Thu, 17 Jun 2010 21:10:55 +0200
>> Cc: emacs-devel@gnu.org, david@harpegolden.net
>> 
>> > Please show a recipe starting from "emacs -Q".
>> 
>> I am sorry, I thought that was clear. Just open a large C file for
>> example, like window.c. Set the variables as suggested earlier in this
>> thread that should prevent "jumping scrolling".
>> 
>> Then just hold down or rapidly press the down arrow. You will see (if
>> your pc is not too fast) that Emacs does the "jumping scrolling" now
>> and then. Not every time, but now and then, probably when it gets
>> behind in screen updating.
>
> It doesn't jump for me.  And, if the machine is too slow and cannot
> keep up with the keyboard's autorepeat rate, then what's wrong with
> the "jumps"?  What would you want Emacs to do instead, if it cannot
> keep up with the input?

Well, you got a point there. I remember that for some time I simply used
something like

(global-set-key (kbd "<down>")
                  (lambda () (interactive)
                    (progn (next-line) (redisplay t))))

which also prevents the jumping. But obviously, if you do that, Emacs
will start to lag behind the input, so that it continues scrolling even
when you're no longer pressing the key, which is worse than jumping.

What I would like is that the input is somehow limited to Emacs'
speed, but maybe this simply isn't possible?

-David

Re: The unwarranted scrolling assumption

[Chad Brown]

On Jun 17, 2010, at 1:17 PM, David Engster wrote:

>> It doesn't jump for me.  And, if the machine is too slow and cannot
>> keep up with the keyboard's autorepeat rate, then what's wrong with
>> the "jumps"?  What would you want Emacs to do instead, if it cannot
>> keep up with the input?
> 
> Well, you got a point there. I remember that for some time I simply used
> something like
> [...]
> 
> What I would like is that the input is somehow limited to Emacs'
> speed, but maybe this simply isn't possible?

This is the question I was trying to ask Lennart also, and I'm glad to see
both progress on an improvement and a return to the core question.  As 
I understand your answer, you're suggesting that somehow users not be 
able to press the down key faster than emacs can scroll, which seems 
like a non-starter to me.  

Many of these systems were first designed in the days where people would 
set a baud rate for the terminal connecting to the machine running emacs,
and then refined or replaced over time.  I doubt many people still use emacs
via a vt100 and a 1400 baud modem (as I once did regularly), but especially
in the era of netbooks and smartphones, we can't expect the display system 
to *always* be faster than the user.  Speeding up in the normal cases (like I 
assume yours and Lennart's) of a relatively fast local machine is a great goal,
but we do have to plan for some method of dealing with `slow' systems also.

It is almost certainly possible to add a heuristic method for emacs to throw
throw away input if it gets too far ahead of redisplay, although those code 
paths seem pretty seriously forbidding to me.  Would this be better?  Would
you want it to throw away all further input, or just scrolling?  It seems hard to 
imagine a situation in which you'd want to throw away scrolling and yet keep
text-modifying commands without somehow recognizing absolute-movement
commands (like end-of-buffer), but for that to work you need a lookahead into 
the input stream.  Such systems would need to be optional, and you'd probably 
really want a way for emacs to indicate that it had thrown away some of your
input (although maybe you could dispense with that if you could convince 
your heuristic that the `middle' scrolling was a no-op).

Hope this helps,
*Chad

Re: The unwarranted scrolling assumption

[David Engster]

Chad Brown writes:
> On Jun 17, 2010, at 1:17 PM, David Engster wrote:
>> What I would like is that the input is somehow limited to Emacs'
>> speed, but maybe this simply isn't possible?

> It is almost certainly possible to add a heuristic method for emacs to throw
> throw away input if it gets too far ahead of redisplay, although those code 
> paths seem pretty seriously forbidding to me.  Would this be better?  Would
> you want it to throw away all further input, or just scrolling?  It seems hard to 
> imagine a situation in which you'd want to throw away scrolling and yet keep
> text-modifying commands without somehow recognizing absolute-movement
> commands (like end-of-buffer), but for that to work you need a lookahead into 
> the input stream.  Such systems would need to be optional, and you'd probably 
> really want a way for emacs to indicate that it had thrown away some of your
> input (although maybe you could dispense with that if you could convince 
> your heuristic that the `middle' scrolling was a no-op).

Yes, I see that this isn't really feasible. I really don't know enough
of the Emacs display engine to make any suggestions how this problem
might be tackled. I know that Emacs is special in this regard since it
enforces that the point position is always visible, and that scrolling
is actually more of a side-effect. However, in this case, I want the
opposite: I first and foremost want to scroll the buffer, with point
movement being a side-effect. I know that I can use the scroll-bars, but
their scrolling behavior is far from smooth in large buffers (and I have
them disabled anyway).

-David

Re: The unwarranted scrolling assumption

[Juanma Barranquero]

On Thu, Jun 17, 2010 at 22:17, David Engster <deng@randomsample.de> wrote:

> But obviously, if you do that, Emacs
> will start to lag behind the input, so that it continues scrolling even
> when you're no longer pressing the key, which is worse than jumping.

No, not to me. YMMV.

    Juanma

Re: The unwarranted scrolling assumption

[Juanma Barranquero]

On Thu, Jun 17, 2010 at 21:23, Eli Zaretskii <eliz@gnu.org> wrote:

> It doesn't jump for me.  And, if the machine is too slow and cannot
> keep up with the keyboard's autorepeat rate, then what's wrong with
> the "jumps"?

For some people (me included), the jumps are the worst answer posible.
I don't want my Emacs to automatically recenter on scroll. Ever.
Fortunately, I have a reasonably fast PC.

> What would you want Emacs to do instead, if it cannot
> keep up with the input?

Stop reading input until it catches its breath.

    Juanma

Re: The unwarranted scrolling assumption

[Eli Zaretskii]

> From: Juanma Barranquero <lekktu@gmail.com>
> Date: Fri, 18 Jun 2010 00:27:00 +0200
> Cc: Lennart Borgman <lennart.borgman@gmail.com>, david@harpegolden.net, emacs-devel@gnu.org, 
> 	storm@cua.dk
> 
> On Thu, Jun 17, 2010 at 21:23, Eli Zaretskii <eliz@gnu.org> wrote:
> 
> > What would you want Emacs to do instead, if it cannot
> > keep up with the input?
> 
> Stop reading input until it catches its breath.

This I can at least understand.  AFAIU, this is not the solution that
Lennart is trying to implement, at least his patch does not (and
cannot) do it.

What you want is clear, but I don't think it's possible without
redesigning Emacs.  Basic Emacs design is that it enters redisplay
only _after_ all input is read and processed; you seem to want it to
work the other way around.

Richard, do you think this can be accomplished, at least in principle?

Re: The unwarranted scrolling assumption

[Juanma Barranquero]

On Fri, Jun 18, 2010 at 10:02, Eli Zaretskii <eliz@gnu.org> wrote:

> This I can at least understand.  AFAIU, this is not the solution that
> Lennart is trying to implement, at least his patch does not (and
> cannot) do it.

Yes, I know.

> What you want is clear, but I don't think it's possible without
> redesigning Emacs.

Which is likely too much work for little gain, so I'm not really
pushing for this to be implemented; fortunately my computer can
currently keep up with scrolling.

    Juanma

Re: The unwarranted scrolling assumption

[Lennart Borgman]

On Fri, Jun 18, 2010 at 12:15 PM, Juanma Barranquero <lekktu@gmail.com> wrote:
> On Fri, Jun 18, 2010 at 10:02, Eli Zaretskii <eliz@gnu.org> wrote:
>
>> This I can at least understand.  AFAIU, this is not the solution that
>> Lennart is trying to implement, at least his patch does not (and
>> cannot) do it.
>
> Yes, I know.


Actually that was what I wanted to do first, but since there are some
bugs it is impossible at the moment. Things would be very difficult if
these where not cured first.


>> What you want is clear, but I don't think it's possible without
>> redesigning Emacs.


There might be other ways round (for example the exact meaning of an
interactive command here), but let us wait with this.


> Which is likely too much work for little gain, so I'm not really
> pushing for this to be implemented; fortunately my computer can
> currently keep up with scrolling.
>
>    Juanma
>
>

Re: The unwarranted scrolling assumption

[Stefan Monnier]

>>> Stop reading input until it catches its breath.

This has two problems, depending on the precise solution used:
- if we don't flush the "overflowing input", then this input
  accumulates, and you end up waiting for your scroll to keep scrolling
  long after you've stopped pressing the key.
- if we do flush the overflowing input, then the user ends up having to
  be careful not to type too fast, lest Emacs will ignore some of the
  keys she presses (could be some keys in the middle of a key-sequence).
So I don't think it's a good solution.  Emacs was designed to handle
a slow redisplay by suspending redisplay when it's too slow to avoid the
above problems.  I think it's a very sound design and don't want to try
and change it, because redisplay has always been on the critical path and
I don't see that changing in the future.

> Which is likely too much work for little gain, so I'm not really
> pushing for this to be implemented; fortunately my computer can
> currently keep up with scrolling.

I think there are several things we can do here:
1 - improve the scrolling behavior such that it doesn't recenter when
    the user specified that it wants line-by-line scrolling.  I hope
    this thread will find a solution to this problem.
2 - speed up redisplay such that the problem occurs less often.
    That would be of benefit in many more cases.
3 - Often redisplay is just fast enough except on a few occasions, and
    some of those occasions are calls to the GC, so making the GC
    faster or asynchronous would help (i.e. optimize up GC code, make it
    generational, or concurrent, younameit, but also try and reduce
    memory allocation to run the GC less often).


        Stefan

Re: The unwarranted scrolling assumption

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Eli Zaretskii <eliz@gnu.org>,  storm@cua.dk,  Richard Stallman <rms@gnu.org>,  emacs-devel@gnu.org
> Date: Fri, 18 Jun 2010 09:29:12 -0400
> 
> 1 - improve the scrolling behavior such that it doesn't recenter when
>     the user specified that it wants line-by-line scrolling.

But if the input includes many input events that move point, then when
we eventually enter redisplay, we could find itself very far away of
the last displayed portion of the buffer.  What does ``line-by-line
scrolling'' means in this case?

Re: The unwarranted scrolling assumption

[Stefan Monnier]

> But if the input includes many input events that move point, then when
> we eventually enter redisplay, we could find itself very far away of
> the last displayed portion of the buffer.  What does ``line-by-line
> scrolling'' means in this case?

It means "move the window-start as little as possible to make point
visible".


        Stefan

Re: The unwarranted scrolling assumption

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: emacs-devel@gnu.org
> Date: Fri, 18 Jun 2010 11:05:17 -0400
> 
> > But if the input includes many input events that move point, then when
> > we eventually enter redisplay, we could find itself very far away of
> > the last displayed portion of the buffer.  What does ``line-by-line
> > scrolling'' means in this case?
> 
> It means "move the window-start as little as possible to make point
> visible".

We already do that, under scroll-conservatively = most-positive-fixnum.

Therefore, each use-case where we recenter with this setting should be
investigated to find the precise reason.

Re: The unwarranted scrolling assumption

[Juanma Barranquero]

On Fri, Jun 18, 2010 at 15:29, Stefan Monnier <monnier@iro.umontreal.ca> wrote:

> This has two problems, depending on the precise solution used:
> - if we don't flush the "overflowing input", then this input
>  accumulates, and you end up waiting for your scroll to keep scrolling
>  long after you've stopped pressing the key.

I think that can already happen (though briefly) when
`redisplay-dont-pause' is non-nil.

> - if we do flush the overflowing input, then the user ends up having to
>  be careful not to type too fast, lest Emacs will ignore some of the
>  keys she presses (could be some keys in the middle of a key-sequence).

That would seem unwise.

> 1 - improve the scrolling behavior such that it doesn't recenter when
>    the user specified that it wants line-by-line scrolling.  I hope
>    this thread will find a solution to this problem.
> 2 - speed up redisplay such that the problem occurs less often.
>    That would be of benefit in many more cases.
> 3 - Often redisplay is just fast enough except on a few occasions, and
>    some of those occasions are calls to the GC, so making the GC
>    faster or asynchronous would help (i.e. optimize up GC code, make it
>    generational, or concurrent, younameit, but also try and reduce
>    memory allocation to run the GC less often).

All fine things to have, for sure.

    Juanma

Re: The unwarranted scrolling assumption

[David Engster]

Eli Zaretskii writes:
>> From: Lennart Borgman <lennart.borgman@gmail.com>
>> Date: Thu, 17 Jun 2010 20:32:27 +0200
>> Cc: emacs-devel@gnu.org, david@harpegolden.net
>> 
>> Several of us has said that the problem is easily reproduceable.
>
> Please show a recipe starting from "emacs -Q".

OK, I'll try. Using Emacs 23.2 GTK GNU/Linux:

* emacs -Q

* Insert in the *scratch* buffer:

  (setq scroll-preserve-screen-position 'always
      scroll-conservatively           most-positive-fixnum
      scroll-step                     0
      redisplay-dont-pause            t)

  and evaluate. These are the recommended settings given in this thread
  to get single-line scrolling.

* Insert the following and evaluate (adapt path accordingly):

  (dotimes (dummy 50)
    (insert-file-contents "~/emacs/share/emacs/23.2/etc/HELLO"))

* Now press <down> and hold key to scroll the buffer. The results will
  be different, depending on the default font and machine speed. On my
  machine (Celeron 900 @2.2GHz), the cursor sometimes jump to the middle
  or beginning of the window while scrolling.

  (BTW, I also notice that the cursor shape sometimes "sticks" in the
  "Hindi" line, but that's new to me).

Now this is just an example. As I've written, I've seen this behavior
for years, especially in large buffers when using different fonts,
elaborate font-locks, etc.

HTH,
-David

Re: The unwarranted scrolling assumption

[Lennart Borgman]

On Thu, Jun 17, 2010 at 9:36 PM, David Engster <deng@randomsample.de> wrote:
>
> Now this is just an example. As I've written, I've seen this behavior
> for years, especially in large buffers when using different fonts,
> elaborate font-locks, etc.

I guess this is because font lock uses narrowing. See my explanation to Eli.

You can try my patch if you want to, but please be aware of that there
were one little part I forgot to fix before sending it.

Re: The unwarranted scrolling assumption

[Stefan Monnier]

>   (setq scroll-preserve-screen-position 'always
>       scroll-conservatively           most-positive-fixnum
>       scroll-step                     0
>       redisplay-dont-pause            t)

>   and evaluate. These are the recommended settings given in this thread
>   to get single-line scrolling.

Actually the redisplay-dont-pause is not recommended, I just thought
maybe it could circumvent the problem.


        Stefan

Re: The unwarranted scrolling assumption

[David Engster]

David De La Harpe Golden writes:
> On 16/06/10 00:59, Lennart Borgman wrote:
>
>> All I do to check this is open window.c and then hit/hold down the
>> down arrow key (or "j" in viper).
>>
>> Does this work without "jumping scrolling" for you? (You have to test
>> for a while, the jumping does not happen always. I is a bit like
>> playing a computer game.)
>>
>
> I see this sometimes, with the recipe you outline.
>
> Sometimes emacs seems to decide to recenter, when it has "too much"
> input while display is taking its time (which I guess it might for
> e.g. font-locked window.c or a bunch of foreign scripts on
> view-hello-file)

Yes, absolutely. This is why scrolling usually works fine for 'simple'
setups, but gets jumpy as soon as you're using font-lock, overlong
lines, different fonts (especially TTFs), flyspell etc. I'm currently
working on a rather large LaTeX-File using AucTeX, font-lock, with a
fixed font for plain text and a TTF font for math formulas, and Emacs is
recentering while scrolling *all* the time. I think over the years I
really tried everything I could find, but nothing fixed this. The only
thing that's actually working a little bit is to set scroll-margin to a
larger value, so that the cursor has some leeway before leaving the
window, but even then it sometimes recenters. Turning off font-lock is
actually what's helping most.

-David

Re: The unwarranted scrolling assumption

[Lennart Borgman]

On Wed, Jun 16, 2010 at 1:24 AM, Sean Sieger <sean.sieger@gmail.com> wrote:
> (setq scroll-preserve-screen-position 'always
>      scroll-conservatively           most-positive-fixnum
>      scroll-step                     0)


Thanks Sean, but no. I still get "jumping scrolling" with this.

Re: The unwarranted scrolling assumption

[Juri Linkov]

> This seems to work well, Lennart:
>
> (setq scroll-preserve-screen-position 'always
>       scroll-conservatively           most-positive-fixnum
>       scroll-step                     0)

BTW, how users are supposed to customize `scroll-conservatively' to
most-positive-fixnum?  To write into the customization integer field
by hand all digits of 2305843009213693951?  ;-)

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: The unwarranted scrolling assumption

[Juanma Barranquero]

On Thu, Jun 17, 2010 at 22:35, Juri Linkov <juri@jurta.org> wrote:

> BTW, how users are supposed to customize `scroll-conservatively' to
> most-positive-fixnum?  To write into the customization integer field
> by hand all digits of 2305843009213693951?  ;-)

Any large integer, like 1000000, will suffice. But you have a point
that the defcustom for `scroll-conservatively' could have an option to
set it to max.

    Juanma

Re: The unwarranted scrolling assumption

[Juri Linkov]

>> BTW, how users are supposed to customize `scroll-conservatively' to
>> most-positive-fixnum?  To write into the customization integer field
>> by hand all digits of 2305843009213693951?  ;-)
>
> Any large integer, like 1000000, will suffice. But you have a point
> that the defcustom for `scroll-conservatively' could have an option to
> set it to max.

The problem is that a large value of `scroll-conservatively'
is an undocumented feature, so maybe it is not recommended?

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: The unwarranted scrolling assumption

[Juanma Barranquero]

On Fri, Jun 18, 2010 at 10:27, Juri Linkov <juri@jurta.org> wrote:

> The problem is that a large value of `scroll-conservatively'
> is an undocumented feature, so maybe it is not recommended?

It is "documented", sort of, in a roundabout way, via `scroll-step':

  scroll-step is a variable defined in `C source code'.
  Its value is 0

  Documentation:
  *The number of lines to try scrolling a window by when point moves out.
  If that fails to bring point back on frame, point is centered instead.
  If this is zero, point is always centered after it moves off frame.
  If you want scrolling to always be a line at a time, you should set
  `scroll-conservatively' to a large value rather than set this to 1.

    Juanma