[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

@@ -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
+        // built-in sampler then applies, exactly as today. A live handle resolves to THE CSO's
-        // yet is left alone and NOT cached as a miss, for the reason ResolveUnitSamplerBackend
+        // OWN TWIN (P4a fable seam F-4, SamplerImpl::ResolveSamplerCsoTwin): the sentence that
-        // gives: the program pass creates the twin later in the same draw, and its Bind moves
+        // stood here - "a handle whose twin does not exist yet is left alone ... the program
-        // the shadow row and so re-opens this memo.
+        // 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
+                    // P4a decline-site S4, RETIRED at the fable seam round (F-4): what stood here
-                    //   left alone and deliberately NOT cached as a miss: the program pass
+                    //   was `g_backendSamplerObjects.FindByHandle(sampler)` - an identity-keyed
-                    //   creates the twin later in the same draw and its Bind moves the shadow
+                    //   table asked for a content-addressed handle, a miss by construction on
-                    //   row, which re-opens this memo. Parity with ResolveUnitSamplerBackend's
+                    //   every draw - with a comment that read the miss as a within-draw ordering
-                    //   null path. SILENT, CONFIRMED AT THE VERIFICATION ROUND: this is a
+                    //   fact. The twin is the CSO's own now; a null answer has already named its
-                    //   within-draw ORDERING fact, not a missing record, and the sampler
+                    //   reason (no record, or a slot that cannot be adopted) and leaves the unit
-                    //   family's own missing-record refusal lives in D's SyncToBackend.
+                    //   as it is, which is the one decline this arm still has.
-                    if (auto* slot = SamplerImpl::g_backendSamplerObjects.FindByHandle(sampler);
+                    if (auto* twin = SamplerImpl::ResolveSamplerCsoTwin(sampler)) {
-                        slot && *slot) {
+                        twin->Bind(unit);
-                        (*slot)->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<Uint32>(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);
                             }

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<BackendSamplerObject*, MG_State::GLState::TextureState::MAX_TEXTURE_IMAGE_UNITS> g_boundSamplersCache;
         TwinRegistry<MG_State::GLState::SamplerObject, BackendSamplerObject, MG_Pipe::MGPipeKind::SamplerCso> 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<BackendSamplerObject>();
+            // 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 {

MobileGL/MG_Backend/DirectGLES/Managers.h

@@ -2583,6 +2583,39 @@ namespace MobileGL::MG_Backend::DirectGLES {
             g_boundSamplersCache;
         extern TwinRegistry<MG_State::GLState::SamplerObject, BackendSamplerObject, MG_Pipe::MGPipeKind::SamplerCso>
             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

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