From d6551fe125209f11445936a90ce2a4207a183917 Mon Sep 17 00:00:00 2001 From: Volker Hilsheimer Date: Tue, 27 Aug 2019 10:08:28 +0200 Subject: QTree/TableView: Don't emit clicked when releasing after double click MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If an item is double clicked, then without this fix, the views emit the clicked() signal when the mousebutton is released after the double click event. This is unexpected and wrong. QAbstractItemView keeps track of the item the mouse was pressed on, to verify that the release occurred on the same item before emitting clicked, and to make sure we have a valid pressed item before initiating drag'n'drop. By resetting d->pressedItem when a double click has been handled, we can prevent the emission of a clicked signal in the next release event. [ChangeLog][QtWidgets][QTreeView] Don't emit clicked signal after a doubleClicked signal. Fixes: QTBUG-77771 Change-Id: I05988e9e2222157f4216cebc40c22507e8d83b82 Reviewed-by: AndrĂ© de la Rocha Reviewed-by: Jesus Fernandez --- src/widgets/itemviews/qabstractitemview.cpp | 1 + src/widgets/itemviews/qtreeview.cpp | 1 + 2 files changed, 2 insertions(+) (limited to 'src') diff --git a/src/widgets/itemviews/qabstractitemview.cpp b/src/widgets/itemviews/qabstractitemview.cpp index 2ed5df46b8..089f398e71 100644 --- a/src/widgets/itemviews/qabstractitemview.cpp +++ b/src/widgets/itemviews/qabstractitemview.cpp @@ -1966,6 +1966,7 @@ void QAbstractItemView::mouseDoubleClickEvent(QMouseEvent *event) if ((event->button() == Qt::LeftButton) && !edit(persistent, DoubleClicked, event) && !style()->styleHint(QStyle::SH_ItemView_ActivateItemOnSingleClick, 0, this)) emit activated(persistent); + d->pressedIndex = QModelIndex(); } #if QT_CONFIG(draganddrop) diff --git a/src/widgets/itemviews/qtreeview.cpp b/src/widgets/itemviews/qtreeview.cpp index 823be1ca08..413cc2a9cd 100644 --- a/src/widgets/itemviews/qtreeview.cpp +++ b/src/widgets/itemviews/qtreeview.cpp @@ -1943,6 +1943,7 @@ void QTreeView::mouseDoubleClickEvent(QMouseEvent *event) if (!style()->styleHint(QStyle::SH_ItemView_ActivateItemOnSingleClick, 0, this)) emit activated(persistent); + d->pressedIndex = QModelIndex(); d->executePostedLayout(); // we need to make sure viewItems is updated if (d->itemsExpandable && d->expandsOnDoubleClick -- cgit v1.2.3 From 2d7fc7a1527211fb9e41310b3ea75ba6d61cee8d Mon Sep 17 00:00:00 2001 From: Laszlo Agocs Date: Mon, 26 Aug 2019 15:19:59 +0200 Subject: rhi: vulkan: Expose the VkRenderPass via the usual mechanisms Qt Quick in turn will expose it via QSGRendererInterface. Essential when adding custom Vulkan rendering into a Qt Quick application because the custom pipeline state objects will need to reference a VkRenderPass. Change-Id: Idf4092cfc3937830fb8123164081059b0d8d030e Reviewed-by: Andy Nichols --- src/gui/rhi/qrhi.cpp | 16 ++++++++++++++++ src/gui/rhi/qrhi_p.h | 2 ++ src/gui/rhi/qrhivulkan.cpp | 13 +++++++++++++ src/gui/rhi/qrhivulkan_p.h | 5 +++++ src/gui/rhi/qrhivulkan_p_p.h | 2 ++ 5 files changed, 38 insertions(+) (limited to 'src') diff --git a/src/gui/rhi/qrhi.cpp b/src/gui/rhi/qrhi.cpp index 1076b21c1e..0f4b3d675a 100644 --- a/src/gui/rhi/qrhi.cpp +++ b/src/gui/rhi/qrhi.cpp @@ -2246,6 +2246,10 @@ QRhiResource::Type QRhiSampler::resourceType() const \internal \inmodule QtGui \brief Render pass resource. + + A render pass, if such a concept exists in the underlying graphics API, is + a collection of attachments (color, depth, stencil) and describes how those + attachments are used. */ /*! @@ -2264,6 +2268,18 @@ QRhiResource::Type QRhiRenderPassDescriptor::resourceType() const return RenderPassDescriptor; } +/*! + \return a pointer to a backend-specific QRhiNativeHandles subclass, such as + QRhiVulkanRenderPassNativeHandles. The returned value is null when exposing + the underlying native resources is not supported by the backend. + + \sa QRhiVulkanRenderPassNativeHandles + */ +const QRhiNativeHandles *QRhiRenderPassDescriptor::nativeHandles() +{ + return nullptr; +} + /*! \class QRhiRenderTarget \internal diff --git a/src/gui/rhi/qrhi_p.h b/src/gui/rhi/qrhi_p.h index df30817ef4..51c60f1831 100644 --- a/src/gui/rhi/qrhi_p.h +++ b/src/gui/rhi/qrhi_p.h @@ -843,6 +843,8 @@ class Q_GUI_EXPORT QRhiRenderPassDescriptor : public QRhiResource public: QRhiResource::Type resourceType() const override; + virtual const QRhiNativeHandles *nativeHandles(); + protected: QRhiRenderPassDescriptor(QRhiImplementation *rhi); }; diff --git a/src/gui/rhi/qrhivulkan.cpp b/src/gui/rhi/qrhivulkan.cpp index 321fd92f88..a0f32252cf 100644 --- a/src/gui/rhi/qrhivulkan.cpp +++ b/src/gui/rhi/qrhivulkan.cpp @@ -182,6 +182,13 @@ QT_BEGIN_NAMESPACE \l{QRhi::endOffsrceenFrame()}{endOffscreenFrame()} pair. */ +/*! + \class QRhiVulkanRenderPassNativeHandles + \internal + \inmodule QtGui + \brief Holds the Vulkan render pass object backing a QRhiRenderPassDescriptor. + */ + static inline VkDeviceSize aligned(VkDeviceSize v, VkDeviceSize byteAlign) { return (v + byteAlign - 1) & ~(byteAlign - 1); @@ -5098,6 +5105,12 @@ void QVkRenderPassDescriptor::release() rhiD->unregisterResource(this); } +const QRhiNativeHandles *QVkRenderPassDescriptor::nativeHandles() +{ + nativeHandlesStruct.renderPass = rp; + return &nativeHandlesStruct; +} + QVkReferenceRenderTarget::QVkReferenceRenderTarget(QRhiImplementation *rhi) : QRhiRenderTarget(rhi) { diff --git a/src/gui/rhi/qrhivulkan_p.h b/src/gui/rhi/qrhivulkan_p.h index 545ef5ad72..ff19c7a54e 100644 --- a/src/gui/rhi/qrhivulkan_p.h +++ b/src/gui/rhi/qrhivulkan_p.h @@ -80,6 +80,11 @@ struct Q_GUI_EXPORT QRhiVulkanCommandBufferNativeHandles : public QRhiNativeHand VkCommandBuffer commandBuffer = VK_NULL_HANDLE; }; +struct Q_GUI_EXPORT QRhiVulkanRenderPassNativeHandles : public QRhiNativeHandles +{ + VkRenderPass renderPass = VK_NULL_HANDLE; +}; + QT_END_NAMESPACE #endif diff --git a/src/gui/rhi/qrhivulkan_p_p.h b/src/gui/rhi/qrhivulkan_p_p.h index 31e0eaa585..e2ea8fec2a 100644 --- a/src/gui/rhi/qrhivulkan_p_p.h +++ b/src/gui/rhi/qrhivulkan_p_p.h @@ -171,9 +171,11 @@ struct QVkRenderPassDescriptor : public QRhiRenderPassDescriptor QVkRenderPassDescriptor(QRhiImplementation *rhi); ~QVkRenderPassDescriptor(); void release() override; + const QRhiNativeHandles *nativeHandles() override; VkRenderPass rp = VK_NULL_HANDLE; bool ownsRp = false; + QRhiVulkanRenderPassNativeHandles nativeHandlesStruct; int lastActiveFrameSlot = -1; }; -- cgit v1.2.3 From 4e97c4061e0ac7d885e62cfaf9ab7eb1918906f1 Mon Sep 17 00:00:00 2001 From: Laszlo Agocs Date: Mon, 26 Aug 2019 16:10:31 +0200 Subject: 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 --- src/gui/rhi/qrhid3d11.cpp | 34 +++++++++--------- src/gui/rhi/qrhid3d11_p_p.h | 3 +- src/gui/rhi/qrhigles2.cpp | 84 +++++++++++++++++++++++++++----------------- src/gui/rhi/qrhigles2_p_p.h | 13 +++---- src/gui/rhi/qrhivulkan.cpp | 30 ++++++++++++---- src/gui/rhi/qrhivulkan_p_p.h | 5 +-- 6 files changed, 102 insertions(+), 67 deletions(-) (limited to 'src') diff --git a/src/gui/rhi/qrhid3d11.cpp b/src/gui/rhi/qrhid3d11.cpp index 9b411c5021..e978536dd7 100644 --- a/src/gui/rhi/qrhid3d11.cpp +++ b/src/gui/rhi/qrhid3d11.cpp @@ -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) { diff --git a/src/gui/rhi/qrhid3d11_p_p.h b/src/gui/rhi/qrhid3d11_p_p.h index 3ceb055b27..096c192183 100644 --- a/src/gui/rhi/qrhid3d11_p_p.h +++ b/src/gui/rhi/qrhid3d11_p_p.h @@ -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); diff --git a/src/gui/rhi/qrhigles2.cpp b/src/gui/rhi/qrhigles2.cpp index 26e7fa2759..2cc489ea8b 100644 --- a/src/gui/rhi/qrhigles2.cpp +++ b/src/gui/rhi/qrhigles2.cpp @@ -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(¤tSwapChain->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(¤tSwapChain->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; diff --git a/src/gui/rhi/qrhigles2_p_p.h b/src/gui/rhi/qrhigles2_p_p.h index 462ce8032c..3602d62122 100644 --- a/src/gui/rhi/qrhigles2_p_p.h +++ b/src/gui/rhi/qrhigles2_p_p.h @@ -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); diff --git a/src/gui/rhi/qrhivulkan.cpp b/src/gui/rhi/qrhivulkan.cpp index a0f32252cf..0381def9f1 100644 --- a/src/gui/rhi/qrhivulkan.cpp +++ b/src/gui/rhi/qrhivulkan.cpp @@ -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) diff --git a/src/gui/rhi/qrhivulkan_p_p.h b/src/gui/rhi/qrhivulkan_p_p.h index e2ea8fec2a..dd9a7d4216 100644 --- a/src/gui/rhi/qrhivulkan_p_p.h +++ b/src/gui/rhi/qrhivulkan_p_p.h @@ -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() { -- cgit v1.2.3 From 0077e0f88cd3dfa59779ef121e4010f309f2ff7a Mon Sep 17 00:00:00 2001 From: Laszlo Agocs Date: Tue, 27 Aug 2019 11:05:48 +0200 Subject: rhi: vulkan: Add missing VK_QUERY_RESULT_WAIT_BIT Mainly to get the validation layer from newer Vulkan SDKs to shut up. Change-Id: I3a00d2e7b5617eb1656625b1b2a919bb3c07feb9 Reviewed-by: Andy Nichols --- src/gui/rhi/qrhivulkan.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/gui/rhi/qrhivulkan.cpp b/src/gui/rhi/qrhivulkan.cpp index 0381def9f1..7806afc4fe 100644 --- a/src/gui/rhi/qrhivulkan.cpp +++ b/src/gui/rhi/qrhivulkan.cpp @@ -1525,7 +1525,8 @@ QRhi::FrameOpResult QRhiVulkan::beginFrame(QRhiSwapChain *swapChain, QRhi::Begin if (frame.timestampQueryIndex >= 0) { quint64 timestamp[2] = { 0, 0 }; VkResult err = df->vkGetQueryPoolResults(dev, timestampQueryPool, frame.timestampQueryIndex, 2, - 2 * sizeof(quint64), timestamp, sizeof(quint64), VK_QUERY_RESULT_64_BIT); + 2 * sizeof(quint64), timestamp, sizeof(quint64), + VK_QUERY_RESULT_64_BIT | VK_QUERY_RESULT_WAIT_BIT); timestampQueryPoolMap.clearBit(frame.timestampQueryIndex / 2); frame.timestampQueryIndex = -1; if (err == VK_SUCCESS) { -- cgit v1.2.3 From 2602209f545ea3873f0dc880c258669a508d283a Mon Sep 17 00:00:00 2001 From: Laszlo Agocs Date: Tue, 27 Aug 2019 12:49:38 +0200 Subject: rhi: vulkan: Introduce secondary command buffer usage As an option. Must opt in via setting ExternalContentsInPass in the flags for beginFrame(). It is somewhat unfortunate to require declaring this up front, but forcing using secondary command buffers always, even though beginExternal() may not be used in many applications, would be an overkill. Change-Id: I8d52bcab40c96f89f140c4c7877b6c459925e3c7 Reviewed-by: Andy Nichols --- src/gui/rhi/qrhi.cpp | 46 +++- src/gui/rhi/qrhi_p.h | 5 +- src/gui/rhi/qrhi_p_p.h | 4 +- src/gui/rhi/qrhid3d11.cpp | 6 +- src/gui/rhi/qrhid3d11_p_p.h | 4 +- src/gui/rhi/qrhigles2.cpp | 6 +- src/gui/rhi/qrhigles2_p_p.h | 4 +- src/gui/rhi/qrhimetal.mm | 7 +- src/gui/rhi/qrhimetal_p_p.h | 4 +- src/gui/rhi/qrhinull.cpp | 6 +- src/gui/rhi/qrhinull_p_p.h | 4 +- src/gui/rhi/qrhivulkan.cpp | 510 ++++++++++++++++++++++++++++++++----------- src/gui/rhi/qrhivulkan_p_p.h | 50 +++-- 13 files changed, 484 insertions(+), 172 deletions(-) (limited to 'src') diff --git a/src/gui/rhi/qrhi.cpp b/src/gui/rhi/qrhi.cpp index 0f4b3d675a..4414b61d55 100644 --- a/src/gui/rhi/qrhi.cpp +++ b/src/gui/rhi/qrhi.cpp @@ -275,6 +275,8 @@ Q_LOGGING_CATEGORY(QRHI_LOG_INFO, "qt.rhi.general") QRhi does not expose APIs for resource barriers or image layout transitions. Such synchronization is done implicitly by the backends, where applicable (for example, Vulkan), by tracking resource usage as necessary. + Buffer and image barriers are inserted before render or compute passes + transparently to the application. \note Resources within a render or compute pass are expected to be bound to a single usage during that pass. For example, a buffer can be used as @@ -554,6 +556,11 @@ Q_LOGGING_CATEGORY(QRHI_LOG_INFO, "qt.rhi.general") /*! \enum QRhi::BeginFrameFlag Flag values for QRhi::beginFrame() + + \value ExternalContentsInPass Specifies that one or more render or compute + passes in this frame will call QRhiCommandBuffer::beginExternal(). Some + backends, Vulkan in particular, will fail if this flag is not set and + beginExternal() is still called. */ /*! @@ -4801,6 +4808,11 @@ const QRhiNativeHandles *QRhiCommandBuffer::nativeHandles() enqueue commands to the current pass' command buffer by calling graphics API functions directly. + \note This is only available when the intent was declared up front in + beginFrame(). Therefore this function must only be called when the frame + was started with specifying QRhi::ExternalContentsInPass in the flags + passed to QRhi::beginFrame(). + With Vulkan or Metal one can query the native command buffer or encoder objects via nativeHandles() and enqueue commands to them. With OpenGL or Direct3D 11 the (device) context can be retrieved from @@ -4818,6 +4830,16 @@ const QRhiNativeHandles *QRhiCommandBuffer::nativeHandles() functions (\c set* or \c draw*) must be called on the QRhiCommandBuffer until endExternal(). + \warning Some backends may return a native command buffer object from + QRhiCommandBuffer::nativeHandles() that is different from the primary one + when inside a beginExternal() - endExternal() block. Therefore it is + important to (re)query the native command buffer object after calling + beginExternal(). In practical terms this means that with Vulkan for example + the externally recorded Vulkan commands are placed onto a secondary command + buffer (with VK_COMMAND_BUFFER_USAGE_RENDER_PASS_CONTINUE_BIT). + nativeHandles() returns this secondary command buffer when called between + begin/endExternal. + \sa endExternal(), nativeHandles() */ void QRhiCommandBuffer::beginExternal() @@ -5124,6 +5146,13 @@ QRhiSwapChain *QRhi::newSwapChain() /*! Starts a new frame targeting the next available buffer of \a swapChain. + A frame consists of resource updates and one or more render and compute + passes. + + \a flags can indicate certain special cases. For example, the fact that + QRhiCommandBuffer::beginExternal() will be called within this new frame + must be declared up front by setting the ExternalContentsInPass flag. + The high level pattern of rendering into a QWindow using a swapchain: \list @@ -5151,9 +5180,7 @@ QRhiSwapChain *QRhi::newSwapChain() \endlist - \a flags is currently unused. - - \sa endFrame() + \sa endFrame(), beginOffscreenFrame() */ QRhi::FrameOpResult QRhi::beginFrame(QRhiSwapChain *swapChain, BeginFrameFlags flags) { @@ -5253,7 +5280,8 @@ int QRhi::currentFrameSlot() const /*! Starts a new offscreen frame. Provides a command buffer suitable for - recording rendering commands in \a cb. + recording rendering commands in \a cb. \a flags is used to indicate + certain special cases, just like with beginFrame(). \note The QRhiCommandBuffer stored to *cb is not owned by the caller. @@ -5287,14 +5315,14 @@ int QRhi::currentFrameSlot() const // image data available in rbResult \endcode - \sa endOffscreenFrame() + \sa endOffscreenFrame(), beginFrame() */ -QRhi::FrameOpResult QRhi::beginOffscreenFrame(QRhiCommandBuffer **cb) +QRhi::FrameOpResult QRhi::beginOffscreenFrame(QRhiCommandBuffer **cb, BeginFrameFlags flags) { if (d->inFrame) qWarning("Attempted to call beginOffscreenFrame() within a still active frame; ignored"); - QRhi::FrameOpResult r = !d->inFrame ? d->beginOffscreenFrame(cb) : FrameOpSuccess; + QRhi::FrameOpResult r = !d->inFrame ? d->beginOffscreenFrame(cb, flags) : FrameOpSuccess; if (r == FrameOpSuccess) d->inFrame = true; @@ -5306,12 +5334,12 @@ QRhi::FrameOpResult QRhi::beginOffscreenFrame(QRhiCommandBuffer **cb) \sa beginOffscreenFrame() */ -QRhi::FrameOpResult QRhi::endOffscreenFrame() +QRhi::FrameOpResult QRhi::endOffscreenFrame(EndFrameFlags flags) { if (!d->inFrame) qWarning("Attempted to call endOffscreenFrame() without an active frame; ignored"); - QRhi::FrameOpResult r = d->inFrame ? d->endOffscreenFrame() : FrameOpSuccess; + QRhi::FrameOpResult r = d->inFrame ? d->endOffscreenFrame(flags) : FrameOpSuccess; d->inFrame = false; qDeleteAll(d->pendingReleaseAndDestroyResources); d->pendingReleaseAndDestroyResources.clear(); diff --git a/src/gui/rhi/qrhi_p.h b/src/gui/rhi/qrhi_p.h index 51c60f1831..2d36c19e99 100644 --- a/src/gui/rhi/qrhi_p.h +++ b/src/gui/rhi/qrhi_p.h @@ -1326,6 +1326,7 @@ public: }; enum BeginFrameFlag { + ExternalContentsInPass = 0x01 }; Q_DECLARE_FLAGS(BeginFrameFlags, BeginFrameFlag) @@ -1386,8 +1387,8 @@ public: bool isRecordingFrame() const; int currentFrameSlot() const; - FrameOpResult beginOffscreenFrame(QRhiCommandBuffer **cb); - FrameOpResult endOffscreenFrame(); + FrameOpResult beginOffscreenFrame(QRhiCommandBuffer **cb, BeginFrameFlags flags = BeginFrameFlags()); + FrameOpResult endOffscreenFrame(EndFrameFlags flags = EndFrameFlags()); QRhi::FrameOpResult finish(); diff --git a/src/gui/rhi/qrhi_p_p.h b/src/gui/rhi/qrhi_p_p.h index b592fe82f2..0914cf268b 100644 --- a/src/gui/rhi/qrhi_p_p.h +++ b/src/gui/rhi/qrhi_p_p.h @@ -95,8 +95,8 @@ public: virtual QRhiSwapChain *createSwapChain() = 0; virtual QRhi::FrameOpResult beginFrame(QRhiSwapChain *swapChain, QRhi::BeginFrameFlags flags) = 0; virtual QRhi::FrameOpResult endFrame(QRhiSwapChain *swapChain, QRhi::EndFrameFlags flags) = 0; - virtual QRhi::FrameOpResult beginOffscreenFrame(QRhiCommandBuffer **cb) = 0; - virtual QRhi::FrameOpResult endOffscreenFrame() = 0; + virtual QRhi::FrameOpResult beginOffscreenFrame(QRhiCommandBuffer **cb, QRhi::BeginFrameFlags flags) = 0; + virtual QRhi::FrameOpResult endOffscreenFrame(QRhi::EndFrameFlags flags) = 0; virtual QRhi::FrameOpResult finish() = 0; virtual void resourceUpdate(QRhiCommandBuffer *cb, QRhiResourceUpdateBatch *resourceUpdates) = 0; diff --git a/src/gui/rhi/qrhid3d11.cpp b/src/gui/rhi/qrhid3d11.cpp index e978536dd7..3e136cdb80 100644 --- a/src/gui/rhi/qrhid3d11.cpp +++ b/src/gui/rhi/qrhid3d11.cpp @@ -991,8 +991,9 @@ QRhi::FrameOpResult QRhiD3D11::endFrame(QRhiSwapChain *swapChain, QRhi::EndFrame return QRhi::FrameOpSuccess; } -QRhi::FrameOpResult QRhiD3D11::beginOffscreenFrame(QRhiCommandBuffer **cb) +QRhi::FrameOpResult QRhiD3D11::beginOffscreenFrame(QRhiCommandBuffer **cb, QRhi::BeginFrameFlags flags) { + Q_UNUSED(flags); ofr.active = true; ofr.cbWrapper.resetState(); @@ -1001,8 +1002,9 @@ QRhi::FrameOpResult QRhiD3D11::beginOffscreenFrame(QRhiCommandBuffer **cb) return QRhi::FrameOpSuccess; } -QRhi::FrameOpResult QRhiD3D11::endOffscreenFrame() +QRhi::FrameOpResult QRhiD3D11::endOffscreenFrame(QRhi::EndFrameFlags flags) { + Q_UNUSED(flags); ofr.active = false; executeCommandBuffer(&ofr.cbWrapper); diff --git a/src/gui/rhi/qrhid3d11_p_p.h b/src/gui/rhi/qrhid3d11_p_p.h index 096c192183..582146315d 100644 --- a/src/gui/rhi/qrhid3d11_p_p.h +++ b/src/gui/rhi/qrhid3d11_p_p.h @@ -569,8 +569,8 @@ public: QRhiSwapChain *createSwapChain() override; QRhi::FrameOpResult beginFrame(QRhiSwapChain *swapChain, QRhi::BeginFrameFlags flags) override; QRhi::FrameOpResult endFrame(QRhiSwapChain *swapChain, QRhi::EndFrameFlags flags) override; - QRhi::FrameOpResult beginOffscreenFrame(QRhiCommandBuffer **cb) override; - QRhi::FrameOpResult endOffscreenFrame() override; + QRhi::FrameOpResult beginOffscreenFrame(QRhiCommandBuffer **cb, QRhi::BeginFrameFlags flags) override; + QRhi::FrameOpResult endOffscreenFrame(QRhi::EndFrameFlags flags) override; QRhi::FrameOpResult finish() override; void resourceUpdate(QRhiCommandBuffer *cb, QRhiResourceUpdateBatch *resourceUpdates) override; diff --git a/src/gui/rhi/qrhigles2.cpp b/src/gui/rhi/qrhigles2.cpp index 2cc489ea8b..f4e711e33e 100644 --- a/src/gui/rhi/qrhigles2.cpp +++ b/src/gui/rhi/qrhigles2.cpp @@ -1184,8 +1184,9 @@ QRhi::FrameOpResult QRhiGles2::endFrame(QRhiSwapChain *swapChain, QRhi::EndFrame return QRhi::FrameOpSuccess; } -QRhi::FrameOpResult QRhiGles2::beginOffscreenFrame(QRhiCommandBuffer **cb) +QRhi::FrameOpResult QRhiGles2::beginOffscreenFrame(QRhiCommandBuffer **cb, QRhi::BeginFrameFlags flags) { + Q_UNUSED(flags); if (!ensureContext()) return QRhi::FrameOpError; @@ -1200,8 +1201,9 @@ QRhi::FrameOpResult QRhiGles2::beginOffscreenFrame(QRhiCommandBuffer **cb) return QRhi::FrameOpSuccess; } -QRhi::FrameOpResult QRhiGles2::endOffscreenFrame() +QRhi::FrameOpResult QRhiGles2::endOffscreenFrame(QRhi::EndFrameFlags flags) { + Q_UNUSED(flags); Q_ASSERT(ofr.active); ofr.active = false; diff --git a/src/gui/rhi/qrhigles2_p_p.h b/src/gui/rhi/qrhigles2_p_p.h index 3602d62122..6da529be92 100644 --- a/src/gui/rhi/qrhigles2_p_p.h +++ b/src/gui/rhi/qrhigles2_p_p.h @@ -602,8 +602,8 @@ public: QRhiSwapChain *createSwapChain() override; QRhi::FrameOpResult beginFrame(QRhiSwapChain *swapChain, QRhi::BeginFrameFlags flags) override; QRhi::FrameOpResult endFrame(QRhiSwapChain *swapChain, QRhi::EndFrameFlags flags) override; - QRhi::FrameOpResult beginOffscreenFrame(QRhiCommandBuffer **cb) override; - QRhi::FrameOpResult endOffscreenFrame() override; + QRhi::FrameOpResult beginOffscreenFrame(QRhiCommandBuffer **cb, QRhi::BeginFrameFlags flags) override; + QRhi::FrameOpResult endOffscreenFrame(QRhi::EndFrameFlags flags) override; QRhi::FrameOpResult finish() override; void resourceUpdate(QRhiCommandBuffer *cb, QRhiResourceUpdateBatch *resourceUpdates) override; diff --git a/src/gui/rhi/qrhimetal.mm b/src/gui/rhi/qrhimetal.mm index b7cc1da3fe..07753c985c 100644 --- a/src/gui/rhi/qrhimetal.mm +++ b/src/gui/rhi/qrhimetal.mm @@ -1243,8 +1243,10 @@ QRhi::FrameOpResult QRhiMetal::endFrame(QRhiSwapChain *swapChain, QRhi::EndFrame return QRhi::FrameOpSuccess; } -QRhi::FrameOpResult QRhiMetal::beginOffscreenFrame(QRhiCommandBuffer **cb) +QRhi::FrameOpResult QRhiMetal::beginOffscreenFrame(QRhiCommandBuffer **cb, QRhi::BeginFrameFlags flags) { + Q_UNUSED(flags); + currentFrameSlot = (currentFrameSlot + 1) % QMTL_FRAMES_IN_FLIGHT; if (swapchains.count() > 1) { for (QMetalSwapChain *sc : qAsConst(swapchains)) { @@ -1268,8 +1270,9 @@ QRhi::FrameOpResult QRhiMetal::beginOffscreenFrame(QRhiCommandBuffer **cb) return QRhi::FrameOpSuccess; } -QRhi::FrameOpResult QRhiMetal::endOffscreenFrame() +QRhi::FrameOpResult QRhiMetal::endOffscreenFrame(QRhi::EndFrameFlags flags) { + Q_UNUSED(flags); Q_ASSERT(d->ofr.active); d->ofr.active = false; diff --git a/src/gui/rhi/qrhimetal_p_p.h b/src/gui/rhi/qrhimetal_p_p.h index 8b0256991d..c448865f4d 100644 --- a/src/gui/rhi/qrhimetal_p_p.h +++ b/src/gui/rhi/qrhimetal_p_p.h @@ -354,8 +354,8 @@ public: QRhiSwapChain *createSwapChain() override; QRhi::FrameOpResult beginFrame(QRhiSwapChain *swapChain, QRhi::BeginFrameFlags flags) override; QRhi::FrameOpResult endFrame(QRhiSwapChain *swapChain, QRhi::EndFrameFlags flags) override; - QRhi::FrameOpResult beginOffscreenFrame(QRhiCommandBuffer **cb) override; - QRhi::FrameOpResult endOffscreenFrame() override; + QRhi::FrameOpResult beginOffscreenFrame(QRhiCommandBuffer **cb, QRhi::BeginFrameFlags flags) override; + QRhi::FrameOpResult endOffscreenFrame(QRhi::EndFrameFlags flags) override; QRhi::FrameOpResult finish() override; void resourceUpdate(QRhiCommandBuffer *cb, QRhiResourceUpdateBatch *resourceUpdates) override; diff --git a/src/gui/rhi/qrhinull.cpp b/src/gui/rhi/qrhinull.cpp index 048dce0dde..dff6e05268 100644 --- a/src/gui/rhi/qrhinull.cpp +++ b/src/gui/rhi/qrhinull.cpp @@ -356,14 +356,16 @@ QRhi::FrameOpResult QRhiNull::endFrame(QRhiSwapChain *swapChain, QRhi::EndFrameF return QRhi::FrameOpSuccess; } -QRhi::FrameOpResult QRhiNull::beginOffscreenFrame(QRhiCommandBuffer **cb) +QRhi::FrameOpResult QRhiNull::beginOffscreenFrame(QRhiCommandBuffer **cb, QRhi::BeginFrameFlags flags) { + Q_UNUSED(flags); *cb = &offscreenCommandBuffer; return QRhi::FrameOpSuccess; } -QRhi::FrameOpResult QRhiNull::endOffscreenFrame() +QRhi::FrameOpResult QRhiNull::endOffscreenFrame(QRhi::EndFrameFlags flags) { + Q_UNUSED(flags); return QRhi::FrameOpSuccess; } diff --git a/src/gui/rhi/qrhinull_p_p.h b/src/gui/rhi/qrhinull_p_p.h index b0227bc110..bdf0d59724 100644 --- a/src/gui/rhi/qrhinull_p_p.h +++ b/src/gui/rhi/qrhinull_p_p.h @@ -220,8 +220,8 @@ public: QRhiSwapChain *createSwapChain() override; QRhi::FrameOpResult beginFrame(QRhiSwapChain *swapChain, QRhi::BeginFrameFlags flags) override; QRhi::FrameOpResult endFrame(QRhiSwapChain *swapChain, QRhi::EndFrameFlags flags) override; - QRhi::FrameOpResult beginOffscreenFrame(QRhiCommandBuffer **cb) override; - QRhi::FrameOpResult endOffscreenFrame() override; + QRhi::FrameOpResult beginOffscreenFrame(QRhiCommandBuffer **cb, QRhi::BeginFrameFlags flags) override; + QRhi::FrameOpResult endOffscreenFrame(QRhi::EndFrameFlags flags) override; QRhi::FrameOpResult finish() override; void resourceUpdate(QRhiCommandBuffer *cb, QRhiResourceUpdateBatch *resourceUpdates) override; diff --git a/src/gui/rhi/qrhivulkan.cpp b/src/gui/rhi/qrhivulkan.cpp index 7806afc4fe..dfc85fb853 100644 --- a/src/gui/rhi/qrhivulkan.cpp +++ b/src/gui/rhi/qrhivulkan.cpp @@ -58,6 +58,28 @@ QT_BEGIN_NAMESPACE and a separate, host visible staging buffer is used to upload data to them. "Dynamic" buffers are in host visible memory and are duplicated (since there can be 2 frames in flight). This is handled transparently to the application. + + Barriers are generated automatically for each render or compute pass, based + on the resources that are used in that pass (in QRhiShaderResourceBindings, + vertex inputs, etc.). This implies deferring the recording of the command + buffer since the barriers have to be placed at the right place (before the + pass), and that can only be done once we know all the things the pass does. + + This in turn has implications for integrating external commands + (beginExternal() - direct Vulkan calls - endExternal()) because that is + incompatible with this approach by nature. Therefore we support another mode + of operation, where each render or compute pass uses one or more secondary + command buffers (recorded right away), with each beginExternal() leading to + closing the current secondary cb, creating a new secondary cb for the + external content, and then starting yet another one in endExternal() for + whatever comes afterwards in the pass. This way the primary command buffer + only has vkCmdExecuteCommand(s) within a renderpass instance + (Begin-EndRenderPass). (i.e. our only subpass is then + VK_SUBPASS_CONTENTS_SECONDARY_COMMAND_BUFFERS instead of + VK_SUBPASS_CONTENTS_INLINE) + + The command buffer management mode is decided on a per frame basis, + controlled by the ExternalContentsInPass flag of beginFrame(). */ /*! @@ -1476,7 +1498,6 @@ static inline bool checkDeviceLost(VkResult err) QRhi::FrameOpResult QRhiVulkan::beginFrame(QRhiSwapChain *swapChain, QRhi::BeginFrameFlags flags) { - Q_UNUSED(flags); QVkSwapChain *swapChainD = QRHI_RES(QVkSwapChain, swapChain); QVkSwapChain::FrameResources &frame(swapChainD->frameRes[swapChainD->currentFrameSlot]); QRhiProfilerPrivate *rhiP = profilerPrivateOrNull(); @@ -1548,7 +1569,7 @@ QRhi::FrameOpResult QRhiVulkan::beginFrame(QRhiSwapChain *swapChain, QRhi::Begin } // build new draw command buffer - QRhi::FrameOpResult cbres = startCommandBuffer(&frame.cmdBuf); + QRhi::FrameOpResult cbres = startPrimaryCommandBuffer(&frame.cmdBuf); if (cbres != QRhi::FrameOpSuccess) return cbres; @@ -1572,6 +1593,8 @@ QRhi::FrameOpResult QRhiVulkan::beginFrame(QRhiSwapChain *swapChain, QRhi::Begin } swapChainD->cbWrapper.cb = frame.cmdBuf; + swapChainD->cbWrapper.useSecondaryCb = flags.testFlag(QRhi::ExternalContentsInPass); + QVkSwapChain::ImageResources &image(swapChainD->imageRes[swapChainD->currentImageIndex]); swapChainD->rtWrapper.d.fb = image.fb; @@ -1592,7 +1615,7 @@ QRhi::FrameOpResult QRhiVulkan::endFrame(QRhiSwapChain *swapChain, QRhi::EndFram QVkSwapChain *swapChainD = QRHI_RES(QVkSwapChain, swapChain); Q_ASSERT(currentSwapChain == swapChainD); - recordCommandBuffer(&swapChainD->cbWrapper); + recordPrimaryCommandBuffer(&swapChainD->cbWrapper); QVkSwapChain::FrameResources &frame(swapChainD->frameRes[swapChainD->currentFrameSlot]); QVkSwapChain::ImageResources &image(swapChainD->imageRes[swapChainD->currentImageIndex]); @@ -1636,10 +1659,10 @@ QRhi::FrameOpResult QRhiVulkan::endFrame(QRhiSwapChain *swapChain, QRhi::EndFram // stop recording and submit to the queue Q_ASSERT(!frame.cmdFenceWaitable); const bool needsPresent = !flags.testFlag(QRhi::SkipPresent); - QRhi::FrameOpResult submitres = endAndSubmitCommandBuffer(frame.cmdBuf, - frame.cmdFence, - frame.imageSemWaitable ? &frame.imageSem : nullptr, - needsPresent ? &frame.drawSem : nullptr); + QRhi::FrameOpResult submitres = endAndSubmitPrimaryCommandBuffer(frame.cmdBuf, + frame.cmdFence, + frame.imageSemWaitable ? &frame.imageSem : nullptr, + needsPresent ? &frame.drawSem : nullptr); if (submitres != QRhi::FrameOpSuccess) return submitres; @@ -1710,7 +1733,7 @@ void QRhiVulkan::prepareNewFrame(QRhiCommandBuffer *cb) finishActiveReadbacks(); // last, in case the readback-completed callback issues rhi calls } -QRhi::FrameOpResult QRhiVulkan::startCommandBuffer(VkCommandBuffer *cb) +QRhi::FrameOpResult QRhiVulkan::startPrimaryCommandBuffer(VkCommandBuffer *cb) { if (*cb) { df->vkFreeCommandBuffers(dev, cmdPool, 1, cb); @@ -1749,8 +1772,8 @@ QRhi::FrameOpResult QRhiVulkan::startCommandBuffer(VkCommandBuffer *cb) return QRhi::FrameOpSuccess; } -QRhi::FrameOpResult QRhiVulkan::endAndSubmitCommandBuffer(VkCommandBuffer cb, VkFence cmdFence, - VkSemaphore *waitSem, VkSemaphore *signalSem) +QRhi::FrameOpResult QRhiVulkan::endAndSubmitPrimaryCommandBuffer(VkCommandBuffer cb, VkFence cmdFence, + VkSemaphore *waitSem, VkSemaphore *signalSem) { VkResult err = df->vkEndCommandBuffer(cb); if (err != VK_SUCCESS) { @@ -1801,9 +1824,9 @@ void QRhiVulkan::waitCommandCompletion(int frameSlot) } } -QRhi::FrameOpResult QRhiVulkan::beginOffscreenFrame(QRhiCommandBuffer **cb) +QRhi::FrameOpResult QRhiVulkan::beginOffscreenFrame(QRhiCommandBuffer **cb, QRhi::BeginFrameFlags flags) { - QRhi::FrameOpResult cbres = startCommandBuffer(&ofr.cbWrapper.cb); + QRhi::FrameOpResult cbres = startPrimaryCommandBuffer(&ofr.cbWrapper.cb); if (cbres != QRhi::FrameOpSuccess) return cbres; @@ -1820,6 +1843,8 @@ QRhi::FrameOpResult QRhiVulkan::beginOffscreenFrame(QRhiCommandBuffer **cb) if (swapchains.count() > 1) waitCommandCompletion(currentFrameSlot); + ofr.cbWrapper.useSecondaryCb = flags.testFlag(QRhi::ExternalContentsInPass); + prepareNewFrame(&ofr.cbWrapper); ofr.active = true; @@ -1827,12 +1852,13 @@ QRhi::FrameOpResult QRhiVulkan::beginOffscreenFrame(QRhiCommandBuffer **cb) return QRhi::FrameOpSuccess; } -QRhi::FrameOpResult QRhiVulkan::endOffscreenFrame() +QRhi::FrameOpResult QRhiVulkan::endOffscreenFrame(QRhi::EndFrameFlags flags) { + Q_UNUSED(flags); Q_ASSERT(ofr.active); ofr.active = false; - recordCommandBuffer(&ofr.cbWrapper); + recordPrimaryCommandBuffer(&ofr.cbWrapper); if (!ofr.cmdFence) { VkFenceCreateInfo fenceInfo; @@ -1845,7 +1871,7 @@ QRhi::FrameOpResult QRhiVulkan::endOffscreenFrame() } } - QRhi::FrameOpResult submitres = endAndSubmitCommandBuffer(ofr.cbWrapper.cb, ofr.cmdFence, nullptr, nullptr); + QRhi::FrameOpResult submitres = endAndSubmitPrimaryCommandBuffer(ofr.cbWrapper.cb, ofr.cmdFence, nullptr, nullptr); if (submitres != QRhi::FrameOpSuccess) return submitres; @@ -1870,18 +1896,18 @@ QRhi::FrameOpResult QRhiVulkan::finish() if (ofr.active) { Q_ASSERT(!currentSwapChain); Q_ASSERT(ofr.cbWrapper.recordingPass == QVkCommandBuffer::NoPass); - recordCommandBuffer(&ofr.cbWrapper); + recordPrimaryCommandBuffer(&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); + recordPrimaryCommandBuffer(&swapChainD->cbWrapper); swapChainD->cbWrapper.resetCommands(); cb = swapChainD->cbWrapper.cb; } - QRhi::FrameOpResult submitres = endAndSubmitCommandBuffer(cb, VK_NULL_HANDLE, nullptr, nullptr); + QRhi::FrameOpResult submitres = endAndSubmitPrimaryCommandBuffer(cb, VK_NULL_HANDLE, nullptr, nullptr); if (submitres != QRhi::FrameOpSuccess) return submitres; } @@ -1891,9 +1917,9 @@ QRhi::FrameOpResult QRhiVulkan::finish() if (inFrame) { // Allocate and begin recording on a new command buffer. if (ofr.active) - startCommandBuffer(&ofr.cbWrapper.cb); + startPrimaryCommandBuffer(&ofr.cbWrapper.cb); else - startCommandBuffer(&swapChainD->frameRes[swapChainD->currentFrameSlot].cmdBuf); + startPrimaryCommandBuffer(&swapChainD->frameRes[swapChainD->currentFrameSlot].cmdBuf); } executeDeferredReleases(true); @@ -1967,6 +1993,69 @@ void QRhiVulkan::resourceUpdate(QRhiCommandBuffer *cb, QRhiResourceUpdateBatch * enqueueResourceUpdates(cbD, resourceUpdates); } +VkCommandBuffer QRhiVulkan::startSecondaryCommandBuffer(QVkRenderTargetData *rtD) +{ + VkCommandBuffer secondaryCb; + + VkCommandBufferAllocateInfo cmdBufInfo; + memset(&cmdBufInfo, 0, sizeof(cmdBufInfo)); + cmdBufInfo.sType = VK_STRUCTURE_TYPE_COMMAND_BUFFER_ALLOCATE_INFO; + cmdBufInfo.commandPool = cmdPool; + cmdBufInfo.level = VK_COMMAND_BUFFER_LEVEL_SECONDARY; + cmdBufInfo.commandBufferCount = 1; + VkResult err = df->vkAllocateCommandBuffers(dev, &cmdBufInfo, &secondaryCb); + if (err != VK_SUCCESS) { + qWarning("Failed to create secondary command buffer: %d", err); + return VK_NULL_HANDLE; + } + + VkCommandBufferBeginInfo cmdBufBeginInfo; + memset(&cmdBufBeginInfo, 0, sizeof(cmdBufBeginInfo)); + cmdBufBeginInfo.sType = VK_STRUCTURE_TYPE_COMMAND_BUFFER_BEGIN_INFO; + cmdBufBeginInfo.flags = rtD ? VK_COMMAND_BUFFER_USAGE_RENDER_PASS_CONTINUE_BIT : 0; + VkCommandBufferInheritanceInfo cmdBufInheritInfo; + memset(&cmdBufInheritInfo, 0, sizeof(cmdBufInheritInfo)); + cmdBufInheritInfo.sType = VK_STRUCTURE_TYPE_COMMAND_BUFFER_INHERITANCE_INFO; + cmdBufInheritInfo.subpass = 0; + if (rtD) { + cmdBufInheritInfo.renderPass = rtD->rp->rp; + cmdBufInheritInfo.framebuffer = rtD->fb; + } + cmdBufBeginInfo.pInheritanceInfo = &cmdBufInheritInfo; + + err = df->vkBeginCommandBuffer(secondaryCb, &cmdBufBeginInfo); + if (err != VK_SUCCESS) { + qWarning("Failed to begin secondary command buffer: %d", err); + df->vkFreeCommandBuffers(dev, cmdPool, 1, &secondaryCb); + return VK_NULL_HANDLE; + } + + return secondaryCb; +} + +void QRhiVulkan::endAndEnqueueSecondaryCommandBuffer(VkCommandBuffer cb, QVkCommandBuffer *cbD) +{ + VkResult err = df->vkEndCommandBuffer(cb); + if (err != VK_SUCCESS) + qWarning("Failed to end secondary command buffer: %d", err); + + QVkCommandBuffer::Command cmd; + cmd.cmd = QVkCommandBuffer::Command::ExecuteSecondary; + cmd.args.executeSecondary.cb = cb; + cbD->commands.append(cmd); + + deferredReleaseSecondaryCommandBuffer(cb); +} + +void QRhiVulkan::deferredReleaseSecondaryCommandBuffer(VkCommandBuffer cb) +{ + QRhiVulkan::DeferredReleaseEntry e; + e.type = QRhiVulkan::DeferredReleaseEntry::CommandBuffer; + e.lastActiveFrameSlot = currentFrameSlot; + e.commandBuffer.cb = cb; + releaseQueue.append(e); +} + void QRhiVulkan::beginPass(QRhiCommandBuffer *cb, QRhiRenderTarget *rt, const QColor &colorClearValue, @@ -2046,6 +2135,9 @@ void QRhiVulkan::beginPass(QRhiCommandBuffer *cb, cmd.args.beginRenderPass.clearValueIndex = cbD->pools.clearValue.count(); cbD->pools.clearValue.append(cvs.constData(), cvs.count()); cbD->commands.append(cmd); + + if (cbD->useSecondaryCb) + cbD->secondaryCbs.append(startSecondaryCommandBuffer(rtD)); } void QRhiVulkan::endPass(QRhiCommandBuffer *cb, QRhiResourceUpdateBatch *resourceUpdates) @@ -2053,6 +2145,13 @@ void QRhiVulkan::endPass(QRhiCommandBuffer *cb, QRhiResourceUpdateBatch *resourc QVkCommandBuffer *cbD = QRHI_RES(QVkCommandBuffer, cb); Q_ASSERT(cbD->recordingPass == QVkCommandBuffer::RenderPass); + if (cbD->useSecondaryCb) { + VkCommandBuffer secondaryCb = cbD->secondaryCbs.last(); + cbD->secondaryCbs.removeLast(); + endAndEnqueueSecondaryCommandBuffer(secondaryCb, cbD); + cbD->resetCachedState(); + } + QVkCommandBuffer::Command cmd; cmd.cmd = QVkCommandBuffer::Command::EndRenderPass; cbD->commands.append(cmd); @@ -2075,6 +2174,9 @@ void QRhiVulkan::beginComputePass(QRhiCommandBuffer *cb, QRhiResourceUpdateBatch enqueueTransitionPassResources(cbD); cbD->recordingPass = QVkCommandBuffer::ComputePass; + + if (cbD->useSecondaryCb) + cbD->secondaryCbs.append(startSecondaryCommandBuffer()); } void QRhiVulkan::endComputePass(QRhiCommandBuffer *cb, QRhiResourceUpdateBatch *resourceUpdates) @@ -2082,6 +2184,13 @@ void QRhiVulkan::endComputePass(QRhiCommandBuffer *cb, QRhiResourceUpdateBatch * QVkCommandBuffer *cbD = QRHI_RES(QVkCommandBuffer, cb); Q_ASSERT(cbD->recordingPass == QVkCommandBuffer::ComputePass); + if (cbD->useSecondaryCb) { + VkCommandBuffer secondaryCb = cbD->secondaryCbs.last(); + cbD->secondaryCbs.removeLast(); + endAndEnqueueSecondaryCommandBuffer(secondaryCb, cbD); + cbD->resetCachedState(); + } + cbD->recordingPass = QVkCommandBuffer::NoPass; if (resourceUpdates) @@ -2096,11 +2205,15 @@ void QRhiVulkan::setComputePipeline(QRhiCommandBuffer *cb, QRhiComputePipeline * Q_ASSERT(cbD->recordingPass == QVkCommandBuffer::ComputePass); if (cbD->currentComputePipeline != ps || cbD->currentPipelineGeneration != psD->generation) { - QVkCommandBuffer::Command cmd; - cmd.cmd = QVkCommandBuffer::Command::BindPipeline; - cmd.args.bindPipeline.bindPoint = VK_PIPELINE_BIND_POINT_COMPUTE; - cmd.args.bindPipeline.pipeline = psD->pipeline; - cbD->commands.append(cmd); + if (cbD->useSecondaryCb) { + df->vkCmdBindPipeline(cbD->secondaryCbs.last(), VK_PIPELINE_BIND_POINT_COMPUTE, psD->pipeline); + } else { + QVkCommandBuffer::Command cmd; + cmd.cmd = QVkCommandBuffer::Command::BindPipeline; + cmd.args.bindPipeline.bindPoint = VK_PIPELINE_BIND_POINT_COMPUTE; + cmd.args.bindPipeline.pipeline = psD->pipeline; + cbD->commands.append(cmd); + } cbD->currentGraphicsPipeline = nullptr; cbD->currentComputePipeline = ps; @@ -2115,12 +2228,16 @@ void QRhiVulkan::dispatch(QRhiCommandBuffer *cb, int x, int y, int z) QVkCommandBuffer *cbD = QRHI_RES(QVkCommandBuffer, cb); Q_ASSERT(cbD->recordingPass == QVkCommandBuffer::ComputePass); - QVkCommandBuffer::Command cmd; - cmd.cmd = QVkCommandBuffer::Command::Dispatch; - cmd.args.dispatch.x = x; - cmd.args.dispatch.y = y; - cmd.args.dispatch.z = z; - cbD->commands.append(cmd); + if (cbD->useSecondaryCb) { + df->vkCmdDispatch(cbD->secondaryCbs.last(), x, y, z); + } else { + QVkCommandBuffer::Command cmd; + cmd.cmd = QVkCommandBuffer::Command::Dispatch; + cmd.args.dispatch.x = x; + cmd.args.dispatch.y = y; + cmd.args.dispatch.z = z; + cbD->commands.append(cmd); + } } VkShaderModule QRhiVulkan::createShader(const QByteArray &spirv) @@ -3025,6 +3142,9 @@ void QRhiVulkan::executeDeferredReleases(bool forced) case QRhiVulkan::DeferredReleaseEntry::StagingBuffer: vmaDestroyBuffer(toVmaAllocator(allocator), e.stagingBuffer.stagingBuffer, toVmaAllocation(e.stagingBuffer.stagingAllocation)); break; + case QRhiVulkan::DeferredReleaseEntry::CommandBuffer: + df->vkFreeCommandBuffers(dev, cmdPool, 1, &e.commandBuffer.cb); + break; default: Q_UNREACHABLE(); break; @@ -3133,8 +3253,10 @@ void QRhiVulkan::enqueueTransitionPassResources(QVkCommandBuffer *cbD) cbD->commands.append(cmd); } -void QRhiVulkan::recordCommandBuffer(QVkCommandBuffer *cbD) +void QRhiVulkan::recordPrimaryCommandBuffer(QVkCommandBuffer *cbD) { + Q_ASSERT(cbD->recordingPass == QVkCommandBuffer::NoPass); + for (QVkCommandBuffer::Command &cmd : cbD->commands) { switch (cmd.cmd) { case QVkCommandBuffer::Command::CopyBuffer: @@ -3176,7 +3298,8 @@ void QRhiVulkan::recordCommandBuffer(QVkCommandBuffer *cbD) break; case QVkCommandBuffer::Command::BeginRenderPass: cmd.args.beginRenderPass.desc.pClearValues = cbD->pools.clearValue.constData() + cmd.args.beginRenderPass.clearValueIndex; - df->vkCmdBeginRenderPass(cbD->cb, &cmd.args.beginRenderPass.desc, VK_SUBPASS_CONTENTS_INLINE); + df->vkCmdBeginRenderPass(cbD->cb, &cmd.args.beginRenderPass.desc, + cbD->useSecondaryCb ? VK_SUBPASS_CONTENTS_SECONDARY_COMMAND_BUFFERS : VK_SUBPASS_CONTENTS_INLINE); break; case QVkCommandBuffer::Command::EndRenderPass: df->vkCmdEndRenderPass(cbD->cb); @@ -3229,13 +3352,15 @@ void QRhiVulkan::recordCommandBuffer(QVkCommandBuffer *cbD) break; case QVkCommandBuffer::Command::DebugMarkerBegin: cmd.args.debugMarkerBegin.marker.pMarkerName = - cbD->pools.debugMarkerName[cmd.args.debugMarkerBegin.markerNameIndex].constData(); + cbD->pools.debugMarkerData[cmd.args.debugMarkerBegin.markerNameIndex].constData(); vkCmdDebugMarkerBegin(cbD->cb, &cmd.args.debugMarkerBegin.marker); break; case QVkCommandBuffer::Command::DebugMarkerEnd: vkCmdDebugMarkerEnd(cbD->cb); break; case QVkCommandBuffer::Command::DebugMarkerInsert: + cmd.args.debugMarkerInsert.marker.pMarkerName = + cbD->pools.debugMarkerData[cmd.args.debugMarkerInsert.markerNameIndex].constData(); vkCmdDebugMarkerInsert(cbD->cb, &cmd.args.debugMarkerInsert.marker); break; case QVkCommandBuffer::Command::TransitionPassResources: @@ -3244,6 +3369,9 @@ void QRhiVulkan::recordCommandBuffer(QVkCommandBuffer *cbD) case QVkCommandBuffer::Command::Dispatch: df->vkCmdDispatch(cbD->cb, cmd.args.dispatch.x, cmd.args.dispatch.y, cmd.args.dispatch.z); break; + case QVkCommandBuffer::Command::ExecuteSecondary: + df->vkCmdExecuteCommands(cbD->cb, 1, &cmd.args.executeSecondary.cb); + break; default: break; } @@ -3664,11 +3792,15 @@ void QRhiVulkan::setGraphicsPipeline(QRhiCommandBuffer *cb, QRhiGraphicsPipeline Q_ASSERT(cbD->recordingPass == QVkCommandBuffer::RenderPass); if (cbD->currentGraphicsPipeline != ps || cbD->currentPipelineGeneration != psD->generation) { - QVkCommandBuffer::Command cmd; - cmd.cmd = QVkCommandBuffer::Command::BindPipeline; - cmd.args.bindPipeline.bindPoint = VK_PIPELINE_BIND_POINT_GRAPHICS; - cmd.args.bindPipeline.pipeline = psD->pipeline; - cbD->commands.append(cmd); + if (cbD->useSecondaryCb) { + df->vkCmdBindPipeline(cbD->secondaryCbs.last(), VK_PIPELINE_BIND_POINT_GRAPHICS, psD->pipeline); + } else { + QVkCommandBuffer::Command cmd; + cmd.cmd = QVkCommandBuffer::Command::BindPipeline; + cmd.args.bindPipeline.bindPoint = VK_PIPELINE_BIND_POINT_GRAPHICS; + cmd.args.bindPipeline.pipeline = psD->pipeline; + cbD->commands.append(cmd); + } cbD->currentGraphicsPipeline = ps; cbD->currentComputePipeline = nullptr; @@ -3866,16 +3998,25 @@ void QRhiVulkan::setShaderResources(QRhiCommandBuffer *cb, QRhiShaderResourceBin } } - QVkCommandBuffer::Command cmd; - cmd.cmd = QVkCommandBuffer::Command::BindDescriptorSet; - cmd.args.bindDescriptorSet.bindPoint = gfxPsD ? VK_PIPELINE_BIND_POINT_GRAPHICS - : VK_PIPELINE_BIND_POINT_COMPUTE; - cmd.args.bindDescriptorSet.pipelineLayout = gfxPsD ? gfxPsD->layout : compPsD->layout; - cmd.args.bindDescriptorSet.descSet = srbD->descSets[descSetIdx]; - cmd.args.bindDescriptorSet.dynamicOffsetCount = dynOfs.count(); - cmd.args.bindDescriptorSet.dynamicOffsetIndex = cbD->pools.dynamicOffset.count(); - cbD->pools.dynamicOffset.append(dynOfs.constData(), dynOfs.count()); - cbD->commands.append(cmd); + if (cbD->useSecondaryCb) { + df->vkCmdBindDescriptorSets(cbD->secondaryCbs.last(), + gfxPsD ? VK_PIPELINE_BIND_POINT_GRAPHICS : VK_PIPELINE_BIND_POINT_COMPUTE, + gfxPsD ? gfxPsD->layout : compPsD->layout, + 0, 1, &srbD->descSets[descSetIdx], + dynOfs.count(), + dynOfs.count() ? dynOfs.constData() : nullptr); + } else { + QVkCommandBuffer::Command cmd; + cmd.cmd = QVkCommandBuffer::Command::BindDescriptorSet; + cmd.args.bindDescriptorSet.bindPoint = gfxPsD ? VK_PIPELINE_BIND_POINT_GRAPHICS + : VK_PIPELINE_BIND_POINT_COMPUTE; + cmd.args.bindDescriptorSet.pipelineLayout = gfxPsD ? gfxPsD->layout : compPsD->layout; + cmd.args.bindDescriptorSet.descSet = srbD->descSets[descSetIdx]; + cmd.args.bindDescriptorSet.dynamicOffsetCount = dynOfs.count(); + cmd.args.bindDescriptorSet.dynamicOffsetIndex = cbD->pools.dynamicOffset.count(); + cbD->pools.dynamicOffset.append(dynOfs.constData(), dynOfs.count()); + cbD->commands.append(cmd); + } if (gfxPsD) { cbD->currentGraphicsSrb = srb; @@ -3931,15 +4072,20 @@ void QRhiVulkan::setVertexInput(QRhiCommandBuffer *cb, QRhiPassResourceTracker::BufVertexInputStage); } - QVkCommandBuffer::Command cmd; - cmd.cmd = QVkCommandBuffer::Command::BindVertexBuffer; - cmd.args.bindVertexBuffer.startBinding = startBinding; - cmd.args.bindVertexBuffer.count = bufs.count(); - cmd.args.bindVertexBuffer.vertexBufferIndex = cbD->pools.vertexBuffer.count(); - cbD->pools.vertexBuffer.append(bufs.constData(), bufs.count()); - cmd.args.bindVertexBuffer.vertexBufferOffsetIndex = cbD->pools.vertexBufferOffset.count(); - cbD->pools.vertexBufferOffset.append(ofs.constData(), ofs.count()); - cbD->commands.append(cmd); + if (cbD->useSecondaryCb) { + df->vkCmdBindVertexBuffers(cbD->secondaryCbs.last(), startBinding, + bufs.count(), bufs.constData(), ofs.constData()); + } else { + QVkCommandBuffer::Command cmd; + cmd.cmd = QVkCommandBuffer::Command::BindVertexBuffer; + cmd.args.bindVertexBuffer.startBinding = startBinding; + cmd.args.bindVertexBuffer.count = bufs.count(); + cmd.args.bindVertexBuffer.vertexBufferIndex = cbD->pools.vertexBuffer.count(); + cbD->pools.vertexBuffer.append(bufs.constData(), bufs.count()); + cmd.args.bindVertexBuffer.vertexBufferOffsetIndex = cbD->pools.vertexBufferOffset.count(); + cbD->pools.vertexBufferOffset.append(ofs.constData(), ofs.count()); + cbD->commands.append(cmd); + } } if (indexBuf) { @@ -3962,12 +4108,16 @@ void QRhiVulkan::setVertexInput(QRhiCommandBuffer *cb, cbD->currentIndexOffset = indexOffset; cbD->currentIndexFormat = type; - QVkCommandBuffer::Command cmd; - cmd.cmd = QVkCommandBuffer::Command::BindIndexBuffer; - cmd.args.bindIndexBuffer.buf = vkindexbuf; - cmd.args.bindIndexBuffer.ofs = indexOffset; - cmd.args.bindIndexBuffer.type = type; - cbD->commands.append(cmd); + if (cbD->useSecondaryCb) { + df->vkCmdBindIndexBuffer(cbD->secondaryCbs.last(), vkindexbuf, indexOffset, type); + } else { + QVkCommandBuffer::Command cmd; + cmd.cmd = QVkCommandBuffer::Command::BindIndexBuffer; + cmd.args.bindIndexBuffer.buf = vkindexbuf; + cmd.args.bindIndexBuffer.ofs = indexOffset; + cmd.args.bindIndexBuffer.type = type; + cbD->commands.append(cmd); + } trackedRegisterBuffer(&passResTracker, ibufD, slot, QRhiPassResourceTracker::BufIndexRead, @@ -3988,7 +4138,6 @@ void QRhiVulkan::setViewport(QRhiCommandBuffer *cb, const QRhiViewport &viewport return; QVkCommandBuffer::Command cmd; - cmd.cmd = QVkCommandBuffer::Command::SetViewport; VkViewport *vp = &cmd.args.setViewport.viewport; vp->x = x; vp->y = y; @@ -3996,16 +4145,26 @@ void QRhiVulkan::setViewport(QRhiCommandBuffer *cb, const QRhiViewport &viewport vp->height = h; vp->minDepth = viewport.minDepth(); vp->maxDepth = viewport.maxDepth(); - cbD->commands.append(cmd); + + if (cbD->useSecondaryCb) { + df->vkCmdSetViewport(cbD->secondaryCbs.last(), 0, 1, vp); + } else { + cmd.cmd = QVkCommandBuffer::Command::SetViewport; + cbD->commands.append(cmd); + } if (!QRHI_RES(QVkGraphicsPipeline, cbD->currentGraphicsPipeline)->m_flags.testFlag(QRhiGraphicsPipeline::UsesScissor)) { - cmd.cmd = QVkCommandBuffer::Command::SetScissor; VkRect2D *s = &cmd.args.setScissor.scissor; s->offset.x = x; s->offset.y = y; s->extent.width = w; s->extent.height = h; - cbD->commands.append(cmd); + if (cbD->useSecondaryCb) { + df->vkCmdSetScissor(cbD->secondaryCbs.last(), 0, 1, s); + } else { + cmd.cmd = QVkCommandBuffer::Command::SetScissor; + cbD->commands.append(cmd); + } } } @@ -4022,13 +4181,18 @@ void QRhiVulkan::setScissor(QRhiCommandBuffer *cb, const QRhiScissor &scissor) return; QVkCommandBuffer::Command cmd; - cmd.cmd = QVkCommandBuffer::Command::SetScissor; VkRect2D *s = &cmd.args.setScissor.scissor; s->offset.x = x; s->offset.y = y; s->extent.width = w; s->extent.height = h; - cbD->commands.append(cmd); + + if (cbD->useSecondaryCb) { + df->vkCmdSetScissor(cbD->secondaryCbs.last(), 0, 1, s); + } else { + cmd.cmd = QVkCommandBuffer::Command::SetScissor; + cbD->commands.append(cmd); + } } void QRhiVulkan::setBlendConstants(QRhiCommandBuffer *cb, const QColor &c) @@ -4036,13 +4200,18 @@ void QRhiVulkan::setBlendConstants(QRhiCommandBuffer *cb, const QColor &c) QVkCommandBuffer *cbD = QRHI_RES(QVkCommandBuffer, cb); Q_ASSERT(cbD->recordingPass == QVkCommandBuffer::RenderPass); - QVkCommandBuffer::Command cmd; - cmd.cmd = QVkCommandBuffer::Command::SetBlendConstants; - cmd.args.setBlendConstants.c[0] = c.redF(); - cmd.args.setBlendConstants.c[1] = c.greenF(); - cmd.args.setBlendConstants.c[2] = c.blueF(); - cmd.args.setBlendConstants.c[3] = c.alphaF(); - cbD->commands.append(cmd); + if (cbD->useSecondaryCb) { + float constants[] = { float(c.redF()), float(c.greenF()), float(c.blueF()), float(c.alphaF()) }; + df->vkCmdSetBlendConstants(cbD->secondaryCbs.last(), constants); + } else { + QVkCommandBuffer::Command cmd; + cmd.cmd = QVkCommandBuffer::Command::SetBlendConstants; + cmd.args.setBlendConstants.c[0] = c.redF(); + cmd.args.setBlendConstants.c[1] = c.greenF(); + cmd.args.setBlendConstants.c[2] = c.blueF(); + cmd.args.setBlendConstants.c[3] = c.alphaF(); + cbD->commands.append(cmd); + } } void QRhiVulkan::setStencilRef(QRhiCommandBuffer *cb, quint32 refValue) @@ -4050,10 +4219,14 @@ void QRhiVulkan::setStencilRef(QRhiCommandBuffer *cb, quint32 refValue) QVkCommandBuffer *cbD = QRHI_RES(QVkCommandBuffer, cb); Q_ASSERT(cbD->recordingPass == QVkCommandBuffer::RenderPass); - QVkCommandBuffer::Command cmd; - cmd.cmd = QVkCommandBuffer::Command::SetStencilRef; - cmd.args.setStencilRef.ref = refValue; - cbD->commands.append(cmd); + if (cbD->useSecondaryCb) { + df->vkCmdSetStencilReference(cbD->secondaryCbs.last(), VK_STENCIL_FRONT_AND_BACK, refValue); + } else { + QVkCommandBuffer::Command cmd; + cmd.cmd = QVkCommandBuffer::Command::SetStencilRef; + cmd.args.setStencilRef.ref = refValue; + cbD->commands.append(cmd); + } } void QRhiVulkan::draw(QRhiCommandBuffer *cb, quint32 vertexCount, @@ -4062,13 +4235,17 @@ void QRhiVulkan::draw(QRhiCommandBuffer *cb, quint32 vertexCount, QVkCommandBuffer *cbD = QRHI_RES(QVkCommandBuffer, cb); Q_ASSERT(cbD->recordingPass == QVkCommandBuffer::RenderPass); - QVkCommandBuffer::Command cmd; - cmd.cmd = QVkCommandBuffer::Command::Draw; - cmd.args.draw.vertexCount = vertexCount; - cmd.args.draw.instanceCount = instanceCount; - cmd.args.draw.firstVertex = firstVertex; - cmd.args.draw.firstInstance = firstInstance; - cbD->commands.append(cmd); + if (cbD->useSecondaryCb) { + df->vkCmdDraw(cbD->secondaryCbs.last(), vertexCount, instanceCount, firstVertex, firstInstance); + } else { + QVkCommandBuffer::Command cmd; + cmd.cmd = QVkCommandBuffer::Command::Draw; + cmd.args.draw.vertexCount = vertexCount; + cmd.args.draw.instanceCount = instanceCount; + cmd.args.draw.firstVertex = firstVertex; + cmd.args.draw.firstInstance = firstInstance; + cbD->commands.append(cmd); + } } void QRhiVulkan::drawIndexed(QRhiCommandBuffer *cb, quint32 indexCount, @@ -4077,14 +4254,19 @@ void QRhiVulkan::drawIndexed(QRhiCommandBuffer *cb, quint32 indexCount, QVkCommandBuffer *cbD = QRHI_RES(QVkCommandBuffer, cb); Q_ASSERT(cbD->recordingPass == QVkCommandBuffer::RenderPass); - QVkCommandBuffer::Command cmd; - cmd.cmd = QVkCommandBuffer::Command::DrawIndexed; - cmd.args.drawIndexed.indexCount = indexCount; - cmd.args.drawIndexed.instanceCount = instanceCount; - cmd.args.drawIndexed.firstIndex = firstIndex; - cmd.args.drawIndexed.vertexOffset = vertexOffset; - cmd.args.drawIndexed.firstInstance = firstInstance; - cbD->commands.append(cmd); + if (cbD->useSecondaryCb) { + df->vkCmdDrawIndexed(cbD->secondaryCbs.last(), indexCount, instanceCount, + firstIndex, vertexOffset, firstInstance); + } else { + QVkCommandBuffer::Command cmd; + cmd.cmd = QVkCommandBuffer::Command::DrawIndexed; + cmd.args.drawIndexed.indexCount = indexCount; + cmd.args.drawIndexed.instanceCount = instanceCount; + cmd.args.drawIndexed.firstIndex = firstIndex; + cmd.args.drawIndexed.vertexOffset = vertexOffset; + cmd.args.drawIndexed.firstInstance = firstInstance; + cbD->commands.append(cmd); + } } void QRhiVulkan::debugMarkBegin(QRhiCommandBuffer *cb, const QByteArray &name) @@ -4097,12 +4279,17 @@ void QRhiVulkan::debugMarkBegin(QRhiCommandBuffer *cb, const QByteArray &name) marker.sType = VK_STRUCTURE_TYPE_DEBUG_MARKER_MARKER_INFO_EXT; QVkCommandBuffer *cbD = QRHI_RES(QVkCommandBuffer, cb); - QVkCommandBuffer::Command cmd; - cmd.cmd = QVkCommandBuffer::Command::DebugMarkerBegin; - cmd.args.debugMarkerBegin.marker = marker; - cmd.args.debugMarkerBegin.markerNameIndex = cbD->pools.debugMarkerName.count(); - cbD->pools.debugMarkerName.append(name); - cbD->commands.append(cmd); + if (cbD->recordingPass != QVkCommandBuffer::NoPass && cbD->useSecondaryCb) { + marker.pMarkerName = name.constData(); + vkCmdDebugMarkerBegin(cbD->secondaryCbs.last(), &marker); + } else { + QVkCommandBuffer::Command cmd; + cmd.cmd = QVkCommandBuffer::Command::DebugMarkerBegin; + cmd.args.debugMarkerBegin.marker = marker; + cmd.args.debugMarkerBegin.markerNameIndex = cbD->pools.debugMarkerData.count(); + cbD->pools.debugMarkerData.append(name); + cbD->commands.append(cmd); + } } void QRhiVulkan::debugMarkEnd(QRhiCommandBuffer *cb) @@ -4111,9 +4298,13 @@ void QRhiVulkan::debugMarkEnd(QRhiCommandBuffer *cb) return; QVkCommandBuffer *cbD = QRHI_RES(QVkCommandBuffer, cb); - QVkCommandBuffer::Command cmd; - cmd.cmd = QVkCommandBuffer::Command::DebugMarkerEnd; - cbD->commands.append(cmd); + if (cbD->recordingPass != QVkCommandBuffer::NoPass && cbD->useSecondaryCb) { + vkCmdDebugMarkerEnd(cbD->secondaryCbs.last()); + } else { + QVkCommandBuffer::Command cmd; + cmd.cmd = QVkCommandBuffer::Command::DebugMarkerEnd; + cbD->commands.append(cmd); + } } void QRhiVulkan::debugMarkMsg(QRhiCommandBuffer *cb, const QByteArray &msg) @@ -4124,13 +4315,19 @@ void QRhiVulkan::debugMarkMsg(QRhiCommandBuffer *cb, const QByteArray &msg) VkDebugMarkerMarkerInfoEXT marker; memset(&marker, 0, sizeof(marker)); marker.sType = VK_STRUCTURE_TYPE_DEBUG_MARKER_MARKER_INFO_EXT; - marker.pMarkerName = msg.constData(); QVkCommandBuffer *cbD = QRHI_RES(QVkCommandBuffer, cb); - QVkCommandBuffer::Command cmd; - cmd.cmd = QVkCommandBuffer::Command::DebugMarkerInsert; - cmd.args.debugMarkerInsert.marker = marker; - cbD->commands.append(cmd); + if (cbD->recordingPass != QVkCommandBuffer::NoPass && cbD->useSecondaryCb) { + marker.pMarkerName = msg.constData(); + vkCmdDebugMarkerInsert(cbD->secondaryCbs.last(), &marker); + } else { + QVkCommandBuffer::Command cmd; + cmd.cmd = QVkCommandBuffer::Command::DebugMarkerInsert; + cmd.args.debugMarkerInsert.marker = marker; + cmd.args.debugMarkerInsert.markerNameIndex = cbD->pools.debugMarkerData.count(); + cbD->pools.debugMarkerData.append(msg); + cbD->commands.append(cmd); + } } const QRhiNativeHandles *QRhiVulkan::nativeHandles(QRhiCommandBuffer *cb) @@ -4138,32 +4335,77 @@ const QRhiNativeHandles *QRhiVulkan::nativeHandles(QRhiCommandBuffer *cb) return QRHI_RES(QVkCommandBuffer, cb)->nativeHandles(); } +static inline QVkRenderTargetData *maybeRenderTargetData(QVkCommandBuffer *cbD) +{ + Q_ASSERT(cbD->currentTarget); + QVkRenderTargetData *rtD = nullptr; + if (cbD->recordingPass == QVkCommandBuffer::RenderPass) { + switch (cbD->currentTarget->resourceType()) { + case QRhiResource::RenderTarget: + rtD = &QRHI_RES(QVkReferenceRenderTarget, cbD->currentTarget)->d; + break; + case QRhiResource::TextureRenderTarget: + rtD = &QRHI_RES(QVkTextureRenderTarget, cbD->currentTarget)->d; + break; + default: + Q_UNREACHABLE(); + break; + } + } + return rtD; +} + void QRhiVulkan::beginExternal(QRhiCommandBuffer *cb) { QVkCommandBuffer *cbD = QRHI_RES(QVkCommandBuffer, cb); - recordCommandBuffer(cbD); - cbD->resetCommands(); + + // When not in a pass, it is simple: record what we have (but do not + // submit), the cb can then be used to record more external commands. + if (cbD->recordingPass == QVkCommandBuffer::NoPass) { + recordPrimaryCommandBuffer(cbD); + cbD->resetCommands(); + return; + } + + // Otherwise, inside a pass, have a secondary command buffer (with + // RENDER_PASS_CONTINUE). Using the main one is not acceptable since we + // cannot just record at this stage, that would mess up the resource + // tracking and commands like TransitionPassResources. + + if (cbD->inExternal) + return; + + if (!cbD->useSecondaryCb) { + qWarning("beginExternal() within a pass is only supported with secondary command buffers. " + "This can be enabled by passing QRhi::ExternalContentsInPass to beginFrame()."); + return; + } + + VkCommandBuffer secondaryCb = cbD->secondaryCbs.last(); + cbD->secondaryCbs.removeLast(); + endAndEnqueueSecondaryCommandBuffer(secondaryCb, cbD); + + VkCommandBuffer extCb = startSecondaryCommandBuffer(maybeRenderTargetData(cbD)); + if (extCb) { + cbD->secondaryCbs.append(extCb); + cbD->inExternal = true; + } } 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); + if (cbD->recordingPass == QVkCommandBuffer::NoPass) { + Q_ASSERT(cbD->commands.isEmpty() && cbD->currentPassResTrackerIndex == -1); + } else if (cbD->inExternal) { + VkCommandBuffer extCb = cbD->secondaryCbs.last(); + cbD->secondaryCbs.removeLast(); + endAndEnqueueSecondaryCommandBuffer(extCb, cbD); + cbD->secondaryCbs.append(startSecondaryCommandBuffer(maybeRenderTargetData(cbD))); } + + cbD->resetCachedState(); } void QRhiVulkan::setObjectName(uint64_t object, VkDebugReportObjectTypeEXT type, const QByteArray &name, int slot) @@ -5818,6 +6060,22 @@ void QVkCommandBuffer::release() // nothing to do here, cb is not owned by us } +const QRhiNativeHandles *QVkCommandBuffer::nativeHandles() +{ + // Ok this is messy but no other way has been devised yet. Outside + // begin(Compute)Pass - end(Compute)Pass it is simple - just return the + // primary VkCommandBuffer. Inside, however, we need to provide the current + // secondary command buffer (typically the one started by beginExternal(), + // in case we are between beginExternal - endExternal inside a pass). + + if (useSecondaryCb && !secondaryCbs.isEmpty()) + nativeHandlesStruct.commandBuffer = secondaryCbs.last(); + else + nativeHandlesStruct.commandBuffer = cb; + + return &nativeHandlesStruct; +} + QVkSwapChain::QVkSwapChain(QRhiImplementation *rhi) : QRhiSwapChain(rhi), rtWrapper(rhi), diff --git a/src/gui/rhi/qrhivulkan_p_p.h b/src/gui/rhi/qrhivulkan_p_p.h index dd9a7d4216..962a1b8eb7 100644 --- a/src/gui/rhi/qrhivulkan_p_p.h +++ b/src/gui/rhi/qrhivulkan_p_p.h @@ -309,13 +309,11 @@ struct QVkCommandBuffer : public QRhiCommandBuffer ~QVkCommandBuffer(); void release() override; - VkCommandBuffer cb = VK_NULL_HANDLE; - QRhiVulkanCommandBufferNativeHandles nativeHandlesStruct; + const QRhiNativeHandles *nativeHandles(); - const QRhiNativeHandles *nativeHandles() { - nativeHandlesStruct.commandBuffer = cb; - return &nativeHandlesStruct; - } + VkCommandBuffer cb = VK_NULL_HANDLE; // primary + bool useSecondaryCb = false; + QRhiVulkanCommandBufferNativeHandles nativeHandlesStruct; enum PassType { NoPass, @@ -326,6 +324,9 @@ struct QVkCommandBuffer : public QRhiCommandBuffer void resetState() { recordingPass = NoPass; currentTarget = nullptr; + + secondaryCbs.clear(); + resetCommands(); resetCachedState(); } @@ -343,6 +344,7 @@ struct QVkCommandBuffer : public QRhiCommandBuffer currentIndexFormat = VK_INDEX_TYPE_UINT16; memset(currentVertexBuffers, 0, sizeof(currentVertexBuffers)); memset(currentVertexOffsets, 0, sizeof(currentVertexOffsets)); + inExternal = false; } PassType recordingPass; @@ -360,6 +362,8 @@ struct QVkCommandBuffer : public QRhiCommandBuffer static const int VERTEX_INPUT_RESOURCE_SLOT_COUNT = 32; VkBuffer currentVertexBuffers[VERTEX_INPUT_RESOURCE_SLOT_COUNT]; quint32 currentVertexOffsets[VERTEX_INPUT_RESOURCE_SLOT_COUNT]; + QVarLengthArray secondaryCbs; + bool inExternal; struct Command { enum Cmd { @@ -386,7 +390,8 @@ struct QVkCommandBuffer : public QRhiCommandBuffer DebugMarkerEnd, DebugMarkerInsert, TransitionPassResources, - Dispatch + Dispatch, + ExecuteSecondary }; Cmd cmd; @@ -495,6 +500,7 @@ struct QVkCommandBuffer : public QRhiCommandBuffer } debugMarkerEnd; struct { VkDebugMarkerMarkerInfoEXT marker; + int markerNameIndex; } debugMarkerInsert; struct { int trackerIndex; @@ -502,6 +508,9 @@ struct QVkCommandBuffer : public QRhiCommandBuffer struct { int x, y, z; } dispatch; + struct { + VkCommandBuffer cb; + } executeSecondary; } args; }; QVector commands; @@ -522,7 +531,7 @@ struct QVkCommandBuffer : public QRhiCommandBuffer pools.dynamicOffset.clear(); pools.vertexBuffer.clear(); pools.vertexBufferOffset.clear(); - pools.debugMarkerName.clear(); + pools.debugMarkerData.clear(); } struct { @@ -531,7 +540,7 @@ struct QVkCommandBuffer : public QRhiCommandBuffer QVarLengthArray dynamicOffset; QVarLengthArray vertexBuffer; QVarLengthArray vertexBufferOffset; - QVarLengthArray debugMarkerName; + QVarLengthArray debugMarkerData; } pools; friend class QRhiVulkan; @@ -595,7 +604,7 @@ struct QVkSwapChain : public QRhiSwapChain bool imageAcquired = false; bool imageSemWaitable = false; quint32 imageIndex = 0; - VkCommandBuffer cmdBuf = VK_NULL_HANDLE; + VkCommandBuffer cmdBuf = VK_NULL_HANDLE; // primary VkFence cmdFence = VK_NULL_HANDLE; bool cmdFenceWaitable = false; int timestampQueryIndex = -1; @@ -640,8 +649,8 @@ public: QRhiSwapChain *createSwapChain() override; QRhi::FrameOpResult beginFrame(QRhiSwapChain *swapChain, QRhi::BeginFrameFlags flags) override; QRhi::FrameOpResult endFrame(QRhiSwapChain *swapChain, QRhi::EndFrameFlags flags) override; - QRhi::FrameOpResult beginOffscreenFrame(QRhiCommandBuffer **cb) override; - QRhi::FrameOpResult endOffscreenFrame() override; + QRhi::FrameOpResult beginOffscreenFrame(QRhiCommandBuffer **cb, QRhi::BeginFrameFlags flags) override; + QRhi::FrameOpResult endOffscreenFrame(QRhi::EndFrameFlags flags) override; QRhi::FrameOpResult finish() override; void resourceUpdate(QRhiCommandBuffer *cb, QRhiResourceUpdateBatch *resourceUpdates) override; @@ -730,9 +739,12 @@ public: VkShaderModule createShader(const QByteArray &spirv); void prepareNewFrame(QRhiCommandBuffer *cb); - QRhi::FrameOpResult startCommandBuffer(VkCommandBuffer *cb); - QRhi::FrameOpResult endAndSubmitCommandBuffer(VkCommandBuffer cb, VkFence cmdFence, - VkSemaphore *waitSem, VkSemaphore *signalSem); + VkCommandBuffer startSecondaryCommandBuffer(QVkRenderTargetData *rtD = nullptr); + void endAndEnqueueSecondaryCommandBuffer(VkCommandBuffer cb, QVkCommandBuffer *cbD); + void deferredReleaseSecondaryCommandBuffer(VkCommandBuffer cb); + QRhi::FrameOpResult startPrimaryCommandBuffer(VkCommandBuffer *cb); + QRhi::FrameOpResult endAndSubmitPrimaryCommandBuffer(VkCommandBuffer cb, VkFence cmdFence, + VkSemaphore *waitSem, VkSemaphore *signalSem); void waitCommandCompletion(int frameSlot); VkDeviceSize subresUploadByteSize(const QRhiTextureSubresourceUploadDescription &subresDesc) const; using BufferImageCopyList = QVarLengthArray; @@ -743,7 +755,7 @@ public: void enqueueResourceUpdates(QVkCommandBuffer *cbD, QRhiResourceUpdateBatch *resourceUpdates); void executeBufferHostWritesForCurrentFrame(QVkBuffer *bufD); void enqueueTransitionPassResources(QVkCommandBuffer *cbD); - void recordCommandBuffer(QVkCommandBuffer *cbD); + void recordPrimaryCommandBuffer(QVkCommandBuffer *cbD); void trackedRegisterBuffer(QRhiPassResourceTracker *passResTracker, QVkBuffer *bufD, int slot, @@ -859,7 +871,8 @@ public: Sampler, TextureRenderTarget, RenderPass, - StagingBuffer + StagingBuffer, + CommandBuffer }; Type type; int lastActiveFrameSlot; // -1 if not used otherwise 0..FRAMES_IN_FLIGHT-1 @@ -906,6 +919,9 @@ public: VkBuffer stagingBuffer; QVkAlloc stagingAllocation; } stagingBuffer; + struct { + VkCommandBuffer cb; + } commandBuffer; }; }; QVector releaseQueue; -- cgit v1.2.3 From 81408c0e76616b127c46779dc14bbcf084a3d87b Mon Sep 17 00:00:00 2001 From: Alex Trotsenko Date: Mon, 19 Aug 2019 14:20:11 +0300 Subject: QEventDispatcherWin32: avoid livelock in a foreign event loop According to Windows docs, GetMessage() function retrieves the messages from the input queue in defined order, where posted messages are processed ahead of input messages, even if they were posted later. Therefore, if the application produces a posted event permanently, as a result of processing that event, user input messages may be blocked due to hard CPU usage by the application. It's not a problem, if an internal Qt event loop is running. By calling sendPostedEvents() on the beginning of processEvents(), we are sending posted events only once per iteration. However, during execution of the foreign loop, we should artificially lower the priority of the WM_QT_SENDPOSTEDEVENTS message in order to enable delivery of other input messages. To solve the problem, it is proposed to postpone the WM_QT_SENDPOSTEDEVENTS message until the message queue becomes empty, as it works for the internal loop. Task-number: QTBUG-77464 Change-Id: I8dedb6837c6fc41aa6f497e67ab2352c2b4f3772 Reviewed-by: Laszlo Agocs --- src/corelib/kernel/qeventdispatcher_win.cpp | 67 +++++++++++++++-------------- src/corelib/kernel/qeventdispatcher_win_p.h | 2 + 2 files changed, 37 insertions(+), 32 deletions(-) (limited to 'src') diff --git a/src/corelib/kernel/qeventdispatcher_win.cpp b/src/corelib/kernel/qeventdispatcher_win.cpp index c15d740f9e..87623f304a 100644 --- a/src/corelib/kernel/qeventdispatcher_win.cpp +++ b/src/corelib/kernel/qeventdispatcher_win.cpp @@ -100,7 +100,7 @@ LRESULT QT_WIN_CALLBACK qt_internal_proc(HWND hwnd, UINT message, WPARAM wp, LPA QEventDispatcherWin32Private::QEventDispatcherWin32Private() : threadId(GetCurrentThreadId()), interrupt(false), internalHwnd(0), - wakeUps(0), activateNotifiersPosted(false), + getMessageHook(0), wakeUps(0), activateNotifiersPosted(false), winEventNotifierActivatedEvent(NULL) { } @@ -245,9 +245,6 @@ LRESULT QT_WIN_CALLBACK qt_internal_proc(HWND hwnd, UINT message, WPARAM wp, LPA case WM_QT_SENDPOSTEDEVENTS: Q_ASSERT(d != 0); - // Allow posting WM_QT_SENDPOSTEDEVENTS message. - d->wakeUps.storeRelaxed(0); - // We send posted events manually, if the window procedure was invoked // by the foreign event loop (e.g. from the native modal dialog). q->sendPostedEvents(); @@ -257,9 +254,9 @@ LRESULT QT_WIN_CALLBACK qt_internal_proc(HWND hwnd, UINT message, WPARAM wp, LPA return DefWindowProc(hwnd, message, wp, lp); } -static inline UINT inputTimerMask() +static inline UINT inputQueueMask() { - UINT result = QS_TIMER | QS_INPUT | QS_RAWINPUT; + UINT result = QS_ALLEVENTS; // QTBUG 28513, QTBUG-29097, QTBUG-29435: QS_TOUCH, QS_POINTER became part of // QS_INPUT in Windows Kit 8. They should not be used when running on pre-Windows 8. #if WINVER > 0x0601 @@ -269,6 +266,25 @@ static inline UINT inputTimerMask() return result; } +LRESULT QT_WIN_CALLBACK qt_GetMessageHook(int code, WPARAM wp, LPARAM lp) +{ + QEventDispatcherWin32 *q = qobject_cast(QAbstractEventDispatcher::instance()); + Q_ASSERT(q != 0); + QEventDispatcherWin32Private *d = q->d_func(); + MSG *msg = reinterpret_cast(lp); + static const UINT mask = inputQueueMask(); + + if (HIWORD(GetQueueStatus(mask)) == 0 && wp == PM_REMOVE) { + // Allow posting WM_QT_SENDPOSTEDEVENTS message. + d->wakeUps.storeRelaxed(0); + if (!(msg->hwnd == d->internalHwnd && msg->message == WM_QT_SENDPOSTEDEVENTS)) { + PostMessage(d->internalHwnd, WM_QT_SENDPOSTEDEVENTS, + WMWP_QT_TOFOREIGNLOOP, 0); + } + } + return d->getMessageHook ? CallNextHookEx(0, code, wp, lp) : 0; +} + // Provide class name and atom for the message window used by // QEventDispatcherWin32Private via Q_GLOBAL_STATIC shared between threads. struct QWindowsMessageWindowClassContext @@ -447,6 +463,14 @@ void QEventDispatcherWin32::createInternalHwnd() return; d->internalHwnd = qt_create_internal_window(this); + // setup GetMessage hook needed to drive our posted events + d->getMessageHook = SetWindowsHookEx(WH_GETMESSAGE, (HOOKPROC) qt_GetMessageHook, NULL, GetCurrentThreadId()); + if (Q_UNLIKELY(!d->getMessageHook)) { + int errorCode = GetLastError(); + qFatal("Qt: INTERNAL ERROR: failed to install GetMessage hook: %d, %ls", + errorCode, qUtf16Printable(qt_error_string(errorCode))); + } + // start all normal timers for (int i = 0; i < d->timerVec.count(); ++i) d->registerTimer(d->timerVec.at(i)); @@ -499,7 +523,6 @@ bool QEventDispatcherWin32::processEvents(QEventLoop::ProcessEventsFlags flags) bool canWait; bool retVal = false; - bool needWM_QT_SENDPOSTEDEVENTS = false; do { DWORD waitRet = 0; DWORD nCount = 0; @@ -549,11 +572,8 @@ bool QEventDispatcherWin32::processEvents(QEventLoop::ProcessEventsFlags flags) if (haveMessage) { if (d->internalHwnd == msg.hwnd && msg.message == WM_QT_SENDPOSTEDEVENTS) { // Set result to 'true', if the message was sent by wakeUp(). - if (msg.wParam == WMWP_QT_FROMWAKEUP) { - d->wakeUps.storeRelaxed(0); + if (msg.wParam == WMWP_QT_FROMWAKEUP) retVal = true; - } - needWM_QT_SENDPOSTEDEVENTS = true; continue; } if (msg.message == WM_TIMER) { @@ -573,22 +593,10 @@ bool QEventDispatcherWin32::processEvents(QEventLoop::ProcessEventsFlags flags) } if (!filterNativeEvent(QByteArrayLiteral("windows_generic_MSG"), &msg, 0)) { - // Post WM_QT_SENDPOSTEDEVENTS before calling external code, - // as it can start a foreign event loop. - if (needWM_QT_SENDPOSTEDEVENTS) { - needWM_QT_SENDPOSTEDEVENTS = false; - PostMessage(d->internalHwnd, WM_QT_SENDPOSTEDEVENTS, - WMWP_QT_TOFOREIGNLOOP, 0); - } TranslateMessage(&msg); DispatchMessage(&msg); } } else if (waitRet - WAIT_OBJECT_0 < nCount) { - if (needWM_QT_SENDPOSTEDEVENTS) { - needWM_QT_SENDPOSTEDEVENTS = false; - PostMessage(d->internalHwnd, WM_QT_SENDPOSTEDEVENTS, - WMWP_QT_TOFOREIGNLOOP, 0); - } activateEventNotifiers(); } else { // nothing todo so break @@ -606,21 +614,12 @@ bool QEventDispatcherWin32::processEvents(QEventLoop::ProcessEventsFlags flags) waitRet = MsgWaitForMultipleObjectsEx(nCount, pHandles, INFINITE, QS_ALLINPUT, MWMO_ALERTABLE | MWMO_INPUTAVAILABLE); emit awake(); if (waitRet - WAIT_OBJECT_0 < nCount) { - if (needWM_QT_SENDPOSTEDEVENTS) { - needWM_QT_SENDPOSTEDEVENTS = false; - PostMessage(d->internalHwnd, WM_QT_SENDPOSTEDEVENTS, - WMWP_QT_TOFOREIGNLOOP, 0); - } activateEventNotifiers(); retVal = true; } } } while (canWait); - if (needWM_QT_SENDPOSTEDEVENTS) - PostMessage(d->internalHwnd, WM_QT_SENDPOSTEDEVENTS, - WMWP_QT_TOFOREIGNLOOP, 0); - return retVal; } @@ -1004,6 +1003,10 @@ void QEventDispatcherWin32::closingDown() d->timerDict.clear(); d->closingDown = true; + + if (d->getMessageHook) + UnhookWindowsHookEx(d->getMessageHook); + d->getMessageHook = 0; } bool QEventDispatcherWin32::event(QEvent *e) diff --git a/src/corelib/kernel/qeventdispatcher_win_p.h b/src/corelib/kernel/qeventdispatcher_win_p.h index 697c07f912..e6620178d8 100644 --- a/src/corelib/kernel/qeventdispatcher_win_p.h +++ b/src/corelib/kernel/qeventdispatcher_win_p.h @@ -113,6 +113,7 @@ protected: private: friend LRESULT QT_WIN_CALLBACK qt_internal_proc(HWND hwnd, UINT message, WPARAM wp, LPARAM lp); + friend LRESULT QT_WIN_CALLBACK qt_GetMessageHook(int, WPARAM, LPARAM); }; struct QSockNot { @@ -166,6 +167,7 @@ public: // internal window handle used for socketnotifiers/timers/etc HWND internalHwnd; + HHOOK getMessageHook; // for controlling when to send posted events QAtomicInt wakeUps; -- cgit v1.2.3 From 108382e236dcdc09a822b89db35cc8bc4509273f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A5rten=20Nordheim?= Date: Thu, 29 Aug 2019 12:50:48 +0200 Subject: Mark QNetworkAccessManager::autoDeleteReplies as const From the API review. Amends cd816d4b6ac4358a92dbda906288ba6d969fc1cd. Change-Id: I3d3e2ef331501fa498545c5eec0e321544165b0d Reviewed-by: Timur Pocheptsov Reviewed-by: Edward Welbourne --- src/network/access/qnetworkaccessmanager.cpp | 2 +- src/network/access/qnetworkaccessmanager.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/network/access/qnetworkaccessmanager.cpp b/src/network/access/qnetworkaccessmanager.cpp index fdc3cd3b3a..76b95b5823 100644 --- a/src/network/access/qnetworkaccessmanager.cpp +++ b/src/network/access/qnetworkaccessmanager.cpp @@ -1689,7 +1689,7 @@ void QNetworkAccessManager::clearConnectionCache() \sa setAutoDeleteReplies, QNetworkRequest::AutoDeleteReplyOnFinishAttribute */ -bool QNetworkAccessManager::autoDeleteReplies() +bool QNetworkAccessManager::autoDeleteReplies() const { return d_func()->autoDeleteReplies; } diff --git a/src/network/access/qnetworkaccessmanager.h b/src/network/access/qnetworkaccessmanager.h index 601d0420ff..98498d07d2 100644 --- a/src/network/access/qnetworkaccessmanager.h +++ b/src/network/access/qnetworkaccessmanager.h @@ -167,7 +167,7 @@ public: void setRedirectPolicy(QNetworkRequest::RedirectPolicy policy); QNetworkRequest::RedirectPolicy redirectPolicy() const; - bool autoDeleteReplies(); + bool autoDeleteReplies() const; void setAutoDeleteReplies(bool autoDelete); Q_SIGNALS: -- cgit v1.2.3