From d516e5fbed3a7eac20229ead34221c732f85cdb6 Mon Sep 17 00:00:00 2001 From: Bjoern Erik Nilsen Date: Tue, 31 Mar 2009 14:58:15 +0200 Subject: Fixes: Minimize QVariant overhead related to QGraphicsItem::itemChange. RevBy: Andreas AutoTest: included --- src/gui/graphicsview/qgraphicsitem.cpp | 70 ++++++++++++++------------ src/gui/graphicsview/qgraphicsscene.cpp | 24 +++++---- tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp | 18 +++++++ 3 files changed, 69 insertions(+), 43 deletions(-) diff --git a/src/gui/graphicsview/qgraphicsitem.cpp b/src/gui/graphicsview/qgraphicsitem.cpp index d27d3cd..11a03b6 100644 --- a/src/gui/graphicsview/qgraphicsitem.cpp +++ b/src/gui/graphicsview/qgraphicsitem.cpp @@ -1024,9 +1024,8 @@ void QGraphicsItem::setParentItem(QGraphicsItem *parent) } if (parent == d_ptr->parent) return; - QVariant variant; - qVariantSetValue(variant, parent); - parent = qVariantValue(itemChange(ItemParentChange, variant)); + const QVariant newParentVariant(itemChange(ItemParentChange, qVariantFromValue(parent))); + parent = qVariantValue(newParentVariant); if (parent == d_ptr->parent) return; @@ -1041,11 +1040,11 @@ void QGraphicsItem::setParentItem(QGraphicsItem *parent) // We anticipate geometry changes prepareGeometryChange(); + const QVariant thisPointerVariant(qVariantFromValue(this)); if (d_ptr->parent) { // Remove from current parent qt_graphicsitem_removeChild(this, &d_ptr->parent->d_func()->children); - qVariantSetValue(variant, this); - d_ptr->parent->itemChange(ItemChildRemovedChange, variant); + d_ptr->parent->itemChange(ItemChildRemovedChange, thisPointerVariant); } if ((d_ptr->parent = parent)) { @@ -1060,8 +1059,7 @@ void QGraphicsItem::setParentItem(QGraphicsItem *parent) } d_ptr->parent->d_func()->children << this; - qVariantSetValue(variant, this); - d_ptr->parent->itemChange(ItemChildAddedChange, variant); + d_ptr->parent->itemChange(ItemChildAddedChange, thisPointerVariant); if (!implicitUpdate) d_ptr->updateHelper(QRectF(), false, true); @@ -1111,7 +1109,7 @@ void QGraphicsItem::setParentItem(QGraphicsItem *parent) d_ptr->invalidateSceneTransformCache(); // Deliver post-change notification - itemChange(QGraphicsItem::ItemParentHasChanged, qVariantFromValue(parent)); + itemChange(QGraphicsItem::ItemParentHasChanged, newParentVariant); } /*! @@ -1372,9 +1370,9 @@ QString QGraphicsItem::toolTip() const */ void QGraphicsItem::setToolTip(const QString &toolTip) { - QString newCursor = itemChange(ItemToolTipChange, toolTip).toString(); - d_ptr->setExtra(QGraphicsItemPrivate::ExtraToolTip, toolTip); - itemChange(ItemToolTipHasChanged, toolTip); + const QVariant toolTipVariant(itemChange(ItemToolTipChange, toolTip)); + d_ptr->setExtra(QGraphicsItemPrivate::ExtraToolTip, toolTipVariant.toString()); + itemChange(ItemToolTipHasChanged, toolTipVariant); } #endif // QT_NO_TOOLTIP @@ -1416,9 +1414,8 @@ QCursor QGraphicsItem::cursor() const */ void QGraphicsItem::setCursor(const QCursor &cursor) { - QCursor newCursor = qVariantValue(itemChange(ItemCursorChange, - qVariantFromValue(cursor))); - d_ptr->setExtra(QGraphicsItemPrivate::ExtraCursor, newCursor); + const QVariant cursorVariant(itemChange(ItemCursorChange, qVariantFromValue(cursor))); + d_ptr->setExtra(QGraphicsItemPrivate::ExtraCursor, qVariantValue(cursorVariant)); d_ptr->hasCursor = 1; if (d_ptr->scene) { foreach (QGraphicsView *view, d_ptr->scene->views()) { @@ -1435,7 +1432,7 @@ void QGraphicsItem::setCursor(const QCursor &cursor) } } } - itemChange(ItemCursorHasChanged, qVariantFromValue(newCursor)); + itemChange(ItemCursorHasChanged, cursorVariant); } /*! @@ -1529,7 +1526,9 @@ void QGraphicsItemPrivate::setVisibleHelper(bool newVisible, bool explicitly, bo return; // Modify the property. - newVisible = q_ptr->itemChange(QGraphicsItem::ItemVisibleChange, quint32(newVisible)).toBool(); + const QVariant newVisibleVariant(q_ptr->itemChange(QGraphicsItem::ItemVisibleChange, + quint32(newVisible))); + newVisible = newVisibleVariant.toBool(); if (visible == quint32(newVisible)) return; visible = newVisible; @@ -1591,7 +1590,7 @@ void QGraphicsItemPrivate::setVisibleHelper(bool newVisible, bool explicitly, bo } // Deliver post-change notification. - q_ptr->itemChange(QGraphicsItem::ItemVisibleHasChanged, quint32(visible)); + q_ptr->itemChange(QGraphicsItem::ItemVisibleHasChanged, newVisibleVariant); } /*! @@ -1697,7 +1696,9 @@ void QGraphicsItemPrivate::setEnabledHelper(bool newEnabled, bool explicitly, bo } // Modify the property. - enabled = q_ptr->itemChange(QGraphicsItem::ItemEnabledChange, quint32(newEnabled)).toBool(); + const QVariant newEnabledVariant(q_ptr->itemChange(QGraphicsItem::ItemEnabledChange, + quint32(newEnabled))); + enabled = newEnabledVariant.toBool(); // Schedule redraw. if (update) @@ -1709,7 +1710,7 @@ void QGraphicsItemPrivate::setEnabledHelper(bool newEnabled, bool explicitly, bo } // Deliver post-change notification. - q_ptr->itemChange(QGraphicsItem::ItemEnabledHasChanged, quint32(enabled)); + q_ptr->itemChange(QGraphicsItem::ItemEnabledHasChanged, newEnabledVariant); } /*! @@ -1796,7 +1797,8 @@ void QGraphicsItem::setSelected(bool selected) selected = false; if (d_ptr->selected == selected) return; - bool newSelected = itemChange(ItemSelectedChange, quint32(selected)).toBool(); + const QVariant newSelectedVariant(itemChange(ItemSelectedChange, quint32(selected))); + bool newSelected = newSelectedVariant.toBool(); if (d_ptr->selected == newSelected) return; d_ptr->selected = newSelected; @@ -1816,7 +1818,7 @@ void QGraphicsItem::setSelected(bool selected) } // Deliver post-change notification. - itemChange(QGraphicsItem::ItemSelectedHasChanged, quint32(d_ptr->selected)); + itemChange(QGraphicsItem::ItemSelectedHasChanged, newSelectedVariant); } /*! @@ -1892,7 +1894,8 @@ qreal QGraphicsItem::effectiveOpacity() const void QGraphicsItem::setOpacity(qreal opacity) { // Notify change. - qreal newOpacity = itemChange(ItemOpacityChange, double(opacity)).toDouble(); + const QVariant newOpacityVariant(itemChange(ItemOpacityChange, double(opacity))); + qreal newOpacity = newOpacityVariant.toDouble(); // Normalize. newOpacity = qBound(0.0, newOpacity, 1.0); @@ -2759,9 +2762,9 @@ void QGraphicsItem::setMatrix(const QMatrix &matrix, bool combine) return; // Notify the item that the matrix is changing. - QVariant variant; - qVariantSetValue(variant, newTransform.toAffine()); - newTransform = QTransform(qVariantValue(itemChange(ItemMatrixChange, variant))); + QVariant newTransformVariant(itemChange(ItemMatrixChange, + qVariantFromValue(newTransform.toAffine()))); + newTransform = QTransform(qVariantValue(newTransformVariant)); if (oldTransform == newTransform) return; @@ -2773,7 +2776,9 @@ void QGraphicsItem::setMatrix(const QMatrix &matrix, bool combine) d_ptr->invalidateSceneTransformCache(); // Send post-notification. - itemChange(ItemTransformHasChanged, newTransform); + // NB! We have to change the value from QMatrix to QTransform. + qVariantSetValue(newTransformVariant, newTransform); + itemChange(ItemTransformHasChanged, newTransformVariant); } /*! @@ -2805,9 +2810,9 @@ void QGraphicsItem::setTransform(const QTransform &matrix, bool combine) return; // Notify the item that the transformation matrix is changing. - QVariant variant; - qVariantSetValue(variant, newTransform); - newTransform = qVariantValue(itemChange(ItemTransformChange, variant)); + const QVariant newTransformVariant(itemChange(ItemTransformChange, + qVariantFromValue(newTransform))); + newTransform = qVariantValue(newTransformVariant); if (oldTransform == newTransform) return; @@ -2819,7 +2824,7 @@ void QGraphicsItem::setTransform(const QTransform &matrix, bool combine) d_ptr->invalidateSceneTransformCache(); // Send post-notification. - itemChange(ItemTransformHasChanged, newTransform); + itemChange(ItemTransformHasChanged, newTransformVariant); } /*! @@ -2967,7 +2972,8 @@ qreal QGraphicsItem::zValue() const */ void QGraphicsItem::setZValue(qreal z) { - qreal newZ = qreal(itemChange(ItemZValueChange, double(z)).toDouble()); + const QVariant newZVariant(itemChange(ItemZValueChange, double(z))); + qreal newZ = qreal(newZVariant.toDouble()); if (newZ == d_ptr->z) return; d_ptr->z = z; @@ -2979,7 +2985,7 @@ void QGraphicsItem::setZValue(qreal z) d_ptr->scene->d_func()->invalidateSortCache(); } - itemChange(ItemZValueHasChanged, double(newZ)); + itemChange(ItemZValueHasChanged, newZVariant); } /*! diff --git a/src/gui/graphicsview/qgraphicsscene.cpp b/src/gui/graphicsview/qgraphicsscene.cpp index b4dc62c..bcc329e 100644 --- a/src/gui/graphicsview/qgraphicsscene.cpp +++ b/src/gui/graphicsview/qgraphicsscene.cpp @@ -632,10 +632,11 @@ void QGraphicsScenePrivate::_q_updateLater() */ void QGraphicsScenePrivate::_q_polishItems() { + const QVariant booleanTrueVariant(true); foreach (QGraphicsItem *item, unpolishedItems) { if (!item->d_ptr->explicitlyHidden) { - item->itemChange(QGraphicsItem::ItemVisibleChange, true); - item->itemChange(QGraphicsItem::ItemVisibleHasChanged, true); + item->itemChange(QGraphicsItem::ItemVisibleChange, booleanTrueVariant); + item->itemChange(QGraphicsItem::ItemVisibleHasChanged, booleanTrueVariant); } if (item->isWidget()) { QEvent event(QEvent::Polish); @@ -691,9 +692,8 @@ void QGraphicsScenePrivate::_q_removeItemLater(QGraphicsItem *item) Q_Q(QGraphicsScene); if (QGraphicsItem *parent = item->d_func()->parent) { - QVariant variant; - qVariantSetValue(variant, item); - parent->itemChange(QGraphicsItem::ItemChildRemovedChange, variant); + parent->itemChange(QGraphicsItem::ItemChildRemovedChange, + qVariantFromValue(item)); parent->d_func()->children.removeAll(item); } @@ -2847,8 +2847,9 @@ void QGraphicsScene::addItem(QGraphicsItem *item) // Notify the item that its scene is changing, and allow the item to // react. - QGraphicsScene *targetScene = qVariantValue(item->itemChange(QGraphicsItem::ItemSceneChange, - qVariantFromValue(this))); + const QVariant newSceneVariant(item->itemChange(QGraphicsItem::ItemSceneChange, + qVariantFromValue(this))); + QGraphicsScene *targetScene = qVariantValue(newSceneVariant); if (targetScene != this) { if (targetScene && item->scene() != targetScene) targetScene->addItem(item); @@ -2942,7 +2943,7 @@ void QGraphicsScene::addItem(QGraphicsItem *item) emit selectionChanged(); // Deliver post-change notification - item->itemChange(QGraphicsItem::ItemSceneHasChanged, qVariantFromValue(this)); + item->itemChange(QGraphicsItem::ItemSceneHasChanged, newSceneVariant); } /*! @@ -3206,8 +3207,9 @@ void QGraphicsScene::removeItem(QGraphicsItem *item) // Notify the item that it's scene is changing to 0, allowing the item to // react. - QGraphicsScene *targetScene = qVariantValue(item->itemChange(QGraphicsItem::ItemSceneChange, - qVariantFromValue(0))); + const QVariant newSceneVariant(item->itemChange(QGraphicsItem::ItemSceneChange, + qVariantFromValue(0))); + QGraphicsScene *targetScene = qVariantValue(newSceneVariant); if (targetScene != 0 && targetScene != this) { targetScene->addItem(item); return; @@ -3305,7 +3307,7 @@ void QGraphicsScene::removeItem(QGraphicsItem *item) emit selectionChanged(); // Deliver post-change notification - item->itemChange(QGraphicsItem::ItemSceneHasChanged, qVariantFromValue(0)); + item->itemChange(QGraphicsItem::ItemSceneHasChanged, newSceneVariant); } /*! diff --git a/tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp b/tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp index 0f2d671..ad0dc97 100644 --- a/tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp +++ b/tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp @@ -3934,8 +3934,26 @@ void tst_QGraphicsItem::itemChange() tester.itemSceneChangeTargetScene = 0; tester.itemChangeReturnValue = QVariant(); scene.removeItem(&tester); + ++changeCount; // ItemSceneChange + ++changeCount; // ItemSceneHasChanged QCOMPARE(tester.scene(), (QGraphicsScene *)0); } + { + // ItemToolTipChange/ItemToolTipHasChanged + const QString toolTip(QLatin1String("I'm soo cool")); + const QString overridenToolTip(QLatin1String("No, you are not soo cool")); + tester.itemChangeReturnValue = overridenToolTip; + tester.setToolTip(toolTip); + ++changeCount; // ItemToolTipChange + ++changeCount; // ItemToolTipHasChanged + QCOMPARE(tester.changes.size(), changeCount); + QCOMPARE(tester.changes.at(changeCount - 2), QGraphicsItem::ItemToolTipChange); + QCOMPARE(tester.values.at(changeCount - 2).toString(), toolTip); + QCOMPARE(tester.changes.at(changeCount - 1), QGraphicsItem::ItemToolTipHasChanged); + QCOMPARE(tester.values.at(changeCount - 1).toString(), overridenToolTip); + QCOMPARE(tester.toolTip(), overridenToolTip); + tester.itemChangeReturnValue = QVariant(); + } } class EventFilterTesterItem : public QGraphicsLineItem -- cgit v0.12