Fixed move a QGraphicsWidget and invalidate its layout at the same time

QGraphicsWidget::setGeometry() could sometimes call
QGraphicsLayoutItem::setGeometry() with an uninitialized rectangle.

This happened in the specific case where the widget had a layout that
was not active, and where setPos was called on the widget.

This would in turn cause it to enter setGeometry() where it would act as
expected. However, due to the fact that we sent a LayoutRequest event
at the end of that function, it could result in that
QGraphicsLayout::activate() would call setGeometry again.
Now, we would actually enter setGeometry, where wd->inSetPos == 1.

Then, we would not enter the "if (!wd->inSetPos)" block nor the
"if (moved)" block. It would then end up calling
QGraphicsLayoutItem::setGeometry(newGeom), where newGeom was
uninitialized.

Bug happens only when
QGraphicsLayout::setInstantInvalidatePropagation(true) was used,
because that was the condition for sending the layout request from
setGeometry()

Tracked down and written by Stanislav Ionascu.

Reviewed-by: Jan-Arve Sæther

2 files changed, 74 insertions, 2 deletions

diff --git a/src/gui/graphicsview/qgraphicswidget.cpp b/src/gui/graphicsview/qgraphicswidget.cpp
index 804394a..7048fcc 100644
--- a/src/gui/graphicsview/qgraphicswidget.cpp
+++ b/src/gui/graphicsview/qgraphicswidget.cpp
@@ -347,11 +347,10 @@ void QGraphicsWidget::setGeometry(const QRectF &rect)
 {
     QGraphicsWidgetPrivate *wd = QGraphicsWidget::d_func();
     QGraphicsLayoutItemPrivate *d = QGraphicsLayoutItem::d_ptr.data();
-    QRectF newGeom;
+    QRectF newGeom = rect;
     QPointF oldPos = d->geom.topLeft();
     if (!wd->inSetPos) {
         setAttribute(Qt::WA_Resized);
-        newGeom = rect;
         newGeom.setSize(rect.size().expandedTo(effectiveSizeHint(Qt::MinimumSize))
                                    .boundedTo(effectiveSizeHint(Qt::MaximumSize)));
 
diff --git a/tests/auto/qgraphicslayout/tst_qgraphicslayout.cpp b/tests/auto/qgraphicslayout/tst_qgraphicslayout.cpp
index d5d56fc..9411f97 100644
--- a/tests/auto/qgraphicslayout/tst_qgraphicslayout.cpp
+++ b/tests/auto/qgraphicslayout/tst_qgraphicslayout.cpp
@@ -63,6 +63,8 @@ private slots:
     void automaticReparenting();
     void verifyActivate();
     void invalidate();
+    void invalidateAndMove_data();
+    void invalidateAndMove();
     void constructors();
     void alternativeLayoutItems();
     void ownership();
@@ -556,6 +558,77 @@ void tst_QGraphicsLayout::invalidate()
     QGraphicsLayout::setInstantInvalidatePropagation(false);
 }
 
+void tst_QGraphicsLayout::invalidateAndMove_data()
+{
+    QTest::addColumn<bool>("instantInvalidatePropagation");
+    QTest::newRow("Without instantInvalidatePropagation") << false;
+    QTest::newRow("With instantInvalidatePropagation") << true;
+
+}
+void tst_QGraphicsLayout::invalidateAndMove()
+{
+    // Check that if we set the position of an item and invalidate its layout at the same
+    // time, the widget keeps its correct size
+    QFETCH(bool, instantInvalidatePropagation);
+    QGraphicsLayout::setInstantInvalidatePropagation(instantInvalidatePropagation);
+    QGraphicsScene scene;
+
+    QGraphicsWidget *widget = new QGraphicsWidget;
+    new QGraphicsLinearLayout(widget);
+
+    widget->setMinimumSize(1,1);
+    widget->setPreferredSize(34,34);
+    widget->setMaximumSize(100,100);
+    widget->resize(widget->preferredSize());
+
+    scene.addItem(widget);
+
+    qApp->processEvents();
+
+    /* Invalidate and reactivate.  The size should not have changed */
+    widget->layout()->invalidate();
+    widget->layout()->activate();
+
+    QCOMPARE(widget->geometry().size(), widget->preferredSize());
+    QCOMPARE(widget->layout()->geometry().size(), widget->preferredSize());
+    qApp->processEvents();
+    QCOMPARE(widget->geometry().size(), widget->preferredSize());
+    QCOMPARE(widget->layout()->geometry().size(), widget->preferredSize());
+
+    widget->layout()->invalidate();
+    widget->setX(1);  //Change just the position using setX
+    QCOMPARE(widget->geometry().size(), widget->preferredSize());
+    QCOMPARE(widget->layout()->geometry().size(), widget->preferredSize());
+    qApp->processEvents();
+    QCOMPARE(widget->geometry().size(), widget->preferredSize());
+    QCOMPARE(widget->layout()->geometry().size(), widget->preferredSize());
+
+    widget->layout()->invalidate();
+    widget->setGeometry(1,1,34,34);  //Change just the position using setGeometry
+    QCOMPARE(widget->geometry().size(), widget->preferredSize());
+    QCOMPARE(widget->layout()->geometry().size(), widget->preferredSize());
+    qApp->processEvents();
+    QCOMPARE(widget->geometry().size(), widget->preferredSize());
+    QCOMPARE(widget->layout()->geometry().size(), widget->preferredSize());
+
+    widget->layout()->invalidate();
+    widget->setGeometry(1,1,60,60);  //Change just the size using setGeometry
+    QCOMPARE(widget->geometry().size(), QSizeF(60,60));
+    QCOMPARE(widget->layout()->geometry().size(), QSizeF(60,60));
+    qApp->processEvents();
+    QCOMPARE(widget->geometry().size(), QSizeF(60,60));
+    QCOMPARE(widget->layout()->geometry().size(), QSizeF(60,60));
+
+    widget->layout()->invalidate();
+    widget->setGeometry(0,0,34,34); //Change the size and position using setGeometry
+    QCOMPARE(widget->geometry().size(), widget->preferredSize());
+    QCOMPARE(widget->layout()->geometry().size(), widget->preferredSize());
+    qApp->processEvents();
+    QCOMPARE(widget->geometry().size(), widget->preferredSize());
+    QCOMPARE(widget->layout()->geometry().size(), widget->preferredSize());
+
+    QGraphicsLayout::setInstantInvalidatePropagation(false);
+}
 class Layout : public QGraphicsLayout
 {
 public: