View Issue Details

IDProjectCategoryView StatusLast Update
0008670ScribusStory Editor / Text Framespublic2010-01-08 23:07
Reportermecirt Assigned Tocbradney  
PrioritynormalSeveritytextReproducibilityalways
Status closedResolutionfixed 
Platformx86-64OSLinuxOS VersionopenSuSE 11.2
Product Version1.5.0svn 
Fixed in Version1.5.0svn 
Summary0008670: Text layouting and 1.2 import speedup (patch)
DescriptionI am attaching a patch which resolves the following two issues:

First, the importer from 1.2 is very slow due to recalculating character styles after each imported character. The patch resolves this, can be tested with the odyssey.sla file attached to bug report 1036.

Second, and likely more importantly, the patch speeds up text layouting by roughly 30% (haven't done exact benchmarking). This is achieved by two changes:

- implementation of Style::validate() is moved to the header file, so that the compiler can inline the function, which in relayouting of a 250-page document is called more than 7 million times.

- getters defined by ATTRDEF return their value as a constant reference, instead of by value. In a 250-paragraph document, this removes nearly a million of memory allocations and deallocations needed due to unneeded creation of temporary ScFace objects - when returning the value by reference, the creation is not done.
Steps To ReproduceSpeedup of the 1.2 importing can be easily tested by attempting to import the odyssey file attached to report 1036.

Layouting speedup can be seen by loading some big text file (preferably not the odyssey file from 1036, as it has other issues due to the 1.2 importer being problematic), then triggering relayout for example by creating a rectangle shape and moving it over the text.
TagsNo tags attached.
Patch

Activities

2010-01-02 19:25

 

scribus-relayout-and-12import-speedup.patch (5,887 bytes)   
Index: scribus/styles/style.cpp
===================================================================
--- scribus/styles/style.cpp	(revision 14454)
+++ scribus/styles/style.cpp	(working copy)
@@ -59,15 +59,6 @@
 		m_contextversion = m_context->version(); 
 }
 
-void Style::validate() const
-{ 
-	if (m_context && m_contextversion != m_context->version()) {
-		const_cast<Style*>(this)->update(m_context); 
-		assert( m_context->checkConsistency() );
-	}
-}
-
-
 const Style* Style::parentStyle() const 
 { 
 	//qDebug() << QString("follow %1").arg(reinterpret_cast<uint>(m_context),16);
Index: scribus/styles/charstyle.h
===================================================================
--- scribus/styles/charstyle.h	(revision 14454)
+++ scribus/styles/charstyle.h	(working copy)
@@ -150,7 +150,7 @@
 	/** getter: validates and returns the attribute's value */
 	
 #define ATTRDEF(attr_TYPE, attr_GETTER, attr_NAME, attr_DEFAULT) \
-	attr_TYPE attr_GETTER() const { validate(); return m_##attr_NAME; }
+	const attr_TYPE &attr_GETTER() const { validate(); return m_##attr_NAME; }
 #include "charstyle.attrdefs.cxx"
 #undef ATTRDEF
 	
Index: scribus/styles/linestyle.h
===================================================================
--- scribus/styles/linestyle.h	(revision 14454)
+++ scribus/styles/linestyle.h	(working copy)
@@ -73,7 +73,7 @@
 	/** getter: validates and returns the attribute's value */
 	
 #define ATTRDEF(attr_TYPE, attr_GETTER, attr_NAME, attr_DEFAULT) \
-	attr_TYPE attr_GETTER() const { validate(); return m_##attr_NAME; }
+	const attr_TYPE &attr_GETTER() const { validate(); return m_##attr_NAME; }
 #include "linestyle.attrdefs.cxx"
 #undef ATTRDEF
 	
Index: scribus/styles/paragraphstyle.h
===================================================================
--- scribus/styles/paragraphstyle.h	(revision 14454)
+++ scribus/styles/paragraphstyle.h	(working copy)
@@ -110,7 +110,7 @@
 	/** getter: validates and returns the attribute's value */
 	
 #define ATTRDEF(attr_TYPE, attr_GETTER, attr_NAME, attr_DEFAULT) \
-	attr_TYPE attr_GETTER() const { validate(); return m_##attr_NAME; }
+	const attr_TYPE &attr_GETTER() const { validate(); return m_##attr_NAME; }
 #include "paragraphstyle.attrdefs.cxx"
 #undef ATTRDEF
 	
Index: scribus/style.h
===================================================================
--- scribus/style.h	(revision 14454)
+++ scribus/style.h	(working copy)
@@ -110,7 +110,12 @@
 	/**
 		Checks if this Style needs an update
 	 */
-	void validate() const;
+	void validate() const {
+		if (m_context && m_contextversion != m_context->version()) {
+			const_cast<Style*>(this)->update(m_context); 
+			assert( m_context->checkConsistency() );
+		}
+	}
 
 	QString shortcut() const { return m_shortcut; }
 	void setShortcut(const QString &shortcut) { m_shortcut = shortcut; }
Index: scribus/plugins/fileloader/scribus12format/scribus12format.cpp
===================================================================
--- scribus/plugins/fileloader/scribus12format/scribus12format.cpp	(revision 14454)
+++ scribus/plugins/fileloader/scribus12format/scribus12format.cpp	(working copy)
@@ -1104,42 +1104,44 @@
 	int ulw = qRound(ScCLocale::toDoubleC(it->attribute("CULW"), -0.1) * 10);
 	int stp = qRound(ScCLocale::toDoubleC(it->attribute("CSTP"), -0.1) * 10);
 	int stw = qRound(ScCLocale::toDoubleC(it->attribute("CSTW"), -0.1) * 10);
+
+	CharStyle style;
+	style.setFont((*doc->AllFonts)[tmpf]);
+	style.setFontSize(size);
+	style.setFillColor(fcolor);
+	style.setTracking(extra);
+	style.setFillShade(shade);
+	style.setFeatures(static_cast<StyleFlag>(cstyle).featureList());
+	style.setStrokeColor(stroke);
+	style.setStrokeShade(shade2);
+	style.setScaleH(qMin(qMax(scale, 100), 4000));
+	style.setScaleV(qMin(qMax(scalev, 100), 4000));
+	style.setBaselineOffset(base);
+	style.setShadowXOffset(shX);
+	style.setShadowYOffset(shY);
+	style.setOutlineWidth(outL);
+	style.setUnderlineOffset(ulp);
+	style.setUnderlineWidth(ulw);
+	style.setStrikethruOffset(stp);
+	style.setStrikethruWidth(stw);
+
+	int pos = obj->itemText.length();
+	if (style != last->Style) {
+		last->Style = style;
+		last->StyleStart = pos;
+	}
+
+
 	for (int cxx=0; cxx<tmp2.length(); ++cxx)
 	{
-		CharStyle style;
 		QChar ch = tmp2.at(cxx);
 		if (ch == QChar(5))
 			ch = SpecialChars::PARSEP;
 		if (ch == QChar(4))
 			ch = SpecialChars::TAB;
-		style.setFont((*doc->AllFonts)[tmpf]);
-		style.setFontSize(size);
-		style.setFillColor(fcolor);
-		style.setTracking(extra);
-		style.setFillShade(shade);
-		style.setFeatures(static_cast<StyleFlag>(cstyle).featureList());
-		style.setStrokeColor(stroke);
-		style.setStrokeShade(shade2);
-		style.setScaleH(qMin(qMax(scale, 100), 4000));
-		style.setScaleV(qMin(qMax(scalev, 100), 4000));
-		style.setBaselineOffset(base);
-		style.setShadowXOffset(shX);
-		style.setShadowYOffset(shY);
-		style.setOutlineWidth(outL);
-		style.setUnderlineOffset(ulp);
-		style.setUnderlineWidth(ulw);
-		style.setStrikethruOffset(stp);
-		style.setStrikethruWidth(stw);
 		int pos = obj->itemText.length();
 		obj->itemText.insertChars(pos, QString(ch));
 
-		if (style != last->Style) {
-			//	qDebug() << QString("new style at %1: %2 -> %3").arg(pos).arg(last->Style.asString()).arg(newStyle.asString());
-			obj->itemText.applyCharStyle(last->StyleStart, pos-last->StyleStart, last->Style);
-			last->Style = style;
-			last->StyleStart = pos;
-		}
-
 		if (ch == SpecialChars::PARSEP || cxx+1 == tmp2.length()) {
 //			qDebug() << QString("scribus12 para: %1 %2 %3 %4").arg(ab)
 //				   .arg(ab < signed(DoVorl.size())? DoVorl[ab] : QString("./."))
@@ -1155,7 +1157,6 @@
 		}
 	}
 	obj->itemText.applyCharStyle(last->StyleStart, obj->itemText.length()-last->StyleStart, last->Style);
-	return;
 }
 
 bool Scribus12Format::loadPage(const QString & fileName, int pageNumber, bool Mpage, QString /*renamedPageName*/)

cbradney

2010-01-02 23:19

administrator   ~0023017

Thank you!.. Applied. Added the inline keyword to be clear.

Issue History

Date Modified Username Field Change
2010-01-02 19:25 mecirt New Issue
2010-01-02 19:25 mecirt File Added: scribus-relayout-and-12import-speedup.patch
2010-01-02 23:19 cbradney Note Added: 0023017
2010-01-02 23:19 cbradney Status new => resolved
2010-01-02 23:19 cbradney Fixed in Version => 1.5.0svn
2010-01-02 23:19 cbradney Resolution open => fixed
2010-01-02 23:19 cbradney Assigned To => cbradney
2010-01-08 23:07 cbradney Status resolved => closed
2015-09-17 20:08 Kunda Category Story Editor / Text Frames => Story Ed/Txt Frames
2015-09-17 20:12 Kunda Category Story Ed/Txt Frames => Story Editor / Text Frames