View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0005577 | Scribus | Internal | public | 2007-04-23 02:57 | 2007-05-01 15:54 |
| Reporter | mkoren | Assigned To | fschmid | ||
| Priority | normal | Severity | major | Reproducibility | always |
| Status | closed | Resolution | fixed | ||
| Platform | x86 | OS | Debian Linux | OS Version | Sarge/Etch |
| Product Version | 1.3.3.8 | ||||
| Fixed in Version | 1.3.3.9cvs | ||||
| Summary | 0005577: _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. | ||||
| Tags | No tags attached. | ||||
| Patch | |||||
|
|
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. |
|
|
If you find a fix that still keeps the stats updated, then please share :) |
|
|
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.... |
|
|
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
{
|
|
|
Franz, please check and apply |
|
|
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. |
|
|
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. :) |
|
|
forward ported to 1.3.4 |
| 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 |
