diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp index 718b7ec0..be01fb2f 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp @@ -3016,6 +3016,7 @@ void main() { DestroySubmitFencePool(); DestroyDeferredDepthMipmapCleanup(); + DestroyMultisampleResolveScratchImage(); DestroyComputePipelines(); // No sweep runs during teardown, but the observers point at this renderer @@ -7194,6 +7195,68 @@ void main() { return true; } + void VulkanRenderer::DestroyMultisampleResolveScratchImage() { + if (m_msResolveScratch.image != VK_NULL_HANDLE) { + vmaDestroyImage(m_allocator, m_msResolveScratch.image, m_msResolveScratch.allocation); + } + m_msResolveScratch = {}; + } + + Bool VulkanRenderer::AcquireMultisampleResolveScratchImage(VkCommandBuffer commandBuffer, VkFormat format, + VkExtent2D extent) { + if (extent.width == 0 || extent.height == 0 || format == VK_FORMAT_UNDEFINED) { + return false; + } + // Grow-only, and never shrink: these blits repeat at one or two sizes, so the steady state + // is one allocation for the whole process. + if (m_msResolveScratch.image == VK_NULL_HANDLE || m_msResolveScratch.format != format || + m_msResolveScratch.extent.width < extent.width || m_msResolveScratch.extent.height < extent.height) { + const VkExtent2D grown = {std::max(extent.width, m_msResolveScratch.extent.width), + std::max(extent.height, m_msResolveScratch.extent.height)}; + DestroyMultisampleResolveScratchImage(); + + VkImageCreateInfo imageInfo{}; + imageInfo.sType = VK_STRUCTURE_TYPE_IMAGE_CREATE_INFO; + imageInfo.imageType = VK_IMAGE_TYPE_2D; + imageInfo.format = format; + imageInfo.extent = {grown.width, grown.height, 1}; + imageInfo.mipLevels = 1; + imageInfo.arrayLayers = 1; + imageInfo.samples = VK_SAMPLE_COUNT_1_BIT; + imageInfo.tiling = VK_IMAGE_TILING_OPTIMAL; + imageInfo.usage = VK_IMAGE_USAGE_TRANSFER_DST_BIT | VK_IMAGE_USAGE_TRANSFER_SRC_BIT; + imageInfo.sharingMode = VK_SHARING_MODE_EXCLUSIVE; + imageInfo.initialLayout = VK_IMAGE_LAYOUT_UNDEFINED; + + VmaAllocationCreateInfo allocationInfo{}; + allocationInfo.usage = VMA_MEMORY_USAGE_AUTO_PREFER_DEVICE; + allocationInfo.requiredFlags = VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT; + if (vmaCreateImage(m_allocator, &imageInfo, &allocationInfo, &m_msResolveScratch.image, + &m_msResolveScratch.allocation, nullptr) != VK_SUCCESS) { + // Soft failure: the caller keeps the direct resolve, which is what shipped before. + MGLOG_E("AcquireMultisampleResolveScratchImage: vmaCreateImage failed (format=%d %ux%u)", + static_cast(format), grown.width, grown.height); + m_msResolveScratch = {}; + return false; + } + m_msResolveScratch.format = format; + m_msResolveScratch.extent = grown; + m_msResolveScratch.layout = VK_IMAGE_LAYOUT_UNDEFINED; + } + + VkPipelineStageFlags srcStageMask = VK_PIPELINE_STAGE_TOP_OF_PIPE_BIT; + VkAccessFlags srcAccessMask = 0; + GetImageTransitionSourceState(m_msResolveScratch.layout, srcStageMask, srcAccessMask); + if (!VkTextureManager::TransitionImageLayout(commandBuffer, m_msResolveScratch.image, + m_msResolveScratch.layout, + VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, srcStageMask, + VK_PIPELINE_STAGE_TRANSFER_BIT, srcAccessMask, + VK_ACCESS_TRANSFER_WRITE_BIT, VK_IMAGE_ASPECT_COLOR_BIT)) { + return false; + } + return true; + } + // The aspects a depth/stencil format actually carries. VkTextureManager keeps its own copy of // this private, and the swapchain's depth/stencil image has no TextureResource to ask. static VkImageAspectFlags GetDepthStencilAspectMaskForFormat(VkFormat format) { @@ -8004,23 +8067,78 @@ void main() { } if (srcBinding.sampleCount != VK_SAMPLE_COUNT_1_BIT && dstBinding.sampleCount == VK_SAMPLE_COUNT_1_BIT) { - // NOTE: vkCmdResolveImage cannot flip, and this region is still built from the raw GL - // offsets. A multisample-resolve blit whose source or destination is the default - // framebuffer therefore keeps the pre-fix behaviour; it needs a resolve-then-blit - // (or blit-then-resolve) split, which is its own change. // GL multisample resolve blits are 1:1 by spec; vkCmdBlitImage cannot read a - // multisampled source. + // multisampled source, so the samples have to come down through vkCmdResolveImage. + const Uint32 resolveWidth = static_cast(std::abs(srcX1 - srcX0)); + const Uint32 resolveHeight = static_cast(std::abs(srcY1 - srcY0)); + + // vkCmdResolveImage takes ONE offset per side, so it cannot express the axis inversion + // that a default-framebuffer rect needs - it would land the mirrored band. When the + // transforms above actually moved the region, split the operation: resolve into a + // single-sample scratch image at raw offsets, then blit THAT into the destination with + // the (already transformed) region, which vkCmdBlitImage can invert. + const Bool regionWasTransformed = + (readIsDefaultFbo || drawIsDefaultFbo) && + (blitRegion.srcOffsets[0].x != srcX0 || blitRegion.srcOffsets[0].y != srcY0 || + blitRegion.srcOffsets[1].x != srcX1 || blitRegion.srcOffsets[1].y != srcY1 || + blitRegion.dstOffsets[0].x != dstX0 || blitRegion.dstOffsets[0].y != dstY0 || + blitRegion.dstOffsets[1].x != dstX1 || blitRegion.dstOffsets[1].y != dstY1); + const Bool useScratchResolve = + regionWasTransformed && resolveWidth > 0 && resolveHeight > 0 && + AcquireMultisampleResolveScratchImage(frame.commandBuffer, srcBinding.format, + {resolveWidth, resolveHeight}); + VkImageResolve resolveRegion{}; resolveRegion.srcSubresource = blitRegion.srcSubresource; - resolveRegion.srcOffset = {std::min(srcX0, srcX1), std::min(srcY0, srcY1), 0}; resolveRegion.dstSubresource = blitRegion.dstSubresource; - resolveRegion.dstOffset = {std::min(dstX0, dstX1), std::min(dstY0, dstY1), 0}; - resolveRegion.extent = {static_cast(std::abs(srcX1 - srcX0)), - static_cast(std::abs(srcY1 - srcY0)), 1}; - vkCmdResolveImage(frame.commandBuffer, - srcBinding.image, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, - dstBinding.image, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, - 1, &resolveRegion); + resolveRegion.extent = {resolveWidth, resolveHeight, 1}; + if (useScratchResolve) { + // The scratch copy is a plain single-layer colour image, and the resolve reads the + // SOURCE band the (possibly inverted) transformed region names - taking its min so + // an inverted pair still describes the same band. + resolveRegion.srcOffset = {std::min(blitRegion.srcOffsets[0].x, blitRegion.srcOffsets[1].x), + std::min(blitRegion.srcOffsets[0].y, blitRegion.srcOffsets[1].y), 0}; + resolveRegion.dstSubresource.aspectMask = VK_IMAGE_ASPECT_COLOR_BIT; + resolveRegion.dstSubresource.mipLevel = 0; + resolveRegion.dstSubresource.baseArrayLayer = 0; + resolveRegion.dstSubresource.layerCount = 1; + resolveRegion.dstOffset = {0, 0, 0}; + vkCmdResolveImage(frame.commandBuffer, + srcBinding.image, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, + m_msResolveScratch.image, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, + 1, &resolveRegion); + + VkImageLayout scratchLayout = VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL; + const Bool scratchReady = VkTextureManager::TransitionImageLayout( + frame.commandBuffer, m_msResolveScratch.image, scratchLayout, + VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, VK_PIPELINE_STAGE_TRANSFER_BIT, + VK_PIPELINE_STAGE_TRANSFER_BIT, VK_ACCESS_TRANSFER_WRITE_BIT, VK_ACCESS_TRANSFER_READ_BIT, + VK_IMAGE_ASPECT_COLOR_BIT); + MOBILEGL_ASSERT(scratchReady, "%s: failed to transition the resolve scratch image", __func__); + m_msResolveScratch.layout = scratchLayout; + + // Second leg: the scratch image holds the resolved band at its own origin, so the + // source side of the region becomes the whole scratch rect and only the + // destination keeps the transform. + VkImageBlit scratchBlit = blitRegion; + scratchBlit.srcSubresource.aspectMask = VK_IMAGE_ASPECT_COLOR_BIT; + scratchBlit.srcSubresource.mipLevel = 0; + scratchBlit.srcSubresource.baseArrayLayer = 0; + scratchBlit.srcSubresource.layerCount = 1; + scratchBlit.srcOffsets[0] = {0, 0, 0}; + scratchBlit.srcOffsets[1] = {static_cast(resolveWidth), static_cast(resolveHeight), 1}; + vkCmdBlitImage(frame.commandBuffer, + m_msResolveScratch.image, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, + dstBinding.image, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, + 1, &scratchBlit, filter == GL_LINEAR ? VK_FILTER_LINEAR : VK_FILTER_NEAREST); + } else { + resolveRegion.srcOffset = {std::min(srcX0, srcX1), std::min(srcY0, srcY1), 0}; + resolveRegion.dstOffset = {std::min(dstX0, dstX1), std::min(dstY0, dstY1), 0}; + vkCmdResolveImage(frame.commandBuffer, + srcBinding.image, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, + dstBinding.image, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, + 1, &resolveRegion); + } } else { vkCmdBlitImage(frame.commandBuffer, srcBinding.image, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.h b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.h index 803a7c4f..c48f11fb 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.h +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.h @@ -368,6 +368,31 @@ namespace MobileGL::MG_Backend::DirectVulkan { Uint32 samplerBinding = 0; }; + // A single-sample staging image for multisample-resolve blits that also have to change + // orientation. vkCmdResolveImage cannot flip (it takes one offset per side, not the + // invertible pair vkCmdBlitImage takes), so a resolve into or out of the default + // framebuffer used to land the mirrored band. Resolving here first and then blitting from + // here separates the two operations, and each one then does only what it can express. + // + // Pooled rather than created per blit: the CTS runs hundreds of these back to back, and + // create-destroy per call would both cost allocations and, worse, need per-call deferred + // destruction to outlive the recording. It grows to the largest extent asked for and is + // reused; format changes recreate it. + struct MultisampleResolveScratchImage { + VkImage image = VK_NULL_HANDLE; + VmaAllocation allocation = VK_NULL_HANDLE; + VkFormat format = VK_FORMAT_UNDEFINED; + VkExtent2D extent = {0, 0}; + VkImageLayout layout = VK_IMAGE_LAYOUT_UNDEFINED; + }; + MultisampleResolveScratchImage m_msResolveScratch; + // Returns a scratch image at least `extent` in size with exactly `format`, transitioned to + // TRANSFER_DST and ready to be resolved into. Null image on failure (the caller then falls + // back to the direct resolve). + Bool AcquireMultisampleResolveScratchImage(VkCommandBuffer commandBuffer, VkFormat format, + VkExtent2D extent); + void DestroyMultisampleResolveScratchImage(); + struct DeferredDepthMipmapCleanup { Vector imageViews; Vector framebuffers; diff --git a/MobileGL/MG_IntegrationTest/Scenarios/ClearThenReadPixelsScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/ClearThenReadPixelsScenario.cpp index e39102a3..f8c7955a 100644 --- a/MobileGL/MG_IntegrationTest/Scenarios/ClearThenReadPixelsScenario.cpp +++ b/MobileGL/MG_IntegrationTest/Scenarios/ClearThenReadPixelsScenario.cpp @@ -233,6 +233,75 @@ void main() { o_color = vec4(0.1, 0.2, 0.3, 1.0); } glDeleteProgram(program); } + // A MULTISAMPLE-RESOLVE blit into the default framebuffer has to change orientation like any + // other, but vkCmdResolveImage takes one offset per side and cannot invert an axis, so it used + // to land the mirrored band. The renderer now resolves into a single-sample scratch image and + // blits from there. The source is painted in two horizontal bands so the mirror is visible; + // a full-extent uniform blit is a fixed point of the flip and would prove nothing. + TEST_F(ClearThenReadPixelsScenario, AMultisampleResolveBlitIntoTheDefaultFramebufferKeepsItsOrientation) { + if (!Ready()) return; + HeadlessGL& gl = Gl(); + const int width = gl.Width(); + const int height = gl.Height(); + ASSERT_GE(height, 8); + + GLint maxSamples = 0; + glGetIntegerv(GL_MAX_SAMPLES, &maxSamples); + if (maxSamples < 2) { + GTEST_SKIP() << "GL_MAX_SAMPLES is " << maxSamples << "; this needs a multisample renderbuffer"; + } + + GLuint fbo = 0, rbo = 0; + glGenFramebuffers(1, &fbo); + glBindFramebuffer(GL_FRAMEBUFFER, fbo); + glGenRenderbuffers(1, &rbo); + glBindRenderbuffer(GL_RENDERBUFFER, rbo); + glRenderbufferStorageMultisample(GL_RENDERBUFFER, 2, GL_RGBA8, width, height); + glFramebufferRenderbuffer(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_RENDERBUFFER, rbo); + if (glCheckFramebufferStatus(GL_FRAMEBUFFER) != GL_FRAMEBUFFER_COMPLETE) { + glDeleteRenderbuffers(1, &rbo); + glDeleteFramebuffers(1, &fbo); + glBindFramebuffer(GL_FRAMEBUFFER, 0); + GTEST_SKIP() << "no complete 2x multisample RGBA8 renderbuffer on this driver"; + } + glViewport(0, 0, width, height); + + // Bottom half red, top half blue - via scissored clears, so no shader is involved. + glEnable(GL_SCISSOR_TEST); + glScissor(0, 0, width, height / 2); + ClearTo(1.0f, 0.0f, 0.0f, 1.0f); + glScissor(0, height / 2, width, height - height / 2); + ClearTo(0.0f, 0.0f, 1.0f, 1.0f); + glDisable(GL_SCISSOR_TEST); + + BindDefaultFramebuffer(); + glViewport(0, 0, width, height); + ClearTo(0.0f, 0.0f, 0.0f, 1.0f); + glBindFramebuffer(GL_READ_FRAMEBUFFER, fbo); + glBindFramebuffer(GL_DRAW_FRAMEBUFFER, 0); + glBlitFramebuffer(0, 0, width, height, 0, 0, width, height, GL_COLOR_BUFFER_BIT, GL_NEAREST); + glBindFramebuffer(GL_FRAMEBUFFER, 0); + EXPECT_EQ(FirstGLError(), 0u); + + const Image resolved = ReadPixels(width, height); + EXPECT_EQ(FirstGLError(), 0u); + const Rgba8 bottom = resolved.At(width / 2, height / 4); + const Rgba8 top = resolved.At(width / 2, height - 1 - height / 4); + EXPECT_GT(bottom.r, 200) << "the bottom band should be red after the resolve, got rgba(" + << static_cast(bottom.r) << ", " << static_cast(bottom.g) << ", " + << static_cast(bottom.b) << ") - blue there means the resolve landed " + << "in the mirrored band"; + EXPECT_LT(bottom.b, 60); + EXPECT_GT(top.b, 200) << "the top band should be blue after the resolve, got rgba(" + << static_cast(top.r) << ", " << static_cast(top.g) << ", " + << static_cast(top.b) << ")"; + EXPECT_LT(top.r, 60); + + glDeleteRenderbuffers(1, &rbo); + glDeleteFramebuffers(1, &fbo); + gl.EndFrame(); + } + // The same ordering claim for the path that DOES open a render pass. It passes today (the // render pass folds the clear into its loadOp and pops it), and it is here so a future change // to the pending-clear lifecycle cannot quietly reverse clear and draw.