rhi: vulkan: metal: Ensure ms color data is written out on PreserveColor

Fixes: QTBUG-123211
Pick-to: 6.7 6.6
Change-Id: Id037f8c5a69c2b0ec18d92fe8bb5a34a0a2b0ea0
Reviewed-by: Andy Nichols <andy.nichols@qt.io>
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>

3 files changed, 28 insertions, 7 deletions

diff --git a/src/gui/rhi/qrhi.cpp b/src/gui/rhi/qrhi.cpp
index 4940b5d85e..874484b518 100644
--- a/src/gui/rhi/qrhi.cpp
+++ b/src/gui/rhi/qrhi.cpp
@@ -2545,9 +2545,10 @@ QRhiColorAttachment::QRhiColorAttachment(QRhiRenderBuffer *renderBuffer)
     \note when multisample resolving is enabled, the multisample data may not be
     written out at all. This means that the multisample texture in a color
     attachment must not be used afterwards with shaders for sampling (or other
-    puroses) whenever a resolve texture is set, since the multisample color
+    purposes) whenever a resolve texture is set, since the multisample color
     buffer is merely an intermediate storage then that gets no data written back
-    on some GPU architectures at all.
+    on some GPU architectures at all. See
+    \l{QRhiTextureRenderTarget::Flag}{PreserveColorContents} for more details.
 
     \note When using setDepthTexture(), not setDepthStencilBuffer(), and the
     depth (stencil) data is not of interest afterwards, set the
@@ -5063,7 +5064,19 @@ QRhiResource::Type QRhiSwapChainRenderTarget::resourceType() const
     \value PreserveColorContents Indicates that the contents of the color
     attachments is to be loaded when starting a render pass, instead of
     clearing. This is potentially more expensive, especially on mobile (tiled)
-    GPUs, but allows preserving the existing contents between passes.
+    GPUs, but allows preserving the existing contents between passes. When doing
+    multisample rendering with a resolve texture set, setting this flag also
+    requests the multisample color data to be stored (written out) to the
+    multisample texture or render buffer. (for non-multisample rendering the
+    color data is always stored, but for MSAA storing the multisample data
+    decreases efficiency for certain GPU architectures, hence defaulting to not
+    writing it out) Note however that this is non-portable: in some cases there
+    is no intermediate multisample texture on the graphics API level, e.g. when
+    using OpenGL ES's \c{GL_EXT_multisampled_render_to_texture} as it is all
+    implicit, handled by the OpenGL ES implementation. In that case,
+    PreserveColorContents will likely have no effect. Therefore, avoid relying
+    on this flag when using multisample rendering and the color attachment is
+    using a multisample QRhiTexture (not QRhiRenderBuffer).
 
     \value PreserveDepthStencilContents Indicates that the contents of the
     depth texture is to be loaded when starting a render pass, instead
diff --git a/src/gui/rhi/qrhimetal.mm b/src/gui/rhi/qrhimetal.mm
index 6302a958c9..2cdac0531a 100644
--- a/src/gui/rhi/qrhimetal.mm
+++ b/src/gui/rhi/qrhimetal.mm
@@ -365,6 +365,8 @@ struct QMetalRenderTargetData
         id<MTLTexture> dsTex = nil;
         bool hasStencil = false;
         bool depthNeedsStore = false;
+        bool preserveColor = false;
+        bool preserveDs = false;
     } fb;
 
     QRhiRenderTargetAttachmentTracker::ResIdList currentResIdList;
@@ -2952,11 +2954,11 @@ void QRhiMetal::beginPass(QRhiCommandBuffer *cb,
         if (!QRhiRenderTargetAttachmentTracker::isUpToDate<QMetalTexture, QMetalRenderBuffer>(rtTex->description(), rtD->currentResIdList))
             rtTex->create();
         cbD->d->currentPassRpDesc = d->createDefaultRenderPass(rtD->dsAttCount, colorClearValue, depthStencilClearValue, rtD->colorAttCount);
-        if (rtTex->m_flags.testFlag(QRhiTextureRenderTarget::PreserveColorContents)) {
+        if (rtD->fb.preserveColor) {
             for (uint i = 0; i < uint(rtD->colorAttCount); ++i)
                 cbD->d->currentPassRpDesc.colorAttachments[i].loadAction = MTLLoadActionLoad;
         }
-        if (rtD->dsAttCount && rtTex->m_flags.testFlag(QRhiTextureRenderTarget::PreserveDepthStencilContents)) {
+        if (rtD->dsAttCount && rtD->fb.preserveDs) {
             cbD->d->currentPassRpDesc.depthAttachment.loadAction = MTLLoadActionLoad;
             cbD->d->currentPassRpDesc.stencilAttachment.loadAction = MTLLoadActionLoad;
         }
@@ -2990,7 +2992,8 @@ void QRhiMetal::beginPass(QRhiCommandBuffer *cb,
         cbD->d->currentPassRpDesc.colorAttachments[i].depthPlane = NSUInteger(rtD->fb.colorAtt[i].slice);
         cbD->d->currentPassRpDesc.colorAttachments[i].level = NSUInteger(rtD->fb.colorAtt[i].level);
         if (rtD->fb.colorAtt[i].resolveTex) {
-            cbD->d->currentPassRpDesc.colorAttachments[i].storeAction = MTLStoreActionMultisampleResolve;
+            cbD->d->currentPassRpDesc.colorAttachments[i].storeAction = rtD->fb.preserveColor ? MTLStoreActionStoreAndMultisampleResolve
+                                                                                              : MTLStoreActionMultisampleResolve;
             cbD->d->currentPassRpDesc.colorAttachments[i].resolveTexture = rtD->fb.colorAtt[i].resolveTex;
             cbD->d->currentPassRpDesc.colorAttachments[i].resolveSlice = NSUInteger(rtD->fb.colorAtt[i].resolveLayer);
             cbD->d->currentPassRpDesc.colorAttachments[i].resolveLevel = NSUInteger(rtD->fb.colorAtt[i].resolveLevel);
@@ -4227,6 +4230,7 @@ bool QMetalTextureRenderTarget::create()
             d->fb.dsTex = depthTexD->d->tex;
             d->fb.hasStencil = rhiD->isStencilSupportingFormat(depthTexD->format());
             d->fb.depthNeedsStore = !m_flags.testFlag(DoNotStoreDepthStencilContents);
+            d->fb.preserveDs = m_flags.testFlag(QRhiTextureRenderTarget::PreserveDepthStencilContents);
             if (d->colorAttCount == 0) {
                 d->pixelSize = depthTexD->pixelSize();
                 d->sampleCount = depthTexD->samples;
@@ -4236,6 +4240,7 @@ bool QMetalTextureRenderTarget::create()
             d->fb.dsTex = depthRbD->d->tex;
             d->fb.hasStencil = true;
             d->fb.depthNeedsStore = false;
+            d->fb.preserveDs = false;
             if (d->colorAttCount == 0) {
                 d->pixelSize = depthRbD->pixelSize();
                 d->sampleCount = depthRbD->samples;
@@ -4246,6 +4251,9 @@ bool QMetalTextureRenderTarget::create()
         d->dsAttCount = 0;
     }
 
+    if (d->colorAttCount > 0)
+        d->fb.preserveColor = m_flags.testFlag(QRhiTextureRenderTarget::PreserveColorContents);
+
     QRhiRenderTargetAttachmentTracker::updateResIdList<QMetalTexture, QMetalRenderBuffer>(m_desc, &d->currentResIdList);
 
     rhiD->registerResource(this, false);
diff --git a/src/gui/rhi/qrhivulkan.cpp b/src/gui/rhi/qrhivulkan.cpp
index c009d22d04..41a0e7ba71 100644
--- a/src/gui/rhi/qrhivulkan.cpp
+++ b/src/gui/rhi/qrhivulkan.cpp
@@ -1457,7 +1457,7 @@ bool QRhiVulkan::createOffscreenRenderPass(QVkRenderPassDescriptor *rpD,
         attDesc.format = vkformat;
         attDesc.samples = samples;
         attDesc.loadOp = preserveColor ? VK_ATTACHMENT_LOAD_OP_LOAD : VK_ATTACHMENT_LOAD_OP_CLEAR;
-        attDesc.storeOp = it->resolveTexture() ? VK_ATTACHMENT_STORE_OP_DONT_CARE : VK_ATTACHMENT_STORE_OP_STORE;
+        attDesc.storeOp = (it->resolveTexture() && !preserveColor) ? VK_ATTACHMENT_STORE_OP_DONT_CARE : VK_ATTACHMENT_STORE_OP_STORE;
         attDesc.stencilLoadOp = VK_ATTACHMENT_LOAD_OP_DONT_CARE;
         attDesc.stencilStoreOp = VK_ATTACHMENT_STORE_OP_DONT_CARE;
         // this has to interact correctly with activateTextureRenderTarget(), hence leaving in COLOR_ATT