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