The *end* of unwanted scrolling

[Edward K. Ream]

I now have a prototype for a more complete solution to the unwanted scrolling.  The prototype is kludgy, and needs a bit of polishing.

As recent work shows, it is hopeless to call w.setStyleSheet for any widget w that is a parent of Leo's body pane.  Such a call resets stylesheets throughout the widget tree, which eventually causes the unwanted scrolling.  I wouldn't call this a Qt bug; in general any change to a stylesheet could, in fact, cause scrolling!

Thus, we must avoid w.setStyleSheet entirely when drawing the focus border around the body pane.  Leo's body pane is contained in a QFrame whose name is 'innerBodyFrame'.  So the solution is to give this QFrame a custom paintEvent method.  Like this::

        class InnerBodyFrame(QtGui.QFrame):
            def paintEvent(self,event):
                # A kludge.  g.app.gui.innerBodyFrameColor is set by paint_qframe.
                if hasattr(g.app.gui,'innerBodyFrameColor'):
                    color = g.app.gui.innerBodyFrameColor
                    painter = QtGui.QPainter()
                    painter.begin(w)
                    painter.fillRect(w.rect(),QtGui.QColor(color))
                    painter.end()

As you can see, the color to be repainted is gui.innerBodyFrameColor, which is set by the gui methods that add or remove the border colors!

For the prototype **only** I added the following to my copy of the Qt stylesheet in myLeoSettings.leo::

    QFrame#innerBodyFrame {
        border: 5px solid red;
    }

This won't work for production:  Leo must compute the *initial* stylesheet for the innnerBodyFrame from the @int focus_border_width setting.  That will be next.

The focus border has caused us all a great deal of trouble. Imo, all this trouble has been worthwhile--the focus border is a great addition to Leo. I'll be upping the complete fix sometime tomorrow after I do more extensive testing.

Edward

P.S. Unwanted scrolling may still be a problem for those using solorized colors in the body text.  I doubt if there is any alternative to using QTextBrowser.setStyleSheet in that case.

EKR

[Edward K. Ream]

On Thursday, September 27, 2012 6:49:10 PM UTC-5, Edward K. Ream wrote:

> P.S. Unwanted scrolling may still be a problem for those using solarized colors in the body text.  I doubt if there is any alternative to using QTextBrowser.setStyleSheet in that case.

This would only be a problem if *different* (solarized) colors were chosen depending on whether the body pane has focus or not.  But now that we have focus borders, the intense flashing involved when switching between the light and dark color schemes is unnecessary.

EKR

[Terry Brown]

On Thu, 27 Sep 2012 17:00:05 -0700 (PDT)
"Edward K. Ream" <edre...@gmail.com> wrote:

> This would only be a problem if *different* (solarized) colors were chosen
> depending on whether the body pane has focus or not. But now that we have
> focus borders, the intense flashing involved when switching between the
> light and dark color schemes is unnecessary.

I don't really think that's how people use the solarized theme, I know
it has dark on light and light on dark variations, but I can't imagine
people have the whole body frame flip on every focus event. I suspect
people just like the light text on dark background alternative. I keep
meaning to try it.

Cheers -Terry

[Edward K. Ream]

On Thu, Sep 27, 2012 at 7:45 PM, Terry Brown <terry_...@yahoo.com> wrote:
> On Thu, 27 Sep 2012 17:00:05 -0700 (PDT)
> "Edward K. Ream" <edre...@gmail.com> wrote:
>
>> This would only be a problem if *different* (solarized) colors were chosen
>> depending on whether the body pane has focus or not. But now that we have
>> focus borders, the intense flashing involved when switching between the
>> light and dark color schemes is unnecessary.
>
> I don't really think that's how people use the solarized theme

I agree. I just wanted to point out that the scroll bug would bite
again if they did.

Edward

[Edward K. Ream]

On Thursday, September 27, 2012 6:49:10 PM UTC-5, Edward K. Ream wrote:

I now have a prototype for a more complete solution to the unwanted scrolling. 

Rev 5465 fixes some more scroll-related problems.  These were much more tractable once w.setStyleSheet calls are mostly gone.

Here is the checkin log:

QQQ
A number of important improvements for scroll position.

- Activated onSliderChanged.
- Call c.p.v.restoreCursorAndScroll() in onActivateEvent.
- Removed the *evil* maintain_scroll setting.
- p/v.restoreCursorAndScroll and p/v.saveCursorAndScroll no longer take a w argument.
QQQ

The scrolling code is getting more and more simple, but there are still subtleties.  I'm not sure whether I put in the maintain_scroll setting, but I consider it completely evil in the present environment.  Indeed, it appears that moving between panes *never* alters either insert point or the scroll position, which is of course how it should be.

As always, I welcome all bug reports, but please DO NOT change any of the scrolling code without consulting with me first.

In particular, calls to QTextBrowser.ensureCursorVisible, or equivalently, c.frame.body.seeInsertPoint or c.frame.body.bodyCtrl.seeInsertPoint must be used only in unusual circumstances, or perhaps even never :-)

Edward

[F.S.]

On Friday, September 28, 2012 4:40:46 PM UTC-7, Edward K. Ream wrote:

On Thursday, September 27, 2012 6:49:10 PM UTC-5, Edward K. Ream wrote:

I now have a prototype for a more complete solution to the unwanted scrolling. 

Rev 5465 fixes some more scroll-related problems.  These were much more tractable once w.setStyleSheet calls are mostly gone.

Rev 5465 is fairly unusable. Switching between large nodes takes forever. Cursor is no longer visible if I navigate away from a node and then come back to it. For large node cursor position must not be lost otherwise it will take a lot of effort to find the old edit position again if one navigates away from a node briefly. 

[Edward K. Ream]

On Saturday, September 29, 2012 1:49:14 AM UTC-5, F.S. wrote:

Rev 5465 is fairly unusable...Cursor is no longer visible if I navigate away from a node and then come back to it. For large node cursor position must not be lost otherwise it will take a lot of effort to find the old edit position again if one navigates away from a node briefly.

Oops!  Sorry about that.  I'll fix this immediately.

Edward

[Edward K. Ream]

On Sat, Sep 29, 2012 at 3:59 AM, Edward K. Ream <edre...@gmail.com> wrote:
> On Saturday, September 29, 2012 1:49:14 AM UTC-5, F.S. wrote:

>> Rev 5465 is fairly unusable

> Oops! Sorry about that. I'll fix this immediately.

Done at rev 5467. Here is the checkin log:

"Disable onSliderChanged: it was ruining the node-selection logic.
This method was useful in other contexts, so disabling it will cause
(minor) unwanted scrolling when using the scrollwheel."

To see the unwanted scrolling, scroll with the scrollwheel and then do
Ctrl-H. This is a hard problem to solve. The reason that using
onSliderChanged doesn't work is that, Doh!, the slider changes when
changing nodes.

It might be possible to lock out onSliderChanged when changing nodes,
but I won't be able to any more programing until Monday.

Edward

[Edward K. Ream]

On Sat, Sep 29, 2012 at 4:21 AM, Edward K. Ream <edre...@gmail.com> wrote:

> It might be possible to lock out onSliderChanged when changing nodes.

Done at rev 5468. Everything appears to work. Please report any
problems immediately.

I just realized that writing unit tests for most scroll cases will be
straightforward. That's on the list for Monday.

Edward

[F.S.]

5468 appears to be problem free! Turning off syntax color made switching large nodes snappy as well.

I would like to understand why syntax color causes such a slow down, but that is another topic for another day. Were you really up at 4am fixing this? Wow!

[Ville M. Vainio]

Switching nodes colorizes the whole new node. This takes time.

--
You received this message because you are subscribed to the Google Groups "leo-editor" group.
To view this discussion on the web visit https://groups.google.com/d/msg/leo-editor/-/g9t185SoGGMJ.
To post to this group, send email to leo-e...@googlegroups.com.
To unsubscribe from this group, send email to leo-editor+...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/leo-editor?hl=en.

[F.S.]

Ville,

I don't seem to observe the visible slowing from syntax coloring when editing large emacs or vim buffers. I am trying to understand why Leo has such a significant degradation in performance.

[Ville M. Vainio]

You don't have big slowdown in leo either when editing a buffer;
slowness appears when you move between nodes (which is equivalent to
opening a new file in emacs/vi).

There are ways to make this faster if we really want, including a
C/C++/cython based colorizer, but that could require a lot of work.

> https://groups.google.com/d/msg/leo-editor/-/RdPOG7ipg7cJ.

[Edward K. Ream]

On Mon, Oct 1, 2012 at 12:37 AM, F.S. <speec...@gmail.com> wrote:

> I don't seem to observe the visible slowing from syntax coloring when editing large emacs or vim buffers. I am trying to understand why Leo has such a significant degradation in performance.

Many thanks for your questions. The association of colorizing with
(file) caching has lead my thoughts in new directions. As I shall
explain, there is no obvious way to speed up Leo's syntax colorizing,
but perhaps the solution to a seemingly intractable problem can be
found...

Leo is different from emacs or vim because Leo recolors the entire
text every time Leo selects a node. In contrast, emacs or vim selects
a buffer once, when loading the file. In fact, Leo does a good job of
incremental coloring, even for very large nodes. In this sense, the
difference in performance could be said to be a difference in
perception.

As the result of your previous questions, I have been investigating
caching syntax-coloring data. Unlike Tk, Qt does not provide an
efficient way of reporting such data: QSyntaxHighlighter.format(n)
only reports on the formatting of the *single* character at position
n. This call returns a *different* instance of QTextCharFormat for
every character in the body text, even if the formats for two
characters are identical.

Actually, there *is* an effective way of caching data: associate a
*separate* instance of QSyntaxHighlighter with every node(!) Taking
this idea a step farther, we could even imagine a separate instance of
QTextBrowser for every node(!!) This last step is almost certainly a
step too far.

But even just creating per-node copies of QSyntaxHighlighter would be
a big memory drain. Presumably, QSyntaxHighlighter contains an
instance of QTextCharFormat for every character in the body text, and
other formatting data as well. Retaining that data for each node would
probably overburden the GC. Although an intriguing idea, creating a
separate instance of QSyntaxHighlighter for each node seems too risky.

So it looks like we need something that QSyntaxHighlighter does not
provide, namely a summary of the formatting in effect when we leave a
node. If we had such a summary, Leo would be able to recolor
previously colored nodes *quickly* when revisiting the node. Such a
scheme would not be a memory buster provided that summaries were
concise.

Using QSyntaxHighlighter.format(n) to compute the summary data when
Leo leaves a node is probably going to be too slow. Because format(n)
returns a separate instance of QTextCharFormat for each character,
just determining whether adjacent characters are formatted alike may
be expensive. I experiment asap...

Could Leo's syntax coloring code create the *concise* summary data?
jEditColorizer.setTag is the obvious place. Alas, computing the
summary data seems very difficult. Character indices change as the
user inserts or deletes characters, so creating a concise summary of
the ranges of characters colored in any particular style appears to
require major computation. It's a puzzle with no obvious solution...

The conclusion is simple: without a fast, memory efficient way of
summarizing the entire state of syntax coloring in a node, Leo must
recolor a node whenever Leo selects it. This is the big difference
between Leo and emacs/vim.

Edward

[Edward K. Ream]

On Mon, Oct 1, 2012 at 2:05 AM, Edward K. Ream <edre...@gmail.com> wrote:

> Could Leo's syntax coloring code create the *concise* summary data? jEditColorizer.setTag is the obvious place. Alas, computing the
summary data seems very difficult.

"Impossible" is probably the proper word here. Indeed, setTag only
knows the indices of the ranges of text within the line being
colorized. That is, QSyntaxHighlighter just feeds lines to the
over-ridden highlightBlock method. As the user types on the line,
highlighting for the line will happen repeatedly, so there is *no way*
to know what the global indices are. This looks like the end of this
line of inquiry.

The only remaining question is whether calling
QSyntaxHighlighter.format(n) for n in range(len(c.p.b)) has any chance
of being fast enough.

Edward

[Edward K. Ream]

On Mon, Oct 1, 2012 at 2:24 AM, Edward K. Ream <edre...@gmail.com> wrote:

> The only remaining question is whether calling QSyntaxHighlighter.format(n) for n in range(len(c.p.b)) has any chance of being fast enough.

The original idea won't work: n is simply an index into the line being
colored, not a global index.

Otoh, this means that QSyntaxHighlighter (QSH) probably contains less
data than I thought. It might actually be feasible to create an
instance of QSH for every node that is ever selected, say
v.highlighter. Doing this will probably be user option, so that
people have some recourse if the GC chokes...I'll investigate later
today. Time for some sleep ;-)

Edward

[Edward K. Ream]

On Mon, Oct 1, 2012 at 3:51 AM, Edward K. Ream <edre...@gmail.com> wrote:
> On Mon, Oct 1, 2012 at 2:24 AM, Edward K. Ream <edre...@gmail.com> wrote:

> The original idea won't work: n is simply an index into the line being
> colored, not a global index.

This is turning into quite an educational project for me.

w.repaint() does *not* involve QSH, so the QTextBrowser must cache the
style info. The question is, where?

To answer this, I looked at the QSH sources. The answer, apparently,
is in a QTextLayout. Indeed, in QSH.applyFormatChanges() there is the
line::

QTextLayout *layout = currentBlock.layout();

and right near the end::

layout->setAdditionalFormats(ranges);

So it should be possible to write a short script to get a concise
summary of the formatting. I might even dare hope that QTextLayout
may simplify the process. We shall see...

Edward

[Edward K. Ream]

On Mon, Oct 1, 2012 at 8:29 AM, Edward K. Ream <edre...@gmail.com> wrote:

> So it should be possible to write a short script to get a concise
> summary of the formatting. I might even dare hope that QTextLayout
> may simplify the process. We shall see...

Yes. There is an undocumented class involved, but Python's dir
function came to the rescue. Here is the script::

import time
w = c.frame.body.bodyCtrl.widget # a subclass of QTextBrowser
doc = w.document()
t1 = time.time()
n = 0
for i in range(doc.blockCount()):
b = doc.findBlockByNumber(i)
s = str(b.text())
layout = b.layout()
for r in layout.additionalFormats():
# r is an undocumented QFormatRange
# r.format is a QTextCharFormat.
# formats are shared only within a single block.
s2 = s[r.start:r.start+r.length]
# print('%20s %s' % (r.format,s2))
brush = r.format.foreground()
color = brush.color()
n += 1

t2 = time.time()
g.es_print('done: %s formats %s sec' % (n,t2-t2))

This script button reports 0.0 seconds even for nodes that take a
noticeable time to redraw. Thus, it appears that it is indeed
possible to create a fast, concise summary of all editing in a node.

There are some gc issues involved. Putting each r.format in a list
hard-crashed Python, so it may be necessary to create a copy of
r.format for caching purposes. Still, this small script demonstrates
the general idea.

To summarize, when revisiting a node, we may be able to recolorize
using the cached summary of syntax coloring rather than calling QSH to
do a full recolor. This promises *much* faster reloading of nodes.

Notes:

1. It's not proven that using cached data to recolor a node will be
fast enough. Experiments should be easy to do. I'll know more soon.

2. Even when using cached data we still have to re-init the QSH when
revisiting a node. Otherwise the QSH will not be able to do
incremental recoloring. In essence, we must tell QSH the starting
state of each line of the text. I believe this is feasible: the
necessary data exist in Leo's colorizing code, and so can be cached.

Edward

[Edward K. Ream]

On Mon, Oct 1, 2012 at 9:44 AM, Edward K. Ream <edre...@gmail.com> wrote:

> There is an undocumented class involved, but Python's dir
> function came to the rescue.

Actually, there is documentation:

http://doc.qt.digia.com/4.7-snapshot/qtextlayout-formatrange.html

EKR

[Viktor Ransmayr]

Hello Edward,

I have upgraded to the latest revision - and - the scrolling problem is still there.

However it has changed, i.e. for me it is only available anymore whenever I
save a node/ the outline.

This behaviour is consistent - and - it happens even with smaller nodes, which
is something that I did not experience before. - HTH.

With kind regards,

Viktor
 

[F.S.]

There is a bug in the script:

g.es_print('done: %s formats %s sec' % (n,t2-t2)) 

is guaranteed to print zero every time :-) Should be t2-t1.

Thanks for the education for us all.

However in terms of importance I would rank dabbrev much ahead of syntax coloring. My large nodes tend to be very regularly structured (repetition of instance creations in bookkeeping, e.g.) so lack of syntax color is not a real concern. Nodes with more obvious clustering properties I can and should break up into smaller nodes.

I also second Viktor that it is an annoyance that the editor still scrolls on its own after saving a file.

[Edward K. Ream]

On Mon, Oct 1, 2012 at 3:00 PM, Viktor Ransmayr
<viktor....@gmail.com> wrote:

> I have upgraded to the latest revision - and - the scrolling problem is still there. However it has changed, i.e. for me it is only available anymore whenever I save a node/ the outline.

Thanks for this report. It has been fixed at rev 5470.

Here is the checkin log:

QQQ
Fixed a bad mistake: onSliderChanged should only change
v.scrollBarSpot if "self" is, in fact, the body pane.

Without this fix, scrolling in the log would reset the body scroll position!

All unit tests pass, but I have no idea how to create a unit test for
this kind of problem.
QQQ

Edward

[Edward K. Ream]

On Mon, Oct 1, 2012 at 3:41 PM, F.S. <speec...@gmail.com> wrote:
> There is a bug in the script:
>
> g.es_print('done: %s formats %s sec' % (n,t2-t2))

Right. The actual time for a node containing a copy of all of
leoApp.py is about 0.06 sec, a huge improvement over recoloring the
entire pane.

> Thanks for the education for us all.

You're welcome. As of the latest rev, test.leo contains two @button
scripts: @button read/write color info.

Select a node, then do write color info followed by read color info.
If you comment out this line of read color info::

layout.setAdditionalFormats(ranges2)

you will see that the text is all black, so this last line is, in
fact, responsible for recoloring the entire node. Quickly!

These two @button scripts will be the basis of color caching, coming soon.

> However in terms of importance I would rank dabbrev much ahead of syntax coloring.

I'll look at dabbrev later today. This is the first time anyone has
mentioned problems with it.

Edward

[F.S.]

As of build 5482, dabbrev-completion and dabbrev-expand cause the insertion point to scroll to the bottom of the body pane. I would also count these scrolls as "unwanted". Interestingly undo restores the scroll position to exactly before the edit was initiated.

[Edward K. Ream]

On Thu, Oct 4, 2012 at 2:28 AM, F.S. <speec...@gmail.com> wrote:

> As of build 5482, dabbrev-completion and dabbrev-expand cause the insertion point to scroll to the bottom of the body pane. I would also count these scrolls as "unwanted". Interestingly undo restores the scroll position to exactly before the edit was initiated.

Thanks for this report. Fixed at rev 5485. I also improved the
before/afterNodeChanged undo/redo methods, but at present those
improvements apply only to the dabbrev commands.

Edward