From 753e4d82be966b82ff6ba41a0e7c4452f494790a Mon Sep 17 00:00:00 2001 From: Laszlo Agocs Date: Sun, 23 Feb 2020 15:05:44 +0100 Subject: rhi: Execute pending host writes on nativeBuffer() query MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Otherwise it is impossible to write an application that pulls out the VkBuffer for a Dynamic QRhiBuffer, and then uses it with custom Vulkan operations that read from the buffer. More precisely, the problem arises only if the buffer in question is not used in combination with any QRhi operations, because in that case there is nothing that would trigger doing the host writes queued up by a resource batch's updateDynamicBuffer(). Task-number: QTBUG-82435 Change-Id: Ieb54422f1493921bc6d4d029be56130cd3a1362a Reviewed-by: Christian Strømme --- src/gui/rhi/qrhivulkan.cpp | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) (limited to 'src/gui/rhi/qrhivulkan.cpp') diff --git a/src/gui/rhi/qrhivulkan.cpp b/src/gui/rhi/qrhivulkan.cpp index d378e2a4ad..8f6f118c9b 100644 --- a/src/gui/rhi/qrhivulkan.cpp +++ b/src/gui/rhi/qrhivulkan.cpp @@ -2903,7 +2903,7 @@ void QRhiVulkan::enqueueResourceUpdates(QVkCommandBuffer *cbD, QRhiResourceUpdat } else if (u.type == QRhiResourceUpdateBatchPrivate::BufferOp::Read) { QVkBuffer *bufD = QRHI_RES(QVkBuffer, u.buf); if (bufD->m_type == QRhiBuffer::Dynamic) { - executeBufferHostWritesForCurrentFrame(bufD); + executeBufferHostWritesForSlot(bufD, currentFrameSlot); void *p = nullptr; VmaAllocation a = toVmaAllocation(bufD->allocations[currentFrameSlot]); VkResult err = vmaMapMemory(toVmaAllocator(allocator), a, &p); @@ -3300,14 +3300,14 @@ void QRhiVulkan::enqueueResourceUpdates(QVkCommandBuffer *cbD, QRhiResourceUpdat ud->free(); } -void QRhiVulkan::executeBufferHostWritesForCurrentFrame(QVkBuffer *bufD) +void QRhiVulkan::executeBufferHostWritesForSlot(QVkBuffer *bufD, int slot) { - if (bufD->pendingDynamicUpdates[currentFrameSlot].isEmpty()) + if (bufD->pendingDynamicUpdates[slot].isEmpty()) return; Q_ASSERT(bufD->m_type == QRhiBuffer::Dynamic); void *p = nullptr; - VmaAllocation a = toVmaAllocation(bufD->allocations[currentFrameSlot]); + VmaAllocation a = toVmaAllocation(bufD->allocations[slot]); // The vmaMap/Unmap are basically a no-op when persistently mapped since it // refcounts; this is great because we don't need to care if the allocation // was created as persistently mapped or not. @@ -3318,7 +3318,7 @@ void QRhiVulkan::executeBufferHostWritesForCurrentFrame(QVkBuffer *bufD) } int changeBegin = -1; int changeEnd = -1; - for (const QRhiResourceUpdateBatchPrivate::BufferOp &u : qAsConst(bufD->pendingDynamicUpdates[currentFrameSlot])) { + for (const QRhiResourceUpdateBatchPrivate::BufferOp &u : qAsConst(bufD->pendingDynamicUpdates[slot])) { Q_ASSERT(bufD == QRHI_RES(QVkBuffer, u.buf)); memcpy(static_cast(p) + u.offset, u.data.constData(), size_t(u.data.size())); if (changeBegin == -1 || u.offset < changeBegin) @@ -3330,7 +3330,7 @@ void QRhiVulkan::executeBufferHostWritesForCurrentFrame(QVkBuffer *bufD) if (changeBegin >= 0) vmaFlushAllocation(toVmaAllocator(allocator), a, VkDeviceSize(changeBegin), VkDeviceSize(changeEnd - changeBegin)); - bufD->pendingDynamicUpdates[currentFrameSlot].clear(); + bufD->pendingDynamicUpdates[slot].clear(); } static void qrhivk_releaseBuffer(const QRhiVulkan::DeferredReleaseEntry &e, void *allocator) @@ -4166,7 +4166,7 @@ void QRhiVulkan::setShaderResources(QRhiCommandBuffer *cb, QRhiShaderResourceBin Q_ASSERT(bufD->m_usage.testFlag(QRhiBuffer::UniformBuffer)); if (bufD->m_type == QRhiBuffer::Dynamic) - executeBufferHostWritesForCurrentFrame(bufD); + executeBufferHostWritesForSlot(bufD, currentFrameSlot); bufD->lastActiveFrameSlot = currentFrameSlot; trackedRegisterBuffer(&passResTracker, bufD, bufD->m_type == QRhiBuffer::Dynamic ? currentFrameSlot : 0, @@ -4240,7 +4240,7 @@ void QRhiVulkan::setShaderResources(QRhiCommandBuffer *cb, QRhiShaderResourceBin Q_ASSERT(bufD->m_usage.testFlag(QRhiBuffer::StorageBuffer)); if (bufD->m_type == QRhiBuffer::Dynamic) - executeBufferHostWritesForCurrentFrame(bufD); + executeBufferHostWritesForSlot(bufD, currentFrameSlot); bufD->lastActiveFrameSlot = currentFrameSlot; QRhiPassResourceTracker::BufferAccess access; @@ -4349,7 +4349,7 @@ void QRhiVulkan::setVertexInput(QRhiCommandBuffer *cb, Q_ASSERT(bufD->m_usage.testFlag(QRhiBuffer::VertexBuffer)); bufD->lastActiveFrameSlot = currentFrameSlot; if (bufD->m_type == QRhiBuffer::Dynamic) - executeBufferHostWritesForCurrentFrame(bufD); + executeBufferHostWritesForSlot(bufD, currentFrameSlot); const VkBuffer vkvertexbuf = bufD->buffers[bufD->m_type == QRhiBuffer::Dynamic ? currentFrameSlot : 0]; if (cbD->currentVertexBuffers[inputSlot] != vkvertexbuf @@ -4395,7 +4395,7 @@ void QRhiVulkan::setVertexInput(QRhiCommandBuffer *cb, Q_ASSERT(ibufD->m_usage.testFlag(QRhiBuffer::IndexBuffer)); ibufD->lastActiveFrameSlot = currentFrameSlot; if (ibufD->m_type == QRhiBuffer::Dynamic) - executeBufferHostWritesForCurrentFrame(ibufD); + executeBufferHostWritesForSlot(ibufD, currentFrameSlot); const int slot = ibufD->m_type == QRhiBuffer::Dynamic ? currentFrameSlot : 0; const VkBuffer vkindexbuf = ibufD->buffers[slot]; @@ -5188,10 +5188,13 @@ bool QVkBuffer::build() QRhiBuffer::NativeBuffer QVkBuffer::nativeBuffer() { if (m_type == Dynamic) { + QRHI_RES_RHI(QRhiVulkan); NativeBuffer b; Q_ASSERT(sizeof(b.objects) / sizeof(b.objects[0]) >= size_t(QVK_FRAMES_IN_FLIGHT)); - for (int i = 0; i < QVK_FRAMES_IN_FLIGHT; ++i) + for (int i = 0; i < QVK_FRAMES_IN_FLIGHT; ++i) { + rhiD->executeBufferHostWritesForSlot(this, i); b.objects[i] = &buffers[i]; + } b.slotCount = QVK_FRAMES_IN_FLIGHT; return b; } -- cgit v1.2.3 From bd2b77120e1db8cb991aff23dd1f99cec792fa8e Mon Sep 17 00:00:00 2001 From: Laszlo Agocs Date: Sat, 29 Feb 2020 15:38:07 +0100 Subject: rhi: vulkan: Sanitize device extension handling Instead of qputenv("QT_VULKAN_DEVICE_EXTENSIONS", "VK_KHR_get_memory_requirements2;VK_NV_ray_tracing"); one can now do params.deviceExtensions = { "VK_KHR_get_memory_requirements2", "VK_NV_ray_tracing" }; on the QRhiVulkanInitParams passed to QRhi::create(). The environment variable stays important for Qt Quick applications, which provide no configurability for the QRhi construction (yet). On the other hand, applications using QRhi directly can now also use the new approach to specify the list of device extensions to enable. In addition, take QVulkanInfoVector into use. There is no reason not to rely on the infrastructure provided by QVulkanInstance. This also implies showing an informative warning for unsupported extensions, instead of merely failing the device creation. (applications will likely not be able to recover of course, but at least the reason for failing is made obvious this way) Task-number: QTBUG-82435 Change-Id: Ib47fd1a10c02be5ceef2c973e61e896c34f92fa3 Reviewed-by: Eirik Aavitsland --- src/gui/rhi/qrhivulkan.cpp | 60 ++++++++++++++++++++++++++++++++-------------- 1 file changed, 42 insertions(+), 18 deletions(-) (limited to 'src/gui/rhi/qrhivulkan.cpp') diff --git a/src/gui/rhi/qrhivulkan.cpp b/src/gui/rhi/qrhivulkan.cpp index 8f6f118c9b..60ab15e89d 100644 --- a/src/gui/rhi/qrhivulkan.cpp +++ b/src/gui/rhi/qrhivulkan.cpp @@ -149,6 +149,10 @@ QT_BEGIN_NAMESPACE for other windows as well, as long as they all have their QWindow::surfaceType() set to QSurface::VulkanSurface. + To request additional extensions to be enabled on the Vulkan device, list them + in deviceExtensions. This can be relevant when integrating with native Vulkan + rendering code. + \section2 Working with existing Vulkan devices When interoperating with another graphics engine, it may be necessary to @@ -299,6 +303,7 @@ QRhiVulkan::QRhiVulkan(QRhiVulkanInitParams *params, QRhiVulkanNativeHandles *im { inst = params->inst; maybeWindow = params->window; // may be null + requestedDeviceExtensions = params->deviceExtensions; importedDevice = importDevice != nullptr; if (importedDevice) { @@ -463,40 +468,59 @@ bool QRhiVulkan::create(QRhi::Flags flags) if (inst->layers().contains("VK_LAYER_LUNARG_standard_validation")) devLayers.append("VK_LAYER_LUNARG_standard_validation"); + QVulkanInfoVector devExts; uint32_t devExtCount = 0; f->vkEnumerateDeviceExtensionProperties(physDev, nullptr, &devExtCount, nullptr); - QVector devExts(devExtCount); - f->vkEnumerateDeviceExtensionProperties(physDev, nullptr, &devExtCount, devExts.data()); + if (devExtCount) { + QVector extProps(devExtCount); + f->vkEnumerateDeviceExtensionProperties(physDev, nullptr, &devExtCount, extProps.data()); + for (const VkExtensionProperties &p : qAsConst(extProps)) + devExts.append({ p.extensionName, p.specVersion }); + } qCDebug(QRHI_LOG_INFO, "%d device extensions available", devExts.count()); QVector requestedDevExts; requestedDevExts.append("VK_KHR_swapchain"); debugMarkersAvailable = false; + if (devExts.contains(VK_EXT_DEBUG_MARKER_EXTENSION_NAME)) { + requestedDevExts.append(VK_EXT_DEBUG_MARKER_EXTENSION_NAME); + debugMarkersAvailable = true; + } + vertexAttribDivisorAvailable = false; - for (const VkExtensionProperties &ext : devExts) { - if (!strcmp(ext.extensionName, VK_EXT_DEBUG_MARKER_EXTENSION_NAME)) { - requestedDevExts.append(VK_EXT_DEBUG_MARKER_EXTENSION_NAME); - debugMarkersAvailable = true; - } else if (!strcmp(ext.extensionName, VK_EXT_VERTEX_ATTRIBUTE_DIVISOR_EXTENSION_NAME)) { - if (inst->extensions().contains(QByteArrayLiteral("VK_KHR_get_physical_device_properties2"))) { - requestedDevExts.append(VK_EXT_VERTEX_ATTRIBUTE_DIVISOR_EXTENSION_NAME); - vertexAttribDivisorAvailable = true; - } + if (devExts.contains(VK_EXT_VERTEX_ATTRIBUTE_DIVISOR_EXTENSION_NAME)) { + if (inst->extensions().contains(QByteArrayLiteral("VK_KHR_get_physical_device_properties2"))) { + requestedDevExts.append(VK_EXT_VERTEX_ATTRIBUTE_DIVISOR_EXTENSION_NAME); + vertexAttribDivisorAvailable = true; } } - QByteArrayList envExtList; - if (qEnvironmentVariableIsSet("QT_VULKAN_DEVICE_EXTENSIONS")) { - envExtList = qgetenv("QT_VULKAN_DEVICE_EXTENSIONS").split(';'); - for (auto ext : requestedDevExts) - envExtList.removeAll(ext); - for (const QByteArray &ext : envExtList) { - if (!ext.isEmpty()) + for (const QByteArray &ext : requestedDeviceExtensions) { + if (!ext.isEmpty()) { + if (devExts.contains(ext)) requestedDevExts.append(ext.constData()); + else + qWarning("Device extension %s is not supported", ext.constData()); } } + QByteArrayList envExtList = qgetenv("QT_VULKAN_DEVICE_EXTENSIONS").split(';'); + for (const QByteArray &ext : envExtList) { + if (!ext.isEmpty() && !requestedDevExts.contains(ext)) { + if (devExts.contains(ext)) + requestedDevExts.append(ext.constData()); + else + qWarning("Device extension %s is not supported", ext.constData()); + } + } + + if (QRHI_LOG_INFO().isEnabled(QtDebugMsg)) { + qCDebug(QRHI_LOG_INFO, "Enabling device extensions:"); + for (const char *ext : requestedDevExts) + qCDebug(QRHI_LOG_INFO, " %s", ext); + } + VkDeviceCreateInfo devInfo; memset(&devInfo, 0, sizeof(devInfo)); devInfo.sType = VK_STRUCTURE_TYPE_DEVICE_CREATE_INFO; -- cgit v1.2.3 From bbc52a043da3b38a2d44a5502c8587a1fdc8ad59 Mon Sep 17 00:00:00 2001 From: Laszlo Agocs Date: Sat, 29 Feb 2020 16:47:55 +0100 Subject: rhi: Add a way to communicate back the native image layout for a QRhiTexture Relevant when doing custom rendering combined with QRhi, and only for APIs like Vulkan, where image layouts are a thing. As shown by demo apps, it is not currently possible to implement a correct application that renders or raytraces into a QRhiTexture's backing VkImage, and then uses that QRhiTexture in a QRhi-based render pass. This is because QRhi has no knowledge of the image layout if it changes due to commands recorded by direct Vulkan calls, and not via QRhi itself. So, except for certain simple cases, one will end up with incorrect image layout transitions in the barriers. (at minimum this will be caught by the validation layer) To remedy this, add a simple function taking the layout as int (we already do the opposite in nativeTexture()). Task-number: QTBUG-82435 Change-Id: Ic9e9c1b820b018f3b236742f99fe99fa6de63d36 Reviewed-by: Eirik Aavitsland --- src/gui/rhi/qrhivulkan.cpp | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'src/gui/rhi/qrhivulkan.cpp') diff --git a/src/gui/rhi/qrhivulkan.cpp b/src/gui/rhi/qrhivulkan.cpp index 60ab15e89d..a92c3e14e9 100644 --- a/src/gui/rhi/qrhivulkan.cpp +++ b/src/gui/rhi/qrhivulkan.cpp @@ -5563,6 +5563,11 @@ QRhiTexture::NativeTexture QVkTexture::nativeTexture() return {&image, usageState.layout}; } +void QVkTexture::setNativeLayout(int layout) +{ + usageState.layout = VkImageLayout(layout); +} + VkImageView QVkTexture::imageViewForLevel(int level) { Q_ASSERT(level >= 0 && level < int(mipLevelCount)); -- cgit v1.2.3