Refactor: use lambdas for visitor pattern in QGraphicsSceneBspTree

This way, the visitor implementations are directly where they are used,
without the need for a virtual function call. The allocation of the
visitors on the heap was anyway overkill (they could just have been
allocated on the stack where they were used).

Change-Id: Ic41fd285e3a45daaf2e17aa5aeee07e754127a00
Reviewed-by: Richard Moe Gustavsen <richard.gustavsen@qt.io>

src/widgets/graphicsview/qgraphicsscene_bsp.cpp

@@ -44,57 +44,14 @@
 
 QT_BEGIN_NAMESPACE
 
-class QGraphicsSceneInsertItemBspTreeVisitor : public QGraphicsSceneBspTreeVisitor
-{
-public:
-    QGraphicsItem *item;
-
-    void visit(QList<QGraphicsItem *> *items) override
-    { items->prepend(item); }
-};
-
-class QGraphicsSceneRemoveItemBspTreeVisitor : public QGraphicsSceneBspTreeVisitor
-{
-public:
-    QGraphicsItem *item;
-
-    void visit(QList<QGraphicsItem *> *items) override
-    { items->removeAll(item); }
-};
-
-class QGraphicsSceneFindItemBspTreeVisitor : public QGraphicsSceneBspTreeVisitor
-{
-public:
-    QList<QGraphicsItem *> *foundItems;
-    bool onlyTopLevelItems;
-
-    void visit(QList<QGraphicsItem *> *items) override
-    {
-        for (int i = 0; i < items->size(); ++i) {
-            QGraphicsItem *item = items->at(i);
-            if (onlyTopLevelItems && item->d_ptr->parent)
-                item = item->topLevelItem();
-            if (!item->d_func()->itemDiscovered && item->d_ptr->visible) {
-                item->d_func()->itemDiscovered = 1;
-                foundItems->prepend(item);
-            }
-        }
-    }
-};
 
 QGraphicsSceneBspTree::QGraphicsSceneBspTree()
     : leafCnt(0)
 {
-    insertVisitor = new QGraphicsSceneInsertItemBspTreeVisitor;
-    removeVisitor = new QGraphicsSceneRemoveItemBspTreeVisitor;
-    findVisitor = new QGraphicsSceneFindItemBspTreeVisitor;
 }
 
 QGraphicsSceneBspTree::~QGraphicsSceneBspTree()
 {
-    delete insertVisitor;
-    delete removeVisitor;
-    delete findVisitor;
 }
 
 void QGraphicsSceneBspTree::initialize(const QRectF &rect, int depth)
@@ -118,14 +75,16 @@ void QGraphicsSceneBspTree::clear()
 
 void QGraphicsSceneBspTree::insertItem(QGraphicsItem *item, const QRectF &rect)
 {
-    insertVisitor->item = item;
+    climbTree([item](QList<QGraphicsItem *> *items){
-    climbTree(insertVisitor, rect);
+        items->prepend(item);
+    }, rect);
 }
 
 void QGraphicsSceneBspTree::removeItem(QGraphicsItem *item, const QRectF &rect)
 {
-    removeVisitor->item = item;
+    climbTree([item](QList<QGraphicsItem *> *items){
-    climbTree(removeVisitor, rect);
+        items->removeAll(item);
+    }, rect);
 }
 
 void QGraphicsSceneBspTree::removeItems(const QSet<QGraphicsItem *> &items)
@@ -144,14 +103,22 @@ void QGraphicsSceneBspTree::removeItems(const QSet<QGraphicsItem *> &items)
 
 QList<QGraphicsItem *> QGraphicsSceneBspTree::items(const QRectF &rect, bool onlyTopLevelItems) const
 {
-    QList<QGraphicsItem *> tmp;
+    QList<QGraphicsItem *> foundItems;
-    findVisitor->foundItems = &tmp;
+    climbTree([&foundItems, onlyTopLevelItems](QList<QGraphicsItem *> *items) {
-    findVisitor->onlyTopLevelItems = onlyTopLevelItems;
+        for (int i = 0; i < items->size(); ++i) {
-    climbTree(findVisitor, rect);
+            QGraphicsItem *item = items->at(i);
+            if (onlyTopLevelItems && item->d_ptr->parent)
+                item = item->topLevelItem();
+            if (!item->d_func()->itemDiscovered && item->d_ptr->visible) {
+                item->d_func()->itemDiscovered = 1;
+                foundItems.prepend(item);
+            }
+        }
+    }, rect);
     // Reset discovery bits.
-    for (int i = 0; i < tmp.size(); ++i)
+    for (const auto &foundItem : qAsConst(foundItems))
-        tmp.at(i)->d_ptr->itemDiscovered = 0;
+        foundItem->d_ptr->itemDiscovered = 0;
-    return tmp;
+    return foundItems;
 }
 
 int QGraphicsSceneBspTree::leafCount() const
@@ -230,7 +197,8 @@ void QGraphicsSceneBspTree::initialize(const QRectF &rect, int depth, int index)
     }
 }
 
-void QGraphicsSceneBspTree::climbTree(QGraphicsSceneBspTreeVisitor *visitor, const QRectF &rect, int index) const
+template<typename Visitor>
+void QGraphicsSceneBspTree::climbTree(Visitor &&visitor, const QRectF &rect, int index) const
 {
     if (nodes.isEmpty())
         return;
@@ -240,7 +208,7 @@ void QGraphicsSceneBspTree::climbTree(QGraphicsSceneBspTreeVisitor *visitor, con
 
     switch (node.type) {
     case Node::Leaf: {
-        visitor->visit(const_cast<QList<QGraphicsItem*>*>(&leaves[node.leafIndex]));
+        visitor(const_cast<QList<QGraphicsItem*>*>(&leaves[node.leafIndex]));
         break;
     }
     case Node::Vertical:

src/widgets/graphicsview/qgraphicsscene_bsp_p.h

@@ -61,10 +61,6 @@ QT_REQUIRE_CONFIG(graphicsview);
 QT_BEGIN_NAMESPACE
 
 class QGraphicsItem;
-class QGraphicsSceneBspTreeVisitor;
-class QGraphicsSceneInsertItemBspTreeVisitor;
-class QGraphicsSceneRemoveItemBspTreeVisitor;
-class QGraphicsSceneFindItemBspTreeVisitor;
 
 class QGraphicsSceneBspTree
 {
@@ -102,24 +98,14 @@ public:
 
 private:
     void initialize(const QRectF &rect, int depth, int index);
-    void climbTree(QGraphicsSceneBspTreeVisitor *visitor, const QRectF &rect, int index = 0) const;
+    template<typename Visitor>
+    void climbTree(Visitor &&visitor, const QRectF &rect, int index = 0) const;
     QRectF rectForIndex(int index) const;
 
     QList<Node> nodes;
     QList<QList<QGraphicsItem *>> leaves;
     int leafCnt;
     QRectF rect;
-
-    QGraphicsSceneInsertItemBspTreeVisitor *insertVisitor;
-    QGraphicsSceneRemoveItemBspTreeVisitor *removeVisitor;
-    QGraphicsSceneFindItemBspTreeVisitor *findVisitor;
-};
-
-class QGraphicsSceneBspTreeVisitor
-{
-public:
-    virtual ~QGraphicsSceneBspTreeVisitor() { }
-    virtual void visit(QList<QGraphicsItem *> *items) = 0;
 };
 
 QT_END_NAMESPACE