From f1d3fa8f1a8947b653df73b1372fd4f5ae90b869 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Sun, 20 Sep 2026 18:52:08 -0400 Subject: [PATCH] [MG_Backend] keep the default framebuffer on the image GL last rendered into across a swap DirectVulkan resolved every default-framebuffer role against m_imageIndexAcquired. Present() advances that to the image it acquires for the frame that is about to start, and a present record is kWaitPresent - the client does not wait for the apply - so a readback belonging to the frame being presented (the retrace snapshot, taken at the swap call) can be applied after the acquire and then resolve an image nothing had written. That is the pure-black sundial-lite DirectVulkan retrace frame; DirectGLES is immune because the driver keeps handing back the buffer the app drew into. Split the GL-visible index out (VulkanRenderer::m_defaultFramebufferImageIndex) and keep it out of the present/acquire path: it only re-points at the acquired image when something WRITES the default framebuffer (DefaultFramebufferWriteIndex, called at the FBO-0 render-pass creation, the deferred-clear-free direct writes and the wire draw/clear/blit resolutions) and is reset on RecreateSwapchain. Reads - glReadPixels both arms, the parked-clear materialization, a blit or copy SOURCED from FBO 0, ResolveWireImage's read role (carrying the resolved index in WireImage) - resolve DefaultFramebufferReadIndex instead, i.e. whatever that is. A readback that resolves a swapchain image with no recorded write now says so once (MGLOG_W_ONCE, both arms) instead of silently handing back raw pixels. New integration scenario DirectVulkan|DirectGLES.DefaultFramebufferAcrossSwap pins both halves: a readback after a swap still sees the drawn frame, and the first draw after a swap lands in the image the readback then resolves. --- .../DirectVulkan/Renderer/VulkanRenderer.cpp | 162 ++++++++++------ .../DirectVulkan/Renderer/VulkanRenderer.h | 29 ++- .../DirectVulkan/Renderer/WireColorBlit.inc | 2 +- .../DirectVulkan/Renderer/WireDraw.inc | 11 +- .../DirectVulkan/Renderer/WireFramebuffer.inc | 53 ++++-- MobileGL/MG_IntegrationTest/CMakeLists.txt | 1 + .../DefaultFramebufferAcrossSwapScenario.cpp | 176 ++++++++++++++++++ 7 files changed, 353 insertions(+), 81 deletions(-) create mode 100644 MobileGL/MG_IntegrationTest/Scenarios/DefaultFramebufferAcrossSwapScenario.cpp diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp index 4aab0c74..268222d5 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp @@ -3411,6 +3411,10 @@ void main() { acquireResult = VK_SUCCESS; } VK_VERIFY(acquireResult, "Initialize, WaitAndAcquireNextImage"); + // The first acquired image is the one the opening frame renders into, so the + // default framebuffer addresses it from the start (see + // m_defaultFramebufferImageIndex). + m_defaultFramebufferImageIndex = m_imageIndexAcquired; } else { MGLOG_W("DirectVulkan: no swapchain at initialization (zero-area window); deferring first acquire"); } @@ -7324,8 +7328,8 @@ void main() { const Bool drawUsesDepthStencil = MGB_CTX->IsCapabilityEnabled(CapabilityInput::DepthTest) || MGB_CTX->IsCapabilityEnabled(CapabilityInput::StencilTest); - auto* renderPassEntry = - m_renderPassManager->GetOrCreateRenderPass(*drawFbo, m_imageIndexAcquired, drawUsesDepthStencil); + auto* renderPassEntry = m_renderPassManager->GetOrCreateRenderPass( + *drawFbo, DefaultFramebufferWriteIndex(drawFbo->IsDefaultFramebuffer()), drawUsesDepthStencil); // nullptr: the framebuffer has an attachment DirectVulkan cannot represent (a texture the // texture manager declined to back, or a view it could not build). The builder logged which // one; drop the draw here, exactly as an unresolvable sampler descriptor drops one in @@ -7337,8 +7341,8 @@ void main() { if (activeRenderPass && !activeRenderPass->CompatibleWith(*renderPassEntry)) { VkRenderPassManager::EndRenderPass(frame.commandBuffer); activeRenderPass = nullptr; - renderPassEntry = - m_renderPassManager->GetOrCreateRenderPass(*drawFbo, m_imageIndexAcquired, drawUsesDepthStencil); + renderPassEntry = m_renderPassManager->GetOrCreateRenderPass( + *drawFbo, DefaultFramebufferWriteIndex(drawFbo->IsDefaultFramebuffer()), drawUsesDepthStencil); if (renderPassEntry == nullptr) { return false; } @@ -7710,7 +7714,8 @@ void main() { } auto* activeRenderPass = VkRenderPassManager::GetActiveRenderPass(); - auto* renderPassEntry = m_renderPassManager->GetOrCreateRenderPass(framebuffer, m_imageIndexAcquired); + auto* renderPassEntry = m_renderPassManager->GetOrCreateRenderPass( + framebuffer, DefaultFramebufferWriteIndex(framebuffer.IsDefaultFramebuffer())); // A declined render pass is the same answer as an empty one for a clear: there is nothing // attached that can be cleared inside a pass. The builder has already logged the reason. if (renderPassEntry == nullptr || renderPassEntry->attachmentCount == 0 || @@ -7743,7 +7748,8 @@ void main() { activeRenderPass = nullptr; // Re-resolve: ending the pass updates tracked attachment layouts, which feed the // entry's load ops and initial layouts. - renderPassEntry = m_renderPassManager->GetOrCreateRenderPass(framebuffer, m_imageIndexAcquired); + renderPassEntry = m_renderPassManager->GetOrCreateRenderPass( + framebuffer, DefaultFramebufferWriteIndex(framebuffer.IsDefaultFramebuffer())); if (renderPassEntry == nullptr) { return ScissoredClearPrep::NoOp; } @@ -8825,7 +8831,10 @@ void main() { Bool VulkanRenderer::MaterializePendingDepthStencilClearForDefaultFramebuffer( VkCommandBuffer commandBuffer, const MG_State::GLState::FramebufferAttachmentObject& attachment, const ClearAttachmentPayload& payload) { - const VkImage depthStencilImage = m_swapchainObject.GetDepthStencilImage(m_imageIndexAcquired); + // The image the readback that triggered this materialization resolves; a parked clear + // must land where the read looks (see m_defaultFramebufferImageIndex). + const Uint32 swapchainImageIndex = DefaultFramebufferReadIndex(true); + const VkImage depthStencilImage = m_swapchainObject.GetDepthStencilImage(swapchainImageIndex); if (depthStencilImage == VK_NULL_HANDLE) { return false; } @@ -8841,7 +8850,7 @@ void main() { return true; } - VkImageLayout currentLayout = m_swapchainObject.GetDepthStencilImageLayout(m_imageIndexAcquired); + VkImageLayout currentLayout = m_swapchainObject.GetDepthStencilImageLayout(swapchainImageIndex); VkPipelineStageFlags srcStageMask = VK_PIPELINE_STAGE_TOP_OF_PIPE_BIT; VkAccessFlags srcAccessMask = 0; GetImageTransitionSourceState(currentLayout, srcStageMask, srcAccessMask); @@ -8876,16 +8885,16 @@ void main() { VK_ACCESS_TRANSFER_WRITE_BIT, dstAccessMask, imageAspects)) { return false; } - m_swapchainObject.SetDepthStencilImageLayout(m_imageIndexAcquired, + m_swapchainObject.SetDepthStencilImageLayout(swapchainImageIndex, VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL); // The image now holds real values, so the next render pass must LOAD them rather than // treat the attachment as undefined and discard the clear that just executed. - m_swapchainObject.SetDepthStencilContentDefined(m_imageIndexAcquired, true); + m_swapchainObject.SetDepthStencilContentDefined(swapchainImageIndex, true); m_clearManager->PopPendingClear(attachment); MGLOG_D("MaterializePendingClearForDefaultFramebuffer: swapchain depth/stencil image %u pending clear " "materialized (aspects=0x%x)", - m_imageIndexAcquired, static_cast(clearAspects)); + swapchainImageIndex, static_cast(clearAspects)); return true; } @@ -8920,11 +8929,14 @@ void main() { return MaterializePendingDepthStencilClearForDefaultFramebuffer(commandBuffer, attachment, payload); } - const VkImage swapchainImage = m_swapchainObject.GetImage(m_imageIndexAcquired); + // Same rule as the depth/stencil twin below: the parked clear has to land on the image + // the readback that reaches this resolves. + const Uint32 swapchainImageIndex = DefaultFramebufferReadIndex(true); + const VkImage swapchainImage = m_swapchainObject.GetImage(swapchainImageIndex); if (swapchainImage == VK_NULL_HANDLE) { return false; } - VkImageLayout currentLayout = m_swapchainObject.GetImageLayout(m_imageIndexAcquired); + VkImageLayout currentLayout = m_swapchainObject.GetImageLayout(swapchainImageIndex); VkPipelineStageFlags srcStageMask = VK_PIPELINE_STAGE_TOP_OF_PIPE_BIT; VkAccessFlags srcAccessMask = 0; GetImageTransitionSourceState(currentLayout, srcStageMask, srcAccessMask); @@ -8965,13 +8977,13 @@ void main() { VK_IMAGE_ASPECT_COLOR_BIT)) { return false; } - m_swapchainObject.SetImageLayout(m_imageIndexAcquired, VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL); + m_swapchainObject.SetImageLayout(swapchainImageIndex, VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL); // Popped, not left behind: the clear has executed, so letting the next render pass load // it again as a loadOp would erase whatever is drawn between here and there. m_clearManager->PopPendingClear(attachment); MGLOG_D("MaterializePendingClearForDefaultFramebuffer: swapchain image %u pending clear materialized", - m_imageIndexAcquired); + swapchainImageIndex); return true; } @@ -8988,9 +9000,14 @@ void main() { BlitImageBinding srcBinding{}; BlitImageBinding dstBinding{}; - if (!ResolveColorBlitBinding(readFbo, true, m_imageIndexAcquired, m_swapchainObject, *m_textureManager, + // The source reads the image the app last rendered into, the destination re-points the + // default framebuffer at the acquired one - see m_defaultFramebufferImageIndex. The + // source resolves first, so a default-FBO source still names the pre-present image. + const Uint32 readDefaultImageIndex = DefaultFramebufferReadIndex(readFbo.IsDefaultFramebuffer()); + const Uint32 drawDefaultImageIndex = DefaultFramebufferWriteIndex(drawIsDefaultFbo); + if (!ResolveColorBlitBinding(readFbo, true, readDefaultImageIndex, m_swapchainObject, *m_textureManager, *m_renderPassManager, srcBinding) || - !ResolveColorBlitBinding(drawFbo, false, m_imageIndexAcquired, m_swapchainObject, *m_textureManager, + !ResolveColorBlitBinding(drawFbo, false, drawDefaultImageIndex, m_swapchainObject, *m_textureManager, *m_renderPassManager, dstBinding)) { return false; } @@ -9031,7 +9048,7 @@ void main() { // A color-only blit never touches depth/stencil: let the default-FBO pass // it opens skip the depth attachment (depth-less flavor). auto* renderPassEntryPtr = - m_renderPassManager->GetOrCreateRenderPass(drawFbo, m_imageIndexAcquired, /*drawUsesDepthStencil=*/false); + m_renderPassManager->GetOrCreateRenderPass(drawFbo, drawDefaultImageIndex, /*drawUsesDepthStencil=*/false); if (renderPassEntryPtr == nullptr) { // Declined (the builder logged which attachment). The caller's contract for `false` is // "this blit was not serviced here", which is the honest answer. @@ -9231,10 +9248,16 @@ void main() { for (const VkImageAspectFlagBits depthStencilAspect : depthStencilAspects) { BlitImageBinding srcBinding{}; BlitImageBinding dstBinding{}; - if (!ResolveFramebufferBlitBinding(*readFbo, true, m_imageIndexAcquired, m_swapchainObject, + // A default-FBO source resolves the image the app last rendered into; a default-FBO + // destination re-points it at the image Present() acquired (see + // m_defaultFramebufferImageIndex). The source's index is taken first, so a blit + // whose two sides are both the default framebuffer still names one image. + const Uint32 srcImageIndex = DefaultFramebufferReadIndex(readIsDefaultFbo); + const Uint32 dstImageIndex = DefaultFramebufferWriteIndex(drawIsDefaultFbo); + if (!ResolveFramebufferBlitBinding(*readFbo, true, srcImageIndex, m_swapchainObject, *m_textureManager, *m_renderPassManager, depthStencilAspect, srcBinding) || - !ResolveFramebufferBlitBinding(*drawFbo, false, m_imageIndexAcquired, m_swapchainObject, + !ResolveFramebufferBlitBinding(*drawFbo, false, dstImageIndex, m_swapchainObject, *m_textureManager, *m_renderPassManager, depthStencilAspect, dstBinding)) { // A buffer named in the mask but absent from either framebuffer copies @@ -9307,7 +9330,7 @@ void main() { } const VkImageLayout srcOriginalLayout = readIsDefaultFbo - ? m_swapchainObject.GetDepthStencilImageLayout(m_imageIndexAcquired) + ? m_swapchainObject.GetDepthStencilImageLayout(srcImageIndex) : *srcBinding.trackedLayout; if (srcOriginalLayout == VK_IMAGE_LAYOUT_UNDEFINED) { MGLOG_E_ONCE("BlitFramebuffer skipped: depth source image layout is undefined"); @@ -9315,7 +9338,7 @@ void main() { } const VkImageLayout dstOriginalLayout = drawIsDefaultFbo - ? m_swapchainObject.GetDepthStencilImageLayout(m_imageIndexAcquired) + ? m_swapchainObject.GetDepthStencilImageLayout(dstImageIndex) : *dstBinding.trackedLayout; const VkImageLayout dstRestoreLayout = dstOriginalLayout == VK_IMAGE_LAYOUT_UNDEFINED ? VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL @@ -9355,7 +9378,7 @@ void main() { srcAccessMask, VK_ACCESS_TRANSFER_READ_BIT, srcBinding.aspectMask, srcBinding.mipLevel, srcBinding.mipLevelCount); MOBILEGL_ASSERT(ok, "%s: failed to transition swapchain depth source image", __func__); - m_swapchainObject.SetDepthStencilImageLayout(m_imageIndexAcquired, srcTrackedLayout); + m_swapchainObject.SetDepthStencilImageLayout(srcImageIndex, srcTrackedLayout); } else { Bool ok = VkTextureManager::TransitionImageLayout( frame.commandBuffer, srcBinding.image, *srcBinding.trackedLayout, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, @@ -9376,7 +9399,7 @@ void main() { dstAccessMask, VK_ACCESS_TRANSFER_WRITE_BIT, dstBinding.aspectMask, dstBinding.mipLevel, dstBinding.mipLevelCount); MOBILEGL_ASSERT(ok, "%s: failed to transition swapchain depth destination image", __func__); - m_swapchainObject.SetDepthStencilImageLayout(m_imageIndexAcquired, dstTrackedLayout); + m_swapchainObject.SetDepthStencilImageLayout(dstImageIndex, dstTrackedLayout); } else { Bool ok = VkTextureManager::TransitionImageLayout( frame.commandBuffer, dstBinding.image, *dstBinding.trackedLayout, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, @@ -9446,14 +9469,14 @@ void main() { VkAccessFlags srcRestoreAccessMask = 0; GetImageTransitionDestinationState(srcOriginalLayout, srcRestoreStageMask, srcRestoreAccessMask); if (readIsDefaultFbo) { - VkImageLayout srcTrackedLayout = m_swapchainObject.GetDepthStencilImageLayout(m_imageIndexAcquired); + VkImageLayout srcTrackedLayout = m_swapchainObject.GetDepthStencilImageLayout(srcImageIndex); Bool ok = VkTextureManager::TransitionImageLayout( frame.commandBuffer, srcBinding.image, srcTrackedLayout, srcOriginalLayout, VK_PIPELINE_STAGE_TRANSFER_BIT, srcRestoreStageMask, VK_ACCESS_TRANSFER_READ_BIT, srcRestoreAccessMask, srcBinding.aspectMask, srcBinding.mipLevel, srcBinding.mipLevelCount); MOBILEGL_ASSERT(ok, "%s: failed to restore swapchain depth source image layout", __func__); - m_swapchainObject.SetDepthStencilImageLayout(m_imageIndexAcquired, srcTrackedLayout); + m_swapchainObject.SetDepthStencilImageLayout(srcImageIndex, srcTrackedLayout); } else { Bool ok = VkTextureManager::TransitionImageLayout( frame.commandBuffer, srcBinding.image, *srcBinding.trackedLayout, srcOriginalLayout, @@ -9467,14 +9490,14 @@ void main() { VkAccessFlags dstRestoreAccessMask = 0; GetImageTransitionDestinationState(dstRestoreLayout, dstRestoreStageMask, dstRestoreAccessMask); if (drawIsDefaultFbo) { - VkImageLayout dstTrackedLayout = m_swapchainObject.GetDepthStencilImageLayout(m_imageIndexAcquired); + VkImageLayout dstTrackedLayout = m_swapchainObject.GetDepthStencilImageLayout(dstImageIndex); Bool ok = VkTextureManager::TransitionImageLayout( frame.commandBuffer, dstBinding.image, dstTrackedLayout, dstRestoreLayout, VK_PIPELINE_STAGE_TRANSFER_BIT, dstRestoreStageMask, VK_ACCESS_TRANSFER_WRITE_BIT, dstRestoreAccessMask, dstBinding.aspectMask, dstBinding.mipLevel, dstBinding.mipLevelCount); MOBILEGL_ASSERT(ok, "%s: failed to restore swapchain depth destination image layout", __func__); - m_swapchainObject.SetDepthStencilImageLayout(m_imageIndexAcquired, dstTrackedLayout); + m_swapchainObject.SetDepthStencilImageLayout(dstImageIndex, dstTrackedLayout); } else { Bool ok = VkTextureManager::TransitionImageLayout( frame.commandBuffer, dstBinding.image, *dstBinding.trackedLayout, dstRestoreLayout, @@ -9490,9 +9513,13 @@ void main() { BlitImageBinding srcBinding{}; BlitImageBinding dstBinding{}; - if (!ResolveColorBlitBinding(*readFbo, true, m_imageIndexAcquired, m_swapchainObject, *m_textureManager, + // Same split as the depth/stencil loop above, and the same ordering rule: the source's + // index is read before the destination's write re-points it. + const Uint32 srcImageIndex = DefaultFramebufferReadIndex(readIsDefaultFbo); + const Uint32 dstImageIndex = DefaultFramebufferWriteIndex(drawIsDefaultFbo); + if (!ResolveColorBlitBinding(*readFbo, true, srcImageIndex, m_swapchainObject, *m_textureManager, *m_renderPassManager, srcBinding) || - !ResolveColorBlitBinding(*drawFbo, false, m_imageIndexAcquired, m_swapchainObject, *m_textureManager, + !ResolveColorBlitBinding(*drawFbo, false, dstImageIndex, m_swapchainObject, *m_textureManager, *m_renderPassManager, dstBinding)) { return; } @@ -9546,10 +9573,10 @@ void main() { } VkImageLayout srcLayout = readIsDefaultFbo - ? m_swapchainObject.GetImageLayout(m_imageIndexAcquired) + ? m_swapchainObject.GetImageLayout(srcImageIndex) : *srcBinding.trackedLayout; VkImageLayout dstLayout = drawIsDefaultFbo - ? m_swapchainObject.GetImageLayout(m_imageIndexAcquired) + ? m_swapchainObject.GetImageLayout(dstImageIndex) : *dstBinding.trackedLayout; const VkImageLayout srcOriginalLayout = srcLayout; const VkImageLayout dstOriginalLayout = dstLayout; @@ -9575,7 +9602,7 @@ void main() { srcStageMask, VK_PIPELINE_STAGE_TRANSFER_BIT, srcAccessMask, VK_ACCESS_TRANSFER_READ_BIT, srcBinding.aspectMask); MOBILEGL_ASSERT(ok, "%s: failed to transition swapchain source image", __func__); - m_swapchainObject.SetImageLayout(m_imageIndexAcquired, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL); + m_swapchainObject.SetImageLayout(srcImageIndex, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL); } else { Bool ok = VkTextureManager::TransitionImageLayout( frame.commandBuffer, srcBinding.image, *srcBinding.trackedLayout, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, @@ -9593,7 +9620,7 @@ void main() { dstStageMask, VK_PIPELINE_STAGE_TRANSFER_BIT, dstAccessMask, VK_ACCESS_TRANSFER_WRITE_BIT, dstBinding.aspectMask); MOBILEGL_ASSERT(ok, "%s: failed to transition swapchain destination image", __func__); - m_swapchainObject.SetImageLayout(m_imageIndexAcquired, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL); + m_swapchainObject.SetImageLayout(dstImageIndex, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL); } else { Bool ok = VkTextureManager::TransitionImageLayout( frame.commandBuffer, dstBinding.image, *dstBinding.trackedLayout, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, @@ -9707,13 +9734,13 @@ void main() { VkAccessFlags srcRestoreAccessMask = 0; GetImageTransitionDestinationState(srcOriginalLayout, srcRestoreStageMask, srcRestoreAccessMask); if (readIsDefaultFbo) { - VkImageLayout srcTrackedLayout = m_swapchainObject.GetImageLayout(m_imageIndexAcquired); + VkImageLayout srcTrackedLayout = m_swapchainObject.GetImageLayout(srcImageIndex); Bool ok = VkTextureManager::TransitionImageLayout( frame.commandBuffer, srcBinding.image, srcTrackedLayout, srcOriginalLayout, VK_PIPELINE_STAGE_TRANSFER_BIT, srcRestoreStageMask, VK_ACCESS_TRANSFER_READ_BIT, srcRestoreAccessMask, srcBinding.aspectMask); MOBILEGL_ASSERT(ok, "%s: failed to restore swapchain source image layout", __func__); - m_swapchainObject.SetImageLayout(m_imageIndexAcquired, srcTrackedLayout); + m_swapchainObject.SetImageLayout(srcImageIndex, srcTrackedLayout); } else { Bool ok = VkTextureManager::TransitionImageLayout( frame.commandBuffer, srcBinding.image, *srcBinding.trackedLayout, srcOriginalLayout, @@ -9726,13 +9753,13 @@ void main() { VkAccessFlags dstRestoreAccessMask = 0; GetImageTransitionDestinationState(dstRestoreLayout, dstRestoreStageMask, dstRestoreAccessMask); if (drawIsDefaultFbo) { - VkImageLayout dstTrackedLayout = m_swapchainObject.GetImageLayout(m_imageIndexAcquired); + VkImageLayout dstTrackedLayout = m_swapchainObject.GetImageLayout(dstImageIndex); Bool ok = VkTextureManager::TransitionImageLayout( frame.commandBuffer, dstBinding.image, dstTrackedLayout, dstRestoreLayout, VK_PIPELINE_STAGE_TRANSFER_BIT, dstRestoreStageMask, VK_ACCESS_TRANSFER_WRITE_BIT, dstRestoreAccessMask, dstBinding.aspectMask); MOBILEGL_ASSERT(ok, "%s: failed to restore swapchain destination image layout", __func__); - m_swapchainObject.SetImageLayout(m_imageIndexAcquired, dstTrackedLayout); + m_swapchainObject.SetImageLayout(dstImageIndex, dstTrackedLayout); } else { Bool ok = VkTextureManager::TransitionImageLayout( frame.commandBuffer, dstBinding.image, *dstBinding.trackedLayout, dstRestoreLayout, @@ -9796,6 +9823,10 @@ void main() { } const Bool readIsDefaultFbo = readFbo->IsDefaultFramebuffer(); + // CopyTexSubImage2D only ever READS the default framebuffer, so it resolves the image + // the app last rendered into (see m_defaultFramebufferImageIndex) - never the one + // Present() has already acquired for the next frame. + const Uint32 srcImageIndex = DefaultFramebufferReadIndex(readIsDefaultFbo); BlitImageBinding dstBinding{}; if (!ResolveTextureCopyDestinationBinding(*destinationTexture, static_cast(level), *m_textureManager, @@ -9806,7 +9837,7 @@ void main() { } BlitImageBinding srcBinding{}; - if (!ResolveTextureCopySourceBinding(*readFbo, m_imageIndexAcquired, m_swapchainObject, *m_textureManager, + if (!ResolveTextureCopySourceBinding(*readFbo, srcImageIndex, m_swapchainObject, *m_textureManager, *m_renderPassManager, dstBinding.aspectMask, srcBinding)) { RecordTextureCopyError(__func__, ErrorCode::InvalidOperation, "CopyTexSubImage2D requires a complete read attachment compatible with the destination texture."); @@ -9836,8 +9867,8 @@ void main() { const Bool srcUsesSwapchainDepth = readIsDefaultFbo && (srcBinding.aspectMask & VK_IMAGE_ASPECT_COLOR_BIT) == 0; const VkImageLayout srcOriginalLayout = readIsDefaultFbo ? (srcUsesSwapchainDepth - ? m_swapchainObject.GetDepthStencilImageLayout(m_imageIndexAcquired) - : m_swapchainObject.GetImageLayout(m_imageIndexAcquired)) + ? m_swapchainObject.GetDepthStencilImageLayout(srcImageIndex) + : m_swapchainObject.GetImageLayout(srcImageIndex)) : *srcBinding.trackedLayout; if (srcOriginalLayout == VK_IMAGE_LAYOUT_UNDEFINED) { RecordTextureCopyError(__func__, ErrorCode::InvalidOperation, @@ -9864,9 +9895,9 @@ void main() { srcBinding.mipLevel, srcBinding.mipLevelCount); MOBILEGL_ASSERT(ok, "%s: failed to transition swapchain source image", __func__); if (srcUsesSwapchainDepth) { - m_swapchainObject.SetDepthStencilImageLayout(m_imageIndexAcquired, srcTrackedLayout); + m_swapchainObject.SetDepthStencilImageLayout(srcImageIndex, srcTrackedLayout); } else { - m_swapchainObject.SetImageLayout(m_imageIndexAcquired, srcTrackedLayout); + m_swapchainObject.SetImageLayout(srcImageIndex, srcTrackedLayout); } } else { Bool ok = VkTextureManager::TransitionImageLayout( @@ -9922,8 +9953,8 @@ void main() { GetImageTransitionDestinationState(srcOriginalLayout, srcRestoreStageMask, srcRestoreAccessMask); if (readIsDefaultFbo) { VkImageLayout srcTrackedLayout = srcUsesSwapchainDepth - ? m_swapchainObject.GetDepthStencilImageLayout(m_imageIndexAcquired) - : m_swapchainObject.GetImageLayout(m_imageIndexAcquired); + ? m_swapchainObject.GetDepthStencilImageLayout(srcImageIndex) + : m_swapchainObject.GetImageLayout(srcImageIndex); Bool ok = VkTextureManager::TransitionImageLayout( frame.commandBuffer, srcBinding.image, srcTrackedLayout, srcOriginalLayout, VK_PIPELINE_STAGE_TRANSFER_BIT, srcRestoreStageMask, @@ -9931,9 +9962,9 @@ void main() { srcBinding.mipLevel, srcBinding.mipLevelCount); MOBILEGL_ASSERT(ok, "%s: failed to restore swapchain source image layout", __func__); if (srcUsesSwapchainDepth) { - m_swapchainObject.SetDepthStencilImageLayout(m_imageIndexAcquired, srcTrackedLayout); + m_swapchainObject.SetDepthStencilImageLayout(srcImageIndex, srcTrackedLayout); } else { - m_swapchainObject.SetImageLayout(m_imageIndexAcquired, srcTrackedLayout); + m_swapchainObject.SetImageLayout(srcImageIndex, srcTrackedLayout); } } else { Bool ok = VkTextureManager::TransitionImageLayout( @@ -10614,13 +10645,26 @@ void main() { } BlitImageBinding srcBinding{}; - if (!ResolveColorBlitBinding(*readFbo, true, m_imageIndexAcquired, m_swapchainObject, *m_textureManager, + // A read resolves the image the app last rendered into, not the one Present() has + // already acquired for the frame that is starting (see m_defaultFramebufferImageIndex). + // A retrace snapshot taken just before a swap is applied after it, and must still find + // the colour the app drew. + const Uint32 srcImageIndex = DefaultFramebufferReadIndex(readIsDefaultFbo); + if (readIsDefaultFbo && !m_swapchainObject.IsImageContentDefined(srcImageIndex)) { + // Named rather than silent: the readback hands back whatever the image holds. The + // ordinary cause is a read of the default framebuffer applied after the Present that + // consumed it, where the app's own colour is still what is expected. + MGLOG_W_ONCE("DirectVulkan::ReadPixels: no write is recorded for swapchain image %u of the " + "default framebuffer; the readback returns its raw pixels", + srcImageIndex); + } + if (!ResolveColorBlitBinding(*readFbo, true, srcImageIndex, m_swapchainObject, *m_textureManager, *m_renderPassManager, srcBinding)) { return; } const VkImageLayout srcOriginalLayout = readIsDefaultFbo - ? m_swapchainObject.GetImageLayout(m_imageIndexAcquired) + ? m_swapchainObject.GetImageLayout(srcImageIndex) : *srcBinding.trackedLayout; if (srcOriginalLayout == VK_IMAGE_LAYOUT_UNDEFINED) { MGLOG_E_ONCE("DirectVulkan::ReadPixels skipped: source image layout is undefined"); @@ -10661,7 +10705,7 @@ void main() { srcStageMask, VK_PIPELINE_STAGE_TRANSFER_BIT, srcAccessMask, VK_ACCESS_TRANSFER_READ_BIT, srcBinding.aspectMask); MOBILEGL_ASSERT(ok, "%s: failed to transition swapchain source image", __func__); - m_swapchainObject.SetImageLayout(m_imageIndexAcquired, trackedLayout); + m_swapchainObject.SetImageLayout(srcImageIndex, trackedLayout); } else { Bool ok = VkTextureManager::TransitionImageLayout( frame.commandBuffer, srcBinding.image, *srcBinding.trackedLayout, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, @@ -10698,13 +10742,13 @@ void main() { VkAccessFlags restoreAccessMask = 0; GetImageTransitionDestinationState(srcOriginalLayout, restoreStageMask, restoreAccessMask); if (readIsDefaultFbo) { - VkImageLayout trackedLayout = m_swapchainObject.GetImageLayout(m_imageIndexAcquired); + VkImageLayout trackedLayout = m_swapchainObject.GetImageLayout(srcImageIndex); Bool ok = VkTextureManager::TransitionImageLayout( frame.commandBuffer, srcBinding.image, trackedLayout, srcOriginalLayout, VK_PIPELINE_STAGE_TRANSFER_BIT, restoreStageMask, VK_ACCESS_TRANSFER_READ_BIT, restoreAccessMask, srcBinding.aspectMask); MOBILEGL_ASSERT(ok, "%s: failed to restore swapchain source image layout", __func__); - m_swapchainObject.SetImageLayout(m_imageIndexAcquired, trackedLayout); + m_swapchainObject.SetImageLayout(srcImageIndex, trackedLayout); } else { Bool ok = VkTextureManager::TransitionImageLayout( frame.commandBuffer, srcBinding.image, *srcBinding.trackedLayout, srcOriginalLayout, @@ -10967,7 +11011,10 @@ void main() { // GL_STENCIL_INDEX) of the default framebuffer leave the caller's buffer untouched - // the whole KHR-GL*.framebuffer_blit family checks exactly that before it blits. if (readIsDefaultFbo) { - const VkImage swapchainDepthImage = m_swapchainObject.GetDepthStencilImage(m_imageIndexAcquired); + // A read names the image the app last rendered into, not the one Present() has + // already acquired (see m_defaultFramebufferImageIndex). + const Uint32 srcImageIndex = DefaultFramebufferReadIndex(true); + const VkImage swapchainDepthImage = m_swapchainObject.GetDepthStencilImage(srcImageIndex); if (swapchainDepthImage == VK_NULL_HANDLE) { MGLOG_E_ONCE("DirectVulkan::ReadDepthStencilPixels skipped: the default framebuffer has no " "depth/stencil image"); @@ -10991,12 +11038,12 @@ void main() { "stencil clear"); } const VkFormat swapchainDepthFormat = m_swapchainObject.GetDepthStencilFormat(); - VkImageLayout trackedLayout = m_swapchainObject.GetDepthStencilImageLayout(m_imageIndexAcquired); + VkImageLayout trackedLayout = m_swapchainObject.GetDepthStencilImageLayout(srcImageIndex); ReadDepthStencilImageToClient(swapchainDepthImage, swapchainDepthFormat, &trackedLayout, GetDepthStencilAspectMaskForFormat(swapchainDepthFormat), 0, 0, x, y, width, height, format, type, pixels, /*defaultFramebufferOrientation=*/true); - m_swapchainObject.SetDepthStencilImageLayout(m_imageIndexAcquired, trackedLayout); + m_swapchainObject.SetDepthStencilImageLayout(srcImageIndex, trackedLayout); return; } @@ -15588,6 +15635,11 @@ void main() { ShutdownSwapchain(); CreateSwapchain(); + // Every image of the fresh swapchain holds garbage, so the default framebuffer restarts + // at index 0: the next write into it re-points the GL-visible index at whatever the next + // acquire returns (see m_defaultFramebufferImageIndex). Without this the index could name + // an image of the swapchain just destroyed. + m_defaultFramebufferImageIndex = 0; VK_VERIFY(m_frameContext.InitializeSwapchainSemaphores(m_device, static_cast(m_swapchainObject.GetImageCount())), "RecreateSwapchain, InitializeSwapchainSemaphores"); diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.h b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.h index 5159918d..19172bd1 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.h +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.h @@ -459,11 +459,17 @@ namespace MobileGL::MG_Backend::DirectVulkan { VkImageAspectFlags aspect = 0; Uint32 level = 0, layer = 0, layers = 1, levels = 1; Bool isDefault = false; + // Set when isDefault: the swapchain image index this role resolved to. + Uint32 swapchainImageIndex = 0; Bool is3D = false; MG_Pipe::MGPipeHandle storage = MG_Pipe::kMGPipeNullHandle; }; + // `isWriteTarget` selects which side of the default framebuffer this resolves: a write + // re-points it at the image Present() acquired, a read keeps the current one (see + // m_defaultFramebufferImageIndex). Reads must pass false. WireImage ResolveWireImage(const MG_Pipe::MGPFramebufferState& fbo, - const MG_Pipe::MGPSurface& surface, VkImageAspectFlags aspect); + const MG_Pipe::MGPSurface& surface, VkImageAspectFlags aspect, + Bool isWriteTarget = false); void TransitionWireImage(WireImage& image, VkImageLayout layout); void ClearWireFramebuffer(const MG_Pipe::MGPFramebufferState& fbo, const ClearAttachmentPayload& payload, GLint drawbuffer = -1); @@ -939,8 +945,29 @@ namespace MobileGL::MG_Backend::DirectVulkan { VkBufferManager m_bufferManager; Uint m_imageIndexAcquired = 0; + // The swapchain image GL addresses as the default framebuffer: the one a readback, a + // blit source or a copy out of it resolves, and the one the app last rendered into. + // Present() runs ahead - it presents the frame and acquires the next image without + // waiting for the apply that would render into it (kWaitPresent) - so this must NOT + // follow m_imageIndexAcquired. Before it existed a readback resolved the freshly + // acquired image, which nothing had written yet: the pure-black sundial-lite frame. + Uint m_defaultFramebufferImageIndex = 0; FrameContext m_frameContext; + // Resolves the swapchain image index a READ of the default framebuffer must use. A + // non-default framebuffer ignores the index; it keeps the acquired one, which still + // keys its render-pass cache. + Uint32 DefaultFramebufferReadIndex(Bool isDefaultFramebuffer) const { + return isDefaultFramebuffer ? m_defaultFramebufferImageIndex : m_imageIndexAcquired; + } + + // Same for a WRITE: the default framebuffer starts addressing the image Present() + // acquired only once something renders into it again. + Uint32 DefaultFramebufferWriteIndex(Bool isDefaultFramebuffer) { + if (isDefaultFramebuffer) m_defaultFramebufferImageIndex = m_imageIndexAcquired; + return DefaultFramebufferReadIndex(isDefaultFramebuffer); + } + UniquePtr m_pipelineFactory; // Single-slot "last pipeline" memo: skip the per-draw GetOrCreatePipeline work (state // gather + synthetic vertex-input rebuild + payload hash + lookup) when the full pipeline diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/WireColorBlit.inc b/MobileGL/MG_Backend/DirectVulkan/Renderer/WireColorBlit.inc index 866c8ccc..530e0527 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/WireColorBlit.inc +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/WireColorBlit.inc @@ -288,7 +288,7 @@ retired.imageViews = {sourceView, destinationView}; retired.descriptorPools.push_back(descriptorPool); RetireWireObjects(Move(retired)); - if (destination.isDefault) m_swapchainObject.SetImageContentDefined(m_imageIndexAcquired, true); + if (destination.isDefault) m_swapchainObject.SetImageContentDefined(destination.swapchainImageIndex, true); return true; } #endif diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/WireDraw.inc b/MobileGL/MG_Backend/DirectVulkan/Renderer/WireDraw.inc index db835c2d..457fd064 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/WireDraw.inc +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/WireDraw.inc @@ -352,10 +352,13 @@ passKey.framebufferSrgb = MGB_CTX->IsCapabilityEnabled(CapabilityInput::FramebufferSrgb); if (fbo->IsDefault) { passKey.swapchain = m_swapchainObject.GetHandle(); - passKey.swapchainImageIndex = m_imageIndexAcquired; + // A drawing pass on the default framebuffer is what makes the freshly acquired + // swapchain image the one GL addresses (see m_defaultFramebufferImageIndex); the + // attachments below resolve the same index, so this key has to be taken after it. + passKey.swapchainImageIndex = DefaultFramebufferWriteIndex(true); } const auto add = [&](const MG_Pipe::MGPSurface& surface, VkImageAspectFlags aspect) -> Uint32 { - WireImage image = ResolveWireImage(*fbo, surface, aspect); + WireImage image = ResolveWireImage(*fbo, surface, aspect, /*isWriteTarget=*/true); if (!image.image) return VK_ATTACHMENT_UNUSED; m_textureManager->FlushPendingUploads(); if (aspect == VK_IMAGE_ASPECT_COLOR_BIT) @@ -391,8 +394,8 @@ if (!image.isDefault && surface.Kind == MG_Pipe::kMGPipeSurfaceKindTexture) m_textureManager->MarkWireTextureGpuWritten(image.storage, image.level, image.layer, image.layers); if (image.isDefault) { - if (aspect == VK_IMAGE_ASPECT_COLOR_BIT) m_swapchainObject.SetImageContentDefined(m_imageIndexAcquired, true); - else m_swapchainObject.SetDepthStencilContentDefined(m_imageIndexAcquired, true); + if (aspect == VK_IMAGE_ASPECT_COLOR_BIT) m_swapchainObject.SetImageContentDefined(image.swapchainImageIndex, true); + else m_swapchainObject.SetDepthStencilContentDefined(image.swapchainImageIndex, true); } return static_cast(attachments.size() - 1); }; diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/WireFramebuffer.inc b/MobileGL/MG_Backend/DirectVulkan/Renderer/WireFramebuffer.inc index d4ca5b4c..b00e9506 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/WireFramebuffer.inc +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/WireFramebuffer.inc @@ -8,20 +8,22 @@ VulkanRenderer::WireImage VulkanRenderer::ResolveWireImage( const MG_Pipe::MGPFramebufferState& fbo, const MG_Pipe::MGPSurface& surface, - VkImageAspectFlags aspect) { + VkImageAspectFlags aspect, Bool isWriteTarget) { WireImage out{}; out.aspect = aspect; if (fbo.IsDefault) { out.isDefault = true; out.extent = m_swapchainObject.GetExtent(); + out.swapchainImageIndex = isWriteTarget ? DefaultFramebufferWriteIndex(true) + : DefaultFramebufferReadIndex(true); if (aspect == VK_IMAGE_ASPECT_COLOR_BIT) { - out.image = m_swapchainObject.GetImage(m_imageIndexAcquired); + out.image = m_swapchainObject.GetImage(out.swapchainImageIndex); out.format = m_swapchainObject.GetSurfaceFormat().format; - out.layout = m_swapchainObject.GetImageLayout(m_imageIndexAcquired); + out.layout = m_swapchainObject.GetImageLayout(out.swapchainImageIndex); } else { - out.image = m_swapchainObject.GetDepthStencilImage(m_imageIndexAcquired); + out.image = m_swapchainObject.GetDepthStencilImage(out.swapchainImageIndex); out.format = m_swapchainObject.GetDepthStencilFormat(); - out.layout = m_swapchainObject.GetDepthStencilImageLayout(m_imageIndexAcquired); + out.layout = m_swapchainObject.GetDepthStencilImageLayout(out.swapchainImageIndex); } return out; } @@ -91,8 +93,8 @@ if (image.trackedLayout) *image.trackedLayout = image.layout; if (image.isDefault) { if (image.aspect == VK_IMAGE_ASPECT_COLOR_BIT) - m_swapchainObject.SetImageLayout(m_imageIndexAcquired, image.layout); - else m_swapchainObject.SetDepthStencilImageLayout(m_imageIndexAcquired, image.layout); + m_swapchainObject.SetImageLayout(image.swapchainImageIndex, image.layout); + else m_swapchainObject.SetDepthStencilImageLayout(image.swapchainImageIndex, image.layout); } } @@ -123,7 +125,7 @@ } } if (!aspect) return; - WireImage image = ResolveWireImage(fbo, surface, aspect); + WireImage image = ResolveWireImage(fbo, surface, aspect, /*isWriteTarget=*/true); if (!image.image) return; m_textureManager->FlushPendingUploads(); VkRect2D area{{0, 0}, image.extent}; @@ -196,8 +198,8 @@ if (!image.isDefault && surface.Kind == MG_Pipe::kMGPipeSurfaceKindTexture) m_textureManager->MarkWireTextureGpuWritten(image.storage, image.level, image.layer, image.layers); if (image.isDefault) { - if (color) m_swapchainObject.SetImageContentDefined(m_imageIndexAcquired, true); - else m_swapchainObject.SetDepthStencilContentDefined(m_imageIndexAcquired, true); + if (color) m_swapchainObject.SetImageContentDefined(image.swapchainImageIndex, true); + else m_swapchainObject.SetDepthStencilContentDefined(image.swapchainImageIndex, true); } }; if (payload.mask & GL_COLOR_BUFFER_BIT) { @@ -224,15 +226,25 @@ if (format == GL_DEPTH_STENCIL) aspect = VK_IMAGE_ASPECT_DEPTH_BIT | VK_IMAGE_ASPECT_STENCIL_BIT; const auto& surface = (aspect & VK_IMAGE_ASPECT_DEPTH_BIT) ? fbo->Depth : aspect == VK_IMAGE_ASPECT_STENCIL_BIT ? fbo->Stencil : fbo->ReadSurface; - WireImage source = ResolveWireImage(*fbo, surface, aspect); + WireImage source = ResolveWireImage(*fbo, surface, aspect, /*isWriteTarget=*/false); if (!source.image) return; + if (source.isDefault && aspect == VK_IMAGE_ASPECT_COLOR_BIT && + !m_swapchainObject.IsImageContentDefined(source.swapchainImageIndex)) { + // Named rather than silent: the readback hands back whatever the image holds. The + // ordinary cause is a readback of the default framebuffer applied after the Present + // that consumed it, where the app's own colour is still what is expected (see + // m_defaultFramebufferImageIndex). + MGLOG_W_ONCE("DirectVulkan::ReadPixels: no write is recorded for swapchain image %u of the " + "default framebuffer; the readback returns its raw pixels", + source.swapchainImageIndex); + } m_textureManager->FlushPendingUploads(); TransitionWireImage(source, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL); if (aspect != VK_IMAGE_ASPECT_COLOR_BIT) { ReadDepthStencilImageToClient(source.image,source.format, source.trackedLayout ? source.trackedLayout : &source.layout,aspect, source.level,source.layer,x,y,width,height,format,type,pixels,source.isDefault); - if (source.isDefault) m_swapchainObject.SetDepthStencilImageLayout(m_imageIndexAcquired,source.layout); + if (source.isDefault) m_swapchainObject.SetDepthStencilImageLayout(source.swapchainImageIndex,source.layout); return; } const SizeT texelSize = aspect == VK_IMAGE_ASPECT_STENCIL_BIT ? 1 : GetReadbackTexelSize(source.format); @@ -485,8 +497,8 @@ state.VerbBlitNamedConsumed = named; const auto blit = [&](const MG_Pipe::MGPSurface& src, const MG_Pipe::MGPSurface& dst, VkImageAspectFlags aspect) { - WireImage source = ResolveWireImage(*read,src,aspect); - WireImage destination = ResolveWireImage(*draw,dst,aspect); + WireImage source = ResolveWireImage(*read,src,aspect, /*isWriteTarget=*/false); + WireImage destination = ResolveWireImage(*draw,dst,aspect, /*isWriteTarget=*/true); if (!source.image || !destination.image) return; m_textureManager->FlushPendingUploads(); if (aspect != VK_IMAGE_ASPECT_COLOR_BIT && source.samples != VK_SAMPLE_COUNT_1_BIT && @@ -552,7 +564,7 @@ if (!BlitWireColorToDefault(resolved, destination, rx0, ry0, rx1, ry1, dx0, dy0, dx1, dy1, filter)) MagmaWireFatal("multisample-default-blit"); - m_swapchainObject.SetImageContentDefined(m_imageIndexAcquired, true); + m_swapchainObject.SetImageContentDefined(destination.swapchainImageIndex, true); } else { TransitionWireImage(resolved, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL); TransitionWireImage(destination, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL); @@ -587,7 +599,7 @@ if (!BlitWireColorToDefault(source, destination, sx0, sy0, sx1, sy1, dx0, dy0, dx1, dy1, filter)) MagmaWireFatal("default-color-blit-shape@P7"); - m_swapchainObject.SetImageContentDefined(m_imageIndexAcquired, true); + m_swapchainObject.SetImageContentDefined(destination.swapchainImageIndex, true); return; } const Bool sameImage = source.image == destination.image; @@ -634,8 +646,8 @@ if (!destination.isDefault && dst.Kind == MG_Pipe::kMGPipeSurfaceKindTexture) m_textureManager->MarkWireTextureGpuWritten(destination.storage, destination.level, destination.layer, destination.layers); if (destination.isDefault) { - if (aspect == VK_IMAGE_ASPECT_COLOR_BIT) m_swapchainObject.SetImageContentDefined(m_imageIndexAcquired,true); - else m_swapchainObject.SetDepthStencilContentDefined(m_imageIndexAcquired,true); + if (aspect == VK_IMAGE_ASPECT_COLOR_BIT) m_swapchainObject.SetImageContentDefined(destination.swapchainImageIndex,true); + else m_swapchainObject.SetDepthStencilContentDefined(destination.swapchainImageIndex,true); } }; if (mask & GL_COLOR_BUFFER_BIT) { @@ -669,8 +681,9 @@ surface.Res = state.VerbCopyTexDst; surface.Level = static_cast(level); WireImage source = ResolveWireImage(*read,aspect & VK_IMAGE_ASPECT_DEPTH_BIT ? read->Depth : - aspect & VK_IMAGE_ASPECT_STENCIL_BIT ? read->Stencil : read->ReadSurface,aspect); - WireImage destination = ResolveWireImage(fake,surface,aspect); + aspect & VK_IMAGE_ASPECT_STENCIL_BIT ? read->Stencil : read->ReadSurface,aspect, + /*isWriteTarget=*/false); + WireImage destination = ResolveWireImage(fake,surface,aspect, /*isWriteTarget=*/false); if (!source.image || !destination.image) return; m_textureManager->FlushPendingUploads(); const Bool sameImage = source.image == destination.image; diff --git a/MobileGL/MG_IntegrationTest/CMakeLists.txt b/MobileGL/MG_IntegrationTest/CMakeLists.txt index eb5351ef..87d37e5a 100644 --- a/MobileGL/MG_IntegrationTest/CMakeLists.txt +++ b/MobileGL/MG_IntegrationTest/CMakeLists.txt @@ -152,6 +152,7 @@ add_executable(MobileGLIntegrationTest # run the same cases with MOBILEGL_TRANSPORT=inproc. Scenarios/TriangleScenario.cpp Scenarios/PersistentCoherentMapScenario.cpp + Scenarios/DefaultFramebufferAcrossSwapScenario.cpp ) target_include_directories(MobileGLIntegrationTest PRIVATE diff --git a/MobileGL/MG_IntegrationTest/Scenarios/DefaultFramebufferAcrossSwapScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/DefaultFramebufferAcrossSwapScenario.cpp new file mode 100644 index 00000000..234eb5c9 --- /dev/null +++ b/MobileGL/MG_IntegrationTest/Scenarios/DefaultFramebufferAcrossSwapScenario.cpp @@ -0,0 +1,176 @@ +// MobileGL - MobileGL/MG_IntegrationTest/Scenarios/DefaultFramebufferAcrossSwapScenario.cpp +// Copyright (c) 2025-2026 MobileGL-Dev +// Licensed under the GNU Lesser General Public License v3.0: +// https://www.gnu.org/licenses/gpl-3.0.txt +// https://www.gnu.org/licenses/lgpl-3.0.txt +// SPDX-License-Identifier: LGPL-3.0-only +// End of Source File Header +// +// Scenario - THE DEFAULT FRAMEBUFFER KEEPS NAMING THE IMAGE GL LAST RENDERED INTO, +// ACROSS A SWAP. +// +// A retrace takes its snapshot at the swap call, BEFORE the present executes: that is the +// frame the trace's golden image shows. A disaggregated backend applies the present record +// asynchronously (kWaitPresent: the client does not wait for the apply), and DirectVulkan's +// Present() ends by acquiring the image for the frame that is about to start - so the +// readback that belongs to the frame being presented can be applied AFTER that acquire. It +// used to resolve the freshly acquired image, which nothing had written: the sundial-lite +// retrace came back pure black (409920 zero pixels) while every offscreen path in it was +// fine, and the same library passed 30/30 locally because the two orders only differ under +// load. DirectGLES never had the bug - the driver keeps handing back the buffer the app drew +// into - which is the control this scenario needs: a DirectGLES failure here is the +// scenario's, not the backend's. +// +// Two claims, and neither can be satisfied by ignoring the other: +// +// * a readback issued after a swap, with no draw in between, still sees the colour the +// app drew into the default framebuffer (that is the retrace snapshot's frame); +// * the NEXT draw into the default framebuffer does land in the image the readback then +// resolves - i.e. the write re-points the default framebuffer at what Present() +// acquired, rather than every later frame inheriting the presented image. +// +// Together they pin the invariant the backend implements (VulkanRenderer.h, +// m_defaultFramebufferImageIndex): Present() runs ahead, a write to the default framebuffer +// is what makes the acquired image the one GL addresses, and reads resolve whatever that is. + +#include +#include + +#include "../Harness/HeadlessGL.h" +#include "../Harness/ScenarioFixture.h" + +#ifdef GLAPI +#undef GLAPI +#endif +#define GL_GLEXT_PROTOTYPES +#include +#include +#undef GL_GLEXT_PROTOTYPES + +namespace MGITest { + namespace { + + constexpr const char* kVS = R"(#version 330 core +in vec2 aPos; +in vec4 aColor; +out vec4 vColor; +void main() { + vColor = aColor; + gl_Position = vec4(aPos, 0.0, 1.0); +} +)"; + + constexpr const char* kFS = R"(#version 330 core +in vec4 vColor; +out vec4 o_color; +void main() { o_color = vColor; } +)"; + + class DefaultFramebufferAcrossSwapScenario : public ScenarioTest {}; + + // One interleaved VBO: aPos (vec2) then aColor (vec4). CompileProgram pins those two + // attribute names to locations 0 and 1. The colours below are fully saturated or fully + // off, so the readback lands on a named colour whatever the driver's exact rounding is. + void DrawFullViewportQuad(unsigned int program, float red, float green, float blue) { + const float vertices[] = { + -1.0f, -1.0f, red, green, blue, 1.0f, + 1.0f, -1.0f, red, green, blue, 1.0f, + -1.0f, 1.0f, red, green, blue, 1.0f, + 1.0f, 1.0f, red, green, blue, 1.0f, + }; + GLuint vao = 0, vbo = 0; + glGenVertexArrays(1, &vao); + glBindVertexArray(vao); + glGenBuffers(1, &vbo); + glBindBuffer(GL_ARRAY_BUFFER, vbo); + glBufferData(GL_ARRAY_BUFFER, sizeof(vertices), vertices, GL_STATIC_DRAW); + glEnableVertexAttribArray(0); + glVertexAttribPointer(0, 2, GL_FLOAT, GL_FALSE, 6 * sizeof(float), nullptr); + glEnableVertexAttribArray(1); + glVertexAttribPointer(1, 4, GL_FLOAT, GL_FALSE, 6 * sizeof(float), + reinterpret_cast(2 * sizeof(float))); + glUseProgram(program); + glDrawArrays(GL_TRIANGLE_STRIP, 0, 4); + glBindVertexArray(0); + glDeleteBuffers(1, &vbo); + glDeleteVertexArrays(1, &vao); + } + + void PaintDefaultFramebuffer(unsigned int program, float red, float green, float blue) { + BindDefaultFramebuffer(); + glViewport(0, 0, HeadlessGL::Get().Width(), HeadlessGL::Get().Height()); + glDisable(GL_SCISSOR_TEST); + glDisable(GL_DEPTH_TEST); + DrawFullViewportQuad(program, red, green, blue); + } + + } // namespace + + TEST_F(DefaultFramebufferAcrossSwapScenario, AReadbackAfterASwapStillSeesTheFrameThatWasDrawn) { + if (!Ready()) return; + HeadlessGL& gl = Gl(); + const int width = gl.Width(); + const int height = gl.Height(); + ASSERT_GE(width, 8); + ASSERT_GE(height, 8); + + std::string error; + const unsigned int program = CompileProgram(kVS, kFS, &error); + ASSERT_NE(program, 0u) << error; + + PaintDefaultFramebuffer(program, 1.0f, 0.0f, 0.0f); + const Image before = ReadPixels(width, height); + ASSERT_TRUE(RegionIsMostly(before, 0, width - 1, 0, height - 1, "red", 0.0, + "the readback of the frame that was just drawn")) + << "the setup frame did not paint; the cross-swap claim would be vacuous"; + EXPECT_EQ(FirstGLError(), 0u); + + // The swap is the frame boundary the retrace snapshot sits on. Nothing is drawn into + // the default framebuffer afterwards, so this is exactly the readback that used to + // resolve the image Present() had just acquired - the black sundial-lite frame. + gl.EndFrame(); + + const Image after = ReadPixels(width, height); + EXPECT_EQ(FirstGLError(), 0u); + EXPECT_TRUE(RegionIsMostly(after, 0, width - 1, 0, height - 1, "red", 0.0, + "the readback of the frame that was presented")) + << "the default framebuffer stopped naming the image the app rendered into once a swap " + "had run; Present() acquires ahead of the app (and a kWaitPresent record is applied " + "without the app waiting), so this readback must not resolve the freshly acquired " + "image " << gl.FrameIndex() << " frames in"; + + gl.EndFrame(); + glDeleteProgram(program); + } + + TEST_F(DefaultFramebufferAcrossSwapScenario, ADrawAfterASwapLandsInTheImageTheReadbackThenSees) { + if (!Ready()) return; + HeadlessGL& gl = Gl(); + const int width = gl.Width(); + const int height = gl.Height(); + ASSERT_GE(width, 8); + ASSERT_GE(height, 8); + + std::string error; + const unsigned int program = CompileProgram(kVS, kFS, &error); + ASSERT_NE(program, 0u) << error; + + PaintDefaultFramebuffer(program, 1.0f, 0.0f, 0.0f); + gl.EndFrame(); + + // The other half of the same rule, and the half that a fix which simply stopped + // following the swapchain index would break: the first draw after a swap has to land in + // the image Present() acquired, and the readback has to follow it there. + PaintDefaultFramebuffer(program, 0.0f, 0.0f, 1.0f); + const Image repainted = ReadPixels(width, height); + EXPECT_EQ(FirstGLError(), 0u); + EXPECT_TRUE(RegionIsMostly(repainted, 0, width - 1, 0, height - 1, "blue", 0.0, + "the readback of the frame drawn after the swap")) + << "a draw issued after a swap was not visible to glReadPixels: the default framebuffer " + "did not re-point at the image Present() acquired"; + + gl.EndFrame(); + glDeleteProgram(program); + } + +} // namespace MGITest