QGraphicsSceneIndex: replace polymorphism with a function pointer

QGraphicsSceneIndexIntersector was used as a kind of polymorpic predicate in
items_helper(). But it had only one function and the instances of it, which
were kept around in QGraphicsSceneIndexPrivate, were re-initialised on every
call to items_helper(), which means that one can just as well pass that
miniscule amount of data to the function itself.

Replaced the polymorphism with a function pointer and the comparison state
(which is conceptually const if it wasn't for the requirement to assign
to it all the time) with a const void * argument.

Effects on Linux AMD64 GCC 4.9-trunk release stripped:
  text:   -1584B
  data:    -376B
  relocs:   -26

I've wrapped the functions in a namespace. The only reason for this is
to keep the functions, previously defined in class bodies, at the same
indentation level so as not to destroy the git-blame history for them.

Change-Id: I8e1f48030047a3c54e881de7c77a3325b3e1f509
Reviewed-by: Alexis Menard <alexis@webkit.org>
Reviewed-by: Andreas Aardal Hanssen <andreas@hanssen.name>

2 files changed, 36 insertions, 67 deletions

diff --git a/src/widgets/graphicsview/qgraphicssceneindex.cpp b/src/widgets/graphicsview/qgraphicssceneindex.cpp
index 398e72e4c3..40f63937f4 100644
--- a/src/widgets/graphicsview/qgraphicssceneindex.cpp
+++ b/src/widgets/graphicsview/qgraphicssceneindex.cpp
@@ -67,12 +67,13 @@
 
 QT_BEGIN_NAMESPACE
 
-class QGraphicsSceneIndexRectIntersector : public QGraphicsSceneIndexIntersector
-{
-public:
-    bool intersect(const QGraphicsItem *item, const QRectF &exposeRect, Qt::ItemSelectionMode mode,
-                   const QTransform &deviceTransform) const
+namespace QtPrivate { // just to keep indentation of the following functions at the same level
+
+    static bool intersect_rect(const QGraphicsItem *item, const QRectF &exposeRect, Qt::ItemSelectionMode mode,
+                               const QTransform &deviceTransform, const void *intersectData)
     {
+        const QRectF sceneRect = *static_cast<const QRectF *>(intersectData);
+
         QRectF brect = item->boundingRect();
         _q_adjustRect(&brect);
 
@@ -117,15 +118,11 @@ public:
         return keep;
     }
 
-    QRectF sceneRect;
-};
-
-class QGraphicsSceneIndexPointIntersector : public QGraphicsSceneIndexIntersector
-{
-public:
-    bool intersect(const QGraphicsItem *item, const QRectF &exposeRect, Qt::ItemSelectionMode mode,
-                   const QTransform &deviceTransform) const
+    static bool intersect_point(const QGraphicsItem *item, const QRectF &exposeRect, Qt::ItemSelectionMode mode,
+                                const QTransform &deviceTransform, const void *intersectData)
     {
+        const QPointF scenePoint = *static_cast<const QPointF *>(intersectData);
+
         QRectF brect = item->boundingRect();
         _q_adjustRect(&brect);
 
@@ -163,15 +160,11 @@ public:
         return keep;
     }
 
-    QPointF scenePoint;
-};
-
-class QGraphicsSceneIndexPathIntersector : public QGraphicsSceneIndexIntersector
-{
-public:
-    bool intersect(const QGraphicsItem *item, const QRectF &exposeRect, Qt::ItemSelectionMode mode,
-                   const QTransform &deviceTransform) const
+    static bool intersect_path(const QGraphicsItem *item, const QRectF &exposeRect, Qt::ItemSelectionMode mode,
+                               const QTransform &deviceTransform, const void *intersectData)
     {
+        const QPainterPath scenePath = *static_cast<const QPainterPath *>(intersectData);
+
         QRectF brect = item->boundingRect();
         _q_adjustRect(&brect);
 
@@ -211,17 +204,13 @@ public:
         return keep;
     }
 
-    QPainterPath scenePath;
-};
+} // namespace QtPrivate
 
 /*!
     Constructs a private scene index.
 */
 QGraphicsSceneIndexPrivate::QGraphicsSceneIndexPrivate(QGraphicsScene *scene) : scene(scene)
 {
-    pointIntersector = new QGraphicsSceneIndexPointIntersector;
-    rectIntersector = new QGraphicsSceneIndexRectIntersector;
-    pathIntersector =  new QGraphicsSceneIndexPathIntersector;
 }
 
 /*!
@@ -229,9 +218,6 @@ QGraphicsSceneIndexPrivate::QGraphicsSceneIndexPrivate(QGraphicsScene *scene) :
 */
 QGraphicsSceneIndexPrivate::~QGraphicsSceneIndexPrivate()
 {
-    delete pointIntersector;
-    delete rectIntersector;
-    delete pathIntersector;
 }
 
 /*!
@@ -268,11 +254,11 @@ bool QGraphicsSceneIndexPrivate::itemCollidesWithPath(const QGraphicsItem *item,
     This function returns the items in ascending order.
 */
 void QGraphicsSceneIndexPrivate::recursive_items_helper(QGraphicsItem *item, QRectF exposeRect,
-                                                        QGraphicsSceneIndexIntersector *intersector,
+                                                        QGraphicsSceneIndexIntersector intersect,
                                                         QList<QGraphicsItem *> *items,
                                                         const QTransform &viewTransform,
                                                         Qt::ItemSelectionMode mode,
-                                                        qreal parentOpacity) const
+                                                        qreal parentOpacity, const void *intersectData) const
 {
     Q_ASSERT(item);
     if (!item->d_ptr->visible)
@@ -295,7 +281,7 @@ void QGraphicsSceneIndexPrivate::recursive_items_helper(QGraphicsItem *item, QRe
     const bool itemClipsChildrenToShape = (item->d_ptr->flags & QGraphicsItem::ItemClipsChildrenToShape);
     bool processItem = !itemIsFullyTransparent;
     if (processItem) {
-        processItem = intersector->intersect(item, exposeRect, mode, viewTransform);
+        processItem = intersect(item, exposeRect, mode, viewTransform, intersectData);
         if (!processItem && (!itemHasChildren || itemClipsChildrenToShape)) {
             if (wasDirtyParentSceneTransform)
                 item->d_ptr->invalidateChildrenSceneTransform();
@@ -326,8 +312,8 @@ void QGraphicsSceneIndexPrivate::recursive_items_helper(QGraphicsItem *item, QRe
                 break;
             if (itemIsFullyTransparent && !(child->d_ptr->flags & QGraphicsItem::ItemIgnoresParentOpacity))
                 continue;
-            recursive_items_helper(child, exposeRect, intersector, items, viewTransform,
-                                   mode, opacity);
+            recursive_items_helper(child, exposeRect, intersect, items, viewTransform,
+                                   mode, opacity, intersectData);
         }
     }
 
@@ -343,8 +329,8 @@ void QGraphicsSceneIndexPrivate::recursive_items_helper(QGraphicsItem *item, QRe
                 child->d_ptr->dirtySceneTransform = 1;
             if (itemIsFullyTransparent && !(child->d_ptr->flags & QGraphicsItem::ItemIgnoresParentOpacity))
                 continue;
-            recursive_items_helper(child, exposeRect, intersector, items, viewTransform,
-                                   mode, opacity);
+            recursive_items_helper(child, exposeRect, intersect, items, viewTransform,
+                                   mode, opacity, intersectData);
         }
     }
 }
@@ -419,8 +405,7 @@ QList<QGraphicsItem *> QGraphicsSceneIndex::items(const QPointF &pos, Qt::ItemSe
 
     Q_D(const QGraphicsSceneIndex);
     QList<QGraphicsItem *> itemList;
-    d->pointIntersector->scenePoint = pos;
-    d->items_helper(QRectF(pos, QSizeF(1, 1)), d->pointIntersector, &itemList, deviceTransform, mode, order);
+    d->items_helper(QRectF(pos, QSizeF(1, 1)), &QtPrivate::intersect_point, &itemList, deviceTransform, mode, order, &pos);
     return itemList;
 }
 
@@ -453,8 +438,7 @@ QList<QGraphicsItem *> QGraphicsSceneIndex::items(const QRectF &rect, Qt::ItemSe
     QRectF exposeRect = rect;
     _q_adjustRect(&exposeRect);
     QList<QGraphicsItem *> itemList;
-    d->rectIntersector->sceneRect = rect;
-    d->items_helper(exposeRect, d->rectIntersector, &itemList, deviceTransform, mode, order);
+    d->items_helper(exposeRect, &QtPrivate::intersect_rect, &itemList, deviceTransform, mode, order, &rect);
     return itemList;
 }
 
@@ -489,8 +473,7 @@ QList<QGraphicsItem *> QGraphicsSceneIndex::items(const QPolygonF &polygon, Qt::
     _q_adjustRect(&exposeRect);
     QPainterPath path;
     path.addPolygon(polygon);
-    d->pathIntersector->scenePath = path;
-    d->items_helper(exposeRect, d->pathIntersector, &itemList, deviceTransform, mode, order);
+    d->items_helper(exposeRect, &QtPrivate::intersect_path, &itemList, deviceTransform, mode, order, &path);
     return itemList;
 }
 
@@ -523,8 +506,7 @@ QList<QGraphicsItem *> QGraphicsSceneIndex::items(const QPainterPath &path, Qt::
     QList<QGraphicsItem *> itemList;
     QRectF exposeRect = path.controlPointRect();
     _q_adjustRect(&exposeRect);
-    d->pathIntersector->scenePath = path;
-    d->items_helper(exposeRect, d->pathIntersector, &itemList, deviceTransform, mode, order);
+    d->items_helper(exposeRect, &QtPrivate::intersect_path, &itemList, deviceTransform, mode, order, &path);
     return itemList;
 }
 
diff --git a/src/widgets/graphicsview/qgraphicssceneindex_p.h b/src/widgets/graphicsview/qgraphicssceneindex_p.h
index 29b321fb1d..215fefe9b7 100644
--- a/src/widgets/graphicsview/qgraphicssceneindex_p.h
+++ b/src/widgets/graphicsview/qgraphicssceneindex_p.h
@@ -66,15 +66,14 @@ QT_BEGIN_NAMESPACE
 
 #if !defined(QT_NO_GRAPHICSVIEW)
 
-class QGraphicsSceneIndexIntersector;
-class QGraphicsSceneIndexPointIntersector;
-class QGraphicsSceneIndexRectIntersector;
-class QGraphicsSceneIndexPathIntersector;
 class QGraphicsSceneIndexPrivate;
 class QPointF;
 class QRectF;
 template<typename T> class QList;
 
+typedef bool (*QGraphicsSceneIndexIntersector)(const QGraphicsItem *item, const QRectF &exposeRect, Qt::ItemSelectionMode mode,
+                                               const QTransform &deviceTransform, const void *data);
+
 class Q_AUTOTEST_EXPORT QGraphicsSceneIndex : public QObject
 {
     Q_OBJECT
@@ -133,27 +132,24 @@ public:
     static bool itemCollidesWithPath(const QGraphicsItem *item, const QPainterPath &path, Qt::ItemSelectionMode mode);
 
     void recursive_items_helper(QGraphicsItem *item, QRectF exposeRect,
-                                QGraphicsSceneIndexIntersector *intersector, QList<QGraphicsItem *> *items,
+                                QGraphicsSceneIndexIntersector intersect, QList<QGraphicsItem *> *items,
                                 const QTransform &viewTransform,
-                                Qt::ItemSelectionMode mode, qreal parentOpacity = 1.0) const;
-    inline void items_helper(const QRectF &rect, QGraphicsSceneIndexIntersector *intersector,
+                                Qt::ItemSelectionMode mode, qreal parentOpacity, const void *intersectData) const;
+    inline void items_helper(const QRectF &rect, QGraphicsSceneIndexIntersector intersect,
                              QList<QGraphicsItem *> *items, const QTransform &viewTransform,
-                             Qt::ItemSelectionMode mode, Qt::SortOrder order) const;
+                             Qt::ItemSelectionMode mode, Qt::SortOrder order, const void *intersectData) const;
 
     QGraphicsScene *scene;
-    QGraphicsSceneIndexPointIntersector *pointIntersector;
-    QGraphicsSceneIndexRectIntersector *rectIntersector;
-    QGraphicsSceneIndexPathIntersector *pathIntersector;
 };
 
-inline void QGraphicsSceneIndexPrivate::items_helper(const QRectF &rect, QGraphicsSceneIndexIntersector *intersector,
+inline void QGraphicsSceneIndexPrivate::items_helper(const QRectF &rect, QGraphicsSceneIndexIntersector intersect,
                                                      QList<QGraphicsItem *> *items, const QTransform &viewTransform,
-                                                     Qt::ItemSelectionMode mode, Qt::SortOrder order) const
+                                                     Qt::ItemSelectionMode mode, Qt::SortOrder order, const void *intersectData) const
 {
     Q_Q(const QGraphicsSceneIndex);
     const QList<QGraphicsItem *> tli = q->estimateTopLevelItems(rect, Qt::AscendingOrder);
     for (int i = 0; i < tli.size(); ++i)
-        recursive_items_helper(tli.at(i), rect, intersector, items, viewTransform, mode);
+        recursive_items_helper(tli.at(i), rect, intersect, items, viewTransform, mode, 1.0, intersectData);
     if (order == Qt::DescendingOrder) {
         const int n = items->size();
         for (int i = 0; i < n / 2; ++i)
@@ -161,15 +157,6 @@ inline void QGraphicsSceneIndexPrivate::items_helper(const QRectF &rect, QGraphi
     }
 }
 
-class QGraphicsSceneIndexIntersector
-{
-public:
-    QGraphicsSceneIndexIntersector() { }
-    virtual ~QGraphicsSceneIndexIntersector() { }
-    virtual bool intersect(const QGraphicsItem *item, const QRectF &exposeRect, Qt::ItemSelectionMode mode,
-                           const QTransform &deviceTransform) const = 0;
-};
-
 #endif // QT_NO_GRAPHICSVIEW
 
 QT_END_NAMESPACE