Re: The unwarranted scrolling assumption

[Lennart Borgman]

On Fri, Jun 18, 2010 at 11:25 PM, Lennart Borgman
<address@hidden> wrote:
> On Fri, Jun 18, 2010 at 9:00 PM, Eli Zaretskii <address@hidden> wrote:
>>> From: Lennart Borgman <address@hidden>
>>> Date: Fri, 18 Jun 2010 19:31:30 +0200
>>> Cc: address@hidden
>>>
>>> +  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?