From 3a5f473a16b3bc64e6793a9a5002d961a2a0762a Mon Sep 17 00:00:00 2001 From: Thierry Bastian Date: Wed, 17 Mar 2010 11:14:15 +0100 Subject: Fix a crash in animation groups when deleting uncontrolled animations The problem was that we were not removing their references from the private object hash and at some point we could access it. Task-number: QTBUG-8910 Reviewed-by: gabi --- src/corelib/animation/qanimationgroup.cpp | 4 ++-- src/corelib/animation/qanimationgroup_p.h | 15 +++++++++++++-- src/corelib/animation/qparallelanimationgroup.cpp | 15 +++++++++------ src/corelib/animation/qparallelanimationgroup_p.h | 2 ++ src/corelib/animation/qsequentialanimationgroup.cpp | 13 ++++++++----- src/corelib/animation/qsequentialanimationgroup_p.h | 2 +- .../tst_qparallelanimationgroup.cpp | 17 ++++++++++++++++- 7 files changed, 51 insertions(+), 17 deletions(-) diff --git a/src/corelib/animation/qanimationgroup.cpp b/src/corelib/animation/qanimationgroup.cpp index a89f949..2decda3 100644 --- a/src/corelib/animation/qanimationgroup.cpp +++ b/src/corelib/animation/qanimationgroup.cpp @@ -244,7 +244,7 @@ QAbstractAnimation *QAnimationGroup::takeAnimation(int index) // in ChildRemoved event d->animations.removeAt(index); animation->setParent(0); - d->animationRemovedAt(index); + d->animationRemoved(index, animation); return animation; } @@ -285,7 +285,7 @@ bool QAnimationGroup::event(QEvent *event) } -void QAnimationGroupPrivate::animationRemovedAt(int index) +void QAnimationGroupPrivate::animationRemoved(int index, QAbstractAnimation *) { Q_Q(QAnimationGroup); Q_UNUSED(index); diff --git a/src/corelib/animation/qanimationgroup_p.h b/src/corelib/animation/qanimationgroup_p.h index 394773b..45603b3 100644 --- a/src/corelib/animation/qanimationgroup_p.h +++ b/src/corelib/animation/qanimationgroup_p.h @@ -72,8 +72,19 @@ public: isGroup = true; } - virtual void animationInsertedAt(int index) { Q_UNUSED(index) }; - virtual void animationRemovedAt(int index); + virtual void animationInsertedAt(int) { } + virtual void animationRemoved(int, QAbstractAnimation *); + + void disconnectUncontrolledAnimation(QAbstractAnimation *anim) + { + //0 for the signal here because we might be called from the animation destructor + QObject::disconnect(anim, 0, q_func(), SLOT(_q_uncontrolledAnimationFinished())); + } + + void connectUncontrolledAnimation(QAbstractAnimation *anim) + { + QObject::connect(anim, SIGNAL(finished()), q_func(), SLOT(_q_uncontrolledAnimationFinished())); + } QList animations; }; diff --git a/src/corelib/animation/qparallelanimationgroup.cpp b/src/corelib/animation/qparallelanimationgroup.cpp index eaa0364..280afed 100644 --- a/src/corelib/animation/qparallelanimationgroup.cpp +++ b/src/corelib/animation/qparallelanimationgroup.cpp @@ -246,11 +246,9 @@ void QParallelAnimationGroupPrivate::_q_uncontrolledAnimationFinished() void QParallelAnimationGroupPrivate::disconnectUncontrolledAnimations() { - Q_Q(QParallelAnimationGroup); - QHash::iterator it = uncontrolledFinishTime.begin(); while (it != uncontrolledFinishTime.end()) { - QObject::disconnect(it.key(), SIGNAL(finished()), q, SLOT(_q_uncontrolledAnimationFinished())); + disconnectUncontrolledAnimation(it.key()); ++it; } @@ -259,13 +257,11 @@ void QParallelAnimationGroupPrivate::disconnectUncontrolledAnimations() void QParallelAnimationGroupPrivate::connectUncontrolledAnimations() { - Q_Q(QParallelAnimationGroup); - for (int i = 0; i < animations.size(); ++i) { QAbstractAnimation *animation = animations.at(i); if (animation->duration() == -1 || animation->loopCount() < 0) { uncontrolledFinishTime[animation] = -1; - QObject::connect(animation, SIGNAL(finished()), q, SLOT(_q_uncontrolledAnimationFinished())); + connectUncontrolledAnimation(animation); } } } @@ -305,6 +301,13 @@ bool QParallelAnimationGroupPrivate::isUncontrolledAnimationFinished(QAbstractAn return uncontrolledFinishTime.value(anim, -1) >= 0; } +void QParallelAnimationGroupPrivate::animationRemoved(int index, QAbstractAnimation *anim) +{ + QAnimationGroupPrivate::animationRemoved(index, anim); + disconnectUncontrolledAnimation(anim); + uncontrolledFinishTime.remove(anim); +} + /*! \reimp */ diff --git a/src/corelib/animation/qparallelanimationgroup_p.h b/src/corelib/animation/qparallelanimationgroup_p.h index a74d496..cab4fa9 100644 --- a/src/corelib/animation/qparallelanimationgroup_p.h +++ b/src/corelib/animation/qparallelanimationgroup_p.h @@ -80,6 +80,8 @@ public: void connectUncontrolledAnimations(); void disconnectUncontrolledAnimations(); + void animationRemoved(int index, QAbstractAnimation *); + // private slot void _q_uncontrolledAnimationFinished(); }; diff --git a/src/corelib/animation/qsequentialanimationgroup.cpp b/src/corelib/animation/qsequentialanimationgroup.cpp index 8ab084a..7617c1f 100644 --- a/src/corelib/animation/qsequentialanimationgroup.cpp +++ b/src/corelib/animation/qsequentialanimationgroup.cpp @@ -479,7 +479,7 @@ void QSequentialAnimationGroupPrivate::activateCurrentAnimation(bool intermediat // connects to the finish signal of uncontrolled animations if (currentAnimation->totalDuration() == -1) - QObject::connect(currentAnimation, SIGNAL(finished()), q, SLOT(_q_uncontrolledAnimationFinished())); + connectUncontrolledAnimation(currentAnimation); currentAnimation->start(); if (!intermediate && state == QSequentialAnimationGroup::Paused) @@ -496,7 +496,7 @@ void QSequentialAnimationGroupPrivate::_q_uncontrolledAnimationFinished() actualDuration.append(-1); actualDuration[currentAnimationIndex] = currentAnimation->currentTime(); - QObject::disconnect(currentAnimation, SIGNAL(finished()), q, SLOT(_q_uncontrolledAnimationFinished())); + disconnectUncontrolledAnimation(currentAnimation); if ((direction == QAbstractAnimation::Forward && currentAnimation == animations.last()) || (direction == QAbstractAnimation::Backward && currentAnimationIndex == 0)) { @@ -543,10 +543,10 @@ void QSequentialAnimationGroupPrivate::animationInsertedAt(int index) the group at index \a index. The animation is no more listed when this method is called. */ -void QSequentialAnimationGroupPrivate::animationRemovedAt(int index) +void QSequentialAnimationGroupPrivate::animationRemoved(int index, QAbstractAnimation *anim) { Q_Q(QSequentialAnimationGroup); - QAnimationGroupPrivate::animationRemovedAt(index); + QAnimationGroupPrivate::animationRemoved(index, anim); Q_ASSERT(currentAnimation); // currentAnimation should always be set @@ -555,7 +555,10 @@ void QSequentialAnimationGroupPrivate::animationRemovedAt(int index) const int currentIndex = animations.indexOf(currentAnimation); if (currentIndex == -1) { - //we're removing the current animation, let's update it to another one + //we're removing the current animation + + disconnectUncontrolledAnimation(currentAnimation); + if (index < animations.count()) setCurrentAnimation(index); //let's try to take the next one else if (index > 0) diff --git a/src/corelib/animation/qsequentialanimationgroup_p.h b/src/corelib/animation/qsequentialanimationgroup_p.h index a55e136..de69916 100644 --- a/src/corelib/animation/qsequentialanimationgroup_p.h +++ b/src/corelib/animation/qsequentialanimationgroup_p.h @@ -85,7 +85,7 @@ public: void activateCurrentAnimation(bool intermediate = false); void animationInsertedAt(int index); - void animationRemovedAt(int index); + void animationRemoved(int index, QAbstractAnimation *anim); bool atEnd() const; diff --git a/tests/auto/qparallelanimationgroup/tst_qparallelanimationgroup.cpp b/tests/auto/qparallelanimationgroup/tst_qparallelanimationgroup.cpp index fb0f3e0..d2d86fb 100644 --- a/tests/auto/qparallelanimationgroup/tst_qparallelanimationgroup.cpp +++ b/tests/auto/qparallelanimationgroup/tst_qparallelanimationgroup.cpp @@ -75,6 +75,8 @@ private slots: void loopCount(); void autoAdd(); void pauseResume(); + + void QTBUG8910_crashWhenRemovingUncontrolledAnimation(); }; tst_QParallelAnimationGroup::tst_QParallelAnimationGroup() @@ -999,9 +1001,22 @@ void tst_QParallelAnimationGroup::pauseResume() QCOMPARE(spy.count(), 2); //this shouldn't have changed group.resume(); QCOMPARE(spy.count(), 2); //this shouldn't have changed +} - +void tst_QParallelAnimationGroup::QTBUG8910_crashWhenRemovingUncontrolledAnimation() +{ + QParallelAnimationGroup group; + TestAnimation *anim = new TestAnimation; + anim->setLoopCount(-1); + TestAnimation *anim2 = new TestAnimation; + anim2->setLoopCount(-1); + group.addAnimation(anim); + group.addAnimation(anim2); + group.start(); + delete anim; + // it would crash here because the internals of the group would still have a reference to anim + delete anim2; } -- cgit v0.12 From e1915815bc5ef86b3844608bba46769da5173363 Mon Sep 17 00:00:00 2001 From: Eskil Abrahamsen Blomfeldt Date: Wed, 17 Mar 2010 10:53:34 +0100 Subject: Optimize speed of QTextLayout and QPainter::drawText QFontEngine::boundingBox() is potentially an expensive function, because the font engines have to consult the underlying font system for the values and usually do not cache the results. To account for negative right bearing in the case of text layouts, we would call the boundingBox() function at every potential break point in the text, causing a great performance hit on text drawing. To minimize the impact of this, we only calculate the right bearing when we have to: 1. If subtracting the minimum right bearing from the width would cause the line to expand beyond its maximum line width, then we have to get the actual metrics of the last glyph to check if we need to break. 2. The line's final textWidth should include the negative right bearing, so unless it has already been calculated, we calculate it when the correct line width has been found. This gives us a potentially huge speed-up, since boundingBox() will in the common case only be called once for the last glyph in each text line. Task-number: QTBUG-9074 Reviewed-by: Simon Hausmann --- src/gui/text/qtextlayout.cpp | 122 +++++++++++++++++++++++++++++-------------- 1 file changed, 82 insertions(+), 40 deletions(-) diff --git a/src/gui/text/qtextlayout.cpp b/src/gui/text/qtextlayout.cpp index af91603..204effa 100644 --- a/src/gui/text/qtextlayout.cpp +++ b/src/gui/text/qtextlayout.cpp @@ -1644,28 +1644,67 @@ namespace { struct LineBreakHelper { - LineBreakHelper() : glyphCount(0), maxGlyphs(0), manualWrap(false) {} + LineBreakHelper() + : glyphCount(0), maxGlyphs(0), currentPosition(0), fontEngine(0), logClusters(0), + manualWrap(false) + { + } + QScriptLine tmpData; QScriptLine spaceData; + QGlyphLayout glyphs; + int glyphCount; int maxGlyphs; + int currentPosition; QFixed minw; QFixed softHyphenWidth; QFixed rightBearing; + QFixed minimumRightBearing; + + QFontEngine *fontEngine; + const unsigned short *logClusters; bool manualWrap; bool checkFullOtherwiseExtend(QScriptLine &line); + + QFixed calculateNewWidth(const QScriptLine &line) const { + return line.textWidth + tmpData.textWidth + spaceData.textWidth + softHyphenWidth + - qMin(rightBearing, QFixed()); + } + + inline glyph_t currentGlyph() const + { + Q_ASSERT(currentPosition > 0); + return glyphs.glyphs[logClusters[currentPosition - 1]]; + } + + inline void adjustRightBearing() + { + if (currentPosition <= 0) + return; + + glyph_metrics_t gi = fontEngine->boundingBox(currentGlyph()); + if (gi.isValid()) + rightBearing = qMin(QFixed(), gi.xoff - gi.x - gi.width); + } + + inline void resetRightBearing() + { + rightBearing = QFixed(1); // Any positive number is defined as invalid since only + // negative right bearings are interesting to us. + } }; inline bool LineBreakHelper::checkFullOtherwiseExtend(QScriptLine &line) { LB_DEBUG("possible break width %f, spacew=%f", tmpData.textWidth.toReal(), spaceData.textWidth.toReal()); - QFixed newWidth = line.textWidth + tmpData.textWidth + spaceData.textWidth + softHyphenWidth + rightBearing; + QFixed newWidth = calculateNewWidth(line); if (line.length && !manualWrap && (newWidth > line.width || glyphCount > maxGlyphs)) return true; @@ -1741,13 +1780,12 @@ void QTextLine::layout_helper(int maxGlyphs) Qt::Alignment alignment = eng->option.alignment(); const HB_CharAttributes *attributes = eng->attributes(); - int pos = line.from; + lbh.currentPosition = line.from; int end = 0; - QGlyphLayout glyphs; - const unsigned short *logClusters = eng->layoutData->logClustersPtr; + lbh.logClusters = eng->layoutData->logClustersPtr; while (newItem < eng->layoutData->items.size()) { - lbh.rightBearing = 0; + lbh.resetRightBearing(); lbh.softHyphenWidth = 0; if (newItem != item) { item = newItem; @@ -1755,13 +1793,19 @@ void QTextLine::layout_helper(int maxGlyphs) if (!current.num_glyphs) { eng->shape(item); attributes = eng->attributes(); - logClusters = eng->layoutData->logClustersPtr; + lbh.logClusters = eng->layoutData->logClustersPtr; } - pos = qMax(line.from, current.position); + lbh.currentPosition = qMax(line.from, current.position); end = current.position + eng->length(item); - glyphs = eng->shapedGlyphs(¤t); + lbh.glyphs = eng->shapedGlyphs(¤t); } 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, @@ -1791,8 +1835,8 @@ void QTextLine::layout_helper(int maxGlyphs) if (!line.length && !lbh.tmpData.length) line.setDefaultHeight(eng); if (eng->option.flags() & QTextOption::ShowLineAndParagraphSeparators) { - addNextCluster(pos, end, lbh.tmpData, lbh.glyphCount, - current, logClusters, glyphs); + addNextCluster(lbh.currentPosition, end, lbh.tmpData, lbh.glyphCount, + current, lbh.logClusters, lbh.glyphs); } else { lbh.tmpData.length++; } @@ -1811,10 +1855,10 @@ void QTextLine::layout_helper(int maxGlyphs) ++lbh.glyphCount; if (lbh.checkFullOtherwiseExtend(line)) goto found; - } else if (attributes[pos].whiteSpace) { - while (pos < end && attributes[pos].whiteSpace) - addNextCluster(pos, end, lbh.spaceData, lbh.glyphCount, - current, logClusters, glyphs); + } else if (attributes[lbh.currentPosition].whiteSpace) { + while (lbh.currentPosition < end && attributes[lbh.currentPosition].whiteSpace) + addNextCluster(lbh.currentPosition, end, lbh.spaceData, lbh.glyphCount, + current, lbh.logClusters, lbh.glyphs); if (!lbh.manualWrap && lbh.spaceData.textWidth > line.width) { lbh.spaceData.textWidth = line.width; // ignore spaces that fall out of the line. @@ -1823,19 +1867,19 @@ void QTextLine::layout_helper(int maxGlyphs) } else { bool sb_or_ws = false; do { - addNextCluster(pos, end, lbh.tmpData, lbh.glyphCount, - current, logClusters, glyphs); + addNextCluster(lbh.currentPosition, end, lbh.tmpData, lbh.glyphCount, + current, lbh.logClusters, lbh.glyphs); - if (attributes[pos].whiteSpace || attributes[pos-1].lineBreakType != HB_NoBreak) { + if (attributes[lbh.currentPosition].whiteSpace || attributes[lbh.currentPosition-1].lineBreakType != HB_NoBreak) { sb_or_ws = true; break; - } else if (breakany && attributes[pos].charStop) { + } else if (breakany && attributes[lbh.currentPosition].charStop) { break; } - } while (pos < end); + } while (lbh.currentPosition < end); lbh.minw = qMax(lbh.tmpData.textWidth, lbh.minw); - if (pos && attributes[pos - 1].lineBreakType == HB_SoftHyphen) { + if (lbh.currentPosition && attributes[lbh.currentPosition - 1].lineBreakType == HB_SoftHyphen) { // if we are splitting up a word because of // a soft hyphen then we ... // @@ -1853,41 +1897,39 @@ void QTextLine::layout_helper(int maxGlyphs) // and thus become invisible again. // if (line.length) - lbh.softHyphenWidth = glyphs.advances_x[logClusters[pos - 1]]; + lbh.softHyphenWidth = lbh.glyphs.advances_x[lbh.logClusters[lbh.currentPosition - 1]]; else if (breakany) - lbh.tmpData.textWidth += glyphs.advances_x[logClusters[pos - 1]]; + lbh.tmpData.textWidth += lbh.glyphs.advances_x[lbh.logClusters[lbh.currentPosition - 1]]; } // The actual width of the text needs to take the right bearing into account. The // right bearing is left-ward, which means that if the rightmost pixel is to the right // of the advance of the glyph, the bearing will be negative. We flip the sign // for the code to be more readable. Logic borrowed from qfontmetrics.cpp. - if (pos) { - QFontEngine *fontEngine = eng->fontEngine(current); - glyph_t glyph = glyphs.glyphs[logClusters[pos - 1]]; - glyph_metrics_t gi = fontEngine->boundingBox(glyph); - if (gi.isValid()) - lbh.rightBearing = qMax(QFixed(), -(gi.xoff - gi.x - gi.width)); - } + // We ignore the right bearing if the minimum negative bearing is too little to + // expand the text beyond the edge. + if (sb_or_ws|breakany) { + if (lbh.calculateNewWidth(line) + lbh.minimumRightBearing > line.width) + lbh.adjustRightBearing(); + if (lbh.checkFullOtherwiseExtend(line)) { + if (!breakany) { + line.textWidth += lbh.softHyphenWidth; + } - if ((sb_or_ws|breakany) && lbh.checkFullOtherwiseExtend(line)) { - if (!breakany) { - line.textWidth += lbh.softHyphenWidth; + goto found; } - - line.textWidth += lbh.rightBearing; - - goto found; } } - if (pos == end) + if (lbh.currentPosition == end) newItem = item + 1; } LB_DEBUG("reached end of line"); lbh.checkFullOtherwiseExtend(line); - line.textWidth += lbh.rightBearing; - found: + if (lbh.rightBearing > 0) // If right bearing has not yet been adjusted + lbh.adjustRightBearing(); + line.textWidth -= qMin(QFixed(), lbh.rightBearing); + if (line.length == 0) { LB_DEBUG("no break available in line, adding temp: length %d, width %f, space: length %d, width %f", lbh.tmpData.length, lbh.tmpData.textWidth.toReal(), -- cgit v0.12 From 6aba7215a0ac2092a7f00015df0d7a80681ef6bc Mon Sep 17 00:00:00 2001 From: Yoann Lopes Date: Wed, 17 Mar 2010 14:28:50 +0100 Subject: Fixes blending problem when paiting non-opaque items with cache enabled. In most cases when partially updating an item, the old content of the cache needs to be replaced with the new content (CompositionMode_Source). But in a specific case when using DeviceCoordinateCache and when the item is transformed, the new content needs to be blended (CompositionMode_SourceAtop) with the old one to avoid incorrect fully transparent background. Autotest included. Task-number: QTBUG-7863 Reviewed-by: bnilsen --- src/gui/graphicsview/qgraphicsscene.cpp | 6 ++ tests/auto/qgraphicsscene/tst_qgraphicsscene.cpp | 132 ++++++++++++++++++++++- 2 files changed, 136 insertions(+), 2 deletions(-) diff --git a/src/gui/graphicsview/qgraphicsscene.cpp b/src/gui/graphicsview/qgraphicsscene.cpp index 4ee2301..6934abc 100644 --- a/src/gui/graphicsview/qgraphicsscene.cpp +++ b/src/gui/graphicsview/qgraphicsscene.cpp @@ -4285,6 +4285,12 @@ static void _q_paintIntoCache(QPixmap *pix, QGraphicsItem *item, const QRegion & if (!subPix.isNull()) { // Blit the subpixmap into the main pixmap. pixmapPainter.begin(pix); + if (item->cacheMode() == QGraphicsItem::DeviceCoordinateCache + && itemToPixmap.type() > QTransform::TxTranslate) { + pixmapPainter.setCompositionMode(QPainter::CompositionMode_SourceAtop); + } else { + pixmapPainter.setCompositionMode(QPainter::CompositionMode_Source); + } pixmapPainter.setClipRegion(pixmapExposed); pixmapPainter.drawPixmap(br.topLeft(), subPix); pixmapPainter.end(); diff --git a/tests/auto/qgraphicsscene/tst_qgraphicsscene.cpp b/tests/auto/qgraphicsscene/tst_qgraphicsscene.cpp index 9d437d6..9cd86a6 100644 --- a/tests/auto/qgraphicsscene/tst_qgraphicsscene.cpp +++ b/tests/auto/qgraphicsscene/tst_qgraphicsscene.cpp @@ -283,6 +283,7 @@ private slots: void task250680_childClip(); void taskQTBUG_5904_crashWithDeviceCoordinateCache(); void taskQT657_paintIntoCacheWithTransparentParts(); + void taskQTBUG_7863_paintIntoCacheWithTransparentParts(); }; void tst_QGraphicsScene::initTestCase() @@ -4348,8 +4349,9 @@ void tst_QGraphicsScene::taskQTBUG_5904_crashWithDeviceCoordinateCache() void tst_QGraphicsScene::taskQT657_paintIntoCacheWithTransparentParts() { + // Test using DeviceCoordinateCache and opaque item QWidget *w = new QWidget(); - w->setPalette(Qt::blue); + w->setPalette(QColor(0, 0, 255)); w->setGeometry(0, 0, 50, 50); QGraphicsScene *scene = new QGraphicsScene(); @@ -4361,7 +4363,7 @@ void tst_QGraphicsScene::taskQT657_paintIntoCacheWithTransparentParts() view->show(); QTest::qWaitForWindowShown(view); - w->update(10,10,10,10); + w->update(10, 10, 10, 10); QTest::qWait(50); QPixmap pix; @@ -4382,6 +4384,132 @@ void tst_QGraphicsScene::taskQT657_paintIntoCacheWithTransparentParts() for(int j = 0; j < im.height(); j++) QCOMPARE(qAlpha(im.pixel(i, j)), 255); } + + delete w; +} + +void tst_QGraphicsScene::taskQTBUG_7863_paintIntoCacheWithTransparentParts() +{ + // Test using DeviceCoordinateCache and semi-transparent item + { + QGraphicsRectItem *backItem = new QGraphicsRectItem(0, 0, 100, 100); + backItem->setBrush(QColor(255, 255, 0)); + QGraphicsRectItem *rectItem = new QGraphicsRectItem(0, 0, 50, 50); + rectItem->setBrush(QColor(0, 0, 255, 125)); + rectItem->setParentItem(backItem); + + QGraphicsScene *scene = new QGraphicsScene(); + QGraphicsView *view = new QGraphicsView(scene); + + scene->addItem(backItem); + rectItem->setCacheMode(QGraphicsItem::DeviceCoordinateCache); + backItem->rotate(15); + + view->show(); + QTest::qWaitForWindowShown(view); + rectItem->update(10, 10, 10, 10); + QTest::qWait(50); + + QPixmap pix; + QGraphicsItemPrivate* itemp = QGraphicsItemPrivate::get(rectItem); + QPixmapCache::Key key = itemp->extraItemCache()->deviceData.value(view->viewport()).key; + QVERIFY(QPixmapCache::find(key, &pix)); + + QTransform t = rectItem->sceneTransform(); + // Map from scene coordinates to pixmap coordinates. + // X origin in the pixmap is the most-left point + // of the item's boundingRect in the scene. + qreal adjust = t.mapRect(rectItem->boundingRect().toRect()).left(); + QRect rect = t.mapRect(QRect(10, 10, 10, 10)).adjusted(-adjust, 0, -adjust + 1, 1); + QPixmap subpix = pix.copy(rect); + + QImage im = subpix.toImage(); + for(int i = 0; i < im.width(); i++) { + for(int j = 0; j < im.height(); j++) { + QCOMPARE(qAlpha(im.pixel(i, j)), 125); + } + } + + delete view; + } + + // Test using ItemCoordinateCache and opaque item + { + QGraphicsRectItem *rectItem = new QGraphicsRectItem(0, 0, 50, 50); + rectItem->setBrush(QColor(0, 0, 255)); + + QGraphicsScene *scene = new QGraphicsScene(); + QGraphicsView *view = new QGraphicsView(scene); + + scene->addItem(rectItem); + rectItem->setCacheMode(QGraphicsItem::ItemCoordinateCache); + rectItem->rotate(15); + + view->show(); + QTest::qWaitForWindowShown(view); + rectItem->update(10, 10, 10, 10); + QTest::qWait(50); + + QPixmap pix; + QGraphicsItemPrivate* itemp = QGraphicsItemPrivate::get(rectItem); + QPixmapCache::Key key = itemp->extraItemCache()->key; + QVERIFY(QPixmapCache::find(key, &pix)); + + QTransform t = rectItem->sceneTransform(); + // Map from scene coordinates to pixmap coordinates. + // X origin in the pixmap is the most-left point + // of the item's boundingRect in the scene. + qreal adjust = t.mapRect(rectItem->boundingRect().toRect()).left(); + QRect rect = t.mapRect(QRect(10, 10, 10, 10)).adjusted(-adjust, 0, -adjust + 1, 1); + QPixmap subpix = pix.copy(rect); + + QImage im = subpix.toImage(); + for(int i = 0; i < im.width(); i++) { + for(int j = 0; j < im.height(); j++) + QCOMPARE(qAlpha(im.pixel(i, j)), 255); + } + + delete view; + } + + // Test using ItemCoordinateCache and semi-transparent item + { + QGraphicsRectItem *rectItem = new QGraphicsRectItem(0, 0, 50, 50); + rectItem->setBrush(QColor(0, 0, 255, 125)); + + QGraphicsScene *scene = new QGraphicsScene(); + QGraphicsView *view = new QGraphicsView(scene); + + scene->addItem(rectItem); + rectItem->setCacheMode(QGraphicsItem::ItemCoordinateCache); + rectItem->rotate(15); + + view->show(); + QTest::qWaitForWindowShown(view); + rectItem->update(10, 10, 10, 10); + QTest::qWait(50); + + QPixmap pix; + QGraphicsItemPrivate* itemp = QGraphicsItemPrivate::get(rectItem); + QPixmapCache::Key key = itemp->extraItemCache()->key; + QVERIFY(QPixmapCache::find(key, &pix)); + + QTransform t = rectItem->sceneTransform(); + // Map from scene coordinates to pixmap coordinates. + // X origin in the pixmap is the most-left point + // of the item's boundingRect in the scene. + qreal adjust = t.mapRect(rectItem->boundingRect().toRect()).left(); + QRect rect = t.mapRect(QRect(10, 10, 10, 10)).adjusted(-adjust, 0, -adjust + 1, 1); + QPixmap subpix = pix.copy(rect); + + QImage im = subpix.toImage(); + for(int i = 0; i < im.width(); i++) { + for(int j = 0; j < im.height(); j++) + QCOMPARE(qAlpha(im.pixel(i, j)), 125); + } + + delete view; + } } QTEST_MAIN(tst_QGraphicsScene) -- cgit v0.12 From 1ebeb971d3382aec0fff927ce2d6ea7b0452584f Mon Sep 17 00:00:00 2001 From: Eskil Abrahamsen Blomfeldt Date: Wed, 17 Mar 2010 14:40:24 +0100 Subject: Optimize getting bearings of a glyph on Windows for true type fonts Usually, the bearing of a glyph is calculated based on the bounding box of the glyph. However, retrieving these bounds is a costly operation on Windows. For true type fonts, we can retrieve the bearings directly by getting the ABC widths, so as an optimization we implement a new virtual function in QFontEngine which retrieves the bearings of the glyph. The default implementation uses boundingBox, but on Windows we try to take the shortcut. Reviewed-by: Simon Hausmann --- src/gui/text/qfontengine.cpp | 14 ++++++++++++++ src/gui/text/qfontengine_p.h | 3 +++ src/gui/text/qfontengine_win.cpp | 24 ++++++++++++++++++++++++ src/gui/text/qfontengine_win_p.h | 2 ++ src/gui/text/qfontmetrics.cpp | 21 +++++++++++++-------- src/gui/text/qtextlayout.cpp | 6 +++--- 6 files changed, 59 insertions(+), 11 deletions(-) diff --git a/src/gui/text/qfontengine.cpp b/src/gui/text/qfontengine.cpp index c000457..3ec389f 100644 --- a/src/gui/text/qfontengine.cpp +++ b/src/gui/text/qfontengine.cpp @@ -379,6 +379,14 @@ void QFontEngine::getGlyphPositions(const QGlyphLayout &glyphs, const QTransform Q_ASSERT(positions.size() == glyphs_out.size()); } +void QFontEngine::getGlyphBearings(glyph_t glyph, qreal *leftBearing, qreal *rightBearing) +{ + glyph_metrics_t gi = boundingBox(glyph); + if (leftBearing != 0) + *leftBearing = gi.x.toReal(); + if (rightBearing != 0) + *rightBearing = (gi.xoff - gi.x - gi.width).toReal(); +} glyph_metrics_t QFontEngine::tightBoundingBox(const QGlyphLayout &glyphs) { @@ -1385,6 +1393,12 @@ glyph_metrics_t QFontEngineMulti::boundingBox(const QGlyphLayout &glyphs) return overall; } +void QFontEngineMulti::getGlyphBearings(glyph_t glyph, qreal *leftBearing, qreal *rightBearing) +{ + int which = highByte(glyph); + engine(which)->getGlyphBearings(stripped(glyph), leftBearing, rightBearing); +} + void QFontEngineMulti::addOutlineToPath(qreal x, qreal y, const QGlyphLayout &glyphs, QPainterPath *path, QTextItem::RenderFlags flags) { diff --git a/src/gui/text/qfontengine_p.h b/src/gui/text/qfontengine_p.h index 71ab5a5..e645caf 100644 --- a/src/gui/text/qfontengine_p.h +++ b/src/gui/text/qfontengine_p.h @@ -206,6 +206,8 @@ public: virtual qreal minLeftBearing() const { return qreal(); } virtual qreal minRightBearing() const { return qreal(); } + virtual void getGlyphBearings(glyph_t glyph, qreal *leftBearing = 0, qreal *rightBearing = 0); + virtual const char *name() const = 0; virtual bool canRender(const QChar *string, int len) = 0; @@ -374,6 +376,7 @@ public: virtual void recalcAdvances(QGlyphLayout *, QTextEngine::ShaperFlags) const; virtual void doKerning(QGlyphLayout *, QTextEngine::ShaperFlags) const; virtual void addOutlineToPath(qreal, qreal, const QGlyphLayout &, QPainterPath *, QTextItem::RenderFlags flags); + virtual void getGlyphBearings(glyph_t glyph, qreal *leftBearing = 0, qreal *rightBearing = 0); virtual QFixed ascent() const; virtual QFixed descent() const; diff --git a/src/gui/text/qfontengine_win.cpp b/src/gui/text/qfontengine_win.cpp index 1a815d3..a133b48 100644 --- a/src/gui/text/qfontengine_win.cpp +++ b/src/gui/text/qfontengine_win.cpp @@ -649,6 +649,30 @@ static const ushort char_table[] = { static const int char_table_entries = sizeof(char_table)/sizeof(ushort); +void QFontEngineWin::getGlyphBearings(glyph_t glyph, qreal *leftBearing, qreal *rightBearing) +{ + HDC hdc = shared_dc(); + SelectObject(hdc, hfont); + +#ifndef Q_WS_WINCE + if (ttf) +#endif + + { + ABC abcWidths; + GetCharABCWidthsI(hdc, glyph, 1, 0, &abcWidths); + if (leftBearing) + *leftBearing = abcWidths.abcA; + if (rightBearing) + *rightBearing = abcWidths.abcC; + } + +#ifndef Q_WS_WINCE + else { + QFontEngine::getGlyphBearings(glyph, leftBearing, rightBearing); + } +#endif +} qreal QFontEngineWin::minLeftBearing() const { diff --git a/src/gui/text/qfontengine_win_p.h b/src/gui/text/qfontengine_win_p.h index f9d8f8b..f19e48e 100644 --- a/src/gui/text/qfontengine_win_p.h +++ b/src/gui/text/qfontengine_win_p.h @@ -106,6 +106,8 @@ public: virtual QImage alphaMapForGlyph(glyph_t, const QTransform &xform); virtual QImage alphaRGBMapForGlyph(glyph_t t, int margin, const QTransform &xform); + virtual void getGlyphBearings(glyph_t glyph, qreal *leftBearing = 0, qreal *rightBearing = 0); + int getGlyphIndexes(const QChar *ch, int numChars, QGlyphLayout *glyphs, bool mirrored) const; void getCMap(); diff --git a/src/gui/text/qfontmetrics.cpp b/src/gui/text/qfontmetrics.cpp index 41d0af1..44a18de 100644 --- a/src/gui/text/qfontmetrics.cpp +++ b/src/gui/text/qfontmetrics.cpp @@ -472,8 +472,9 @@ int QFontMetrics::leftBearing(QChar ch) const int nglyphs = 9; engine->stringToCMap(&ch, 1, &glyphs, &nglyphs, 0); // ### can nglyphs != 1 happen at all? Not currently I think - glyph_metrics_t gi = engine->boundingBox(glyphs.glyphs[0]); - return qRound(gi.x); + qreal lb; + engine->getGlyphBearings(glyphs.glyphs[0], &lb); + return qRound(lb); } /*! @@ -506,8 +507,9 @@ int QFontMetrics::rightBearing(QChar ch) const int nglyphs = 9; engine->stringToCMap(&ch, 1, &glyphs, &nglyphs, 0); // ### can nglyphs != 1 happen at all? Not currently I think - glyph_metrics_t gi = engine->boundingBox(glyphs.glyphs[0]); - return qRound(gi.xoff - gi.x - gi.width); + qreal rb; + engine->getGlyphBearings(glyphs.glyphs[0], 0, &rb); + return qRound(rb); } /*! @@ -1317,8 +1319,9 @@ qreal QFontMetricsF::leftBearing(QChar ch) const int nglyphs = 9; engine->stringToCMap(&ch, 1, &glyphs, &nglyphs, 0); // ### can nglyphs != 1 happen at all? Not currently I think - glyph_metrics_t gi = engine->boundingBox(glyphs.glyphs[0]); - return gi.x.toReal(); + qreal lb; + engine->getGlyphBearings(glyphs.glyphs[0], &lb); + return lb; } /*! @@ -1351,8 +1354,10 @@ qreal QFontMetricsF::rightBearing(QChar ch) const int nglyphs = 9; engine->stringToCMap(&ch, 1, &glyphs, &nglyphs, 0); // ### can nglyphs != 1 happen at all? Not currently I think - glyph_metrics_t gi = engine->boundingBox(glyphs.glyphs[0]); - return (gi.xoff - gi.x - gi.width).toReal(); + qreal rb; + engine->getGlyphBearings(glyphs.glyphs[0], 0, &rb); + return rb; + } /*! diff --git a/src/gui/text/qtextlayout.cpp b/src/gui/text/qtextlayout.cpp index 204effa..3c0e85e 100644 --- a/src/gui/text/qtextlayout.cpp +++ b/src/gui/text/qtextlayout.cpp @@ -1688,9 +1688,9 @@ namespace { if (currentPosition <= 0) return; - glyph_metrics_t gi = fontEngine->boundingBox(currentGlyph()); - if (gi.isValid()) - rightBearing = qMin(QFixed(), gi.xoff - gi.x - gi.width); + qreal rb; + fontEngine->getGlyphBearings(currentGlyph(), 0, &rb); + rightBearing = qMin(QFixed(), QFixed::fromReal(rb)); } inline void resetRightBearing() -- cgit v0.12