From afb326f07109da0035112e6f56e683e37b8a5d72 Mon Sep 17 00:00:00 2001 From: Lars Knoll Date: Fri, 17 Nov 2017 10:59:47 +0100 Subject: Refactor lockedAlphaMapForGlyph Simply return a Glyph pointer and not a QImage to avoid allocating and deleting lots of d pointers for QImage when drawing text. Saves one new/delete pair per glyph drawn and speeds up text drawing by 10% for relatively large glyphs (probably more for smaller ones). The qtext::paintLayoutToPixmap() benchmark shows a 16% improvement in performance with this change. Renamed the method to glyphData(). Change-Id: I7a353de521e4f4321c770fb1ac6043d33f6f332c Reviewed-by: Eskil Abrahamsen Blomfeldt --- src/gui/painting/qpaintengine_raster.cpp | 35 ++++++++++++------ src/gui/text/qfontengine.cpp | 25 ++----------- src/gui/text/qfontengine_p.h | 23 ++++++++---- .../fontdatabases/freetype/qfontengine_ft.cpp | 41 ++++------------------ .../fontdatabases/freetype/qfontengine_ft_p.h | 20 ++--------- 5 files changed, 53 insertions(+), 91 deletions(-) (limited to 'src') diff --git a/src/gui/painting/qpaintengine_raster.cpp b/src/gui/painting/qpaintengine_raster.cpp index 096e4a5c5b..885c46e121 100644 --- a/src/gui/painting/qpaintengine_raster.cpp +++ b/src/gui/painting/qpaintengine_raster.cpp @@ -2908,19 +2908,34 @@ bool QRasterPaintEngine::drawCachedGlyphs(int numGlyphs, const glyph_t *glyphs, for (int i = 0; i < numGlyphs; i++) { QFixed spp = fontEngine->subPixelPositionForX(positions[i].x); - QPoint offset; - const QImage *alphaMap = fontEngine->lockedAlphaMapForGlyph(glyphs[i], spp, neededFormat, s->matrix, - &offset); - if (alphaMap == 0 || alphaMap->isNull()) + const QFontEngine::Glyph *alphaMap = fontEngine->glyphData(glyphs[i], spp, neededFormat, s->matrix); + if (!alphaMap) continue; - alphaPenBlt(alphaMap->constBits(), alphaMap->bytesPerLine(), alphaMap->depth(), - qFloor(positions[i].x) + offset.x(), - qRound(positions[i].y) + offset.y(), - alphaMap->width(), alphaMap->height(), + int depth; + int bytesPerLine; + switch (alphaMap->format) { + case QFontEngine::Format_Mono: + depth = 1; + bytesPerLine = ((alphaMap->width + 31) & ~31) >> 3; + break; + case QFontEngine::Format_A8: + depth = 8; + bytesPerLine = (alphaMap->width + 3) & ~3; + break; + case QFontEngine::Format_A32: + depth = 32; + bytesPerLine = alphaMap->width * 4; + break; + default: + Q_UNREACHABLE(); + }; + + alphaPenBlt(alphaMap->data, bytesPerLine, depth, + qFloor(positions[i].x) + alphaMap->x, + qRound(positions[i].y) - alphaMap->y, + alphaMap->width, alphaMap->height, fontEngine->expectsGammaCorrectedBlending()); - - fontEngine->unlockAlphaMapForGlyph(); } } else { diff --git a/src/gui/text/qfontengine.cpp b/src/gui/text/qfontengine.cpp index 5506d88f02..1895ac8283 100644 --- a/src/gui/text/qfontengine.cpp +++ b/src/gui/text/qfontengine.cpp @@ -923,29 +923,10 @@ QFixed QFontEngine::subPixelPositionForX(QFixed x) const return subPixelPosition; } -QImage *QFontEngine::lockedAlphaMapForGlyph(glyph_t glyph, QFixed subPixelPosition, - QFontEngine::GlyphFormat neededFormat, - const QTransform &t, QPoint *offset) +QFontEngine::Glyph *QFontEngine::glyphData(glyph_t, QFixed, + QFontEngine::GlyphFormat, const QTransform &) { - Q_ASSERT(currentlyLockedAlphaMap.isNull()); - if (neededFormat == Format_None) - neededFormat = Format_A32; - - if (neededFormat != Format_A32) - currentlyLockedAlphaMap = alphaMapForGlyph(glyph, subPixelPosition, t); - else - currentlyLockedAlphaMap = alphaRGBMapForGlyph(glyph, subPixelPosition, t); - - if (offset != 0) - *offset = QPoint(0, 0); - - return ¤tlyLockedAlphaMap; -} - -void QFontEngine::unlockAlphaMapForGlyph() -{ - Q_ASSERT(!currentlyLockedAlphaMap.isNull()); - currentlyLockedAlphaMap = QImage(); + return nullptr; } QImage QFontEngine::alphaMapForGlyph(glyph_t glyph) diff --git a/src/gui/text/qfontengine_p.h b/src/gui/text/qfontengine_p.h index 682395ece6..48dcdbeff7 100644 --- a/src/gui/text/qfontengine_p.h +++ b/src/gui/text/qfontengine_p.h @@ -124,6 +124,22 @@ public: }; Q_DECLARE_FLAGS(ShaperFlags, ShaperFlag) + /* Used with the Freetype font engine. We don't cache glyphs that are too large anyway, so we can make this struct rather small */ + struct Glyph { + Glyph() = default; + ~Glyph() { delete [] data; } + short linearAdvance = 0; + unsigned char width = 0; + unsigned char height = 0; + short x = 0; + short y = 0; + short advance = 0; + signed char format = 0; + uchar *data = nullptr; + private: + Q_DISABLE_COPY(Glyph); + }; + virtual ~QFontEngine(); inline Type type() const { return m_type; } @@ -191,11 +207,7 @@ public: virtual QImage alphaMapForGlyph(glyph_t, QFixed subPixelPosition, const QTransform &t); virtual QImage alphaRGBMapForGlyph(glyph_t, QFixed subPixelPosition, const QTransform &t); virtual QImage bitmapForGlyph(glyph_t, QFixed subPixelPosition, const QTransform &t, const QColor &color = QColor()); - virtual QImage *lockedAlphaMapForGlyph(glyph_t glyph, QFixed subPixelPosition, - GlyphFormat neededFormat, - const QTransform &t = QTransform(), - QPoint *offset = 0); - virtual void unlockAlphaMapForGlyph(); + virtual Glyph *glyphData(glyph_t glyph, QFixed subPixelPosition, GlyphFormat neededFormat, const QTransform &t); virtual bool hasInternalCaching() const { return false; } virtual glyph_metrics_t alphaMapBoundingBox(glyph_t glyph, QFixed /*subPixelPosition*/, const QTransform &matrix, GlyphFormat /*format*/) @@ -346,7 +358,6 @@ public: void loadKerningPairs(QFixed scalingFactor); GlyphFormat glyphFormat; - QImage currentlyLockedAlphaMap; int m_subPixelPositionCount; // Number of positions within a single pixel for this cache inline QVariant userData() const { return m_userData; } diff --git a/src/platformsupport/fontdatabases/freetype/qfontengine_ft.cpp b/src/platformsupport/fontdatabases/freetype/qfontengine_ft.cpp index ef80d68bfe..e132442e37 100644 --- a/src/platformsupport/fontdatabases/freetype/qfontengine_ft.cpp +++ b/src/platformsupport/fontdatabases/freetype/qfontengine_ft.cpp @@ -106,7 +106,7 @@ static bool ft_getSfntTable(void *user_data, uint tag, uchar *buffer, uint *leng return result; } -static QFontEngineFT::Glyph emptyGlyph = {0, 0, 0, 0, 0, 0, 0, 0}; +static QFontEngineFT::Glyph emptyGlyph; static const QFontEngine::HintStyle ftInitialDefaultHintStyle = #ifdef Q_OS_WIN @@ -556,11 +556,6 @@ void QFreetypeFace::addBitmapToPath(FT_GlyphSlot slot, const QFixedPoint &point, slot->bitmap.buffer, slot->bitmap.pitch, slot->bitmap.width, slot->bitmap.rows, path); } -QFontEngineFT::Glyph::~Glyph() -{ - delete [] data; -} - struct LcdFilterDummy { static inline void filterPixel(uchar &, uchar &, uchar &) @@ -1986,11 +1981,10 @@ static inline QImage alphaMapFromGlyphData(QFontEngineFT::Glyph *glyph, QFontEng return img; } -QImage *QFontEngineFT::lockedAlphaMapForGlyph(glyph_t glyphIndex, QFixed subPixelPosition, - QFontEngine::GlyphFormat neededFormat, - const QTransform &t, QPoint *offset) +QFontEngine::Glyph *QFontEngineFT::glyphData(glyph_t glyphIndex, QFixed subPixelPosition, + QFontEngine::GlyphFormat neededFormat, const QTransform &t) { - Q_ASSERT(currentlyLockedAlphaMap.isNull()); + Q_ASSERT(cacheEnabled); if (isBitmapFont()) neededFormat = Format_Mono; @@ -2000,33 +1994,10 @@ QImage *QFontEngineFT::lockedAlphaMapForGlyph(glyph_t glyphIndex, QFixed subPixe neededFormat = Format_A8; Glyph *glyph = loadGlyphFor(glyphIndex, subPixelPosition, neededFormat, t); - - if (offset != 0 && glyph != 0) - *offset = QPoint(glyph->x, -glyph->y); - - currentlyLockedAlphaMap = alphaMapFromGlyphData(glyph, neededFormat); - - const bool glyphHasGeometry = glyph != nullptr && glyph->height != 0 && glyph->width != 0; - if (!cacheEnabled && glyph != &emptyGlyph) { - currentlyLockedAlphaMap = currentlyLockedAlphaMap.copy(); - delete glyph; - } - - if (!glyphHasGeometry) + if (!glyph || !glyph->width || !glyph->height) return nullptr; - if (currentlyLockedAlphaMap.isNull()) - return QFontEngine::lockedAlphaMapForGlyph(glyphIndex, subPixelPosition, neededFormat, t, offset); - - QImageData *data = currentlyLockedAlphaMap.data_ptr(); - data->is_locked = true; - - return ¤tlyLockedAlphaMap; -} - -void QFontEngineFT::unlockAlphaMapForGlyph() -{ - QFontEngine::unlockAlphaMapForGlyph(); + return glyph; } static inline bool is2dRotation(const QTransform &t) diff --git a/src/platformsupport/fontdatabases/freetype/qfontengine_ft_p.h b/src/platformsupport/fontdatabases/freetype/qfontengine_ft_p.h index 109bae86e9..2863d206d2 100644 --- a/src/platformsupport/fontdatabases/freetype/qfontengine_ft_p.h +++ b/src/platformsupport/fontdatabases/freetype/qfontengine_ft_p.h @@ -129,20 +129,6 @@ private: class QFontEngineFT : public QFontEngine { public: - - /* we don't cache glyphs that are too large anyway, so we can make this struct rather small */ - struct Glyph { - ~Glyph(); - int linearAdvance : 22; - unsigned char width; - unsigned char height; - short x; - short y; - short advance; - signed char format; - uchar *data; - }; - struct GlyphInfo { int linearAdvance; unsigned short width; @@ -241,11 +227,9 @@ private: QFixed subPixelPosition, const QTransform &matrix, QFontEngine::GlyphFormat format) override; - QImage *lockedAlphaMapForGlyph(glyph_t glyph, QFixed subPixelPosition, - GlyphFormat neededFormat, const QTransform &t, - QPoint *offset) override; + Glyph *glyphData(glyph_t glyph, QFixed subPixelPosition, + GlyphFormat neededFormat, const QTransform &t) override; bool hasInternalCaching() const override { return cacheEnabled; } - void unlockAlphaMapForGlyph() override; bool expectsGammaCorrectedBlending() const override; void removeGlyphFromCache(glyph_t glyph) override; -- cgit v1.2.3