whytofly
/
qt6-bb10
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

rhi: Attempt to fix up the logic around beginExternal() 

Mainly for Vulkan where it lacked the recording of the still queued
commands. Uncovered by Qt Quick examples that integrate custom Vulkan
rendering.

This still has an issue that needs to be tackled separately. (we probably
will switch to using a dedicated secondary command buffer with
RENDER_PASS_CONTINUE_BIT for the external commands, and then just have
a vkCmdExecuteCommands in our own queue instead of recording everything in
beginExternal).

The possibility of losing glMemoryBarrier() calls due to begin/endExternal()
with the OpenGL backend is fixed too. The logic here mirrors Vulkan
to some extent except that we do not have a concept of (and so the trouble
with) renderpass instances.

Clean up around the implementations of finish() as well. Attempting to share
code via a "flushCommandBuffer" function is admirable but is not worth it
since some semantics are different. (finish() cannot be called within a
begin/endPass, unlike begin/endExternal).

Change-Id: I5137db598d6a40d484e53678f5c919abf750d9ed
Reviewed-by: Andy Nichols <andy.nichols@qt.io>
bb10
Laszlo Agocs 2019-08-26 16:10:31 +02:00
parent 2d7fc7a152
commit 4e97c4061e
6 changed files with 102 additions and 67 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

34
src/gui/rhi/qrhid3d11.cpp
View File

@ -873,8 +873,10 @@ const QRhiNativeHandles *QRhiD3D11::nativeHandles(QRhiCommandBuffer *cb)
void QRhiD3D11::beginExternal(QRhiCommandBuffer *cb)
{
Q_UNUSED(cb);
flushCommandBuffer();
QD3D11CommandBuffer *cbD = QRHI_RES(QD3D11CommandBuffer, cb);
// no timestampSwapChain, in order to avoid timestamp mess
executeCommandBuffer(cbD);
cbD->resetCommands();
}
void QRhiD3D11::endExternal(QRhiCommandBuffer *cb)
@ -1135,27 +1137,25 @@ static inline bool isDepthTextureFormat(QRhiTexture::Format format)
QRhi::FrameOpResult QRhiD3D11::finish()
{
if (inFrame)
flushCommandBuffer();
if (inFrame) {
if (ofr.active) {
Q_ASSERT(!contextState.currentSwapChain);
Q_ASSERT(ofr.cbWrapper.recordingPass == QD3D11CommandBuffer::NoPass);
executeCommandBuffer(&ofr.cbWrapper);
ofr.cbWrapper.resetCommands();
} else {
Q_ASSERT(contextState.currentSwapChain);
Q_ASSERT(contextState.currentSwapChain->cb.recordingPass == QD3D11CommandBuffer::NoPass);
executeCommandBuffer(&contextState.currentSwapChain->cb); // no timestampSwapChain, in order to avoid timestamp mess
contextState.currentSwapChain->cb.resetCommands();
}
}
finishActiveReadbacks();
return QRhi::FrameOpSuccess;
}
void QRhiD3D11::flushCommandBuffer()
{
if (ofr.active) {
Q_ASSERT(!contextState.currentSwapChain);
executeCommandBuffer(&ofr.cbWrapper);
ofr.cbWrapper.resetCommands();
} else {
Q_ASSERT(contextState.currentSwapChain);
executeCommandBuffer(&contextState.currentSwapChain->cb); // no timestampSwapChain, in order to avoid timestamp mess
contextState.currentSwapChain->cb.resetCommands();
}
}
void QRhiD3D11::enqueueSubresUpload(QD3D11Texture *texD, QD3D11CommandBuffer *cbD,
int layer, int level, const QRhiTextureSubresourceUploadDescription &subresDesc)
{

3
src/gui/rhi/qrhid3d11_p_p.h
View File

@ -473,9 +473,9 @@ struct QD3D11CommandBuffer : public QRhiCommandBuffer
imageRetainPool.clear();
}
void resetState() {
resetCommands();
recordingPass = NoPass;
currentTarget = nullptr;
resetCommands();
resetCachedState();
}
void resetCachedState() {
@ -633,7 +633,6 @@ public:
void sendVMemStatsToProfiler() override;
void makeThreadLocalNativeContextCurrent() override;
void flushCommandBuffer();
void enqueueSubresUpload(QD3D11Texture *texD, QD3D11CommandBuffer *cbD,
int layer, int level, const QRhiTextureSubresourceUploadDescription &subresDesc);
void enqueueResourceUpdates(QRhiCommandBuffer *cb, QRhiResourceUpdateBatch *resourceUpdates);

84
src/gui/rhi/qrhigles2.cpp
View File

@ -1094,15 +1094,36 @@ const QRhiNativeHandles *QRhiGles2::nativeHandles(QRhiCommandBuffer *cb)
void QRhiGles2::beginExternal(QRhiCommandBuffer *cb)
{
Q_UNUSED(cb);
flushCommandBuffer(); // also ensures the context is current
if (ofr.active) {
Q_ASSERT(!currentSwapChain);
if (!ensureContext())
return;
} else {
Q_ASSERT(currentSwapChain);
if (!ensureContext(currentSwapChain->surface))
return;
}
QGles2CommandBuffer *cbD = QRHI_RES(QGles2CommandBuffer, cb);
executeCommandBuffer(cbD);
cbD->resetCommands();
}
void QRhiGles2::endExternal(QRhiCommandBuffer *cb)
{
QGles2CommandBuffer *cbD = QRHI_RES(QGles2CommandBuffer, cb);
Q_ASSERT(cbD->commands.isEmpty());
Q_ASSERT(cbD->commands.isEmpty() && cbD->currentPassResTrackerIndex == -1);
cbD->resetCachedState();
if (cbD->recordingPass != QGles2CommandBuffer::NoPass) {
// Commands that come after this point need a resource tracker and also
// a BarriersForPass command enqueued. (the ones we had from
// beginPass() are now gone since beginExternal() processed all that
// due to calling executeCommandBuffer()).
enqueueBarriersForPass(cbD);
}
if (cbD->currentTarget)
enqueueBindFramebuffer(cbD->currentTarget, cbD);
}
@ -1196,23 +1217,22 @@ QRhi::FrameOpResult QRhiGles2::endOffscreenFrame()
QRhi::FrameOpResult QRhiGles2::finish()
{
return inFrame ? flushCommandBuffer() : QRhi::FrameOpSuccess;
}
QRhi::FrameOpResult QRhiGles2::flushCommandBuffer()
{
if (ofr.active) {
Q_ASSERT(!currentSwapChain);
if (!ensureContext())
return QRhi::FrameOpError;
executeCommandBuffer(&ofr.cbWrapper);
ofr.cbWrapper.resetCommands();
} else {
Q_ASSERT(currentSwapChain);
if (!ensureContext(currentSwapChain->surface))
return QRhi::FrameOpError;
executeCommandBuffer(&currentSwapChain->cb);
currentSwapChain->cb.resetCommands();
if (inFrame) {
if (ofr.active) {
Q_ASSERT(!currentSwapChain);
Q_ASSERT(ofr.cbWrapper.recordingPass == QGles2CommandBuffer::NoPass);
if (!ensureContext())
return QRhi::FrameOpError;
executeCommandBuffer(&ofr.cbWrapper);
ofr.cbWrapper.resetCommands();
} else {
Q_ASSERT(currentSwapChain);
Q_ASSERT(currentSwapChain->cb.recordingPass == QGles2CommandBuffer::NoPass);
if (!ensureContext(currentSwapChain->surface))
return QRhi::FrameOpError;
executeCommandBuffer(&currentSwapChain->cb);
currentSwapChain->cb.resetCommands();
}
}
return QRhi::FrameOpSuccess;
}
@ -2483,6 +2503,16 @@ QGles2RenderTargetData *QRhiGles2::enqueueBindFramebuffer(QRhiRenderTarget *rt,
return rtD;
}
void QRhiGles2::enqueueBarriersForPass(QGles2CommandBuffer *cbD)
{
cbD->passResTrackers.append(QRhiPassResourceTracker());
cbD->currentPassResTrackerIndex = cbD->passResTrackers.count() - 1;
QGles2CommandBuffer::Command cmd;
cmd.cmd = QGles2CommandBuffer::Command::BarriersForPass;
cmd.args.barriersForPass.trackerIndex = cbD->currentPassResTrackerIndex;
cbD->commands.append(cmd);
}
void QRhiGles2::beginPass(QRhiCommandBuffer *cb,
QRhiRenderTarget *rt,
const QColor &colorClearValue,
@ -2497,12 +2527,7 @@ void QRhiGles2::beginPass(QRhiCommandBuffer *cb,
// Get a new resource tracker. Then add a command that will generate
// glMemoryBarrier() calls based on that tracker when submitted.
cbD->passResTrackers.append(QRhiPassResourceTracker());
cbD->currentPassResTrackerIndex = cbD->passResTrackers.count() - 1;
QGles2CommandBuffer::Command cmd;
cmd.cmd = QGles2CommandBuffer::Command::BarriersForPass;
cmd.args.barriersForPass.trackerIndex = cbD->currentPassResTrackerIndex;
cbD->commands.append(cmd);
enqueueBarriersForPass(cbD);
bool wantsColorClear, wantsDsClear;
QGles2RenderTargetData *rtD = enqueueBindFramebuffer(rt, cbD, &wantsColorClear, &wantsDsClear);
@ -2578,12 +2603,7 @@ void QRhiGles2::beginComputePass(QRhiCommandBuffer *cb, QRhiResourceUpdateBatch
if (resourceUpdates)
enqueueResourceUpdates(cb, resourceUpdates);
cbD->passResTrackers.append(QRhiPassResourceTracker());
cbD->currentPassResTrackerIndex = cbD->passResTrackers.count() - 1;
QGles2CommandBuffer::Command cmd;
cmd.cmd = QGles2CommandBuffer::Command::BarriersForPass;
cmd.args.barriersForPass.trackerIndex = cbD->currentPassResTrackerIndex;
cbD->commands.append(cmd);
enqueueBarriersForPass(cbD);
cbD->recordingPass = QGles2CommandBuffer::ComputePass;

13
src/gui/rhi/qrhigles2_p_p.h
View File

@ -525,19 +525,16 @@ struct QGles2CommandBuffer : public QRhiCommandBuffer
}
void resetCommands() {
commands.clear();
// beginExternal() can lead to calling resetCommands() inside a pass,
// hence the condition
if (recordingPass == NoPass) {
passResTrackers.clear();
currentPassResTrackerIndex = -1;
}
dataRetainPool.clear();
imageRetainPool.clear();
passResTrackers.clear();
currentPassResTrackerIndex = -1;
}
void resetState() {
resetCommands();
recordingPass = NoPass;
currentTarget = nullptr;
resetCommands();
resetCachedState();
}
void resetCachedState() {
@ -671,7 +668,6 @@ public:
bool ensureContext(QSurface *surface = nullptr) const;
void executeDeferredReleases();
QRhi::FrameOpResult flushCommandBuffer();
void trackedBufferBarrier(QGles2CommandBuffer *cbD, QGles2Buffer *bufD, QGles2Buffer::Access access);
void trackedImageBarrier(QGles2CommandBuffer *cbD, QGles2Texture *texD, QGles2Texture::Access access);
void enqueueSubresUpload(QGles2Texture *texD, QGles2CommandBuffer *cbD,
@ -692,6 +688,7 @@ public:
const uint *dynOfsPairs, int dynOfsCount);
QGles2RenderTargetData *enqueueBindFramebuffer(QRhiRenderTarget *rt, QGles2CommandBuffer *cbD,
bool *wantsColorClear = nullptr, bool *wantsDsClear = nullptr);
void enqueueBarriersForPass(QGles2CommandBuffer *cbD);
int effectiveSampleCount(int sampleCount) const;
bool compileShader(GLuint program, const QRhiShaderStage &shaderStage,
QShaderDescription *desc, int *glslVersionUsed);

30
src/gui/rhi/qrhivulkan.cpp
View File

@ -1868,12 +1868,16 @@ QRhi::FrameOpResult QRhiVulkan::finish()
VkCommandBuffer cb;
if (ofr.active) {
Q_ASSERT(!currentSwapChain);
Q_ASSERT(ofr.cbWrapper.recordingPass == QVkCommandBuffer::NoPass);
recordCommandBuffer(&ofr.cbWrapper);
ofr.cbWrapper.resetCommands();
cb = ofr.cbWrapper.cb;
} else {
Q_ASSERT(currentSwapChain);
Q_ASSERT(currentSwapChain->cbWrapper.recordingPass == QVkCommandBuffer::NoPass);
swapChainD = currentSwapChain;
recordCommandBuffer(&swapChainD->cbWrapper);
swapChainD->cbWrapper.resetCommands();
cb = swapChainD->cbWrapper.cb;
}
QRhi::FrameOpResult submitres = endAndSubmitCommandBuffer(cb, VK_NULL_HANDLE, nullptr, nullptr);
@ -3120,17 +3124,16 @@ VkSampleCountFlagBits QRhiVulkan::effectiveSampleCount(int sampleCount)
void QRhiVulkan::enqueueTransitionPassResources(QVkCommandBuffer *cbD)
{
cbD->passResTrackers.append(QRhiPassResourceTracker());
cbD->currentPassResTrackerIndex = cbD->passResTrackers.count() - 1;
QVkCommandBuffer::Command cmd;
cmd.cmd = QVkCommandBuffer::Command::TransitionPassResources;
cmd.args.transitionResources.trackerIndex = cbD->passResTrackers.count() - 1;
cbD->commands.append(cmd);
cbD->currentPassResTrackerIndex = cbD->passResTrackers.count() - 1;
}
void QRhiVulkan::recordCommandBuffer(QVkCommandBuffer *cbD)
{
Q_ASSERT(cbD->recordingPass == QVkCommandBuffer::NoPass);
for (QVkCommandBuffer::Command &cmd : cbD->commands) {
switch (cmd.cmd) {
case QVkCommandBuffer::Command::CopyBuffer:
@ -3244,8 +3247,6 @@ void QRhiVulkan::recordCommandBuffer(QVkCommandBuffer *cbD)
break;
}
}
cbD->resetCommands();
}
static inline VkAccessFlags toVkAccess(QRhiPassResourceTracker::BufferAccess access)
@ -4138,13 +4139,30 @@ const QRhiNativeHandles *QRhiVulkan::nativeHandles(QRhiCommandBuffer *cb)
void QRhiVulkan::beginExternal(QRhiCommandBuffer *cb)
{
Q_UNUSED(cb);
QVkCommandBuffer *cbD = QRHI_RES(QVkCommandBuffer, cb);
recordCommandBuffer(cbD);
cbD->resetCommands();
}
void QRhiVulkan::endExternal(QRhiCommandBuffer *cb)
{
QVkCommandBuffer *cbD = QRHI_RES(QVkCommandBuffer, cb);
Q_ASSERT(cbD->commands.isEmpty() && cbD->currentPassResTrackerIndex == -1);
cbD->resetCachedState();
// ### FIXME this is all broken (leads to barriers within a renderpass
// instance which (1) would need a self-dependency for the subpass and (2) is
// not what we want anyway since it then has a different sychronization scope).
//
// To be replaced with a secondary command buffer for the external content.
//
if (cbD->recordingPass != QVkCommandBuffer::NoPass) {
// Commands that come after this point need a resource tracker and the
// corresponding barriers. Note that we are inside a renderpass
// instance here and that needs a self-dependency for the subpass.
enqueueTransitionPassResources(cbD);
}
}
void QRhiVulkan::setObjectName(uint64_t object, VkDebugReportObjectTypeEXT type, const QByteArray &name, int slot)

5
src/gui/rhi/qrhivulkan_p_p.h
View File

@ -324,9 +324,9 @@ struct QVkCommandBuffer : public QRhiCommandBuffer
};
void resetState() {
resetCommands();
recordingPass = NoPass;
currentTarget = nullptr;
resetCommands();
resetCachedState();
}
@ -510,9 +510,10 @@ struct QVkCommandBuffer : public QRhiCommandBuffer
void resetCommands() {
commands.clear();
resetPools();
passResTrackers.clear();
currentPassResTrackerIndex = -1;
resetPools();
}
void resetPools() {