From 24da1e0f6d949087dedc33d60491dd7e0981944d Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Tue, 22 Sep 2026 21:19:57 -0400 Subject: [PATCH] [MG_Backend, MG_IntegrationTest] (P7): the default draw framebuffer is asked the multisample depth/stencil shape question before any aspect runs, so a scaled COLOR|DEPTH resolve onto the window writes nothing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - BlitWireFramebuffers' round-2 pre-pass left a default DRAW framebuffer to the per-aspect lambdas on the belief that a default draw behind a multisample read was ResolveWireDepthStencil's region Fatal whatever the shape. It is not: that arm calls DeclineWireDepthStencilResolveShape BEFORE its destination.isDefault region check, so a scaled (or X-mirrored) COLOR|DEPTH blit out of a multisample framebuffer onto the window ran the colour lambda first (BlitWireColorToDefault wrote the swapchain image) and then recorded INVALID_OPERATION on its depth - the partial effect magma-b2.md ยง2.9 item 3 declared closed. - The guard drops !draw->IsDefault, and the pre-pass resolves both sides as READS (isWriteTarget=false): only their format and samples are consulted, and DefaultFramebufferReadIndex is const, so nothing re-points the default framebuffer at the acquired image before a write. - The flipped-resolve case gains a leg: the window is primed blue at depth 0.5, the multisample source (red below, green above) is blown up over the whole of it with COLOR|DEPTH, the call must be INVALID_OPERATION, and three probes (corner, centre, far corner) must read the priming on both aspects through glReadPixels - the only way the harness's pbuffer default framebuffer can be looked at. - Red-once (split + spawn, restored): the round-2 guard (&& !draw->IsDefault) put back -> DirectVulkan.{Split,Spawn}.MsFlip. fail "the declined COLOR|DEPTH scale onto the window must not have written its colour at (1, 1)" reading {255,0,0,255} (the source's bottom band), and {0,255,0,255} at (64, 48) and (126, 94), for {0,0,255,255}; the INVALID_OPERATION assertion still held, which is the partial effect exactly. - Verified: the 107 multisample blit/resolve and depth/stencil readback entries on the three wire arms (MsFlip/MsFlip1/MsBlit/MsResolve0/1, Full x3 for both readback scenarios) and the monolith DirectVulkan.DepthStencilReadback* entries pass, pre-existing skips unchanged. --- .../DirectVulkan/Renderer/WireFramebuffer.inc | 19 ++++--- .../DepthStencilReadbackMatrixScenario.cpp | 53 +++++++++++++++++++ 2 files changed, 66 insertions(+), 6 deletions(-) diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/WireFramebuffer.inc b/MobileGL/MG_Backend/DirectVulkan/Renderer/WireFramebuffer.inc index 4d27fbbf..423257ab 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/WireFramebuffer.inc +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/WireFramebuffer.inc @@ -819,16 +819,23 @@ // images, so it is asked here, before any aspect runs, for every depth/stencil aspect // the mask names that would take the multisample resolve arm. // - // A default READ framebuffer is never multisampled here, and a default DRAW one behind a - // multisample read is ResolveWireDepthStencil's region Fatal whatever the shape - and - // resolving the default draw image early would move its write index - so both are left - // to the lambda. - if ((mask & (GL_DEPTH_BUFFER_BIT | GL_STENCIL_BUFFER_BIT)) && !read->IsDefault && !draw->IsDefault) { + // A default READ framebuffer is never multisampled here, so it is left to the lambda. A + // default DRAW framebuffer is asked like any other (review round 3): round 2 left it out + // on the belief that a default draw behind a multisample read was ResolveWireDepthStencil's + // region Fatal whatever the shape, but that arm asks DeclineWireDepthStencilResolveShape + // BEFORE its region check - so a scaled (or X-mirrored) COLOR|DEPTH blit out of a + // multisample framebuffer onto the window scaled and wrote its colour through + // BlitWireColorToDefault and then recorded INVALID_OPERATION on its depth, the very + // partial effect this block exists to close. Both sides are resolved as READS + // (isWriteTarget=false): only their format and samples are consulted here, and a write + // resolve of the default framebuffer would re-point it at the acquired swapchain image + // before anything had been written to it. + if ((mask & (GL_DEPTH_BUFFER_BIT | GL_STENCIL_BUFFER_BIT)) && !read->IsDefault) { const auto declines = [&](const MG_Pipe::MGPSurface& src, const MG_Pipe::MGPSurface& dst, VkImageAspectFlags aspect) -> Bool { const WireImage source = ResolveWireImage(*read, src, aspect, /*isWriteTarget=*/false); if (!source.image || source.samples == VK_SAMPLE_COUNT_1_BIT) return false; - const WireImage destination = ResolveWireImage(*draw, dst, aspect, /*isWriteTarget=*/true); + const WireImage destination = ResolveWireImage(*draw, dst, aspect, /*isWriteTarget=*/false); if (!destination.image || destination.samples != VK_SAMPLE_COUNT_1_BIT) return false; return DeclineWireDepthStencilResolveShape(source, destination, sx0, sy0, sx1, sy1, dx0, dy0, dx1, dy1); diff --git a/MobileGL/MG_IntegrationTest/Scenarios/DepthStencilReadbackMatrixScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/DepthStencilReadbackMatrixScenario.cpp index 15a1796e..aaefe9c5 100644 --- a/MobileGL/MG_IntegrationTest/Scenarios/DepthStencilReadbackMatrixScenario.cpp +++ b/MobileGL/MG_IntegrationTest/Scenarios/DepthStencilReadbackMatrixScenario.cpp @@ -614,6 +614,59 @@ namespace MGITest { "the empty-destination decline must have left the depth alone"); EXPECT_EQ(FirstGLError(), 0u) << "the session survived the empty-destination decline"; + // THE SAME SCALE ONTO THE WINDOW (review round 3). The default framebuffer was the one + // destination the round-2 pre-pass left to the per-aspect lambdas, on the belief that a + // default draw framebuffer behind a multisample read was the resolve arm's region Fatal + // whatever the shape - but that arm asks the shape question BEFORE its region check, so + // a scaled COLOR|DEPTH resolve onto the window scaled and wrote its colour through the + // swapchain blit and then recorded INVALID_OPERATION on its depth: the partial effect + // the previous leg pins for a user framebuffer, reproduced on the only other kind of + // destination there is. The window is primed blue at depth 0.5, the source (red below, + // green above) is blown up over the whole of it, and three probes - a corner, the + // centre, the far corner - must all still read the priming on both aspects. The + // harness's pbuffer default framebuffer has no other way of being looked at than + // glReadPixels, which is what both probes use. + { + HeadlessGL& gl = Gl(); + const int windowWidth = gl.Width(); + const int windowHeight = gl.Height(); + ASSERT_GE(windowWidth, 4); + ASSERT_GE(windowHeight, 4); + glBindFramebuffer(GL_FRAMEBUFFER, 0); + glDisable(GL_SCISSOR_TEST); + glViewport(0, 0, windowWidth, windowHeight); + glColorMask(GL_TRUE, GL_TRUE, GL_TRUE, GL_TRUE); + glDepthMask(GL_TRUE); + glClearColor(0, 0, 1, 1); + glClearDepth(0.5); + glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT); + ASSERT_EQ(FirstGLError(), 0u) << "priming the default framebuffer"; + glBindFramebuffer(GL_READ_FRAMEBUFFER, multisampled.fbo); + glBindFramebuffer(GL_DRAW_FRAMEBUFFER, 0); + glBlitFramebuffer(0, 0, kWidth, kHeight, 0, 0, windowWidth, windowHeight, + GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT, GL_NEAREST); + glFinish(); + EXPECT_EQ(FirstGLError(), GLenum(GL_INVALID_OPERATION)) + << "a scaled multisample COLOR|DEPTH blit onto the default framebuffer is INVALID_OPERATION as a whole"; + glBindFramebuffer(GL_FRAMEBUFFER, 0); + const int probeX[3] = {1, windowWidth / 2, windowWidth - 2}; + const int probeY[3] = {1, windowHeight / 2, windowHeight - 2}; + for (int i = 0; i < 3; ++i) { + const int x = probeX[i], y = probeY[i]; + std::array window{}; + glReadPixels(x, y, 1, 1, GL_RGBA, GL_UNSIGNED_BYTE, window.data()); + EXPECT_EQ(window, (std::array{0, 0, 255, 255})) + << "the declined COLOR|DEPTH scale onto the window must not have written its colour at (" + << x << ", " << y << ")"; + float depth = kDepthPoison; + glReadPixels(x, y, 1, 1, GL_DEPTH_COMPONENT, GL_FLOAT, &depth); + EXPECT_NEAR(depth, 0.5f, 1.0f / 4096.0f) + << "the declined COLOR|DEPTH scale onto the window must have left its depth alone at (" + << x << ", " << y << ")"; + } + EXPECT_EQ(FirstGLError(), 0u) << "the session survived the default-framebuffer decline"; + } + DestroySource(resolved); DestroySource(multisampled); glBindFramebuffer(GL_FRAMEBUFFER, 0);