scroll issues

[F.S.]

The scroll issue was getting unbearable for me so I tried switching back to older versions. As of Leo 4.8 final the scroll issue is not present but it is in Leo 4.9. So for now I will be using Leo 4.8 instead. I back ported Terry Brown's fix to qt log printing and so far I am happy with 4.8.

Hopefully this could give some clue on what may be the cause (problem should be in the diff between 4.8 and 4.9).

[Edward K. Ream]

On Mon, Sep 24, 2012 at 1:47 PM, F.S. <speec...@gmail.com> wrote:

> The scroll issue was getting unbearable for me so I tried switching back to older versions. As of Leo 4.8 final the scroll issue is not present but it is in Leo 4.9. So for now I will be using Leo 4.8 instead.

Thanks for this. It is a good clue. I'll attempt a fix immediately.

Edward

[Terry Brown]

I agree this is a horrible problem that needs fixing, if I encountered
it early in an inspection of Leo I'd drop the thing as unfinished.

FS - I think you posted a file that exhibited the problem, but I don't
think it was exhibiting the problem for me, which might make sense if
it's related to font heights etc.

Yep, I just found your file, and it's still not doing it for me. I'll
have to make a point of trying to pin it down next time I encounter
it. Usually it's when I'm in a hurry and just tolerate it until I move
on to another node.

FS - if you use bzr, can't remember if you do, it doesn't take that
long to pin down the exact revision where the problem came in, say the
current rev. is 5800 and you don't see the problem in 4200, you test
5100, then 4700, then 4400 etc. - because you essentially cut the
search space in half every time it doesn't take that many tests.
That's what I'll do next time I see it happening, unless you beat me to
it.

Cheers -Terry

[F.S.]

That is a very good idea Terry, even though it cost me quite a bit of time mainly due to my unfamiliarity with bzr. To make a long story short I eventually tracked down the issue to revision 4163. It does not manifest in revision 4162 so 4163 was the first revision that introduced the bug. Diff shows quite a bit of change in the qtGui.py w.r.t. cursors and other events. Commit message said:"Some heavy-duty hacking to event handlers to make mouse clicks work like alt-x or ctrl-g. Catching click events is much safer than catching focus events. All unit tests, but this must be considered experimental code. Fortunately, only a few methods were changed."

I will have to leave the heavy lifting to Ed and you.

[F.S.]

I can also confirm that the problem does appear to be font size related. When I set font to 14px instead of 12px in the body pane, I don't notice the problem any longer.

Terry, can you try the file I posted with 12px font and see what happens. It reliably produced the jump issue for me with the out-of-box leoSettings.

On Monday, September 24, 2012 1:48:19 PM UTC-7, Terry wrote:

[Edward K. Ream]

On Mon, Sep 24, 2012 at 10:14 PM, F.S. <speec...@gmail.com> wrote:

> 4163 was the first revision that introduced the bug. Diff shows quite a bit of change in the qtGui.py w.r.t. cursors and other events.

Thanks for this. I spent several hours yesterday on this project, but
having the diff between 4162 and 4163 will help tremendously.

It's also good to know that font size matters, as you mention in the
following post.

This bug must be killed once and for all. The only way to do that is
to understand *exactly* why the bug happens. It's not good enough
just to make a change that appears to fix the problem for a particular
node or file. What's required is deep understanding so that we will
have confidence that this kind of thing can never happen again, with
any file or with any node. This is my goal, and I'm not going to do
anything else until its accomplished.

Edward

[Edward K. Ream]

On Tue, Sep 25, 2012 at 6:34 AM, Edward K. Ream <edre...@gmail.com> wrote:
> On Mon, Sep 24, 2012 at 10:14 PM, F.S. <speec...@gmail.com> wrote:
>
>> 4163 was the first revision that introduced the bug. Diff shows quite a bit of change in the qtGui.py w.r.t. cursors and other events.

Happily, I guess you could say, this bug bit me in the last week, so I
have a reliable test node. It's simply the docstring for the
screencast plugin. I've copied it to test.leo for testing, but I
haven't pushed anything.

To reproduce the bug, simply scroll to the end of the node, either
with the mouse wheel or Ctrl-End or even with lots of arrow keys. Now
click near the end of the body text and the text scrolls so the cursor
is invisible.

Per the diff, my attention was drawn to this line in
leoQtBaseTextWidget.mouseReleaseEvent::

c.k.keyboardQuit(setFocus=False)

Disabling this line makes the unwanted scroll go away. Amazingly, the
following comment precedes this line::

# 2011/10/02: Calling k.keyboardQuit here causes some
# unwanted scrolling in rare cases, but seemingly that
# can't be helped: removing this call would be confusing.

Knowing that the unwanted scroll happens somewhere in the
k.keyboardQuit logic is a good start, but now we must find out exactly
what is causing the unwanted scroll.

F.S., could you verify that disabling this line "fixes" the bug for
you with the latest bzr version? Thanks.

Edward

[Terry Brown]

On Mon, 24 Sep 2012 23:24:27 -0700 (PDT)
"F.S." <██████████@████████> wrote:

> Terry, can you try the file I posted with 12px font and see what happens.
> It reliably produced the jump issue for me with the out-of-box leoSettings.

I'm very busy at the moment, in part because I'm going to be on
vacation all of next week, but it seems like Edward is on the trail.
Thanks for doing the bzr search FS, that makes it a lot easier.

Cheers -Terry

[Edward K. Ream]

On Tue, Sep 25, 2012 at 7:54 AM, Edward K. Ream <edre...@gmail.com> wrote:

> Knowing that the unwanted scroll happens somewhere in the k.keyboardQuit logic is a good start, but now we must find out exactly what is causing the unwanted scroll.

The problem is the call to w.setStyleSheet(s) in
leoQtBody.setEditorColors. This is called from k.keyboardQuit in
order to set background or border color in the body pane. When this
call is disabled, the problem goes away.

It's pretty clear why w.setStyleSheet might cause scrolling problems:
stylesheets could affect font size, so scrolling info becomes dubious.
This might also explain the dependency of the bug on font size.

My initial attempt at saving/restoring scrollbar position in
setEditorColors haven't worked, but I'm pretty sure a fix can be found
soon.

Edward

[Terry Brown]

In case it helps:
https://groups.google.com/forum/#!topic/leo-editor/vd2d3pMS_Kk

[Edward K. Ream]

On Tue, Sep 25, 2012 at 8:40 AM, Terry Brown <terry_...@yahoo.com> wrote:
> On Tue, 25 Sep 2012 08:30:13 -0500
> "Edward K. Ream" <edre...@gmail.com> wrote:
>
>> My initial attempt at saving/restoring scrollbar position in
setEditorColors haven't worked, but I'm pretty sure a fix can be found soon.

> In case it helps:

Thanks for this. w.verticalScrollBar().sliderPosition() doesn't seem
to be correct just at the moment update_style_sheet needs it. I'll
investigate using your probe.

Edward

[F.S.]

I can confirm that disabling  c.k.keyboardQuit  does make the scroll problem go away, using the latest release.

Do you use any tool to move around the code base? Such as ctags or cscope?

[Terry Brown]

On Tue, 25 Sep 2012 10:47:20 -0500
"Edward K. Ream" <edre...@gmail.com> wrote:

> Thanks for this. w.verticalScrollBar().sliderPosition() doesn't seem
> to be correct just at the moment update_style_sheet needs it. I'll
> investigate using your probe.

For some reason this just bubbled to the top of my mind...

Why does update_style_sheet need sliderPosition()? Seems they're
unrelated topics.

Cheers -Terry

[Edward K. Ream]

On Tue, Sep 25, 2012 at 3:20 PM, Terry Brown <terry_...@yahoo.com> wrote:

> Why does update_style_sheet need sliderPosition()?

I was trying to save and restore the slider position, but that doesn't work.

Rev 5459 is an almost perfect fix for the problem. Here is the checkin log.

QQQQQ
A good, but not perfect fix for unwanted scrolling in the body pane:

The problem was the call to w.setStyleSheet in
g.app.gui.update_style_sheet. Apparently, this causes a layout-request
event that spoils the scroll position.

The fixes:

1. update_style_sheet does nothing if the new stylesheet is the same as the old.

2. Added lockout to mouseReleaseEvent. update_style_sheet does nothing
if the lockout is set.

3. mouseReleaseEvent sets c.p.v.insertPoint if appropriate.

Hitting Ctrl-H can still cause a small unwanted scroll, but now the
insert point remains visible.
QQQQQ

My present mental model for what is happening is that various events
can trigger a layout-request event, and apparently this gets honored
at idle time. This request can reset carefully saved scrollbar
positions. Or so I think now.

Terry, your probe was useful in the debugging process, but really it
just highlighted how strange the behavior is/was. I tried lots of
workarounds, such as calls to repaint or blockSignals, all to no
avail.

As the checkin log indicates, this rev fixes a mostly unrelated bug:
the mouse-click handler now sets c.p.v.insertPoint if the click
happens in the body pane. This ensure that the line with the cursor
is visible the next time focus returns to the body pane.

There is still an easy way to get a small amount of unwanted
scrolling. Just put the cursor at the end of a large amount of text
in the body pane and then hit Ctrl-H. This will cause the body text
to scroll so that the line with the cursor is at the bottom of the
body pane. I don't know why this happens, but I think we can live
with it for now. Note that this scrolling remains even if the
g.no_see and g.no_scroll switches are on. That doesn't prove that Leo
is innocent, but it indicates that the problem will be hard to find if
Leo is guilty.

I suspect that the latest code is at least as good as the Leo 4.8 code
base. F.S., I hope you agree.

Edward

[Edward K. Ream]

On Tue, Sep 25, 2012 at 11:28 AM, F.S. <speec...@gmail.com> wrote:
> I can confirm that disabling c.k.keyboardQuit does make the scroll problem go away, using the latest release.

Thanks for the confirmation. As I just indicated in another reply,
rev 5459 mostly fixes the bug.

> Do you use any tool to move around the code base?

I use Leo, especially clones to gather nodes. The clone-find-all
command can be very helpful. Leo's code completer can use ctags.

Edward

[F.S.]

That is very good news! I will definitely give clone-find-all a try.

I have some other issues and questions in general and I will ask them in separate threads.

[F.S.]

On Tuesday, September 25, 2012 1:50:19 PM UTC-7, Edward K. Ream wrote:

The fixes:

1. update_style_sheet does nothing if the new stylesheet is the same as the old.

2. Added lockout to mouseReleaseEvent. update_style_sheet does nothing
if the lockout is set.

3. mouseReleaseEvent sets c.p.v.insertPoint if appropriate.

I just noticed that after some usage the background color of the body pane turns from the default pink to yellow (the default color of outline pane). It turns back to the default color after a while, sometimes when I moved away from and then back to the Leo window, or sometimes after scrolling. The two colors are close anyway so no big deal. Just thought this might be a side effect of a stylesheet lockout and thought you may be interested in knowing.

 

I suspect that the latest code is at least as good as the Leo 4.8 code
base.  F.S., I hope you agree.

Edward 

Performance is very impressive for pure python codes. Opening and browsing through the whole code base (qtGui alone is over 10k lines!) is snappy. The outline mode seems to enable you to put a lot more code in one file without the traditional difficulty of finding things.

 

I am inspired and trying to learn more about Leo. Terry's reference to @shadow tree was a great pointer. I am also reading the docs. The part about the robust MVC architecture is great. I work with metadata quite a bit so I appreciate the comments on separation of metadata from data as well.

[Edward K. Ream]

On Wed, Sep 26, 2012 at 12:25 AM, F.S. <speec...@gmail.com> wrote:

> I just noticed that after some usage the background color of the body pane
> turns from the default pink to yellow (the default color of outline pane).

> ...this might be a

> side effect of a stylesheet lockout and thought you may be interested in
> knowing.

Yes, I'm interested. The lockout only happens during a mouse-click
event in the body pane. I'm not at all sure that it's a good idea,
but otoh, if the event causes a change to the stylesheet the scroll
bug will be reactivated.

Hmm, I just tried switching between the outline pane and the body pane
with the mouse. Somehow things mostly work, but I'm not sure why!
That is, the outline border is drawn correctly (by changing a
stylesheet). There *is* some unwanted scrolling, but it's not
horrible: the clicked position stays on the screen.

Are you switching between panes with the mouse? If so, I would
discourage you from doing so, for several reasons. First, mousing
around will damage your body eventually. I learned that the hard way.
Second, Leo is designed to be key friendly. You might as well come
up to speed quickly.

Also, what version of Leo are you using? Lately, the default
background color for the body pane is white, regardless of what pane
has focus, because the border color shows the pane with focus much
more clearly than faint background colors.

Edward

[F.S.]

I am using the latest Leo. Just verified that the default setting of background color is still pink for body pane:

    background-color: #fdf5f5; /* A kind of pink. */

    selection-color: white;

    selection-background-color: lightgrey;

I will play around with it a bit as you suggested. Could you post or update the repo with the new settings you are referring to? The white on grey for selection does not work that great as the white is really faint against the selection background and is hard to read. 

The vr pane is white and it looks nice.

I also noticed the jumping when using mouse click. Using C-T seems to not cause the jump. I have been trying to use key combos as much as possible in general because at the end of the day I could already feel the wrist pain if I use the mouse too much and I don't want the injury to get worse or permanent.

[Edward K. Ream]

On Tue, Sep 25, 2012 at 3:50 PM, Edward K. Ream <edre...@gmail.com> wrote:

> Rev 5459 is an almost perfect fix for the problem.

But not perfect enough, imo. As noted, Ctrl-H will still cause
unwanted scrolling.

Looking at the sources, I see that w.setStyleSheet generates an
internal polish event, which presumably is the culprit.

I am beginning to think that changing the stylesheet in order to draw
the border is a fools errand. An alternative would be to embed the
body pane in a plain widget, and just set the background color of that
widget. Presumably, that would not cause a wholesale restyling of the
embedded text widget. In fact, it should have no effect on any
embedded widgets.

In any case, the present situation is intolerable.

Edward

[Terry Brown]

On Thu, 27 Sep 2012 11:51:13 -0500

"Edward K. Ream" <edre...@gmail.com> wrote:

> On Tue, Sep 25, 2012 at 3:50 PM, Edward K. Ream <edre...@gmail.com> wrote:
>
> > Rev 5459 is an almost perfect fix for the problem.
>
> But not perfect enough, imo. As noted, Ctrl-H will still cause
> unwanted scrolling.
>
> Looking at the sources, I see that w.setStyleSheet generates an
> internal polish event, which presumably is the culprit.

So the border must be present at all times, and only change color, not
thickness, otherwise the text will wiggle as the border comes and goes.

Given that, can't the scroll position be read before the call to
w.setStyleSheet? I know that's obvious and there are all kinds of
complexities of which I'm blissfully unaware, but it seems the scroll
position should be right.

Also, is the problem really restoring the scroll position? Seems the
scroll jump would occur as the current body gains and loses focus, even
when no node switching is happening. So can scroll position
restoration be suppressed except for a one time thing when a new node
is selected?

Really, it's very simple when you don't know what you're talking
about :-)

Cheers -Terry

[Edward K. Ream]

On Thursday, September 27, 2012 12:31:33 PM UTC-5, Terry wrote:

> can't the scroll position be read before the call to w.setStyleSheet?

Doesn't work.

[big snip]

> Really, it's very simple when you don't know what you're talking about :-)

Hahaha.

Well, I now have a dirt simple solution!!  Rather than draw the border *in* the QTextBrowser, the trick is draw the border *around* the QTextBrowser! This leaves the QTextBrowser completely unchanged, and so nothing *at all* happens in the body pane. I should have thought of this a long, long time ago.

The actual code is fairly clean:  the g.app.gui.add/remove_border methods change the stylesheet of c.frame.top.innerBodyFrame (a QFrame) instead of the body text widget itself.

There is one glitch:  on Windows at least, a QFrame with a 5px border looks very different from a QTextBrowser with a 5px border.  Why am I not surprised ;-)  As a workaround, we will need two settings:

    @int focus_border_width = 5 # As always, controls the width of the border in the tree and log panes.
    @int focus_body_border_width = 2 # Controls the width of the border around the body pane.

OMG, this *still* doesn't work.  The body pane still scrolls in my test case. The next step will be to set the color of the QFrame without a stylesheet, which hopefully will prevent the propagation of the stylesheet update logic...

Edward

[F.S.]

It has been a while :-) This issue seems to have come back (or was it ever resolved?) as far back as this January. I just downloaded the newest version and it is still there. It is quite annoying which is why I am digging up this old thread.

[Terry Brown]

On Mon, 24 Mar 2014 17:28:40 -0700 (PDT)
"F.S." <speec...@gmail.com> wrote:

> It has been a while :-) This issue seems to have come back (or was it
> ever resolved?) as far back as this January. I just downloaded the
> newest version and it is still there. It is quite annoying which is
> why I am digging up this old thread.

Hi F.S., long time no see :)

Although it's useful to remember the old thread, there have been
changes with the way styling works, and, unfortunately, I can't
remember whether the old border code is being used with the option of
using the new styling code instead, or if the new styling is doing
borders now.

If you can provide an minimal outline which demonstrates the problem,
that would help. And what is the problem? :-) Things wiggling around
because of border widths or something?

Cheers -Terry

[F.S.]

Hi Terry,

Good to hear from you again. This seems like one tough bug. I dug some more and found some more threads on this:

https://groups.google.com/forum/#!searchin/leo-editor/F.S./leo-editor/eH5cspNNOD0/SZm95-olO-EJ

https://groups.google.com/forum/#!searchin/leo-editor/F.S./leo-editor/vd2d3pMS_Kk/V4NXP_bIA3EJ

At the end of first thread I did attache a jumpbug.leo file that was problematic for me, but unfortunately I just tried it and it does not seem to work any more. Or I guess that is a good thing! 

So I anonymized the thing I was working on (I did notice that "replace all" is rather painfully slow and I don't know how to specify wildcard matching) and will attach it to the end of this post. I don't use any custom settings. If I scroll to the end of the node and I change the focus in anyway the scroll would jump.

[Terry Brown]

On Tue, 25 Mar 2014 10:23:54 -0700 (PDT)
"F.S." <speec...@gmail.com> wrote:

> So I anonymized the thing I was working on (I did notice that
> "replace all" is rather painfully slow and I don't know how to
> specify wildcard matching) and will attach it to the end of this
> post. I don't use any custom settings. If I scroll to the end of the
> node and I change the focus in anyway the scroll would jump.

Thanks for doing that, it made it easy for me to test and confirm that
the current gui hierarchy manipulating border code still exists and is
used and causes the problem you show, and the style based border
highlighting which is disabled be default does not have the same
problem.

I will try and switch the default over asap.

Cheers -Terry

[Edward K. Ream]

On Tue, Mar 25, 2014 at 12:37 PM, Terry Brown <terry_...@yahoo.com> wrote:
> On Tue, 25 Mar 2014 10:23:54 -0700 (PDT)
> "F.S." <speec...@gmail.com> wrote:
>

>> So I anonymized the thing I was working on...

>
> Thanks for doing that, it made it easy for me to test and confirm that
> the current gui hierarchy manipulating border code still exists and is
> used and causes the problem you show, and the style based border
> highlighting which is disabled be default does not have the same
> problem.
>
> I will try and switch the default over asap.

Thanks, Terry, for your help with this.

Edward