From 8cdc9ac5b2310e42c4fecdb001e1f8ddeecc4aa1 Mon Sep 17 00:00:00 2001 From: Laszlo Agocs Date: Thu, 7 May 2020 16:23:48 +0200 Subject: rhi: vulkan: Fix calling finish() twice with some copy commands in-between MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The native command buffer handle was not updated, so the subsequent finish() call attempted to record an invalid VkCommandBuffer. The problem was not present with offscreen frames, only when finish() is called with a swapchain-based frame active. Task-number: QTBUG-84066 Change-Id: I9c4cb701c3dbbc28f237d6ae1cbf65aafd1fa95f Reviewed-by: Christian Strømme --- tests/auto/gui/rhi/qrhi/tst_qrhi.cpp | 159 ++++++++++++++++++++++++++++++----- 1 file changed, 140 insertions(+), 19 deletions(-) (limited to 'tests/auto/gui/rhi/qrhi/tst_qrhi.cpp') diff --git a/tests/auto/gui/rhi/qrhi/tst_qrhi.cpp b/tests/auto/gui/rhi/qrhi/tst_qrhi.cpp index f437a7f1f8..d3524a39b7 100644 --- a/tests/auto/gui/rhi/qrhi/tst_qrhi.cpp +++ b/tests/auto/gui/rhi/qrhi/tst_qrhi.cpp @@ -101,12 +101,16 @@ private slots: void renderToTextureTexturedQuadAndUniformBuffer(); void renderToWindowSimple_data(); void renderToWindowSimple(); + void finishWithinSwapchainFrame_data(); + void finishWithinSwapchainFrame(); void srbLayoutCompatibility_data(); void srbLayoutCompatibility(); void renderPassDescriptorCompatibility_data(); void renderPassDescriptorCompatibility(); private: + void setWindowType(QWindow *window, QRhi::Implementation impl); + struct { QRhiNullInitParams null; #ifdef TST_GL @@ -2081,26 +2085,8 @@ void tst_QRhi::renderToTextureTexturedQuadAndUniformBuffer() QCOMPARE(result1.pixel(28, 178), empty); } -void tst_QRhi::renderToWindowSimple_data() -{ - rhiTestData(); -} - -void tst_QRhi::renderToWindowSimple() +void tst_QRhi::setWindowType(QWindow *window, QRhi::Implementation impl) { - QFETCH(QRhi::Implementation, impl); - QFETCH(QRhiInitParams *, initParams); - -#ifdef Q_OS_WINRT - if (impl == QRhi::D3D11) - QSKIP("Skipping window-based QRhi rendering on WinRT as the platform and the D3D11 backend are not prepared for this yet"); -#endif - - QScopedPointer rhi(QRhi::create(impl, initParams, QRhi::Flags(), nullptr)); - if (!rhi) - QSKIP("QRhi could not be created, skipping testing rendering"); - - QScopedPointer window(new QWindow); switch (impl) { case QRhi::OpenGLES2: #if QT_CONFIG(opengl) @@ -2122,6 +2108,29 @@ void tst_QRhi::renderToWindowSimple() default: break; } +} + +void tst_QRhi::renderToWindowSimple_data() +{ + rhiTestData(); +} + +void tst_QRhi::renderToWindowSimple() +{ + QFETCH(QRhi::Implementation, impl); + QFETCH(QRhiInitParams *, initParams); + +#ifdef Q_OS_WINRT + if (impl == QRhi::D3D11) + QSKIP("Skipping window-based QRhi rendering on WinRT as the platform and the D3D11 backend are not prepared for this yet"); +#endif + + QScopedPointer rhi(QRhi::create(impl, initParams, QRhi::Flags(), nullptr)); + if (!rhi) + QSKIP("QRhi could not be created, skipping testing rendering"); + + QScopedPointer window(new QWindow); + setWindowType(window.data(), impl); window->setGeometry(0, 0, 640, 480); window->show(); @@ -2241,6 +2250,118 @@ void tst_QRhi::renderToWindowSimple() QVERIFY(redCount < blueCount); } +void tst_QRhi::finishWithinSwapchainFrame_data() +{ + rhiTestData(); +} + +void tst_QRhi::finishWithinSwapchainFrame() +{ + QFETCH(QRhi::Implementation, impl); + QFETCH(QRhiInitParams *, initParams); + +#ifdef Q_OS_WINRT + if (impl == QRhi::D3D11) + QSKIP("Skipping window-based QRhi rendering on WinRT as the platform and the D3D11 backend are not prepared for this yet"); +#endif + + QScopedPointer rhi(QRhi::create(impl, initParams, QRhi::Flags(), nullptr)); + if (!rhi) + QSKIP("QRhi could not be created, skipping testing rendering"); + + QScopedPointer window(new QWindow); + setWindowType(window.data(), impl); + + window->setGeometry(0, 0, 640, 480); + window->show(); + QVERIFY(QTest::qWaitForWindowExposed(window.data())); + + QScopedPointer swapChain(rhi->newSwapChain()); + swapChain->setWindow(window.data()); + swapChain->setFlags(QRhiSwapChain::UsedAsTransferSource); + QScopedPointer rpDesc(swapChain->newCompatibleRenderPassDescriptor()); + swapChain->setRenderPassDescriptor(rpDesc.data()); + QVERIFY(swapChain->buildOrResize()); + + QScopedPointer srb(rhi->newShaderResourceBindings()); + QVERIFY(srb->build()); + + QScopedPointer pipeline(rhi->newGraphicsPipeline()); + QShader vs = loadShader(":/data/simple.vert.qsb"); + QVERIFY(vs.isValid()); + QShader fs = loadShader(":/data/simple.frag.qsb"); + QVERIFY(fs.isValid()); + pipeline->setShaderStages({ { QRhiShaderStage::Vertex, vs }, { QRhiShaderStage::Fragment, fs } }); + QRhiVertexInputLayout inputLayout; + inputLayout.setBindings({ { 2 * sizeof(float) } }); + inputLayout.setAttributes({ { 0, 0, QRhiVertexInputAttribute::Float2, 0 } }); + pipeline->setVertexInputLayout(inputLayout); + pipeline->setShaderResourceBindings(srb.data()); + pipeline->setRenderPassDescriptor(rpDesc.data()); + QVERIFY(pipeline->build()); + + static const float vertices[] = { + -1.0f, -1.0f, + 1.0f, -1.0f, + 0.0f, 1.0f + }; + QScopedPointer vbuf(rhi->newBuffer(QRhiBuffer::Immutable, QRhiBuffer::VertexBuffer, sizeof(vertices))); + QVERIFY(vbuf->build()); + + // exercise begin/endExternal() just a little bit, hence ExternalContentsInPass + QVERIFY(rhi->beginFrame(swapChain.data(), QRhi::ExternalContentsInPass) == QRhi::FrameOpSuccess); + QRhiCommandBuffer *cb = swapChain->currentFrameCommandBuffer(); + QRhiRenderTarget *rt = swapChain->currentFrameRenderTarget(); + const QSize outputSize = swapChain->currentPixelSize(); + + // repeat a sequence of upload, renderpass, readback, finish a number of + // times within the same frame + for (int i = 0; i < 5; ++i) { + QRhiResourceUpdateBatch *updates = rhi->nextResourceUpdateBatch(); + updates->uploadStaticBuffer(vbuf.data(), vertices); + + cb->beginPass(rt, Qt::blue, { 1.0f, 0 }, updates); + + // just have some commands, do not bother with draw calls + cb->setGraphicsPipeline(pipeline.data()); + QRhiViewport viewport(0, 0, float(outputSize.width()), float(outputSize.height())); + cb->setViewport(viewport); + + // do a dummy begin/endExternal round: interesting for Vulkan because + // there this may start end then submit a secondary command buffer + cb->beginExternal(); + cb->endExternal(); + + cb->endPass(); + + QRhiReadbackResult readResult; + bool ok = false; + readResult.completed = [&readResult, &ok, impl] { + QImage wrapperImage(reinterpret_cast(readResult.data.constData()), + readResult.pixelSize.width(), readResult.pixelSize.height(), + QImage::Format_ARGB32_Premultiplied); + if (readResult.format == QRhiTexture::RGBA8) + wrapperImage = wrapperImage.rgbSwapped(); + + if (impl != QRhi::Null) + ok = qBlue(wrapperImage.pixel(43, 89)) > 250; + else + ok = true; // the Null backend does not actually render + }; + QRhiResourceUpdateBatch *readbackBatch = rhi->nextResourceUpdateBatch(); + readbackBatch->readBackTexture({}, &readResult); // read back the current backbuffer + cb->resourceUpdate(readbackBatch); + + // force submit what we have so far, wait for the queue, and then start + // a new primary command buffer + rhi->finish(); + + QVERIFY(ok); + } + + rhi->endFrame(swapChain.data()); +} + void tst_QRhi::srbLayoutCompatibility_data() { rhiTestData(); -- cgit v1.2.3