From 50ddb3451007a94d87c064216603a3e3e6e8b9c6 Mon Sep 17 00:00:00 2001 From: Andreas Aardal Hanssen Date: Mon, 8 Jun 2009 18:58:57 +0200 Subject: Add ItemSendsGeometryChanges, replacing itemChangeEnabled(). This flag toggles whether we should send notifications for setPos, setMatrix, and setTransform. It's off by default. Docs have been updated. All autotests pass. This change also cleans up a bit so that we both have readable code, and keeping the optimized path for when we need to send the notifications. By enabling this flag by default we are going to trigger regressions in end-user code. Reviewed-by: bnilsen --- examples/graphicsview/elasticnodes/node.cpp | 1 + src/gui/graphicsview/qgraphicsitem.cpp | 161 ++++++++++++++----------- src/gui/graphicsview/qgraphicsitem.h | 3 +- src/gui/graphicsview/qgraphicsitem_p.h | 5 +- src/gui/graphicsview/qgraphicswidget.cpp | 3 + src/gui/graphicsview/qgraphicswidget_p.cpp | 2 + tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp | 58 ++++++++- 7 files changed, 158 insertions(+), 75 deletions(-) diff --git a/examples/graphicsview/elasticnodes/node.cpp b/examples/graphicsview/elasticnodes/node.cpp index 6942fa0..53fe994 100644 --- a/examples/graphicsview/elasticnodes/node.cpp +++ b/examples/graphicsview/elasticnodes/node.cpp @@ -52,6 +52,7 @@ Node::Node(GraphWidget *graphWidget) : graph(graphWidget) { setFlag(ItemIsMovable); + setFlag(ItemSendsGeometryChanges); setCacheMode(DeviceCoordinateCache); setZValue(1); } diff --git a/src/gui/graphicsview/qgraphicsitem.cpp b/src/gui/graphicsview/qgraphicsitem.cpp index 6679bd5..3138faa 100644 --- a/src/gui/graphicsview/qgraphicsitem.cpp +++ b/src/gui/graphicsview/qgraphicsitem.cpp @@ -39,8 +39,6 @@ ** ****************************************************************************/ -#define QGRAPHICSITEM_NO_ITEMCHANGE - /*! \class QGraphicsItem \brief The QGraphicsItem class is the base class for all graphical @@ -310,7 +308,14 @@ \value ItemHasNoContents The item does not paint anything (i.e., calling paint() on the item has no effect). You should set this flag on items that do not need to be painted to ensure that Graphics View avoids unnecessary - painting preparations. + painting preparations. This flag was introduced in Qt 4.6. + + \value ItemSendsGeometryChanges The item enables itemChange() + notifications for ItemPositionChange, ItemPositionHasChanged, + ItemMatrixChange, ItemTransformChange, and ItemTransformHasChanged. For + performance reasons, these notifications are disabled by default. You must + enable this flag to receive notifications for position and transform + changes. This flag was introduced in Qt 4.6. */ /*! @@ -349,33 +354,36 @@ changing. This value is obsolete; you can use ItemTransformChange instead. \value ItemPositionChange The item's position changes. This notification - is only sent when the item's local position changes, relative to its - parent, has changed (i.e., as a result of calling setPos() or - moveBy()). The value argument is the new position (i.e., a QPointF). You - can call pos() to get the original position. Do not call setPos() or - moveBy() in itemChange() as this notification is delivered; instead, you - can return the new, adjusted position from itemChange(). After this - notification, QGraphicsItem immediately sends the ItemPositionHasChanged - notification if the position changed. + is sent if the ItemSendsGeometryChanges flag is enabled, and when the + item's local position changes, relative to its parent (i.e., as a result + of calling setPos() or moveBy()). The value argument is the new position + (i.e., a QPointF). You can call pos() to get the original position. Do + not call setPos() or moveBy() in itemChange() as this notification is + delivered; instead, you can return the new, adjusted position from + itemChange(). After this notification, QGraphicsItem immediately sends the + ItemPositionHasChanged notification if the position changed. \value ItemPositionHasChanged The item's position has changed. This - notification is only sent after the item's local position, relative to its - parent, has changed. The value argument is the new position (the same as - pos()), and QGraphicsItem ignores the return value for this notification - (i.e., a read-only notification). + notification is sent if the ItemSendsGeometryChanges flag is enabled, and + after the item's local position, relative to its parent, has changed. The + value argument is the new position (the same as pos()), and QGraphicsItem + ignores the return value for this notification (i.e., a read-only + notification). \value ItemTransformChange The item's transformation matrix changes. This - notification is only sent when the item's local transformation matrix - changes (i.e., as a result of calling setTransform(). The value - argument is the new matrix (i.e., a QTransform); to get the old matrix, - call transform(). Do not call setTransform() or set any of the transformation - properties in itemChange() as this notification is delivered; - instead, you can return the new matrix from itemChange(). - This notification is not sent if you change the transformation properties. + notification is send if the ItemSendsGeometryChanges flag is enabled, and + when the item's local transformation matrix changes (i.e., as a result of + calling setTransform(). The value argument is the new matrix (i.e., a + QTransform); to get the old matrix, call transform(). Do not call + setTransform() or set any of the transformation properties in itemChange() + as this notification is delivered; instead, you can return the new matrix + from itemChange(). This notification is not sent if you change the + transformation properties. \value ItemTransformHasChanged The item's transformation matrix has - changed either because setTransform is called, or one of the transformation - properties is changed. This notification is only sent after the item's local + changed either because setTransform is called, or one of the + transformation properties is changed. This notification is sent if the + ItemSendsGeometryChanges flag is enabled, and after the item's local transformation matrix has changed. The value argument is the new matrix (same as transform()), and QGraphicsItem ignores the return value for this notification (i.e., a read-only notification). @@ -1369,7 +1377,9 @@ static void _q_qgraphicsItemSetFlag(QGraphicsItem *item, QGraphicsItem::Graphics item was selected, and \a flags does not enabled ItemIsSelectable, the item is automatically unselected. - By default, no flags are enabled. + By default, no flags are enabled. (QGraphicsWidget enables the + ItemSendsGeometryChanges flag by default in order to track position + changes.) \sa flags(), setFlag() */ @@ -2050,12 +2060,8 @@ qreal QGraphicsItem::effectiveOpacity() const void QGraphicsItem::setOpacity(qreal opacity) { // Notify change. -#ifndef QGRAPHICSITEM_NO_ITEMCHANGE const QVariant newOpacityVariant(itemChange(ItemOpacityChange, double(opacity))); qreal newOpacity = newOpacityVariant.toDouble(); -#else - qreal newOpacity = opacity; -#endif // Normalize. newOpacity = qBound(0.0, newOpacity, 1.0); @@ -2067,9 +2073,7 @@ void QGraphicsItem::setOpacity(qreal opacity) d_ptr->opacity = newOpacity; // Notify change. -#ifndef QGRAPHICSITEM_NO_ITEMCHANGE - itemChange(ItemOpacityHasChanged, newOpacity); -#endif + itemChange(ItemOpacityHasChanged, newOpacityVariant); // Update. if (d_ptr->scene) @@ -2508,42 +2512,33 @@ QPointF QGraphicsItem::scenePos() const /*! \internal - Sets the position \a pos and notifies the change. If \a update is true, - the item is also updated; otherwise it is not updated before and after the - change. + Sets the position \a pos. */ void QGraphicsItemPrivate::setPosHelper(const QPointF &pos) { Q_Q(QGraphicsItem); - if (this->pos == pos) - return; - - // Notify the item that the position is changing. -#ifndef QGRAPHICSITEM_NO_ITEMCHANGE - const QVariant newPosVariant(q->itemChange(QGraphicsItem::ItemPositionChange, pos)); - QPointF newPos = newPosVariant.toPointF(); - if (newPos == this->pos) - return; -#else - QPointF newPos = pos; -#endif - - // Update and repositition. inSetPosHelper = 1; - updateCachedClipPathFromSetPosHelper(newPos); + updateCachedClipPathFromSetPosHelper(pos); if (scene) q->prepareGeometryChange(); - this->pos = newPos; + this->pos = pos; dirtySceneTransform = 1; - - // Send post-notification. -#ifndef QGRAPHICSITEM_NO_ITEMCHANGE - q->itemChange(QGraphicsItem::ItemPositionHasChanged, newPosVariant); -#endif inSetPosHelper = 0; } /*! + \internal + + Sets the transform \a transform. +*/ +void QGraphicsItemPrivate::setTransformHelper(const QTransform &transform) +{ + q_ptr->prepareGeometryChange(); + transformData->transform = transform; + dirtySceneTransform = 1; +} + +/*! Sets the position of the item to \a pos, which is in parent coordinates. For items with no parent, \a pos is in scene coordinates. @@ -2555,7 +2550,26 @@ void QGraphicsItemPrivate::setPosHelper(const QPointF &pos) */ void QGraphicsItem::setPos(const QPointF &pos) { - d_ptr->setPosHelper(pos); + if (d_ptr->pos == pos) + return; + + // Update and repositition. + if (!(d_ptr->flags & ItemSendsGeometryChanges)) { + d_ptr->setPosHelper(pos); + return; + } + + // Notify the item that the position is changing. + const QVariant newPosVariant(itemChange(ItemPositionChange, qVariantFromValue(pos))); + QPointF newPos = newPosVariant.toPointF(); + if (newPos == d_ptr->pos) + return; + + // Update and repositition. + d_ptr->setPosHelper(newPos); + + // Send post-notification. + itemChange(QGraphicsItem::ItemPositionHasChanged, newPosVariant); } /*! @@ -3240,20 +3254,23 @@ void QGraphicsItem::setMatrix(const QMatrix &matrix, bool combine) if (d_ptr->transformData->transform == newTransform) return; + // Update and set the new transformation. + if (!(d_ptr->flags & ItemSendsGeometryChanges)) { + d_ptr->setTransformHelper(newTransform); + return; + } + // Notify the item that the transformation matrix is changing. - const QVariant newTransformVariant(itemChange(ItemTransformChange, - qVariantFromValue(newTransform))); - newTransform = qVariantValue(newTransformVariant); + const QVariant newMatrixVariant = qVariantFromValue(newTransform.toAffine()); + newTransform = QTransform(qVariantValue(itemChange(ItemMatrixChange, newMatrixVariant))); if (d_ptr->transformData->transform == newTransform) return; // Update and set the new transformation. - prepareGeometryChange(); - d_ptr->transformData->transform = newTransform; - d_ptr->dirtySceneTransform = 1; - + d_ptr->setTransformHelper(newTransform); + // Send post-notification. - itemChange(ItemTransformHasChanged, newTransformVariant); + itemChange(ItemTransformHasChanged, qVariantFromValue(newTransform)); } /*! @@ -3284,7 +3301,12 @@ void QGraphicsItem::setTransform(const QTransform &matrix, bool combine) if (d_ptr->transformData->transform == newTransform) return; - // Notify the item that the transformation matrix is changing. + // Update and set the new transformation. + if (!(d_ptr->flags & ItemSendsGeometryChanges)) { + d_ptr->setTransformHelper(newTransform); + return; + } + // Notify the item that the transformation matrix is changing. const QVariant newTransformVariant(itemChange(ItemTransformChange, qVariantFromValue(newTransform))); @@ -3293,9 +3315,7 @@ void QGraphicsItem::setTransform(const QTransform &matrix, bool combine) return; // Update and set the new transformation. - prepareGeometryChange(); - d_ptr->transformData->transform = newTransform; - d_ptr->dirtySceneTransform = 1; + d_ptr->setTransformHelper(newTransform); // Send post-notification. itemChange(ItemTransformHasChanged, newTransformVariant); @@ -9514,6 +9534,9 @@ QDebug operator<<(QDebug debug, QGraphicsItem::GraphicsItemFlag flag) case QGraphicsItem::ItemHasNoContents: str = "ItemHasNoContents"; break; + case QGraphicsItem::ItemSendsGeometryChanges: + str = "ItemSendsGeometryChanges"; + break; } debug << str; return debug; diff --git a/src/gui/graphicsview/qgraphicsitem.h b/src/gui/graphicsview/qgraphicsitem.h index 7e33c83..cff4f1f 100644 --- a/src/gui/graphicsview/qgraphicsitem.h +++ b/src/gui/graphicsview/qgraphicsitem.h @@ -96,7 +96,8 @@ public: ItemDoesntPropagateOpacityToChildren = 0x80, ItemStacksBehindParent = 0x100, ItemUsesExtendedStyleOption = 0x200, - ItemHasNoContents = 0x400 + ItemHasNoContents = 0x400, + ItemSendsGeometryChanges = 0x800 // NB! Don't forget to increase the d_ptr->flags bit field by 1 when adding a new flag. }; Q_DECLARE_FLAGS(GraphicsItemFlags, GraphicsItemFlag) diff --git a/src/gui/graphicsview/qgraphicsitem_p.h b/src/gui/graphicsview/qgraphicsitem_p.h index 13fee8f..e2a37a1 100644 --- a/src/gui/graphicsview/qgraphicsitem_p.h +++ b/src/gui/graphicsview/qgraphicsitem_p.h @@ -175,6 +175,7 @@ public: static bool movableAncestorIsSelected(const QGraphicsItem *item); void setPosHelper(const QPointF &pos); + void setTransformHelper(const QTransform &transform); void setVisibleHelper(bool newVisible, bool explicitly, bool update = true); void setEnabledHelper(bool newEnabled, bool explicitly, bool update = true); bool discardUpdateRequest(bool ignoreClipping = false, bool ignoreVisibleBit = false, @@ -397,12 +398,12 @@ public: quint32 fullUpdatePending : 1; // New 32 bits - quint32 flags : 11; + quint32 flags : 12; quint32 dirtyChildrenBoundingRect : 1; quint32 paintedViewBoundingRectsNeedRepaint : 1; quint32 dirtySceneTransform : 1; quint32 geometryChanged : 1; - quint32 unused : 17; // feel free to use + quint32 unused : 16; // feel free to use // Optional stacking order int globalStackingOrder; diff --git a/src/gui/graphicsview/qgraphicswidget.cpp b/src/gui/graphicsview/qgraphicswidget.cpp index 3296aee..7314167 100644 --- a/src/gui/graphicsview/qgraphicswidget.cpp +++ b/src/gui/graphicsview/qgraphicswidget.cpp @@ -1057,6 +1057,9 @@ void QGraphicsWidget::updateGeometry() ItemParentChange both to deliver \l ParentChange events, and for managing the focus chain. + QGraphicsWidget enables the ItemSendsGeometryChanges flag by default in + order to track position changes. + \sa propertyChange() */ QVariant QGraphicsWidget::itemChange(GraphicsItemChange change, const QVariant &value) diff --git a/src/gui/graphicsview/qgraphicswidget_p.cpp b/src/gui/graphicsview/qgraphicswidget_p.cpp index a435758..557b883 100644 --- a/src/gui/graphicsview/qgraphicswidget_p.cpp +++ b/src/gui/graphicsview/qgraphicswidget_p.cpp @@ -77,7 +77,9 @@ void QGraphicsWidgetPrivate::init(QGraphicsItem *parentItem, Qt::WindowFlags wFl resolveLayoutDirection(); q->unsetWindowFrameMargins(); q->setFlag(QGraphicsItem::ItemUsesExtendedStyleOption); + q->setFlag(QGraphicsItem::ItemSendsGeometryChanges); } + qreal QGraphicsWidgetPrivate::titleBarHeight(const QStyleOptionTitleBar &options) const { Q_Q(const QGraphicsWidget); diff --git a/tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp b/tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp index c6f9824..d6ecbba 100644 --- a/tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp +++ b/tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp @@ -224,6 +224,7 @@ private slots: void setTransformProperties_data(); void setTransformProperties(); void itemUsesExtendedStyleOption(); + void itemSendsGeometryChanges(); void moveItem(); // task specific tests below me @@ -3730,8 +3731,20 @@ void tst_QGraphicsItem::defaultItemTest_QGraphicsEllipseItem() class ItemChangeTester : public QGraphicsRectItem { public: - ItemChangeTester(){} - ItemChangeTester(QGraphicsItem *parent) : QGraphicsRectItem(parent) {} + ItemChangeTester() + { setFlag(ItemSendsGeometryChanges); clear(); } + ItemChangeTester(QGraphicsItem *parent) : QGraphicsRectItem(parent) + { setFlag(ItemSendsGeometryChanges); clear(); } + + void clear() + { + itemChangeReturnValue = QVariant(); + itemSceneChangeTargetScene = 0; + changes.clear(); + values.clear(); + oldValues.clear(); + } + QVariant itemChangeReturnValue; QGraphicsScene *itemSceneChangeTargetScene; @@ -3932,7 +3945,8 @@ void tst_QGraphicsItem::itemChange() QCOMPARE(tester.changes.size(), changeCount); QCOMPARE(tester.changes.at(tester.changes.size() - 2), QGraphicsItem::ItemFlagsChange); QCOMPARE(tester.changes.at(tester.changes.size() - 1), QGraphicsItem::ItemFlagsHaveChanged); - QCOMPARE(tester.values.at(tester.values.size() - 2), qVariantFromValue(QGraphicsItem::ItemIsSelectable)); + QVariant expectedFlags = qVariantFromValue(QGraphicsItem::ItemIsSelectable | QGraphicsItem::ItemSendsGeometryChanges); + QCOMPARE(tester.values.at(tester.values.size() - 2), expectedFlags); QCOMPARE(tester.values.at(tester.values.size() - 1), qVariantFromValue(QGraphicsItem::ItemIsSelectable)); } { @@ -6745,6 +6759,44 @@ void tst_QGraphicsItem::itemUsesExtendedStyleOption() QTest::qWait(125); } +void tst_QGraphicsItem::itemSendsGeometryChanges() +{ + ItemChangeTester item; + item.setFlags(0); + item.clear(); + + QTransform x = QTransform().rotate(45); + QPointF pos(10, 10); + qreal o(0.5); + item.setTransform(x); + item.setPos(pos); + QCOMPARE(item.transform(), x); + QCOMPARE(item.pos(), pos); + QCOMPARE(item.changes.size(), 0); + + item.setOpacity(o); + QCOMPARE(item.changes.size(), 2); // opacity + + item.setFlag(QGraphicsItem::ItemSendsGeometryChanges); + QCOMPARE(item.changes.size(), 4); // flags + item.setTransform(QTransform()); + item.setPos(QPointF()); + QCOMPARE(item.changes.size(), 8); // transform + pos + QCOMPARE(item.transform(), QTransform()); + QCOMPARE(item.pos(), QPointF()); + QCOMPARE(item.opacity(), o); + + QCOMPARE(item.changes, QList() + << QGraphicsItem::ItemOpacityChange + << QGraphicsItem::ItemOpacityHasChanged + << QGraphicsItem::ItemFlagsChange + << QGraphicsItem::ItemFlagsHaveChanged + << QGraphicsItem::ItemTransformChange + << QGraphicsItem::ItemTransformHasChanged + << QGraphicsItem::ItemPositionChange + << QGraphicsItem::ItemPositionHasChanged); +} + // Make sure we update moved items correctly. void tst_QGraphicsItem::moveItem() { -- cgit v0.12