diff options
author | Andreas Aardal Hanssen <andreas.aardal.hanssen@nokia.com> | 2009-08-13 10:20:52 (GMT) |
---|---|---|
committer | Andreas Aardal Hanssen <andreas.aardal.hanssen@nokia.com> | 2009-08-13 13:42:01 (GMT) |
commit | ff01481900f1d19d392c8ed8fe0f3b5c85751b8e (patch) | |
tree | 7746aaea3d97643637096049ee5dcc12412b6657 /src/gui | |
parent | 547af9bf8a2ec074a698f0b92ec9e2a1a935eb14 (diff) | |
download | Qt-ff01481900f1d19d392c8ed8fe0f3b5c85751b8e.zip Qt-ff01481900f1d19d392c8ed8fe0f3b5c85751b8e.tar.gz Qt-ff01481900f1d19d392c8ed8fe0f3b5c85751b8e.tar.bz2 |
Fix focus proxy deletion bugs/crashes in QGraphicsItem.
This change would have been much simpler if either QGraphicsItem
inherited QObject, or if we had some similar QPointer-like class that
supported QGraphicsItem. The issue is this: Each item can delegate
another item to be its focus proxy. That item can be a parent or child,
or something completely unrelated. Either of the two items can be
deleted independently.
The former solution was to store backpointers in a map in the scene.
Problem is, the items may not be in a scene when this happens, they
may be removed from the scene, and the items may be moved between
two scenes.
The bad part about this fix is that it adds another pointer to
QGraphicsItemPrivate.
Reviewed-by: Shane Kearns <shane.kearns@sosco.com>
Diffstat (limited to 'src/gui')
-rw-r--r-- | src/gui/graphicsview/qgraphicsitem.cpp | 29 | ||||
-rw-r--r-- | src/gui/graphicsview/qgraphicsitem_p.h | 2 | ||||
-rw-r--r-- | src/gui/graphicsview/qgraphicsscene.cpp | 6 | ||||
-rw-r--r-- | src/gui/graphicsview/qgraphicsscene_p.h | 1 |
4 files changed, 24 insertions, 14 deletions
diff --git a/src/gui/graphicsview/qgraphicsitem.cpp b/src/gui/graphicsview/qgraphicsitem.cpp index c3e5501..537dab7 100644 --- a/src/gui/graphicsview/qgraphicsitem.cpp +++ b/src/gui/graphicsview/qgraphicsitem.cpp @@ -1196,10 +1196,12 @@ QGraphicsItem::~QGraphicsItem() Q_ASSERT(d_ptr->children.isEmpty()); } - if (d_ptr->scene) + if (d_ptr->scene) { d_ptr->scene->d_func()->removeItemHelper(this); - else + } else { + d_ptr->resetFocusProxy(); d_ptr->setParentItemHelper(0); + } if (d_ptr->transformData) { for(int i = 0; i < d_ptr->transformData->graphicsTransforms.size(); ++i) { @@ -2613,13 +2615,11 @@ void QGraphicsItem::setFocusProxy(QGraphicsItem *item) } QGraphicsItem *lastFocusProxy = d_ptr->focusProxy; + if (lastFocusProxy) + lastFocusProxy->d_ptr->focusProxyRefs.removeOne(&d_ptr->focusProxy); d_ptr->focusProxy = item; - if (d_ptr->scene) { - if (lastFocusProxy) - d_ptr->scene->d_func()->focusProxyReverseMap.remove(lastFocusProxy, this); - if (item) - d_ptr->scene->d_func()->focusProxyReverseMap.insert(item, this); - } + if (item) + item->d_ptr->focusProxyRefs << &d_ptr->focusProxy; } /*! @@ -4626,6 +4626,19 @@ void QGraphicsItemPrivate::clearSubFocus() /*! \internal + Sets the focusProxy pointer to 0 for all items that have this item as their + focusProxy. ### Qt 5: Use QPointer instead. +*/ +void QGraphicsItemPrivate::resetFocusProxy() +{ + for (int i = 0; i < focusProxyRefs.size(); ++i) + *focusProxyRefs.at(i) = 0; + focusProxyRefs.clear(); +} + +/*! + \internal + Tells us if it is a proxy widget */ bool QGraphicsItemPrivate::isProxyWidget() const diff --git a/src/gui/graphicsview/qgraphicsitem_p.h b/src/gui/graphicsview/qgraphicsitem_p.h index 982cdfc..c654d4f 100644 --- a/src/gui/graphicsview/qgraphicsitem_p.h +++ b/src/gui/graphicsview/qgraphicsitem_p.h @@ -398,6 +398,7 @@ public: void setSubFocus(); void clearSubFocus(); + void resetFocusProxy(); inline QTransform transformToParent() const; inline void ensureSortedChildren(); @@ -419,6 +420,7 @@ public: int siblingIndex; int depth; QGraphicsItem *focusProxy; + QList<QGraphicsItem **> focusProxyRefs; QGraphicsItem *subFocusItem; Qt::InputMethodHints imHints; diff --git a/src/gui/graphicsview/qgraphicsscene.cpp b/src/gui/graphicsview/qgraphicsscene.cpp index bdd1ac6..2178850 100644 --- a/src/gui/graphicsview/qgraphicsscene.cpp +++ b/src/gui/graphicsview/qgraphicsscene.cpp @@ -494,11 +494,7 @@ void QGraphicsScenePrivate::removeItemHelper(QGraphicsItem *item) item->d_func()->scene = 0; // Unregister focus proxy. - QMultiHash<QGraphicsItem *, QGraphicsItem *>::iterator it = focusProxyReverseMap.find(item); - while (it != focusProxyReverseMap.end() && it.key() == item) { - it.value()->d_ptr->focusProxy = 0; - it = focusProxyReverseMap.erase(it); - } + item->d_ptr->resetFocusProxy(); // Remove from parent, or unregister from toplevels. if (QGraphicsItem *parentItem = item->parentItem()) { diff --git a/src/gui/graphicsview/qgraphicsscene_p.h b/src/gui/graphicsview/qgraphicsscene_p.h index 8b53306..836522d 100644 --- a/src/gui/graphicsview/qgraphicsscene_p.h +++ b/src/gui/graphicsview/qgraphicsscene_p.h @@ -131,7 +131,6 @@ public: QGraphicsWidget *activeWindow; int activationRefCount; void setFocusItemHelper(QGraphicsItem *item, Qt::FocusReason focusReason); - QMultiHash<QGraphicsItem *, QGraphicsItem *> focusProxyReverseMap; QList<QGraphicsWidget *> popupWidgets; void addPopup(QGraphicsWidget *widget); |