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>

1 files changed, 14 insertions, 12 deletions

diff --git a/src/gui/painting/qbackingstore.cpp b/src/gui/painting/qbackingstore.cpp
index f76b9864fc..762938e764 100644
--- a/src/gui/painting/qbackingstore.cpp
+++ b/src/gui/painting/qbackingstore.cpp
@@ -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?