Fix qt_scrollRectInImage when scrolling outside of the image

We were clipping the source rect to the image, both pre and post
scrolling, but did not apply the same logic to the target position.
By computing the target position based on the already clipped source
rect we ensure that the target position is also correct.

This was causing valgrind warnings on Linux, and crashes on Windows,
when trying to test the lower level QBackingStore::scroll() function.
The reason we were not seeing this in practice was that QWidget does
its own sanitation and clipping of the arguments before passing them
on.

As a drive-by, fix the access of image to use constBits instead of a
manual cast, and rename variables to better reflect their use.

Pick-to: 6.3 6.2 5.15
Change-Id: Ibc190c2ef825e634956758f612a018f642f4202b
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Eirik Aavitsland <eirik.aavitsland@qt.io>
This commit is contained in:
Tor Arne Vestbø 2021-12-10 17:53:25 +01:00
parent 701294e362
commit ae6dc7d6df
2 changed files with 53 additions and 12 deletions

View File

@ -298,32 +298,34 @@ bool QBackingStore::hasStaticContents() const
void Q_GUI_EXPORT qt_scrollRectInImage(QImage &img, const QRect &rect, const QPoint &offset)
{
// make sure we don't detach
uchar *mem = const_cast<uchar*>(const_cast<const QImage &>(img).bits());
uchar *mem = const_cast<uchar*>(img.constBits());
qsizetype lineskip = img.bytesPerLine();
int depth = img.depth() >> 3;
const QRect imageRect(0, 0, img.width(), img.height());
const QRect r = rect & imageRect & imageRect.translated(-offset);
const QPoint p = rect.topLeft() + offset;
if (r.isEmpty())
const QRect sourceRect = rect.intersected(imageRect).intersected(imageRect.translated(-offset));
if (sourceRect.isEmpty())
return;
const QRect destRect = sourceRect.translated(offset);
Q_ASSERT_X(imageRect.contains(destRect), "qt_scrollRectInImage",
"The sourceRect should already account for clipping, both pre and post scroll");
const uchar *src;
uchar *dest;
if (r.top() < p.y()) {
src = mem + r.bottom() * lineskip + r.left() * depth;
dest = mem + (p.y() + r.height() - 1) * lineskip + p.x() * depth;
if (sourceRect.top() < destRect.top()) {
src = mem + sourceRect.bottom() * lineskip + sourceRect.left() * depth;
dest = mem + (destRect.top() + sourceRect.height() - 1) * lineskip + destRect.left() * depth;
lineskip = -lineskip;
} else {
src = mem + r.top() * lineskip + r.left() * depth;
dest = mem + p.y() * lineskip + p.x() * depth;
src = mem + sourceRect.top() * lineskip + sourceRect.left() * depth;
dest = mem + destRect.top() * lineskip + destRect.left() * depth;
}
const int w = r.width();
int h = r.height();
const int w = sourceRect.width();
int h = sourceRect.height();
const int bytes = w * depth;
// overlapping segments?

View File

@ -43,8 +43,47 @@ class tst_QBackingStore : public QObject
private slots:
void flush();
void scrollRectInImage_data();
void scrollRectInImage();
};
void tst_QBackingStore::scrollRectInImage_data()
{
QTest::addColumn<QRect>("rect");
QTest::addColumn<QPoint>("offset");
QTest::newRow("empty rect") << QRect() << QPoint();
QTest::newRow("rect outside image") << QRect(-100, -100, 1000, 1000) << QPoint(10, 10);
QTest::newRow("scroll outside positive") << QRect(10, 10, 10, 10) << QPoint(1000, 1000);
QTest::newRow("scroll outside negative") << QRect(10, 10, 10, 10) << QPoint(-1000, -1000);
QTest::newRow("sub-rect positive scroll") << QRect(100, 100, 50, 50) << QPoint(10, 10);
QTest::newRow("sub-rect negative scroll") << QRect(100, 100, 50, 50) << QPoint(-10, -10);
QTest::newRow("positive vertical only") << QRect(100, 100, 50, 50) << QPoint(0, 10);
QTest::newRow("negative vertical only") << QRect(100, 100, 50, 50) << QPoint(0, -10);
QTest::newRow("positive horizontal only") << QRect(100, 100, 50, 50) << QPoint(10, 0);
QTest::newRow("negative horizontal only") << QRect(100, 100, 50, 50) << QPoint(-10, 0);
QTest::newRow("whole rect positive") << QRect(0, 0, 250, 250) << QPoint(10, 10);
QTest::newRow("whole rect negative") << QRect(0, 0, 250, 250) << QPoint(-10, -10);
}
QT_BEGIN_NAMESPACE
Q_GUI_EXPORT void qt_scrollRectInImage(QImage &, const QRect &, const QPoint &);
QT_END_NAMESPACE
void tst_QBackingStore::scrollRectInImage()
{
QImage test(250, 250, QImage::Format_ARGB32_Premultiplied);
QFETCH(QRect, rect);
QFETCH(QPoint, offset);
qt_scrollRectInImage(test, rect, offset);
}
class Window : public QWindow
{
public: