Skip to content

Commit

Permalink
Make sure we paint overlapped children and siblings when moving far
Browse files Browse the repository at this point in the history 
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 <[email protected]> in
earlier attempts to fix this bug.

Fixes: QTBUG-26269
Pick-to: 6.2
Change-Id: If7443863f5eee79a80220cd587522122f42a21e4
Reviewed-by: Qt CI Bot <[email protected]>
Reviewed-by: Allan Sandfeld Jensen <[email protected]>
  • Loading branch information
@vohi
vohi committed Nov 15, 2021
1 parent 9879d41 commit 22634e0
Show file tree
Hide file tree
Showing 2 changed files with 131 additions and 12 deletions.
24 changes: 13 additions & 11 deletions src/widgets/kernel/qwidgetrepaintmanager.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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()) {
Expand All @@ -493,6 +493,7 @@ void QWidgetPrivate::moveRect(const QRect &rect, int dx, int dy)
childExpose -= r.translated(dx, dy);
}
}
isMoved = true;
}

childExpose -= overlappedExpose;
Expand All @@ -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;
}
}

Expand All @@ -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);
Expand Down
119 changes: 118 additions & 1 deletion tests/auto/widgets/kernel/qwidgetrepaintmanager/tst_qwidgetrepaintmanager.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@

#include <QTest>
#include <QPainter>
#include <QWidget>
#include <QScrollArea>
#include <QScrollBar>
#include <QApplication>

#include <private/qhighdpiscaling_p.h>
Expand Down Expand Up @@ -94,6 +95,7 @@ private slots:
void opaqueChildren();
void staticContents();
void scroll();
void moveWithOverlap();

private:
const int m_fuzz;
Expand Down Expand Up @@ -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"

0 comments on commit 22634e0

Please sign in to comment.