Fixes: Minimize QVariant overhead related to QGraphicsItem::itemChange.

RevBy:    Andreas
AutoTest: included

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<QGraphicsItem *>(variant, parent);
-    parent = qVariantValue<QGraphicsItem *>(itemChange(ItemParentChange, variant));
+    const QVariant newParentVariant(itemChange(ItemParentChange, qVariantFromValue<QGraphicsItem *>(parent)));
+    parent = qVariantValue<QGraphicsItem *>(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<QGraphicsItem *>(this));
     if (d_ptr->parent) {
         // Remove from current parent
         qt_graphicsitem_removeChild(this, &d_ptr->parent->d_func()->children);
-        qVariantSetValue<QGraphicsItem *>(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<QGraphicsItem *>(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<QGraphicsItem *>(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<QCursor>(itemChange(ItemCursorChange,
-                                                          qVariantFromValue<QCursor>(cursor)));
-    d_ptr->setExtra(QGraphicsItemPrivate::ExtraCursor, newCursor);
+    const QVariant cursorVariant(itemChange(ItemCursorChange, qVariantFromValue<QCursor>(cursor)));
+    d_ptr->setExtra(QGraphicsItemPrivate::ExtraCursor, qVariantValue<QCursor>(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<QCursor>(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<qreal>(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<QMatrix>(variant, newTransform.toAffine());
-    newTransform = QTransform(qVariantValue<QMatrix>(itemChange(ItemMatrixChange, variant)));
+    QVariant newTransformVariant(itemChange(ItemMatrixChange,
+                                            qVariantFromValue<QMatrix>(newTransform.toAffine())));
+    newTransform = QTransform(qVariantValue<QMatrix>(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<QTransform>(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<QTransform>(variant, newTransform);
-    newTransform = qVariantValue<QTransform>(itemChange(ItemTransformChange, variant));
+    const QVariant newTransformVariant(itemChange(ItemTransformChange,
+                                                  qVariantFromValue<QTransform>(newTransform)));
+    newTransform = qVariantValue<QTransform>(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<QGraphicsItem *>(variant, item);
-        parent->itemChange(QGraphicsItem::ItemChildRemovedChange, variant);
+        parent->itemChange(QGraphicsItem::ItemChildRemovedChange,
+                           qVariantFromValue<QGraphicsItem *>(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<QGraphicsScene *>(item->itemChange(QGraphicsItem::ItemSceneChange,
-                                                                                   qVariantFromValue<QGraphicsScene *>(this)));
+    const QVariant newSceneVariant(item->itemChange(QGraphicsItem::ItemSceneChange,
+                                                    qVariantFromValue<QGraphicsScene *>(this)));
+    QGraphicsScene *targetScene = qVariantValue<QGraphicsScene *>(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<QGraphicsScene *>(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<QGraphicsScene *>(item->itemChange(QGraphicsItem::ItemSceneChange,
-                                                                                   qVariantFromValue<QGraphicsScene *>(0)));
+    const QVariant newSceneVariant(item->itemChange(QGraphicsItem::ItemSceneChange,
+                                                    qVariantFromValue<QGraphicsScene *>(0)));
+    QGraphicsScene *targetScene = qVariantValue<QGraphicsScene *>(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<QGraphicsScene *>(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