[Fix, Test] (MG_State, MG_Impl): glIsProgramPipeline answers for the first bind, not for the materialization every pipeline command now does

MobileGL/MG_Impl/GLImpl/Drawing/GL_Drawing.cpp

@@ -674,6 +674,12 @@ namespace MobileGL::MG_Impl::GLImpl {
         // dereferences GetProgramForDraw() unconditionally, so "no current program" has to be
         // stopped here or it is a null dereference rather than the INVALID_OPERATION the spec
         // asks for - reachable through a bound pipeline that supplies no graphics stage.
+        //
+        // AFTER the argument checks, unlike the sibling draw entry points, and deliberately:
+        // the argument rules here are properties of the call rather than of GL state, and
+        // NegativeApiErrorsTest.IndirectParameterDrawsCheckBothBuffers pins the INVALID_VALUE
+        // they produce for a call made with no program bound. Same precedence decision, and
+        // the same reason, as DispatchComputeIndirect above.
         if (!ValidateCurrentProgramForExecution(__func__)) return;
         auto multiDrawElementsIndirectCount = MG_Backend::gBackendFunctionsTable.GL.MultiDrawElementsIndirectCount;
         if (!multiDrawElementsIndirectCount) {
@@ -694,7 +700,7 @@ namespace MobileGL::MG_Impl::GLImpl {
                                        4 * sizeof(Uint32), __func__)) {
             return;
         }
-        // See MultiDrawElementsIndirectCount.
+        // See MultiDrawElementsIndirectCount, including why this one goes last.
         if (!ValidateCurrentProgramForExecution(__func__)) return;
         auto multiDrawArraysIndirectCount = MG_Backend::gBackendFunctionsTable.GL.MultiDrawArraysIndirectCount;
         if (!multiDrawArraysIndirectCount) {

MobileGL/MG_IntegrationTest/Scenarios/ProgramPipelineScenario.cpp

@@ -133,6 +133,47 @@ void main() { gl_Position = i_position; }
 
     } // namespace
 
+    // The root cause of the cluster, stated as the two halves it actually has.
+    //
+    // Half one: glGenProgramPipelines only reserves a name, and every pipeline command used to
+    // demand a materialized object - so the spec's own call order (stages attached BEFORE the
+    // first bind, GL 4.6 core 7.4) was rejected with GL_INVALID_OPERATION and the stages were
+    // never recorded. Half two is the trap that fix walks into: the object now appears the
+    // moment anything needs somewhere to put state, so "the object exists" stops being the
+    // right answer for glIsProgramPipeline, which the spec ties to the first BIND. A pure
+    // query must not turn a reserved name into a program pipeline either.
+    TEST_F(ProgramPipelineScenario, AReservedNameTakesStateBeforeItIsAProgramPipeline) {
+        if (!Ready()) return;
+
+        const GLuint vs = MakeSeparable(GL_VERTEX_SHADER, kSeparableVS);
+        if (vs == 0) return;
+        const GLuint pipeline = MakePipeline();
+        ASSERT_NE(pipeline, 0u);
+        EXPECT_EQ(glIsProgramPipeline(pipeline), GL_FALSE) << "a merely reserved name is not a pipeline yet";
+
+        // A query answers out of default state - and leaves the name exactly as it found it.
+        GLint validateStatus = -1;
+        glGetProgramPipelineiv(pipeline, GL_VALIDATE_STATUS, &validateStatus);
+        EXPECT_EQ(FirstGLError(), 0u) << "querying a reserved pipeline name must not be an error";
+        EXPECT_EQ(validateStatus, 0) << "a pipeline that was never validated reports VALIDATE_STATUS 0";
+        EXPECT_EQ(glIsProgramPipeline(pipeline), GL_FALSE) << "a pure query must not create the object";
+
+        // ...and glUseProgramStages RECORDS the stage on the reserved name rather than
+        // rejecting it, which is the whole defect: without this the pipeline stayed empty.
+        glUseProgramStages(pipeline, GL_VERTEX_SHADER_BIT, vs);
+        EXPECT_EQ(FirstGLError(), 0u) << "glUseProgramStages before the first bind must be accepted";
+        GLint stageProgram = 0;
+        glGetProgramPipelineiv(pipeline, GL_VERTEX_SHADER, &stageProgram);
+        EXPECT_EQ(static_cast<GLuint>(stageProgram), vs) << "the stage program was not recorded";
+        EXPECT_EQ(glIsProgramPipeline(pipeline), GL_FALSE) << "taking state is still not being bound";
+
+        // The bind is what the spec ties glIsProgramPipeline to.
+        glBindProgramPipeline(pipeline);
+        EXPECT_EQ(glIsProgramPipeline(pipeline), GL_TRUE);
+        EXPECT_EQ(FirstGLError(), 0u);
+        glBindProgramPipeline(0);
+    }
+
     // The floor: a two-stage pipeline must paint. If this fails, nothing above it can pass, and
     // the eight shared conformance cases have exactly one cause.
     TEST_F(ProgramPipelineScenario, ATwoStagePipelinePaintsWhatItsStagesDescribe) {

MobileGL/MG_State/GLState/Core.cpp

@@ -383,6 +383,15 @@ namespace MobileGL::MG_State {
         //
         // Location-by-location so that arrays are carried across whole, and via the padded
         // storage span so a mat3's std140 column padding travels with it.
+        //
+        // KNOWN LIMIT, inherent to flattening rather than to this copy: SSO gives each stage
+        // program its own storage for a uniform, so two stage programs may declare the same
+        // name and hold different values - but the composite is one link and has one slot for
+        // it. RefreshCompositeUniforms walks the stages in order, so the last graphics stage
+        // that declares the name wins, including when it is only holding the zero default and
+        // an earlier stage held a written value. Fixing it properly means mirroring only the
+        // uniforms a program has actually been written to, which wants a per-location dirty
+        // set on ProgramObject.
         static void MirrorUniformValues(ProgramObject& source, ProgramObject& destination) {
             if (!source.GetLinkStatus() || !destination.GetLinkStatus()) return;
             const char* sourceUbo = static_cast<const char*>(source.GetUBOData());
@@ -1062,26 +1071,42 @@ namespace MobileGL::MG_State {
         // Program pipeline
         void GLContext::GenProgramPipelineNames(Uint number, Vector<Uint>& pipelines) {
             pipelines.resize(number);
-            // Names only: glIsProgramPipeline must answer GL_FALSE until one is bound or created.
+            // Names only. The OBJECT appears as soon as a command needs somewhere to put state
+            // (see MaterializeProgramPipelineObject), but glIsProgramPipeline still answers
+            // GL_FALSE until the name is bound or created - see IsProgramPipelineObject.
             m_programPipelineNames.Generate(number, pipelines.data());
         }
 
         void GLContext::CreateProgramPipelineObject(Uint index) {
-            m_programPipelines[index] = MakeShared<ProgramPipelineObject>(index);
+            const auto object = MakeShared<ProgramPipelineObject>(index);
+            // glCreateProgramPipelines makes the object outright, so it answers
+            // glIsProgramPipeline immediately - unlike a name that only got here through
+            // GenProgramPipelines plus a command that materialized it.
+            object->MarkEverBound();
+            m_programPipelines[index] = object;
         }
 
         Bool GLContext::ValidateProgramPipelineName(Uint index) const {
             return index == 0 || m_programPipelineNames.IsValid(index);
         }
 
+        // glIsProgramPipeline. Materialization is NOT the test: the object now appears as soon
+        // as any command takes state from a reserved name, and two of those commands are the
+        // pure queries glGetProgramPipelineiv / glGetProgramPipelineInfoLog - so keying this on
+        // map membership would let merely READING a gen'd name turn it into an object. GL 4.6
+        // core 7.4 gives the real rule: a GenProgramPipelines name acquires program pipeline
+        // state when it is first bound. Same shape as IsTransformFeedbackObject.
         Bool GLContext::IsProgramPipelineObject(Uint index) const {
             if (index == 0 || !m_programPipelineNames.IsValid(index)) return false;
-            return m_programPipelines.find(index) != m_programPipelines.end();
+            const auto it = m_programPipelines.find(index);
+            return it != m_programPipelines.end() && it->second && it->second->GetEverBound();
         }
 
         void GLContext::BindProgramPipelineObject(Uint index) {
             if (index != 0) {
-                MaterializeProgramPipelineObject(index);
+                if (const auto& object = MaterializeProgramPipelineObject(index)) {
+                    object->MarkEverBound();
+                }
             }
             m_boundProgramPipeline = index;
         }

MobileGL/MG_State/GLState/Core.h

@@ -457,8 +457,10 @@ namespace MobileGL {
                 UnorderedMap<Uint, TransformFeedbackObjectState> m_transformFeedbackObjects;
                 IndexGenerator<Uint> m_transformFeedbackNames;
                 Uint m_boundTransformFeedback = 0;
-                // Map membership IS object existence here: a pipeline has no stateful default
-                // object 0, so no everBound flag is needed.
+                // Map membership is object EXISTENCE, which is not the same as the answer
+                // glIsProgramPipeline gives: any command that needs somewhere to put state
+                // materializes a reserved name, so the object can exist well before it is
+                // bound. ProgramPipelineObject::everBound carries the Is* answer.
                 UnorderedMap<Uint, SharedPtr<ProgramPipelineObject>> m_programPipelines;
                 IndexGenerator<Uint> m_programPipelineNames;
                 Uint m_boundProgramPipeline = 0;

MobileGL/MG_State/GLState/ProgramState/ProgramPipelineObject.h

@@ -40,6 +40,17 @@ namespace MobileGL {
 
                 Uint GetExternalIndex() const { return m_externalIndex; }
 
+                // glIsProgramPipeline's answer, and NOT the same question as "does this object
+                // exist" (GL 4.6 core 7.4: a GenProgramPipelines name "acquires program pipeline
+                // state only when first bound"). The object is materialized by any of the
+                // commands that take state from a reserved name - including the pure queries
+                // glGetProgramPipelineiv and glGetProgramPipelineInfoLog, which have to answer
+                // out of default state without ever making the name report as an object. So
+                // existence is map membership and this is a separate latch, exactly as
+                // TransformFeedbackObject::everBound is.
+                Bool GetEverBound() const { return m_everBound; }
+                void MarkEverBound() { m_everBound = true; }
+
                 // The stages a DRAW is built from: every stage but compute. GL 4.6 core 7.4
                 // makes the compute stage exclusive - a program object containing a compute
                 // shader may contain no other stage, and a pipeline's compute stage is
@@ -56,7 +67,17 @@ namespace MobileGL {
                 // GRAPHICS stages are composited into a single hidden program object, rebuilt
                 // whenever the stage set - or any stage program's own link - changes. The
                 // signature is what that "changes" means: a stage program's lifetime id pins the
-                // object and its backend state version pins the link generation. It covers
+                // object and its backend state version pins the link generation.
+                //
+                // The backend state version is BLUNTER than that description: glUniform1i on a
+                // sampler and glUniformBlockBinding bump it too, so either one throws the
+                // composite away and relinks it on the next draw. That is correct but slow, and
+                // it is a shape the SSO conformance cases hit in a loop. Narrowing it to
+                // GetLinkVersion() means the composite must instead pick those two up the way it
+                // picks up uniform values (below) - the sampler half already works that way,
+                // the block-binding half does not yet, which is why this still keys on the
+                // blunter version.
+                // It covers
                 // exactly the stages the composite is built from, so attaching or relinking a
                 // compute stage never invalidates a perfectly good graphics composite - and the
                 // compute stage, having no composite of its own, can never collide with it.
@@ -120,6 +141,7 @@ namespace MobileGL {
                 String m_infoLog;
                 const Uint m_externalIndex = 0;
                 Bool m_validateStatus = false;
+                Bool m_everBound = false;
             };
         } // namespace GLState
     } // namespace MG_State

MobileGL/MG_Test/Program/AsyncLinkTest.cpp

@@ -628,8 +628,8 @@ TEST_F(AsyncLinkTest, DrawThroughAPipelineWithAPendingStageProgramJoinsFirst) {
     GLuint pipeline = 0;
     GenProgramPipelines(1, &pipeline);
     ASSERT_NE(pipeline, 0u);
-    // Bind before UseProgramStages: glGenProgramPipelines only reserves the name, and the
-    // first bind is what turns it into an object glUseProgramStages can find.
+    // Bound first only because this test draws through the pipeline; glUseProgramStages no
+    // longer needs it (it materializes a reserved name itself, GL 4.6 core 7.4).
     BindProgramPipeline(pipeline);
     UseProgramStages(pipeline, GL_VERTEX_SHADER_BIT, vsProgram);
     ASSERT_EQ(GetError(), GL_NO_ERROR);