From a6a55e1aa9fb62e4d6ec4b2ba596f2c93eb00bbd Mon Sep 17 00:00:00 2001
From: David Faure <david.faure@kdab.com>
Date: Thu, 4 Apr 2024 23:19:46 +0200
Subject: [PATCH] Fix inspection of graphics scenes with nested items

The SceneModel had a broken parent() implementation,
it always returned row = 0 for toplevel items, instead of
looking up the position in the topLevelItems() list.

While at it, make this model more robust against the fact
that the order of QGraphicsItem::childItems() can change
due to stackBefore() or changing the Z order, so sort that
list (by pointer address) to make it stable.

There's still one way to break that model:
QGraphicsItem::setParentItem(). But that's for another day.
---
 plugins/sceneinspector/scenemodel.cpp | 37 +++++++++++++++++++++------
 plugins/sceneinspector/scenemodel.h   |  2 ++
 2 files changed, 31 insertions(+), 8 deletions(-)

diff --git a/plugins/sceneinspector/scenemodel.cpp b/plugins/sceneinspector/scenemodel.cpp
index c5670e235e..b3b8581ea1 100644
--- a/plugins/sceneinspector/scenemodel.cpp
+++ b/plugins/sceneinspector/scenemodel.cpp
@@ -106,6 +106,29 @@ int SceneModel::rowCount(const QModelIndex &parent) const
     return topLevelItems().size();
 }
 
+// QGraphicsItem::childItems() sorts them by stacking order, which takes into account the
+// items' insertion order, calls to QGraphicsItem::stackBefore(), and Z-values
+// That's too prone to change under our feet without telling us, breaking model invariants
+// So always just sort them... by pointer address, for lack of a better idea
+namespace
+{
+    QList<QGraphicsItem *> sortedChildItems(QGraphicsItem *parent)
+    {
+        auto items = parent->childItems();
+        std::sort(items.begin(), items.end());
+        return items;
+    }
+}
+
+int SceneModel::rowForItem(QGraphicsItem *item) const
+{
+    auto parent = item->parentItem();
+    if (parent)
+        return sortedChildItems(parent).indexOf(item);
+    else
+        return topLevelItems().indexOf(item);
+}
+
 QModelIndex SceneModel::parent(const QModelIndex &child) const
 {
     if (!child.isValid())
@@ -113,9 +136,7 @@ QModelIndex SceneModel::parent(const QModelIndex &child) const
     QGraphicsItem *item = static_cast<QGraphicsItem *>(child.internalPointer());
     if (!item->parentItem())
         return QModelIndex();
-    int row = 0;
-    if (item->parentItem()->parentItem())
-        row = item->parentItem()->parentItem()->childItems().indexOf(item->parentItem());
+    const int row = rowForItem(item->parentItem());
     return createIndex(row, 0, item->parentItem());
 }
 
@@ -128,7 +149,7 @@ QModelIndex SceneModel::index(int row, int column, const QModelIndex &parent) co
     QGraphicsItem *parentItem = static_cast<QGraphicsItem *>(parent.internalPointer());
     if (!parentItem || row < 0 || row >= parentItem->childItems().size())
         return QModelIndex();
-    return createIndex(row, column, parentItem->childItems().at(row));
+    return createIndex(row, column, sortedChildItems(parentItem).at(row));
 }
 
 QList<QGraphicsItem *> SceneModel::topLevelItems() const
@@ -136,10 +157,10 @@ QList<QGraphicsItem *> SceneModel::topLevelItems() const
     QList<QGraphicsItem *> topLevel;
     if (!m_scene)
         return topLevel;
-    Q_FOREACH (QGraphicsItem *item, m_scene->items()) {
-        if (!item->parentItem())
-            topLevel.push_back(item);
-    }
+    const auto allItems = m_scene->items();
+    const auto isTopLevel = [](QGraphicsItem *item) { return !item->parentItem(); };
+    std::copy_if(allItems.begin(), allItems.end(), std::back_inserter(topLevel), isTopLevel);
+    std::sort(topLevel.begin(), topLevel.end());
     return topLevel;
 }
 
diff --git a/plugins/sceneinspector/scenemodel.h b/plugins/sceneinspector/scenemodel.h
index c40bde2444..05ae3d952d 100644
--- a/plugins/sceneinspector/scenemodel.h
+++ b/plugins/sceneinspector/scenemodel.h
@@ -45,6 +45,8 @@ class SceneModel : public QAbstractItemModel
 
 private:
     QList<QGraphicsItem *> topLevelItems() const;
+    int rowForItem(QGraphicsItem *item) const;
+
     /// Returns a string type name for the given QGV item type id
     QString typeName(int itemType) const;