From 0222852b940fe84dd6c4e4ef28edc40289d62bc5 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Wed, 30 Sep 2026 03:44:29 -0400 Subject: [PATCH] [Fix] (Espryt, P8-SE): a spawn / tcp server publishes the default framebuffer's depth/stencil format (it has no frontend placeholder; the case leaves pending-fix for the three arms); the server CPU mip arm declines by name (generate-mipmap-store-declined) instead of filtering a store the driver wrote over, and E's refused-readback knob reaches it (CpuMipRefused. entries); WireIndirectDrawScenario case 3 runs on the Espryt split arms; copy-image-shadow-mirror goes and unmigrated_emulation_sites.py holds PipeCatalogueTest's list to the call sites --- .github/workflows/test.yml | 7 + MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp | 105 ++++++--- MobileGL/MG_IntegrationTest/CMakeLists.txt | 38 +++ .../GenerateMipmapServerScenario.cpp | 109 ++++++++- .../Scenarios/WireIndirectDrawScenario.cpp | 13 +- MobileGL/MG_Test/Pipe/PipeCatalogueTest.cpp | 16 +- scripts/ci/unmigrated_emulation_sites.py | 217 ++++++++++++++++++ scripts/data/split_coverage_exemptions.txt | 2 +- 8 files changed, 459 insertions(+), 48 deletions(-) create mode 100755 scripts/ci/unmigrated_emulation_sites.py diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 1a281348..8f5a991a 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -1470,6 +1470,13 @@ jobs: python3 scripts/ci/split_coverage.py --self-test python3 scripts/ci/split_coverage.py build-split + # P8-SE: PipeCatalogueTest's unmigrated-emulation list equals the MGPipeUnmigratedEmulation + # call sites in the tree, one site per name. A source lint: no build needed. + - name: Unmigrated-emulation list against its call sites (P8-SE) + run: | + python3 scripts/ci/unmigrated_emulation_sites.py --self-test + python3 scripts/ci/unmigrated_emulation_sites.py + - name: Unit tests on the split runtime working-directory: build-split env: diff --git a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp index e99c2aa8..0e93d4b3 100644 --- a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp +++ b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp @@ -13020,16 +13020,30 @@ namespace MobileGL::MG_Backend::DirectGLES { return s_forced; } + // WireTextureReadback.inc (included further down) defines it: E's extent-scoped stand-in for a + // driver that refuses to read a colour level back, asked here too so a host reaches the store + // route below (P8-SE). + static Bool WireTextureReadbackRefusedForTesting(const IntVec3& extent); + // The base level as tight RGB floats: the driver's texels first, the staged store second. + // + // P8-SE: THE STORE ANSWERS ONLY FOR A TEXTURE THE DRIVER NEVER WROTE (P8-E's sticky mark). A + // render target's level 0 lives on the driver; the store still holds whatever crossed before + // the draw, and filtering it would hand the chain stale texels. `driverWrittenBefore` is the + // mark as it stood BEFORE this generation noted its own write (GenerateMipmap's split arm), + // because that note is unconditional and would otherwise decline every store read. Anything + // the store cannot stand for is a named decline, `declined` says why. static Bool ReadThreeChannelFloatLevelOnServer(MG_Pipe::MGPipeHandle mipRes, const MG_Pipe::MGPipeResourceRecord& record, const SharedPtr& backendTexture, TextureTarget textureTarget, TextureUploadTarget face, Uint32 level, - const IntVec3& extent, Vector& rgb) { + const IntVec3& extent, Bool driverWrittenBefore, + Vector& rgb, const char*& declined) { const auto format = static_cast(record.Desc.InternalFormat); const SizeT texels = static_cast(extent.x()) * static_cast(extent.y()); Vector bytes; - if (ReadTextureLevelTight(backendTexture->GetBackendTextureId(), textureTarget, face, format, + if (!WireTextureReadbackRefusedForTesting(extent) && + ReadTextureLevelTight(backendTexture->GetBackendTextureId(), textureTarget, face, format, static_cast(level), extent, backendTexture->RequiresImageBindableStorage(), GL_RGB, GL_FLOAT, bytes) && bytes.size() == texels * 3 * sizeof(Float)) { @@ -13037,17 +13051,26 @@ namespace MobileGL::MG_Backend::DirectGLES { Memcpy(rgb.data(), bytes.data(), bytes.size()); return true; } - // A view's staged bytes live under its storage owner's key, in the owner's levels; a view - // whose level the driver cannot read has no answer here. - if (!MG_Pipe::MGPipeHandleIsNull(record.Desc.ViewOf)) return false; auto& store = MG_Remote::Server::ServerStagedTexture(); const Uint64 key = MG_Remote::Server::StagedTextureStore::KeyForHandle(mipRes); const auto faceCode = static_cast(face); const auto levelCode = static_cast(level); - if (!store.CopiesIntoServerStorage() || !store.IsCovered(key, faceCode, levelCode)) return false; const Bool half = format == TextureInternalFormat::RGB16F; const SizeT componentBytes = half ? sizeof(Uint16) : sizeof(Float); - if (store.LevelByteSize(key, faceCode, levelCode) != texels * 3 * componentBytes) return false; + // A view's staged bytes live under its storage owner's key, in the owner's levels; a view + // whose level the driver cannot read has no answer here. + if (!MG_Pipe::MGPipeHandleIsNull(record.Desc.ViewOf)) { + declined = "a view has no staged level of its own"; + } else if (!store.CopiesIntoServerStorage()) { + declined = "this arm keeps no staged store"; + } else if (driverWrittenBefore) { + declined = "the driver has written the texture since its bytes crossed"; + } else if (!store.IsCovered(key, faceCode, levelCode)) { + declined = "not every byte of the level crossed"; + } else if (store.LevelByteSize(key, faceCode, levelCode) != texels * 3 * componentBytes) { + declined = "the staged level is not the size of the level"; + } + if (declined != nullptr) return false; const Uint8* staged = store.RequireLevelBytes(key, faceCode, levelCode, "generate-mipmap-server-cpu"); rgb.resize(texels * 3); for (SizeT i = 0; i < texels * 3; ++i) { @@ -13068,7 +13091,7 @@ namespace MobileGL::MG_Backend::DirectGLES { static void GenerateThreeChannelFloatMipmapOnServer(MG_Pipe::MGPipeHandle mipRes, const MG_Pipe::MGPipeResourceRecord& record, const SharedPtr& backendTexture, - const char* why) { + Bool driverWrittenBefore, const char* why) { const auto& desc = record.Desc; const auto format = static_cast(desc.InternalFormat); const auto textureTarget = TextureTargetForPipeResourceTarget(desc.Target); @@ -13103,12 +13126,14 @@ namespace MobileGL::MG_Backend::DirectGLES { IntVec3 extent = MG_Remote::Server::StagedTextureMipExtent(desc.Target, desc.Width, desc.Height, desc.Depth, window.Base); Vector level; + const char* declined = nullptr; if (!ReadThreeChannelFloatLevelOnServer(mipRes, record, backendTexture, textureTarget, face, window.Base, - extent, level)) { - MGLOG_E_ONCE("MGPipe: generate-mipmap-server-cpu declines texture {%u, %u}: base level %u is " - "neither readable from the driver nor covered by the staged store; levels %u..%u are " - "left as they are", - mipRes.Slot, mipRes.Gen, window.Base, window.Base + 1, window.End - 1); + extent, driverWrittenBefore, level, declined)) { + MGLOG_E_ONCE("MGPipe: generate-mipmap-server-cpu declines texture {%u, %u}: base level %u could not " + "be read back from the driver and the staged store cannot stand for it (%s); levels " + "%u..%u are left as they are - DECLINED (generate-mipmap-store-declined)", + mipRes.Slot, mipRes.Gen, window.Base, declined != nullptr ? declined : "?", + window.Base + 1, window.End - 1); return; } for (Uint32 dst = window.Base + 1; dst < window.End; ++dst) { @@ -13178,9 +13203,13 @@ namespace MobileGL::MG_Backend::DirectGLES { // client's level shadows. P8-B2 retired it (GenerateThreeChannelFloatMipmapOnServer reads the // server's own level), so no arm here reads client memory any more; the WaitClass flip itself // is a protocol change and is left to the integrator. + // + // `driverWrittenBefore`: P8-E's driver-written mark on the texture's staged store as it stood + // before this call noted its own write - the CPU arm's store route reads it (P8-SE). static void GenerateMipmapByRecord(GLenum target, MG_Pipe::MGPipeHandle mipRes, const MG_Pipe::MGPipeResourceRecord& record, - const SharedPtr& backendTexture) { + const SharedPtr& backendTexture, + Bool driverWrittenBefore) { const auto format = static_cast(record.Desc.InternalFormat); const auto textureTarget = TextureTargetForPipeResourceTarget(record.Desc.Target); const Bool threeChannelFloat = @@ -13199,7 +13228,7 @@ namespace MobileGL::MG_Backend::DirectGLES { return; } if (threeChannelFloat && ForcedServerCpuMipmap()) { - GenerateThreeChannelFloatMipmapOnServer(mipRes, record, backendTexture, + GenerateThreeChannelFloatMipmapOnServer(mipRes, record, backendTexture, driverWrittenBefore, "forced by MGITEST_ESPRYT_FORCE_CPU_MIPMAP"); return; } @@ -13229,7 +13258,8 @@ namespace MobileGL::MG_Backend::DirectGLES { refused = g_GLESFuncs.glGetError(); } if (refused == GL_NO_ERROR) return; - GenerateThreeChannelFloatMipmapOnServer(mipRes, record, backendTexture, "the driver refused glGenerateMipmap"); + GenerateThreeChannelFloatMipmapOnServer(mipRes, record, backendTexture, driverWrittenBefore, + "the driver refused glGenerateMipmap"); } #endif @@ -13256,6 +13286,10 @@ namespace MobileGL::MG_Backend::DirectGLES { // for GenerateMipmap move to kWaitNone. if (MG_Config::Transport != MG_Config::TransportMode::Monolith) { const auto mipRes = MG_Pipe::MGPipeApplier().VerbMipRes; + // P8-SE: whether the staged store still stands for the texture is read BEFORE the note + // below sets the mark, or the CPU arm's store route could never answer at all. + const Bool driverWrittenBefore = MG_Remote::Server::ServerStagedTexture().IsDriverWritten( + MG_Remote::Server::StagedTextureStore::KeyForHandle(mipRes)); // Every arm GenerateMipmapByRecord takes writes the levels above the base on the // driver only (P8-B2's CPU arm uploads its levels there too), so the note is // unconditional. @@ -13268,7 +13302,7 @@ namespace MobileGL::MG_Backend::DirectGLES { mipRes.Slot, mipRes.Gen); return; } - GenerateMipmapByRecord(target, mipRes, *record, backendTexture); + GenerateMipmapByRecord(target, mipRes, *record, backendTexture, driverWrittenBefore); return; } #endif @@ -13545,14 +13579,10 @@ namespace MobileGL::MG_Backend::DirectGLES { // ordinary correct path and must still run. if (MG_Config::Transport != MG_Config::TransportMode::Monolith) return; #endif -#if MOBILEGL_PIPE_PUSH - // P4a (D-M). glCopyImageSubData's CPU-shadow mirror copies the source level's shadow - // rows into the DESTINATION's shadow so a later readback of the destination sees what - // the GPU copy put there. Both shadows are the client's, so this is the clearest case - // in the family of an emulation that cannot survive a split; ROADMAP puts the move - // itself in P8 and this names the site until then. - MG_Pipe::MGPipeUnmigratedEmulation("copy-image-shadow-mirror"); -#endif + // P4a (D-M) named this site `copy-image-shadow-mirror` with MGPipeUnmigratedEmulation. + // P8-SE removed the call: the return above means no transport reaches it, and on a + // monolith arm the call was a no-op (PipeCatalogueTest's list and the lint + // scripts/ci/unmigrated_emulation_sites.py follow). const auto srcUploadTarget = srcEndpoint.Texture->GetUploadTargets()[0]; const auto dstUploadTarget = dstEndpoint.Texture->GetUploadTargets()[0]; @@ -15293,10 +15323,10 @@ namespace MobileGL::MG_Backend::DirectGLES { // Session::Fail entirely, which is the funnel the census gate watches. // // THE FUNNEL KEEPS ITS TEETH FOR THE OTHER SITES: MGPipeUnmigratedEmulation still - // names generate-mipmap-storage, generate-mipmap-cpu-fallback and - // copy-image-shadow-mirror, on monolith arms only. Only this call site is retired here, - // and only because its own premise did not hold (texture-remint-pull went the same way - // in P9 W2, and P8-B the two split generate-mipmap names, for the same reason). + // names generate-mipmap-storage and generate-mipmap-cpu-fallback, on monolith arms only + // (P8-SE dropped copy-image-shadow-mirror, a site no transport reached). Only this call + // site is retired here, and only because its own premise did not hold (texture-remint-pull + // went the same way in P9 W2, and P8-B the two split generate-mipmap names, likewise). if (MG_Config::Transport != MG_Config::TransportMode::Monolith) { if (shadow == nullptr) { MGLOG_E_ONCE("GetTexImage: no level shadow on this side for target=0x%x level=%d, and under an " @@ -16198,7 +16228,18 @@ namespace MobileGL::MG_Backend::DirectGLES { // resizing depth/stencil without colour would report the default framebuffer incomplete. static void PublishDefaultFramebufferDepthStencilFormat() { auto& defaultFBOInfo = MG_Impl::GLImpl::FramebufferImpl::pDefaultFramebufferInfo; +#if MOBILEGL_PIPE_PUSH + // P8-SE: THE PLACEHOLDER IS ONLY THE MONOLITH ARM'S TARGET. With a transport the answer is + // posted (OnSurfaceChanged, below) and the client writes its own placeholder; a spawn or tcp + // server process has no frontend at all, so pDefaultFramebufferInfo is null there. Gating on + // it returned before the post, the client kept Init.cpp's GL_DEPTH32F_STENCIL8 guess for a + // D24S8 pbuffer, and a blit into a buffer allocated from that guess lost depth and stencil + // (notes/p8/SE.md). Inproc only passed because client and server share the process. + if (!g_GLESFuncs.glGetFramebufferAttachmentParameteriv) return; + if (!defaultFBOInfo && MG_Pipe::gMGPipeCallbacks.OnSurfaceChanged == nullptr) return; +#else if (!defaultFBOInfo || !g_GLESFuncs.glGetFramebufferAttachmentParameteriv) return; +#endif // GL_DEPTH / GL_STENCIL are the default framebuffer's spellings; a user framebuffer // would need GL_DEPTH_ATTACHMENT / GL_STENCIL_ATTACHMENT and answers GL_INVALID_ENUM @@ -16279,8 +16320,14 @@ namespace MobileGL::MG_Backend::DirectGLES { stencilFormat = TextureInternalFormat::StencilIndex8; } +#if MOBILEGL_PIPE_PUSH + // Null in a spawn / tcp server (above); only the monolith arm below writes through them. + auto* depthTexture = defaultFBOInfo ? defaultFBOInfo->depthAttachment.get() : nullptr; + auto* stencilTexture = defaultFBOInfo ? defaultFBOInfo->stencilAttachment.get() : nullptr; +#else auto* depthTexture = defaultFBOInfo->depthAttachment.get(); auto* stencilTexture = defaultFBOInfo->stencilAttachment.get(); +#endif #if MOBILEGL_PIPE_PUSH if (MG_Pipe::gMGPipeCallbacks.OnSurfaceChanged != nullptr) { // P5c ev (CONTRACT-P5C ยง4.2): with an active transport the attachments are CLIENT diff --git a/MobileGL/MG_IntegrationTest/CMakeLists.txt b/MobileGL/MG_IntegrationTest/CMakeLists.txt index 830e57f3..a61e6cb7 100644 --- a/MobileGL/MG_IntegrationTest/CMakeLists.txt +++ b/MobileGL/MG_IntegrationTest/CMakeLists.txt @@ -4863,6 +4863,44 @@ if (MOBILEGL_BUILD_DISAGGREGATED) endforeach() endif() +# P8-SE (docs/Disaggregated/notes/p8/SE.md): Espryt split residuals. +if (MOBILEGL_BUILD_DISAGGREGATED) + # The default framebuffer's depth/stencil format reaches a spawn / tcp client now (the server + # process has no frontend placeholder, and the publish used to return on it before posting), + # so the case A left as `pending-fix` joins the three Espryt arms. + mgl_itest_register_split_arms( + "DepthStencilReadbackAttachmentShapeScenario.DefaultFramebufferDepthStencilFormatIsBlitCompatible" "") + # B2's CPU mip arm x P8-E's driver-written mark: the forced-CPU knob plus E's refused-readback + # knob at the base level's extent (16x16), both read by the SERVER - so inproc and spawn only, + # and their own label as B's `CpuMip.` entries have (the tcp arm's server is the shared + # TcpServer.Start fixture, and the refusal there would reach every 16x16 colour readback). + foreach(mglItestArm IN ITEMS Split Spawn) + string(TOUPPER "${mglItestArm}" mglItestArmUpper) + string(TOLOWER "${mglItestArm}" mglItestArmLower) + foreach(cpuMipCase Rgb16fDrawnBaseWithARefusedReadbackDeclinesByName + Rgb16fUploadedBaseWithARefusedReadbackIsFilteredFromTheStore) + gtest_discover_tests(MobileGLIntegrationTest + TEST_PREFIX "DirectGLES.${mglItestArm}.CpuMipRefused." + TEST_FILTER "GenerateMipmapServerScenario.${cpuMipCase}" + DISCOVERY_TIMEOUT 30 + PROPERTIES + LABELS "integration-gpu\\;integration-cpu-mip-${mglItestArmLower}" + TIMEOUT ${MGL_ITEST_TIMEOUT} + ENVIRONMENT "${MGL_ITEST_GLES_${mglItestArmUpper}_ENVIRONMENT}\;MGITEST_ESPRYT_FORCE_CPU_MIPMAP=1\;MGITEST_ESPRYT_REFUSE_TEXTURE_READBACK_EXTENT=16x16\;MOBILEGL_LOG_FILE_PATH=${CMAKE_CURRENT_BINARY_DIR}/p8se-cpu-mip-refused-${mglItestArmLower}-${cpuMipCase}.log") + endforeach() + endforeach() + # PipeCatalogueTest's unmigrated-emulation list against the call sites in the tree (a source + # lint, so the unit lane runs it wherever the split build's unit tests run; CI also runs it + # beside split_coverage.py). + add_test(NAME PipeCatalogue.UnmigratedEmulationSitesSelfTest + COMMAND "${Python3_EXECUTABLE}" "${CMAKE_SOURCE_DIR}/scripts/ci/unmigrated_emulation_sites.py" --self-test) + add_test(NAME PipeCatalogue.UnmigratedEmulationSitesMatchTheList + COMMAND "${Python3_EXECUTABLE}" "${CMAKE_SOURCE_DIR}/scripts/ci/unmigrated_emulation_sites.py" + "${CMAKE_SOURCE_DIR}") + set_tests_properties(PipeCatalogue.UnmigratedEmulationSitesSelfTest + PipeCatalogue.UnmigratedEmulationSitesMatchTheList PROPERTIES LABELS unit TIMEOUT 60) +endif() + # ===================================================================================== # P7 PACKAGE L: THE INFORMATIONAL MAGMA CENSUS - TIER 2 OF TWO # ===================================================================================== diff --git a/MobileGL/MG_IntegrationTest/Scenarios/GenerateMipmapServerScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/GenerateMipmapServerScenario.cpp index dadb194b..be3b29bc 100644 --- a/MobileGL/MG_IntegrationTest/Scenarios/GenerateMipmapServerScenario.cpp +++ b/MobileGL/MG_IntegrationTest/Scenarios/GenerateMipmapServerScenario.cpp @@ -29,6 +29,14 @@ // Espryt's server-side CPU filter, which a host driver never needs: llvmpipe generates both // formats natively. Under the knob the server log must name that arm, or the entry proved nothing. // +// THE CPU ARM'S STORE ROUTE (P8-SE). When the driver cannot read the base level back, the server's +// CPU arm filters the staged store's copy of it - but only for a texture the driver never wrote +// (P8-E's sticky mark), because a render target's level 0 exists only on the driver and the store +// still holds what crossed before the draw. The `CpuMipRefused.` entries add +// MGITEST_ESPRYT_REFUSE_TEXTURE_READBACK_EXTENT=16x16 (P8-E's knob, also read by the SERVER) to the +// forced-CPU knob: a render-target base must decline by name (generate-mipmap-store-declined) and +// never filter the stale store, and an uploaded-only base must still be filtered from the store. +// // MONOLITH DEFECTS, recorded for dev (ID-P8-3) and skipped by name on the monolith arm they live // on unless MGITEST_RUN_MONOLITH_MIP_DEFECTS=1 is set (which is how each was shown red): // M1 Espryt: the RGB16F / RGB32F CPU filter reads the frontend's level shadow, which a GPU @@ -68,6 +76,10 @@ namespace MGITest { constexpr const char* kForceCpuMipmap = "MGITEST_ESPRYT_FORCE_CPU_MIPMAP"; constexpr const char* kCpuMipmapMarker = "generate-mipmap-server-cpu"; constexpr const char* kRunMonolithDefects = "MGITEST_RUN_MONOLITH_MIP_DEFECTS"; + // P8-SE: E's server-read knob; the entries set it to kEdge x kEdge, the base level's extent. + constexpr const char* kRefuseReadback = "MGITEST_ESPRYT_REFUSE_TEXTURE_READBACK_EXTENT"; + constexpr const char* kStoreDeclinedMarker = "generate-mipmap-store-declined"; + constexpr const char* kStoreFilteredMarker = "filtering the staged store's copy"; constexpr const char* kVertexSource = R"(#version 430 core void main() { @@ -123,6 +135,7 @@ void main() { o_color = gl_FragCoord.y < u_split ? u_bottom : u_top; } m_wire = SplitRuntimeSkipReason().empty(); m_espryt = Gl().BackendName() == "DirectGLES"; m_forcedCpu = std::getenv(kForceCpuMipmap) != nullptr; + m_refusedReadback = std::getenv(kRefuseReadback) != nullptr; std::string error; m_sampleProgram = CompileProgram(kVertexSource, kSampleSource, &error); ASSERT_NE(m_sampleProgram, 0u) << error; @@ -368,6 +381,84 @@ void main() { o_color = gl_FragCoord.y < u_split ? u_bottom : u_top; } // upload, so the chain must come from the GPU's level 0 and never from the uploaded texels. void RenderTargetBase(const Format& format) { if (MonolithDefect(true, false)) GTEST_SKIP() << kM1; + PaintSentinelBaseAsRenderTarget(format); + if (HasFatalFailure() || IsSkipped()) return; + Generate(); + const int last = 4; + for (int level = 0; level < last; ++level) + ExpectHalves(SampleLevel(level, last), format, kBottom, kTop, 0, 0, + std::string(format.name) + " render-target base, level " + std::to_string(level)); + ExpectCpuArmNamed(); + EXPECT_EQ(FirstGLError(), 0u); + } + + // ---- P8-SE: the CPU arm's store route under a refused readback ---------------------- + + // Only the `CpuMipRefused.` entries carry both server knobs; anywhere else the store route + // is unreachable on a host (its driver reads every level back) and the case says so. + void SkipUnlessTheStoreRouteIsArmed() { + if (m_wire && m_espryt && m_forcedCpu && m_refusedReadback) return; + GTEST_SKIP() << "the store route needs Espryt's split server with " << kForceCpuMipmap << " and " + << kRefuseReadback << "=" << kEdge << "x" << kEdge << " (the CpuMipRefused. entries)"; + } + + // The render-target base again, with the driver refusing to read level 0 back: the store + // holds the uploaded sentinel, the driver the drawn halves, and P8-E's mark says so. The + // arm must decline by name - a stale chain of sentinel texels is the defect. + void RefusedReadbackOfADrawnBase(const Format& format) { + SkipUnlessTheStoreRouteIsArmed(); + if (IsSkipped()) return; + PaintSentinelBaseAsRenderTarget(format); + if (HasFatalFailure() || IsSkipped()) return; + Generate(); + const std::string log = PipeStatsWindow::ReadServerLogSince(m_mark); + EXPECT_NE(log.find(kStoreDeclinedMarker), std::string::npos) + << "the base level was drawn and its readback refused, but the server log after glGenerateMipmap " + "has no '" << kStoreDeclinedMarker << "' line"; + EXPECT_EQ(log.find(kStoreFilteredMarker), std::string::npos) + << "the CPU arm filtered the staged store's copy of a level the driver has since drawn over"; + const int last = 4; + ExpectHalves(SampleLevel(0, last), format, kBottom, kTop, 0, 0, + std::string(format.name) + " drawn base (untouched by the decline)"); + for (int level = 1; level < last; ++level) { + const Image image = SampleLevel(level, last); + const Rgba8 got = image.At(0, 0); + EXPECT_FALSE(std::abs(got.r - ToByte(kSentinel[0])) <= 2 && std::abs(got.g - ToByte(kSentinel[1])) <= 2 && + std::abs(got.b - ToByte(kSentinel[2])) <= 2) + << format.name << " level " << level << " is the uploaded sentinel " << got + << ": the chain was filtered from the stale staged store"; + } + ExpectCpuArmNamed(); + FirstGLError(); + } + + // The control: a base that only ever crossed as an upload IS what the store holds, so a + // refused readback is answered from the store and the chain is right. This is what tells + // the check above from "the store route always declines" - the generation's own + // driver-write note must not count against the level it reads. + void RefusedReadbackOfAnUploadedBase(const Format& format) { + SkipUnlessTheStoreRouteIsArmed(); + if (IsSkipped()) return; + NewTexture(); + DefineLevel(format, 0, kEdge, Halves(format, kEdge, kBottom, kTop, 0, 0)); + ASSERT_EQ(FirstGLError(), 0u) << "define level 0"; + Generate(); + const std::string log = PipeStatsWindow::ReadServerLogSince(m_mark); + EXPECT_NE(log.find(kStoreFilteredMarker), std::string::npos) + << "the uploaded base's readback was refused, but the server log has no '" << kStoreFilteredMarker + << "' line: the store route did not answer"; + EXPECT_EQ(log.find(kStoreDeclinedMarker), std::string::npos) + << "the store route declined a level that only ever crossed as an upload"; + const int last = 4; + for (int level = 1; level < last; ++level) + ExpectHalves(SampleLevel(level, last), format, kBottom, kTop, 0, 0, + std::string(format.name) + " uploaded base, store-filtered level " + + std::to_string(level)); + EXPECT_EQ(FirstGLError(), 0u); + } + + // Level 0 uploaded as the sentinel, then drawn over as a render target (bottom / top halves). + void PaintSentinelBaseAsRenderTarget(const Format& format) { NewTexture(); DefineLevel(format, 0, kEdge, Flat(format, kEdge, kSentinel, 0)); glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST); @@ -390,18 +481,12 @@ void main() { o_color = gl_FragCoord.y < u_split ? u_bottom : u_top; } glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, 0, 0); glBindFramebuffer(GL_FRAMEBUFFER, 0); ASSERT_EQ(FirstGLError(), 0u) << "draw into level 0"; - Generate(); - const int last = 4; - for (int level = 0; level < last; ++level) - ExpectHalves(SampleLevel(level, last), format, kBottom, kTop, 0, 0, - std::string(format.name) + " render-target base, level " + std::to_string(level)); - ExpectCpuArmNamed(); - EXPECT_EQ(FirstGLError(), 0u); } bool m_wire = false; bool m_espryt = false; bool m_forcedCpu = false; + bool m_refusedReadback = false; GLuint m_sampleProgram = 0; GLuint m_paintProgram = 0; GLuint m_vao = 0; @@ -457,4 +542,14 @@ void main() { o_color = gl_FragCoord.y < u_split ? u_bottom : u_top; } RenderTargetBase(kRgb32F); } + // P8-SE: the CPU arm's store route (header; the CpuMipRefused. entries). + TEST_F(GenerateMipmapServerScenario, Rgb16fDrawnBaseWithARefusedReadbackDeclinesByName) { + if (!Ready()) return; + RefusedReadbackOfADrawnBase(kRgb16F); + } + TEST_F(GenerateMipmapServerScenario, Rgb16fUploadedBaseWithARefusedReadbackIsFilteredFromTheStore) { + if (!Ready()) return; + RefusedReadbackOfAnUploadedBase(kRgb16F); + } + } // namespace MGITest diff --git a/MobileGL/MG_IntegrationTest/Scenarios/WireIndirectDrawScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/WireIndirectDrawScenario.cpp index 87a38fc3..abd86e4b 100644 --- a/MobileGL/MG_IntegrationTest/Scenarios/WireIndirectDrawScenario.cpp +++ b/MobileGL/MG_IntegrationTest/Scenarios/WireIndirectDrawScenario.cpp @@ -296,13 +296,16 @@ void main() { words[gl_GlobalInvocationID.x] = source[gl_GlobalInvocationID.x]; EXPECT_EQ(delta.barriers, 2u) << "one INDIRECT_COMMAND_READ barrier per freshly written store"; } - // The COUNT word a compute shader wrote. Magma only: Espryt reads that word from a - // server shadow GPU writes do not update, on both arms (OPEN-QUESTIONS 15, P8-C's - // package), so its answer here is a known divergence rather than this package's claim. + // The COUNT word a compute shader wrote. Espryt reads that word on the CPU from the + // server's staged copy of the parameter buffer; P8-C refreshes that copy after a shader + // write (SplitHostBytesForCpuRead), so its split arms answer here too (P8-SE). Espryt's + // MONOLITH arm still reads the frontend shadow without a sync (OPEN-QUESTIONS 15, C.md's + // monolith defects), left to P13 (ID-P8-13). TEST_F(WireIndirectDrawScenario, ComputeWrittenCountWordIsTheOneTheGpuReads) { if (!Ready() || IsSkipped()) return; - if (!m_magma) { - GTEST_SKIP() << "Espryt reads the count word from its shadow on both arms (OQ15, P8-C)"; + if (!m_magma && m_transport == "monolith") { + GTEST_SKIP() << "Espryt's monolith arm reads the count word from the frontend shadow without " + "a sync (OQ15; recorded for P13, ID-P8-13)"; } Fill(m_indirect, GL_DRAW_INDIRECT_BUFFER, std::vector{Elements(0), Elements(3), Elements(2)}); Fill(m_parameter, GL_PARAMETER_BUFFER, std::vector{3}); diff --git a/MobileGL/MG_Test/Pipe/PipeCatalogueTest.cpp b/MobileGL/MG_Test/Pipe/PipeCatalogueTest.cpp index bd63dc3e..d0258743 100644 --- a/MobileGL/MG_Test/Pipe/PipeCatalogueTest.cpp +++ b/MobileGL/MG_Test/Pipe/PipeCatalogueTest.cpp @@ -1423,7 +1423,9 @@ TEST(PipeCatalogue, ResourceRespecifyAcksOnlyImmutableStorage) { // gate rather than a surprise three phases later. // // The names are pinned here rather than counted in the backend, because the count alone cannot -// say WHICH one was lost. The purity gate greps the count; this says what the count is of. +// say WHICH one was lost. P8-SE: scripts/ci/unmigrated_emulation_sites.py holds this list against +// the `MGPipeUnmigratedEmulation("...")` call sites in the tree - equal sets, one site per name - +// because nothing did before and the list drifted twice unseen (P8-B, and P8-SE's dead name). TEST(PipeCatalogue, EveryUnmigratedEmulationIsNamedOnce) { // Every one of these is an emulation that reads or writes CLIENT memory a split server // would not have: a CPU shadow mirror, a CPU mipmap fallback and a shadow-conversion @@ -1440,16 +1442,18 @@ TEST(PipeCatalogue, EveryUnmigratedEmulationIsNamedOnce) { // had: "generate-mipmap-cpu-filter", the split arm's RGB16F/RGB32F refusal in // GenerateMipmapByRecord. P8-B2 retired it (the server filters its own level, // GenerateThreeChannelFloatMipmapOnServer), and P8-B1 retired the split arm's two - // "generate-mipmap-storage" sites (the record's plan is the window). What is left is these - // four names, one call site each, and every one of them is on a MONOLITH arm - the split - // arms no longer reach an unmigrated emulation (notes/p8/B.md). + // "generate-mipmap-storage" sites (the record's plan is the window). Every name left is on a + // MONOLITH arm - the split arms no longer reach an unmigrated emulation (notes/p8/B.md). + // + // THREE, NOT FOUR (P8-SE). "copy-image-shadow-mirror" sat after the mirror's own + // `Transport != Monolith` return, so no transport ever reached it and on monolith it was a + // no-op; the call and the name went together (notes/p8/SE.md). const char* const kNames[] = { - "copy-image-shadow-mirror", // the glCopyImageSubData CPU-shadow mirror "generate-mipmap-storage", // EnsureGenerateMipmapStorageAllocated (monolith grow) "generate-mipmap-cpu-fallback", // GenerateThreeChannelFloatMipmapOnCpu "get-tex-image-shadow", // GetTexImageViaShadowConversion }; - EXPECT_EQ(std::size(kNames), 4u); + EXPECT_EQ(std::size(kNames), 3u); // No duplicates: two sites sharing a name would make the grepped count and this list // disagree in the one direction nobody would notice. for (SizeT i = 0; i < std::size(kNames); ++i) { diff --git a/scripts/ci/unmigrated_emulation_sites.py b/scripts/ci/unmigrated_emulation_sites.py new file mode 100755 index 00000000..b3d554a7 --- /dev/null +++ b/scripts/ci/unmigrated_emulation_sites.py @@ -0,0 +1,217 @@ +#!/usr/bin/env python3 +"""P8-SE: PipeCatalogueTest's unmigrated-emulation list IS the set of call sites in the tree. + +THE QUESTION. `MGPipeUnmigratedEmulation("")` (MG_Pipe/PipeApply.h) names one emulation +that reaches into client memory; a split server aborts on it, a monolith arm runs past it. The +test `PipeCatalogue.EveryUnmigratedEmulationIsNamedOnce` (MG_Test/Pipe/PipeCatalogueTest.cpp) +pins the LIST of names, and nothing compared that list with the tree: it drifted twice without a +red (P8-B found a name the list never had; P8-SE a name whose only call site no transport could +reach). This asserts, every one an error: + + * every name in the test's `kNames[]` has EXACTLY ONE call site in the product sources; + * every call site's name is in `kNames[]`; + * `kNames[]` has no duplicate; + * every call is spelled with a string literal (a computed name cannot be checked). + +PRODUCT SOURCES are MobileGL/ minus MG_Test/ and MG_IntegrationTest/ (tests call the function +with literals of their own), in C/C++ files (.c .cc .cpp .h .hpp .inc .mm). Comments are masked +before matching, string literals are kept, so a name quoted in a comment is not a call site. + +Usage: unmigrated_emulation_sites.py [repo-root] (default: this script's repository) + unmigrated_emulation_sites.py --self-test +""" +import argparse +from pathlib import Path +import re +import sys +import tempfile + +LIST_FILE = Path("MobileGL/MG_Test/Pipe/PipeCatalogueTest.cpp") +TEST_NAME = "EveryUnmigratedEmulationIsNamedOnce" +SOURCE_ROOT = Path("MobileGL") +EXCLUDED = (Path("MobileGL/MG_Test"), Path("MobileGL/MG_IntegrationTest")) +SUFFIXES = {".c", ".cc", ".cpp", ".h", ".hpp", ".inc", ".mm"} +CALL = re.compile(r"\bMGPipeUnmigratedEmulation\s*\(") +LITERAL_CALL = re.compile(r'\bMGPipeUnmigratedEmulation\s*\(\s*"((?:[^"\\]|\\.)*)"\s*\)') +# The declaration and the definition: the return type (and any qualifier) right before the name +# on its line. +DECLARATION = re.compile(r"\bvoid\s+(?:[A-Za-z_]\w*::)*$") + + +def mask_comments(text): + """`text` with every comment character replaced by a space (newlines kept, so offsets and line + numbers survive); string and character literals are left as they are.""" + out = list(text) + i, n = 0, len(text) + while i < n: + c = text[i] + if c == "/" and i + 1 < n and text[i + 1] == "/": + while i < n and text[i] != "\n": + out[i] = " " + i += 1 + elif c == "/" and i + 1 < n and text[i + 1] == "*": + end = text.find("*/", i + 2) + end = n if end < 0 else end + 2 + for k in range(i, end): + if text[k] != "\n": + out[k] = " " + i = end + elif c in "\"'": + # R"(...)" raw strings: skip to the matching delimiter. + if c == '"' and i > 0 and text[i - 1] == "R": + close = text.find("(", i) + delim = text[i + 1:close] + end = text.find(")" + delim + '"', close) + i = n if end < 0 else end + len(delim) + 2 + continue + i += 1 + while i < n and text[i] != c: + i += 2 if text[i] == "\\" else 1 + i += 1 + else: + i += 1 + return "".join(out) + + +def list_names(root, errors): + path = root / LIST_FILE + try: + text = mask_comments(path.read_text(encoding="utf-8")) + except OSError as exc: + errors.append(f"{LIST_FILE}: cannot read ({exc})") + return [] + test = text.find(TEST_NAME) + block = re.search(r"kNames\s*\[\s*\]\s*=\s*\{(.*?)\};", text[test:], re.S) if test >= 0 else None + if block is None: + errors.append(f"{LIST_FILE}: no `kNames[] = {{ ... }};` in {TEST_NAME}") + return [] + return re.findall(r'"((?:[^"\\]|\\.)*)"', block.group(1)) + + +def call_sites(root, errors): + """{name: [file:line, ...]} over the product sources.""" + sites = {} + base = root / SOURCE_ROOT + for path in sorted(base.rglob("*")): + if path.suffix not in SUFFIXES or not path.is_file(): + continue + rel = path.relative_to(root) + if any(rel.is_relative_to(ex) for ex in EXCLUDED): + continue + text = mask_comments(path.read_text(encoding="utf-8", errors="replace")) + if "MGPipeUnmigratedEmulation" not in text: + continue + literal_starts = set() + for match in LITERAL_CALL.finditer(text): + line = text.count("\n", 0, match.start()) + 1 + sites.setdefault(match.group(1), []).append(f"{rel.as_posix()}:{line}") + literal_starts.add(match.start()) + for match in CALL.finditer(text): + if match.start() in literal_starts: + continue + prefix = text[text.rfind("\n", 0, match.start()) + 1:match.start()] + if DECLARATION.search(prefix): + continue + line = text.count("\n", 0, match.start()) + 1 + errors.append(f"{rel.as_posix()}:{line}: MGPipeUnmigratedEmulation called without a string " + f"literal - the name cannot be checked against {LIST_FILE.name}") + return sites + + +def check(root): + errors = [] + names = list_names(root, errors) + sites = call_sites(root, errors) + seen = set() + for name in names: + if name in seen: + errors.append(f"{LIST_FILE}: \"{name}\" is listed twice") + seen.add(name) + for name in sorted(seen): + where = sites.get(name, []) + if not where: + errors.append(f"\"{name}\" is in {LIST_FILE.name}'s list and has no call site in the tree " + f"(retired? take it out of the list)") + elif len(where) > 1: + errors.append(f"\"{name}\" has {len(where)} call sites ({', '.join(where)}); one site per name") + for name in sorted(set(sites) - seen): + errors.append(f"\"{name}\" is called at {', '.join(sites[name])} and is not in {LIST_FILE.name}'s " + f"list") + return names, sites, errors + + +def run(root): + names, sites, errors = check(root) + for err in errors: + print(f"error: {err}") + total = sum(len(v) for v in sites.values()) + state = "OK" if not errors else f"{len(errors)} error(s)" + print(f"unmigrated emulation sites: {len(names)} listed, {total} call sites - {state}") + return 1 if errors else 0 + + +def self_test(): + """Each rule reds on a synthetic tree and a matching tree is green.""" + listing = """ +TEST(PipeCatalogue, EveryUnmigratedEmulationIsNamedOnce) { + // "commented-out-name" is not in the list + const char* const kNames[] = { +%s + }; +} +""" + product = """ + void MGPipeUnmigratedEmulation(const char* name); + void Site() { + // MGPipeUnmigratedEmulation("in-a-comment"); + /* MGPipeUnmigratedEmulation("in-a-block-comment"); */ +%s + } +""" + cases = [ + ("a matching tree", ['"a",', '"b",'], ['MGPipeUnmigratedEmulation("a");', + 'MG_Pipe::MGPipeUnmigratedEmulation( "b" );'], 0), + ("a stray listed name", ['"a",', '"b",', '"stray",'], ['MGPipeUnmigratedEmulation("a");', + 'MGPipeUnmigratedEmulation("b");'], 1), + ("an unlisted call site", ['"a",'], ['MGPipeUnmigratedEmulation("a");', + 'MGPipeUnmigratedEmulation("b");'], 1), + ("two sites for one name", ['"a",'], ['MGPipeUnmigratedEmulation("a");', + 'MGPipeUnmigratedEmulation("a");'], 1), + ("a duplicated list entry", ['"a",', '"a",'], ['MGPipeUnmigratedEmulation("a");'], 1), + ("a computed name", ['"a",'], ['MGPipeUnmigratedEmulation("a");', + 'MGPipeUnmigratedEmulation(name);'], 1), + ] + failures = 0 + for label, listed, calls, want in cases: + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + (root / LIST_FILE).parent.mkdir(parents=True) + (root / LIST_FILE).write_text(listing % "\n".join(" " + s for s in listed)) + src = root / "MobileGL/MG_Backend/Site.cpp" + src.parent.mkdir(parents=True) + src.write_text(product % "\n".join(" " + c for c in calls)) + # A test's own literal call is not a product call site. + other = root / "MobileGL/MG_Test/Other/OtherTest.cpp" + other.parent.mkdir(parents=True) + other.write_text('void T() { MGPipeUnmigratedEmulation("only-in-a-test"); }\n') + _, _, errors = check(root) + got = 1 if errors else 0 + if got != want: + failures += 1 + print(f"self-test FAILED: {label}: rc {got} (want {want}); errors: {errors}") + print("self-test " + ("OK" if failures == 0 else f"FAILED ({failures})") + f": {len(cases)} cases") + return 1 if failures else 0 + + +def main(): + parser = argparse.ArgumentParser(description=__doc__.splitlines()[0]) + parser.add_argument("root", nargs="?", default=str(Path(__file__).resolve().parents[2])) + parser.add_argument("--self-test", action="store_true") + args = parser.parse_args() + if args.self_test: + return self_test() + return run(Path(args.root)) + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/scripts/data/split_coverage_exemptions.txt b/scripts/data/split_coverage_exemptions.txt index e4b82f6c..97df063a 100644 --- a/scripts/data/split_coverage_exemptions.txt +++ b/scripts/data/split_coverage_exemptions.txt @@ -62,4 +62,4 @@ SpirvShaderBinaryScenario.SpecializeShaderErrorSurfaceMatchesTheExtension no-rec ProgramPipelineScenario.AReservedNameTakesStateBeforeItIsAProgramPipeline no-records a reserved pipeline name's state is client-side and draws nothing # ---- pending-fix ------------------------------------------------------------------------------- -DirectGLES:DepthStencilReadbackAttachmentShapeScenario.DefaultFramebufferDepthStencilFormatIsBlitCompatible pending-fix Espryt spawn+tcp blit the default framebuffer's depth out as 0 (inproc and all Magma arms green, knob or not); no P8 package owns it - DEBTS.md row requested in notes/p8/A.md +# (none: P8-SE fixed the default-framebuffer depth/stencil case and registered it, notes/p8/SE.md)