Slow scrolling? [Ubuntu]

[dmoore]

By the way making the amount of lines per wheel-click configurable is a good thing in my opinion.
Shall I prepare a patch for it ?

I think the problem with the scrolling is that there is no acceleration, not that the scrolling is slow.
At least firefox seems to have some acceleration in it, but I may be wrong as my mouse doesn't have a lock which limits the rotations (the mechanism making the mouse click when scrolling).

put another way, discarding mousewheel events prevents scrolling above a certain speed.

EDIT: I should add that I think making the mousewheel scrolling increment customizable is nonetheless a good idea. Happy to test a patch.

[Jenna]

Could you please test the attached patch to Editor.cxx.

It should optimize scrolling of just a few lines, but at least on gtk (when calling ScintillaWX::ScrollText() ) it seems to slow down scrolling a lot.

[dmoore]

Could you please test the attached patch to Editor.cxx.

It should optimize scrolling of just a few lines, but at least on gtk (when calling ScintillaWX::ScrollText() ) it seems to slow down scrolling a lot.

Well done Jens! Looks like that's the fix. Many thanks. I don't think I would ever have found this myself. (Curious how you figured out that this was the problem).

This appears to fix mousewheel scrolling too! (or trackpad scrolling, at least)

So the odd thing is that, at least for me, those calls seemed to only create problems with C::B. When I run the STC sample I don't have a problem. Even if I put a scintilla widget in a separate window in C::B, which you think would be as close to the wxWidgets STC sample as possible, the problem returns. Is it possible that the drawing to screen was creating problems with wxAUI?

[dmoore]

So this is odd:

Code

void Editor::ScrollText(int /* linesToMove */) {
	//Platform::DebugPrintf("Editor::ScrollText %d\n", linesToMove);
	Redraw();
}

So if they hadn't done this

Code

void ScintillaWX::ScrollText(int linesToMove) {
    int dy = vs.lineHeight * (linesToMove);
    sci->ScrollWindow(0, dy);
    sci->Update();
}

(Editor is a base class of SctinillaWX)

Then there would be no lag.

[dmoore]

So this patch actually seems to give slightly better performance

Code

Index: src/sdk/wxscintilla/src/ScintillaWX.cpp
===================================================================
--- src/sdk/wxscintilla/src/ScintillaWX.cpp	(revision 8138)
+++ src/sdk/wxscintilla/src/ScintillaWX.cpp	(working copy)
@@ -417,7 +417,7 @@
 void ScintillaWX::ScrollText(int linesToMove) {
     int dy = vs.lineHeight * (linesToMove);
     sci->ScrollWindow(0, dy);
-    sci->Update();
+//    sci->Update(); //causes slow-down on GTK+ systems (Linux)
 }
 
 void ScintillaWX::SetVerticalScrollPos() {

I don't know if the missing Update will create artifacts? I didn't see any issues. (Maybe on windows? -- I did not test)

[dmoore]

We can probably also drop the crap related to ticket #9057. (Although it doesn't seem to do any harm either)

BTW, comparing scite with C::B without the sci->update call:
* by default, scite scrolls 4 lines per wheel motion vs 3 lines in C:::B.
* a full mousewheel motion (moving as much as possible in one motion) in scite scrolls about 70 lines vs 35 lines in C::B.

I personally prefer C::B's current behavior, but it would be good to add the customization to the lines per motion. But perhaps scite also allows for some acceleration of the step size upon repeated moves? That might be nice to have.

[MortenMacFly]

Could you please test the attached patch to Editor.cxx.

Ah - my favourite again: "Error: Chunk info expected.". Nevermind - these few lines I can do by hand...

[dmoore]

Could you please test the attached patch to Editor.cxx.

Ah - my favourite again: "Error: Chunk info expected.". Nevermind - these few lines I can do by hand...

The one liner I propose about would be instead of the one Jens supplied (a mere refinement of his fine work, of course).

[Jenna]

Could you please test the attached patch to Editor.cxx.

Ah - my favourite again: "Error: Chunk info expected.". Nevermind - these few lines I can do by hand...

I will test it with tortoise later the day.
It should wotk, but I removed the comment lines from git manually, maybe I removed too much (it was late yesterday).

[Jenna]

So this is odd:

Code

void Editor::ScrollText(int /* linesToMove */) {
	//Platform::DebugPrintf("Editor::ScrollText %d\n", linesToMove);
	Redraw();
}

So if they hadn't done this

Code

void ScintillaWX::ScrollText(int linesToMove) {
    int dy = vs.lineHeight * (linesToMove);
    sci->ScrollWindow(0, dy);
    sci->Update();
}

(Editor is a base class of SctinillaWX)

Then there would be no lag.

I thought about doing this way, but I decided to patch scintilla's sources (again), because the (admittedly small) overhead for is not needed at all.
I did not do any exact measuring which of the two ways is shorter.
But the original ScrollTo took about 35 ~ 50 ms per line and the "new" one takes 0 ~ 1 ms, so it does not really matter if it is takes another 0.5 ms or not I think.

[Jenna]

We can probably also drop the crap related to ticket #9057. (Although it doesn't seem to do any harm either)

BTW, comparing scite with C::B without the sci->update call:
* by default, scite scrolls 4 lines per wheel motion vs 3 lines in C:::B.
* a full mousewheel motion (moving as much as possible in one motion) in scite scrolls about 70 lines vs 35 lines in C::B.

I personally prefer C::B's current behavior, but it would be good to add the customization to the lines per motion. But perhaps scite also allows for some acceleration of the step size upon repeated moves? That might be nice to have.

At least the amount of scrolled lines can be made configurable (as posted before), because wxWidgets uses three lines in any cases (until) they find a way to find the systems preferred scroll amount.
It might also be possible to accelerate the scrolling, if more scroll events come in in short time, but this needs a threshhold to determin what is short and a threshhold for the maximal acceleration.

[dmoore]

I thought about doing this way, but I decided to patch scintilla's sources (again), because the (admittedly small) overhead for is not needed at all.
I did not do any exact measuring which of the two ways is shorter.
But the original ScrollTo took about 35 ~ 50 ms per line and the "new" one takes 0 ~ 1 ms, so it does not really matter if it is takes another 0.5 ms or not I think.

On my netbook I thought there was a slight performance improvement to doing the patch the way I proposed (just removing the sci->Update call). I could be imagining things, but it did seem to help to have the partial scroll instead of full redraw.

[Jenna]

I did a short mesurement with wx2.9, because the stopwatch there has µs.
The avaregd time call to ScrollTo differs for about 25 µs, so it is probably better to change ScintillWX instead of scintilla's sources (again).

[MortenMacFly]

so it is probably better to change ScintillWX instead of scintilla's sources (again).

...meaning your patch is obsolete and superseeded by the ones of dmoore? I wonder if it has side-effects on Windows...

[Jenna]

so it is probably better to change ScintillWX instead of scintilla's sources (again).

...meaning your patch is obsolete and superseeded by the ones of dmoore? I wonder if it has side-effects on Windows...

It's basically the same, it avoids the call of wxWindows::Update() on every scrol, which immediately updates the whole invalidated area.
But the original scintilla Redraw() invalidates the whole client area and the other one (most likely) only the scrolled part.

But in both cases real repainting is done later.

I will test it on windows.