From 86876744f07cbaa01daca6869b896741878c39a3 Mon Sep 17 00:00:00 2001 From: Laszlo Agocs Date: Mon, 23 Sep 2019 13:31:57 +0200 Subject: Ensure drawable size atomicity within a frame MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Revert surfacePixelSize() to be a getter only. With Metal this will mean returning the "live" layer size (and so not the layer.drawableSize), which is in line with what we expect with other backends. Instead, we leave it to the swapchain's buildOrResize() to "commit" the size by setting drawableSize on the layer. With typical application or Qt Quick logic this ensures that layer.drawableSize is set once and stays static until we get to process the next resize - on the rendering thread. This of course would still mean that there was a race when a client queries surfacePixelSize() to set the depth-stencil buffer size that is associated with a swapchain. (because that must happen before calling buildOrResize() according to the current semantics) That can however be solved in a quite elegant way, it turns out, because we already have a flag that indicates if a QRhiRenderBuffer is used in combination with (and only in combination with) a swapchain. If we simply say that setting the UsedWithSwapChainOnly flag provides automatic sizing as well (so no setPixelSize() call is needed), clients can simply get rid of the problematic surfacePixelSize() query and everything works. Task-number: QTBUG-78641 Change-Id: Ib1bfc9ef8531bcce033d1f1e5d4d5b4984d6d69f Reviewed-by: Tor Arne Vestbø --- .../hellominimalcrossgfxtriangle.cpp | 10 +++------- tests/manual/rhi/multiwindow/multiwindow.cpp | 8 ++------ tests/manual/rhi/multiwindow_threaded/multiwindow_threaded.cpp | 8 +++----- tests/manual/rhi/shared/examplefw.h | 10 +++------- 4 files changed, 11 insertions(+), 25 deletions(-) (limited to 'tests/manual') diff --git a/tests/manual/rhi/hellominimalcrossgfxtriangle/hellominimalcrossgfxtriangle.cpp b/tests/manual/rhi/hellominimalcrossgfxtriangle/hellominimalcrossgfxtriangle.cpp index 3c39ff1719..ea1fefc308 100644 --- a/tests/manual/rhi/hellominimalcrossgfxtriangle/hellominimalcrossgfxtriangle.cpp +++ b/tests/manual/rhi/hellominimalcrossgfxtriangle/hellominimalcrossgfxtriangle.cpp @@ -298,7 +298,7 @@ void Window::init() m_sc = m_r->newSwapChain(); // allow depth-stencil, although we do not actually enable depth test/write for the triangle m_ds = m_r->newRenderBuffer(QRhiRenderBuffer::DepthStencil, - QSize(), // no need to set the size yet + QSize(), // no need to set the size here, due to UsedWithSwapChainOnly 1, QRhiRenderBuffer::UsedWithSwapChainOnly); releasePool << m_ds; @@ -376,16 +376,12 @@ void Window::releaseResources() void Window::resizeSwapChain() { - const QSize outputSize = m_sc->surfacePixelSize(); - - m_ds->setPixelSize(outputSize); - m_ds->build(); // == m_ds->release(); m_ds->build(); - - m_hasSwapChain = m_sc->buildOrResize(); + m_hasSwapChain = m_sc->buildOrResize(); // also handles m_ds m_elapsedMs = 0; m_elapsedCount = 0; + const QSize outputSize = m_sc->currentPixelSize(); m_proj = m_r->clipSpaceCorrMatrix(); m_proj.perspective(45.0f, outputSize.width() / (float) outputSize.height(), 0.01f, 100.0f); m_proj.translate(0, 0, -4); diff --git a/tests/manual/rhi/multiwindow/multiwindow.cpp b/tests/manual/rhi/multiwindow/multiwindow.cpp index 4c5d5c345a..4d5de16a58 100644 --- a/tests/manual/rhi/multiwindow/multiwindow.cpp +++ b/tests/manual/rhi/multiwindow/multiwindow.cpp @@ -400,7 +400,7 @@ void Window::init() { m_sc = r.r->newSwapChain(); m_ds = r.r->newRenderBuffer(QRhiRenderBuffer::DepthStencil, - QSize(), // no need to set the size yet + QSize(), 1, QRhiRenderBuffer::UsedWithSwapChainOnly); m_releasePool << m_ds; @@ -427,13 +427,9 @@ void Window::releaseResources() void Window::resizeSwapChain() { - const QSize outputSize = m_sc->surfacePixelSize(); - - m_ds->setPixelSize(outputSize); - m_ds->build(); - m_hasSwapChain = m_sc->buildOrResize(); + const QSize outputSize = m_sc->currentPixelSize(); m_proj = r.r->clipSpaceCorrMatrix(); m_proj.perspective(45.0f, outputSize.width() / (float) outputSize.height(), 0.01f, 1000.0f); m_proj.translate(0, 0, -4); diff --git a/tests/manual/rhi/multiwindow_threaded/multiwindow_threaded.cpp b/tests/manual/rhi/multiwindow_threaded/multiwindow_threaded.cpp index 53185bddb2..37c6cd04c3 100644 --- a/tests/manual/rhi/multiwindow_threaded/multiwindow_threaded.cpp +++ b/tests/manual/rhi/multiwindow_threaded/multiwindow_threaded.cpp @@ -441,7 +441,7 @@ void Renderer::init() { m_sc = r->newSwapChain(); m_ds = r->newRenderBuffer(QRhiRenderBuffer::DepthStencil, - QSize(), // no need to set the size yet + QSize(), 1, QRhiRenderBuffer::UsedWithSwapChainOnly); m_releasePool << m_ds; @@ -543,11 +543,9 @@ void Renderer::render(bool newlyExposed, bool wakeBeforePresent) auto buildOrResizeSwapChain = [this] { qDebug() << "renderer" << this << "build or resize swapchain for window" << window; - const QSize outputSize = m_sc->surfacePixelSize(); - qDebug() << " size is" << outputSize; - m_ds->setPixelSize(outputSize); - m_ds->build(); m_hasSwapChain = m_sc->buildOrResize(); + const QSize outputSize = m_sc->currentPixelSize(); + qDebug() << " size is" << outputSize; m_proj = r->clipSpaceCorrMatrix(); m_proj.perspective(45.0f, outputSize.width() / (float) outputSize.height(), 0.01f, 100.0f); m_proj.translate(0, 0, -4); diff --git a/tests/manual/rhi/shared/examplefw.h b/tests/manual/rhi/shared/examplefw.h index cd62530c82..d28bbea0a8 100644 --- a/tests/manual/rhi/shared/examplefw.h +++ b/tests/manual/rhi/shared/examplefw.h @@ -307,7 +307,7 @@ void Window::init() m_sc = m_r->newSwapChain(); // allow depth-stencil, although we do not actually enable depth test/write for the triangle m_ds = m_r->newRenderBuffer(QRhiRenderBuffer::DepthStencil, - QSize(), // no need to set the size yet + QSize(), // no need to set the size here, due to UsedWithSwapChainOnly sampleCount, QRhiRenderBuffer::UsedWithSwapChainOnly); m_sc->setWindow(this); @@ -344,16 +344,12 @@ void Window::releaseResources() void Window::resizeSwapChain() { - const QSize outputSize = m_sc->surfacePixelSize(); - - m_ds->setPixelSize(outputSize); - m_ds->build(); // == m_ds->release(); m_ds->build(); - - m_hasSwapChain = m_sc->buildOrResize(); + m_hasSwapChain = m_sc->buildOrResize(); // also handles m_ds m_frameCount = 0; m_timer.restart(); + const QSize outputSize = m_sc->currentPixelSize(); m_proj = m_r->clipSpaceCorrMatrix(); m_proj.perspective(45.0f, outputSize.width() / (float) outputSize.height(), 0.01f, 1000.0f); m_proj.translate(0, 0, -4); -- cgit v1.2.3