rhi: Do not rely on unspecified relation between readbacks and FramesInFlight

Add a new queriable resource limit value MaxAsyncReadbackFrames. Change
the autotest to rely on this instead of relying on the unspecified,
works-by-accident relation between readbacks and FramesInFlight. This
way even if the behavior diverges in some backend in the future, clients
(well written ones, that is), will continue to function correctly.

Also clarify the docs for FramesInFlight, and change d3d and gl to return
the correct value (which is 1 from QRhi perspective; the expanded docs now
explain a bit what this really means and what it does not).

Change-Id: I0486715570a9e6fc5d3dc431694d1712875dfe01
Reviewed-by: Andy Nichols <andy.nichols@qt.io>

7 files changed, 61 insertions, 17 deletions

diff --git a/src/gui/rhi/qrhi.cpp b/src/gui/rhi/qrhi.cpp
index f857142bd7..584fbb263d 100644
--- a/src/gui/rhi/qrhi.cpp
+++ b/src/gui/rhi/qrhi.cpp
@@ -627,18 +627,27 @@ Q_LOGGING_CATEGORY(QRHI_LOG_INFO, "qt.rhi.general")
     is what some OpenGL ES implementations provide.
 
     \value FramesInFlight The number of frames the backend may keep "in
-    flight". The value has no relevance, and is unspecified, with backends like
-    OpenGL and Direct3D 11. With backends like Vulkan or Metal, it is the
-    responsibility of QRhi to block whenever starting a new frame and finding
-    the CPU is already \c{N - 1} frames ahead of the GPU (because the command
-    buffer submitted in frame no. \c{current} - \c{N} has not yet completed).
-    The value N is what is returned from here, and is typically 2. This can be
-    relevant to applications that integrate rendering done directly with the
-    graphics API, as such rendering code may want to perform double (if the
-    value is 2) buffering for resources, such as, buffers, similarly to the
-    QRhi backends themselves. The current frame slot index (a value running 0,
-    1, .., N-1, then wrapping around) is retrievable from
-    QRhi::currentFrameSlot().
+    flight": with backends like Vulkan or Metal, it is the responsibility of
+    QRhi to block whenever starting a new frame and finding the CPU is already
+    \c{N - 1} frames ahead of the GPU (because the command buffer submitted in
+    frame no. \c{current} - \c{N} has not yet completed). The value N is what
+    is returned from here, and is typically 2. This can be relevant to
+    applications that integrate rendering done directly with the graphics API,
+    as such rendering code may want to perform double (if the value is 2)
+    buffering for resources, such as, buffers, similarly to the QRhi backends
+    themselves. The current frame slot index (a value running 0, 1, .., N-1,
+    then wrapping around) is retrievable from QRhi::currentFrameSlot(). The
+    value is 1 for backends where the graphics API offers no such low level
+    control over the command submission process. Note that pipelining may still
+    happen even when this value is 1 (some backends, such as D3D11, are
+    designed to attempt to enable this, for instance, by using an update
+    strategy for uniform buffers that does not stall the pipeline), but that is
+    then not controlled by QRhi and so not reflected here in the API.
+
+    \value MaxAsyncReadbackFrames The number of \l{QRhi::endFrame()}{submitted}
+    frames (including the one that contains the readback) after which an
+    asynchronous texture or buffer readback is guaranteed to complete upon
+    \l{QRhi::beginFrame()}{starting a new frame}.
  */
 
 /*!
@@ -4339,7 +4348,15 @@ void QRhiResourceUpdateBatch::uploadStaticBuffer(QRhiBuffer *buf, const void *da
     is supported only when the QRhi::ReadBackNonUniformBuffer feature is
     reported as supported.
 
-    \a readBackTexture(), QRhi::isFeatureSupported()
+   \note The asynchronous readback is guaranteed to have completed when one of
+   the following conditions is met: \l{QRhi::finish()}{finish()} has been
+   called; or, at least \c N frames have been \l{QRhi::endFrame()}{submitted},
+   including the frame that issued the readback operation, and the
+   \l{QRhi::beginFrame()}{recording of a new frame} has been started, where \c
+   N is the \l{QRhi::resourceLimit()}{resource limit value} returned for
+   QRhi::MaxAsyncReadbackFrames.
+
+   \sa readBackTexture(), QRhi::isFeatureSupported(), QRhi::resourceLimit()
  */
 void QRhiResourceUpdateBatch::readBackBuffer(QRhiBuffer *buf, int offset, int size, QRhiBufferReadbackResult *result)
 {
@@ -4430,6 +4447,16 @@ void QRhiResourceUpdateBatch::copyTexture(QRhiTexture *dst, QRhiTexture *src, co
    happens with a byte ordered format. A \l{QRhiTexture::RGBA8}{RGBA8} texture
    maps therefore to byte ordered QImage formats, such as,
    QImage::Format_RGBA8888.
+
+   \note The asynchronous readback is guaranteed to have completed when one of
+   the following conditions is met: \l{QRhi::finish()}{finish()} has been
+   called; or, at least \c N frames have been \l{QRhi::endFrame()}{submitted},
+   including the frame that issued the readback operation, and the
+   \l{QRhi::beginFrame()}{recording of a new frame} has been started, where \c
+   N is the \l{QRhi::resourceLimit()}{resource limit value} returned for
+   QRhi::MaxAsyncReadbackFrames.
+
+   \sa readBackBuffer(), QRhi::resourceLimit()
  */
 void QRhiResourceUpdateBatch::readBackTexture(const QRhiReadbackDescription &rb, QRhiReadbackResult *result)
 {
diff --git a/src/gui/rhi/qrhi_p.h b/src/gui/rhi/qrhi_p.h
index 7d03ac8aaa..be96ebbf05 100644
--- a/src/gui/rhi/qrhi_p.h
+++ b/src/gui/rhi/qrhi_p.h
@@ -1448,7 +1448,8 @@ public:
         TextureSizeMin = 1,
         TextureSizeMax,
         MaxColorAttachments,
-        FramesInFlight
+        FramesInFlight,
+        MaxAsyncReadbackFrames
     };
 
     ~QRhi();
diff --git a/src/gui/rhi/qrhid3d11.cpp b/src/gui/rhi/qrhid3d11.cpp
index 883aa68df7..221ec6e6e5 100644
--- a/src/gui/rhi/qrhid3d11.cpp
+++ b/src/gui/rhi/qrhid3d11.cpp
@@ -482,7 +482,13 @@ int QRhiD3D11::resourceLimit(QRhi::ResourceLimit limit) const
     case QRhi::MaxColorAttachments:
         return 8;
     case QRhi::FramesInFlight:
-        return 2; // dummy
+        // From our perspective. What D3D does internally is another question
+        // (there could be pipelining, helped f.ex. by our MAP_DISCARD based
+        // uniform buffer update strategy), but that's out of our hands and
+        // does not concern us here.
+        return 1;
+    case QRhi::MaxAsyncReadbackFrames:
+        return 1;
     default:
         Q_UNREACHABLE();
         return 0;
diff --git a/src/gui/rhi/qrhigles2.cpp b/src/gui/rhi/qrhigles2.cpp
index f2b9f7081b..df0b255bed 100644
--- a/src/gui/rhi/qrhigles2.cpp
+++ b/src/gui/rhi/qrhigles2.cpp
@@ -787,7 +787,11 @@ int QRhiGles2::resourceLimit(QRhi::ResourceLimit limit) const
     case QRhi::MaxColorAttachments:
         return caps.maxDrawBuffers;
     case QRhi::FramesInFlight:
-        return 2; // dummy
+        // From our perspective. What the GL impl does internally is another
+        // question, but that's out of our hands and does not concern us here.
+        return 1;
+    case QRhi::MaxAsyncReadbackFrames:
+        return 1;
     default:
         Q_UNREACHABLE();
         return 0;
diff --git a/src/gui/rhi/qrhimetal.mm b/src/gui/rhi/qrhimetal.mm
index 5b77086983..b0813bfc77 100644
--- a/src/gui/rhi/qrhimetal.mm
+++ b/src/gui/rhi/qrhimetal.mm
@@ -581,6 +581,8 @@ int QRhiMetal::resourceLimit(QRhi::ResourceLimit limit) const
         return 8;
     case QRhi::FramesInFlight:
         return QMTL_FRAMES_IN_FLIGHT;
+    case QRhi::MaxAsyncReadbackFrames:
+        return QMTL_FRAMES_IN_FLIGHT;
     default:
         Q_UNREACHABLE();
         return 0;
diff --git a/src/gui/rhi/qrhinull.cpp b/src/gui/rhi/qrhinull.cpp
index 4c59900aa6..8c07e09b32 100644
--- a/src/gui/rhi/qrhinull.cpp
+++ b/src/gui/rhi/qrhinull.cpp
@@ -146,7 +146,9 @@ int QRhiNull::resourceLimit(QRhi::ResourceLimit limit) const
     case QRhi::MaxColorAttachments:
         return 8;
     case QRhi::FramesInFlight:
-        return 2; // dummy
+        return 1;
+    case QRhi::MaxAsyncReadbackFrames:
+        return 1;
     default:
         Q_UNREACHABLE();
         return 0;
diff --git a/src/gui/rhi/qrhivulkan.cpp b/src/gui/rhi/qrhivulkan.cpp
index 840eed5a79..5d6e9b21bc 100644
--- a/src/gui/rhi/qrhivulkan.cpp
+++ b/src/gui/rhi/qrhivulkan.cpp
@@ -4009,6 +4009,8 @@ int QRhiVulkan::resourceLimit(QRhi::ResourceLimit limit) const
         return int(physDevProperties.limits.maxColorAttachments);
     case QRhi::FramesInFlight:
         return QVK_FRAMES_IN_FLIGHT;
+    case QRhi::MaxAsyncReadbackFrames:
+        return QVK_FRAMES_IN_FLIGHT;
     default:
         Q_UNREACHABLE();
         return 0;