From 7cc8e62f74f66f20c216a8bc8c3187c989a3dcb4 Mon Sep 17 00:00:00 2001 From: Eskil Abrahamsen Blomfeldt Date: Thu, 27 May 2010 09:08:14 +0200 Subject: Optimize initialization of QStaticText Since QStaticText would be created once and used often, not much thought was put into optimizing its initialization. For simplicity, the code would do two drawText() passes. Since this was an unnecessary overhead, the extra pass has been removed and replaced by memmoves instead. Initialization is now twice as fast. Reviewed-by: Samuel --- src/gui/text/qstatictext.cpp | 163 +++++++++++++++++++++++-------------------- src/gui/text/qstatictext_p.h | 19 +++-- 2 files changed, 101 insertions(+), 81 deletions(-) diff --git a/src/gui/text/qstatictext.cpp b/src/gui/text/qstatictext.cpp index c7817c6..2889a96 100644 --- a/src/gui/text/qstatictext.cpp +++ b/src/gui/text/qstatictext.cpp @@ -363,23 +363,24 @@ QSizeF QStaticText::size() const } QStaticTextPrivate::QStaticTextPrivate() - : textWidth(-1.0), items(0), itemCount(0), glyphPool(0), positionPool(0), + : textWidth(-1.0), items(0), itemCount(0), glyphPool(0), positionPool(0), charPool(0), needsRelayout(true), useBackendOptimizations(false), textFormat(Qt::AutoText) { } QStaticTextPrivate::QStaticTextPrivate(const QStaticTextPrivate &other) : text(other.text), font(other.font), textWidth(other.textWidth), matrix(other.matrix), - items(0), itemCount(0), glyphPool(0), positionPool(0), needsRelayout(true), + items(0), itemCount(0), glyphPool(0), positionPool(0), charPool(0), needsRelayout(true), useBackendOptimizations(other.useBackendOptimizations), textFormat(other.textFormat) { } QStaticTextPrivate::~QStaticTextPrivate() { - delete[] items; + delete[] items; delete[] glyphPool; delete[] positionPool; + delete[] charPool; } QStaticTextPrivate *QStaticTextPrivate::get(const QStaticText *q) @@ -395,15 +396,8 @@ namespace { class DrawTextItemRecorder: public QPaintEngine { public: - DrawTextItemRecorder(int expectedItemCount, QStaticTextItem *items, - int expectedGlyphCount, QFixedPoint *positionPool, glyph_t *glyphPool) - : m_items(items), - m_itemCount(0), m_glyphCount(0), - m_expectedItemCount(expectedItemCount), - m_expectedGlyphCount(expectedGlyphCount), - m_glyphPool(glyphPool), - m_positionPool(positionPool), - m_dirtyPen(false) + DrawTextItemRecorder(bool useBackendOptimizations, int numChars) + : m_dirtyPen(false), m_useBackendOptimizations(useBackendOptimizations) { } @@ -415,26 +409,19 @@ namespace { virtual void drawTextItem(const QPointF &position, const QTextItem &textItem) { - const QTextItemInt &ti = static_cast(textItem); - - m_itemCount++; - m_glyphCount += ti.glyphs.numGlyphs; - if (m_items == 0) - return; - - Q_ASSERT(m_itemCount <= m_expectedItemCount); - Q_ASSERT(m_glyphCount <= m_expectedGlyphCount); - - QStaticTextItem *currentItem = (m_items + (m_itemCount - 1)); - currentItem->fontEngine = ti.fontEngine; - currentItem->font = ti.font(); - currentItem->chars = ti.chars; - currentItem->numChars = ti.num_chars; - currentItem->numGlyphs = ti.glyphs.numGlyphs; - currentItem->glyphs = m_glyphPool; - currentItem->glyphPositions = m_positionPool; + const QTextItemInt &ti = static_cast(textItem); + + QStaticTextItem currentItem; + currentItem.fontEngine = ti.fontEngine; + currentItem.font = ti.font(); + currentItem.charOffset = m_chars.size(); + currentItem.numChars = ti.num_chars; + currentItem.numGlyphs = ti.glyphs.numGlyphs; + currentItem.glyphOffset = m_glyphs.size(); // Store offset into glyph pool + currentItem.positionOffset = m_glyphs.size(); // Offset into position pool + currentItem.useBackendOptimizations = m_useBackendOptimizations; if (m_dirtyPen) - currentItem->color = state->pen().color(); + currentItem.color = state->pen().color(); QTransform matrix = state->transform(); matrix.translate(position.x(), position.y()); @@ -447,13 +434,21 @@ namespace { Q_ASSERT(size == ti.glyphs.numGlyphs); Q_ASSERT(size == positions.size()); - memmove(currentItem->glyphs, glyphs.constData(), sizeof(glyph_t) * size); - memmove(currentItem->glyphPositions, positions.constData(), sizeof(QFixedPoint) * size); + m_glyphs.resize(m_glyphs.size() + size); + m_positions.resize(m_glyphs.size()); + m_chars.resize(m_chars.size() + ti.num_chars); - m_glyphPool += size; - m_positionPool += size; - } + glyph_t *glyphsDestination = m_glyphs.data() + currentItem.glyphOffset; + memmove(glyphsDestination, glyphs.constData(), sizeof(glyph_t) * currentItem.numGlyphs); + QFixedPoint *positionsDestination = m_positions.data() + currentItem.positionOffset; + memmove(positionsDestination, positions.constData(), sizeof(QFixedPoint) * currentItem.numGlyphs); + + QChar *charsDestination = m_chars.data() + currentItem.charOffset; + memmove(charsDestination, ti.chars, sizeof(QChar) * currentItem.numChars); + + m_items.append(currentItem); + } virtual bool begin(QPaintDevice *) { return true; } virtual bool end() { return true; } @@ -463,38 +458,42 @@ namespace { return User; } - int itemCount() const + QVector items() const { - return m_itemCount; + return m_items; } - int glyphCount() const + QVector positions() const { - return m_glyphCount; + return m_positions; } - private: - QStaticTextItem *m_items; - int m_itemCount; - int m_glyphCount; - int m_expectedItemCount; - int m_expectedGlyphCount; + QVector glyphs() const + { + return m_glyphs; + } + + QVector chars() const + { + return m_chars; + } - glyph_t *m_glyphPool; - QFixedPoint *m_positionPool; + private: + QVector m_items; + QVector m_positions; + QVector m_glyphs; + QVector m_chars; bool m_dirtyPen; + bool m_useBackendOptimizations; }; class DrawTextItemDevice: public QPaintDevice { public: - DrawTextItemDevice(int expectedItemCount = -1, QStaticTextItem *items = 0, - int expectedGlyphCount = -1, QFixedPoint *positionPool = 0, - glyph_t *glyphPool = 0) + DrawTextItemDevice(bool useBackendOptimizations, int numChars) { - m_paintEngine = new DrawTextItemRecorder(expectedItemCount, items, - expectedGlyphCount, positionPool, glyphPool); + m_paintEngine = new DrawTextItemRecorder(useBackendOptimizations, numChars); } ~DrawTextItemDevice() @@ -538,14 +537,24 @@ namespace { return m_paintEngine; } - int itemCount() const + QVector glyphs() const { - return m_paintEngine->itemCount(); + return m_paintEngine->glyphs(); } - int glyphCount() const + QVector positions() const { - return m_paintEngine->glyphCount(); + return m_paintEngine->positions(); + } + + QVector items() const + { + return m_paintEngine->items(); + } + + QVector chars() const + { + return m_paintEngine->chars(); } private: @@ -616,42 +625,42 @@ void QStaticTextPrivate::init() delete[] items; delete[] glyphPool; delete[] positionPool; + delete[] charPool; position = QPointF(0, 0); - // Draw once to count number of items and glyphs, so that we can use as little memory - // as possible to store the data - DrawTextItemDevice counterDevice; + DrawTextItemDevice device(useBackendOptimizations, text.size()); { - QPainter painter(&counterDevice); + QPainter painter(&device); painter.setFont(font); painter.setTransform(matrix); paintText(QPointF(0, 0), &painter); - } - itemCount = counterDevice.itemCount(); + QVector deviceItems = device.items(); + QVector positions = device.positions(); + QVector glyphs = device.glyphs(); + QVector chars = device.chars(); + + itemCount = deviceItems.size(); items = new QStaticTextItem[itemCount]; - if (useBackendOptimizations) { - for (int i=0; i Date: Thu, 27 May 2010 09:52:25 +0200 Subject: Fixed compilation of QtOpenGL. Reviewed-by: Eskil --- src/opengl/gl2paintengineex/qpaintengineex_opengl2.cpp | 2 +- src/opengl/qpaintengine_opengl.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/opengl/gl2paintengineex/qpaintengineex_opengl2.cpp b/src/opengl/gl2paintengineex/qpaintengineex_opengl2.cpp index 4461358..5758b25 100644 --- a/src/opengl/gl2paintengineex/qpaintengineex_opengl2.cpp +++ b/src/opengl/gl2paintengineex/qpaintengineex_opengl2.cpp @@ -1401,7 +1401,7 @@ void QGL2PaintEngineEx::drawTextItem(const QPointF &p, const QTextItem &textItem { QStaticTextItem staticTextItem; - staticTextItem.chars = ti.chars; + staticTextItem.chars = const_cast(ti.chars); staticTextItem.fontEngine = ti.fontEngine; staticTextItem.glyphs = glyphs.data(); staticTextItem.numChars = ti.num_chars; diff --git a/src/opengl/qpaintengine_opengl.cpp b/src/opengl/qpaintengine_opengl.cpp index 28d37bc..12c487d 100644 --- a/src/opengl/qpaintengine_opengl.cpp +++ b/src/opengl/qpaintengine_opengl.cpp @@ -4999,7 +4999,7 @@ void QOpenGLPaintEngine::drawTextItem(const QPointF &p, const QTextItem &textIte { QStaticTextItem staticTextItem; - staticTextItem.chars = ti.chars; + staticTextItem.chars = const_cast(ti.chars); staticTextItem.fontEngine = ti.fontEngine; staticTextItem.glyphs = glyphs.data(); staticTextItem.numChars = ti.num_chars; -- cgit v0.12 From ea68e4ddcf903bde74ded9d588a09863491d1d56 Mon Sep 17 00:00:00 2001 From: Eskil Abrahamsen Blomfeldt Date: Thu, 27 May 2010 10:28:52 +0200 Subject: Replace memmove with memcpy qMemCopy is faster than memmove, and there's no chance of overlapping source and destination memory in these cases. Reviewed-by: Samuel --- src/gui/text/qstatictext.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/gui/text/qstatictext.cpp b/src/gui/text/qstatictext.cpp index 2889a96..84c1d96 100644 --- a/src/gui/text/qstatictext.cpp +++ b/src/gui/text/qstatictext.cpp @@ -439,13 +439,13 @@ namespace { m_chars.resize(m_chars.size() + ti.num_chars); glyph_t *glyphsDestination = m_glyphs.data() + currentItem.glyphOffset; - memmove(glyphsDestination, glyphs.constData(), sizeof(glyph_t) * currentItem.numGlyphs); + qMemCopy(glyphsDestination, glyphs.constData(), sizeof(glyph_t) * currentItem.numGlyphs); QFixedPoint *positionsDestination = m_positions.data() + currentItem.positionOffset; - memmove(positionsDestination, positions.constData(), sizeof(QFixedPoint) * currentItem.numGlyphs); + qMemCopy(positionsDestination, positions.constData(), sizeof(QFixedPoint) * currentItem.numGlyphs); QChar *charsDestination = m_chars.data() + currentItem.charOffset; - memmove(charsDestination, ti.chars, sizeof(QChar) * currentItem.numChars); + qMemCopy(charsDestination, ti.chars, sizeof(QChar) * currentItem.numChars); m_items.append(currentItem); } @@ -647,13 +647,13 @@ void QStaticTextPrivate::init() items = new QStaticTextItem[itemCount]; glyphPool = new glyph_t[glyphs.size()]; - memmove(glyphPool, glyphs.constData(), glyphs.size() * sizeof(glyph_t)); + qMemCopy(glyphPool, glyphs.constData(), glyphs.size() * sizeof(glyph_t)); positionPool = new QFixedPoint[positions.size()]; - memmove(positionPool, positions.constData(), positions.size() * sizeof(QFixedPoint)); + qMemCopy(positionPool, positions.constData(), positions.size() * sizeof(QFixedPoint)); charPool = new QChar[chars.size()]; - memmove(charPool, chars.constData(), chars.size() * sizeof(QChar)); + qMemCopy(charPool, chars.constData(), chars.size() * sizeof(QChar)); for (int i=0; i Date: Thu, 27 May 2010 10:50:40 +0200 Subject: Fix a bug in QDirectFBPixmapData::fromImage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every image that was converted with flags & Qt::NoOpaqueDetection would be considered to have an alpha channel. This patch fixes that and simplifies the code a little. Merge-request: 637 Reviewed-by: Samuel Rødal --- .../gfxdrivers/directfb/qdirectfbpixmap.cpp | 25 +++++++++------------- src/plugins/gfxdrivers/directfb/qdirectfbpixmap.h | 2 +- 2 files changed, 11 insertions(+), 16 deletions(-) diff --git a/src/plugins/gfxdrivers/directfb/qdirectfbpixmap.cpp b/src/plugins/gfxdrivers/directfb/qdirectfbpixmap.cpp index 80366d1..f704432 100644 --- a/src/plugins/gfxdrivers/directfb/qdirectfbpixmap.cpp +++ b/src/plugins/gfxdrivers/directfb/qdirectfbpixmap.cpp @@ -91,6 +91,7 @@ void QDirectFBPixmapData::resize(int width, int height) setSerialNumber(++global_ser_no); } +#ifdef QT_DIRECTFB_OPAQUE_DETECTION // mostly duplicated from qimage.cpp (QImageData::checkForAlphaPixels) static bool checkForAlphaPixels(const QImage &img) { @@ -160,12 +161,16 @@ static bool checkForAlphaPixels(const QImage &img) return false; } +#endif // QT_DIRECTFB_OPAQUE_DETECTION -bool QDirectFBPixmapData::hasAlphaChannel(const QImage &img) +bool QDirectFBPixmapData::hasAlphaChannel(const QImage &img, Qt::ImageConversionFlags flags) { -#ifndef QT_NO_DIRECTFB_OPAQUE_DETECTION - return checkForAlphaPixels(img); + if (img.depth() == 1) + return true; +#ifdef QT_DIRECTFB_OPAQUE_DETECTION + return ((flags & Qt::NoOpaqueDetection) ? img.hasAlphaChannel() : checkForAlphaPixels(img)); #else + Q_UNUSED(flags); return img.hasAlphaChannel(); #endif } @@ -287,19 +292,9 @@ bool QDirectFBPixmapData::fromDataBufferDescription(const DFBDataBufferDescripti #endif -void QDirectFBPixmapData::fromImage(const QImage &img, - Qt::ImageConversionFlags flags) +void QDirectFBPixmapData::fromImage(const QImage &img, Qt::ImageConversionFlags flags) { - if (img.depth() == 1) { - alpha = true; -#ifndef QT_NO_DIRECTFB_OPAQUE_DETECTION - } else if (flags & Qt::NoOpaqueDetection || QDirectFBPixmapData::hasAlphaChannel(img)) { - alpha = true; -#else - } else if (img.hasAlphaChannel()) { - alpha = true; -#endif - } + alpha = QDirectFBPixmapData::hasAlphaChannel(img, flags); imageFormat = alpha ? screen->alphaPixmapFormat() : screen->pixelFormat(); QImage image; if ((flags & ~Qt::NoOpaqueDetection) != Qt::AutoColor) { diff --git a/src/plugins/gfxdrivers/directfb/qdirectfbpixmap.h b/src/plugins/gfxdrivers/directfb/qdirectfbpixmap.h index da6edc6..343b26e 100644 --- a/src/plugins/gfxdrivers/directfb/qdirectfbpixmap.h +++ b/src/plugins/gfxdrivers/directfb/qdirectfbpixmap.h @@ -86,8 +86,8 @@ public: virtual int metric(QPaintDevice::PaintDeviceMetric m) const { return QDirectFBPaintDevice::metric(m); } inline QImage::Format pixelFormat() const { return imageFormat; } - static bool hasAlphaChannel(const QImage &img); inline bool hasAlphaChannel() const { return alpha; } + static bool hasAlphaChannel(const QImage &img, Qt::ImageConversionFlags flags = Qt::AutoColor); private: #ifdef QT_DIRECTFB_IMAGEPROVIDER bool fromDataBufferDescription(const DFBDataBufferDescription &dataBuffer); -- cgit v0.12 From 2c54f49584023633c5992579d23dc6819ec79c9e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B8rn=20Erik=20Nilsen?= Date: Thu, 27 May 2010 15:28:26 +0200 Subject: Wrong QGraphicsItem::childrenBoundingRect() when applying effects. Problem was that we used the children's raw bounding rect instead of using their effective bounding rect when calculating the bounds. Auto test included. Task-number: QTBUG-10756 --- src/gui/graphicsview/qgraphicsitem.cpp | 6 +++--- tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp | 29 ++++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/src/gui/graphicsview/qgraphicsitem.cpp b/src/gui/graphicsview/qgraphicsitem.cpp index db6c4c5..36d21a6 100644 --- a/src/gui/graphicsview/qgraphicsitem.cpp +++ b/src/gui/graphicsview/qgraphicsitem.cpp @@ -1275,14 +1275,14 @@ void QGraphicsItemPrivate::childrenBoundingRectHelper(QTransform *x, QRectF *rec QTransform matrix = childd->transformToParent(); if (x) matrix *= *x; - *rect |= matrix.mapRect(child->boundingRect()); + *rect |= matrix.mapRect(child->d_ptr->effectiveBoundingRect()); if (!childd->children.isEmpty()) childd->childrenBoundingRectHelper(&matrix, rect); } else { if (x) - *rect |= x->mapRect(child->boundingRect()); + *rect |= x->mapRect(child->d_ptr->effectiveBoundingRect()); else - *rect |= child->boundingRect(); + *rect |= child->d_ptr->effectiveBoundingRect(); if (!childd->children.isEmpty()) childd->childrenBoundingRectHelper(x, rect); } diff --git a/tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp b/tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp index 300afc3..5547b02 100644 --- a/tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp +++ b/tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp @@ -348,6 +348,7 @@ private slots: void childrenBoundingRect2(); void childrenBoundingRect3(); void childrenBoundingRect4(); + void childrenBoundingRect5(); void group(); void setGroup(); void setGroup2(); @@ -3369,6 +3370,34 @@ void tst_QGraphicsItem::childrenBoundingRect4() QCOMPARE(rect2->childrenBoundingRect(), rect3->boundingRect()); } +void tst_QGraphicsItem::childrenBoundingRect5() +{ + QGraphicsScene scene; + + QGraphicsRectItem *parent = scene.addRect(QRectF(0, 0, 100, 100)); + QGraphicsRectItem *child = scene.addRect(QRectF(0, 0, 100, 100)); + child->setParentItem(parent); + + QGraphicsView view(&scene); + view.show(); + + QTest::qWaitForWindowShown(&view); + + // Try to mess up the cached bounding rect. + QRectF expectedChildrenBoundingRect = parent->boundingRect(); + QCOMPARE(parent->childrenBoundingRect(), expectedChildrenBoundingRect); + + // Apply some effects. + QGraphicsDropShadowEffect *dropShadow = new QGraphicsDropShadowEffect; + dropShadow->setOffset(25, 25); + child->setGraphicsEffect(dropShadow); + parent->setGraphicsEffect(new QGraphicsOpacityEffect); + + QVERIFY(parent->childrenBoundingRect() != expectedChildrenBoundingRect); + expectedChildrenBoundingRect |= dropShadow->boundingRect(); + QCOMPARE(parent->childrenBoundingRect(), expectedChildrenBoundingRect); +} + void tst_QGraphicsItem::group() { QGraphicsScene scene; -- cgit v0.12