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>
This commit is contained in:
Laszlo Agocs 2020-02-23 15:05:44 +01:00
parent 7b8616859a
commit 753e4d82be
7 changed files with 43 additions and 30 deletions

View File

@ -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
*/

View File

@ -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 };
}

View File

@ -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);

View File

@ -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;
}

View File

@ -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,

View File

@ -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;
}

View File

@ -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,