AuroraMiddleware/qt5base-lts
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Ensure drawable size atomicity within a frame

Browse Source 
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ø <tor.arne.vestbo@qt.io>
This commit is contained in:
Laszlo Agocs 2019-09-23 13:31:57 +02:00
parent 08a601035c
commit 86876744f0
9 changed files with 100 additions and 100 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

50
src/gui/rhi/qrhi.cpp
View File

@ -1981,6 +1981,14 @@ QRhiResource::Type QRhiBuffer::resourceType() const
How the renderbuffer is implemented by a backend is not exposed to the
applications. In some cases it may be backed by ordinary textures, while in
others there may be a different kind of native resource used.
Renderbuffers that are used as (and are only used as) depth-stencil buffers
in combination with a QRhiSwapChain's color buffers should have the
UsedWithSwapChainOnly flag set. This serves a double purpose: such buffers,
depending on the backend and the underlying APIs, be more efficient, and
QRhi provides automatic sizing behavior to match the color buffers, which
means calling setPixelSize() and build() are not necessary for such
renderbuffers.
*/
/*!
@ -1996,12 +2004,15 @@ QRhiResource::Type QRhiBuffer::resourceType() const
Flag values for flags() and setFlags()
\value UsedWithSwapChainOnly For DepthStencil renderbuffers this indicates
that the renderbuffer is only used in combination with a QRhiSwapChain and
never in other ways. Relevant with some backends, while others ignore it.
With OpenGL where a separate windowing system interface API is in use (EGL,
GLX, etc.), the flag is important since it avoids creating any actual
resource as there is already a windowing system provided depth/stencil
buffer as requested by QSurfaceFormat.
that the renderbuffer is only used in combination with a QRhiSwapChain, and
never in any other way. This provides automatic sizing and resource
rebuilding, so calling setPixelSize() or build() is not needed whenever
this flag is set. This flag value may also trigger backend-specific
behavior, for example with OpenGL, where a separate windowing system
interface API is in use (EGL, GLX, etc.), the flag is especially important
as it avoids creating any actual renderbuffer resource as there is already
a windowing system provided depth/stencil buffer as requested by
QSurfaceFormat.
*/
/*!
@ -3351,7 +3362,7 @@ QRhiResource::Type QRhiGraphicsPipeline::resourceType() const
{
sc = rhi->newSwapChain();
ds = rhi->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);
sc->setWindow(window);
@ -3363,9 +3374,6 @@ QRhiResource::Type QRhiGraphicsPipeline::resourceType() const
void resizeSwapChain()
{
const QSize outputSize = sc->surfacePixelSize();
ds->setPixelSize(outputSize);
ds->build();
hasSwapChain = sc->buildOrResize();
}
@ -3559,10 +3567,24 @@ QRhiResource::Type QRhiSwapChain::resourceType() const
\return The size of the window's associated surface or layer. Do not assume
this is the same as QWindow::size() * QWindow::devicePixelRatio().
Can be called before buildOrResize() (but with window() already set), which
allows setting the correct size for the depth-stencil buffer that is then
used together with the swapchain's color buffers. Also used in combination
with currentPixelSize() to detect size changes.
\note Can also be called before buildOrResize(), if at least window() is
already set) This in combination with currentPixelSize() allows to detect
when a swapchain needs to be resized. However, watch out for the fact that
the size of the underlying native object (surface, layer, or similar) is
"live", so whenever this function is called, it returns the latest value
reported by the underlying implementation, without any atomicity guarantee.
Therefore, using this function to determine pixel sizes for graphics
resources that are used in a frame is strongly discouraged. Rely on
currentPixelSize() instead which returns a size that is atomic and will not
change between buildOrResize() invocations.
\note For depth-stencil buffers used in combination with the swapchain's
color buffers, it is strongly recommended to rely on the automatic sizing
and rebuilding behavior provided by the
QRhiRenderBuffer:UsedWithSwapChainOnly flag. Avoid querying the surface
size via this function just to get a size that can be passed to
QRhiRenderBuffer::setPixelSize() as that would suffer from the lack of
atomicity as described above.
\sa currentPixelSize()
*/

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

@ -3939,9 +3939,16 @@ bool QD3D11SwapChain::buildOrResize()
m_depthStencil->sampleCount(), m_sampleCount);
}
if (m_depthStencil && m_depthStencil->pixelSize() != pixelSize) {
qWarning("Depth-stencil buffer's size (%dx%d) does not match the surface size (%dx%d). Expect problems.",
m_depthStencil->pixelSize().width(), m_depthStencil->pixelSize().height(),
pixelSize.width(), pixelSize.height());
if (m_depthStencil->flags().testFlag(QRhiRenderBuffer::UsedWithSwapChainOnly)) {
m_depthStencil->setPixelSize(pixelSize);
if (!m_depthStencil->build())
qWarning("Failed to rebuild swapchain's associated depth-stencil buffer for size %dx%d",
pixelSize.width(), pixelSize.height());
} else {
qWarning("Depth-stencil buffer's size (%dx%d) does not match the surface size (%dx%d). Expect problems.",
m_depthStencil->pixelSize().width(), m_depthStencil->pixelSize().height(),
pixelSize.width(), pixelSize.height());
}
}
currentFrameSlot = 0;

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

@ -3707,6 +3707,13 @@ bool QGles2SwapChain::buildOrResize()
m_currentPixelSize = surfacePixelSize();
pixelSize = m_currentPixelSize;
if (m_depthStencil && m_depthStencil->flags().testFlag(QRhiRenderBuffer::UsedWithSwapChainOnly)
&& m_depthStencil->pixelSize() != pixelSize)
{
m_depthStencil->setPixelSize(pixelSize);
m_depthStencil->build();
}
rt.d.rp = QRHI_RES(QGles2RenderPassDescriptor, m_renderPassDesc);
rt.d.pixelSize = pixelSize;
rt.d.dpr = float(m_window->devicePixelRatio());

81
src/gui/rhi/qrhimetal.mm
View File

@ -1036,44 +1036,11 @@ void QRhiMetal::setVertexInput(QRhiCommandBuffer *cb,
}
}
QSize safeOutputSize(QRhiMetal *rhiD, QMetalCommandBuffer *cbD)
{
QSize size = cbD->currentTarget->pixelSize();
// So now we have the issue that the texture (drawable) size may have
// changed again since swapchain buildOrResize() was called. This can
// happen for example when interactively resizing the window a lot in one
// go, and command buffer building happens on a dedicated thread (f.ex.
// using the threaded render loop of Qt Quick).
//
// This is only an issue when running in debug mode with XCode because Metal
// validation will fail when setting viewport or scissor with the real size
// being smaller than what we think it is. So query the drawable size right
// here, in debug mode at least.
//
// In addition, we have to take the smaller of the two widths and heights
// to be safe, apparently. In some cases validation seems to think that the
// "render pass width" (or height) is the old(?) value.
#ifdef QT_DEBUG
if (cbD->currentTarget->resourceType() == QRhiResource::RenderTarget) {
Q_ASSERT(rhiD->currentSwapChain);
const QSize otherSize = rhiD->currentSwapChain->surfacePixelSize();
size.setWidth(qMin(size.width(), otherSize.width()));
size.setHeight(qMin(size.height(), otherSize.height()));
}
#else
Q_UNUSED(rhiD);
#endif
return size;
}
void QRhiMetal::setViewport(QRhiCommandBuffer *cb, const QRhiViewport &viewport)
{
QMetalCommandBuffer *cbD = QRHI_RES(QMetalCommandBuffer, cb);
Q_ASSERT(cbD->recordingPass == QMetalCommandBuffer::RenderPass);
const QSize outputSize = safeOutputSize(this, cbD);
const QSize outputSize = cbD->currentTarget->pixelSize();
// x,y is top-left in MTLViewportRect but bottom-left in QRhiViewport
float x, y, w, h;
@ -1105,7 +1072,7 @@ void QRhiMetal::setScissor(QRhiCommandBuffer *cb, const QRhiScissor &scissor)
QMetalCommandBuffer *cbD = QRHI_RES(QMetalCommandBuffer, cb);
Q_ASSERT(cbD->recordingPass == QMetalCommandBuffer::RenderPass);
Q_ASSERT(QRHI_RES(QMetalGraphicsPipeline, cbD->currentGraphicsPipeline)->m_flags.testFlag(QRhiGraphicsPipeline::UsesScissor));
const QSize outputSize = safeOutputSize(this, cbD);
const QSize outputSize = cbD->currentTarget->pixelSize();
// x,y is top-left in MTLScissorRect but bottom-left in QRhiScissor
int x, y, w, h;
@ -3529,20 +3496,8 @@ QRhiRenderTarget *QMetalSwapChain::currentFrameRenderTarget()
QSize QMetalSwapChain::surfacePixelSize()
{
// may be called before build, must not access other than m_*
NSView *v = (NSView *) m_window->winId();
if (v) {
CAMetalLayer *layer = (CAMetalLayer *) [v layer];
if (layer) {
CGSize size = layer.bounds.size;
size.width *= layer.contentsScale;
size.height *= layer.contentsScale;
layer.drawableSize = size;
return QSize(int(size.width), int(size.height));
}
}
return QSize();
Q_ASSERT(m_window);
return m_window->size() * m_window->devicePixelRatio();
}
QRhiRenderPassDescriptor *QMetalSwapChain::newCompatibleRenderPassDescriptor()
@ -3593,8 +3548,9 @@ bool QMetalSwapChain::buildOrResize()
return false;
}
NSView *v = (NSView *) window->winId();
d->layer = (CAMetalLayer *) [v layer];
NSView *view = reinterpret_cast<NSView *>(window->winId());
Q_ASSERT(view);
d->layer = static_cast<CAMetalLayer *>(view.layer);
Q_ASSERT(d->layer);
chooseFormats();
@ -3623,7 +3579,15 @@ bool QMetalSwapChain::buildOrResize()
d->layer.opaque = YES;
}
m_currentPixelSize = surfacePixelSize();
// Now set the layer's drawableSize which will stay set to the same value
// until the next buildOrResize(), thus ensuring atomicity with regards to
// the drawable size in frames.
CGSize layerSize = d->layer.bounds.size;
layerSize.width *= d->layer.contentsScale;
layerSize.height *= d->layer.contentsScale;
d->layer.drawableSize = layerSize;
m_currentPixelSize = QSizeF::fromCGSize(layerSize).toSize();
pixelSize = m_currentPixelSize;
[d->layer setDevice: rhiD->d->dev];
@ -3644,9 +3608,16 @@ bool QMetalSwapChain::buildOrResize()
m_depthStencil->sampleCount(), m_sampleCount);
}
if (m_depthStencil && m_depthStencil->pixelSize() != pixelSize) {
qWarning("Depth-stencil buffer's size (%dx%d) does not match the layer size (%dx%d). Expect problems.",
m_depthStencil->pixelSize().width(), m_depthStencil->pixelSize().height(),
pixelSize.width(), pixelSize.height());
if (m_depthStencil->flags().testFlag(QRhiRenderBuffer::UsedWithSwapChainOnly)) {
m_depthStencil->setPixelSize(pixelSize);
if (!m_depthStencil->build())
qWarning("Failed to rebuild swapchain's associated depth-stencil buffer for size %dx%d",
pixelSize.width(), pixelSize.height());
} else {
qWarning("Depth-stencil buffer's size (%dx%d) does not match the layer size (%dx%d). Expect problems.",
m_depthStencil->pixelSize().width(), m_depthStencil->pixelSize().height(),
pixelSize.width(), pixelSize.height());
}
}
rtWrapper.d->pixelSize = pixelSize;

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

@ -6315,9 +6315,16 @@ bool QVkSwapChain::buildOrResize()
m_depthStencil->sampleCount(), m_sampleCount);
}
if (m_depthStencil && m_depthStencil->pixelSize() != pixelSize) {
qWarning("Depth-stencil buffer's size (%dx%d) does not match the surface size (%dx%d). Expect problems.",
m_depthStencil->pixelSize().width(), m_depthStencil->pixelSize().height(),
pixelSize.width(), pixelSize.height());
if (m_depthStencil->flags().testFlag(QRhiRenderBuffer::UsedWithSwapChainOnly)) {
m_depthStencil->setPixelSize(pixelSize);
if (!m_depthStencil->build())
qWarning("Failed to rebuild swapchain's associated depth-stencil buffer for size %dx%d",
pixelSize.width(), pixelSize.height());
} else {
qWarning("Depth-stencil buffer's size (%dx%d) does not match the surface size (%dx%d). Expect problems.",
m_depthStencil->pixelSize().width(), m_depthStencil->pixelSize().height(),
pixelSize.width(), pixelSize.height());
}
}
if (!m_renderPassDesc)

10
tests/manual/rhi/hellominimalcrossgfxtriangle/hellominimalcrossgfxtriangle.cpp
View File

@ -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);

8
tests/manual/rhi/multiwindow/multiwindow.cpp
View File

@ -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);

8
tests/manual/rhi/multiwindow_threaded/multiwindow_threaded.cpp
View File

@ -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);

10
tests/manual/rhi/shared/examplefw.h
View File

@ -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);