View Issue Details

IDProjectCategoryView StatusLast Update
0005577ScribusInternalpublic2007-05-01 15:54
Reportermkoren Assigned Tofschmid  
PrioritynormalSeveritymajorReproducibilityalways
Status closedResolutionfixed 
Platformx86OSDebian LinuxOS VersionSarge/Etch
Product Version1.3.3.8 
Fixed in Version1.3.3.9cvs 
Summary0005577: _Huge_ increase in SE speed when applying styles after removing bogus signal connections
Description(Note - this isn't quite a finished report, but promising enough to suggest including in 1.3.3.9 in case it's about to be released. :)

While working on a fix for the Story Editor for 0005484, I got sidetracked trying to figure out what makes it so slow at applying styles to paragraphs. (I don't know about faster computers, but on mine it's always really slow even for average paragraphs.)

After commenting out everything conceivable in the changeAlign* methods and geting no change, I then tried commenting out the connections involving "Editor, SIGNAL(textChanged()" in connectSignals(), and found that it was really fast! Since everything mostly still works without them, I think disabling them during style application and then calling the relevant slots once manually at the end would probably work.

Since the Editor class doesn't define a signal textChanged, I presume it's inherited from QT. My theory is it is emitted by QT on every single character update, and thus the methods to update the GUI or something were unintentionally being called for every character in the loops in changeAlign*, instead of once per paragraph.

I'll look at this more thoroughly tomorrow and submit a patch, but I think this will be an enormous usability improvement for working with the SE.
TagsNo tags attached.
Patch

Activities

cbradney

2007-04-23 08:59

administrator   ~0015969

Its the words/paras etc calculation thats slowing it down AFAIK. It will not be fixed in 1.3.3.x, at least, we are not spending any time on it.

cbradney

2007-04-23 19:37

administrator   ~0015972

If you find a fix that still keeps the stats updated, then please share :)

mkoren

2007-04-23 21:00

reporter   ~0015982

The stats are fine, even with this trivial hack to story.cpp:

2423c2423
< connect(Editor, SIGNAL(textChanged()), this, SLOT(modifiedText()));
---
> // connect(Editor, SIGNAL(textChanged()), this, SLOT(modifiedText()));
2430c2430
< connect(Editor, SIGNAL(textChanged()), EditorBar, SLOT(doRepaint()));
---
> // connect(Editor, SIGNAL(textChanged()), EditorBar, SLOT(doRepaint()));

They shouldn't be changing while setting a style anyway.

Anyway, it's not updating the stats that's the problem, it's doing it n times for an n-character paragraph. :) I'm compiling a proper fix now....

mkoren

2007-04-23 21:58

reporter   ~0015984

Here's a decent-looking patch that seems to work.

2007-04-23 21:59

 

story.cpp.patch1_speed.1338.diff (2,204 bytes)   
--- story.cpp	2007-02-21 17:05:34.000000000 -0600
+++ story.cpp.speedpatch	2007-04-23 16:18:26.000000000 -0500
@@ -3338,6 +3338,9 @@
 	if (Editor->StyledText.count() != 0)
 	{
 		disconnect(Editor, SIGNAL(cursorPositionChanged(int, int)), this, SLOT(updateProps(int, int)));
+		//Disconnect these too as they otherwise run for every character - Michael Koren #5577
+		disconnect(Editor, SIGNAL(textChanged()), this, SLOT(modifiedText()));
+		disconnect(Editor, SIGNAL(textChanged()), EditorBar, SLOT(doRepaint()));
 		SEditor::ChList *chars;
 		(*Editor->ParagStyles.at(pa)) = Editor->currentParaStyle;
 		if (Editor->StyledText.at(pa)->count() > 0)
@@ -3391,7 +3394,11 @@
 		Editor->setCursorPosition(pa, 0);
 		updateProps(pa, 0);
 		Editor->ensureCursorVisible();
+		//Add this once here to replace the signal. modifiedText() is called below already  - Michael Koren #5577
+		EditorBar->doRepaint();
 		connect(Editor, SIGNAL(cursorPositionChanged(int, int)), this, SLOT(updateProps(int, int)));
+		connect(Editor, SIGNAL(textChanged()), this, SLOT(modifiedText()));
+		connect(Editor, SIGNAL(textChanged()), EditorBar, SLOT(doRepaint()));
 	}
 	else
 	{
@@ -3466,6 +3473,9 @@
 	if (Editor->StyledText.count() != 0)
 	{
 		disconnect(Editor, SIGNAL(cursorPositionChanged(int, int)), this, SLOT(updateProps(int, int)));
+		//disconnect these too as they otherwise run for every character - Michael Koren #5577
+		disconnect(Editor, SIGNAL(textChanged()), this, SLOT(modifiedText()));
+		disconnect(Editor, SIGNAL(textChanged()), EditorBar, SLOT(doRepaint()));
 		int PStart, PEnd, SelStart, SelEnd, PStart2, PEnd2, SelStart2, SelEnd2;
 		SEditor::ChList *chars;
 		if (Editor->hasSelectedText())
@@ -3539,7 +3549,11 @@
 		Editor->setCursorPosition(p, i);
 		Editor->ensureCursorVisible();
 		updateProps(p, i);
+		//Add this once here to replace the signal. modifiedText() is called below already  - Michael Koren #5577
+		EditorBar->doRepaint();
 		connect(Editor, SIGNAL(cursorPositionChanged(int, int)), this, SLOT(updateProps(int, int)));
+		connect(Editor, SIGNAL(textChanged()), this, SLOT(modifiedText()));
+		connect(Editor, SIGNAL(textChanged()), EditorBar, SLOT(doRepaint()));
 	}
 	else
 	{

cbradney

2007-04-24 22:29

administrator   ~0015999

Franz, please check and apply

mkoren

2007-04-25 02:29

reporter   ~0016002

BTW this only fixes slowness changing styles; the slowness scrolling the cursor is still there, which is where the stat updating problem probably comes in. That one doesn't look easy to fix. :)

Incidentally, I'm not sure if those two connections do anything at all now that I'm calling the target slots directly at the end, so they might could just be removed in case they're slowing down something else as well, but I didn't look into it.

mkoren

2007-04-25 04:09

reporter   ~0016003

Um, it might do to look into 0005590 as well while looking at this, since it involves the same kind of updates. But that occurs both with and without this patch, and I have no idea what causes it. :)

avox

2007-04-25 22:15

administrator   ~0016012

forward ported to 1.3.4

Issue History

Date Modified Username Field Change
2007-04-23 02:57 mkoren New Issue
2007-04-23 08:59 cbradney Note Added: 0015969
2007-04-23 19:37 cbradney Note Added: 0015972
2007-04-23 21:00 mkoren Note Added: 0015982
2007-04-23 21:58 mkoren Note Added: 0015984
2007-04-23 21:59 mkoren File Added: story.cpp.patch1_speed.1338.diff
2007-04-24 22:28 cbradney Status new => assigned
2007-04-24 22:28 cbradney Assigned To => fschmid
2007-04-24 22:29 cbradney Note Added: 0015999
2007-04-25 02:29 mkoren Note Added: 0016002
2007-04-25 04:09 mkoren Note Added: 0016003
2007-04-25 19:41 fschmid Status assigned => resolved
2007-04-25 19:41 fschmid Fixed in Version => 1.3.3.9cvs
2007-04-25 19:41 fschmid Resolution open => fixed
2007-04-25 22:15 avox Note Added: 0016012
2007-05-01 15:54 cbradney Status resolved => closed