Remove scene transform cache from QGraphicsItem.

Now that we have a recursive painting algorithm these types of
optimizations are no longer necessary. In fact they only cause more
problems and clutter up the code unnecessarily. Removing this also
removes extra overhead from moving and transforming items.

Reviewed-by: Lars

4 files changed, 10 insertions, 78 deletions

diff --git a/src/gui/graphicsview/qgraphicsitem.cpp b/src/gui/graphicsview/qgraphicsitem.cpp
index 14658cf..c1848e0 100644
--- a/src/gui/graphicsview/qgraphicsitem.cpp
+++ b/src/gui/graphicsview/qgraphicsitem.cpp
@@ -918,9 +918,6 @@ void QGraphicsItemPrivate::setParentItemHelper(QGraphicsItem *newParent, bool de
     // Resolve depth.
     resolveDepth(parent ? parent->d_ptr->depth : -1);
 
-    // Invalidate transform cache.
-    invalidateSceneTransformCache();
-
     // Deliver post-change notification
     q->itemChange(QGraphicsItem::ItemParentHasChanged, newParentVariant);
 }
@@ -2526,7 +2523,6 @@ void QGraphicsItemPrivate::setPosHelper(const QPointF &pos)
     if (scene)
         q->prepareGeometryChange();
     this->pos = newPos;
-    invalidateSceneTransformCache();
 
     // Send post-notification.
     q->itemChange(QGraphicsItem::ItemPositionHasChanged, newPosVariant);
@@ -2667,44 +2663,15 @@ QMatrix QGraphicsItem::sceneMatrix() const
 */
 QTransform QGraphicsItem::sceneTransform() const
 {
-    // Check if there's any entry in the transform cache.
-    QGraphicsScenePrivate *sd = d_ptr->scene ? d_ptr->scene->d_func() : 0;
-    int index = d_ptr->sceneTransformIndex;
-    if (sd && index != -1 && sd->validTransforms.testBit(index))
-        return sd->sceneTransformCache[index];
-
-    // Calculate local transform.
     QTransform m;
-    if (d_ptr->hasTransform) {
-        m = transform();
-        if (!d_ptr->pos.isNull())
-            m *= QTransform::fromTranslate(d_ptr->pos.x(), d_ptr->pos.y());
-    } else if (!d_ptr->pos.isNull()) {
-        m = QTransform::fromTranslate(d_ptr->pos.x(), d_ptr->pos.y());
-    }
-
-    // Combine with parent and add to cache.
-    if (d_ptr->parent) {
-        m *= d_ptr->parent->sceneTransform();
-        // Don't cache toplevels
-        if (sd) {
-            if (index == -1) {
-                if (!sd->freeSceneTransformSlots.isEmpty()) {
-                    index = sd->freeSceneTransformSlots.last();
-                    sd->freeSceneTransformSlots.pop_back();
-                } else {
-                    index = sd->sceneTransformCache.size();
-                }
-                d_ptr->sceneTransformIndex = index;
-                if (index >= sd->validTransforms.size()) {
-                    sd->validTransforms.resize(index + 1);
-                    sd->sceneTransformCache.resize(index + 1);
-                }
-            }
-            sd->validTransforms.setBit(index, 1);
-            sd->sceneTransformCache[index] = m;
-        }
-    }
+    const QGraphicsItem *p = this;
+    do {
+        if (p->d_ptr->hasTransform)
+            m *= p->transform();
+        const QPointF &pos = p->d_ptr->pos;
+        if (!pos.isNull())
+            m *= QTransform::fromTranslate(pos.x(), pos.y());
+    } while ((p = p->d_ptr->parent));
     return m;
 }
 
@@ -2933,7 +2900,6 @@ void QGraphicsItem::setMatrix(const QMatrix &matrix, bool combine)
         d_ptr->transform = new QTransform(newTransform);
     else
         *d_ptr->transform = newTransform;
-    d_ptr->invalidateSceneTransformCache();
 
     // Send post-notification.
     // NB! We have to change the value from QMatrix to QTransform.
@@ -2982,7 +2948,6 @@ void QGraphicsItem::setTransform(const QTransform &matrix, bool combine)
         d_ptr->transform = new QTransform(newTransform);
     else
         *d_ptr->transform = newTransform;
-    d_ptr->invalidateSceneTransformCache();
 
     // Send post-notification.
     itemChange(ItemTransformHasChanged, newTransformVariant);
@@ -3912,19 +3877,6 @@ void QGraphicsItemPrivate::resolveDepth(int parentDepth)
 /*!
     \internal
 */
-void QGraphicsItemPrivate::invalidateSceneTransformCache()
-{
-    if (!scene || (parent && sceneTransformIndex == -1))
-        return;
-    if (sceneTransformIndex != -1)
-        scene->d_func()->validTransforms.setBit(sceneTransformIndex, 0);
-    for (int i = 0; i < children.size(); ++i)
-        children.at(i)->d_ptr->invalidateSceneTransformCache();
-}
-
-/*!
-    \internal
-*/
 void QGraphicsItemPrivate::addChild(QGraphicsItem *child)
 {
     child->d_ptr->siblingIndex = children.size();
diff --git a/src/gui/graphicsview/qgraphicsitem_p.h b/src/gui/graphicsview/qgraphicsitem_p.h
index 5c7e67c..5a6401a 100644
--- a/src/gui/graphicsview/qgraphicsitem_p.h
+++ b/src/gui/graphicsview/qgraphicsitem_p.h
@@ -155,7 +155,6 @@ public:
         inDirtyList(0),
         paintedViewBoundingRectsNeedRepaint(0),
         globalStackingOrder(-1),
-        sceneTransformIndex(-1),
         q_ptr(0)
     {
     }
@@ -182,7 +181,6 @@ public:
     void updateEffectiveOpacity();
     void resolveEffectiveOpacity(qreal effectiveParentOpacity);
     void resolveDepth(int parentDepth);
-    void invalidateSceneTransformCache();
     void addChild(QGraphicsItem *child);
     void removeChild(QGraphicsItem *child);
     void setParentItemHelper(QGraphicsItem *parent, bool deleting);
@@ -365,7 +363,6 @@ public:
 
     // Optional stacking order
     int globalStackingOrder;
-    int sceneTransformIndex;
 
     struct DecomposedTransform;
     DecomposedTransform *decomposedTransform() const
diff --git a/src/gui/graphicsview/qgraphicsscene.cpp b/src/gui/graphicsview/qgraphicsscene.cpp
index d004ed2..86e7cdb 100644
--- a/src/gui/graphicsview/qgraphicsscene.cpp
+++ b/src/gui/graphicsview/qgraphicsscene.cpp
@@ -823,13 +823,6 @@ void QGraphicsScenePrivate::_q_removeItemLater(QGraphicsItem *item)
             ++iterator;
     }
 
-    // Remove from scene transform cache
-    int transformIndex = item->d_func()->sceneTransformIndex;
-    if (transformIndex != -1) {
-        validTransforms.setBit(transformIndex, 0);
-        freeSceneTransformSlots.append(transformIndex);
-    }
-
     // Reset the mouse grabber
     if (mouseGrabberItems.contains(item))
         ungrabMouse(item, /* item is dying */ true);
@@ -3340,14 +3333,6 @@ void QGraphicsScene::removeItem(QGraphicsItem *item)
         d->unindexedItems.removeAll(item);
     }
 
-    // Remove from scene transform cache
-    int transformIndex = item->d_func()->sceneTransformIndex;
-    if (transformIndex != -1) {
-        d->validTransforms.setBit(transformIndex, 0);
-        d->freeSceneTransformSlots.append(transformIndex);
-        item->d_func()->sceneTransformIndex = -1;
-    }
-
     if (item == d->focusItem)
         d->focusItem = 0;
     if (item == d->lastFocusItem)
@@ -3817,6 +3802,8 @@ bool QGraphicsScene::event(QEvent *event)
         // items from inside event handlers, this list can quickly end up
         // having stale pointers in it. We need to clear it before dispatching
         // events that use it.
+        // ### this should only be cleared if we received a new mouse move event,
+        // which relies on us fixing the replay mechanism in QGraphicsView.
         d->cachedItemsUnderMouse.clear();
     default:
         break;
diff --git a/src/gui/graphicsview/qgraphicsscene_p.h b/src/gui/graphicsview/qgraphicsscene_p.h
index f2226bf..bf7ac0a 100644
--- a/src/gui/graphicsview/qgraphicsscene_p.h
+++ b/src/gui/graphicsview/qgraphicsscene_p.h
@@ -289,10 +289,6 @@ public:
     void setPalette_helper(const QPalette &palette);
     void resolvePalette();
     void updatePalette(const QPalette &palette);
-
-    mutable QVector<QTransform> sceneTransformCache;
-    mutable QBitArray validTransforms;
-    mutable QVector<int> freeSceneTransformSlots;
 };
 
 QT_END_NAMESPACE