[Fix, Test] (Pipe): mix the program in use and the params aggregate into the sampler-view shutter - set_sampler_views is resolved for the current program and a glUseProgram alone never re-emitted it, so a texture bound to an empty slot under one program was never synced for the next (P4a seam F-1 / F-1b)

MobileGL/MG_Impl/Pipe/Tracker.h

@@ -349,6 +349,18 @@ namespace MobileGL::MG_Pipe {
             Uint64 bindings = 0;
             Uint64 constants = 0;
             Uint64 programImages = 0;
+            // THE PROGRAM INPUT OF THE PROGRAM-RESOLVED VIEW SET (P4a fable seam F-1).
+            // set_sampler_views is resolved for the program in use (SamplerEmit.h: the sampler
+            // uniform's TYPE picks which of a unit's targets is the view) and the emitter
+            // memoises that resolution on (lifetime id, link version, backend state version). A
+            // shutter that read only the texture generations therefore missed a glUseProgram:
+            // `glBindTexture x N; glUseProgram(P1); draw; glUseProgram(P2); draw` moved nothing
+            // bit 12 read, so the view set stayed P1's - and E's record epoch, keyed on the two
+            // set serials, then never rebuilt the texture sync list for P2 either. This value is
+            // that memo key, and bit 12 mixes it in below: over-firing costs one re-resolution
+            // the set-hash suppressor absorbs, under-firing left the record describing the
+            // previous program's units.
+            Uint64 opaqueUnits = 0;
             if (program) {
                 shader = MGPipeMixShutter(program->GetLifetimeId(), program->GetLinkVersion());
                 bindings = MGPipeMixShutter(
@@ -358,6 +370,7 @@ namespace MobileGL::MG_Pipe {
                     program->GetUniformWriteSetVersion());
                 constants = MGPipeMixShutter(program->GetLifetimeId(), program->GetUBOContentVersion());
                 programImages = program->GetImageUnitVersion();
+                opaqueUnits = MGPipeMixShutter(shader, program->GetBackendStateVersion());
             } else if (const auto& pipeline = ctx.GetBoundProgramPipeline(); pipeline) {
                 using Pipeline = MG_State::GLState::ProgramPipelineObject;
                 // THE FIELDS ARE READ DIRECTLY RATHER THAN THROUGH THE TWO FUNCTIONS THAT
@@ -401,6 +414,10 @@ namespace MobileGL::MG_Pipe {
                 Uint64 stageLinks = static_cast<Uint64>(ctx.GetBoundProgramPipelineName());
                 Uint64 stageState = 0;
                 Uint64 stageImages = 0;
+                // The per-stage sampler/image unit assignments alone (glUniform1i on a stage
+                // program's sampler moves its backend state version and reaches the composite
+                // through the uniform mirror), for bit 12's program input below.
+                Uint64 stageOpaque = 0;
                 for (SizeT stage = 0; stage < Pipeline::kGraphicsStageCount; ++stage) {
                     const auto& staged = pipeline->GetStageProgram(static_cast<ShaderStage>(stage));
                     if (!staged) continue;
@@ -412,12 +429,14 @@ namespace MobileGL::MG_Pipe {
                                                           staged->GetBlockBindingVersion())),
                         staged->GetUniformWriteSetVersion());
                     stageImages = MGPipeMixShutter(stageImages, staged->GetImageUnitVersion());
+                    stageOpaque = MGPipeMixShutter(stageOpaque, staged->GetBackendStateVersion());
                 }
                 shader = stageLinks;
                 stageState = MGPipeMixShutter(stageLinks, stageState);
                 bindings = MGPipeMixShutter(stageState, stageImages);
                 constants = stageState;
                 programImages = MGPipeMixShutter(stageLinks, stageImages);
+                opaqueUnits = MGPipeMixShutter(stageLinks, stageOpaque);
             }
             now[Index(MGPipeDirty::NewShader)] = shader;
             now[Index(MGPipeDirty::NewShaderBindings)] = bindings;
@@ -491,8 +510,16 @@ namespace MobileGL::MG_Pipe {
                         ctx.GetFramebufferBindingSlot(FramebufferTarget::Draw).GetVersion())),
                 m_readFramebufferBind.Observe(
                     ctx.GetFramebufferBindingSlot(FramebufferTarget::Read).GetVersion()));
-            now[Index(MGPipeDirty::NewSamplerViews)] =
+            // Bit 12 reads FOUR things (F-1): the two texture aggregates, the bind generation
-                MGPipeMixShutter(textureContent, ctx.GetTextureBindGeneration());
+            // and the program input computed above. The params aggregate is here because
+            // SamplerEmit.h drops a unit's view to null when SamplesAsIncompleteTexture says so,
+            // and that predicate reads the effective sampler's filters - a glTexParameteri(
+            // MIN_FILTER) that completes a texture fired bit 13 and not this one, so the entry
+            // stayed null. The program input is here because the set is resolved FOR THE
+            // PROGRAM IN USE, and a glUseProgram alone moved nothing this shutter read.
+            now[Index(MGPipeDirty::NewSamplerViews)] = MGPipeMixShutter(
+                MGPipeMixShutter(MGPipeMixShutter(textureContent, textureParams), ctx.GetTextureBindGeneration()),
+                opaqueUnits);
             // Bit 13, WIDENED AT P4a FOR BIT 11's REASON and found the same way. glBindSampler
             // moves NEITHER half of what this used to read: GL_Sampler.cpp's BindSampler_State
             // goes through NoteTextureUnitTouched and TextureUnit::SetSamplerObject, and both

MobileGL/MG_IntegrationTest/Scenarios/P4aSeamAuditScenario.cpp

@@ -471,5 +471,103 @@ void main() { imageStore(i1, 0, imageLoad(i0, 0) + uvec4(2u, 0u, 0u, 0u)); }
             EXPECT_EQ(FirstGLError(), 0u) << GLErrorName(FirstGLError());
         }
 
+        // -----------------------------------------------------------------------------------
+        // F-1 / F-1b: a program switch re-resolves the view set, and the texture sync list with it
+        // -----------------------------------------------------------------------------------
+        //
+        // The sequence the audit named, and every step of it is ordinary: two programs sampling two
+        // different units, a texture bound to a unit's EMPTY 2D slot - the unit was already touched
+        // through another target, so the high-water mark does not move - while a program that does
+        // not sample it is in use, then the switch to the one that does. Nothing between the two
+        // draws touches a parameter, a level or a populated slot, which is exactly what leaves the
+        // record epoch - and the program-independent texture sync list keyed on it - unmoved on
+        // the tree the audit read: the second program sampled an unbound unit and drew black.
+        TEST_F(P4aSeamAuditScenario, ATextureBoundToAnEmptySlotUnderOneProgramIsSampledByTheNext) {
+            if (!Ready()) return;
+
+            std::string error;
+            const GLuint first = CompileProgram(kQuadVS, kFetchFS, &error);
+            ASSERT_NE(first, 0u) << error;
+            const GLuint second = CompileProgram(kQuadVS, kFetchFS, &error);
+            ASSERT_NE(second, 0u) << error;
+            glUseProgram(first);
+            glUniform1i(glGetUniformLocation(first, "uTex"), 0);
+            glUseProgram(second);
+            glUniform1i(glGetUniformLocation(second, "uTex"), 1);
+            glUseProgram(0);
+
+            // Every texture exists, complete, with its parameters set, BEFORE the first draw: a
+            // parameter or a level defined between the two draws would move the sampling-resolution
+            // generation and rescue the list by accident.
+            const GLuint red = MakeSolidTexture2D(255, 0, 0);
+            const GLuint green = MakeSolidTexture2D(0, 255, 0);
+            GLuint touch3D = 0;
+            glGenTextures(1, &touch3D);
+            glBindTexture(GL_TEXTURE_3D, touch3D);
+            const std::uint8_t blue[2 * 2 * 2 * 4] = {0, 0, 255, 255, 0, 0, 255, 255, 0, 0, 255, 255, 0, 0, 255, 255,
+                                                     0, 0, 255, 255, 0, 0, 255, 255, 0, 0, 255, 255, 0, 0, 255, 255};
+            glTexImage3D(GL_TEXTURE_3D, 0, GL_RGBA8, 2, 2, 2, 0, GL_RGBA, GL_UNSIGNED_BYTE, blue);
+            glTexParameteri(GL_TEXTURE_3D, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
+            glTexParameteri(GL_TEXTURE_3D, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
+            glBindTexture(GL_TEXTURE_3D, 0);
+            glBindTexture(GL_TEXTURE_2D, 0);
+            ASSERT_EQ(FirstGLError(), 0u) << "texture setup left a GL error behind";
+
+            ColorFbo target = MakeColorFbo(kSize, kSize);
+            ASSERT_NE(target.fbo, 0u);
+            BindFbo(target);
+            glBindVertexArray(m_vao);
+
+            // Unit 1 is TOUCHED through its 3D slot; its 2D slot stays empty. Unit 0 holds red.
+            // The first program is in use BEFORE the first verb (the clear), so the very first
+            // view set that goes out is already resolved for it - measured: with no program in
+            // use at the clear the first set is [null, null], and the bind below then re-resolves
+            // to [red, null], a DIFFERENT set that moves the serial and rescues the case by
+            // accident.
+            glActiveTexture(GL_TEXTURE1);
+            glBindTexture(GL_TEXTURE_3D, touch3D);
+            glActiveTexture(GL_TEXTURE0);
+            glBindTexture(GL_TEXTURE_2D, red);
+            glUseProgram(first);
+            ClearTo(0.0f, 0.0f, 1.0f, 1.0f);
+            DrawQuad();
+
+            // THE BIND ONTO THE EMPTY SLOT, under a program that does not sample unit 1, and a
+            // draw with THAT program so the bind's own re-resolution of the view set happens under
+            // it (the bind generation fires bit 12 at the next verb; a switch inside the same verb
+            // gap would let that fire resolve under the second program by accident) ...
+            glActiveTexture(GL_TEXTURE1);
+            glBindTexture(GL_TEXTURE_2D, green);
+            glActiveTexture(GL_TEXTURE0);
+            DrawQuad();
+            // ... and THE SWITCH to the one that does sample it. No other state moves.
+            glUseProgram(second);
+            DrawQuad();
+            EXPECT_EQ(FirstGLError(), 0u) << "the two draws 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, "green", 0.0,
+                                       "the draw after the program switch"))
+                << "black means the second program sampled an unbound unit: the texture bound to the "
+                   "empty slot was never synced because the view set - and E's record epoch with it - "
+                   "did not move on the program switch (F-1 / F-1b); red means the first program's "
+                   "set was still in force";
+
+            glActiveTexture(GL_TEXTURE1);
+            glBindTexture(GL_TEXTURE_2D, 0);
+            glBindTexture(GL_TEXTURE_3D, 0);
+            glActiveTexture(GL_TEXTURE0);
+            glBindTexture(GL_TEXTURE_2D, 0);
+            glUseProgram(0);
+            DestroyColorFbo(target);
+            glDeleteTextures(1, &red);
+            glDeleteTextures(1, &green);
+            glDeleteTextures(1, &touch3D);
+            glDeleteProgram(first);
+            glDeleteProgram(second);
+            EXPECT_EQ(FirstGLError(), 0u) << GLErrorName(FirstGLError());
+        }
+
     } // namespace
 } // namespace MGITest

MobileGL/MG_Pipe/DirtySurface.def

@@ -299,11 +299,15 @@
     /* begins with one of the ten words the pattern matched.                                */ \
     /*   UseProgram is bit 6's whole subject: the shutter is                                */ \
     /*     Mix(GetCurrentProgram()->GetLifetimeId(), GetLinkVersion()) and glUseProgram is  */  \
-    /*     what moves the object it reads through. Two call sites.                          */ \
+    /*     what moves the object it reads through. Two call sites. AND SINCE THE FABLE     */  \
+    /*     SEAM ROUND (F-1) IT IS BIT 12's TOO: set_sampler_views is resolved for the       */  \
+    /*     program in use, so its shutter mixes the same identity bit 6 reads, and a         */  \
+    /*     glUseProgram alone moves both. Undecided for the same reason as bit 6 (the        */  \
+    /*     taint below), marked the same way.                                                */  \
     /*   BindVertexArray is bit 5's, for the same reason one level down: the shutter mixes  */  \
     /*     the bound VAO's identity with its configuration version, and this is the bind.   */  \
     /*     Three call sites.                                                                */ \
-    X(UseProgram,                         NEW_SHADER)                                          \
+    X(UseProgram,                         NEW_SHADER|NEW_SAMPLER_VIEWS)                        \
     X(BindVertexArray,                    NEW_VERTEX_ELEMENTS)                                 \
     /* NOT NEW_SHADER, and the derivation refutes it outright rather than leaving it a      */  \
     /* judgement: this mutator writes m_boundProgramPipeline (plus the pipeline name table) */  \
@@ -387,11 +391,12 @@
 // row nor outlive its reason.
 //
 // IT WAS EMPTY UNTIL P4a, and it stops being empty for a reason that is a property of the
-// SCANNER rather than of the two rows. Both entries below are bit answers that are plainly
+// SCANNER rather than of the rows. Every entry below is a bit answer that is plainly true -
-// true - glUseProgram is what moves the object bit 6's shutter reads through, and
+// glUseProgram is what moves the object bits 6, 12 and 14's shutters read through (the
-// glBindVertexArray is what moves the object bit 5's shutter reads through - and the write
+// program in use; bits 12 and 14 since the fable seam round, F-1 / F-2), and glBindVertexArray
-// analysis cannot say so, because each of them reaches, BY NAME, a body that writes a member
+// is what moves the object bit 5's shutter reads through - and the write analysis cannot say
-// with no m_ prefix:
+// so, because each of the two mutators reaches, BY NAME, a body that writes a member with no
+// m_ prefix:
 //
 //   UseProgram      -> DestroyProgramSlot() writes `attachedShaders`
 //   BindVertexArray -> a call spelled `Bind(` resolves to every body of that name, one of
@@ -403,13 +408,15 @@
 // never be claimed about code the script could not read. Widening the taint rule to ignore
 // non-m_ writes would weaken the one mechanism that catches a genuine under-fire, so the rows
 // are MARKED, with the tool's own reason, rather than the tool being made more permissive.
-// Control 9c is what proves a marked row still needs the mark, and control 18 is what fails
+// Control 21 is what proves every marked row still needs its mark (it reads this list, so a
-// the moment either of these becomes decidable and the mark outlives its reason.
+// row that gains a bit here is counted rather than assumed), and control 18 is what fails the
+// moment any of them becomes decidable and the mark outlives its reason.
 //
 // The ten mutators that reach a tainted body (--check prints the count) all carry a prose
-// answer, which no derivation checks; these two are the first that carry a bit answer.
+// answer, which no derivation checks; these two mutators are the first that carry a bit answer.
 #define MGP_DIRTY_SURFACE_UNDECIDED_LIST(X)                                                     \
     X(UseProgram,      NEW_SHADER)                                                              \
+    X(UseProgram,      NEW_SAMPLER_VIEWS)                                                       \
     X(BindVertexArray, NEW_VERTEX_ELEMENTS)
 
 // clang-format on

MobileGL/MG_Test/Pipe/TrackerTest.cpp

@@ -82,6 +82,8 @@ namespace {
     X(TrackerWalk, UseProgramZeroLeavesTheBoundPipelineDrivingTheProgramBits) \
     X(TrackerAggregates, ATextureStorageDefinitionMovesTheFramebufferAggregateToo) \
     X(TrackerAggregates, ARenderbufferStorageDefinitionMovesTheFramebufferAggregate) \
+    X(TrackerWalk, AProgramSwitchAloneFiresTheSamplerViewBit) \
+    X(TrackerWalk, ATextureParameterAloneFiresTheSamplerViewBit) \
     X(TrackerAttribPayload, AFloatWriteCarriesTheFloatBitsAndNamesItsClass) \
     X(TrackerAttribPayload, AnIntWriteCarriesTheIntWordsAndNamesItsClass) \
     X(TrackerAttribPayload, AUintWriteCarriesTheUintWordsAndNamesItsClass) \
@@ -786,6 +788,44 @@ namespace {
         EXPECT_NE(dirty & MGPipeDirtyBit(MGPipeDirty::NewGlobalConstants), 0u);
     }
 
+    // P4a FABLE SEAM F-1. set_sampler_views is resolved for the PROGRAM IN USE (the sampler
+    // uniform's type picks which of a unit's targets is the view), and bit 12's shutter read
+    // only the texture-content aggregate and the bind generation - so `glUseProgram(P1); draw;
+    // glUseProgram(P2); draw` never re-emitted the set and the record went on describing P1's
+    // units. A program switch alone, with no bind and no texture change, has to fire it.
+    TEST_F(TrackerWalk, AProgramSwitchAloneFiresTheSamplerViewBit) {
+        const Uint first = Ctx().CreateProgram();
+        const Uint second = Ctx().CreateProgram();
+        Ctx().UseProgram(first);
+        Walk();
+        ASSERT_EQ(Walk(), 0u) << "the fixture did not reach a steady state";
+
+        Ctx().UseProgram(second);
+        const Uint32 dirty = Walk();
+        EXPECT_NE(dirty & MGPipeDirtyBit(MGPipeDirty::NewSamplerViews), 0u)
+            << "the view set is resolved for the program in use and a glUseProgram alone did not "
+               "re-emit it (F-1)";
+        EXPECT_EQ(Walk(), 0u) << "the widened shutter fires forever";
+    }
+
+    // The other input F-1 added: the params aggregate. SamplerEmit.h drops a unit's view to
+    // null when SamplesAsIncompleteTexture says so, and that predicate reads the effective
+    // sampler's filters and the level range - a glTexParameteri that completes a texture fired
+    // bit 13 and left the view entry null.
+    TEST_F(TrackerWalk, ATextureParameterAloneFiresTheSamplerViewBit) {
+        const auto& tex = Ctx().CreateTextureObject(1, TextureTarget::Texture2D);
+        ASSERT_TRUE(tex != nullptr);
+        Walk();
+        ASSERT_EQ(Walk(), 0u) << "the fixture did not reach a steady state";
+
+        tex->SetMaxLevel(4);
+        const Uint32 dirty = Walk();
+        EXPECT_NE(dirty & MGPipeDirtyBit(MGPipeDirty::NewSamplerViews), 0u)
+            << "completeness is a view-set input and a parameter change did not re-resolve it";
+        EXPECT_NE(dirty & MGPipeDirtyBit(MGPipeDirty::NewSamplers), 0u);
+        EXPECT_EQ(Walk(), 0u);
+    }
+
     // ===================================================================================
     // set_vertex_attrib_defaults' payload (P2 brief D10)
     // ===================================================================================

scripts/gen_pipe_dirty_surface.py

@@ -1811,17 +1811,21 @@ def self_test(scanned, bits, publishers, movers, moved, outside=None, undecided_
                 "20%s (the %s row is STALE the moment the scan stops finding it)"
                 % ("abcd"[index], name))
 
-    # 21. THE TWO UNDECIDED MARKS ARE STILL LOAD-BEARING. Dropping them has to make --check
+    # 21. EVERY UNDECIDED MARK IS STILL LOAD-BEARING. Dropping them all has to make --check
-    #     refuse both rows as unmarked UNDECIDED - which is what says the marks are covering a
+    #     refuse EVERY marked (mutator, bit) as an unmarked UNDECIDED, and nothing else - which
-    #     real blind spot rather than a verdict the analysis could give today. Control 18 is
+    #     is what says each mark is covering a real blind spot rather than a verdict the
-    #     the other direction: a mark the derivation DOES decide is itself a problem, so
+    #     analysis could give today. Control 18 is the other direction: a mark the derivation
-    #     neither of these can outlive its reason.
+    #     DOES decide is itself a problem, so no mark can outlive its reason. Read from the
+    #     file's own list rather than spelled here, so a row that gains a bit - UseProgram
+    #     gained NEW_SAMPLER_VIEWS and NEW_SHADER_IMAGES at the P4a fable seam round - cannot
+    #     silently turn this control into one that counts the wrong number.
     problems, _, _, undecided_rows = object_class_problems(real, bits, movers, moved, outside, {})
-    tripped(any(p.startswith("UNDECIDED answer NEW_SHADER for UseProgram") for p in problems)
+    marked_pairs = sorted((mutator, bit) for mutator, marks in real_marks.items() for bit in marks)
-            and any(p.startswith("UNDECIDED answer NEW_VERTEX_ELEMENTS for BindVertexArray")
+    tripped(marked_pairs
-                    for p in problems)
+            and all(any(p.startswith("UNDECIDED answer %s for %s" % (bit, mutator)) for p in problems)
-            and len(undecided_rows) == 2,
+                    for mutator, bit in marked_pairs)
-            "21 (the two P4a undecided marks are still needed)")
+            and len(undecided_rows) == len(marked_pairs),
+            "21 (every P4a undecided mark - %d of them - is still needed)" % len(marked_pairs))
 
     # THE POSITIVE CONTROLS. (a) The row that was wrong in round 3: SetPixelStoreParam writes
     # NEW_PIXEL_PACK's shutter member sixteen times, through a token-pasting macro; it has