View Issue Details

IDProjectCategoryView StatusLast Update
0011026ScribusFontspublic2012-11-13 20:39
Reportercezaryece Assigned Tojghali  
PrioritynormalSeverityminorReproducibilityN/A
Status closedResolutionfixed 
Product Version1.5.0svn 
Fixed in Version1.5.0svn 
Summary0011026: Speed up some font routines
DescriptionUsing valgrind profiler I have found that lot of PageItem_TextFrame::layout() cost take glyphLayout() routine, specially QMap lookup operations.
In small investigation and reading of Qt manual I have found that QHash container is much faster in lookup than QMap.
Replacing QMap declarations by QHash in scface_ttf.h I got faster text layouting.

Results from valgrind while opening some big file (above 600 000 chars):
PageItem_TextFrame::layout() cost (I think in processor cycles)
with QMap - 555 524 Mega
with QHash - 293 423 Mega

Measuring test file opening time results are not so spectacular, but it reduces from 21 sec to 18 sec, but looks like initial font initialization takes longer time now, but layouting time was reduced about 30%.

I think there may be many other places where such optimization is possible giving further speed up in text layout what is very important for handling of long documents in Scribus.
Additional InformationAll changes in diff file.
TagsNo tags attached.
Patch

Activities

cezaryece

2012-08-10 09:15

developer  

scface_ttf_speedup.diff (1,932 bytes)   
diff --git a/Scribus/scribus/fonts/scface_ttf.h b/Scribus/scribus/fonts/scface_ttf.h
index dc57b33..83a2db5 100644
--- a/Scribus/scribus/fonts/scface_ttf.h
+++ b/Scribus/scribus/fonts/scface_ttf.h
@@ -10,7 +10,7 @@ for which a new license (GPL+exception) is in place.
 #include "scribusapi.h"
 #include "fonts/ftface.h"
 
-
+#include <QHash>
 #include FT_TRUETYPE_TABLES_H
 #include FT_TRUETYPE_TAGS_H
 
@@ -21,7 +21,7 @@ for which a new license (GPL+exception) is in place.
  */
 class SCRIBUS_API KernFeature
 {
-	typedef QMap<quint16, QList<quint16> > ClassDefTable; // <Class index (0 to N) , list of glyphs >
+	typedef QHash<quint16, QList<quint16> > ClassDefTable; // <Class index (0 to N) , list of glyphs >
 
 	public:
 		/**
@@ -49,11 +49,11 @@ class SCRIBUS_API KernFeature
 	private:
 		bool m_valid;
 		QByteArray GPOSTableRaw;
-		QMap<quint16,QList<quint16> > coverages;
-		mutable QMap<quint16, QMap<quint16, double> > pairs;
-		QMap< quint16, QMap<quint16, ClassDefTable> > classGlyphFirst; // < subtable offset, map<offset, class definition table> > for first glyph
-		QMap< quint16, QMap<quint16, ClassDefTable> > classGlyphSecond; // < subtable offset, map<offset, class definition table> > for second glyph
-		QMap< quint16, QMap<int, QMap<int, double> > > classValue; // < subtable offset, map<class1, map<class2, value> > >
+		QHash<quint16,QList<quint16> > coverages;
+		mutable QHash<quint16, QHash<quint16, double> > pairs;
+		QHash< quint16, QHash<quint16, ClassDefTable> > classGlyphFirst; // < subtable offset, map<offset, class definition table> > for first glyph
+		QHash< quint16, QHash<quint16, ClassDefTable> > classGlyphSecond; // < subtable offset, map<offset, class definition table> > for second glyph
+		QHash< quint16, QHash<int, QHash<int, double> > > classValue; // < subtable offset, map<class1, map<class2, value> > >
 
 		void makeCoverage();
 		void makePairs ( quint16 subtableOffset );
scface_ttf_speedup.diff (1,932 bytes)   

avox

2012-08-10 09:26

administrator   ~0028798

good catch. maybe we should take a closer look at OTF kerning in general. All other table lookups are handled by freetype

cezaryece

2012-08-10 13:49

developer  

text_layout_speedup.diff (30,333 bytes)   
diff --git a/Scribus/scribus/fonts/scface.h b/Scribus/scribus/fonts/scface.h
index 8281505..e3e81f5 100644
--- a/Scribus/scribus/fonts/scface.h
+++ b/Scribus/scribus/fonts/scface.h
@@ -23,6 +23,7 @@ virtual:      dispatch to constituents, handle embedding (-)
 #include <QString>
 //#include <QVector>
 #include <QMap>
+#include <QHash>
 //#include <QArray>
 #include <utility>
 
@@ -133,9 +134,9 @@ public:
 		Status cachedStatus;
 		
 		// caches
-		mutable QMap<uint,qreal>    m_glyphWidth;
-		mutable QMap<uint,GlyphData> m_glyphOutline;
-		mutable QMap<uint, uint>     m_cMap;
+		mutable QHash<int,qreal>    m_glyphWidth;
+		mutable QHash<int,GlyphData> m_glyphOutline;
+		mutable QHash<int, int>     m_cMap;
 		
 		// fill caches & members
 		
diff --git a/Scribus/scribus/fonts/scface_ttf.h b/Scribus/scribus/fonts/scface_ttf.h
index dc57b33..83a2db5 100644
--- a/Scribus/scribus/fonts/scface_ttf.h
+++ b/Scribus/scribus/fonts/scface_ttf.h
@@ -10,7 +10,7 @@ for which a new license (GPL+exception) is in place.
 #include "scribusapi.h"
 #include "fonts/ftface.h"
 
-
+#include <QHash>
 #include FT_TRUETYPE_TABLES_H
 #include FT_TRUETYPE_TAGS_H
 
@@ -21,7 +21,7 @@ for which a new license (GPL+exception) is in place.
  */
 class SCRIBUS_API KernFeature
 {
-	typedef QMap<quint16, QList<quint16> > ClassDefTable; // <Class index (0 to N) , list of glyphs >
+	typedef QHash<quint16, QList<quint16> > ClassDefTable; // <Class index (0 to N) , list of glyphs >
 
 	public:
 		/**
@@ -49,11 +49,11 @@ class SCRIBUS_API KernFeature
 	private:
 		bool m_valid;
 		QByteArray GPOSTableRaw;
-		QMap<quint16,QList<quint16> > coverages;
-		mutable QMap<quint16, QMap<quint16, double> > pairs;
-		QMap< quint16, QMap<quint16, ClassDefTable> > classGlyphFirst; // < subtable offset, map<offset, class definition table> > for first glyph
-		QMap< quint16, QMap<quint16, ClassDefTable> > classGlyphSecond; // < subtable offset, map<offset, class definition table> > for second glyph
-		QMap< quint16, QMap<int, QMap<int, double> > > classValue; // < subtable offset, map<class1, map<class2, value> > >
+		QHash<quint16,QList<quint16> > coverages;
+		mutable QHash<quint16, QHash<quint16, double> > pairs;
+		QHash< quint16, QHash<quint16, ClassDefTable> > classGlyphFirst; // < subtable offset, map<offset, class definition table> > for first glyph
+		QHash< quint16, QHash<quint16, ClassDefTable> > classGlyphSecond; // < subtable offset, map<offset, class definition table> > for second glyph
+		QHash< quint16, QHash<int, QHash<int, double> > > classValue; // < subtable offset, map<class1, map<class2, value> > >
 
 		void makeCoverage();
 		void makePairs ( quint16 subtableOffset );
diff --git a/Scribus/scribus/pageitem.cpp b/Scribus/scribus/pageitem.cpp
index cd7f342..18c67e8 100644
--- a/Scribus/scribus/pageitem.cpp
+++ b/Scribus/scribus/pageitem.cpp
@@ -2302,7 +2302,8 @@ void PageItem::SetQColor(QColor *tmp, QString farbe, double shad)
 double PageItem::layoutGlyphs(const CharStyle& style, const QString& chars, GlyphLayout& layout)
 {
 	double retval = 0.0;
-	double asce = style.font().ascent(style.fontSize() / 10.0);
+	const ScFace* font = &style.font();
+	double asce = font->ascent(style.fontSize() / 10.0);
 	int chst = style.effects() & 1919;
 /*	if (chars[0] == SpecialChars::ZWSPACE ||
 		chars[0] == SpecialChars::ZWNBSPACE ||
@@ -2319,7 +2320,7 @@ double PageItem::layoutGlyphs(const CharStyle& style, const QString& chars, Glyp
 	}
 	else */
 	{
-		layout.glyph = style.font().char2CMap(chars[0].unicode());
+		layout.glyph = font->char2CMap(chars[0].unicode());
 	}
 	double tracking = 0.0;
 	if ( (style.effects() & ScStyle_StartOfLine) == 0)
@@ -2348,7 +2349,7 @@ double PageItem::layoutGlyphs(const CharStyle& style, const QString& chars, Glyp
 		layout.scaleV *= style.scaleV() / 1000.0;
 		if (chst & ScStyle_AllCaps)
 		{
-			layout.glyph = style.font().char2CMap(chars[0].toUpper().unicode());
+			layout.glyph = font->char2CMap(chars[0].toUpper().unicode());
 		}
 		if (chst & ScStyle_SmallCaps)
 		{
@@ -2356,7 +2357,7 @@ double PageItem::layoutGlyphs(const CharStyle& style, const QString& chars, Glyp
 			QChar uc = chars[0].toUpper();
 			if (uc != chars[0])
 			{
-				layout.glyph = style.font().char2CMap(chars[0].toUpper().unicode());
+				layout.glyph = font->char2CMap(chars[0].toUpper().unicode());
 				layout.scaleV *= smallcapsScale;
 				layout.scaleH *= smallcapsScale;
 			}
@@ -2368,14 +2369,14 @@ double PageItem::layoutGlyphs(const CharStyle& style, const QString& chars, Glyp
 	}	
 	
 /*	if (layout.glyph == (ScFace::CONTROL_GLYPHS + SpecialChars::NBSPACE.unicode())) {
-		uint replGlyph = style.font().char2CMap(QChar(' '));
-		layout.xadvance = style.font().glyphWidth(replGlyph, style.fontSize() / 10) * layout.scaleH;
-		layout.yadvance = style.font().glyphBBox(replGlyph, style.fontSize() / 10).ascent * layout.scaleV;
+		uint replGlyph = font->char2CMap(QChar(' '));
+		layout.xadvance = font->glyphWidth(replGlyph, style.fontSize() / 10) * layout.scaleH;
+		layout.yadvance = font->glyphBBox(replGlyph, style.fontSize() / 10).ascent * layout.scaleV;
 	}
 	else if (layout.glyph == (ScFace::CONTROL_GLYPHS + SpecialChars::NBHYPHEN.unicode())) {
-		uint replGlyph = style.font().char2CMap(QChar('-'));
-		layout.xadvance = style.font().glyphWidth(replGlyph, style.fontSize() / 10) * layout.scaleH;
-		layout.yadvance = style.font().glyphBBox(replGlyph, style.fontSize() / 10).ascent * layout.scaleV;
+		uint replGlyph = font->char2CMap(QChar('-'));
+		layout.xadvance = font->glyphWidth(replGlyph, style.fontSize() / 10) * layout.scaleH;
+		layout.yadvance = font->glyphBBox(replGlyph, style.fontSize() / 10).ascent * layout.scaleV;
 	}
 	else if (layout.glyph >= ScFace::CONTROL_GLYPHS) {
 		layout.xadvance = 0;
@@ -2383,8 +2384,8 @@ double PageItem::layoutGlyphs(const CharStyle& style, const QString& chars, Glyp
 	}
 	else */
 	{
-		layout.xadvance = style.font().glyphWidth(layout.glyph, style.fontSize() / 10) * layout.scaleH;
-		layout.yadvance = style.font().glyphBBox(layout.glyph, style.fontSize() / 10).ascent * layout.scaleV;
+		layout.xadvance = font->glyphWidth(layout.glyph, style.fontSize() / 10) * layout.scaleH;
+		layout.yadvance = font->glyphBBox(layout.glyph, style.fontSize() / 10).ascent * layout.scaleV;
 	}
 	if (layout.xadvance > 0)
 		layout.xadvance += tracking;
@@ -2392,7 +2393,7 @@ double PageItem::layoutGlyphs(const CharStyle& style, const QString& chars, Glyp
 	if (chars.length() > 1) {
 		layout.grow();
 		layoutGlyphs(style, chars.mid(1), *layout.more);
-		layout.xadvance += style.font().glyphKerning(layout.glyph, layout.more->glyph, style.fontSize() / 10) * layout.scaleH;
+		layout.xadvance += font->glyphKerning(layout.glyph, layout.more->glyph, style.fontSize() / 10) * layout.scaleH;
 		if (layout.more->yadvance > layout.yadvance)
 			layout.yadvance = layout.more->yadvance;
 	}
@@ -2405,8 +2406,9 @@ double PageItem::layoutGlyphs(const CharStyle& style, const QString& chars, Glyp
 void PageItem::drawGlyphs(ScPainter *p, const CharStyle& style, GlyphLayout& glyphs)
 {
 	uint glyph = glyphs.glyph;
+	const ScFace* font = &style.font();
 	if ((m_Doc->guidesPrefs().showControls) &&
-		(glyph == style.font().char2CMap(QChar(' ')) || glyph >=  ScFace::CONTROL_GLYPHS))
+		(glyph == font->char2CMap(QChar(' ')) || glyph >=  ScFace::CONTROL_GLYPHS))
 	{
 		bool stroke = false;
 		if (glyph >=  ScFace::CONTROL_GLYPHS)
@@ -2449,7 +2451,7 @@ void PageItem::drawGlyphs(ScPainter *p, const CharStyle& style, GlyphLayout& gly
 		}
 		else if (glyph == SpecialChars::NBHYPHEN.unicode())
 		{
-			points = style.font().glyphOutline(style.font().char2CMap(QChar('-')), style.fontSize() / 100);
+			points = font->glyphOutline(font->char2CMap(QChar('-')), style.fontSize() / 100);
 			chma4.translate(glyphs.xoffset, glyphs.yoffset-((style.fontSize() / 10.0) * glyphs.scaleV));
 		}
 		else if (glyph == SpecialChars::SHYPHEN.unicode())
@@ -2495,9 +2497,9 @@ void PageItem::drawGlyphs(ScPainter *p, const CharStyle& style, GlyphLayout& gly
 	}
 	else if (glyph == (ScFace::CONTROL_GLYPHS + SpecialChars::NBSPACE.unicode()) ||
 			 glyph == (ScFace::CONTROL_GLYPHS + 32)) 
-		glyph = style.font().char2CMap(QChar(' '));
+		glyph = font->char2CMap(QChar(' '));
 	else if (glyph == (ScFace::CONTROL_GLYPHS + SpecialChars::NBHYPHEN.unicode()))
-		glyph = style.font().char2CMap(QChar('-'));
+		glyph = font->char2CMap(QChar('-'));
 	
 	if (glyph >= ScFace::CONTROL_GLYPHS || (style.effects() & ScStyle_SuppressSpace)) {
 //		qDebug("drawGlyphs: skipping %d", glyph);
@@ -2509,29 +2511,29 @@ void PageItem::drawGlyphs(ScPainter *p, const CharStyle& style, GlyphLayout& gly
 		}			
 		return;
 	}
-//	if (style.font().canRender(QChar(glyph)))
+//	if (font->canRender(QChar(glyph)))
 	{
-		FPointArray gly = style.font().glyphOutline(glyph);
+		FPointArray gly = font->glyphOutline(glyph);
 		// Do underlining first so you can get typographically correct
 		// underlines when drawing a white outline
-		if (((style.effects() & ScStyle_Underline) || ((style.effects() & ScStyle_UnderlineWords) && glyph != style.font().char2CMap(QChar(' ')))) && (style.strokeColor() != CommonStrings::None))
+		if (((style.effects() & ScStyle_Underline) || ((style.effects() & ScStyle_UnderlineWords) && glyph != font->char2CMap(QChar(' ')))) && (style.strokeColor() != CommonStrings::None))
 		{
 			double st, lw;
 			if ((style.underlineOffset() != -1) || (style.underlineWidth() != -1))
 			{
 				if (style.underlineOffset() != -1)
-					st = (style.underlineOffset() / 1000.0) * (style.font().descent(style.fontSize() / 10.0));
+					st = (style.underlineOffset() / 1000.0) * (font->descent(style.fontSize() / 10.0));
 				else
-					st = style.font().underlinePos(style.fontSize() / 10.0);
+					st = font->underlinePos(style.fontSize() / 10.0);
 				if (style.underlineWidth() != -1)
 					lw = (style.underlineWidth() / 1000.0) * (style.fontSize() / 10.0);
 				else
-					lw = qMax(style.font().strokeWidth(style.fontSize() / 10.0), 1.0);
+					lw = qMax(font->strokeWidth(style.fontSize() / 10.0), 1.0);
 			}
 			else
 			{
-				st = style.font().underlinePos(style.fontSize() / 10.0);
-				lw = qMax(style.font().strokeWidth(style.fontSize() / 10.0), 1.0);
+				st = font->underlinePos(style.fontSize() / 10.0);
+				lw = qMax(font->strokeWidth(style.fontSize() / 10.0), 1.0);
 			}
 			if (style.baselineOffset() != 0)
 				st += (style.fontSize() / 10.0) * glyphs.scaleV * (style.baselineOffset() / 1000.0);
@@ -2597,7 +2599,7 @@ void PageItem::drawGlyphs(ScPainter *p, const CharStyle& style, GlyphLayout& gly
 				p->setLineWidth(style.fontSize() * glyphs.scaleV * style.outlineWidth() * 2 / 10000.0);
 				p->strokePath();
 			}
-			else if ((style.font().isStroked()) && (style.strokeColor() != CommonStrings::None) && ((style.fontSize() * glyphs.scaleV * style.outlineWidth() / 10000.0) != 0))
+			else if ((font->isStroked()) && (style.strokeColor() != CommonStrings::None) && ((style.fontSize() * glyphs.scaleV * style.outlineWidth() / 10000.0) != 0))
 			{
 				QColor tmp = p->brush();
 				p->setPen(tmp, 1, Qt::SolidLine, Qt::FlatCap, Qt::MiterJoin);
@@ -2638,18 +2640,18 @@ void PageItem::drawGlyphs(ScPainter *p, const CharStyle& style, GlyphLayout& gly
 			if ((style.strikethruOffset() != -1) || (style.strikethruWidth() != -1))
 			{
 				if (style.strikethruOffset() != -1)
-					st = (style.strikethruOffset() / 1000.0) * (style.font().ascent(style.fontSize() / 10.0));
+					st = (style.strikethruOffset() / 1000.0) * (font->ascent(style.fontSize() / 10.0));
 				else
-					st = style.font().strikeoutPos(style.fontSize() / 10.0);
+					st = font->strikeoutPos(style.fontSize() / 10.0);
 				if (style.strikethruWidth() != -1)
 					lw = (style.strikethruWidth() / 1000.0) * (style.fontSize() / 10.0);
 				else
-					lw = qMax(style.font().strokeWidth(style.fontSize() / 10.0), 1.0);
+					lw = qMax(font->strokeWidth(style.fontSize() / 10.0), 1.0);
 			}
 			else
 			{
-				st = style.font().strikeoutPos(style.fontSize() / 10.0);
-				lw = qMax(style.font().strokeWidth(style.fontSize() / 10.0), 1.0);
+				st = font->strikeoutPos(style.fontSize() / 10.0);
+				lw = qMax(font->strokeWidth(style.fontSize() / 10.0), 1.0);
 			}
 			if (style.baselineOffset() != 0)
 				st += (style.fontSize() / 10.0) * glyphs.scaleV * (style.baselineOffset() / 1000.0);
diff --git a/Scribus/scribus/pageitem_textframe.cpp b/Scribus/scribus/pageitem_textframe.cpp
index ecf4c84..17eeff8 100644
--- a/Scribus/scribus/pageitem_textframe.cpp
+++ b/Scribus/scribus/pageitem_textframe.cpp
@@ -970,17 +970,18 @@ static void justifyLine(StoryText& itemText, LineSpec& line)
 	// measure natural widths for glyphs and spaces
 	for (int sof = line.firstItem; sof <= line.lastItem; ++sof)
 	{
-		if (!SpecialChars::isExpandingSpace(itemText.text(sof)))
+		ScText* hl = itemText.item(sof);
+		if (!SpecialChars::isExpandingSpace(hl->ch))
 		{
-			glyphNatural += itemText.item(sof)->glyph.wide();
+			glyphNatural += hl->glyph.wide();
 		}
-		else if ( (itemText.item(sof)->effects() & ScStyle_SuppressSpace) == 0)
+		else if ( (hl->effects() & ScStyle_SuppressSpace) == 0)
 		{
-			spaceNatural += itemText.item(sof)->glyph.wide();
-			if (imSpace < 0.0 || imSpace > itemText.item(sof)->glyph.wide())
-				imSpace = itemText.item(sof)->glyph.wide();
+			spaceNatural += hl->glyph.wide();
+			if (imSpace < 0.0 || imSpace > hl->glyph.wide())
+				imSpace = hl->glyph.wide();
 		}
-		if (sof != line.firstItem && implicitSpace(itemText.item(sof - 1), itemText.item(sof))) {
+		if (sof != line.firstItem && implicitSpace(itemText.item(sof - 1), hl)) {
 			spaceInsertion += 1;
 		}
 	}
@@ -1041,11 +1042,12 @@ static void justifyLine(StoryText& itemText, LineSpec& line)
 	// distribute whitespace on spaces and glyphs
 	for (int yof = startItem; yof <= line.lastItem; ++yof)
 	{
-		double wide = itemText.item(yof)->glyph.wide();
-		if (!SpecialChars::isExpandingSpace(itemText.text(yof)))
+		ScText* hl = itemText.item(yof);
+		double wide = hl->glyph.wide();
+		if (!SpecialChars::isExpandingSpace(hl->ch))
 		{
-			itemText.item(yof)->glyph.last()->xadvance += wide * glyphExtension;
-			GlyphLayout* glyph = &(itemText.item(yof)->glyph);
+			hl->glyph.last()->xadvance += wide * glyphExtension;
+			GlyphLayout* glyph = &(hl->glyph);
 			while (glyph)
 			{
 				glyph->xoffset *= glyphScale;
@@ -1053,11 +1055,11 @@ static void justifyLine(StoryText& itemText, LineSpec& line)
 				glyph = glyph->more;
 			}
 		}
-		else if ((itemText.item(yof)->effects() & ScStyle_SuppressSpace) == 0)
+		else if ((hl->effects() & ScStyle_SuppressSpace) == 0)
 		{
-			itemText.item(yof)->glyph.last()->xadvance += wide * spaceExtension;
+			hl->glyph.last()->xadvance += wide * spaceExtension;
 		}
-		if (yof != line.firstItem && implicitSpace(itemText.item(yof - 1), itemText.item(yof))) {
+		if (yof != line.firstItem && implicitSpace(itemText.item(yof - 1), hl)) {
 			itemText.item(yof - 1)->glyph.last()->xadvance += imSpace;
 		}
 	}
@@ -1127,10 +1129,11 @@ static double opticalRightMargin(const StoryText& itemText, const LineSpec& line
 		--b;
 	if (b >= line.firstItem) 
 	{
-		double chs = itemText.charStyle(b).fontSize() * (itemText.charStyle(b).scaleH() / 1000.0);
+		const CharStyle* chStyle = &itemText.charStyle(b);
+		double chs = chStyle->fontSize() * (chStyle->scaleH() / 1000.0);
 		QChar chr = (itemText.item(b)->effects() & ScStyle_SoftHyphenVisible) ? 
 			QChar('-') : itemText.text(b);
-		double rightCorr = itemText.charStyle(b).font().realCharWidth(chr, chs / 10.0);
+		double rightCorr = chStyle->font().realCharWidth(chr, chs / 10.0);
 		if (QString("-,.`ยด'~").indexOf(chr) >= 0
 			|| chr == QChar(0x2018)
 			|| chr == QChar(0x2019)
@@ -1153,8 +1156,8 @@ static double opticalRightMargin(const StoryText& itemText, const LineSpec& line
 				 )
 			rightCorr *= 0.5;
 		else {
-			rightCorr = itemText.charStyle(b).font().charWidth(chr, chs / 10.0);
-			rightCorr -= itemText.charStyle(b).font().charWidth(chr, chs / 10.0, QChar('.'));
+			rightCorr = chStyle->font().charWidth(chr, chs / 10.0);
+			rightCorr -= chStyle->font().charWidth(chr, chs / 10.0, QChar('.'));
 		}
 		return rightCorr;
 	}
@@ -1523,6 +1526,7 @@ void PageItem_TextFrame::layout()
 					itemText.setCharStyle(a, chstr.length(),charStyle);
 				}
 			}
+			const ScFace* font = &charStyle.font();
 
 			{  // local block for 'fl'
 				StyleFlag fl = hl->effects();
@@ -1601,11 +1605,11 @@ void PageItem_TextFrame::layout()
 				// FIXME : we should ensure that fonts are loaded before calls to layout()
 				// ScFace::realCharHeight()/Ascent() ensure font is loaded thanks to an indirect call to char2CMap()
 				// ScFace::ascent() can be called safely afterwards
-				double realCharHeight = charStyle.font().realCharHeight(chstr[0], 1);
-				double realCharAscent = charStyle.font().realCharAscent(chstr[0], 1);
-				double fontAscent     = charStyle.font().ascent(style.charStyle().fontSize() / 10.0);
+				double realCharHeight = font->realCharHeight(chstr[0], 1);
+				double realCharAscent = font->realCharAscent(chstr[0], 1);
+				double fontAscent     = font->ascent(style.charStyle().fontSize() / 10.0);
 				if (realCharHeight == 0.0)
-					realCharHeight = charStyle.font().height(style.charStyle().fontSize() / 10.0);
+					realCharHeight = font->height(style.charStyle().fontSize() / 10.0);
 				if (realCharAscent == 0.0)
 					realCharAscent = fontAscent;
 				chsd = (10 * ((DropCapDrop + fontAscent) / realCharHeight));
@@ -1650,8 +1654,8 @@ void PageItem_TextFrame::layout()
 				// apply kerning
 				if (a+1 < itemText.length())
 				{
-					uint glyph2 = charStyle.font().char2CMap(itemText.text(a+1));
-					double kern= charStyle.font().glyphKerning(hl->glyph.glyph, glyph2, chs / 10.0) * hl->glyph.scaleH;
+					uint glyph2 = font->char2CMap(itemText.text(a+1));
+					double kern= font->glyphKerning(hl->glyph.glyph, glyph2, chs / 10.0) * hl->glyph.scaleH;
 					wide += kern;
 					hl->glyph.xadvance += kern;
 					// change xadvance, xoffset according to JIS X4051
@@ -1739,7 +1743,7 @@ void PageItem_TextFrame::layout()
 				{
 					double itemHeight = hl->getItem(m_Doc)->height() + hl->getItem(m_Doc)->lineWidth();
 					if (itemHeight == 0)
-						itemHeight = charStyle.font().height(style.charStyle().fontSize() / 10.0);
+						itemHeight = font->height(style.charStyle().fontSize() / 10.0);
 					wide = hl->getItem(m_Doc)->width() + hl->getItem(m_Doc)->lineWidth();
 					asce = hl->getItem(m_Doc)->height() + hl->getItem(m_Doc)->lineWidth();
 					realAsce = calculateLineSpacing (style, this) * DropLines;
@@ -1749,12 +1753,12 @@ void PageItem_TextFrame::layout()
 				}
 				else
 				{
-					double realCharHeight = charStyle.font().realCharHeight(chstr[0], charStyle.fontSize() / 10.0);
+					double realCharHeight = font->realCharHeight(chstr[0], charStyle.fontSize() / 10.0);
 					if (realCharHeight == 0)
-						realCharHeight = charStyle.font().height(style.charStyle().fontSize() / 10.0);
-					wide = (charStyle.font().realCharWidth(chstr[0], chsd / 10.0) * scaleH) + (1 - scaleH); //dropcaps are to close to next glyphs if glyphs are hard scaled
-					realAsce = charStyle.font().realCharHeight(chstr[0], chsd / 10.0) * scaleV + offset;
-					asce = charStyle.font().ascent(hlcsize10);
+						realCharHeight = font->height(style.charStyle().fontSize() / 10.0);
+					wide = (font->realCharWidth(chstr[0], chsd / 10.0) * scaleH) + (1 - scaleH); //dropcaps are to close to next glyphs if glyphs are hard scaled
+					realAsce = font->realCharHeight(chstr[0], chsd / 10.0) * scaleV + offset;
+					asce = font->ascent(hlcsize10);
 					// qDebug() QString("dropcaps pre: chsd=%1 realCharHeight = %2 chstr=%3").arg(chsd).arg(asce).arg(chstr2[0]);
 					hl->glyph.scaleH /= hl->glyph.scaleV;
 					hl->glyph.scaleV = (realAsce / realCharHeight);
@@ -1777,8 +1781,8 @@ void PageItem_TextFrame::layout()
 					desc = realDesc = 0;
 				else
 				{
-					desc = -charStyle.font().descent(hlcsize10);
-					realDesc = charStyle.font().realCharDescent(chstr[0], hlcsize10) * scaleV - offset;
+					desc = -font->descent(hlcsize10);
+					realDesc = font->realCharDescent(chstr[0], hlcsize10) * scaleV - offset;
 					current.rememberShrinkStretch(hl->ch, wide, style);
 				}
 				if ((hl->ch == SpecialChars::OBJECT) && (hl->hasObject(m_Doc)))
@@ -1788,8 +1792,8 @@ void PageItem_TextFrame::layout()
 				}
 				else
 				{
-					asce = charStyle.font().ascent(hlcsize10);
-					realAsce = charStyle.font().realCharAscent(chstr[0], hlcsize10) * scaleV + offset;
+					asce = font->ascent(hlcsize10);
+					realAsce = font->realCharAscent(chstr[0], hlcsize10) * scaleV + offset;
 				}
 			}
 
@@ -2017,7 +2021,7 @@ void PageItem_TextFrame::layout()
 					if ((hl->ch == SpecialChars::OBJECT) && (hl->hasObject(m_Doc)))
 						diff = (hl->getItem(m_Doc)->height() + hl->getItem(m_Doc)->lineWidth()) * scaleV + offset - (current.yPos - lastLineY);
 					else
-						diff = charStyle.font().realCharAscent(QChar('l'), hlcsize10) * scaleV + offset - (current.yPos - lastLineY);
+						diff = font->realCharAscent(QChar('l'), hlcsize10) * scaleV + offset - (current.yPos - lastLineY);
 				}
 				else
 				{
@@ -2118,7 +2122,7 @@ void PageItem_TextFrame::layout()
 							if (tglyph)
 							{
 								tglyph->fillChar = tabs.fillChar;
-								tglyph->glyph    = charStyle.font().char2CMap(tabs.fillChar);
+								tglyph->glyph    = font->char2CMap(tabs.fillChar);
 								tglyph->yoffset  = hl->glyph.yoffset;
 								tglyph->scaleV   = tglyph->scaleH = chs / charStyle.fontSize();
 								tglyph->xadvance = 0;
@@ -2134,7 +2138,7 @@ void PageItem_TextFrame::layout()
 			
 			// remember y pos
 			if (DropCmode)
-				hl->glyph.yoffset -= charStyle.font().realCharHeight(chstr[0], chsd / 10.0) - charStyle.font().realCharAscent(chstr[0], chsd / 10.0);
+				hl->glyph.yoffset -= font->realCharHeight(chstr[0], chsd / 10.0) - font->realCharAscent(chstr[0], chsd / 10.0);
 
 			// remember x pos
 			double breakPos = current.xPos;
@@ -2190,7 +2194,7 @@ void PageItem_TextFrame::layout()
 			double hyphWidth = 0.0;
 			bool inOverflow = false;
 			if (hl->effects() & ScStyle_HyphenationPossible || hl->ch == SpecialChars::SHYPHEN)
-				hyphWidth = charStyle.font().charWidth('-', hlcsize10) * (charStyle.scaleH() / 1000.0);
+				hyphWidth = font->charWidth('-', hlcsize10) * (charStyle.scaleH() / 1000.0);
 			if ((current.isEndOfLine(style.rightMargin() + hyphWidth)) || current.isEndOfCol(realDesc) || SpecialChars::isBreak(hl->ch, Cols > 1) || (current.xPos - current.maxShrink + hyphWidth) >= current.mustLineEnd)
 			{
 				//end of row reached - right column, end of column, break char or line must end
@@ -2508,7 +2512,7 @@ void PageItem_TextFrame::layout()
 						// go back to last break position
 						style = itemText.paragraphStyle(a);
 						if (style.lineSpacingMode() == ParagraphStyle::AutomaticLineSpacing)
-							style.setLineSpacing(charStyle.font().height(hlcsize10) * autoLS);
+							style.setLineSpacing(font->height(hlcsize10) * autoLS);
 						else if (style.lineSpacingMode() == ParagraphStyle::BaselineGridLineSpacing)
 							style.setLineSpacing(m_Doc->guidesPrefs().valueBaselineGrid);
 						
@@ -2532,8 +2536,8 @@ void PageItem_TextFrame::layout()
 								current.hyphenCount++;
 							hl->setEffects(hl->effects() | ScStyle_SoftHyphenVisible);
 							hl->glyph.grow();
-							hl->glyph.more->glyph = charStyle.font().char2CMap(QChar('-'));
-							hl->glyph.more->xadvance = charStyle.font().charWidth('-', itemText.charStyle(a).fontSize() / 10.0) * scaleH; //FIX ME - hyphen is not rendered with proper width - check yhis with large glyphs horizontal scaling eg. 20%
+							hl->glyph.more->glyph = font->char2CMap(QChar('-'));
+							hl->glyph.more->xadvance = font->charWidth('-', itemText.charStyle(a).fontSize() / 10.0) * scaleH; //FIX ME - hyphen is not rendered with proper width - check yhis with large glyphs horizontal scaling eg. 20%
 							hyphWidth = hl->glyph.more->xadvance;
 						}
 						else
@@ -3021,8 +3025,10 @@ void PageItem_TextFrame::DrawObj_Item(ScPainter *p, QRectF cullingArea)
 									xcoZli -= hls->glyph.xoffset;
 								}
 							}
-							desc = - charStyleS.font().descent(charStyleS.fontSize() / 10.0);
-							asce = charStyleS.font().ascent(charStyleS.fontSize() / 10.0);
+							const ScFace* font = &charStyleS.font();
+							double fontSize = charStyleS.fontSize() / 10.0;
+							desc = - font->descent(fontSize);
+							asce = font->ascent(fontSize);
 							wide = hls->glyph.wide();
 							QRectF scr;
 							if ((hls->ch == SpecialChars::OBJECT)  && (hls->hasObject(m_Doc)))
@@ -3088,8 +3094,10 @@ void PageItem_TextFrame::DrawObj_Item(ScPainter *p, QRectF cullingArea)
 
 				if (!m_Doc->RePos)
 				{
-					desc = - charStyle.font().descent(charStyle.fontSize() / 10.0);
-					asce = charStyle.font().ascent(charStyle.fontSize() / 10.0);
+					const ScFace* font = &charStyle.font();
+					double fontSize = charStyle.fontSize() / 10.0;
+					desc = - font->descent(fontSize);
+					asce = font->ascent(fontSize);
 					if (((selected && Select) || ((NextBox != 0 || BackBox != 0) && selected)) && (m_Doc->appMode == modeEdit || m_Doc->appMode == modeEditTable))
 					{
 						// set text color to highlight if its selected
diff --git a/Scribus/scribus/pdflib_core.cpp b/Scribus/scribus/pdflib_core.cpp
index a047525..ca6cd25 100644
--- a/Scribus/scribus/pdflib_core.cpp
+++ b/Scribus/scribus/pdflib_core.cpp
@@ -944,7 +944,8 @@ bool PDFLibCore::PDF_Begin_Doc(const QString& fn, SCFonts &AllFonts, QMap<QStrin
 	for (uint ax = 0; ax < ar; ++ax)
 		ind2PDFabr[ax] = tmpf[ax];
 	QList<PageItem*> allItems;
-	for (QHash<int, PageItem*>::iterator itf = doc.FrameItems.begin(); itf != doc.FrameItems.end(); ++itf)
+	QHash<int, PageItem*>::iterator FIend = doc.FrameItems.end();
+	for (QHash<int, PageItem*>::iterator itf = doc.FrameItems.begin(); itf != FIend; ++itf)
 	{
 		pgit = itf.value();
 		if (pgit->isGroup())
@@ -1190,8 +1191,9 @@ bool PDFLibCore::PDF_Begin_Doc(const QString& fn, SCFonts &AllFonts, QMap<QStrin
 		a++;
 	}
 	QMap<QString,QMap<uint, FPointArray> >::Iterator it;
+	QMap<QString,QMap<uint, FPointArray> >::Iterator RUend = ReallyUsed.end();
 	a = 0;
-	for (it = ReallyUsed.begin(); it != ReallyUsed.end(); ++it)
+	for (it = ReallyUsed.begin(); it != RUend; ++it)
 	{
 		ScFace& face(AllFonts[it.key()]);
 		ScFace::FontFormat fformat = face.format();
@@ -1210,12 +1212,13 @@ bool PDFLibCore::PDF_Begin_Doc(const QString& fn, SCFonts &AllFonts, QMap<QStrin
 				QStringList charProcs;
 				QString encoding = "<< /Type /Encoding\n/Differences [ 0\n";
 				QString fon("");
-				QMap<uint, uint> glyphMapping;
+				QHash<uint, uint> glyphMapping;
 				QMap<uint,std::pair<QChar,QString> > gl;
 				face.glyphNames(gl);
 				QMap<uint,FPointArray>& RealGlyphs(it.value());
 				QMap<uint,FPointArray>::Iterator ig;
-				for (ig = RealGlyphs.begin(); ig != RealGlyphs.end(); ++ig)
+				QMap<uint,FPointArray>::Iterator RGend = RealGlyphs.end();
+				for (ig = RealGlyphs.begin(); ig != RGend; ++ig)
 				{
 					FPoint np, np1, np2;
 					bool nPath = true;
@@ -1330,7 +1333,8 @@ bool PDFLibCore::PDF_Begin_Doc(const QString& fn, SCFonts &AllFonts, QMap<QStrin
 				QString fon("");
 				QMap<uint,FPointArray>& RealGlyphs(it.value());
 				QMap<uint,FPointArray>::Iterator ig;
-				for (ig = RealGlyphs.begin(); ig != RealGlyphs.end(); ++ig)
+				QMap<uint,FPointArray>::Iterator RGend = RealGlyphs.end();
+				for (ig = RealGlyphs.begin(); ig != RGend; ++ig)
 				{
 					FPoint np, np1, np2;
 					bool nPath = true;
@@ -1535,7 +1539,8 @@ bool PDFLibCore::PDF_Begin_Doc(const QString& fn, SCFonts &AllFonts, QMap<QStrin
 			face.glyphNames(gl);
 			int nglyphs = 0;
 			QMap<uint,std::pair<QChar,QString> >::Iterator gli;
-			for (gli = gl.begin(); gli != gl.end(); ++gli)
+			QMap<uint,std::pair<QChar,QString> >::Iterator GLend = gl.end();
+			for (gli = gl.begin(); gli != GLend; ++gli)
 			{
 				if (gli.key() > static_cast<uint>(nglyphs))
 					nglyphs = gli.key();
diff --git a/Scribus/scribus/pdflib_core.h b/Scribus/scribus/pdflib_core.h
index cab033a..6627ce6 100644
--- a/Scribus/scribus/pdflib_core.h
+++ b/Scribus/scribus/pdflib_core.h
@@ -303,7 +303,7 @@ private:
 	bool usingGUI;
 	double bleedDisplacementX;
 	double bleedDisplacementY;
-	QMap<QString, QMap<uint, uint> > Type3Fonts;
+	QMap<QString, QHash<uint, uint> > Type3Fonts;
 	QString xmpPacket;
 	QStack<QPointF> groupStackPos;
 
diff --git a/Scribus/scribus/pslib.cpp b/Scribus/scribus/pslib.cpp
index 3a5bb8b..15527a2 100644
--- a/Scribus/scribus/pslib.cpp
+++ b/Scribus/scribus/pslib.cpp
@@ -162,7 +162,8 @@ PSLib::PSLib(PrintOptions &options, bool psart, SCFonts &AllFonts, QMap<QString,
 			FontDesc += "/" + encodedName + " " + IToStr(RealGlyphs.count()+1) + " dict def\n";
 			FontDesc += encodedName + " begin\n";
 			QMap<uint,FPointArray>::Iterator ig;
-			for (ig = RealGlyphs.begin(); ig != RealGlyphs.end(); ++ig)
+			QMap<uint,FPointArray>::Iterator end = RealGlyphs.end();
+			for (ig = RealGlyphs.begin(); ig != end; ++ig)
 			{
 				FontDesc += "/G"+IToStr(ig.key())+" { newpath\n";
 				FPoint np, np1, np2;
diff --git a/Scribus/scribus/styles/charstyle.cpp b/Scribus/scribus/styles/charstyle.cpp
index 0c17d3c..78322ae 100644
--- a/Scribus/scribus/styles/charstyle.cpp
+++ b/Scribus/scribus/styles/charstyle.cpp
@@ -280,7 +280,8 @@ void CharStyle::updateFeatures()
 void CharStyle::runFeatures(const QStringList& featureList, const CharStyle* parent)
 {
 	QStringList::ConstIterator it;
-	for (it = featureList.begin(); it != featureList.end(); ++it)
+	QStringList::ConstIterator end = featureList.end();
+	for (it = featureList.begin(); it != end; ++it)
 	{
 		QString feature = it->trimmed();
 		if (feature == INHERIT)
diff --git a/Scribus/scribus/text/storytext.cpp b/Scribus/scribus/text/storytext.cpp
index c760855..a50c033 100644
--- a/Scribus/scribus/text/storytext.cpp
+++ b/Scribus/scribus/text/storytext.cpp
@@ -827,7 +827,7 @@ const ParagraphStyle & StoryText::paragraphStyle(int pos) const
 	else {
 //		qDebug() << QString("using parstyle at %1").arg(pos);
 	}
-	assert (that->d->at(pos)->parstyle);
+	Q_ASSERT (that->d->at(pos)->parstyle);
 	return *that->d->at(pos)->parstyle;
 }
 
text_layout_speedup.diff (30,333 bytes)   

cezaryece

2012-08-10 13:49

developer   ~0028800

Next diff contains further optimizations, specially in PageItem_TextFrame::layout().

jghali

2012-08-14 16:07

administrator   ~0028817

I committed most of the patch. The most notable omission is the one of scface_ttf.h. With compiler optimizations turned on, i could not show that replacing QMap by QHash lead to real improvements. In most of cases i could see a 5-10% slowdown

What i committed allows to achieve a 10-15% improvement in layout() execution times however, which is already nice.

Issue History

Date Modified Username Field Change
2012-08-10 09:15 cezaryece New Issue
2012-08-10 09:15 cezaryece File Added: scface_ttf_speedup.diff
2012-08-10 09:17 cezaryece Description Updated
2012-08-10 09:26 avox Note Added: 0028798
2012-08-10 13:49 cezaryece File Added: text_layout_speedup.diff
2012-08-10 13:49 cezaryece Note Added: 0028800
2012-08-14 16:07 jghali Note Added: 0028817
2012-08-14 16:07 jghali Status new => resolved
2012-08-14 16:07 jghali Fixed in Version => 1.5.0svn
2012-08-14 16:07 jghali Resolution open => fixed
2012-08-14 16:07 jghali Assigned To => jghali
2012-11-13 20:39 cbradney Status resolved => closed