Slow scrolling? [Ubuntu]

[p2rkw]

I found another strange issue, sometimes curly braces aren't correctly highlighted.
Here's my test case:

Code

int func()
{
	return 0;
}

word word word word word word word word  /*line b*/

call(arg0,
		arg1, arg2
		)

word word word word word word word word /*line a*/

word word word word word

Select "word" in line 'a', and move cursor up to func(). Notice that highlight gone when cursor meet ')' (whole screen redraw?), curly braces should by highlighted correctly. After that move cursor down to '}' - matching curly bracket isn't highlighted, but if you for instance add breakpoint or toggle bookmark, brace will be highlighted.

[Alpha]

Bisected: revision 8278 introduces the problem.  Something must have gone wrong with the Scintilla update.

I forgot to mention, that was on Windows.

Now I see it here also (I tested it with 8248).

... which means that either I incorrectly bisected (which is a possibility), or there are multiple (platform specific?) items that are contributing to the problem.  Although, it sounds as though the source may already have been found.

And what's more:
after using the wxScintilla UI-Event in the stc-sample to call HighlightOccurrences instead the wxUpdatUI event, the issue is there also.

That means you are right and it is most likely a scintilla and not a wxScintilla issue (bug?).

[Jenna]

Now I see it here also (I tested it with 8248).

... which means that either I incorrectly bisected (which is a possibility), or there are multiple (platform specific?) items that are contributing to the problem.

That was not clear, sorry.

I meant I tested with 8248 before (when I had no issue).
It's definiotely 8278.
Until then every DoPaint-call which ended with paintState abandone did a Refresh() of the control.
The no longer existing refresh might also be the cause for the not correctly working HighlightBraces, because the appropriate function is called from the UpdateUI function also.

In cbAuiNotebook I stumbled over similar issues when doing the MinimizeFreeSpace-calls (to move the tabs as far to the right as possible, if there is free space).
It did not work from inside the OnResize event, so I just set a flag in the OnResize event-chain and do the real work in an OnIdle call.
Maybe we need something similar here (setting a flag that an UpdateUI-event occured and do the real work from inside an OnIdle call).
We would need just one flag for all painting related stuff inside the UpdateUI-function, even if we would have more things later.

I will not be at home today, so I can not follow the discussion until this evening.

[dmoore]

The no longer existing refresh might also be the cause for the not correctly working HighlightBraces, because the appropriate function is called from the UpdateUI function also.

Makes sense. Doesn't that feature also rely on indicators?

Maybe we need something similar here (setting a flag that an UpdateUI-event occured and do the real work from inside an OnIdle call).
We would need just one flag for all painting related stuff inside the UpdateUI-function, even if we would have more things later.

I guess that would work, but doesn't it force an extra paint each keypress? (Probably doesn't make a difference to performance)

[Jenna]

The no longer existing refresh might also be the cause for the not correctly working HighlightBraces, because the appropriate function is called from the UpdateUI function also.

Makes sense. Doesn't that feature also rely on indicators?

Yes !

Maybe we need something similar here (setting a flag that an UpdateUI-event occured and do the real work from inside an OnIdle call).
We would need just one flag for all painting related stuff inside the UpdateUI-function, even if we would have more things later.

I guess that would work, but doesn't it force an extra paint each keypress? (Probably doesn't make a difference to performance)

Why should there be an extra paint ?

You can try the attached patch.
I also moved the NotifyPlugins call into the OnIdle-function, to ensure plugins that want to paint if the UI is updated do not run into the same trouble.

[MortenMacFly]

Until then every DoPaint-call which ended with paintState abandone did a Refresh() of the control.

Wait a sec... you should also have a look at the scintilla repo. From what I see there is a bugfix recently which may cover this, too (I am unable to present a link to a diff though). Or is this really a wxScintilla issue?

[Jenna]

Until then every DoPaint-call which ended with paintState abandone did a Refresh() of the control.

Wait a sec... you should also have a look at the scintilla repo. From what I see there is a bugfix recently which may cover this, too (I am unable to present a link to a diff though). Or is this really a wxScintilla issue?

It might be a scintilla issue, but it's discovered by a change in wxScintilla.

I try if I find the bugfix you mentioned.

[Jenna]

The only thing I can find in scintillas repo, what is related to painting and comes after the source was tagged to 3.2.2 is this:
http://scintilla.hg.sourceforge.net/hgweb/scintilla/scintilla/rev/c39df2a9f97a
But it is gtk3 and toolbar retlated and does not change anything.

[MortenMacFly]

http://scintilla.hg.sourceforge.net/hgweb/scintilla/scintilla/rev/c39df2a9f97a
But it is gtk3 and toolbar related and does not change anything.

Yes, that's what I meant. I did only recall the part with "paintAbandoned". So its not related... that's fine.

[dmoore]

I think it must be a wxScintilla issue. I got some help from the scintilla developer here

For some reason the DoFullPaint after the paintAbandoned in DoPaint doesn't seem to do a full redraw. (As we've been discussing, you can see the changes to DoPaint that might be causing this here). I wonder if the full redraw might be somehow being clipped because they need to reset the clip area? (Pure speculation)

[Jenna]

As stated here:

Some platforms behave differently when redraw calls are made inside paints so may need to schedule a paint on idle. They also differ in the information they provide about the paint region so may require overriding PaintContains.

It might be needed to paint in OnIdle-function as my patch doese.

Refresh() just invalidates the whole DC and initiates a repaint in the next message loop (this means outside the actual paint-event).
I don't know which solution is the better one.

[dmoore]

I don't know which solution is the better one.

Either one will work and should have about the same performance, so I doubt it matters.

Is it worth filing a bug against wxScintilla?

I tried to get to the bottom of this one, but keep hitting a wall. I stepped through the code with the debugger. After paintAbandoned is triggered a new DoFullPaint gets called from DoPaint. As far as I can tell the Paint call in DoFullPaint does attempt to redraw every line as should happen, but it seems to get clipped. I try clearing the clip region of the DC before the paint call, but that doesn't seem to do anything. It's hard to debug, because the debugger will often hang (especially on linux).

[dmoore]

So I guess that I have to give up and say that there is some platform limitation that prevents repaints outside the clip region in a paint handler. This affects both GTK and Windows. Not sure if it also affects Mac.

Reverting part of the 3.2.2 patch as follows would fix the problem:

Code

Index: src/sdk/wxscintilla/src/ScintillaWX.cpp
===================================================================
--- src/sdk/wxscintilla/src/ScintillaWX.cpp	(revision 8358)
+++ src/sdk/wxscintilla/src/ScintillaWX.cpp	(working copy)
@@ -911,7 +911,8 @@
     if (paintState == paintAbandoned) {
         // Painting area was insufficient to cover new styling or brace
         // highlight positions
-        FullPaintDC(dc);
+        FullPaint();
     }
     paintState = notPainting;
 }
@@ -919,8 +920,12 @@
 
 // Force the whole window to be repainted
 void ScintillaWX::FullPaint() {
-    wxClientDC dc(sci);
-    FullPaintDC(&dc);
+#ifndef __WXMAC__
+    sci->Refresh(false);
+#endif
+    sci->Update();
 }

And at least on GTK seems to be free of nasty sideeffects such as generating duplicate SCN_UPDATEUI events.
  
However, something very similar to this FullPaint/FullpaintDC code appears in the native windows Scintilla port, so if it is broken here why isn't it also broken on the native windows version? The GTK version invalidates the window, and so effectively orders a full redraw. There's a recent change to the Mac version because I guess trying to repaint on the same DC didn't work there either:

i.e. Is attempting to re-paint during a paint a "platform limitation" that applies to all known platforms?

[MortenMacFly]

Reverting part of the 3.2.2 patch as follows would fix the problem:

If it works, feel free to commit, but please encapsulate it into /* C::B begin */, /* C::B end */ comments. In fact, I was indeed a bit curious about that change, because I also saw that in SciTE and ScintillaQT this is handled differently. Hence I thought "the wx guys should know" but it seems I was wrong. I didn't realise this bug until posted here... sorry.

[Alpha]

I have tested this patch on Windows Vista and 7; it worked fine for me in those environments.