Define inverted QRects consistently

Changes the definition of invalid QRects to be more consistent.
This simplifies the logic, and makes it possible for us to fix
normalized() so dimensions don't change.

The actual API is not changed except for inverted rects.

Only one use-case for the old normalized() function existed,
and has been reimplemented as QRect::span().

Fixes: QTBUG-22934
Change-Id: I29dad2952dc6c8e84a6d931898dc7e43d66780f3
Reviewed-by: hjk <hjk@qt.io>
Reviewed-by: Lars Knoll <lars.knoll@qt.io>

3 files changed, 103 insertions, 119 deletions

diff --git a/src/corelib/tools/qrect.cpp b/src/corelib/tools/qrect.cpp
index 2bdf1094da..45af85a4df 100644
--- a/src/corelib/tools/qrect.cpp
+++ b/src/corelib/tools/qrect.cpp
@@ -223,9 +223,14 @@ QT_BEGIN_NAMESPACE
 /*!
     \fn QRect::QRect(const QPoint &topLeft, const QPoint &bottomRight)
 
-    Constructs a rectangle with the given \a topLeft and \a bottomRight corners.
+    Constructs a rectangle with the given \a topLeft and \a bottomRight corners, both included.
 
-    \sa setTopLeft(), setBottomRight()
+    If \a bottomRight is to higher and to the left of \a topLeft, the rectangle defined
+    is instead non-inclusive of the corners.
+
+    \note To ensure both points are included regardless of relative order, use span().
+
+    \sa setTopLeft(), setBottomRight(), span()
 */
 
 
@@ -295,27 +300,22 @@ QT_BEGIN_NAMESPACE
     non-negative width and height.
 
     If width() < 0 the function swaps the left and right corners, and
-    it swaps the top and bottom corners if height() < 0.
+    it swaps the top and bottom corners if height() < 0. The corners
+    are at the same time changed from being non-inclusive to inclusive.
 
     \sa isValid(), isEmpty()
 */
 
 QRect QRect::normalized() const noexcept
 {
-    QRect r;
-    if (x2 < x1 - 1) {                                // swap bad x values
-        r.x1 = x2;
-        r.x2 = x1;
-    } else {
-        r.x1 = x1;
-        r.x2 = x2;
+    QRect r(*this);
+    if (x2 < x1) {                                // swap bad x values
+        r.x1 = x2 + 1;
+        r.x2 = x1 - 1;
     }
-    if (y2 < y1 - 1) {                                // swap bad y values
-        r.y1 = y2;
-        r.y2 = y1;
-    } else {
-        r.y1 = y1;
-        r.y2 = y2;
+    if (y2 < y1) {                                // swap bad y values
+        r.y1 = y2 + 1;
+        r.y2 = y1 - 1;
     }
     return r;
 }
@@ -824,8 +824,8 @@ bool QRect::contains(const QPoint &p, bool proper) const noexcept
 {
     int l, r;
     if (x2 < x1 - 1) {
-        l = x2;
-        r = x1;
+        l = x2 + 1;
+        r = x1 - 1;
     } else {
         l = x1;
         r = x2;
@@ -839,8 +839,8 @@ bool QRect::contains(const QPoint &p, bool proper) const noexcept
     }
     int t, b;
     if (y2 < y1 - 1) {
-        t = y2;
-        b = y1;
+        t = y2 + 1;
+        b = y1 - 1;
     } else {
         t = y1;
         b = y2;
@@ -890,16 +890,16 @@ bool QRect::contains(const QRect &r, bool proper) const noexcept
         return false;
 
     int l1 = x1;
-    int r1 = x1;
-    if (x2 - x1 + 1 < 0)
-        l1 = x2;
+    int r1 = x1 - 1;
+    if (x2 < x1 - 1)
+        l1 = x2 + 1;
     else
         r1 = x2;
 
     int l2 = r.x1;
-    int r2 = r.x1;
-    if (r.x2 - r.x1 + 1 < 0)
-        l2 = r.x2;
+    int r2 = r.x1 - 1;
+    if (r.x2 < r.x1 - 1)
+        l2 = r.x2 + 1;
     else
         r2 = r.x2;
 
@@ -912,16 +912,16 @@ bool QRect::contains(const QRect &r, bool proper) const noexcept
     }
 
     int t1 = y1;
-    int b1 = y1;
-    if (y2 - y1 + 1 < 0)
-        t1 = y2;
+    int b1 = y1 - 1;
+    if (y2 < y1 - 1)
+        t1 = y2 + 1;
     else
         b1 = y2;
 
     int t2 = r.y1;
-    int b2 = r.y1;
-    if (r.y2 - r.y1 + 1 < 0)
-        t2 = r.y2;
+    int b2 = r.y1 - 1;
+    if (r.y2 < r.y1 - 1)
+        t2 = r.y2 + 1;
     else
         b2 = r.y2;
 
@@ -970,30 +970,30 @@ QRect QRect::operator|(const QRect &r) const noexcept
         return *this;
 
     int l1 = x1;
-    int r1 = x1;
-    if (x2 - x1 + 1 < 0)
-        l1 = x2;
+    int r1 = x1 - 1;
+    if (x2 < x1 - 1)
+        l1 = x2 + 1;
     else
         r1 = x2;
 
     int l2 = r.x1;
-    int r2 = r.x1;
-    if (r.x2 - r.x1 + 1 < 0)
-        l2 = r.x2;
+    int r2 = r.x1 - 1;
+    if (r.x2 < r.x1 - 1)
+        l2 = r.x2 + 1;
     else
         r2 = r.x2;
 
     int t1 = y1;
-    int b1 = y1;
-    if (y2 - y1 + 1 < 0)
-        t1 = y2;
+    int b1 = y1 - 1;
+    if (y2 < y1 - 1)
+        t1 = y2 + 1;
     else
         b1 = y2;
 
     int t2 = r.y1;
-    int b2 = r.y1;
-    if (r.y2 - r.y1 + 1 < 0)
-        t2 = r.y2;
+    int b2 = r.y1 - 1;
+    if (r.y2 < r.y1 - 1)
+        t2 = r.y2 + 1;
     else
         b2 = r.y2;
 
@@ -1032,35 +1032,35 @@ QRect QRect::operator&(const QRect &r) const noexcept
         return QRect();
 
     int l1 = x1;
-    int r1 = x1;
-    if (x2 - x1 + 1 < 0)
-        l1 = x2;
-    else
-        r1 = x2;
+    int r1 = x2;
+    if (x2 < x1 - 1) {
+        l1 = x2 + 1;
+        r1 = x1 - 1;
+    }
 
     int l2 = r.x1;
-    int r2 = r.x1;
-    if (r.x2 - r.x1 + 1 < 0)
-        l2 = r.x2;
-    else
-        r2 = r.x2;
+    int r2 = r.x2;
+    if (r.x2 < r.x1 - 1) {
+        l2 = r.x2 + 1;
+        r2 = r.x1 - 1;
+    }
 
     if (l1 > r2 || l2 > r1)
         return QRect();
 
     int t1 = y1;
-    int b1 = y1;
-    if (y2 - y1 + 1 < 0)
-        t1 = y2;
-    else
-        b1 = y2;
+    int b1 = y2;
+    if (y2 < y1 - 1) {
+        t1 = y2 + 1;
+        b1 = y1 - 1;
+    }
 
     int t2 = r.y1;
-    int b2 = r.y1;
-    if (r.y2 - r.y1 + 1 < 0)
-        t2 = r.y2;
-    else
-        b2 = r.y2;
+    int b2 = r.y2;
+    if (r.y2 < r.y1 - 1) {
+        t2 = r.y2 + 1;
+        b2 = r.y1 - 1;
+    }
 
     if (t1 > b2 || t2 > b1)
         return QRect();
@@ -1104,35 +1104,35 @@ bool QRect::intersects(const QRect &r) const noexcept
         return false;
 
     int l1 = x1;
-    int r1 = x1;
-    if (x2 - x1 + 1 < 0)
-        l1 = x2;
-    else
-        r1 = x2;
+    int r1 = x2;
+    if (x2 < x1 - 1) {
+        l1 = x2 + 1;
+        r1 = x1 - 1;
+    }
 
     int l2 = r.x1;
-    int r2 = r.x1;
-    if (r.x2 - r.x1 + 1 < 0)
-        l2 = r.x2;
-    else
-        r2 = r.x2;
+    int r2 = r.x2;
+    if (r.x2 < r.x1 - 1) {
+        l2 = r.x2 + 1;
+        r2 = r.x1 - 1;
+    }
 
     if (l1 > r2 || l2 > r1)
         return false;
 
     int t1 = y1;
-    int b1 = y1;
-    if (y2 - y1 + 1 < 0)
-        t1 = y2;
-    else
-        b1 = y2;
+    int b1 = y2;
+    if (y2 < y1 - 1) {
+        t1 = y2 + 1;
+        b1 = y1 - 1;
+    }
 
     int t2 = r.y1;
-    int b2 = r.y1;
-    if (r.y2 - r.y1 + 1 < 0)
-        t2 = r.y2;
-    else
-        b2 = r.y2;
+    int b2 = r.y2;
+    if (r.y2 < r.y1 - 1) {
+        t2 = r.y2 + 1;
+        b2 = r.y1 - 1;
+    }
 
     if (t1 > b2 || t2 > b1)
         return false;
@@ -1225,6 +1225,14 @@ bool QRect::intersects(const QRect &r) const noexcept
     \since 5.1
 */
 
+/*!
+    \fn static QRect QRect::span(const QPoint &p1, const QPoint &p2)
+
+    Returns a rectangle spanning the two points, including both and everything
+    in between.
+
+    \since 6.0
+*/
 
 /*****************************************************************************
   QRect stream functions
diff --git a/src/corelib/tools/qrect.h b/src/corelib/tools/qrect.h
index cfe92e81e6..5deb220b2d 100644
--- a/src/corelib/tools/qrect.h
+++ b/src/corelib/tools/qrect.h
@@ -146,6 +146,8 @@ public:
     constexpr inline QRect &operator+=(const QMargins &margins) noexcept;
     constexpr inline QRect &operator-=(const QMargins &margins) noexcept;
 
+    Q_REQUIRED_RESULT static constexpr inline QRect span(const QPoint &p1, const QPoint &p2) noexcept;
+
     friend constexpr inline bool operator==(const QRect &, const QRect &) noexcept;
     friend constexpr inline bool operator!=(const QRect &, const QRect &) noexcept;
     friend constexpr inline size_t qHash(const QRect &, size_t) noexcept;
@@ -505,6 +507,12 @@ constexpr inline QRect &QRect::operator-=(const QMargins &margins) noexcept
     return *this;
 }
 
+constexpr QRect QRect::span(const QPoint &p1, const QPoint &p2) noexcept
+{
+    return QRect(QPoint(qMin(p1.x(), p2.x()), qMin(p1.y(), p2.y())),
+                 QPoint(qMax(p1.x(), p2.x()), qMax(p1.y(), p2.y())));
+}
+
 #ifndef QT_NO_DEBUG_STREAM
 Q_CORE_EXPORT QDebug operator<<(QDebug, const QRect &);
 #endif
diff --git a/src/gui/painting/qpaintengine_raster.cpp b/src/gui/painting/qpaintengine_raster.cpp
index f34b4bb9a2..781888dd5b 100644
--- a/src/gui/painting/qpaintengine_raster.cpp
+++ b/src/gui/painting/qpaintengine_raster.cpp
@@ -271,35 +271,6 @@ static void qt_debug_path(const QPainterPath &path)
 }
 #endif
 
-// QRect::normalized() will change the width/height of the rectangle due to
-// its incusive-integer definition of left/right vs width. This is not
-// something we want to change in QRect as that would potentially introduce
-// regressions all over the place, so we implement a straightforward
-// normalized here. QRectF already does this, so QRectF::normalized() is ok to
-// use.
-static QRect qrect_normalized(const QRect &rect)
-{
-    int x, y, w, h;
-    if (Q_UNLIKELY(rect.width() < 0)) {
-        x = rect.x() + rect.width();
-        w = -rect.width();
-    } else {
-        x = rect.x();
-        w = rect.width();
-    }
-
-    if (Q_UNLIKELY(rect.height() < 0)) {
-        y = rect.y() + rect.height();
-        h = -rect.height();
-    } else {
-        y = rect.y();
-        h = rect.height();
-    }
-
-    return QRect(x, y, w, h);
-}
-
-
 QRasterPaintEnginePrivate::QRasterPaintEnginePrivate() :
     QPaintEngineExPrivate(),
     cachedLines(0)
@@ -1320,9 +1291,7 @@ void QRasterPaintEngine::clip(const QRect &rect, Qt::ClipOperation op)
 bool QRasterPaintEngine::setClipRectInDeviceCoords(const QRect &r, Qt::ClipOperation op)
 {
     Q_D(QRasterPaintEngine);
-    // normalize before using the & operator which uses QRect::normalize()
-    // internally which will give us the wrong values.
-    QRect clipRect = qrect_normalized(r) & d->deviceRect;
+    QRect clipRect = r & d->deviceRect;
     QRasterPaintEngineState *s = state();
 
     if (op == Qt::ReplaceClip || s->clip == nullptr) {
@@ -1557,7 +1526,7 @@ void QRasterPaintEngine::drawRects(const QRect *rects, int rectCount)
             int offset_x = int(s->matrix.dx());
             int offset_y = int(s->matrix.dy());
             while (r < lastRect) {
-                QRect rect = qrect_normalized(*r);
+                QRect rect = r->normalized();
                 QRect rr = rect.translated(offset_x, offset_y);
                 fillRect_normalized(rr, &s->brushData, d);
                 ++r;
@@ -3006,8 +2975,8 @@ bool QRasterPaintEnginePrivate::isUnclipped(const QRect &rect,
     Q_Q(const QRasterPaintEngine);
     const QRasterPaintEngineState *s = q->state();
     const QClipData *cl = clip();
+    QRect r = rect.normalized();
     if (!cl) {
-        QRect r = qrect_normalized(rect);
         // inline contains() for performance (we know the rects are normalized)
         const QRect &r1 = deviceRect;
         return (r.left() >= r1.left() && r.right() <= r1.right()
@@ -3022,7 +2991,6 @@ bool QRasterPaintEnginePrivate::isUnclipped(const QRect &rect,
     if (s->flags.antialiased)
         ++penWidth;
 
-    QRect r = qrect_normalized(rect);
     if (penWidth > 0) {
         r.setX(r.x() - penWidth);
         r.setY(r.y() - penWidth);