Skip to content

Commit

Permalink
Fix inspection of graphics scenes with nested items
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
dfaure-kdab committed Apr 5, 2024
1 parent 08ad63c commit a6a55e1
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 8 deletions.
37 changes: 29 additions & 8 deletions plugins/sceneinspector/scenemodel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -106,16 +106,37 @@ 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())
return {};
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());
}

Expand All @@ -128,18 +149,18 @@ 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
{
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;
}

Expand Down
2 changes: 2 additions & 0 deletions plugins/sceneinspector/scenemodel.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down

0 comments on commit a6a55e1

Please sign in to comment.