View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0015419 | Scribus | Internal | public | 2018-09-06 19:00 | 2019-06-04 20:56 |
Reporter | ale | Assigned To | jghali | ||
Priority | normal | Severity | minor | Reproducibility | N/A |
Status | closed | Resolution | fixed | ||
Product Version | 1.5.5.svn | ||||
Fixed in Version | 1.5.5.svn | ||||
Summary | 0015419: small refactoring for scribusXml | ||||
Description | here is the patch for one of the change propositions i have mentioned in the mailing list... - rename cryptic variables - refactor out a function | ||||
Tags | No tags attached. | ||||
Patch | No | ||||
|
no-emg-or-emmap.diff (2,123 bytes)
diff --git a/scribus/scribusXml.cpp b/scribus/scribusXml.cpp index 191aa6e77..afb196e1b 100644 --- a/scribus/scribusXml.cpp +++ b/scribus/scribusXml.cpp @@ -160,14 +160,9 @@ QString ScriXmlDoc::WriteElem(ScribusDoc *doc, Selection* selection) PageItem *item; QString documentStr = ""; item = selection->itemAt(0); - QList<PageItem*> emG; - QMap<int, PageItem*> emMap; - emG.clear(); - for (int cor = 0; cor < selection->count(); ++cor) - { - emMap.insert(doc->Items->indexOf(selection->itemAt(cor)), selection->itemAt(cor)); - } - emG = emMap.values(); + + auto items = getItemsFromSelection(doc, selection); + double selectionWidth = 0; double selectionHeight = 0; if (selection->isMultipleSelection()) @@ -198,9 +193,9 @@ QString ScriXmlDoc::WriteElem(ScribusDoc *doc, Selection* selection) retImg.fill( qRgba(0, 0, 0, 0) ); ScPainter *painter = new ScPainter(&retImg, retImg.width(), retImg.height(), 1, 0); painter->setZoomFactor(scaleI); - for (int em = 0; em < emG.count(); ++em) + for (int em = 0; em < items.count(); ++em) { - PageItem* embedded = emG.at(em); + PageItem* embedded = items.at(em); painter->save(); painter->translate(-xp, -yp); embedded->invalid = true; @@ -234,3 +229,13 @@ ScElemMimeData* ScriXmlDoc::WriteToMimeData(ScribusDoc *doc, Selection *selectio md->setScribusElem(WriteElem(doc, selection)); return md; } + +QList<PageItem*> ScriXmlDoc::getItemsFromSelection(ScribusDoc *doc, Selection* selection) +{ + QList<PageItem*> result; + QMap<int, PageItem*> items; + for (auto item: selection->items()) { + items.insert(doc->Items->indexOf(item), item); + } + return items.values(); +} diff --git a/scribus/scribusXml.h b/scribus/scribusXml.h index 53e51237e..5d0a269c3 100644 --- a/scribus/scribusXml.h +++ b/scribus/scribusXml.h @@ -53,9 +53,11 @@ public: static QString WriteElem(ScribusDoc *doc, Selection *selection); static ScElemMimeData* WriteToMimeData(ScribusDoc *doc, Selection *selection); +private: + static QList<PageItem*> getItemsFromSelection(ScribusDoc *doc, Selection* selection); }; -#endif // _SCRIBUS_CONFIG_ +#endif |
|
... even if i don't understand what's the use of first creating a map... it would probably be simpler to directly create a vector and, then, eventually sort it (if it's really needed...) |
|
>> it would probably be simpler to directly create a vector and, then, eventually sort it (if it's really needed...) QMap is ordered, ie QMap elements are already sorted by keys. We need to keep elements in stack order here and QMap allows us to do that simply. |
|
Committed with a small change. Be careful when using range for with Qt containers. On such containers range for loops are more efficient if the iterated container is const. If it's not, range for will call non const version of begin() and end() which may detach the container, ie create a copy of its data... which will be pretty slow. For non const containers, Qt's foreach will often be more efficient than C++11 range for. |
|
ok, thanks for the correction! i did not know about it! for the stack order: i was mostly wondering, why the items are not in the right from the beginning... instead of ordering them each time they get used... but, well, maybe most element are never used so it might be too much effort to keep the whole document ordered (if it's possible at all). and thanks for committing the patch... |
Date Modified | Username | Field | Change |
---|---|---|---|
2018-09-06 19:00 | ale | New Issue | |
2018-09-06 19:00 | ale | File Added: no-emg-or-emmap.diff | |
2018-09-06 19:02 | ale | Note Added: 0045436 | |
2018-09-06 21:11 | jghali | Category | - => Internal |
2018-09-07 19:17 | jghali | Note Added: 0045443 | |
2018-09-07 19:24 | jghali | Note Edited: 0045443 | |
2018-09-07 19:47 | jghali | Summary | small refactoring in scribusXml => small refactoring for scribusXml |
2018-09-07 19:51 | jghali | Note Added: 0045444 | |
2018-09-07 19:52 | jghali | Assigned To | => jghali |
2018-09-07 19:52 | jghali | Status | new => resolved |
2018-09-07 19:52 | jghali | Resolution | open => fixed |
2018-09-07 19:52 | jghali | Fixed in Version | => 1.5.5.svn |
2018-09-07 19:56 | jghali | Note Edited: 0045444 | |
2018-09-07 20:30 | jghali | Note Edited: 0045444 | |
2018-09-09 13:49 | ale | Note Added: 0045449 | |
2019-06-04 20:56 | cbradney | Status | resolved => closed |