rhi: Execute pending host writes on nativeBuffer() query

Otherwise it is impossible to write an application that pulls out the
VkBuffer for a Dynamic QRhiBuffer, and then uses it with custom Vulkan
operations that read from the buffer. More precisely, the problem arises
only if the buffer in question is not used in combination with any QRhi
operations, because in that case there is nothing that would trigger
doing the host writes queued up by a resource batch's updateDynamicBuffer().

Task-number: QTBUG-82435
Change-Id: Ieb54422f1493921bc6d4d029be56130cd3a1362a
Reviewed-by: Christian Strømme <christian.stromme@qt.io>

7 files changed, 43 insertions, 30 deletions

diff --git a/src/gui/rhi/qrhi.cpp b/src/gui/rhi/qrhi.cpp
index 7a0d53e1e4..6243bcda58 100644
--- a/src/gui/rhi/qrhi.cpp
+++ b/src/gui/rhi/qrhi.cpp
@@ -2048,11 +2048,9 @@ QRhiResource::Type QRhiBuffer::resourceType() const
     UniformBuffer may not even be backed by a native buffer object at all if
     uniform buffers are not used or supported by a given backend and graphics
     API. There are also differences to how data is written to the buffer and
-    the type of backing memory used, and, if host visible memory is involved,
-    when memory writes become available and visible. Therefore, in general it
-    is recommended to limit native buffer object access to vertex and index
-    buffers with types Static or Immutable, because these operate in a
-    relatively uniform manner with all backends.
+    the type of backing memory used. For buffers backed by host visible memory,
+    calling this function guarantees that pending host writes are executed for
+    all the returned native buffers.
 
     \sa QRhi::currentFrameSlot(), QRhi::FramesInFlight
  */
diff --git a/src/gui/rhi/qrhid3d11.cpp b/src/gui/rhi/qrhid3d11.cpp
index f7c7f4a9f2..ce39b42cf5 100644
--- a/src/gui/rhi/qrhid3d11.cpp
+++ b/src/gui/rhi/qrhid3d11.cpp
@@ -608,7 +608,7 @@ void QRhiD3D11::setShaderResources(QRhiCommandBuffer *cb, QRhiShaderResourceBind
         {
             QD3D11Buffer *bufD = QRHI_RES(QD3D11Buffer, b->u.ubuf.buf);
             if (bufD->m_type == QRhiBuffer::Dynamic)
-                executeBufferHostWritesForCurrentFrame(bufD);
+                executeBufferHostWrites(bufD);
 
             if (bufD->generation != bd.ubuf.generation || bufD->m_id != bd.ubuf.id) {
                 srbUpdate = true;
@@ -725,7 +725,7 @@ void QRhiD3D11::setVertexInput(QRhiCommandBuffer *cb,
         QD3D11Buffer *bufD = QRHI_RES(QD3D11Buffer, bindings[i].first);
         Q_ASSERT(bufD->m_usage.testFlag(QRhiBuffer::VertexBuffer));
         if (bufD->m_type == QRhiBuffer::Dynamic)
-            executeBufferHostWritesForCurrentFrame(bufD);
+            executeBufferHostWrites(bufD);
 
         if (cbD->currentVertexBuffers[inputSlot] != bufD->buffer
                 || cbD->currentVertexOffsets[inputSlot] != bindings[i].second)
@@ -757,7 +757,7 @@ void QRhiD3D11::setVertexInput(QRhiCommandBuffer *cb,
         QD3D11Buffer *ibufD = QRHI_RES(QD3D11Buffer, indexBuf);
         Q_ASSERT(ibufD->m_usage.testFlag(QRhiBuffer::IndexBuffer));
         if (ibufD->m_type == QRhiBuffer::Dynamic)
-            executeBufferHostWritesForCurrentFrame(ibufD);
+            executeBufferHostWrites(ibufD);
 
         const DXGI_FORMAT dxgiFormat = indexFormat == QRhiCommandBuffer::IndexUInt16 ? DXGI_FORMAT_R16_UINT
                                                                                      : DXGI_FORMAT_R32_UINT;
@@ -1920,7 +1920,7 @@ void QRhiD3D11::updateShaderResourceBindings(QD3D11ShaderResourceBindings *srbD)
     srbD->csUAVs.finish();
 }
 
-void QRhiD3D11::executeBufferHostWritesForCurrentFrame(QD3D11Buffer *bufD)
+void QRhiD3D11::executeBufferHostWrites(QD3D11Buffer *bufD)
 {
     if (!bufD->hasPendingDynamicUpdates)
         return;
@@ -2388,6 +2388,10 @@ bool QD3D11Buffer::build()
 
 QRhiBuffer::NativeBuffer QD3D11Buffer::nativeBuffer()
 {
+    if (m_type == Dynamic) {
+        QRHI_RES_RHI(QRhiD3D11);
+        rhiD->executeBufferHostWrites(this);
+    }
     return { { &buffer }, 1 };
 }
 
diff --git a/src/gui/rhi/qrhid3d11_p_p.h b/src/gui/rhi/qrhid3d11_p_p.h
index 04751397f7..b70f541c68 100644
--- a/src/gui/rhi/qrhid3d11_p_p.h
+++ b/src/gui/rhi/qrhid3d11_p_p.h
@@ -643,7 +643,7 @@ public:
                              int layer, int level, const QRhiTextureSubresourceUploadDescription &subresDesc);
     void enqueueResourceUpdates(QRhiCommandBuffer *cb, QRhiResourceUpdateBatch *resourceUpdates);
     void updateShaderResourceBindings(QD3D11ShaderResourceBindings *srbD);
-    void executeBufferHostWritesForCurrentFrame(QD3D11Buffer *bufD);
+    void executeBufferHostWrites(QD3D11Buffer *bufD);
     void bindShaderResources(QD3D11ShaderResourceBindings *srbD,
                              const uint *dynOfsPairs, int dynOfsPairCount,
                              bool offsetOnlyChange);
diff --git a/src/gui/rhi/qrhimetal.mm b/src/gui/rhi/qrhimetal.mm
index 48a562ef1d..314c58b0b7 100644
--- a/src/gui/rhi/qrhimetal.mm
+++ b/src/gui/rhi/qrhimetal.mm
@@ -1827,16 +1827,15 @@ void QRhiMetal::enqueueResourceUpdates(QRhiCommandBuffer *cb, QRhiResourceUpdate
 }
 
 // this handles all types of buffers, not just Dynamic
-void QRhiMetal::executeBufferHostWritesForCurrentFrame(QMetalBuffer *bufD)
+void QRhiMetal::executeBufferHostWritesForSlot(QMetalBuffer *bufD, int slot)
 {
-    const int idx = bufD->d->slotted ? currentFrameSlot : 0;
-    if (bufD->d->pendingUpdates[idx].isEmpty())
+    if (bufD->d->pendingUpdates[slot].isEmpty())
         return;
 
-    void *p = [bufD->d->buf[idx] contents];
+    void *p = [bufD->d->buf[slot] contents];
     int changeBegin = -1;
     int changeEnd = -1;
-    for (const QRhiResourceUpdateBatchPrivate::BufferOp &u : qAsConst(bufD->d->pendingUpdates[idx])) {
+    for (const QRhiResourceUpdateBatchPrivate::BufferOp &u : qAsConst(bufD->d->pendingUpdates[slot])) {
         Q_ASSERT(bufD == QRHI_RES(QMetalBuffer, u.buf));
         memcpy(static_cast<char *>(p) + u.offset, u.data.constData(), size_t(u.data.size()));
         if (changeBegin == -1 || u.offset < changeBegin)
@@ -1846,10 +1845,15 @@ void QRhiMetal::executeBufferHostWritesForCurrentFrame(QMetalBuffer *bufD)
     }
 #ifdef Q_OS_MACOS
     if (changeBegin >= 0 && bufD->d->managed)
-        [bufD->d->buf[idx] didModifyRange: NSMakeRange(NSUInteger(changeBegin), NSUInteger(changeEnd - changeBegin))];
+        [bufD->d->buf[slot] didModifyRange: NSMakeRange(NSUInteger(changeBegin), NSUInteger(changeEnd - changeBegin))];
 #endif
 
-    bufD->d->pendingUpdates[idx].clear();
+    bufD->d->pendingUpdates[slot].clear();
+}
+
+void QRhiMetal::executeBufferHostWritesForCurrentFrame(QMetalBuffer *bufD)
+{
+    executeBufferHostWritesForSlot(bufD, bufD->d->slotted ? currentFrameSlot : 0);
 }
 
 void QRhiMetal::resourceUpdate(QRhiCommandBuffer *cb, QRhiResourceUpdateBatch *resourceUpdates)
@@ -2205,8 +2209,11 @@ QRhiBuffer::NativeBuffer QMetalBuffer::nativeBuffer()
     if (d->slotted) {
         NativeBuffer b;
         Q_ASSERT(sizeof(b.objects) / sizeof(b.objects[0]) >= size_t(QMTL_FRAMES_IN_FLIGHT));
-        for (int i = 0; i < QMTL_FRAMES_IN_FLIGHT; ++i)
+        for (int i = 0; i < QMTL_FRAMES_IN_FLIGHT; ++i) {
+            QRHI_RES_RHI(QRhiMetal);
+            rhiD->executeBufferHostWritesForSlot(this, i);
             b.objects[i] = &d->buf[i];
+        }
         b.slotCount = QMTL_FRAMES_IN_FLIGHT;
         return b;
     }
diff --git a/src/gui/rhi/qrhimetal_p_p.h b/src/gui/rhi/qrhimetal_p_p.h
index 212b731b71..a5af5611a6 100644
--- a/src/gui/rhi/qrhimetal_p_p.h
+++ b/src/gui/rhi/qrhimetal_p_p.h
@@ -437,6 +437,7 @@ public:
                              int layer, int level, const QRhiTextureSubresourceUploadDescription &subresDesc,
                              qsizetype *curOfs);
     void enqueueResourceUpdates(QRhiCommandBuffer *cb, QRhiResourceUpdateBatch *resourceUpdates);
+    void executeBufferHostWritesForSlot(QMetalBuffer *bufD, int slot);
     void executeBufferHostWritesForCurrentFrame(QMetalBuffer *bufD);
     static const int SUPPORTED_STAGES = 3;
     void enqueueShaderResourceBindings(QMetalShaderResourceBindings *srbD,
diff --git a/src/gui/rhi/qrhivulkan.cpp b/src/gui/rhi/qrhivulkan.cpp
index d378e2a4ad..8f6f118c9b 100644
--- a/src/gui/rhi/qrhivulkan.cpp
+++ b/src/gui/rhi/qrhivulkan.cpp
@@ -2903,7 +2903,7 @@ void QRhiVulkan::enqueueResourceUpdates(QVkCommandBuffer *cbD, QRhiResourceUpdat
         } else if (u.type == QRhiResourceUpdateBatchPrivate::BufferOp::Read) {
             QVkBuffer *bufD = QRHI_RES(QVkBuffer, u.buf);
             if (bufD->m_type == QRhiBuffer::Dynamic) {
-                executeBufferHostWritesForCurrentFrame(bufD);
+                executeBufferHostWritesForSlot(bufD, currentFrameSlot);
                 void *p = nullptr;
                 VmaAllocation a = toVmaAllocation(bufD->allocations[currentFrameSlot]);
                 VkResult err = vmaMapMemory(toVmaAllocator(allocator), a, &p);
@@ -3300,14 +3300,14 @@ void QRhiVulkan::enqueueResourceUpdates(QVkCommandBuffer *cbD, QRhiResourceUpdat
     ud->free();
 }
 
-void QRhiVulkan::executeBufferHostWritesForCurrentFrame(QVkBuffer *bufD)
+void QRhiVulkan::executeBufferHostWritesForSlot(QVkBuffer *bufD, int slot)
 {
-    if (bufD->pendingDynamicUpdates[currentFrameSlot].isEmpty())
+    if (bufD->pendingDynamicUpdates[slot].isEmpty())
         return;
 
     Q_ASSERT(bufD->m_type == QRhiBuffer::Dynamic);
     void *p = nullptr;
-    VmaAllocation a = toVmaAllocation(bufD->allocations[currentFrameSlot]);
+    VmaAllocation a = toVmaAllocation(bufD->allocations[slot]);
     // The vmaMap/Unmap are basically a no-op when persistently mapped since it
     // refcounts; this is great because we don't need to care if the allocation
     // was created as persistently mapped or not.
@@ -3318,7 +3318,7 @@ void QRhiVulkan::executeBufferHostWritesForCurrentFrame(QVkBuffer *bufD)
     }
     int changeBegin = -1;
     int changeEnd = -1;
-    for (const QRhiResourceUpdateBatchPrivate::BufferOp &u : qAsConst(bufD->pendingDynamicUpdates[currentFrameSlot])) {
+    for (const QRhiResourceUpdateBatchPrivate::BufferOp &u : qAsConst(bufD->pendingDynamicUpdates[slot])) {
         Q_ASSERT(bufD == QRHI_RES(QVkBuffer, u.buf));
         memcpy(static_cast<char *>(p) + u.offset, u.data.constData(), size_t(u.data.size()));
         if (changeBegin == -1 || u.offset < changeBegin)
@@ -3330,7 +3330,7 @@ void QRhiVulkan::executeBufferHostWritesForCurrentFrame(QVkBuffer *bufD)
     if (changeBegin >= 0)
         vmaFlushAllocation(toVmaAllocator(allocator), a, VkDeviceSize(changeBegin), VkDeviceSize(changeEnd - changeBegin));
 
-    bufD->pendingDynamicUpdates[currentFrameSlot].clear();
+    bufD->pendingDynamicUpdates[slot].clear();
 }
 
 static void qrhivk_releaseBuffer(const QRhiVulkan::DeferredReleaseEntry &e, void *allocator)
@@ -4166,7 +4166,7 @@ void QRhiVulkan::setShaderResources(QRhiCommandBuffer *cb, QRhiShaderResourceBin
             Q_ASSERT(bufD->m_usage.testFlag(QRhiBuffer::UniformBuffer));
 
             if (bufD->m_type == QRhiBuffer::Dynamic)
-                executeBufferHostWritesForCurrentFrame(bufD);
+                executeBufferHostWritesForSlot(bufD, currentFrameSlot);
 
             bufD->lastActiveFrameSlot = currentFrameSlot;
             trackedRegisterBuffer(&passResTracker, bufD, bufD->m_type == QRhiBuffer::Dynamic ? currentFrameSlot : 0,
@@ -4240,7 +4240,7 @@ void QRhiVulkan::setShaderResources(QRhiCommandBuffer *cb, QRhiShaderResourceBin
             Q_ASSERT(bufD->m_usage.testFlag(QRhiBuffer::StorageBuffer));
 
             if (bufD->m_type == QRhiBuffer::Dynamic)
-                executeBufferHostWritesForCurrentFrame(bufD);
+                executeBufferHostWritesForSlot(bufD, currentFrameSlot);
 
             bufD->lastActiveFrameSlot = currentFrameSlot;
             QRhiPassResourceTracker::BufferAccess access;
@@ -4349,7 +4349,7 @@ void QRhiVulkan::setVertexInput(QRhiCommandBuffer *cb,
         Q_ASSERT(bufD->m_usage.testFlag(QRhiBuffer::VertexBuffer));
         bufD->lastActiveFrameSlot = currentFrameSlot;
         if (bufD->m_type == QRhiBuffer::Dynamic)
-            executeBufferHostWritesForCurrentFrame(bufD);
+            executeBufferHostWritesForSlot(bufD, currentFrameSlot);
 
         const VkBuffer vkvertexbuf = bufD->buffers[bufD->m_type == QRhiBuffer::Dynamic ? currentFrameSlot : 0];
         if (cbD->currentVertexBuffers[inputSlot] != vkvertexbuf
@@ -4395,7 +4395,7 @@ void QRhiVulkan::setVertexInput(QRhiCommandBuffer *cb,
         Q_ASSERT(ibufD->m_usage.testFlag(QRhiBuffer::IndexBuffer));
         ibufD->lastActiveFrameSlot = currentFrameSlot;
         if (ibufD->m_type == QRhiBuffer::Dynamic)
-            executeBufferHostWritesForCurrentFrame(ibufD);
+            executeBufferHostWritesForSlot(ibufD, currentFrameSlot);
 
         const int slot = ibufD->m_type == QRhiBuffer::Dynamic ? currentFrameSlot : 0;
         const VkBuffer vkindexbuf = ibufD->buffers[slot];
@@ -5188,10 +5188,13 @@ bool QVkBuffer::build()
 QRhiBuffer::NativeBuffer QVkBuffer::nativeBuffer()
 {
     if (m_type == Dynamic) {
+        QRHI_RES_RHI(QRhiVulkan);
         NativeBuffer b;
         Q_ASSERT(sizeof(b.objects) / sizeof(b.objects[0]) >= size_t(QVK_FRAMES_IN_FLIGHT));
-        for (int i = 0; i < QVK_FRAMES_IN_FLIGHT; ++i)
+        for (int i = 0; i < QVK_FRAMES_IN_FLIGHT; ++i) {
+            rhiD->executeBufferHostWritesForSlot(this, i);
             b.objects[i] = &buffers[i];
+        }
         b.slotCount = QVK_FRAMES_IN_FLIGHT;
         return b;
     }
diff --git a/src/gui/rhi/qrhivulkan_p_p.h b/src/gui/rhi/qrhivulkan_p_p.h
index 6322882569..0f0a89cbff 100644
--- a/src/gui/rhi/qrhivulkan_p_p.h
+++ b/src/gui/rhi/qrhivulkan_p_p.h
@@ -778,7 +778,7 @@ public:
                              size_t *curOfs, void *mp,
                              BufferImageCopyList *copyInfos);
     void enqueueResourceUpdates(QVkCommandBuffer *cbD, QRhiResourceUpdateBatch *resourceUpdates);
-    void executeBufferHostWritesForCurrentFrame(QVkBuffer *bufD);
+    void executeBufferHostWritesForSlot(QVkBuffer *bufD, int slot);
     void enqueueTransitionPassResources(QVkCommandBuffer *cbD);
     void recordPrimaryCommandBuffer(QVkCommandBuffer *cbD);
     void trackedRegisterBuffer(QRhiPassResourceTracker *passResTracker,