From 0b996f435fb0da42e4f563986ef8214d542398f4 Mon Sep 17 00:00:00 2001 From: Jiang Jiang Date: Fri, 8 Jul 2011 17:40:43 +0200 Subject: Fix crash when app font is added Loading app fonts will clear the application font cache, but QFontPrivate::engineWithScript will try to load the font again, in Mac the font engine used here must be the one used for shaping, because subsequent sub font engines may be added to it during the shaping process (QCoreTextFontEngineMulti::stringToCMap). That is why we need to fetch the font engine directly from QTextEngine's fontEngine cache instead of QFontCache. Task-number: QTBUG-20250 Reviewed-by: Eskil (cherry picked from commit 1f90ae36cff8acf581d1624bf011fe3a55c623c0) --- src/gui/text/qtextengine.cpp | 35 +++++++++++++++++++++-------------- src/gui/text/qtextengine_p.h | 4 +++- src/gui/text/qtextlayout.cpp | 8 ++++---- 3 files changed, 28 insertions(+), 19 deletions(-) diff --git a/src/gui/text/qtextengine.cpp b/src/gui/text/qtextengine.cpp index 9271f34..8374c62 100644 --- a/src/gui/text/qtextengine.cpp +++ b/src/gui/text/qtextengine.cpp @@ -2778,6 +2778,22 @@ QTextItemInt::QTextItemInt(const QScriptItem &si, QFont *font, const QTextCharFo : justified(false), underlineStyle(QTextCharFormat::NoUnderline), charFormat(format), num_chars(0), chars(0), logClusters(0), f(0), fontEngine(0) { + f = font; + fontEngine = f->d->engineForScript(si.analysis.script); + Q_ASSERT(fontEngine); + + initWithScriptItem(si); +} + +QTextItemInt::QTextItemInt(const QGlyphLayout &g, QFont *font, const QChar *chars_, int numChars, QFontEngine *fe, const QTextCharFormat &format) + : flags(0), justified(false), underlineStyle(QTextCharFormat::NoUnderline), charFormat(format), + num_chars(numChars), chars(chars_), logClusters(0), f(font), glyphs(g), fontEngine(fe) +{ +} + +// Fix up flags and underlineStyle with given info +void QTextItemInt::initWithScriptItem(const QScriptItem &si) +{ // explicitly initialize flags so that initFontAttributes can be called // multiple times on the same TextItem flags = 0; @@ -2785,13 +2801,10 @@ QTextItemInt::QTextItemInt(const QScriptItem &si, QFont *font, const QTextCharFo flags |= QTextItem::RightToLeft; ascent = si.ascent; descent = si.descent; - f = font; - fontEngine = f->d->engineForScript(si.analysis.script); - Q_ASSERT(fontEngine); - if (format.hasProperty(QTextFormat::TextUnderlineStyle)) { - underlineStyle = format.underlineStyle(); - } else if (format.boolProperty(QTextFormat::FontUnderline) + if (charFormat.hasProperty(QTextFormat::TextUnderlineStyle)) { + underlineStyle = charFormat.underlineStyle(); + } else if (charFormat.boolProperty(QTextFormat::FontUnderline) || f->d->underline) { underlineStyle = QTextCharFormat::SingleUnderline; } @@ -2800,18 +2813,12 @@ QTextItemInt::QTextItemInt(const QScriptItem &si, QFont *font, const QTextCharFo if (underlineStyle == QTextCharFormat::SingleUnderline) flags |= QTextItem::Underline; - if (f->d->overline || format.fontOverline()) + if (f->d->overline || charFormat.fontOverline()) flags |= QTextItem::Overline; - if (f->d->strikeOut || format.fontStrikeOut()) + if (f->d->strikeOut || charFormat.fontStrikeOut()) flags |= QTextItem::StrikeOut; } -QTextItemInt::QTextItemInt(const QGlyphLayout &g, QFont *font, const QChar *chars_, int numChars, QFontEngine *fe) - : flags(0), justified(false), underlineStyle(QTextCharFormat::NoUnderline), - num_chars(numChars), chars(chars_), logClusters(0), f(font), glyphs(g), fontEngine(fe) -{ -} - QTextItemInt QTextItemInt::midItem(QFontEngine *fontEngine, int firstGlyphIndex, int numGlyphs) const { QTextItemInt ti = *this; diff --git a/src/gui/text/qtextengine_p.h b/src/gui/text/qtextengine_p.h index 09610ff..86551a4 100644 --- a/src/gui/text/qtextengine_p.h +++ b/src/gui/text/qtextengine_p.h @@ -311,11 +311,13 @@ public: logClusters(0), f(0), fontEngine(0) {} QTextItemInt(const QScriptItem &si, QFont *font, const QTextCharFormat &format = QTextCharFormat()); - QTextItemInt(const QGlyphLayout &g, QFont *font, const QChar *chars, int numChars, QFontEngine *fe); + QTextItemInt(const QGlyphLayout &g, QFont *font, const QChar *chars, int numChars, QFontEngine *fe, + const QTextCharFormat &format = QTextCharFormat()); /// copy the structure items, adjusting the glyphs arrays to the right subarrays. /// the width of the returned QTextItemInt is not adjusted, for speed reasons QTextItemInt midItem(QFontEngine *fontEngine, int firstGlyphIndex, int numGlyphs) const; + void initWithScriptItem(const QScriptItem &si); QFixed descent; QFixed ascent; diff --git a/src/gui/text/qtextlayout.cpp b/src/gui/text/qtextlayout.cpp index 0fbf838..86c6f48 100644 --- a/src/gui/text/qtextlayout.cpp +++ b/src/gui/text/qtextlayout.cpp @@ -2319,13 +2319,13 @@ void QTextLine::draw(QPainter *p, const QPointF &pos, const QTextLayout::FormatR unsigned short *logClusters = eng->logClusters(&si); QGlyphLayout glyphs = eng->shapedGlyphs(&si); - QTextItemInt gf(si, &f, format); - gf.glyphs = glyphs.mid(iterator.glyphsStart, iterator.glyphsEnd - iterator.glyphsStart); - gf.chars = eng->layoutData->string.unicode() + iterator.itemStart; + QTextItemInt gf(glyphs.mid(iterator.glyphsStart, iterator.glyphsEnd - iterator.glyphsStart), + &f, eng->layoutData->string.unicode() + iterator.itemStart, + iterator.itemEnd - iterator.itemStart, eng->fontEngine(si), format); gf.logClusters = logClusters + iterator.itemStart - si.position; - gf.num_chars = iterator.itemEnd - iterator.itemStart; gf.width = iterator.itemWidth; gf.justified = line.justified; + gf.initWithScriptItem(si); Q_ASSERT(gf.fontEngine); -- cgit v0.12 From c4b5e186c7718f0a03d6ed529f546e7debfbcf86 Mon Sep 17 00:00:00 2001 From: Michael Brasser Date: Fri, 9 Jul 2010 11:33:56 +1000 Subject: Optimize text layout. When laying out text, a significant amount of time was being spent querying for the font engine. This patch: * changes the layout to only query for the engine when a new script item is encountered. * adds an internal cache of the previous result to QTextEngine::fontEngine(). This catches the important case of multiline text with few font engine changes. With these changes layout costs are now approximately 60% of what they were previously, as measured by the text layout benchmarks. Reviewed-by: Eskil Abrahamsen Blomfeldt (cherry picked from commit 0c56bef89b6e3fe4c9fb32eb8b51a6ea316a89fa) --- src/gui/text/qtextengine.cpp | 65 +++++++++++++++++++++++++++++++------------- src/gui/text/qtextengine_p.h | 17 ++++++++++++ src/gui/text/qtextlayout.cpp | 14 ++++++---- 3 files changed, 71 insertions(+), 25 deletions(-) diff --git a/src/gui/text/qtextengine.cpp b/src/gui/text/qtextengine.cpp index 8374c62..03717dc 100644 --- a/src/gui/text/qtextengine.cpp +++ b/src/gui/text/qtextengine.cpp @@ -1392,6 +1392,7 @@ void QTextEngine::invalidate() maxWidth = 0; if (specialData) specialData->resolvedFormatIndices.clear(); + feCache.reset(); } void QTextEngine::clearLineData() @@ -1783,6 +1784,13 @@ QFont QTextEngine::font(const QScriptItem &si) const return font; } +QTextEngine::FontEngineCache::FontEngineCache() +{ + reset(); +} + +//we cache the previous results of this function, as calling it numerous times with the same effective +//input is common (and hard to cache at a higher level) QFontEngine *QTextEngine::fontEngine(const QScriptItem &si, QFixed *ascent, QFixed *descent, QFixed *leading) const { QFontEngine *engine = 0; @@ -1791,28 +1799,47 @@ QFontEngine *QTextEngine::fontEngine(const QScriptItem &si, QFixed *ascent, QFix QFont font = fnt; if (hasFormats()) { - QTextCharFormat f = format(&si); - font = f.font(); - - if (block.docHandle() && block.docHandle()->layout()) { - // Make sure we get the right dpi on printers - QPaintDevice *pdev = block.docHandle()->layout()->paintDevice(); - if (pdev) - font = QFont(font, pdev); + if (feCache.prevFontEngine && feCache.prevPosition == si.position && feCache.prevLength == length(&si) && feCache.prevScript == script) { + engine = feCache.prevFontEngine; + scaledEngine = feCache.prevScaledFontEngine; } else { - font = font.resolve(fnt); - } - engine = font.d->engineForScript(script); - QTextCharFormat::VerticalAlignment valign = f.verticalAlignment(); - if (valign == QTextCharFormat::AlignSuperScript || valign == QTextCharFormat::AlignSubScript) { - if (font.pointSize() != -1) - font.setPointSize((font.pointSize() * 2) / 3); - else - font.setPixelSize((font.pixelSize() * 2) / 3); - scaledEngine = font.d->engineForScript(script); + QTextCharFormat f = format(&si); + font = f.font(); + + if (block.docHandle() && block.docHandle()->layout()) { + // Make sure we get the right dpi on printers + QPaintDevice *pdev = block.docHandle()->layout()->paintDevice(); + if (pdev) + font = QFont(font, pdev); + } else { + font = font.resolve(fnt); + } + engine = font.d->engineForScript(script); + QTextCharFormat::VerticalAlignment valign = f.verticalAlignment(); + if (valign == QTextCharFormat::AlignSuperScript || valign == QTextCharFormat::AlignSubScript) { + if (font.pointSize() != -1) + font.setPointSize((font.pointSize() * 2) / 3); + else + font.setPixelSize((font.pixelSize() * 2) / 3); + scaledEngine = font.d->engineForScript(script); + } + feCache.prevFontEngine = engine; + feCache.prevScaledFontEngine = scaledEngine; + feCache.prevScript = script; + feCache.prevPosition = si.position; + feCache.prevLength = length(&si); } } else { - engine = font.d->engineForScript(script); + if (feCache.prevFontEngine && feCache.prevScript == script && feCache.prevPosition == -1) + engine = feCache.prevFontEngine; + else { + engine = font.d->engineForScript(script); + feCache.prevFontEngine = engine; + feCache.prevScript = script; + feCache.prevPosition = -1; + feCache.prevLength = -1; + feCache.prevScaledFontEngine = 0; + } } if (si.analysis.flags == QScriptAnalysis::SmallCaps) { diff --git a/src/gui/text/qtextengine_p.h b/src/gui/text/qtextengine_p.h index 86551a4..c920c7b 100644 --- a/src/gui/text/qtextengine_p.h +++ b/src/gui/text/qtextengine_p.h @@ -558,6 +558,23 @@ public: mutable QScriptLineArray lines; + struct FontEngineCache { + FontEngineCache(); + mutable QFontEngine *prevFontEngine; + mutable QFontEngine *prevScaledFontEngine; + mutable int prevScript; + mutable int prevPosition; + mutable int prevLength; + inline void reset() { + prevFontEngine = 0; + prevScaledFontEngine = 0; + prevScript = -1; + prevPosition = -1; + prevLength = -1; + } + }; + mutable FontEngineCache feCache; + QString text; QFont fnt; QTextBlock block; diff --git a/src/gui/text/qtextlayout.cpp b/src/gui/text/qtextlayout.cpp index 86c6f48..183bcea 100644 --- a/src/gui/text/qtextlayout.cpp +++ b/src/gui/text/qtextlayout.cpp @@ -386,6 +386,7 @@ QTextLayout::~QTextLayout() void QTextLayout::setFont(const QFont &font) { d->fnt = font; + d->feCache.reset(); } /*! @@ -518,6 +519,7 @@ void QTextLayout::setAdditionalFormats(const QList &formatList) } if (d->block.docHandle()) d->block.docHandle()->documentChange(d->block.position(), d->block.length()); + d->feCache.reset(); } /*! @@ -1867,14 +1869,14 @@ void QTextLine::layout_helper(int maxGlyphs) lbh.currentPosition = qMax(line.from, current.position); end = current.position + eng->length(item); lbh.glyphs = eng->shapedGlyphs(¤t); + QFontEngine *fontEngine = eng->fontEngine(current); + if (lbh.fontEngine != fontEngine) { + lbh.fontEngine = fontEngine; + lbh.minimumRightBearing = qMin(QFixed(), + QFixed::fromReal(fontEngine->minRightBearing())); + } } const QScriptItem ¤t = eng->layoutData->items[item]; - QFontEngine *fontEngine = eng->fontEngine(current); - if (lbh.fontEngine != fontEngine) { - lbh.fontEngine = fontEngine; - lbh.minimumRightBearing = qMin(QFixed(), - QFixed::fromReal(fontEngine->minRightBearing())); - } lbh.tmpData.leading = qMax(lbh.tmpData.leading + lbh.tmpData.ascent, current.leading + current.ascent) - qMax(lbh.tmpData.ascent, -- cgit v0.12 From 5f2b6dd2a50275bc05ae5d7e9dd8902d6d49d9df Mon Sep 17 00:00:00 2001 From: Jiang Jiang Date: Tue, 22 Feb 2011 15:49:34 +0100 Subject: Keep reference count for cached font engines in QTextEngine So that if these font engines are deallocated elsewhere (by QFontCache for instance), we can still access them in QTextEngine. Task-number: QTBUG-17603 Reviewed-by: Eskil (cherry picked from commit 6e23fb69e441871829765ff512e90fed17b6798d) --- src/gui/text/qtextengine.cpp | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/gui/text/qtextengine.cpp b/src/gui/text/qtextengine.cpp index 03717dc..6322245 100644 --- a/src/gui/text/qtextengine.cpp +++ b/src/gui/text/qtextengine.cpp @@ -1385,6 +1385,15 @@ void QTextEngine::shape(int item) const } } +static inline void releaseCachedFontEngine(QFontEngine *fontEngine) +{ + if (fontEngine) { + fontEngine->ref.deref(); + if (fontEngine->cache_count == 0 && fontEngine->ref == 0) + delete fontEngine; + } +} + void QTextEngine::invalidate() { freeMemory(); @@ -1392,6 +1401,9 @@ void QTextEngine::invalidate() maxWidth = 0; if (specialData) specialData->resolvedFormatIndices.clear(); + + releaseCachedFontEngine(feCache.prevFontEngine); + releaseCachedFontEngine(feCache.prevScaledFontEngine); feCache.reset(); } @@ -1824,7 +1836,9 @@ QFontEngine *QTextEngine::fontEngine(const QScriptItem &si, QFixed *ascent, QFix scaledEngine = font.d->engineForScript(script); } feCache.prevFontEngine = engine; + engine->ref.ref(); feCache.prevScaledFontEngine = scaledEngine; + scaledEngine->ref.ref(); feCache.prevScript = script; feCache.prevPosition = si.position; feCache.prevLength = length(&si); @@ -1835,6 +1849,7 @@ QFontEngine *QTextEngine::fontEngine(const QScriptItem &si, QFixed *ascent, QFix else { engine = font.d->engineForScript(script); feCache.prevFontEngine = engine; + engine->ref.ref(); feCache.prevScript = script; feCache.prevPosition = -1; feCache.prevLength = -1; -- cgit v0.12 From ade1378a4fc479d7ff3de3eb636c222c195a9082 Mon Sep 17 00:00:00 2001 From: Jiang Jiang Date: Wed, 23 Feb 2011 12:07:10 +0100 Subject: Check engine existence before increasing reference count Reviewed-by: TrustMe (cherry picked from commit 244620438700464a862ceab7c881974a5b1d1fea) --- src/gui/text/qtextengine.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/gui/text/qtextengine.cpp b/src/gui/text/qtextengine.cpp index 6322245..648f5c0 100644 --- a/src/gui/text/qtextengine.cpp +++ b/src/gui/text/qtextengine.cpp @@ -1836,9 +1836,11 @@ QFontEngine *QTextEngine::fontEngine(const QScriptItem &si, QFixed *ascent, QFix scaledEngine = font.d->engineForScript(script); } feCache.prevFontEngine = engine; - engine->ref.ref(); + if (engine) + engine->ref.ref(); feCache.prevScaledFontEngine = scaledEngine; - scaledEngine->ref.ref(); + if (scaledEngine) + scaledEngine->ref.ref(); feCache.prevScript = script; feCache.prevPosition = si.position; feCache.prevLength = length(&si); @@ -1849,7 +1851,8 @@ QFontEngine *QTextEngine::fontEngine(const QScriptItem &si, QFixed *ascent, QFix else { engine = font.d->engineForScript(script); feCache.prevFontEngine = engine; - engine->ref.ref(); + if (engine) + engine->ref.ref(); feCache.prevScript = script; feCache.prevPosition = -1; feCache.prevLength = -1; -- cgit v0.12