From 431bda67dce3e38ee35c532cec2febc1920ffb18 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Wed, 23 Sep 2026 15:20:17 -0400 Subject: [PATCH] [MG_Backend, MG_IntegrationTest] (P7): a real-probe negative control proves the depth-resolve measurement can see a broken render-pass arm on the host --- .../DirectVulkan/Renderer/VulkanRenderer.cpp | 17 +++++--- .../Renderer/WireDepthResolveArm.h | 20 +++++++-- .../Renderer/WireDepthResolveProbe.cpp | 21 ++++++---- .../Renderer/WireDepthResolveProbe.h | 4 ++ MobileGL/MG_IntegrationTest/CMakeLists.txt | 28 +++++++++++++ .../DepthStencilReadbackMatrixScenario.cpp | 42 +++++++++++++++++-- .../MG_Test/Pipeline/PipelineQuirkTest.cpp | 14 +++++++ scripts/ci/spawn_lane_parity.py | 4 +- 8 files changed, 130 insertions(+), 20 deletions(-) diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp index 0c15fa71..fa325ee2 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp @@ -15826,7 +15826,10 @@ void main() { // lane runs on a device with the defect - and the DirectVulkan.{Split,Spawn}.MsResolveBug./ // MsFlipBug. entries force `bug` and read ResolveWireDepthStencil's arm line back from the // server log. The canned measurement is evaluated like a real one, so those entries go red - // when the evaluation stops detecting the defect. + // when the evaluation stops detecting the defect. MGITEST_MAGMA_DEPTH_RESOLVE_PROBE=elide-subject + // runs the REAL probe with its render-pass resolve left unrecorded (the sentinel survives, the + // shader control still resolves), so the DirectVulkan.{Split,Spawn}.MsResolveElide. entries go + // red when the real recording, readback or tally stops detecting it too. void VulkanRenderer::ArmWireDepthResolveOrder() { m_wirePreferShaderDepthResolve = false; if (MG_Config::Transport == MG_Config::TransportMode::Monolith) return; @@ -15838,8 +15841,8 @@ void main() { const char* knobValue = std::getenv("MGITEST_MAGMA_DEPTH_RESOLVE_PROBE"); const WireDepthResolveProbeKnob knob = ParseWireDepthResolveProbeKnob(knobValue); if (knob == WireDepthResolveProbeKnob::Unrecognised) - MGLOG_W("DirectVulkan: MGITEST_MAGMA_DEPTH_RESOLVE_PROBE=%s is neither `bug` nor `clean`; " - "the resolve probe runs as if it were unset", knobValue); + MGLOG_W("DirectVulkan: MGITEST_MAGMA_DEPTH_RESOLVE_PROBE=%s is not `bug`, `clean` or " + "`elide-subject`; the resolve probe runs as if it were unset", knobValue); WireDepthResolveArmChoice choice = ChooseWireDepthResolveArm(knob, renderPassArmAvailable, [&]() { WireDepthResolveProbeContext context; context.physicalDevice = m_physicalDevice.handle; @@ -15850,6 +15853,7 @@ void main() { context.depthResolveModes = m_wireDepthResolveModes; context.stencilResolveModes = m_wireStencilResolveModes; context.shaderStencilExport = m_wireShaderStencilExport; + context.elideSubject = knob == WireDepthResolveProbeKnob::ElideSubject; return RunWireDepthResolveProbe(context); }); if (choice.measurement.fenceWaitTimedOut) { @@ -15865,8 +15869,11 @@ void main() { } MGLOG_I("DirectVulkan: depth/stencil resolve probe (%s) verdict=%s: %s%s%s", choice.measurement.fromKnob ? "forced by MGITEST_MAGMA_DEPTH_RESOLVE_PROBE" - : renderPassArmAvailable ? "measured on this device" - : "not run: no render-pass arm", + : choice.measurement.subjectElided + ? "measured on this device with its render-pass resolve elided by " + "MGITEST_MAGMA_DEPTH_RESOLVE_PROBE=elide-subject" + : renderPassArmAvailable ? "measured on this device" + : "not run: no render-pass arm", WireDepthResolveProbeVerdictName(choice.verdict), choice.preferShader ? "the shader pass resolves first, the no-draw render pass is the fallback" : (renderPassArmAvailable || choice.measurement.fromKnob) diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/WireDepthResolveArm.h b/MobileGL/MG_Backend/DirectVulkan/Renderer/WireDepthResolveArm.h index bf7d9500..a9d240cf 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/WireDepthResolveArm.h +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/WireDepthResolveArm.h @@ -75,8 +75,11 @@ namespace MobileGL::MG_Backend::DirectVulkan { // (the queue may still be executing it), and nothing was read back. Bool fenceWaitTimedOut = false; String failureReason; - // Came from MGITEST_MAGMA_DEPTH_RESOLVE_PROBE, not from the device. + // Came from MGITEST_MAGMA_DEPTH_RESOLVE_PROBE=bug|clean, not from the device. Bool fromKnob = false; + // Measured on the device, but with MGITEST_MAGMA_DEPTH_RESOLVE_PROBE=elide-subject: the probe + // recorded no render-pass resolve at all, so the subject's target kept its sentinel. + Bool subjectElided = false; Vector formats; }; @@ -137,12 +140,22 @@ namespace MobileGL::MG_Backend::DirectVulkan { // with a canned one that says exactly that, and nothing else: the canned measurement still goes // through EvaluateWireDepthResolveProbe and the same member, so an entry that forces `bug` is red // when the evaluation stops detecting the defect. - enum class WireDepthResolveProbeKnob : Uint8 { Measure, ForceBug, ForceClean, Unrecognised }; + // + // A canned measurement cannot see the REAL probe - its recording, readback and tally - stop + // detecting the defect, and on lavapipe the real subject is clean, so nothing would. The third + // value, MGITEST_MAGMA_DEPTH_RESOLVE_PROBE=elide-subject, is that negative control: the REAL + // probe runs (ChooseWireDepthResolveArm treats it as Measure) and the caller passes it on as + // WireDepthResolveProbeContext::elideSubject, so the probe records no render-pass resolve and + // the real readback keeps the sentinel while the shader control still runs. The + // DirectVulkan.{Split,Spawn}.MsResolveElide. entries then require the defect verdict from the + // measurement itself. + enum class WireDepthResolveProbeKnob : Uint8 { Measure, ForceBug, ForceClean, ElideSubject, Unrecognised }; inline WireDepthResolveProbeKnob ParseWireDepthResolveProbeKnob(const char* value) { if (value == nullptr || *value == '\0') return WireDepthResolveProbeKnob::Measure; if (std::strcmp(value, "bug") == 0) return WireDepthResolveProbeKnob::ForceBug; if (std::strcmp(value, "clean") == 0) return WireDepthResolveProbeKnob::ForceClean; + if (std::strcmp(value, "elide-subject") == 0) return WireDepthResolveProbeKnob::ElideSubject; return WireDepthResolveProbeKnob::Unrecognised; } @@ -184,7 +197,8 @@ namespace MobileGL::MG_Backend::DirectVulkan { // The whole decision, with the Vulkan half behind `measure` (called at most once, and only when // the knob does not decide and the render-pass arm exists at all: without it the shader pass - // is the only arm and there is no order to choose). + // is the only arm and there is no order to choose). ElideSubject does not decide: it is + // measured like Measure, and the elision is the measuring callable's to apply. template WireDepthResolveArmChoice ChooseWireDepthResolveArm(WireDepthResolveProbeKnob knob, Bool renderPassArmAvailable, MeasureFn&& measure) { diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/WireDepthResolveProbe.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/WireDepthResolveProbe.cpp index e4eb48b2..d3ab8555 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/WireDepthResolveProbe.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/WireDepthResolveProbe.cpp @@ -405,6 +405,7 @@ namespace MobileGL::MG_Backend::DirectVulkan { WireDepthResolveProbeMeasurement RunWireDepthResolveProbe(const WireDepthResolveProbeContext& context) { WireDepthResolveProbeMeasurement measurement; + measurement.subjectElided = context.elideSubject; const VkDevice device = context.device; if (device == VK_NULL_HANDLE || context.queue == VK_NULL_HANDLE || context.createRenderPass2 == nullptr) { measurement.failureReason = "no device, queue or vkCreateRenderPass2"; @@ -762,14 +763,18 @@ namespace MobileGL::MG_Backend::DirectVulkan { vkCmdPipelineBarrier(cmd, VK_PIPELINE_STAGE_ALL_COMMANDS_BIT, VK_PIPELINE_STAGE_ALL_COMMANDS_BIT, 0, 1, &memory, 0, nullptr, 0, nullptr); } - // (4) THE SUBJECT: the render-pass arm, begun and ended with no draw. - for (const FormatWork& item : work) { - VkRenderPassBeginInfo begin{VK_STRUCTURE_TYPE_RENDER_PASS_BEGIN_INFO}; - begin.renderPass = item.resolvePass; - begin.framebuffer = item.resolveFramebuffer; - begin.renderArea = fullRect; - vkCmdBeginRenderPass(cmd, &begin, VK_SUBPASS_CONTENTS_INLINE); - vkCmdEndRenderPass(cmd); + // (4) THE SUBJECT: the render-pass arm, begun and ended with no draw. Under the test-only + // elideSubject it is not recorded at all: its target keeps the sentinel cleared in (2) and + // the layout it was left in, which is what (6) expects either way. + if (!context.elideSubject) { + for (const FormatWork& item : work) { + VkRenderPassBeginInfo begin{VK_STRUCTURE_TYPE_RENDER_PASS_BEGIN_INFO}; + begin.renderPass = item.resolvePass; + begin.framebuffer = item.resolveFramebuffer; + begin.renderArea = fullRect; + vkCmdBeginRenderPass(cmd, &begin, VK_SUBPASS_CONTENTS_INLINE); + vkCmdEndRenderPass(cmd); + } } // (5) THE CONTROL: the shader arm on the twin source, which TransitionWireImage moves to // SHADER_READ_ONLY_OPTIMAL first (both aspects of a packed format). diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/WireDepthResolveProbe.h b/MobileGL/MG_Backend/DirectVulkan/Renderer/WireDepthResolveProbe.h index 13c7cb91..9b8eab78 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/WireDepthResolveProbe.h +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/WireDepthResolveProbe.h @@ -86,6 +86,10 @@ namespace MobileGL::MG_Backend::DirectVulkan { VkResolveModeFlags depthResolveModes = 0; VkResolveModeFlags stencilResolveModes = 0; Bool shaderStencilExport = false; + // TEST ONLY (MGITEST_MAGMA_DEPTH_RESOLVE_PROBE=elide-subject, WireDepthResolveArm.h): record + // NO render-pass resolve, so every subject target keeps its sentinel - the Adreno 830's + // reading - while the shader control runs as always. Everything else is the real probe. + Bool elideSubject = false; }; // Per format the device can host as a 4x multisample depth/stencil attachment AND for which diff --git a/MobileGL/MG_IntegrationTest/CMakeLists.txt b/MobileGL/MG_IntegrationTest/CMakeLists.txt index 48450f76..eaeb2db3 100644 --- a/MobileGL/MG_IntegrationTest/CMakeLists.txt +++ b/MobileGL/MG_IntegrationTest/CMakeLists.txt @@ -3452,6 +3452,34 @@ if (MOBILEGL_BUILD_DISAGGREGATED) ENVIRONMENT "${mglItestMagmaWireEnvironment}\;MGITEST_MAGMA_DEPTH_RESOLVE_PROBE=bug\;MOBILEGL_LOG_FILE_PATH=${CMAKE_CURRENT_BINARY_DIR}/p7-${probeBugLog}${mglItestArmLogTag}.log") endforeach() endif() + # ---- P7 gate 5 (g5-msprobe critic): THE REAL PROBE'S NEGATIVE CONTROL ------------------ + # + # The Bug entries above hand the arm choice a CANNED measurement, and MsResolve0 only pins + # lavapipe's clean answer, so no entry could see the REAL probe (WireDepthResolveProbe.cpp - + # its recording, readback and tally) stop detecting a render pass that writes nothing. + # MGITEST_MAGMA_DEPTH_RESOLVE_PROBE=elide-subject runs the real probe with its render-pass + # resolve left unrecorded - the subject's target keeps the sentinel, the Adreno 830's + # reading - while the shader control runs as always. The entry asserts the case's pixels, + # that the real measurement reports verdict=render-pass-resolve-broken with a reading whose + # render pass kept the sentinel beside a resolving control, and that the shader pass + # resolved and the render pass never did + # (DepthStencilReadbackMatrixScenario::ExpectTheResolveArmWhereTheLaneAsks). + # + # Split+spawn only, the server-side-knob reason above, named in spawn_lane_parity.py's + # MAGMA_SERVER_ENV_KNOB_NO_TCP. + # + # Red once (executed, reverted): EvaluateWireDepthResolveProbe answering Clean for every + # measurement fails both entries; the probe's subject tally counting every texel as + # resolved fails both entries while the four Bug entries stay green. + if(mglItestServerEnvKnobArm) + gtest_discover_tests(MobileGLIntegrationTest + TEST_PREFIX "DirectVulkan.${mglItestArm}.MsResolveElide." + TEST_FILTER "DepthStencilReadbackMatrixScenario.AResolvedMultisampleDepthReadsBackFromTheDestination" + DISCOVERY_TIMEOUT 30 + PROPERTIES LABELS "${mglItestMagmaLabels}" TIMEOUT ${MGL_ITEST_TIMEOUT} + ${mglItestArmProperties} + ENVIRONMENT "${mglItestMagmaWireEnvironment}\;MGITEST_MAGMA_DEPTH_RESOLVE_PROBE=elide-subject\;MOBILEGL_LOG_FILE_PATH=${CMAKE_CURRENT_BINARY_DIR}/p7-msresolveelide${mglItestArmLogTag}.log") + endif() foreach(inPlaceCase CopyImageInPlaceAcrossLevelsAndLayers CopyImageInPlaceOverlapDeclinesAndLeavesTheLevelAlone) gtest_discover_tests(MobileGLIntegrationTest diff --git a/MobileGL/MG_IntegrationTest/Scenarios/DepthStencilReadbackMatrixScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/DepthStencilReadbackMatrixScenario.cpp index 817088a5..b5073002 100644 --- a/MobileGL/MG_IntegrationTest/Scenarios/DepthStencilReadbackMatrixScenario.cpp +++ b/MobileGL/MG_IntegrationTest/Scenarios/DepthStencilReadbackMatrixScenario.cpp @@ -244,6 +244,12 @@ namespace MGITest { // place of running the probe: the shader pass must resolve, the render pass never; // - MsResolve1. / MsFlip1. set MGITEST_MAGMA_FORCE_SHADER_DEPTH_RESOLVE=1, which drops // the render-pass arm altogether: same assertion; + // - MsResolveElide. sets MGITEST_MAGMA_DEPTH_RESOLVE_PROBE=elide-subject, which runs the + // REAL probe with its render-pass resolve left unrecorded: the measurement itself (not + // a canned one) must report the defect - a reading whose render pass kept the sentinel + // in every texel while the shader control resolved all of them - and the shader pass + // must then resolve, the render pass never. The Bug entries cannot see the real + // probe's recording, readback or tally stop detecting the defect; this one does; // - MsResolve0. on split and spawn sets MGITEST_EXPECT_DEPTH_RESOLVE_PROBE=clean - a // marker only this function reads - and asserts the REAL probe ran on the lane's // device (lavapipe), found the render pass clean, and that the render pass resolved. @@ -256,13 +262,16 @@ namespace MGITest { // // Red once (executed, reverted): EvaluateWireDepthResolveProbe answering Clean for every // measurement fails the four Bug entries here (the render-pass line is in the log and - // the verdict line says clean) while their pixels stay green. + // the verdict line says clean) while their pixels stay green. The two Elide entries go + // red under that too, and ALONE go red when the real probe's tally counts every subject + // texel as resolved (the canned Bug readings never pass through it). static void ExpectTheResolveArmWhereTheLaneAsks(const char* what) { const std::string probe = SplitLane::MarkerValue("MGITEST_MAGMA_DEPTH_RESOLVE_PROBE"); const bool forcedBug = probe == "bug"; + const bool elidedSubject = probe == "elide-subject"; const bool renderPassDropped = SplitLane::MarkerIsOne("MGITEST_MAGMA_FORCE_SHADER_DEPTH_RESOLVE"); const bool measuredClean = SplitLane::MarkerValue("MGITEST_EXPECT_DEPTH_RESOLVE_PROBE") == "clean"; - if (!forcedBug && !renderPassDropped && !measuredClean) return; + if (!forcedBug && !elidedSubject && !renderPassDropped && !measuredClean) return; if (PipeStatsWindow::ServerLibraryLogPath().empty()) { ADD_FAILURE() << what << ": the entry names the resolve arm (probe=" << probe << ", force-shader=" << renderPassDropped << ", expect-clean=" << measuredClean @@ -277,7 +286,7 @@ namespace MGITest { const bool renderPassResolved = server.find("ResolveWireDepthStencil: resolved by the VK_KHR_depth_stencil_resolve render pass") != std::string::npos; - if (forcedBug || renderPassDropped) { + if (forcedBug || elidedSubject || renderPassDropped) { EXPECT_TRUE(shaderResolved) << what << ": the server never resolved with the shader pass (probe=" << probe << ", force-shader=" << renderPassDropped << ")"; EXPECT_FALSE(renderPassResolved) @@ -288,6 +297,33 @@ namespace MGITest { EXPECT_NE(server.find("verdict=render-pass-resolve-broken"), std::string::npos) << what << ": the forced `bug` measurement did not evaluate to the defect verdict"; } + if (elidedSubject) { + // The source phrase is the REAL measurement's (a canned one says "forced by"), + // and ArmWireDepthResolveOrder states it once per process. + EXPECT_NE(server.find("depth/stencil resolve probe (measured on this device with its render-pass " + "resolve elided by MGITEST_MAGMA_DEPTH_RESOLVE_PROBE=elide-subject) " + "verdict=render-pass-resolve-broken"), + std::string::npos) + << what << ": the real probe, its render-pass resolve elided, did not report the defect"; + // DescribeWireDepthResolveFormat's reading, one line per probed format: some + // format's render pass matched no texel and kept the sentinel in all 16, and its + // shader control resolved all 16. + bool sentinelKeptBesideAResolvedControl = false; + for (std::string::size_type at = server.find("depth/stencil resolve probe "); + at != std::string::npos && !sentinelKeptBesideAResolvedControl; + at = server.find("depth/stencil resolve probe ", at + 1)) { + const std::string::size_type end = server.find('\n', at); + const std::string line = + server.substr(at, end == std::string::npos ? std::string::npos : end - at); + sentinelKeptBesideAResolvedControl = + line.find(" x4: ") != std::string::npos && + line.find("render pass 0/16 (first ") != std::string::npos && + line.find("sentinel 16), shader control 16/16") != std::string::npos; + } + EXPECT_TRUE(sentinelKeptBesideAResolvedControl) + << what << ": no real reading shows the elided render pass's sentinel kept beside a " + "shader control that resolved"; + } return; } EXPECT_NE(server.find("depth/stencil resolve probe (measured on this device) verdict=clean"), diff --git a/MobileGL/MG_Test/Pipeline/PipelineQuirkTest.cpp b/MobileGL/MG_Test/Pipeline/PipelineQuirkTest.cpp index 733bb337..b79fa22f 100644 --- a/MobileGL/MG_Test/Pipeline/PipelineQuirkTest.cpp +++ b/MobileGL/MG_Test/Pipeline/PipelineQuirkTest.cpp @@ -706,6 +706,20 @@ TEST(PipelineQuirkTest, WireDepthResolveProbeKnobReplacesTheMeasurementNotTheVer EXPECT_EQ(measured, 1) << "an unrecognised value runs the probe as if the knob were unset"; EXPECT_FALSE(other.measurement.fromKnob); EXPECT_EQ(other.verdict, WireDepthResolveProbeVerdict::Clean); + // `elide-subject` is the REAL probe's negative control: it decides nothing here - the device + // probe runs (the caller elides its render-pass resolve) and its reading is evaluated as-is. + EXPECT_EQ(ParseWireDepthResolveProbeKnob("elide-subject"), WireDepthResolveProbeKnob::ElideSubject); + const auto elided = ChooseWireDepthResolveArm(WireDepthResolveProbeKnob::ElideSubject, true, [&] { + ++measured; + return FakeMeasurement({FakeFormat("D24_UNORM_S8_UINT", kRenderPassWroteNothing, kRenderPassWroteNothing)}); + }); + EXPECT_EQ(measured, 2) << "elide-subject must run the device probe, not a canned reading"; + EXPECT_FALSE(elided.measurement.fromKnob); + EXPECT_EQ(elided.verdict, WireDepthResolveProbeVerdict::RenderPassResolveBroken); + EXPECT_TRUE(elided.preferShader); + const auto noArm = ChooseWireDepthResolveArm(WireDepthResolveProbeKnob::ElideSubject, false, measure); + EXPECT_EQ(measured, 2) << "without a render-pass arm there is nothing to elide or measure"; + EXPECT_EQ(noArm.verdict, WireDepthResolveProbeVerdict::NotRun); #else GTEST_SKIP() << "the wire resolve arms exist only in the disaggregated build"; #endif diff --git a/scripts/ci/spawn_lane_parity.py b/scripts/ci/spawn_lane_parity.py index ea5b6f1b..4a80209f 100644 --- a/scripts/ci/spawn_lane_parity.py +++ b/scripts/ci/spawn_lane_parity.py @@ -90,9 +90,11 @@ MAGMA_INPROC_ONLY = ("MagmaRunAheadScenario.", "MagmaWireCacheScenario.") # in place of running its resolve probe - the knob-free `.MsResolve0.` / `.MsFlip.` keep their tcp # entries (MsResolve0's split/spawn copies also carry a client-read expectation marker for the # real probe's verdict, which the tcp copy cannot: same name on all three arms, no exception). +# `.MsResolveElide.` (g5-msprobe critic) sets MGITEST_MAGMA_DEPTH_RESOLVE_PROBE=elide-subject, which +# the same server-side arm choice reads to run the REAL probe with its render-pass resolve left out. MAGMA_SERVER_ENV_KNOB_NO_TCP = (".ShaderMip1.", ".ShaderMip2.", ".DepthMip.", ".DefaultBlitShape1.", ".MsResolve1.", ".StaleSerial.", ".MsFlip1.", - ".MsResolveBug.", ".MsFlipBug.", + ".MsResolveBug.", ".MsFlipBug.", ".MsResolveElide.", ".Reclaim.MagmaWireReclaimScenario.RespecifiesWithNoDrawBetweenKeepTheLiveStoreCountBounded", ".Reclaim.MagmaWireReclaimScenario.RespecifyAndDrawEachStoreInOneFrameStaysWithinTheDeferredBudget", ".Reclaim.MagmaWireReclaimScenario.ManySmallRespecifyAndDrawRoundsStayUnderTheStoreCountCeiling",