Make sure we paint overlapped children and siblings when moving far

In paintAndFlush, QWidgetRepaintManager subtracts opaque children if
the target isn't overlapped and isMoved is set to true. So in moveRect,
set isMoved to true after the blitting of movable areas, and reset it to
false if we have overlapped sibling or child regions. Otherwise, moving
so far that sourceRect is invalid (none of the original pixels are
visible after the move) we end up in a code path that sets isMoved to
true even with overlapping children or siblings, which then breaks
paintAndFlush's assumptions.

Reuse the test case written by Sergiy Korobov <tiamatenko@gmail.com> in
earlier attempts to fix this bug.

Fixes: QTBUG-26269
Pick-to: 6.2
Change-Id: If7443863f5eee79a80220cd587522122f42a21e4
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>

2 files changed, 131 insertions, 12 deletions

diff --git a/src/widgets/kernel/qwidgetrepaintmanager.cpp b/src/widgets/kernel/qwidgetrepaintmanager.cpp
index 977a746c5a..d9375c7281 100644
--- a/src/widgets/kernel/qwidgetrepaintmanager.cpp
+++ b/src/widgets/kernel/qwidgetrepaintmanager.cpp
@@ -449,12 +449,8 @@ void QWidgetPrivate::moveRect(const QRect &rect, int dx, int dy)
     QWidget *pw = q->parentWidget();
     QPoint toplevelOffset = pw->mapTo(tlw, QPoint());
     QWidgetPrivate *pd = pw->d_func();
-    QRect clipR(pd->clipRect());
+    const QRect clipR(pd->clipRect());
     const QRect newRect(rect.translated(dx, dy));
-    QRect destRect = rect.intersected(clipR);
-    if (destRect.isValid())
-        destRect = destRect.translated(dx, dy).intersected(clipR);
-    const QRect sourceRect(destRect.translated(-dx, -dy));
     const QRect parentRect(rect & clipR);
     const bool nativeWithTextureChild = textureChildSeen && hasPlatformWindow(q);
 
@@ -476,9 +472,13 @@ void QWidgetPrivate::moveRect(const QRect &rect, int dx, int dy)
         pd->invalidateBackingStore(parentR);
         invalidateBackingStore((newRect & clipR).translated(-data.crect.topLeft()));
     } else {
+        QRect destRect = rect.intersected(clipR);
+        if (destRect.isValid())
+            destRect = destRect.translated(dx, dy).intersected(clipR);
+        const QRect sourceRect(destRect.translated(-dx, -dy));
 
         QWidgetRepaintManager *repaintManager = x->repaintManager.get();
-        QRegion childExpose(newRect & clipR);
+        QRegion childExpose = QRegion(newRect) & clipR;
         QRegion overlappedExpose;
 
         if (sourceRect.isValid()) {
@@ -493,6 +493,7 @@ void QWidgetPrivate::moveRect(const QRect &rect, int dx, int dy)
                         childExpose -= r.translated(dx, dy);
                     }
                 }
+                isMoved = true;
             }
 
             childExpose -= overlappedExpose;
@@ -503,14 +504,17 @@ void QWidgetPrivate::moveRect(const QRect &rect, int dx, int dy)
 
         const bool childUpdatesEnabled = q->updatesEnabled();
         if (childUpdatesEnabled) {
+            // As per paintAndFlush, reset isMoved if we have overlapping
+            // or child regions that need to be painted.
             if (!overlappedExpose.isEmpty()) {
                 overlappedExpose.translate(-data.crect.topLeft());
                 invalidateBackingStore(overlappedExpose);
+                isMoved = false;
             }
             if (!childExpose.isEmpty()) {
                 childExpose.translate(-data.crect.topLeft());
                 repaintManager->markDirty(childExpose, q);
-                isMoved = true;
+                isMoved = false;
             }
         }
 
@@ -519,12 +523,10 @@ void QWidgetPrivate::moveRect(const QRect &rect, int dx, int dy)
         if (extra && extra->hasMask)
             parentExpose += QRegion(newRect) - extra->mask.translated(data.crect.topLeft());
 
-        if (!parentExpose.isEmpty()) {
+        if (!parentExpose.isEmpty())
             repaintManager->markDirty(parentExpose, pw);
-            pd->isMoved = true;
-        }
 
-        if (childUpdatesEnabled) {
+        if (childUpdatesEnabled && sourceRect.isValid()) {
             QRegion needsFlush(sourceRect);
             needsFlush += destRect;
             repaintManager->markNeedsFlush(pw, needsFlush, toplevelOffset);
diff --git a/tests/auto/widgets/kernel/qwidgetrepaintmanager/tst_qwidgetrepaintmanager.cpp b/tests/auto/widgets/kernel/qwidgetrepaintmanager/tst_qwidgetrepaintmanager.cpp
index 52b179fedb..490c2a7d8c 100644
--- a/tests/auto/widgets/kernel/qwidgetrepaintmanager/tst_qwidgetrepaintmanager.cpp
+++ b/tests/auto/widgets/kernel/qwidgetrepaintmanager/tst_qwidgetrepaintmanager.cpp
@@ -29,7 +29,8 @@
 
 #include <QTest>
 #include <QPainter>
-#include <QWidget>
+#include <QScrollArea>
+#include <QScrollBar>
 #include <QApplication>
 
 #include <private/qhighdpiscaling_p.h>
@@ -94,6 +95,7 @@ private slots:
     void opaqueChildren();
     void staticContents();
     void scroll();
+    void moveWithOverlap();
 
 private:
     const int m_fuzz;
@@ -246,5 +248,120 @@ void tst_QWidgetRepaintManager::scroll()
     QCOMPARE(widget.takePaintedRegions(), QRegion());
 }
 
+
+/*!
+    Verify that overlapping children are repainted correctly when
+    a widget is moved (via a scroll area) for such a distance that
+    none of the old area is still visible. QTBUG-26269
+*/
+void tst_QWidgetRepaintManager::moveWithOverlap()
+{
+    if (QStringList{"android"}.contains(QGuiApplication::platformName()))
+        QSKIP("This test fails on Android");
+
+    class MainWindow : public QWidget
+    {
+    public:
+        MainWindow(QWidget *parent = 0)
+            : QWidget(parent, Qt::WindowStaysOnTopHint)
+        {
+            m_scrollArea = new QScrollArea(this);
+            QWidget *w = new QWidget;
+            w->setPalette(QPalette(Qt::gray));
+            w->setAutoFillBackground(true);
+            m_scrollArea->setWidget(w);
+            m_scrollArea->resize(500, 100);
+            w->resize(5000, 600);
+
+            m_topWidget = new QWidget(this);
+            m_topWidget->setPalette(QPalette(Qt::red));
+            m_topWidget->setAutoFillBackground(true);
+            m_topWidget->resize(300, 200);
+        }
+
+        void resizeEvent(QResizeEvent *e) override
+        {
+            QWidget::resizeEvent(e);
+            // move scroll area and top widget to the center of the main window
+            scrollArea()->move((width() - scrollArea()->width()) / 2, (height() - scrollArea()->height()) / 2);
+            topWidget()->move((width() - topWidget()->width()) / 2, (height() - topWidget()->height()) / 2);
+        }
+
+
+        inline QScrollArea *scrollArea() const { return m_scrollArea; }
+        inline QWidget *topWidget() const { return m_topWidget; }
+
+        bool grabWidgetBackground(QWidget *w)
+        {
+            // To check widget's background we should compare two screenshots:
+            // the first one is taken by system tools through QScreen::grabWindow(),
+            // the second one is taken by Qt rendering to a pixmap via QWidget::grab().
+
+            QScreen *screen = w->screen();
+            const QRect screenGeometry = screen->geometry();
+            QPoint globalPos = w->mapToGlobal(QPoint(0, 0));
+            if (globalPos.x() >= screenGeometry.width())
+                globalPos.rx() -= screenGeometry.x();
+            if (globalPos.y() >= screenGeometry.height())
+                globalPos.ry() -= screenGeometry.y();
+
+            return QTest::qWaitFor([&]{
+                QImage systemScreenshot = screen->grabWindow(winId(),
+                                                             globalPos.x(), globalPos.y(),
+                                                             w->width(), w->height()).toImage();
+                systemScreenshot = systemScreenshot.convertToFormat(QImage::Format_RGB32);
+                QImage qtScreenshot = w->grab().toImage().convertToFormat(systemScreenshot.format());
+                return systemScreenshot == qtScreenshot;
+            });
+        };
+
+    private:
+        QScrollArea *m_scrollArea;
+        QWidget *m_topWidget;
+    };
+
+    MainWindow w;
+    w.showFullScreen();
+
+    QVERIFY(QTest::qWaitForWindowActive(&w));
+
+    bool result = w.grabWidgetBackground(w.topWidget());
+    // if this fails already, then the system we test on can't compare screenshots from grabbed widgets,
+    // and we have to skip this test. Possible reasons are that showing the window took too long, differences
+    // in surface formats, or unrelated bugs in QScreen::grabWindow.
+    if (!result)
+        QSKIP("Cannot compare QWidget::grab with QScreen::grabWindow on this machine");
+
+    // scroll the horizontal slider to the right side
+    {
+        w.scrollArea()->horizontalScrollBar()->setValue(w.scrollArea()->horizontalScrollBar()->maximum());
+        QVERIFY(w.grabWidgetBackground(w.topWidget()));
+    }
+
+    // scroll the vertical slider down
+    {
+        w.scrollArea()->verticalScrollBar()->setValue(w.scrollArea()->verticalScrollBar()->maximum());
+        QVERIFY(w.grabWidgetBackground(w.topWidget()));
+    }
+
+    // hide the top widget
+    {
+        w.topWidget()->hide();
+        QVERIFY(w.grabWidgetBackground(w.scrollArea()->viewport()));
+    }
+
+    // scroll the horizontal slider to the left side
+    {
+        w.scrollArea()->horizontalScrollBar()->setValue(w.scrollArea()->horizontalScrollBar()->minimum());
+        QVERIFY(w.grabWidgetBackground(w.scrollArea()->viewport()));
+    }
+
+    // scroll the vertical slider up
+    {
+        w.scrollArea()->verticalScrollBar()->setValue(w.scrollArea()->verticalScrollBar()->minimum());
+        QVERIFY(w.grabWidgetBackground(w.scrollArea()->viewport()));
+    }
+}
+
 QTEST_MAIN(tst_QWidgetRepaintManager)
 #include "tst_qwidgetrepaintmanager.moc"