rhi: Unify handling of special cases for scissor and viewport rects

With OpenGL a scissor (or viewport) rectangle is not allowed to have a negative
width or height. Everything else is allowed. This is also the semantic we wish to
keep for QRhiViewport and QRhiScissor.

This raises some problems. For instance, when we do bottom-left - top-left
rectangle conversion, the case of partially out of bounds rects needs to be
taken into account.

Otherwise, Qt Quick ends up in wrong scissoring in certain cases, typically when
the QQuickWindow size is decreased so the content does not fit because that will
then start generating negative x, y scissors for clipping (which is perfectly
valid but the QRhi backends need to be able to deal with it)

Then there is the problem of having to clamp width and height carefully, because
some validation layers for some APIs will reject a viewport or scissor with
partially out of bounds rectangles.

To verify all this, add a new manual test, based on the cubemap one. (cubemap was
chosen because that is an ideal test scene as it fills the viewport completely, and
so it is visually straightforward when a scissor rectangle is moving around over it)

Fixes: QTBUG-78702
Change-Id: I60614836432ea9934fc0dbd0ac7e88931f476542
Reviewed-by: Christian Strømme <christian.stromme@qt.io>

3 files changed, 49 insertions, 21 deletions

diff --git a/src/gui/rhi/qrhi.cpp b/src/gui/rhi/qrhi.cpp
index 858be0159b..ee4caa47a0 100644
--- a/src/gui/rhi/qrhi.cpp
+++ b/src/gui/rhi/qrhi.cpp
@@ -707,8 +707,8 @@ QDebug operator<<(QDebug dbg, const QRhiDepthStencilClearValue &v)
 
     Used with QRhiCommandBuffer::setViewport().
 
-    \note QRhi assumes OpenGL-style viewport coordinates, meaning x and y are
-    bottom-left.
+    QRhi assumes OpenGL-style viewport coordinates, meaning x and y are
+    bottom-left. Negative width or height are not allowed.
 
     Typical usage is like the following:
 
@@ -737,7 +737,9 @@ QDebug operator<<(QDebug dbg, const QRhiDepthStencilClearValue &v)
     Constructs a viewport description with the rectangle specified by \a x, \a
     y, \a w, \a h and the depth range \a minDepth and \a maxDepth.
 
-    \note x and y are assumed to be the bottom-left position.
+    \note \a x and \a y are assumed to be the bottom-left position. \a w and \a
+    h should not be negative, the viewport will be ignored by
+    QRhiCommandBuffer::setViewport() otherwise.
 
     \sa QRhi::clipSpaceCorrMatrix()
  */
@@ -810,8 +812,12 @@ QDebug operator<<(QDebug dbg, const QRhiViewport &v)
     only possible with a QRhiGraphicsPipeline that has
     QRhiGraphicsPipeline::UsesScissor set.
 
-    \note QRhi assumes OpenGL-style scissor coordinates, meaning x and y are
-    bottom-left.
+    QRhi assumes OpenGL-style scissor coordinates, meaning x and y are
+    bottom-left. Negative width or height are not allowed. However, apart from
+    that, the flexible OpenGL semantics apply: negative x and y, partially out
+    of bounds rectangles, etc. will be handled gracefully, clamping as
+    appropriate. Therefore, any rendering logic targeting OpenGL can feed
+    scissor rectangles into QRhiScissor as-is, without any adaptation.
 
     \sa QRhiCommandBuffer::setScissor(), QRhiViewport
  */
@@ -826,7 +832,11 @@ QDebug operator<<(QDebug dbg, const QRhiViewport &v)
     Constructs a scissor with the rectangle specified by \a x, \a y, \a w, and
     \a h.
 
-    \note x and y are assumed to be the bottom-left position.
+    \note \a x and \a y are assumed to be the bottom-left position. Negative \a w
+    or \a h are not allowed, such scissor rectangles will be ignored by
+    QRhiCommandBuffer. Other than that, the flexible OpenGL semantics apply:
+    negative x and y, partially out of bounds rectangles, etc. will be handled
+    gracefully, clamping as appropriate.
  */
 QRhiScissor::QRhiScissor(int x, int y, int w, int h)
     : m_rect { { x, y, w, h } }
diff --git a/src/gui/rhi/qrhi_p_p.h b/src/gui/rhi/qrhi_p_p.h
index 63f27b6de4..be2808549c 100644
--- a/src/gui/rhi/qrhi_p_p.h
+++ b/src/gui/rhi/qrhi_p_p.h
@@ -233,27 +233,37 @@ bool qrhi_toTopLeftRenderTargetRect(const QSize &outputSize, const std::array<T,
                                     T *x, T *y, T *w, T *h)
 {
     // x,y are bottom-left in QRhiScissor and QRhiViewport but top-left in
-    // Vulkan/Metal/D3D. We also need proper clamping since some
-    // validation/debug layers are allergic to out of bounds scissor or
-    // viewport rects.
+    // Vulkan/Metal/D3D. Our input is an OpenGL-style scissor rect where both
+    // negative x or y, and partly or completely out of bounds rects are
+    // allowed. The only thing the input here cannot have is a negative width
+    // or height. We must handle all other input gracefully, clamping to a zero
+    // width or height rect in the worst case, and ensuring the resulting rect
+    // is inside the rendertarget's bounds because some APIs' validation/debug
+    // layers are allergic to out of bounds scissor or viewport rects.
 
     const T outputWidth = outputSize.width();
     const T outputHeight = outputSize.height();
     const T inputWidth = r[2];
     const T inputHeight = r[3];
 
-    *x = qMax<T>(0, r[0]);
-    *y = qMax<T>(0, outputHeight - (r[1] + inputHeight));
-    *w = inputWidth;
-    *h = inputHeight;
-
-    if (*x >= outputWidth || *y >= outputHeight)
+    if (inputWidth < 0 || inputHeight < 0)
         return false;
 
+    *x = r[0];
+    *y = outputHeight - (r[1] + inputHeight);
+
+    const T widthOffset = *x < 0 ? -*x : 0;
+    const T heightOffset = *y < 0 ? -*y : 0;
+
+    *x = qBound<T>(0, *x, outputWidth - 1);
+    *y = qBound<T>(0, *y, outputHeight - 1);
+    *w = qMax<T>(0, inputWidth - widthOffset);
+    *h = qMax<T>(0, inputHeight - heightOffset);
+
     if (*x + *w > outputWidth)
-        *w = outputWidth - *x;
+        *w = qMax<T>(0, outputWidth - *x - 1);
     if (*y + *h > outputHeight)
-        *h = outputHeight - *y;
+        *h = qMax<T>(0, outputHeight - *y - 1);
 
     return true;
 }
diff --git a/src/gui/rhi/qrhigles2.cpp b/src/gui/rhi/qrhigles2.cpp
index 2d51d892e3..838c28efa2 100644
--- a/src/gui/rhi/qrhigles2.cpp
+++ b/src/gui/rhi/qrhigles2.cpp
@@ -1004,8 +1004,12 @@ void QRhiGles2::setViewport(QRhiCommandBuffer *cb, const QRhiViewport &viewport)
     QGles2CommandBuffer::Command cmd;
     cmd.cmd = QGles2CommandBuffer::Command::Viewport;
     const std::array<float, 4> r = viewport.viewport();
-    cmd.args.viewport.x = qMax(0.0f, r[0]);
-    cmd.args.viewport.y = qMax(0.0f, r[1]);
+    // A negative width or height is an error. A negative x or y is not.
+    if (r[2] < 0.0f || r[3] < 0.0f)
+        return;
+
+    cmd.args.viewport.x = r[0];
+    cmd.args.viewport.y = r[1];
     cmd.args.viewport.w = r[2];
     cmd.args.viewport.h = r[3];
     cmd.args.viewport.d0 = viewport.minDepth();
@@ -1021,8 +1025,12 @@ void QRhiGles2::setScissor(QRhiCommandBuffer *cb, const QRhiScissor &scissor)
     QGles2CommandBuffer::Command cmd;
     cmd.cmd = QGles2CommandBuffer::Command::Scissor;
     const std::array<int, 4> r = scissor.scissor();
-    cmd.args.scissor.x = qMax(0, r[0]);
-    cmd.args.scissor.y = qMax(0, r[1]);
+    // A negative width or height is an error. A negative x or y is not.
+    if (r[2] < 0 || r[3] < 0)
+        return;
+
+    cmd.args.scissor.x = r[0];
+    cmd.args.scissor.y = r[1];
     cmd.args.scissor.w = r[2];
     cmd.args.scissor.h = r[3];
     cbD->commands.append(cmd);