From ccb7b6b331adb4925ce94b4178a8df45a0eba760 Mon Sep 17 00:00:00 2001 From: rereview Date: Tue, 8 Sep 2026 20:29:24 -0400 Subject: [PATCH] [Fix, Test] (Espryt): drive a unit's sampler through the CSO's own twin on the handle arm - the record arm looked a content-addressed handle up in the identity-keyed registry and could never hit, so only the pre-handle program pass ever put a glBindSampler'd object on the driver (P4a seam F-4) --- MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp | 59 ++++++++++--- MobileGL/MG_Backend/DirectGLES/Managers.cpp | 63 +++++++++++++- MobileGL/MG_Backend/DirectGLES/Managers.h | 33 +++++++ .../Scenarios/P4aSeamAuditScenario.cpp | 86 +++++++++++++++++++ 4 files changed, 225 insertions(+), 16 deletions(-) diff --git a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp index 23865c86..5225c987 100644 --- a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp +++ b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp @@ -4777,10 +4777,15 @@ namespace MobileGL::MG_Backend::DirectGLES { // // A NULL HANDLE MEANS "this unit has no sampler object", which is the record's way of // saying what the frontend's null SharedPtr says, and it unbinds - the texture's own - // built-in sampler then applies, exactly as today. A handle whose twin does not exist - // yet is left alone and NOT cached as a miss, for the reason ResolveUnitSamplerBackend - // gives: the program pass creates the twin later in the same draw, and its Bind moves - // the shadow row and so re-opens this memo. + // built-in sampler then applies, exactly as today. A live handle resolves to THE CSO's + // OWN TWIN (P4a fable seam F-4, SamplerImpl::ResolveSamplerCsoTwin): the sentence that + // stood here - "a handle whose twin does not exist yet is left alone ... the program + // pass creates the twin later in the same draw" - described a lookup that could never + // hit, because the handle is content-addressed and the registry's twins were minted off + // lifetime ids, so this arm bound nothing on every draw and only the pre-handle program + // pass ever put a glBindSampler'd object on the driver. The program pass now binds the + // same CSO twin for the units it samples, which is what stops the two arms ping-ponging + // a unit between two driver samplers. // // Declines - falls through to the frontend walk - until the set has arrived, and the // COUNT says so rather than the serial (MGPipeApplierReset advances serials whether or @@ -4837,16 +4842,15 @@ namespace MobileGL::MG_Backend::DirectGLES { SamplerImpl::UnbindSampler(unit); continue; } - // P4a decline-site S4: M - a live handle whose twin does not exist yet is - // left alone and deliberately NOT cached as a miss: the program pass - // creates the twin later in the same draw and its Bind moves the shadow - // row, which re-opens this memo. Parity with ResolveUnitSamplerBackend's - // null path. SILENT, CONFIRMED AT THE VERIFICATION ROUND: this is a - // within-draw ORDERING fact, not a missing record, and the sampler - // family's own missing-record refusal lives in D's SyncToBackend. - if (auto* slot = SamplerImpl::g_backendSamplerObjects.FindByHandle(sampler); - slot && *slot) { - (*slot)->Bind(unit); + // P4a decline-site S4, RETIRED at the fable seam round (F-4): what stood here + // was `g_backendSamplerObjects.FindByHandle(sampler)` - an identity-keyed + // table asked for a content-addressed handle, a miss by construction on + // every draw - with a comment that read the miss as a within-draw ordering + // fact. The twin is the CSO's own now; a null answer has already named its + // reason (no record, or a slot that cannot be adopted) and leaves the unit + // as it is, which is the one decline this arm still has. + if (auto* twin = SamplerImpl::ResolveSamplerCsoTwin(sampler)) { + twin->Bind(unit); } } walkedFromRecords = true; @@ -5292,6 +5296,30 @@ namespace MobileGL::MG_Backend::DirectGLES { GetRawDepthFetchSampler()->Bind(unit); MGLOG_D("Using raw depth fetch sampler on unit %d.", unit); } else if (samplerObject) { +#if MOBILEGL_PIPE_PUSH + // P4a fable seam F-4: on the handle arm the unit's sampler is THE + // CSO's OWN TWIN, the same one BindCurrentUnitSamplers' record arm + // binds, so this pass and that walk cannot hand the unit back and + // forth between two driver samplers. The handle is read only inside + // the received window - outside it BoundSamplerStates holds whatever + // an earlier, wider set left - and a unit the window does not + // describe, or a handle whose record is gone, takes the pre-handle + // path below, which carries the handle to SyncToBackend and speaks + // there. + SamplerImpl::BackendSamplerObject* csoTwin = nullptr; + if (SamplerSubsystemEnabled()) { + const auto& st = MG_Pipe::MGPipeApplier(); + const auto index = static_cast(unit); + if (index >= st.SamplerStateStart && + index - st.SamplerStateStart < st.SamplerStateCount && + index < st.BoundSamplerStates.size()) { + csoTwin = SamplerImpl::ResolveSamplerCsoTwin(st.BoundSamplerStates[index]); + } + } + if (csoTwin != nullptr) { + csoTwin->Bind(unit); + } else { +#endif auto* backendSampler = ResolveUnitSamplerBackend(unit, samplerObject); if (!backendSampler) { auto& backendObj = @@ -5314,6 +5342,9 @@ namespace MobileGL::MG_Backend::DirectGLES { // unit: without this the driver kept sampling with the texture's own // parameters and every sampler object was inert. backendSampler->Bind(unit); +#if MOBILEGL_PIPE_PUSH + } +#endif } else { SamplerImpl::UnbindSampler(unit); } diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.cpp b/MobileGL/MG_Backend/DirectGLES/Managers.cpp index cad87c47..7c4226f2 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.cpp +++ b/MobileGL/MG_Backend/DirectGLES/Managers.cpp @@ -12316,8 +12316,19 @@ namespace MobileGL::MG_Backend::DirectGLES { ZoneScopedC(TRACY_ZONECOLOR_BACKEND); #endif if (!stateSamplerObject) { +#if MOBILEGL_PIPE_PUSH + // A CSO TWIN HAS NO FRONTEND OBJECT (P4a fable seam F-4, ResolveSamplerCsoTwin): + // the content-addressed handle is its identity and the applier's record its + // only authority, so a null object beside a live handle is the record arm and + // not the pre-P4a error. Everything below that names the object is guarded on + // it; the pull build's text is the three lines the #if brackets. + if (MG_Pipe::MGPipeHandleIsNull(pushedCso)) { +#endif MGLOG_E_ONCE("State sampler object is null, cannot sync to backend."); return; +#if MOBILEGL_PIPE_PUSH + } +#endif } // P4a (D-F1): a SamplerObject is a pure 100-byte value with no driver-side per-object @@ -12331,6 +12342,9 @@ namespace MobileGL::MG_Backend::DirectGLES { // pre-P4a text token for token (D-P). #if MOBILEGL_PIPE_PUSH const SamplerParameters* pushedParams = nullptr; + // The GL name for the two log lines below, or 0 for a CSO twin, which has no object + // to name (F-4): the handle in the same line is its name. + const Uint samplerName = stateSamplerObject ? stateSamplerObject->GetExternalIndex() : 0u; if (SamplerSubsystemEnabled()) { // THE HANDLE COMES FROM THE CALLER, NOT FROM THIS TWIN'S REGISTRY, and the // difference is the seam D's verification round found on the integrated tree. @@ -12353,12 +12367,12 @@ namespace MobileGL::MG_Backend::DirectGLES { if (record == nullptr) { MGLOG_E_ONCE("MGPipe: sampler %u has no applier record on the handle arm, so its " "parameters cannot be pushed (handle {%u, %u})", - stateSamplerObject->GetExternalIndex(), pushedCso.Slot, pushedCso.Gen); + samplerName, pushedCso.Slot, pushedCso.Gen); return; } if (m_isInitialized && m_syncedSamplerSerial != 0 && m_syncedSamplerSerial == record->Serial) { MGLOG_D("Sampler parameters have not changed for sampler ID: %u, skipping sync.", - stateSamplerObject->GetExternalIndex()); + samplerName); return; } m_syncedSamplerSerial = record->Serial; @@ -12396,6 +12410,11 @@ namespace MobileGL::MG_Backend::DirectGLES { pushedParams = &stateSamplerObject->GetAllSamplerParameters(); } } else { + // A CSO twin is only ever resolved on the handle arm (ResolveSamplerCsoTwin gates + // on SamplerSubsystemEnabled()), so a null object cannot reach the legacy body + // below; stated as a return rather than assumed, because that body dereferences + // it. + if (!stateSamplerObject) return; #if !MOBILEGL_PIPE_LEGACY_MEMOS // UNREACHABLE: ResolveSamplerSubsystemArm stops at its first call when the bit is // clear and the pre-handle arm is not compiled. Kept, and kept loud. @@ -12426,8 +12445,12 @@ namespace MobileGL::MG_Backend::DirectGLES { m_syncedSamplerVersion = currentSamplerVersion; #endif +#if MOBILEGL_PIPE_PUSH + MGLOG_D("Syncing sampler with backend ID %u to backend for state ID %u", m_backendSamplerId, samplerName); +#else MGLOG_D("Syncing sampler with backend ID %u to backend for state ID %u", m_backendSamplerId, stateSamplerObject->GetExternalIndex()); +#endif #if MOBILEGL_PIPE_PUSH const SamplerParameters& samplerParams = *pushedParams; @@ -12544,6 +12567,42 @@ namespace MobileGL::MG_Backend::DirectGLES { Array g_boundSamplersCache; TwinRegistry g_backendSamplerObjects; + +#if MOBILEGL_PIPE_PUSH + // P4a fable seam F-4. THE TWIN FOR A CONTENT-ADDRESSED SamplerCso HANDLE, keyed by that + // handle and synced from its record - see the declaration for why the identity-keyed + // lookup it replaces could never hit. + BackendSamplerObject* ResolveSamplerCsoTwin(MG_Pipe::MGPipeHandle cso) { + if (MG_Pipe::MGPipeHandleIsNull(cso)) return nullptr; + // THE RECORD FIRST, before the table is touched: a handle with no record is a seam + // defect (the client minted and named a CSO it never described, or evicted one a + // standing set still names), and adopting a slot for it would leave a twin that + // syncs nothing. The census stem is the sampler family's. + const auto* record = PipeSamplerCsoRecordForHandle(cso); + if (record == nullptr) { + MGLOG_E_ONCE("MGPipe: sampler CSO {%u, %u} has no applier record on the handle arm, so no " + "driver sampler can be built for it and the unit keeps what it holds", + cso.Slot, cso.Gen); + return nullptr; + } + // The same slot table the identity twins live in: one allocator serves both handle + // families of this kind, so a content-addressed slot and an identity-minted slot can + // never coincide, and the generation discipline (forward = recycle, backward = + // refused) is what retires a twin whose CSO the client's LRU evicted and re-minted. + auto* slot = g_backendSamplerObjects.GetOrCreateByHandle(cso); + if (slot == nullptr) { + MGLOG_E_ONCE("MGPipe: sampler CSO {%u, %u} cannot be adopted on the handle arm (the slot's " + "live generation is %u), so no driver sampler is built for it", + cso.Slot, cso.Gen, g_backendSamplerObjects.LiveGenAt(cso.Slot)); + return nullptr; + } + if (!*slot) *slot = MakeShared(); + // Serial-gated inside: a CSO whose record did not move since this twin last synced + // costs the record lookup above and one compare. + (*slot)->SyncToBackend(nullptr, cso); + return slot->get(); + } +#endif } // namespace SamplerImpl namespace RenderbufferImpl { diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.h b/MobileGL/MG_Backend/DirectGLES/Managers.h index 3d7c2a34..d9406051 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.h +++ b/MobileGL/MG_Backend/DirectGLES/Managers.h @@ -2583,6 +2583,39 @@ namespace MobileGL::MG_Backend::DirectGLES { g_boundSamplersCache; extern TwinRegistry g_backendSamplerObjects; + +#if MOBILEGL_PIPE_PUSH + // P4a FABLE SEAM F-4: THE TWIN FOR A CONTENT-ADDRESSED SamplerCso HANDLE. + // + // bind_sampler_states carries, per unit, the handle of a CSO the client allocated BY + // CONTENT (SamplerEmit.h: MGPipeSlots().Allocate with no lifetime id), while every twin + // in g_backendSamplerObjects was minted off a SamplerObject's lifetime id - two disjoint + // slot families out of one allocator. So `g_backendSamplerObjects.FindByHandle( + // BoundSamplerStates[unit])` (the record arm of BindCurrentUnitSamplers, E's S4) could + // never find a twin, the record arm bound nothing on every draw, and every glBindSampler + // reached the driver only through the pre-handle program pass - S-1's confusion one + // loop over, and exactly what SamplerEmit.h:201-205 forbids ("a backend must NOT key a + // sampler twin on a SamplerObject's lifetime id; the twin's life is + // create_sampler_state -> delete_sampler_state"). + // + // This is the twin keyed the way the record is: resolved-or-created AT THE CSO HANDLE + // (GetOrCreateByHandle, the same slot table, a slot the identity family can never hold) + // and synced from the record it names, serial-gated. Two callers bind it - the record + // arm of BindCurrentUnitSamplers and the program pass's sampler override - so the two + // cannot ping-pong the unit between an identity twin and a CSO twin. Its death is the + // slot's recycle: the client's LRU eviction drops the record and frees the slot, and + // the next handout at that slot arrives with a moved generation, which GetOrCreate( + // handle) answers by resetting the twin (the driver sampler goes with it). A twin for + // an evicted CSO therefore lives until its slot is reused - bounded by the cache's + // capacity, never by draw count - and there is no delete_sampler_state hook to retire it + // earlier; the ops table carries none for this kind. + // + // Null, loudly, when the handle names no record (a client seam) or cannot be adopted + // (a generation behind the slot's live entry); null silently for the null handle. The + // pre-handle arm - a twin keyed on the frontend object - is untouched and still serves + // the raw-depth-fetch sampler and every caller that carries no handle. + BackendSamplerObject* ResolveSamplerCsoTwin(MG_Pipe::MGPipeHandle cso); +#endif } // namespace SamplerImpl #if MOBILEGL_PIPE_PUSH diff --git a/MobileGL/MG_IntegrationTest/Scenarios/P4aSeamAuditScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/P4aSeamAuditScenario.cpp index afb38c2f..f56d9403 100644 --- a/MobileGL/MG_IntegrationTest/Scenarios/P4aSeamAuditScenario.cpp +++ b/MobileGL/MG_IntegrationTest/Scenarios/P4aSeamAuditScenario.cpp @@ -648,5 +648,91 @@ void main() { imageStore(i1, 0, imageLoad(i0, 0) + uvec4(2u, 0u, 0u, 0u)); } EXPECT_EQ(FirstGLError(), 0u) << GLErrorName(FirstGLError()); } + // ----------------------------------------------------------------------------------- + // F-4: the unit's driver sampler is the CSO's own twin on the handle arm + // ----------------------------------------------------------------------------------- + // + // Public-GL half: a glBindSampler'd object whose wrap differs from the texture's built-in + // sampler wins (GL 4.6 core 8.10) - every arm passes this, because the pre-handle program + // pass bound the object through its identity twin. White-box half, on Espryt's handle arm: + // the sampler the unit carries on the driver must be the twin Espryt holds AT THE CSO + // HANDLE bind_sampler_states named for the unit. On the tree the audit read that twin did + // not exist - the handle is content-addressed, the registry's twins were minted off + // lifetime ids - so the record arm bound nothing on every draw. + TEST_F(P4aSeamAuditScenario, ABoundSamplerObjectIsDrivenThroughItsCsoTwinOnTheHandleArm) { + if (!Ready()) return; + + std::string error; + const GLuint program = CompileProgram(kQuadVS, kOutsideSampleFS, &error); + ASSERT_NE(program, 0u) << error; + + // The texture's built-in sampler REPEATS, so (1.5, 1.5) reads the red texel through it; + // the sampler object CLAMPS TO A WHITE BORDER, so the same coordinate reads white + // through it. White is a Vulkan palette border colour, so Magma needs no extension. + const GLuint red = MakeSolidTexture2D(255, 0, 0); + glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_REPEAT); + glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_REPEAT); + GLuint sampler = 0; + glGenSamplers(1, &sampler); + glSamplerParameteri(sampler, GL_TEXTURE_MIN_FILTER, GL_NEAREST); + glSamplerParameteri(sampler, GL_TEXTURE_MAG_FILTER, GL_NEAREST); + glSamplerParameteri(sampler, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_BORDER); + glSamplerParameteri(sampler, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_BORDER); + const GLfloat white[4] = {1.0f, 1.0f, 1.0f, 1.0f}; + glSamplerParameterfv(sampler, GL_TEXTURE_BORDER_COLOR, white); + ASSERT_EQ(FirstGLError(), 0u) << "sampler setup left a GL error behind"; + + ColorFbo target = MakeColorFbo(kSize, kSize); + ASSERT_NE(target.fbo, 0u); + BindFbo(target); + glBindVertexArray(m_vao); + glActiveTexture(GL_TEXTURE0); + glBindTexture(GL_TEXTURE_2D, red); + glBindSampler(0, sampler); + glUseProgram(program); + glUniform1i(glGetUniformLocation(program, "uTex"), 0); + ClearTo(0.0f, 0.0f, 1.0f, 1.0f); + DrawQuad(); + EXPECT_EQ(FirstGLError(), 0u) << "the draw left a GL error behind"; + + const Image image = ReadPixels(kSize, kSize); + ASSERT_FALSE(image.Empty()); + EXPECT_TRUE(RegionIsMostly(image, kInset, kSize - 1 - kInset, kInset, kSize - 1 - kInset, "white", 0.0, + "the draw through the bound sampler object")) + << "red means the texture's own REPEAT sampler applied instead of the bound object's " + "CLAMP_TO_BORDER"; + + if (SamplerHandleArmIsLive("F-4")) { + EsprytUnitSamplerPeek peek{}; + ASSERT_TRUE(PeekEsprytUnitSampler(0, sampler, &peek)); + std::cout << "[ P4aSeamAudit ] white-box: unit 0 driver sampler " << peek.BoundSamplerId + << ", bind_sampler_states handle {" << peek.CsoHandleSlot << ", " << peek.CsoHandleGen + << "} inside window " << (peek.UnitInsideWindow ? "yes" : "no") << ", CSO twin " + << peek.CsoTwinSamplerId << ", identity twin " << peek.IdentityTwinSamplerId << std::endl; + EXPECT_TRUE(peek.UnitInsideWindow) << "bind_sampler_states did not describe unit 0"; + EXPECT_NE(peek.CsoHandleSlot, 0u) << "bind_sampler_states names no CSO for a unit that carries " + "a sampler object"; + EXPECT_NE(peek.CsoTwinSamplerId, 0u) + << "Espryt holds no twin at the CSO handle bind_sampler_states named: the record arm's " + "lookup went to the identity-keyed registry with a content-addressed handle and " + "could never hit (F-4)"; + EXPECT_EQ(peek.BoundSamplerId, peek.CsoTwinSamplerId) + << "the driver sampler on unit 0 is not the CSO's twin, so it was put there by the " + "pre-handle program pass and not by the record arm (F-4)"; + EXPECT_EQ(peek.IdentityTwinSamplerId, 0u) + << "an identity-keyed twin was minted for the sampler object on the handle arm: the " + "pre-handle pass is still the one doing the binding"; + } + + glBindSampler(0, 0); + glBindTexture(GL_TEXTURE_2D, 0); + glUseProgram(0); + DestroyColorFbo(target); + glDeleteSamplers(1, &sampler); + glDeleteTextures(1, &red); + glDeleteProgram(program); + EXPECT_EQ(FirstGLError(), 0u) << GLErrorName(FirstGLError()); + } + } // namespace } // namespace MGITest