diff --git a/MobileGL/MG_Impl/GLImpl/Program/GL_Program.cpp b/MobileGL/MG_Impl/GLImpl/Program/GL_Program.cpp index 8aa87a53..d02e2956 100644 --- a/MobileGL/MG_Impl/GLImpl/Program/GL_Program.cpp +++ b/MobileGL/MG_Impl/GLImpl/Program/GL_Program.cpp @@ -1119,6 +1119,17 @@ namespace MobileGL::MG_Impl::GLImpl { if (!programObject.IsUniformOpaqueAtLocation(location)) { MGLOG_D("%s: program = %d, location = %d, maxLocation = %d", __func__, programObject.GetExternalIndex(), location, programObject.GetMaxUniformLocation()); + // Record the write for the pipeline composite's uniform mirror, which copies only + // the locations a stage program has actually been written to (see + // ProgramObject::MarkUniformWrittenAtLocation). Here rather than further down + // because every exit below is still a write as far as GL is concerned: the + // buffered-write detour returns early, the bytes-equal dedupe returns early, and + // even the no-backing-storage bail is a uniform the application addressed. This is + // the funnel EVERY glUniform* and glProgramUniform* entry point reaches, once per + // LOCATION - so an array element write marks that element and nothing else. On a + // program that can never be a pipeline stage - the monolithic glUseProgram path, + // which is where the thousands of calls per frame are - this is one bool branch. + programObject.MarkUniformWrittenAtLocation(location); // Everything up to and including the clamp is phase-A data (the uniform's GL type // decides its size), so it is answered without joining anything. const SizeT size = programObject.GetUniformSizesInBytes(location); diff --git a/MobileGL/MG_IntegrationTest/Scenarios/ProgramPipelineScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/ProgramPipelineScenario.cpp index a00e8af5..c764730b 100644 --- a/MobileGL/MG_IntegrationTest/Scenarios/ProgramPipelineScenario.cpp +++ b/MobileGL/MG_IntegrationTest/Scenarios/ProgramPipelineScenario.cpp @@ -599,6 +599,241 @@ void main() { o_color = u_colour; } gl.EndFrame(); } + // The shared-header idiom, drawn: BOTH stages declare `u_mvp` because they both include the + // same header, and only the VERTEX program is ever written to. + // + // The composite has one slot for `u_mvp`, and mirroring every active uniform of every stage + // in stage order meant the fragment program's untouched zero matrix landed last and won. + // The vertex stage then transformed every vertex by a zero matrix and the frame came out + // empty - from an application that had done nothing wrong, with no GL error anywhere to say + // so. Only uniforms a stage has actually been written to are mirrored now. + TEST_F(ProgramPipelineScenario, AUniformDeclaredInTwoStagesKeepsTheValueTheWrittenStageHolds) { + if (!Ready()) return; + HeadlessGL& gl = Gl(); + const int width = gl.Width(); + const int height = gl.Height(); + + // The same declaration in both stages, exactly as a shared header produces it. The + // fragment stage does not even USE it for its output - declaring it is enough. + static const char* kSharedMvpVS = R"(#version 430 core +out gl_PerVertex { vec4 gl_Position; }; +uniform mat4 u_mvp; +void main() +{ + vec4 corner = vec4(0.0, 0.0, 0.0, 1.0); + switch (gl_VertexID) + { + case 0: corner = vec4(-1.0, -1.0, 0.0, 1.0); break; + case 1: corner = vec4( 1.0, -1.0, 0.0, 1.0); break; + case 2: corner = vec4(-1.0, 1.0, 0.0, 1.0); break; + case 3: corner = vec4( 1.0, 1.0, 0.0, 1.0); break; + } + gl_Position = u_mvp * corner; +} +)"; + static const char* kSharedMvpFS = R"(#version 430 core +uniform mat4 u_mvp; +out vec4 o_color; +void main() { o_color = vec4(0.0, 1.0, 0.0, u_mvp[3][3]); } +)"; + + const GLuint vs = MakeSeparable(GL_VERTEX_SHADER, kSharedMvpVS); + const GLuint fs = MakeSeparable(GL_FRAGMENT_SHADER, kSharedMvpFS); + if (vs == 0 || fs == 0) return; + + const GLuint pipeline = MakePipeline(); + glUseProgramStages(pipeline, GL_VERTEX_SHADER_BIT, vs); + glUseProgramStages(pipeline, GL_FRAGMENT_SHADER_BIT, fs); + glBindProgramPipeline(pipeline); + + // Written through the VERTEX program only - which is the whole point. The fragment + // program's `u_mvp` is left at GL's zero default and must not win the composite's slot. + glActiveShaderProgram(pipeline, vs); + const GLint location = glGetUniformLocation(vs, "u_mvp"); + ASSERT_NE(location, -1); + const GLfloat identity[16] = {1.0f, 0.0f, 0.0f, 0.0f, 0.0f, 1.0f, 0.0f, 0.0f, + 0.0f, 0.0f, 1.0f, 0.0f, 0.0f, 0.0f, 0.0f, 1.0f}; + glUniformMatrix4fv(location, 1, GL_FALSE, identity); + ASSERT_EQ(FirstGLError(), 0u) << "glUniformMatrix4fv through the active shader program errored"; + + GLuint vao = 0; + glGenVertexArrays(1, &vao); + glBindVertexArray(vao); + BindDefaultFramebuffer(); + glViewport(0, 0, width, height); + glDisable(GL_SCISSOR_TEST); + glDisable(GL_DEPTH_TEST); + ClearTo(0.0f, 0.0f, 0.0f, 1.0f); + glUseProgram(0); + glDrawArrays(GL_TRIANGLE_STRIP, 0, 4); + + // A zero matrix collapses all four corners onto the origin and paints nothing at all, so + // "green over the whole viewport" IS the assertion that the written matrix was the one + // the draw used. (The fragment stage reads u_mvp too - into the alpha channel - purely + // so the optimizer cannot delete its declaration and make the case vacuous.) + const Image painted = ReadPixels(width, height); + EXPECT_TRUE(RegionIsMostly(painted, 2, width - 3, 2, height - 3, "green", 0.0, + "a pipeline whose u_mvp is declared in both stages and written in one")); + EXPECT_EQ(FirstGLError(), 0u) << "the shared-uniform pipeline draw leaked a GL error"; + + glBindVertexArray(0); + glDeleteVertexArrays(1, &vao); + gl.EndFrame(); + } + + // Rebinding a uniform block AFTER the pipeline has already drawn once. + // + // This is the shape the composite cache key change put weight on. The composite used to be + // thrown away and relinked whenever glUniformBlockBinding moved a stage program's backend + // state version, so the second draw here got a brand-new composite that happened to pick the + // new binding up on the way. Now the composite SURVIVES the rebinding, which means the only + // thing that can carry the new binding to the draw is the refresh path - so this case is + // what says that path is really doing the work. + TEST_F(ProgramPipelineScenario, RebindingAUniformBlockBetweenDrawsReachesTheNextDraw) { + if (!Ready()) return; + HeadlessGL& gl = Gl(); + const int width = gl.Width(); + const int height = gl.Height(); + + static const char* kUniformBlockFS = R"(#version 430 core +layout(std140) uniform Colour { vec4 u_colour; }; +out vec4 o_color; +void main() { o_color = u_colour; } +)"; + const GLuint vs = MakeSeparable(GL_VERTEX_SHADER, kSeparableVS); + const GLuint fs = MakeSeparable(GL_FRAGMENT_SHADER, kUniformBlockFS); + if (vs == 0 || fs == 0) return; + + // Two buffers on two different binding points, holding two different colours. + const GLfloat red[4] = {1.0f, 0.0f, 0.0f, 1.0f}; + const GLfloat green[4] = {0.0f, 1.0f, 0.0f, 1.0f}; + constexpr GLuint kFirstBinding = 2; + constexpr GLuint kSecondBinding = 5; + GLuint buffers[2] = {0, 0}; + glGenBuffers(2, buffers); + glBindBuffer(GL_UNIFORM_BUFFER, buffers[0]); + glBufferData(GL_UNIFORM_BUFFER, sizeof(red), red, GL_STATIC_DRAW); + glBindBufferBase(GL_UNIFORM_BUFFER, kFirstBinding, buffers[0]); + glBindBuffer(GL_UNIFORM_BUFFER, buffers[1]); + glBufferData(GL_UNIFORM_BUFFER, sizeof(green), green, GL_STATIC_DRAW); + glBindBufferBase(GL_UNIFORM_BUFFER, kSecondBinding, buffers[1]); + glBindBuffer(GL_UNIFORM_BUFFER, 0); + + const GLuint blockIndex = glGetUniformBlockIndex(fs, "Colour"); + ASSERT_NE(blockIndex, GL_INVALID_INDEX); + glUniformBlockBinding(fs, blockIndex, kFirstBinding); + + const GLuint pipeline = MakePipeline(); + glUseProgramStages(pipeline, GL_VERTEX_SHADER_BIT, vs); + glUseProgramStages(pipeline, GL_FRAGMENT_SHADER_BIT, fs); + + GLuint vao = 0; + glGenVertexArrays(1, &vao); + glBindVertexArray(vao); + BindDefaultFramebuffer(); + glViewport(0, 0, width, height); + glDisable(GL_SCISSOR_TEST); + glDisable(GL_DEPTH_TEST); + glUseProgram(0); + glBindProgramPipeline(pipeline); + + // Draw one: the composite is built here, against binding 2. + ClearTo(0.0f, 0.0f, 0.0f, 1.0f); + glDrawArrays(GL_TRIANGLE_STRIP, 0, 4); + const Image first = ReadPixels(width, height); + EXPECT_TRUE(RegionIsMostly(first, 2, width - 3, 2, height - 3, "red", 0.0, + "the first pipeline draw, with Colour on binding 2")); + ASSERT_EQ(FirstGLError(), 0u) << "the first uniform-block pipeline draw leaked a GL error"; + + // Move the block to the other binding point, with the composite already built and cached. + glUniformBlockBinding(fs, blockIndex, kSecondBinding); + ASSERT_EQ(FirstGLError(), 0u) << "rebinding a uniform block between draws errored"; + + // Draw two must read the OTHER buffer. + ClearTo(0.0f, 0.0f, 0.0f, 1.0f); + glDrawArrays(GL_TRIANGLE_STRIP, 0, 4); + const Image second = ReadPixels(width, height); + EXPECT_TRUE(RegionIsMostly(second, 2, width - 3, 2, height - 3, "green", 0.0, + "the second pipeline draw, after Colour was rebound to binding 5")); + EXPECT_EQ(FirstGLError(), 0u) << "the rebound uniform-block pipeline draw leaked a GL error"; + + glBindVertexArray(0); + glDeleteVertexArrays(1, &vao); + glDeleteBuffers(2, buffers); + gl.EndFrame(); + } + + // The sampler-unit half of the same question, in a loop: set a unit, draw, repeat. This is + // the shape KHR-GL42.shader_image_load_store.advanced-sso-* and the compute_shader SSO cases + // run, and the one that used to relink the composite on every single iteration. The pixels + // pin what the loop must PRODUCE; the composite-identity assertion that pins what it must + // COST lives in the MG_Test unit suite, where the object itself is reachable. + TEST_F(ProgramPipelineScenario, ASamplerUnitRewrittenBetweenDrawsKeepsPaintingTheRightTexture) { + if (!Ready()) return; + HeadlessGL& gl = Gl(); + const int width = gl.Width(); + const int height = gl.Height(); + + static const char* kSamplerFS = R"(#version 430 core +uniform sampler2D u_tex; +out vec4 o_color; +void main() { o_color = texture(u_tex, vec2(0.5)); } +)"; + const GLuint vs = MakeSeparable(GL_VERTEX_SHADER, kSeparableVS); + const GLuint fs = MakeSeparable(GL_FRAGMENT_SHADER, kSamplerFS); + if (vs == 0 || fs == 0) return; + + // One texture per unit, each a different solid colour, so the pixels say which unit the + // draw actually sampled. + constexpr int kUnits = 4; + const GLubyte colours[kUnits][4] = {{255, 0, 0, 255}, {0, 255, 0, 255}, {0, 0, 255, 255}, {255, 255, 0, 255}}; + const char* names[kUnits] = {"red", "green", "blue", "yellow"}; + GLuint textures[kUnits] = {}; + glGenTextures(kUnits, textures); + for (int unit = 0; unit < kUnits; ++unit) { + glActiveTexture(GL_TEXTURE0 + unit); + glBindTexture(GL_TEXTURE_2D, textures[unit]); + glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA8, 1, 1, 0, GL_RGBA, GL_UNSIGNED_BYTE, colours[unit]); + glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST); + glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST); + } + glActiveTexture(GL_TEXTURE0); + ASSERT_EQ(FirstGLError(), 0u) << "texture setup left a GL error behind"; + + const GLuint pipeline = MakePipeline(); + glUseProgramStages(pipeline, GL_VERTEX_SHADER_BIT, vs); + glUseProgramStages(pipeline, GL_FRAGMENT_SHADER_BIT, fs); + glBindProgramPipeline(pipeline); + glActiveShaderProgram(pipeline, fs); + const GLint sampler = glGetUniformLocation(fs, "u_tex"); + ASSERT_NE(sampler, -1); + + GLuint vao = 0; + glGenVertexArrays(1, &vao); + glBindVertexArray(vao); + BindDefaultFramebuffer(); + glViewport(0, 0, width, height); + glDisable(GL_SCISSOR_TEST); + glDisable(GL_DEPTH_TEST); + glUseProgram(0); + + for (int unit = 0; unit < kUnits; ++unit) { + glUniform1i(sampler, unit); + ClearTo(0.0f, 0.0f, 0.0f, 1.0f); + glDrawArrays(GL_TRIANGLE_STRIP, 0, 4); + const Image painted = ReadPixels(width, height); + EXPECT_TRUE(RegionIsMostly(painted, 2, width - 3, 2, height - 3, names[unit], 0.0, + "a pipeline draw after its sampler was pointed at another unit")) + << "unit " << unit; + EXPECT_EQ(FirstGLError(), 0u) << "the sampler-rewrite pipeline draw leaked a GL error at unit " << unit; + } + + glBindVertexArray(0); + glDeleteVertexArrays(1, &vao); + glDeleteTextures(kUnits, textures); + gl.EndFrame(); + } + // build-separable / build-monolithic reduce to this: a separable program and a monolithic one // must both be usable, and switching between pipeline and glUseProgram must leave no error. TEST_F(ProgramPipelineScenario, SwitchingBetweenAPipelineAndAMonolithicProgramLeavesNoError) { diff --git a/MobileGL/MG_State/GLState/Core.cpp b/MobileGL/MG_State/GLState/Core.cpp index a943f103..90508c5a 100644 --- a/MobileGL/MG_State/GLState/Core.cpp +++ b/MobileGL/MG_State/GLState/Core.cpp @@ -384,23 +384,49 @@ 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. + // WHICH uniforms: exactly the ones `source` has been WRITTEN to since its last link + // (ProgramObject's per-location dirty set), and that restriction is a correctness fix + // as much as it is the reason this is cheap. + // + // 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 + // with one slot for it, and RefreshCompositeUniforms walks the stages in order. When + // every active uniform was copied unconditionally, the LAST graphics stage that merely + // DECLARED a name won, even while holding nothing but GL's zero default, and an + // earlier stage's written value was overwritten with zeros on the way to the draw. The + // shared-header idiom - the same `uniform mat4 u_mvp` declared in the VS and the FS, + // written through glActiveShaderProgram(pipe, vs) - rendered nothing because of it. + // Copying only written uniforms makes that case, which is the overwhelmingly common + // one, simply correct: an unwritten declaration has nothing to say and says nothing. + // + // WHEN BOTH STAGES WROTE THE SAME NAME there is no single right answer available - + // GL_ARB_separate_shader_objects gives the two values separate storage and the + // composite has one slot - so the rule is LAST WRITTEN-TO GRAPHICS STAGE WINS, in + // ShaderStage enum order (Vertex .. Fragment), decided by the stage walk in + // RefreshCompositeUniforms. It is deterministic, and it is strictly better than what + // it replaces: only a stage that actually holds an application-written value can now + // take the slot. True last-WRITE-wins would need a global write ordering the dirty set + // does not carry. + // + // An unwritten uniform is not left to chance either: the composite links the same + // shader objects the stages do, so its own link seeds it with the same declared + // initializers (ApplyUniformInitialValues), which is precisely the value GL says an + // unwritten uniform reads. static void MirrorUniformValues(ProgramObject& source, ProgramObject& destination) { if (!source.GetLinkStatus() || !destination.GetLinkStatus()) return; + // O(uniforms written), not O(uniforms declared). The two name lookups below are + // string hashes into both programs' location maps, and doing them for every active + // uniform of every stage on every gate trip was hundreds of them per draw on a + // large program. A stage nothing has been written to costs one empty() test. + const Vector& writtenIndices = source.GetWrittenUniformIndices(); + if (writtenIndices.empty()) return; + const char* sourceUbo = static_cast(source.GetUBOData()); char* destinationUbo = static_cast(destination.MapUBO()); const SizeT sourceUboSize = source.GetUBOSize(); const SizeT destinationUboSize = destination.GetUBOSize(); - const Uint uniformCount = source.GetUniformCount(); - for (Uint index = 0; index < uniformCount; ++index) { + for (const Uint index : writtenIndices) { const String& name = source.GetActiveUniformName(index); if (name.empty()) continue; const Int sourceBase = source.GetUniformLocation(name); @@ -418,6 +444,10 @@ namespace MobileGL::MG_State { !destination.IsValidUniformLocation(destinationLocation)) { break; } + // Per ELEMENT, not per array: `arr[3] = x` must carry element 3 and leave + // the elements another stage owns alone. `continue`, not `break` - the + // written elements of an array need not be a prefix of it. + if (!source.IsUniformWrittenAtLocation(static_cast(sourceLocation))) continue; // Stop at the end of EITHER side's array rather than walking onto the // neighbouring uniform of whichever program has the shorter one. if (!source.UniformLocationsAliasSameUniform(sourceBase, sourceLocation) || @@ -551,13 +581,13 @@ namespace MobileGL::MG_State { if (!pipeline) return nullProgram; // P1 join site J1. ComputeDrawProgramSignature() keys the composite cache on each - // stage program's lifetimeId and backendStateVersion - NON-artifact fields, so - // they do not pass through ProgramObject's join gate and a pending link would - // stay pending right through the signature. Since the version is bumped both at - // enqueue and at publish, the signature computed inside a pending window is one - // that will never be produced again: every draw would miss the cache and rebuild - // (and relink) the composite. Join first, so the signature describes settled - // programs. In steady state this is a null check per stage. + // stage program's lifetimeId and linkVersion - NON-artifact fields, so they do not + // pass through ProgramObject's join gate and a pending link would stay pending + // right through the signature. Since the version is bumped both at enqueue and at + // publish, the signature computed inside a pending window is one that will never + // be produced again: every draw would miss the cache and rebuild (and relink) the + // composite. Join first, so the signature describes settled programs. In steady + // state this is a null check per stage. for (SizeT stage = 0; stage < ProgramPipelineObject::kGraphicsStageCount; ++stage) { const auto& stageProgram = pipeline->GetStageProgram(static_cast(stage)); if (stageProgram) stageProgram->JoinLinkAndSpirv(); diff --git a/MobileGL/MG_State/GLState/ProgramState/ProgramObject.cpp b/MobileGL/MG_State/GLState/ProgramState/ProgramObject.cpp index 0a572a3e..41f81b6f 100644 --- a/MobileGL/MG_State/GLState/ProgramState/ProgramObject.cpp +++ b/MobileGL/MG_State/GLState/ProgramState/ProgramObject.cpp @@ -326,6 +326,11 @@ namespace MobileGL::MG_State::GLState { artifacts.linkedExplicitUniformLocations.clear(); artifacts.uniformInitialValues.clear(); artifacts.uniformIndexInTProgram.clear(); + // GL resets every uniform to its initial value at link, so nothing is "written since + // link" any more - and the locations these bits index no longer mean anything either. + artifacts.writtenUniformLocationBits.clear(); + artifacts.writtenUniformIndexBits.clear(); + artifacts.writtenUniformIndices.clear(); artifacts.uniformSamplerOrImageUnitIndex.clear(); artifacts.explicitOpaqueUniformBindings.clear(); artifacts.uniformBlockIndexByName.clear(); diff --git a/MobileGL/MG_State/GLState/ProgramState/ProgramObject.h b/MobileGL/MG_State/GLState/ProgramState/ProgramObject.h index 6bc49abc..5ebed93b 100644 --- a/MobileGL/MG_State/GLState/ProgramState/ProgramObject.h +++ b/MobileGL/MG_State/GLState/ProgramState/ProgramObject.h @@ -341,6 +341,80 @@ namespace MobileGL::MG_State::GLState { return UniformStorageSpanInBytes(GetUniformTType(location), GetUniformSizesInBytes(location)); } + // ---- "written since link": the per-location dirty set the pipeline composite mirrors from ---- + // + // A pipeline's stage programs each own their uniform storage, but the composite the draw + // goes through has ONE slot per name. Mirroring every active uniform of every stage + // therefore lets the last stage that merely DECLARES a name overwrite the value an + // earlier stage was actually written with - the shared-header idiom (the same + // `uniform mat4 u_mvp` in the VS and the FS) rendered nothing because of it. Recording + // which locations an application has written is what lets the mirror carry only those. + // + // WHO PAYS: only a program that could ever be a pipeline stage, decided by the latch + // below. glUseProgram's uniform path - thousands of calls per frame in Minecraft - pays + // one predictable bool branch and nothing else. + // + // GRANULARITY is per LOCATION, not per name: glUniform*v writes array elements at + // element locations, and a program that wrote `arr[3]` and nothing else must mirror + // exactly that element. The compact index list beside it is what keeps the mirror + // O(uniforms actually written) instead of O(active uniforms) - it is the set of GL + // active-uniform indices owning at least one written location, so the mirror does its + // two name lookups once per written uniform rather than once per uniform in the program. + // + // NOT counted as a write: the declared initializers ProgramLinkTask seeds at link + // (ApplyUniformInitialValues). They are a property of the SHADERS, and the composite + // links the very same shader objects, so it seeds itself with the identical values - + // there is nothing to carry. Counting them would also re-introduce the bug this set + // exists to fix, by letting a stage that only declares `uniform float f = 0.0;` clobber + // the value the application wrote for `f` in another stage. + Bool TracksUniformWrites() const { return m_tracksUniformWrites; } + + // Records that `location` has been written since the last link. Cheap and idempotent; + // a no-op on a program that can never be a pipeline stage. + void MarkUniformWrittenAtLocation(Uint location) { + if (!m_tracksUniformWrites) return; + LinkArtifacts& artifacts = Artifacts(); + if (!IsValidUniformLocation(artifacts, static_cast(location))) return; + + const SizeT locationWord = location / 64u; + if (locationWord >= artifacts.writtenUniformLocationBits.size()) { + artifacts.writtenUniformLocationBits.resize( + static_cast(artifacts.maxUniformLocation) / 64u + 1u, 0u); + if (locationWord >= artifacts.writtenUniformLocationBits.size()) return; + } + artifacts.writtenUniformLocationBits[locationWord] |= Uint64{1} << (location % 64u); + + // Add the owning GL active-uniform index to the compact list, once. + const Int tIndex = artifacts.uniformIndexInTProgram[location]; + if (tIndex < 0 || static_cast(tIndex) >= artifacts.tProgramUniformIndexToGl.size()) return; + const Int glIndex = artifacts.tProgramUniformIndexToGl[tIndex]; + // -1 is a uniform the relaxed parse swept out of the GL-visible index space; the + // mirror enumerates GL indices, so there is nothing it could look such a one up by. + if (glIndex < 0) return; + const SizeT indexWord = static_cast(glIndex) / 64u; + if (indexWord >= artifacts.writtenUniformIndexBits.size()) { + artifacts.writtenUniformIndexBits.resize( + static_cast(artifacts.activeUniformCount) / 64u + 1u, 0u); + if (indexWord >= artifacts.writtenUniformIndexBits.size()) return; + } + const Uint64 indexBit = Uint64{1} << (static_cast(glIndex) % 64u); + if ((artifacts.writtenUniformIndexBits[indexWord] & indexBit) != 0) return; + artifacts.writtenUniformIndexBits[indexWord] |= indexBit; + artifacts.writtenUniformIndices.push_back(static_cast(glIndex)); + } + + Bool IsUniformWrittenAtLocation(Uint location) const { + const auto& bits = Artifacts().writtenUniformLocationBits; + const SizeT locationWord = location / 64u; + return locationWord < bits.size() && + (bits[locationWord] & (Uint64{1} << (location % 64u))) != 0; + } + + // GL active-uniform indices owning at least one written location. Empty for every + // program that has not been written to since its last link - and for every program + // that never asked to be separable, which is what makes the mirror free for them. + const Vector& GetWrittenUniformIndices() const { return Artifacts().writtenUniformIndices; } + Int GetAttributeLocation(const String& name) { const auto it = std::find(Artifacts().attribs.begin(), Artifacts().attribs.end(), name); return (it == Artifacts().attribs.end()) ? -1 : (Int)std::distance(Artifacts().attribs.begin(), it); @@ -488,10 +562,15 @@ namespace MobileGL::MG_State::GLState { } void SetUniformSamplerOrImageUnitIndex(Uint location, Int unit) { - if (location >= Artifacts().uniformSamplerOrImageUnitIndex.size() || - Artifacts().uniformSamplerOrImageUnitIndex[location] == unit) { - return; - } + if (location >= Artifacts().uniformSamplerOrImageUnitIndex.size()) return; + // BEFORE the equality bail-out, not after: "written" is about the application + // having addressed the uniform, not about the bytes changing. glUniform1i(s, 0) on + // a sampler that already reads 0 still has to beat another stage's untouched + // declaration of the same name in the composite - which is only possible if the + // write is recorded. (The mirror is the only reader, and it runs this same setter + // on the composite, where the latch is off.) + MarkUniformWrittenAtLocation(location); + if (Artifacts().uniformSamplerOrImageUnitIndex[location] == unit) return; Artifacts().uniformSamplerOrImageUnitIndex[location] = unit; ++m_backendStateVersion; } @@ -511,7 +590,32 @@ namespace MobileGL::MG_State::GLState { // subset of the stages of a program pipeline. Only takes effect on the next link, // which is why it is plain state here rather than something Link() consults. Bool GetSeparable() const { return m_separable; } - void SetSeparable(Bool separable) { m_separable = separable; } + void SetSeparable(Bool separable) { + m_separable = separable; + // ---- arming the uniform-write tracking latch ---- + // + // The predicate wanted is "this program can ever be a pipeline stage", and + // GetSeparable() is NOT it in either direction. GL_PROGRAM_SEPARABLE takes effect + // at the NEXT link, so it can read true on a program glUseProgramStages would + // still reject; that direction is merely wasteful. The other direction is a + // correctness hole: glProgramParameteri may clear the flag AFTER a separable link, + // and glUseProgramStages tests the state the program was LINKED with, so such a + // program is still a legal stage while GetSeparable() reads false. Tracking driven + // by the live flag would stop recording writes on a program the composite is still + // mirroring from, and those uniforms would silently stop reaching the draw. + // + // "Attached to a pipeline" is not usable either, and for a more basic reason: + // glProgramUniform* legitimately runs before glUseProgramStages, so the marks have + // to already exist by the time the program becomes a stage. + // + // So: a MONOTONE latch, armed the first time GL_PROGRAM_SEPARABLE is requested + // true and never cleared. It over-approximates - a program that was separable once + // keeps paying the bookkeeping - and over-approximating only ever costs a bitset, + // never a wrong value. glCreateShaderProgramv arms it through this same setter. + // A program that never asks (every monolithic glUseProgram program, which is the + // hot uniform path) never arms it and pays one bool branch per glUniform*. + if (separable) m_tracksUniformWrites = true; + } // glProgramBinary always fails here (there is no format it could accept) and the // spec then requires the program's LINK_STATUS to read FALSE. void MarkLinkFailedByProgramBinary() { @@ -764,6 +868,16 @@ namespace MobileGL::MG_State::GLState { // Applied into the uniform shadow at the phase-B publish (ApplyUniformInitialValues). Vector uniformInitialValues; UnorderedMap uniformLocations; + // ---- "written since link" (see MarkUniformWrittenAtLocation) ---- + // In LinkArtifacts deliberately: a link is exactly the event that retracts every + // write (GL resets uniforms to their initial values), so living here means the set + // is cleared by the same three paths that clear the rest of a link's output - + // Link()'s whole-struct reset, ResetLinkArtifacts, and the publish's move - and no + // fourth reset site can be forgotten. Empty (and never allocated) for a program + // that never asked to be separable. + Vector writtenUniformLocationBits; + Vector writtenUniformIndexBits; + Vector writtenUniformIndices; // Ordered by location, // aka. uniformIndexInTProgram[loc] == "uniform index of TProgram at location `loc`" Vector uniformIndexInTProgram; @@ -1079,6 +1193,10 @@ namespace MobileGL::MG_State::GLState { Bool m_deleteStatus = false; Bool m_binaryRetrievableHint = false; Bool m_separable = false; + // Monotone "this program may ever be a pipeline stage" latch; see SetSeparable for why + // it is a latch and not just m_separable. Outside LinkArtifacts on purpose: a relink + // clears the write SET, but a program that was separable is still separable after it. + Bool m_tracksUniformWrites = false; Bool m_validateStatus = true; // Mutable, like m_artifacts and for the same reason: publishing a pending link is a // READ-side operation (the first gated getter is what pulls the result in), and the diff --git a/MobileGL/MG_State/GLState/ProgramState/ProgramPipelineObject.h b/MobileGL/MG_State/GLState/ProgramState/ProgramPipelineObject.h index 5856a0f1..491419de 100644 --- a/MobileGL/MG_State/GLState/ProgramState/ProgramPipelineObject.h +++ b/MobileGL/MG_State/GLState/ProgramState/ProgramPipelineObject.h @@ -67,20 +67,30 @@ 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. + // object and its LINK version pins the link generation. 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. // - // 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. + // GetLinkVersion() and NOT GetBackendStateVersion(), which is what this used to + // key on. The backend state version moves on every glUniform1i to a sampler and + // every glUniformBlockBinding, so the "set a sampler unit, draw" loop that the + // SSO conformance cases run threw the composite away and REBUILT it on every + // single draw: a fresh ProgramObject, a full Link(true) settled synchronously + // (glslang + SPIR-V + spirv-opt), a full re-mirror, and a brand-new program + // identity that invalidated both backends' per-program registries and pipeline + // memos along the way. The composite's CONTENT depends on the link generations + // and nothing else, and m_linkVersion is bumped by exactly those + // (BumpLinkObservableVersions). + // + // The prerequisite that makes the narrowing legal: because the composite no + // longer rebuilds when per-program uniform STATE changes, every such change must + // reach it through the refresh below instead. Both do - sampler/image units via + // MirrorUniformValues, interface block bindings via MirrorBlockBindings - and + // the two setters that write them still bump the counters the REFRESH gate reads + // (see ComputeUniformMirrorVersions), which is a separate question from what + // this signature reads. They are the only two writers of m_backendStateVersion + // outside the link paths, so nothing else was ever riding on the rebuild. using DrawProgramSignature = Array; DrawProgramSignature ComputeDrawProgramSignature() const { @@ -89,7 +99,7 @@ namespace MobileGL { const auto& program = m_stagePrograms[stage]; if (!program) continue; signature[stage * 2] = program->GetLifetimeId(); - signature[stage * 2 + 1] = program->GetBackendStateVersion(); + signature[stage * 2 + 1] = program->GetLinkVersion(); } return signature; } @@ -102,6 +112,14 @@ namespace MobileGL { // the per-stage versions "needs it" is measured against. All zero after a // rebuild, because a fresh composite holds only what its shaders declared and // so needs a full refresh. + // + // backendStateVersion belongs HERE even though ComputeDrawProgramSignature no + // longer reads it, and that is the whole point of the split: a sampler-unit or + // uniform-block-binding write must still trip the MIRROR (it is now the only + // route those values have to the composite) while deliberately NOT tripping the + // rebuild. uboContentVersion covers ordinary uniform writes, and + // blockBindingVersion covers the storage-block setter, which moves neither of + // the other two. using UniformMirrorVersions = Array; UniformMirrorVersions ComputeUniformMirrorVersions() const { diff --git a/MobileGL/MG_Test/Program/CMakeLists.txt b/MobileGL/MG_Test/Program/CMakeLists.txt index 808e53c5..7f29b1c5 100644 --- a/MobileGL/MG_Test/Program/CMakeLists.txt +++ b/MobileGL/MG_Test/Program/CMakeLists.txt @@ -180,6 +180,22 @@ target_link_libraries( ${LINK_LIBRARIES} ) +add_executable( + ProgramPipelineCompositeTest + ProgramPipelineCompositeTest.cpp +) + +target_include_directories(ProgramPipelineCompositeTest PRIVATE + ${MGL_ROOT}/include + ${MGL_ROOT}/MobileGL +) + +target_link_libraries( + ProgramPipelineCompositeTest PRIVATE + GTest::gtest_main + ${LINK_LIBRARIES} +) + add_executable( ProgramInterfaceTest ProgramInterfaceTest.cpp @@ -211,6 +227,7 @@ include(GoogleTest) gtest_discover_tests(ProgramUtilTest DISCOVERY_TIMEOUT 30 PROPERTIES LABELS unit) gtest_discover_tests(ProgramTest DISCOVERY_TIMEOUT 30 PROPERTIES LABELS unit) gtest_discover_tests(ProgramInterfaceTest DISCOVERY_TIMEOUT 30 PROPERTIES LABELS unit) +gtest_discover_tests(ProgramPipelineCompositeTest DISCOVERY_TIMEOUT 30 PROPERTIES LABELS unit) gtest_discover_tests(XfbBlockVaryingTest DISCOVERY_TIMEOUT 30 PROPERTIES LABELS unit) # Heavier than the rest of the unit suite by design: several cases deliberately saturate the # compile pool so there is something in flight to race against. diff --git a/MobileGL/MG_Test/Program/ProgramPipelineCompositeTest.cpp b/MobileGL/MG_Test/Program/ProgramPipelineCompositeTest.cpp new file mode 100644 index 00000000..c7a979a7 --- /dev/null +++ b/MobileGL/MG_Test/Program/ProgramPipelineCompositeTest.cpp @@ -0,0 +1,424 @@ +// MobileGL - MobileGL/MG_Test/Program/ProgramPipelineCompositeTest.cpp +// Copyright (c) 2025-2026 MobileGL-Dev +// Licensed under the GNU Lesser General Public License v3.0: +// https://www.gnu.org/licenses/gpl-3.0.txt +// https://www.gnu.org/licenses/lgpl-3.0.txt +// SPDX-License-Identifier: LGPL-3.0-only +// End of Source File Header + +// The hidden composite program a pipeline draw goes through (MG_State/GLState/Core.cpp, +// GetProgramForDraw), interrogated directly rather than through pixels. +// +// Two properties live here that the integration scenarios cannot see, because both are about +// the composite as an OBJECT rather than about what it paints: +// +// 1. WHICH stage's uniform value ends up in its single slot when several stages declare the +// same name. The rendering cases pin the answer for the shapes an application actually +// writes; these pin the rule itself, including the tie. +// 2. WHETHER it is the same object from one draw to the next. A composite rebuild is a full +// synchronous Link() plus a new program identity that empties both backends' per-program +// registries, and nothing about the resulting IMAGE would change if it happened on every +// draw - so an assertion on pixels can never catch that regression. + +#include + +#include +#include +#include + +#include "Config.h" +#include "Includes.h" +#include "Init.h" +#include "MG_Impl/GLImpl/Getter/GL_Getter.h" +#include "MG_Impl/GLImpl/Program/GL_Program.h" +#include "MG_Impl/GLImpl/Program/GL_ProgramPipeline.h" +#include "MG_State/GLState/Core.h" + +using namespace MobileGL; +using namespace MobileGL::MG_Impl::GLImpl; + +namespace { + + // Both stages declare `u_shared`, which is the shared-header idiom (one header included by + // every stage) and the shape that used to render nothing: the fragment stage's untouched + // zero default overwrote the vertex stage's written value on the way into the composite. + const char* kSharedUniformVs = R"(#version 430 core +out gl_PerVertex { vec4 gl_Position; }; +uniform vec4 u_shared; +uniform vec4 u_vsOnly; +void main() { gl_Position = u_shared + u_vsOnly; } +)"; + + const char* kSharedUniformFs = R"(#version 430 core +uniform vec4 u_shared; +out vec4 o_color; +void main() { o_color = u_shared; } +)"; + + const char* kArrayUniformVs = R"(#version 430 core +out gl_PerVertex { vec4 gl_Position; }; +uniform vec4 u_arr[4]; +void main() { gl_Position = u_arr[0] + u_arr[1] + u_arr[2] + u_arr[3]; } +)"; + + const char* kArrayUniformFs = R"(#version 430 core +uniform vec4 u_arr[4]; +out vec4 o_color; +void main() { o_color = u_arr[0] + u_arr[1] + u_arr[2] + u_arr[3]; } +)"; + + const char* kSamplerVs = R"(#version 430 core +out gl_PerVertex { vec4 gl_Position; }; +void main() { gl_Position = vec4(0.0, 0.0, 0.0, 1.0); } +)"; + + const char* kSamplerFs = R"(#version 430 core +uniform sampler2D u_tex; +out vec4 o_color; +void main() { o_color = texture(u_tex, vec2(0.0)); } +)"; + + class ProgramPipelineCompositeTest : public ::testing::Test { + protected: + void SetUp() override { MobileGL::Initialize(); } + + // Built by hand rather than through glCreateShaderProgramv, for the reason AsyncLinkTest + // gives: that entry point detaches the shader right after linking, so a relink would + // leave the stage program with nothing to composite from - and one of the cases below + // relinks on purpose. + GLuint MakeSeparableProgram(const GLenum stage, const char* source) { + const GLuint shader = CreateShader(stage); + ShaderSource(shader, 1, &source, nullptr); + CompileShader(shader); + const GLuint program = CreateProgram(); + ProgramParameteri(program, GL_PROGRAM_SEPARABLE, GL_TRUE); + AttachShader(program, shader); + LinkProgram(program); + GLint linked = GL_FALSE; + GetProgramiv(program, GL_LINK_STATUS, &linked); + EXPECT_EQ(linked, GL_TRUE) << "separable stage program did not link"; + return program; + } + + // The composite the next draw would run, settled. + static SharedPtr DrawProgram() { + return MG_State::pGLContext->GetProgramForDraw(); + } + + // A uniform's value read out of a program's own shadow, by name. This is what the draw + // would upload, which is the thing under test - glGetUniform* would answer the same for + // the STAGE programs but has no way to name the composite at all. + static std::vector ReadVec4(MG_State::GLState::ProgramObject& program, const String& name) { + const Int location = program.GetUniformLocation(name); + if (location < 0) return {}; + const Uint offset = program.GetUniformOffset(static_cast(location)); + const auto* ubo = static_cast(program.GetUBOData()); + if (ubo == nullptr || offset == MG_State::GLState::ProgramObject::kInvalidUniformOffset || + offset + 4 * sizeof(float) > program.GetUBOSize()) { + return {}; + } + std::vector value(4); + std::memcpy(value.data(), ubo + offset, 4 * sizeof(float)); + return value; + } + }; + +} // namespace + +// --------------------------------------------------------------------------------------- +// Which stage wins the composite's single slot +// --------------------------------------------------------------------------------------- + +// THE defect. Both stages declare `u_shared`; only the VERTEX program is ever written to. +// Walking the stages in order and copying every active uniform unconditionally meant the +// fragment stage's untouched zero default landed last and won, so the composite drew zeros - a +// whole frame of nothing, from a program that had been set up entirely correctly. +TEST_F(ProgramPipelineCompositeTest, AWrittenStageValueIsNotClobberedByAnotherStagesUntouchedDeclaration) { + const GLuint vs = MakeSeparableProgram(GL_VERTEX_SHADER, kSharedUniformVs); + const GLuint fs = MakeSeparableProgram(GL_FRAGMENT_SHADER, kSharedUniformFs); + + GLuint pipeline = 0; + GenProgramPipelines(1, &pipeline); + BindProgramPipeline(pipeline); + UseProgramStages(pipeline, GL_VERTEX_SHADER_BIT, vs); + UseProgramStages(pipeline, GL_FRAGMENT_SHADER_BIT, fs); + ASSERT_EQ(GetError(), GL_NO_ERROR); + + // Exactly what an application does: point glUniform* at the vertex stage and write there. + // The fragment program is never written to and holds nothing but GL's zero default. + ActiveShaderProgram(pipeline, vs); + const GLint location = GetUniformLocation(vs, "u_shared"); + ASSERT_GE(location, 0); + const float written[4] = {0.25f, 0.5f, 0.75f, 1.0f}; + Uniform4fv(location, 1, written); + ASSERT_EQ(GetError(), GL_NO_ERROR); + + const auto composite = DrawProgram(); + ASSERT_NE(composite, nullptr); + const std::vector value = ReadVec4(*composite, "u_shared"); + ASSERT_EQ(value.size(), 4u) << "u_shared has no backing storage in the composite"; + EXPECT_EQ(value, (std::vector{0.25f, 0.5f, 0.75f, 1.0f})) + << "the fragment stage's untouched declaration overwrote the vertex stage's written value"; + + // The uniform only one stage declares is unaffected either way; it is here so a mirror that + // copied nothing at all would not pass this case by accident. + ActiveShaderProgram(pipeline, vs); + const GLint vsOnly = GetUniformLocation(vs, "u_vsOnly"); + ASSERT_GE(vsOnly, 0); + const float other[4] = {1.0f, 2.0f, 3.0f, 4.0f}; + Uniform4fv(vsOnly, 1, other); + const auto refreshed = DrawProgram(); + EXPECT_EQ(ReadVec4(*refreshed, "u_vsOnly"), (std::vector{1.0f, 2.0f, 3.0f, 4.0f})); + EXPECT_EQ(GetError(), GL_NO_ERROR); + + BindProgramPipeline(0); + DeleteProgramPipelines(1, &pipeline); +} + +// The tie the fix cannot make disappear: BOTH stages were written, and the composite still has +// one slot. The documented rule is last WRITTEN-TO graphics stage wins, in ShaderStage enum +// order - deterministic, and reachable only by a stage holding a real application value. +TEST_F(ProgramPipelineCompositeTest, WhenBothStagesWereWrittenTheLastGraphicsStageWins) { + const GLuint vs = MakeSeparableProgram(GL_VERTEX_SHADER, kSharedUniformVs); + const GLuint fs = MakeSeparableProgram(GL_FRAGMENT_SHADER, kSharedUniformFs); + + GLuint pipeline = 0; + GenProgramPipelines(1, &pipeline); + BindProgramPipeline(pipeline); + UseProgramStages(pipeline, GL_VERTEX_SHADER_BIT, vs); + UseProgramStages(pipeline, GL_FRAGMENT_SHADER_BIT, fs); + + const float fromVs[4] = {1.0f, 1.0f, 1.0f, 1.0f}; + const float fromFs[4] = {2.0f, 2.0f, 2.0f, 2.0f}; + // Written in the order VS then FS... + ProgramUniform4fv(vs, GetUniformLocation(vs, "u_shared"), 1, fromVs); + ProgramUniform4fv(fs, GetUniformLocation(fs, "u_shared"), 1, fromFs); + ASSERT_EQ(GetError(), GL_NO_ERROR); + EXPECT_EQ(ReadVec4(*DrawProgram(), "u_shared"), (std::vector{2.0f, 2.0f, 2.0f, 2.0f})); + + // ...and in the order FS then VS. The answer is the same, because the rule is stage order + // and not write order - which is the honest statement of what the dirty set can support. + ProgramUniform4fv(fs, GetUniformLocation(fs, "u_shared"), 1, fromFs); + ProgramUniform4fv(vs, GetUniformLocation(vs, "u_shared"), 1, fromVs); + ASSERT_EQ(GetError(), GL_NO_ERROR); + EXPECT_EQ(ReadVec4(*DrawProgram(), "u_shared"), (std::vector{2.0f, 2.0f, 2.0f, 2.0f})) + << "the both-written tie must be decided by stage order, deterministically"; + + BindProgramPipeline(0); + DeleteProgramPipelines(1, &pipeline); +} + +// glProgramUniform* addresses a program by NAME and needs neither a current program nor an +// active shader program, so it is a write path that never touches the pipeline at all. It has +// to record the write exactly like glUniform* does. +TEST_F(ProgramPipelineCompositeTest, ProgramUniformOnAnUnboundStageProgramReachesTheComposite) { + const GLuint vs = MakeSeparableProgram(GL_VERTEX_SHADER, kSharedUniformVs); + const GLuint fs = MakeSeparableProgram(GL_FRAGMENT_SHADER, kSharedUniformFs); + + GLuint pipeline = 0; + GenProgramPipelines(1, &pipeline); + UseProgramStages(pipeline, GL_VERTEX_SHADER_BIT, vs); + UseProgramStages(pipeline, GL_FRAGMENT_SHADER_BIT, fs); + + // Deliberately BEFORE the bind, and with no glActiveShaderProgram anywhere: the write has + // to survive from here to a draw that has not been set up yet. + const float written[4] = {9.0f, 8.0f, 7.0f, 6.0f}; + ProgramUniform4fv(vs, GetUniformLocation(vs, "u_vsOnly"), 1, written); + ASSERT_EQ(GetError(), GL_NO_ERROR); + + BindProgramPipeline(pipeline); + EXPECT_EQ(ReadVec4(*DrawProgram(), "u_vsOnly"), (std::vector{9.0f, 8.0f, 7.0f, 6.0f})); + EXPECT_EQ(GetError(), GL_NO_ERROR); + + BindProgramPipeline(0); + DeleteProgramPipelines(1, &pipeline); +} + +// Array uniforms are written at ELEMENT locations, so the record has to be per location and not +// per name: a stage that wrote `u_arr[2]` and nothing else must carry element 2 across and +// leave the rest to whichever stage owns them. +TEST_F(ProgramPipelineCompositeTest, ArrayElementWritesMirrorPerElement) { + const GLuint vs = MakeSeparableProgram(GL_VERTEX_SHADER, kArrayUniformVs); + const GLuint fs = MakeSeparableProgram(GL_FRAGMENT_SHADER, kArrayUniformFs); + + GLuint pipeline = 0; + GenProgramPipelines(1, &pipeline); + BindProgramPipeline(pipeline); + UseProgramStages(pipeline, GL_VERTEX_SHADER_BIT, vs); + UseProgramStages(pipeline, GL_FRAGMENT_SHADER_BIT, fs); + + // Non-prefix on purpose: elements 1 and 3 from the vertex stage, element 2 from the fragment + // stage, element 0 from nobody. A per-name record would have carried whole arrays and let + // one stage's zeros take the other's elements. + const float one[4] = {11.0f, 11.0f, 11.0f, 11.0f}; + const float three[4] = {33.0f, 33.0f, 33.0f, 33.0f}; + const float two[4] = {22.0f, 22.0f, 22.0f, 22.0f}; + ProgramUniform4fv(vs, GetUniformLocation(vs, "u_arr[1]"), 1, one); + ProgramUniform4fv(vs, GetUniformLocation(vs, "u_arr[3]"), 1, three); + ProgramUniform4fv(fs, GetUniformLocation(fs, "u_arr[2]"), 1, two); + ASSERT_EQ(GetError(), GL_NO_ERROR); + + const auto composite = DrawProgram(); + ASSERT_NE(composite, nullptr); + EXPECT_EQ(ReadVec4(*composite, "u_arr[0]"), (std::vector{0.0f, 0.0f, 0.0f, 0.0f})); + EXPECT_EQ(ReadVec4(*composite, "u_arr[1]"), (std::vector{11.0f, 11.0f, 11.0f, 11.0f})); + EXPECT_EQ(ReadVec4(*composite, "u_arr[2]"), (std::vector{22.0f, 22.0f, 22.0f, 22.0f})); + EXPECT_EQ(ReadVec4(*composite, "u_arr[3]"), (std::vector{33.0f, 33.0f, 33.0f, 33.0f})); + EXPECT_EQ(GetError(), GL_NO_ERROR); + + // A multi-element glUniform*v run marks each location it actually reaches. + const float tail[8] = {44.0f, 44.0f, 44.0f, 44.0f, 55.0f, 55.0f, 55.0f, 55.0f}; + ActiveShaderProgram(pipeline, fs); + Uniform4fv(GetUniformLocation(fs, "u_arr[2]"), 2, tail); + ASSERT_EQ(GetError(), GL_NO_ERROR); + const auto refreshed = DrawProgram(); + EXPECT_EQ(ReadVec4(*refreshed, "u_arr[2]"), (std::vector{44.0f, 44.0f, 44.0f, 44.0f})); + EXPECT_EQ(ReadVec4(*refreshed, "u_arr[3]"), (std::vector{55.0f, 55.0f, 55.0f, 55.0f})) + << "the second element of a count=2 write was never recorded"; + + BindProgramPipeline(0); + DeleteProgramPipelines(1, &pipeline); +} + +// Relinking resets a program's uniforms to their initial values (GL 4.6 core 7.6), so the record +// of what was written has to be reset with them. If it survived, the composite built after the +// relink would be handed values the stage program no longer holds. +TEST_F(ProgramPipelineCompositeTest, RelinkingAStageProgramClearsWhatItHadWritten) { + const GLuint vs = MakeSeparableProgram(GL_VERTEX_SHADER, kSharedUniformVs); + const GLuint fs = MakeSeparableProgram(GL_FRAGMENT_SHADER, kSharedUniformFs); + + GLuint pipeline = 0; + GenProgramPipelines(1, &pipeline); + BindProgramPipeline(pipeline); + UseProgramStages(pipeline, GL_VERTEX_SHADER_BIT, vs); + UseProgramStages(pipeline, GL_FRAGMENT_SHADER_BIT, fs); + + const float written[4] = {5.0f, 6.0f, 7.0f, 8.0f}; + ProgramUniform4fv(vs, GetUniformLocation(vs, "u_vsOnly"), 1, written); + ASSERT_EQ(ReadVec4(*DrawProgram(), "u_vsOnly"), (std::vector{5.0f, 6.0f, 7.0f, 8.0f})); + + LinkProgram(vs); + GLint linked = GL_FALSE; + GetProgramiv(vs, GL_LINK_STATUS, &linked); + ASSERT_EQ(linked, GL_TRUE); + + const auto composite = DrawProgram(); + ASSERT_NE(composite, nullptr); + EXPECT_EQ(ReadVec4(*composite, "u_vsOnly"), (std::vector{0.0f, 0.0f, 0.0f, 0.0f})) + << "a relinked stage program carried its pre-relink value into the new composite"; + EXPECT_EQ(GetError(), GL_NO_ERROR); + + // ...and writing again after the relink is recorded afresh. + const float rewritten[4] = {1.5f, 2.5f, 3.5f, 4.5f}; + ProgramUniform4fv(vs, GetUniformLocation(vs, "u_vsOnly"), 1, rewritten); + EXPECT_EQ(ReadVec4(*DrawProgram(), "u_vsOnly"), (std::vector{1.5f, 2.5f, 3.5f, 4.5f})); + EXPECT_EQ(GetError(), GL_NO_ERROR); + + BindProgramPipeline(0); + DeleteProgramPipelines(1, &pipeline); +} + +// --------------------------------------------------------------------------------------- +// Composite cache stability +// --------------------------------------------------------------------------------------- + +// The SSO-conformance shape, and the reason the composite cache stopped being keyed on the +// backend state version: pick a stage program, then per draw set a sampler unit and draw. +// glUniform1i on a sampler bumps that version, so the signature changed on every iteration and +// every single draw threw the composite away and relinked it - glslang, SPIR-V and spirv-opt, +// synchronously, inside the draw - handing the backends a brand-new program identity each time. +// +// Asserted on the composite POINTER, which is the honest observable: it is the object both +// backends key their per-program registries and pipeline memos on, so "same pointer" is exactly +// the property that was lost. +TEST_F(ProgramPipelineCompositeTest, ASamplerWritePerDrawDoesNotRebuildTheComposite) { + const GLuint vs = MakeSeparableProgram(GL_VERTEX_SHADER, kSamplerVs); + const GLuint fs = MakeSeparableProgram(GL_FRAGMENT_SHADER, kSamplerFs); + + GLuint pipeline = 0; + GenProgramPipelines(1, &pipeline); + BindProgramPipeline(pipeline); + UseProgramStages(pipeline, GL_VERTEX_SHADER_BIT, vs); + UseProgramStages(pipeline, GL_FRAGMENT_SHADER_BIT, fs); + ActiveShaderProgram(pipeline, fs); + ASSERT_EQ(GetError(), GL_NO_ERROR); + + const GLint sampler = GetUniformLocation(fs, "u_tex"); + ASSERT_GE(sampler, 0); + + const auto first = DrawProgram(); + ASSERT_NE(first, nullptr); + const Uint64 firstLifetime = first->GetLifetimeId(); + const Int compositeSampler = first->GetUniformLocation("u_tex"); + ASSERT_GE(compositeSampler, 0); + + for (GLint unit = 0; unit < 8; ++unit) { + Uniform1i(sampler, unit); + const auto composite = DrawProgram(); + ASSERT_NE(composite, nullptr); + EXPECT_EQ(composite.get(), first.get()) + << "the composite was rebuilt by a sampler-unit write at unit " << unit; + EXPECT_EQ(composite->GetLifetimeId(), firstLifetime) << "the composite's identity changed at unit " << unit; + // The value still has to ARRIVE - the whole point is that the mirror carries it now that + // the rebuild no longer does. + EXPECT_EQ(composite->GetUniformSamplerOrImageUnitIndex(static_cast(compositeSampler)), unit) + << "the sampler unit did not reach the composite at unit " << unit; + } + EXPECT_EQ(GetError(), GL_NO_ERROR); + + // A relink, by contrast, MUST replace it: that is the one thing the signature still tracks. + LinkProgram(fs); + GLint linked = GL_FALSE; + GetProgramiv(fs, GL_LINK_STATUS, &linked); + ASSERT_EQ(linked, GL_TRUE); + const auto afterRelink = DrawProgram(); + ASSERT_NE(afterRelink, nullptr); + EXPECT_NE(afterRelink.get(), first.get()) << "a relinked stage program must rebuild the composite"; + + BindProgramPipeline(0); + DeleteProgramPipelines(1, &pipeline); +} + +// The monolithic path must be untouched by any of this: a plain glUseProgram program is not +// separable, records nothing, and is its own draw program. +TEST_F(ProgramPipelineCompositeTest, AMonolithicProgramRecordsNothingAndIsItsOwnDrawProgram) { + const char* vsSource = R"(#version 430 core +uniform vec4 u_shared; +void main() { gl_Position = u_shared; } +)"; + const char* fsSource = R"(#version 430 core +uniform vec4 u_shared; +out vec4 o_color; +void main() { o_color = u_shared; } +)"; + const GLuint vsShader = CreateShader(GL_VERTEX_SHADER); + ShaderSource(vsShader, 1, &vsSource, nullptr); + CompileShader(vsShader); + const GLuint fsShader = CreateShader(GL_FRAGMENT_SHADER); + ShaderSource(fsShader, 1, &fsSource, nullptr); + CompileShader(fsShader); + + const GLuint program = CreateProgram(); + AttachShader(program, vsShader); + AttachShader(program, fsShader); + LinkProgram(program); + GLint linked = GL_FALSE; + GetProgramiv(program, GL_LINK_STATUS, &linked); + ASSERT_EQ(linked, GL_TRUE); + + UseProgram(program); + const float written[4] = {1.0f, 2.0f, 3.0f, 4.0f}; + Uniform4fv(GetUniformLocation(program, "u_shared"), 1, written); + ASSERT_EQ(GetError(), GL_NO_ERROR); + + const auto drawProgram = DrawProgram(); + ASSERT_NE(drawProgram, nullptr); + EXPECT_EQ(drawProgram->GetExternalIndex(), program) << "a current program IS the draw program"; + // Nothing was recorded, because nothing ever asked this program to be separable - which is + // what keeps the hot uniform path free of the bookkeeping. + EXPECT_FALSE(drawProgram->TracksUniformWrites()); + EXPECT_TRUE(drawProgram->GetWrittenUniformIndices().empty()); + EXPECT_EQ(ReadVec4(*drawProgram, "u_shared"), (std::vector{1.0f, 2.0f, 3.0f, 4.0f})); + + UseProgram(0); +}