From 07a0408a28497a9d15f7c480d6e55b36646bab95 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Thu, 27 Aug 2026 01:47:02 -0400 Subject: [PATCH] [Fix] (ShaderTranspiler): lower gl_NumSamples onto a reserved global-UBO uniform, restore ES preamble extension macros, tolerate a repeated #version --- .../MG_Impl/GLImpl/Drawing/GL_Drawing.cpp | 83 ++++-- MobileGL/MG_Impl/GLImpl/Getter/GL_Getter.cpp | 38 +-- MobileGL/MG_Impl/GLImpl/Getter/GL_Getter.h | 9 + .../GLState/ProgramState/ProgramLinkTask.cpp | 14 + .../GLState/ProgramState/ProgramObject.h | 40 +++ .../GLState/ProgramState/ProgramSpirvTask.cpp | 13 + .../ShaderSourceProcessor.cpp | 242 +++++++++++++++++- MobileGL/MG_Util/ShaderTranspiler/Types.h | 14 + 8 files changed, 396 insertions(+), 57 deletions(-) diff --git a/MobileGL/MG_Impl/GLImpl/Drawing/GL_Drawing.cpp b/MobileGL/MG_Impl/GLImpl/Drawing/GL_Drawing.cpp index 5f1f6abb..69548d90 100644 --- a/MobileGL/MG_Impl/GLImpl/Drawing/GL_Drawing.cpp +++ b/MobileGL/MG_Impl/GLImpl/Drawing/GL_Drawing.cpp @@ -34,8 +34,12 @@ namespace MobileGL::MG_Impl::GLImpl { return true; } - static Bool ValidateCurrentProgramForExecution(const char* functionName) { - const auto& currentProgram = MG_State::pGLContext->GetProgramForDraw(); + // Takes the ALREADY-RESOLVED draw program rather than looking it up: GLContext::GetProgramForDraw + // is not a plain getter (it settles the program's link and SPIR-V jobs so every version a + // backend samples during this draw describes the program it is drawing), so the draw funnel + // below resolves it exactly once and hands it to both users. + static Bool ValidateResolvedProgramForDraw(const SharedPtr& currentProgram, + const char* functionName) { if (!ValidateProgramForExecution(currentProgram, functionName)) return false; // GL 4.6 core 7.4.1, the pipeline validation rule every vertex-transferring command @@ -52,8 +56,8 @@ namespace MobileGL::MG_Impl::GLImpl { // deliberately NOT rejected: the rule above names the three pre-rasterization stages, and // nothing else here should start refusing draws GL accepts. // - // Only ValidateCurrentProgramForExecution, never ValidateProgramForExecution itself, so a - // dispatch - which shares that helper and legitimately has no vertex stage - is untouched. + // On the DRAW path only, never in ValidateProgramForExecution itself, so a dispatch - + // which shares that helper and legitimately has no vertex stage - is untouched. const Bool hasPreRasterizationStage = currentProgram->HasLinkedShaderStage(ShaderStage::Geometry) || currentProgram->HasLinkedShaderStage(ShaderStage::TessControl) || currentProgram->HasLinkedShaderStage(ShaderStage::TessEval); @@ -69,6 +73,35 @@ namespace MobileGL::MG_Impl::GLImpl { return true; } + // gl_NumSamples has no SPIR-V built-in, so the source pipeline lowers it onto a reserved + // default-block uniform (see InjectNumSamplesBuiltinShim). This is where that uniform is paid + // for: the value is a property of the DRAW FRAMEBUFFER, not of the program, so one program + // drawn into a 4x target and then into the default framebuffer must see 4 and then 1 - which + // rules out baking it at link time. + // + // Per draw rather than on framebuffer changes because the pair (program, framebuffer) is what + // decides the value and either half can move between draws. It costs a phase-A flag read for + // every program that has no shim, and a 4-byte compare for the ones that do: the write only + // bumps the UBO content version when the number actually changes, so a run of draws into one + // framebuffer re-uploads nothing. + static void PublishDrawFramebufferSampleCount(const SharedPtr& program) { + if (!program || !program->UsesReservedNumSamples()) return; + // GL 4.6 core 15.2.2: gl_NumSamples is the number of samples in the framebuffer, or ONE + // when the target is not multisampled - where glGetIntegerv(GL_SAMPLES) answers zero. + program->WriteReservedNumSamples(static_cast(std::max(ResolveDrawFramebufferSampleCount(), 1))); + } + + // The one funnel every drawing command passes through. Order is load-bearing: validate first + // (a rejected draw must leave state alone), then publish the sample count - which reads the + // DRAW FRAMEBUFFER binding, so it has to run after the caller's framebuffer state is settled + // and before the backend consumes the program's UBO content version. + static Bool PrepareCurrentProgramForDraw(const char* functionName) { + const auto& currentProgram = MG_State::pGLContext->GetProgramForDraw(); + if (!ValidateResolvedProgramForDraw(currentProgram, functionName)) return false; + PublishDrawFramebufferSampleCount(currentProgram); + return true; + } + // A dispatch resolves its program through the DISPATCH accessor: with a pipeline bound // that is the pipeline's compute stage program, not the graphics composite a draw would // build - which no longer contains a compute stage to find at all. @@ -828,14 +861,14 @@ namespace MobileGL::MG_Impl::GLImpl { void MultiDrawElementsIndirect(GLenum mode, GLenum type, const void* indirect, GLsizei drawcount, GLsizei stride) { if (!ValidatePrimitiveModeEnum(__func__, mode)) return; - if (!ValidateCurrentProgramForExecution(__func__)) return; + if (!PrepareCurrentProgramForDraw(__func__)) return; if (!ValidatePrimitiveModeForBackend(__func__, mode)) return; MultiDrawElementsIndirect_Backend(mode, type, indirect, drawcount, stride); } void MultiDrawArraysIndirect(GLenum mode, const void* indirect, GLsizei drawcount, GLsizei stride) { if (!ValidatePrimitiveModeEnum(__func__, mode)) return; - if (!ValidateCurrentProgramForExecution(__func__)) return; + if (!PrepareCurrentProgramForDraw(__func__)) return; if (!ValidatePrimitiveModeForBackend(__func__, mode)) return; MultiDrawArraysIndirect_Backend(mode, indirect, drawcount, stride); } @@ -913,7 +946,7 @@ namespace MobileGL::MG_Impl::GLImpl { // 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; + if (!PrepareCurrentProgramForDraw(__func__)) return; auto multiDrawElementsIndirectCount = MG_Backend::gBackendFunctionsTable.GL.MultiDrawElementsIndirectCount; if (!multiDrawElementsIndirectCount) { MG_State::pGLContext->RecordError( @@ -934,7 +967,7 @@ namespace MobileGL::MG_Impl::GLImpl { return; } // See MultiDrawElementsIndirectCount, including why this one goes last. - if (!ValidateCurrentProgramForExecution(__func__)) return; + if (!PrepareCurrentProgramForDraw(__func__)) return; auto multiDrawArraysIndirectCount = MG_Backend::gBackendFunctionsTable.GL.MultiDrawArraysIndirectCount; if (!multiDrawArraysIndirectCount) { MG_State::pGLContext->RecordError( @@ -949,7 +982,7 @@ namespace MobileGL::MG_Impl::GLImpl { void DrawRangeElementsBaseVertex(GLenum mode, GLuint start, GLuint end, GLsizei count, GLenum type, const void* indices, GLint basevertex) { if (!ValidatePrimitiveModeEnum(__func__, mode)) return; - if (!ValidateCurrentProgramForExecution(__func__)) return; + if (!PrepareCurrentProgramForDraw(__func__)) return; if (!ValidatePrimitiveModeForBackend(__func__, mode)) return; if (!ValidateDrawElementsIndexType(__func__, type)) return; if (!ValidateNonNegativeDrawArgument(__func__, "count", count)) return; @@ -959,7 +992,7 @@ namespace MobileGL::MG_Impl::GLImpl { void DrawRangeElements(GLenum mode, GLuint start, GLuint end, GLsizei count, GLenum type, const void* indices) { if (!ValidatePrimitiveModeEnum(__func__, mode)) return; - if (!ValidateCurrentProgramForExecution(__func__)) return; + if (!PrepareCurrentProgramForDraw(__func__)) return; if (!ValidatePrimitiveModeForBackend(__func__, mode)) return; DrawRangeElements_Backend(mode, start, end, count, type, indices); } @@ -967,7 +1000,7 @@ namespace MobileGL::MG_Impl::GLImpl { void DrawElementsInstancedBaseVertexBaseInstance(GLenum mode, GLsizei count, GLenum type, const void* indices, GLsizei instancecount, GLint basevertex, GLuint baseinstance) { if (!ValidatePrimitiveModeEnum(__func__, mode)) return; - if (!ValidateCurrentProgramForExecution(__func__)) return; + if (!PrepareCurrentProgramForDraw(__func__)) return; if (!ValidatePrimitiveModeForBackend(__func__, mode)) return; DrawElementsInstancedBaseVertexBaseInstance_Backend(mode, count, type, indices, instancecount, basevertex, baseinstance); @@ -976,7 +1009,7 @@ namespace MobileGL::MG_Impl::GLImpl { void DrawElementsInstancedBaseVertex(GLenum mode, GLsizei count, GLenum type, const void* indices, GLsizei instancecount, GLint basevertex) { if (!ValidatePrimitiveModeEnum(__func__, mode)) return; - if (!ValidateCurrentProgramForExecution(__func__)) return; + if (!PrepareCurrentProgramForDraw(__func__)) return; if (!ValidatePrimitiveModeForBackend(__func__, mode)) return; if (!ValidateDrawElementsIndexType(__func__, type)) return; if (!ValidateNonNegativeDrawArgument(__func__, "count", count)) return; @@ -987,21 +1020,21 @@ namespace MobileGL::MG_Impl::GLImpl { void DrawElementsInstancedBaseInstance(GLenum mode, GLsizei count, GLenum type, const void* indices, GLsizei instancecount, GLuint baseinstance) { if (!ValidatePrimitiveModeEnum(__func__, mode)) return; - if (!ValidateCurrentProgramForExecution(__func__)) return; + if (!PrepareCurrentProgramForDraw(__func__)) return; if (!ValidatePrimitiveModeForBackend(__func__, mode)) return; DrawElementsInstancedBaseInstance_Backend(mode, count, type, indices, instancecount, baseinstance); } void DrawElementsInstanced(GLenum mode, GLsizei count, GLenum type, const void* indices, GLsizei instancecount) { if (!ValidatePrimitiveModeEnum(__func__, mode)) return; - if (!ValidateCurrentProgramForExecution(__func__)) return; + if (!PrepareCurrentProgramForDraw(__func__)) return; if (!ValidatePrimitiveModeForBackend(__func__, mode)) return; DrawElementsInstanced_Backend(mode, count, type, indices, instancecount); } void DrawElementsIndirect(GLenum mode, GLenum type, const void* indirect) { if (!ValidatePrimitiveModeEnum(__func__, mode)) return; - if (!ValidateCurrentProgramForExecution(__func__)) return; + if (!PrepareCurrentProgramForDraw(__func__)) return; if (!ValidatePrimitiveModeForBackend(__func__, mode)) return; if (!ValidateDrawElementsIndexType(__func__, type)) return; if (!ValidateIndirectDrawSource(__func__, indirect, kDrawElementsIndirectCommandBytes)) return; @@ -1011,21 +1044,21 @@ namespace MobileGL::MG_Impl::GLImpl { void DrawArraysInstancedBaseInstance(GLenum mode, GLint first, GLsizei count, GLsizei instancecount, GLuint baseinstance) { if (!ValidatePrimitiveModeEnum(__func__, mode)) return; - if (!ValidateCurrentProgramForExecution(__func__)) return; + if (!PrepareCurrentProgramForDraw(__func__)) return; if (!ValidatePrimitiveModeForBackend(__func__, mode)) return; DrawArraysInstancedBaseInstance_Backend(mode, first, count, instancecount, baseinstance); } void DrawArraysInstanced(GLenum mode, GLint first, GLsizei count, GLsizei instancecount) { if (!ValidatePrimitiveModeEnum(__func__, mode)) return; - if (!ValidateCurrentProgramForExecution(__func__)) return; + if (!PrepareCurrentProgramForDraw(__func__)) return; if (!ValidatePrimitiveModeForBackend(__func__, mode)) return; DrawArraysInstanced_Backend(mode, first, count, instancecount); } void DrawArraysIndirect(GLenum mode, const void* indirect) { if (!ValidatePrimitiveModeEnum(__func__, mode)) return; - if (!ValidateCurrentProgramForExecution(__func__)) return; + if (!PrepareCurrentProgramForDraw(__func__)) return; if (!ValidatePrimitiveModeForBackend(__func__, mode)) return; if (!ValidateIndirectDrawSource(__func__, indirect, kDrawArraysIndirectCommandBytes)) return; DrawArraysIndirect_Backend(mode, indirect); @@ -1033,7 +1066,7 @@ namespace MobileGL::MG_Impl::GLImpl { void DrawElementsBaseVertex(GLenum mode, GLsizei count, GLenum type, const void* indices, GLint basevertex) { if (!ValidatePrimitiveModeEnum(__func__, mode)) return; - if (!ValidateCurrentProgramForExecution(__func__)) return; + if (!PrepareCurrentProgramForDraw(__func__)) return; if (!ValidatePrimitiveModeForBackend(__func__, mode)) return; if (!ValidateDrawElementsIndexType(__func__, type)) return; if (!ValidateNonNegativeDrawArgument(__func__, "count", count)) return; @@ -1043,7 +1076,7 @@ namespace MobileGL::MG_Impl::GLImpl { void DrawArrays(GLenum mode, GLint first, GLsizei count) { if (!ValidatePrimitiveModeEnum(__func__, mode)) return; - if (!ValidateCurrentProgramForExecution(__func__)) return; + if (!PrepareCurrentProgramForDraw(__func__)) return; if (!ValidatePrimitiveModeForBackend(__func__, mode)) return; AccountTransformFeedbackPrimitives(mode, count); DrawArrays_Backend(mode, first, count); @@ -1051,7 +1084,7 @@ namespace MobileGL::MG_Impl::GLImpl { void MultiDrawArrays(GLenum mode, const GLint* first, const GLsizei* count, GLsizei drawcount) { if (!ValidatePrimitiveModeEnum(__func__, mode)) return; - if (!ValidateCurrentProgramForExecution(__func__)) return; + if (!PrepareCurrentProgramForDraw(__func__)) return; if (!ValidatePrimitiveModeForBackend(__func__, mode)) return; if (drawcount < 0) { MG_State::pGLContext->RecordError( @@ -1065,7 +1098,7 @@ namespace MobileGL::MG_Impl::GLImpl { void MultiDrawElements(GLenum mode, const GLsizei* count, GLenum type, const void* const* indices, GLsizei drawcount) { if (!ValidatePrimitiveModeEnum(__func__, mode)) return; - if (!ValidateCurrentProgramForExecution(__func__)) return; + if (!PrepareCurrentProgramForDraw(__func__)) return; if (!ValidatePrimitiveModeForBackend(__func__, mode)) return; MultiDrawElements_Backend(mode, count, type, indices, drawcount); } @@ -1073,7 +1106,7 @@ namespace MobileGL::MG_Impl::GLImpl { void MultiDrawElementsBaseVertex(GLenum mode, const GLsizei* count, GLenum type, const void* const* indices, GLsizei drawcount, const GLint* basevertex) { if (!ValidatePrimitiveModeEnum(__func__, mode)) return; - if (!ValidateCurrentProgramForExecution(__func__)) return; + if (!PrepareCurrentProgramForDraw(__func__)) return; if (!ValidatePrimitiveModeForBackend(__func__, mode)) return; if (!ValidateDrawElementsIndexType(__func__, type)) return; if (!ValidateNonNegativeDrawArgument(__func__, "drawcount", drawcount)) return; @@ -1097,7 +1130,7 @@ namespace MobileGL::MG_Impl::GLImpl { void DrawElements(GLenum mode, GLsizei count, GLenum type, const void* indices) { if (!ValidatePrimitiveModeEnum(__func__, mode)) return; - if (!ValidateCurrentProgramForExecution(__func__)) return; + if (!PrepareCurrentProgramForDraw(__func__)) return; if (!ValidatePrimitiveModeForBackend(__func__, mode)) return; AccountTransformFeedbackPrimitives(mode, count); DrawElements_Backend(mode, count, type, indices); @@ -1521,7 +1554,7 @@ namespace MobileGL::MG_Impl::GLImpl { // (GL 4.6 core 10.3.7). static void DrawTransformFeedbackImpl(const char* functionName, GLenum mode, GLuint id, GLuint stream, GLsizei instancecount) { - if (!ValidateCurrentProgramForExecution(functionName)) return; + if (!PrepareCurrentProgramForDraw(functionName)) return; if (!ValidatePrimitiveModeForBackend(functionName, mode)) return; if (instancecount < 0) { MG_State::pGLContext->RecordError( diff --git a/MobileGL/MG_Impl/GLImpl/Getter/GL_Getter.cpp b/MobileGL/MG_Impl/GLImpl/Getter/GL_Getter.cpp index 57e4b70b..096c26a3 100644 --- a/MobileGL/MG_Impl/GLImpl/Getter/GL_Getter.cpp +++ b/MobileGL/MG_Impl/GLImpl/Getter/GL_Getter.cpp @@ -304,24 +304,6 @@ namespace MobileGL::MG_Impl::GLImpl { return true; } - GLint ResolveDrawFramebufferSampleCount() { - const auto& drawFbo = - MG_State::pGLContext->GetFramebufferBindingSlot(FramebufferTarget::Draw).GetBoundObject(); - if (!drawFbo) return 0; - - GLint maxSamples = 0; - for (const auto& attachment : drawFbo->GetAllAttachmentObjects()) { - if (attachment.IsRenderbuffer() && attachment.GetRenderbuffer()) { - maxSamples = std::max(maxSamples, static_cast(attachment.GetRenderbuffer()->GetSamples())); - } else if (attachment.IsTexture() && attachment.GetTexture()) { - // Multisample texture attachments count too (GL_SAMPLE_BUFFERS must - // report 1 for any multisampled draw framebuffer). - maxSamples = std::max(maxSamples, static_cast(attachment.GetTexture()->GetSamples())); - } - } - return maxSamples; - } - void RecordIndexedOnlyGetterError(const char* functionName, GLenum pname) { MG_State::pGLContext->RecordError( ErrorCode::InvalidEnum, @@ -484,6 +466,26 @@ namespace MobileGL::MG_Impl::GLImpl { return std::max(MG_Backend::pActiveBackendObject->GetDynamicParameters().MaxSamples, kFrontendMaxSamples); } + // Declared in GL_Getter.h, so that the draw path can feed the same number to the reserved + // gl_NumSamples stand-in that glGetIntegerv(GL_SAMPLES) reports. + GLint ResolveDrawFramebufferSampleCount() { + const auto& drawFbo = + MG_State::pGLContext->GetFramebufferBindingSlot(FramebufferTarget::Draw).GetBoundObject(); + if (!drawFbo) return 0; + + GLint maxSamples = 0; + for (const auto& attachment : drawFbo->GetAllAttachmentObjects()) { + if (attachment.IsRenderbuffer() && attachment.GetRenderbuffer()) { + maxSamples = std::max(maxSamples, static_cast(attachment.GetRenderbuffer()->GetSamples())); + } else if (attachment.IsTexture() && attachment.GetTexture()) { + // Multisample texture attachments count too (GL_SAMPLE_BUFFERS must + // report 1 for any multisampled draw framebuffer). + maxSamples = std::max(maxSamples, static_cast(attachment.GetTexture()->GetSamples())); + } + } + return maxSamples; + } + /* @INSERTION_POINT:FUNCTION_IMPLEMENTATION@ */ const GLubyte* GetString(GLenum name) { static String vendorString; diff --git a/MobileGL/MG_Impl/GLImpl/Getter/GL_Getter.h b/MobileGL/MG_Impl/GLImpl/Getter/GL_Getter.h index ee8faf65..6fa09f7f 100644 --- a/MobileGL/MG_Impl/GLImpl/Getter/GL_Getter.h +++ b/MobileGL/MG_Impl/GLImpl/Getter/GL_Getter.h @@ -28,4 +28,13 @@ namespace MobileGL::MG_Impl::GLImpl { // core minimum. Frontend multisample validators have to honour this ceiling for every // format, otherwise MobileGL rejects a sample count it advertised itself. GLint GetAdvertisedMaxSamples(); + // What glGetIntegerv(GL_SAMPLES) answers for the CURRENT draw framebuffer: the largest sample + // count over its attachments, and 0 for a single-sample or default framebuffer (GL 4.6 core + // 9.2.3 / 22.2 - GL_SAMPLE_BUFFERS is 1 exactly when this is non-zero). + // + // Shared rather than duplicated because two callers need the identical number and disagreeing + // would be a silent bug: the query itself, and the draw path's write of the reserved + // gl_NumSamples stand-in - a shader comparing gl_NumSamples against glGetIntegerv(GL_SAMPLES) + // is exactly what the sample_variables CTS does. + GLint ResolveDrawFramebufferSampleCount(); } // namespace MobileGL::MG_Impl::GLImpl diff --git a/MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.cpp b/MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.cpp index e938a3f7..627ada37 100644 --- a/MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.cpp +++ b/MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.cpp @@ -1136,6 +1136,20 @@ namespace MobileGL::MG_State::GLState { in.externalIndex, uniform.name.c_str()); continue; } + // The gl_NumSamples stand-in InjectNumSamplesBuiltinShim declared. It is a driver + // uniform, not the application's: gl_NumSamples is a BUILT-IN, so a conformant + // implementation reports nothing for it in GL_ACTIVE_UNIFORMS, glGetActiveUniform or + // glGetUniformLocation, and nothing may write it through glUniform* either. Filtering + // it here does both, and costs it no storage: BuildGlobalUboRouting takes its offset + // from the SPIR-V metadata by name, not from the GL location space. + if (isGlobalUboMember(uniform) && + uniform.name == MG_Util::ShaderTranspiler::NUM_SAMPLES_UNIFORM_NAME) { + artifacts.usesReservedNumSamples = true; + MGLOG_D("ProgramObject %u: Reflection - reserved gl_NumSamples stand-in '%s' hidden from the GL " + "uniform surface", + in.externalIndex, uniform.name.c_str()); + continue; + } if (isBufferVariable(uniform)) { MGLOG_D("ProgramObject %u: Reflection - buffer variable '%s' filtered from the GL uniform " "surface", diff --git a/MobileGL/MG_State/GLState/ProgramState/ProgramObject.h b/MobileGL/MG_State/GLState/ProgramState/ProgramObject.h index 0d37e5dc..f991a6ba 100644 --- a/MobileGL/MG_State/GLState/ProgramState/ProgramObject.h +++ b/MobileGL/MG_State/GLState/ProgramState/ProgramObject.h @@ -787,6 +787,33 @@ namespace MobileGL::MG_State::GLState { void MarkUBOContentDirty() const { if (++m_uboContentVersion == ~0u) m_uboContentVersion = 0; } + + // ---- the reserved gl_NumSamples stand-in (ShaderTranspiler::NUM_SAMPLES_UNIFORM_NAME) ---- + // + // PHASE A: answerable without joining the SPIR-V job, which is what lets the draw path ask + // every program this question and pay nothing for the overwhelming majority that say no. + Bool UsesReservedNumSamples() const { return Artifacts().usesReservedNumSamples; } + + // Publishes `samples` into the global-UBO shadow. Returns false when there is nowhere to + // put it - no shim in this program, no SPIR-V (a cancelled phase B), or the optimizer + // dropped the member because nothing read it after all - all of which are ordinary states, + // not errors. A value-identical write is dropped without bumping the content version, so a + // steady stream of draws into one framebuffer does not force a re-upload per draw. + Bool WriteReservedNumSamples(Int samples) { + if (!UsesReservedNumSamples()) return false; + SpirvArtifacts& spirv = Spirv(); + const Uint offset = spirv.reservedNumSamplesOffset; + if (offset == kInvalidUniformOffset) return false; + if (static_cast(offset) + sizeof(Int) > spirv.globalUboScratch.size()) return false; + + Uint8* const slot = spirv.globalUboScratch.data() + offset; + Int current = 0; + Memcpy(¤t, slot, sizeof(Int)); + if (current == samples) return true; + Memcpy(slot, &samples, sizeof(Int)); + MarkUBOContentDirty(); + return true; + } // ---- glUniform* inside the phase-A -> phase-B window ---- // // True while the program is fully linked and fully queryable but its uniform shadow's @@ -1296,6 +1323,14 @@ namespace MobileGL::MG_State::GLState { std::set uniformBlocksWithoutBinding; Uint activeUniformCount = 0; + // This program's fragment stage read gl_NumSamples, so the source pipeline lowered it + // onto the reserved default-block uniform (ShaderTranspiler::NUM_SAMPLES_UNIFORM_NAME) + // and the draw path owes it the draw framebuffer's sample count before every draw. + // + // PHASE A on purpose, even though the byte offset it needs is phase-B output: the + // gate has to be answerable without joining the SPIR-V job, or every draw of every + // program would pay a join to discover it has nothing to write. + Bool usesReservedNumSamples = false; Uint maxUniformLocation = 0; Int uniformNameMaxLength = 0; Int attribInNameMaxLength = 0; @@ -1348,6 +1383,11 @@ namespace MobileGL::MG_State::GLState { // kInvalidUniformOffset. Sized maxUniformLocation + 1 by the routing pass. Vector uniformOffsets; Vector globalUboScratch; + // Byte offset of the reserved gl_NumSamples stand-in inside globalUboScratch, or + // kInvalidUniformOffset. Taken by NAME from the SPIR-V metadata rather than through + // uniformOffsets, because the member has no GL location at all: the link task keeps + // it out of the GL-visible uniform index space so no application can see or write it. + Uint reservedNumSamplesOffset = kInvalidUniformOffset; // False for a program whose SPIR-V was never produced (phase B cancelled at // teardown or by a relink) or whose optimizer run failed. GL has no way to // retract a LINK_STATUS it already reported true, so such a program stays diff --git a/MobileGL/MG_State/GLState/ProgramState/ProgramSpirvTask.cpp b/MobileGL/MG_State/GLState/ProgramState/ProgramSpirvTask.cpp index f2c1021d..03761473 100644 --- a/MobileGL/MG_State/GLState/ProgramState/ProgramSpirvTask.cpp +++ b/MobileGL/MG_State/GLState/ProgramState/ProgramSpirvTask.cpp @@ -278,6 +278,7 @@ namespace MobileGL::MG_State::GLState { artifacts.uniformOffsets.clear(); artifacts.globalUboScratch.clear(); + artifacts.reservedNumSamplesOffset = ProgramObject::kInvalidUniformOffset; // kInvalidUniformOffset marks locations that end up without global-UBO backing // (e.g. the optimizer eliminated every use of the uniform); the fallback pass // below gives those locations tail storage so glUniform* always has a target. @@ -311,6 +312,18 @@ namespace MobileGL::MG_State::GLState { artifacts.globalUboScratch.resize(size); } for (const auto& [name, offset] : meta.plainUniformOffsetsInUBO) { + // The gl_NumSamples stand-in is routed by NAME and nothing else. It has no GL + // location to look up - DoReflection hides it from the GL uniform index space + // precisely so no application can address it - so the lookup below would find + // nothing and log it as unbacked. Only the fragment stage declares it, and + // every stage's copy sits at the same offset in the one shared global UBO. + if (name == NUM_SAMPLES_UNIFORM_NAME) { + artifacts.reservedNumSamplesOffset = offset; + MGLOG_D("ProgramObject %u: BuildGlobalUboRouting - reserved gl_NumSamples stand-in '%s' " + "backed at UBO offset %u", + externalIndex, name.c_str(), offset); + continue; + } // SPIRV-Reflect leaf names never carry a "[0]" suffix; frontend // reflection keys arrays as "arr[0]" (GL naming), so retry with the // suffix before declaring the uniform unbacked. diff --git a/MobileGL/MG_Util/ShaderTranspiler/ShaderSourceProcessor.cpp b/MobileGL/MG_Util/ShaderTranspiler/ShaderSourceProcessor.cpp index 3b7f8c7a..480f9f62 100644 --- a/MobileGL/MG_Util/ShaderTranspiler/ShaderSourceProcessor.cpp +++ b/MobileGL/MG_Util/ShaderTranspiler/ShaderSourceProcessor.cpp @@ -234,6 +234,17 @@ namespace { // Whether the parsed #version directive is a well-formed one MobileGL should rewrite. A // malformed directive (see IsRecognizedGlslVersion) is left alone for glslang to reject. bool hasValidVersionDirective = false; + // Every extension the source NAMES in an "#extension : " directive, and + // the subset whose behavior switches it on. Both are needed and they are not the same + // question: glslang's ES preamble defines an extension's macro whatever behavior the + // shader later asks for (it is a preamble, it runs first), while whether gl_NumSamples is + // a legal identifier depends on the extension actually being ENABLED. + std::set namedExtensions; + std::set enabledExtensions; + // Byte ranges [begin, end) of every #version directive AFTER the first that repeats it + // exactly - same version number, same profile, both well-formed. See + // BlankRedundantVersionDirectives for why these are tolerated and nothing else is. + Vector> redundantVersionDirectives; bool HasVersionDirective() const { return versionDirectiveStart != MobileGL::String::npos; } }; @@ -262,7 +273,7 @@ namespace { SkipDirectiveWhitespace(code, probe, lineEnd); const MobileGL::String directive = ReadDirectiveIdentifier(code, probe, lineEnd); - if (directive == "version" && !info.HasVersionDirective()) { + if (directive == "version") { SkipDirectiveWhitespace(code, probe, lineEnd); unsigned version = 0; bool hasVersionDigits = false; @@ -272,30 +283,39 @@ namespace { probe++; } if (hasVersionDigits) { - info.version = version; - info.versionDirectiveStart = directiveStart; - info.versionDirectiveEnd = lineEnd + (hasLineBreak ? 1 : 0); SkipDirectiveWhitespace(code, probe, lineEnd); - const MobileGL::String profile = ReadDirectiveIdentifier(code, probe, lineEnd); + const MobileGL::String profileToken = ReadDirectiveIdentifier(code, probe, lineEnd); + MobileGL::ShaderProfile profile = MobileGL::ShaderProfile::Core; bool profileTokenValid = true; - if (profile.empty() || profile == "core") { - info.profile = MobileGL::ShaderProfile::Core; - } else if (profile == "es" || profile == "ES") { - info.profile = MobileGL::ShaderProfile::ES; - } else if (profile == "compatibility") { - info.profile = MobileGL::ShaderProfile::Compatibility; + if (profileToken.empty() || profileToken == "core") { + profile = MobileGL::ShaderProfile::Core; + } else if (profileToken == "es" || profileToken == "ES") { + profile = MobileGL::ShaderProfile::ES; + } else if (profileToken == "compatibility") { + profile = MobileGL::ShaderProfile::Compatibility; } else { // "#version 330 foo": an unrecognized profile keyword. Keep Core for any // downstream routing, but mark the directive malformed. - info.profile = MobileGL::ShaderProfile::Core; + profile = MobileGL::ShaderProfile::Core; profileTokenValid = false; } // Comments are already masked to spaces, so anything non-blank left on the // line is real trailing garbage: "#version 330 foobar" / "#version 330.0". SkipDirectiveWhitespace(code, probe, lineEnd); const bool hasTrailingTokens = probe < lineEnd; - info.hasValidVersionDirective = - IsRecognizedGlslVersion(info.version) && profileTokenValid && !hasTrailingTokens; + const bool directiveIsValid = + IsRecognizedGlslVersion(version) && profileTokenValid && !hasTrailingTokens; + + if (!info.HasVersionDirective()) { + info.version = version; + info.profile = profile; + info.versionDirectiveStart = directiveStart; + info.versionDirectiveEnd = lineEnd + (hasLineBreak ? 1 : 0); + info.hasValidVersionDirective = directiveIsValid; + } else if (directiveIsValid && info.hasValidVersionDirective && version == info.version && + profile == info.profile) { + info.redundantVersionDirectives.push_back({directiveStart, lineEnd}); + } } } else if (directive == "extension") { SkipDirectiveWhitespace(code, probe, lineEnd); @@ -309,6 +329,10 @@ namespace { extension == "GL_NV_gpu_shader5"; const bool enablesExtension = behavior == "enable" || behavior == "require" || behavior == "warn"; + if (!extension.empty()) { + info.namedExtensions.insert(extension); + if (enablesExtension) info.enabledExtensions.insert(extension); + } // Gate the whole source if it ever opts into either extension. This is deliberately // conservative around conditional directives and keeps legal sample qualifiers intact. info.enablesGpuShader5 = info.enablesGpuShader5 || (isGpuShader5 && enablesExtension); @@ -367,9 +391,40 @@ namespace { // (FindAfterVersionDirective -> InspectShaderLanguage). Each branch below leaves the bytes // ahead of the directive untouched apart from the BOM erase, and each replacement text is // exactly one newline-terminated line, so the arithmetic is exact in all three cases. + // An exact repeat of the #version directive the shader already declared, blanked out. + // + // Strictly a repeat: InspectShaderLanguage only records a range here when the FIRST directive + // was well-formed and the later one is well-formed, names the same version number and the same + // profile, and is therefore semantically a no-op. Everything else - a differing version, a + // malformed one, or a lone #version that is simply not first - is left exactly where the + // application put it, so KHR-GL33.shaders.preprocessor.directive.version_not_first_statement_* + // and the version_invalid_token_* family keep failing to compile the way they must. + // + // Why tolerate even the repeat: glShaderSource concatenates its strings with nothing added + // between them (GL 4.6 core 7.1), and a caller that puts a #version at the head of BOTH strings + // gets the second one spliced into the tail of the first - which is exactly what VK-GL-CTS's + // ShaderImageLoadStoreBase::BuildProgram does (kGLSLPrec ends without a newline, and + // NegativeUniform's own sources begin with "#version 310 es"). Desktop drivers accept it; the + // duplicate says nothing new, so honouring it costs no semantics. + // + // Blanked rather than erased so that every offset in `info` - which was measured against this + // same source - stays valid, and so the line count, and with it __LINE__ and every glslang + // diagnostic, is untouched. + void BlankRedundantVersionDirectives(MobileGL::String& source, const ShaderLanguageInfo& info) { + for (const auto& [begin, end] : info.redundantVersionDirectives) { + if (begin >= source.size() || end > source.size() || begin >= end) continue; + std::fill(source.begin() + static_cast(begin), + source.begin() + static_cast(end), ' '); + } + } + SizeT NormalizeVersionDirective(MobileGL::String& source, const ShaderLanguageInfo& info) { const SizeT bomBytes = info.hasUtf8Bom ? 3 : 0; + // First, while every offset in `info` still refers to the untouched source. Each range + // lies strictly after the first directive, so nothing below has to account for it. + BlankRedundantVersionDirectives(source, info); + // A malformed #version (329, 331, bad profile, float/trailing tokens) is left exactly as the // application wrote it so glslang rejects it - rewriting it to "#version 330 core" would // silently legalize the CTS directive.version_* rejection cases. Still drop a leading BOM so @@ -1435,6 +1490,158 @@ namespace { "#define gl_DepthRange mg_DepthRange\n"; source.insert(afterVersion.Get(source), shim); } + + // Whole-identifier search over an already-masked source. A bare find() would fire on + // "mg_NumSamplesFoo" and on the word inside a comment; this fires only on the token. + bool MaskedSourceHasIdentifier(const MobileGL::String& masked, MobileGL::StringView identifier) { + SizeT pos = 0; + while ((pos = masked.find(identifier.data(), pos, identifier.size())) != MobileGL::String::npos) { + const SizeT end = pos + identifier.size(); + const bool hasLeftBoundary = pos == 0 || !IsIdentifierChar(masked[pos - 1]); + const bool hasRightBoundary = end >= masked.size() || !IsIdentifierChar(masked[end]); + if (hasLeftBoundary && hasRightBoundary) return true; + pos = end; + } + return false; + } + + // The extension macros glslang's ES preamble defines and its DESKTOP preamble does not + // (TParseVersions::getPreamble, Versions.cpp). Transcribed rather than derived because the + // preamble is a string literal inside glslang with no programmatic accessor; the SET is what + // matters, and it is stable - these are the AEP/OES/EXT names ESSL has carried since 3.10. + // + // GL_ES and GL_FRAGMENT_PRECISION_HIGH are DELIBERATELY absent. The shader really is being + // compiled as desktop by the time this runs, so flipping an `#ifdef GL_ES` branch would hand + // glslang the ESSL half of a shader written to be portable - which is the branch that does not + // parse under core 4.60. (GL_FRAGMENT_PRECISION_HIGH is in glslang's desktop preamble anyway.) + bool IsEsOnlyPreambleExtensionMacro(const MobileGL::String& name, unsigned version) { + // Guarded by an ES version in glslang's preamble; the rest are unconditional. + if (name == "GL_NV_shader_noperspective_interpolation") return version >= 300; + + static const std::set kEsOnlyPreambleMacros = { + "GL_ANDROID_extension_pack_es31a", + "GL_EXT_YUV_target", + "GL_EXT_blend_func_extended", + "GL_EXT_frag_depth", + "GL_EXT_geometry_point_size", + "GL_EXT_geometry_shader", + "GL_EXT_gpu_shader5", + "GL_EXT_primitive_bounding_box", + "GL_EXT_shader_implicit_conversions", + "GL_EXT_shader_io_blocks", + "GL_EXT_shader_texture_lod", + "GL_EXT_shadow_samplers", + "GL_EXT_tessellation_point_size", + "GL_EXT_tessellation_shader", + "GL_EXT_texture_buffer", + "GL_EXT_texture_cube_map_array", + "GL_OES_EGL_image_external", + "GL_OES_EGL_image_external_essl3", + "GL_OES_geometry_point_size", + "GL_OES_geometry_shader", + "GL_OES_gpu_shader5", + "GL_OES_primitive_bounding_box", + "GL_OES_sample_variables", + "GL_OES_shader_image_atomic", + "GL_OES_shader_io_blocks", + "GL_OES_shader_multisample_interpolation", + "GL_OES_standard_derivatives", + "GL_OES_tessellation_point_size", + "GL_OES_tessellation_shader", + "GL_OES_texture_3D", + "GL_OES_texture_buffer", + "GL_OES_texture_cube_map_array", + "GL_OES_texture_storage_multisample_2d_array", + }; + return kEsOnlyPreambleMacros.count(name) != 0; + } + + // GetNormalizedVersionDirective rewrites every ES-profile shader to "#version 460 core", so + // glslang deduces a desktop profile and emits its DESKTOP preamble - and every ES-only + // extension macro the shader is entitled to disappears with it. A CTS shader guarded by + // `#if !GL_OES_sample_variables / this is broken / #endif` then takes the broken branch. + // + // The extension BEHAVIOUR survives the rewrite (glslang honours "#extension X : require" under + // either profile), so this is a preamble-fidelity gap and nothing more; restoring the macro is + // the whole fix. + // + // Strictly limited to extensions the source itself NAMES in an #extension directive. Any macro + // injected into a desktop parse can flip a preprocessor branch, and the ES preamble carries + // three dozen of them - defining the lot would rewrite shaders that never asked. + void InjectEsPreambleExtensionMacros(const ShaderLanguageInfo& info, MobileGL::String& source, + AfterVersionAnchor& afterVersion) { + // Only where the rewrite actually happened: a malformed directive is left for glslang to + // reject, and a desktop source already gets the preamble it is entitled to. + if (info.profile != MobileGL::ShaderProfile::ES) return; + if (!info.hasValidVersionDirective) return; + if (info.namedExtensions.empty()) return; + + MobileGL::String shim; + // std::set iteration order, so the injected block is deterministic for the translation + // cache and for the byte-exact preprocessor tests. + for (const MobileGL::String& extension : info.namedExtensions) { + if (!IsEsOnlyPreambleExtensionMacro(extension, info.version)) continue; + shim += "#define " + extension + " 1\n"; + } + if (shim.empty()) return; + source.insert(afterVersion.Get(source), shim); + } + + // gl_NumSamples is legal in this source only where glslang would have declared it with a + // non-SPIR-V target (Initialize.cpp): desktop from 4.00 core, or from 1.30 with + // ARB_sample_shading; ESSL from 3.20, or from 3.10 with OES_sample_variables. + // + // The gate matters because the shim ends in "#define gl_NumSamples mg_NumSamples", and a + // #define is not scoped by anything: defining it for a source where the built-in does not + // exist would silently legalize a shader a conformant implementation rejects. + bool SourceMayUseSampleVariables(const ShaderLanguageInfo& info) { + if (!info.HasVersionDirective() || !info.hasValidVersionDirective) return false; + if (info.profile == MobileGL::ShaderProfile::ES) { + if (info.version >= 320) return true; + return info.version >= 310 && info.enabledExtensions.count("GL_OES_sample_variables") != 0; + } + if (info.version >= 400) return true; + return info.version >= 130 && info.enabledExtensions.count("GL_ARB_sample_shading") != 0; + } + + // gl_NumSamples has no SPIR-V built-in to lower to, so glslang declares it only when it is NOT + // targeting SPIR-V - both the desktop branch and the ES branch of Initialize.cpp wrap the + // `uniform int gl_NumSamples;` line in `if (spvVersion.spv == 0)`. MobileGL always targets + // SPIR-V (ShaderCompiler sets EShTargetSpv on the OpenGL path as well as the Vulkan one), so + // the symbol is never in the table and every shader that reads it dies at compile time with + // "'gl_NumSamples' : undeclared identifier". + // + // Lower it to a real uniform instead. `uniform int mg_NumSamples;` is a default-block uniform, + // which the relaxed parse folds into MGL_GLOBAL_UBO - the one buffer BOTH backends already + // upload per draw - and the draw path writes the current draw framebuffer's sample count into + // it. Deliberately not a link-time constant: one program may be drawn into framebuffers of + // different sample counts, and baking the count at link would quietly hand it the wrong one. + // + // The alternative - deleting the `spvVersion.spv == 0` guard in the glslang fork - is worse, + // and not only because it is a fork change: glslang would then place a `gl_`-prefixed member + // inside MGL_GLOBAL_UBO, and ESSL reserves `gl_`, so the ES driver would reject SPIRV-Cross's + // output on the DirectGLES path. + void InjectNumSamplesBuiltinShim(MobileGL::ShaderStage stage, const ShaderLanguageInfo& info, + MobileGL::String& source, AfterVersionAnchor& afterVersion) { + // gl_NumSamples exists in the fragment stage only, in every profile. + if (stage != MobileGL::ShaderStage::Fragment) return; + if (!SourceMayUseSampleVariables(info)) return; + // Cheap reject before paying for the mask; the token cannot be there if the bytes are not. + if (source.find("gl_NumSamples") == MobileGL::String::npos) return; + + const MobileGL::String masked = MaskCommentsAndQuotedText(source); + if (!MaskedSourceHasIdentifier(masked, "gl_NumSamples")) return; + // Someone already occupies the name - a re-preprocess of an already-shimmed source, or an + // application that happens to use it. Either way a second declaration would not compile. + if (MaskedSourceHasIdentifier(masked, MobileGL::MG_Util::ShaderTranspiler::NUM_SAMPLES_UNIFORM_NAME)) { + return; + } + + constexpr const char* shim = + "uniform int mg_NumSamples;\n" + "#define gl_NumSamples mg_NumSamples\n"; + source.insert(afterVersion.Get(source), shim); + } } // namespace namespace MobileGL { @@ -1463,6 +1670,12 @@ namespace MobileGL { // via MaskCommentsAndQuotedText/TokenizeCode, so the source we hand glslang keeps them. NormalizeLineDirectives(source, afterVersion.Get(source)); + // Before anything that could branch on one: an ES source rewritten to desktop has + // lost glslang's ES preamble, and the macros it carried are what the shader's own + // #if guards read. Keyed off originalLanguage because the directive has already + // been rewritten by now and no longer says "es". + InjectEsPreambleExtensionMacros(originalLanguage, source, afterVersion); + // noperspective is intentionally NOT touched here. It is core in desktop GLSL (1.30+) // and maps to the core SPIR-V NoPerspective decoration, which DirectVulkan renders // natively and SPIRV-Cross turns into ESSL `noperspective` + the @@ -1487,6 +1700,7 @@ namespace MobileGL { ModernizeLegacyGLSL(stage, source, afterVersion); InjectDepthRangeBuiltinShim(stage, source, afterVersion); + InjectNumSamplesBuiltinShim(stage, originalLanguage, source, afterVersion); } diff --git a/MobileGL/MG_Util/ShaderTranspiler/Types.h b/MobileGL/MG_Util/ShaderTranspiler/Types.h index 0b7dc74b..517988a0 100644 --- a/MobileGL/MG_Util/ShaderTranspiler/Types.h +++ b/MobileGL/MG_Util/ShaderTranspiler/Types.h @@ -14,6 +14,20 @@ namespace MobileGL { namespace MG_Util { namespace ShaderTranspiler { inline const char* GLOBAL_UBO_NAME = "MGL_GLOBAL_UBO"; + // The default-block uniform InjectNumSamplesBuiltinShim declares to stand in for the + // gl_NumSamples built-in, which glslang does not put in the symbol table under a + // SPIR-V target (Initialize.cpp guards both the desktop and the ES declaration on + // `spvVersion.spv == 0`, and MobileGL always targets SPIR-V). The relaxed parse folds + // it into GLOBAL_UBO_NAME like any other default-block uniform, which is what lets + // BOTH backends pick the value up from the one buffer they already upload; the link + // task keeps it out of the GL-visible uniform surface, and the draw path writes the + // current draw framebuffer's sample count into it. + // + // RESERVED, not merely conventional: a shader that declares this name itself keeps + // the shim from firing (the injector bails on it), but if it declares the name AND + // uses gl_NumSamples the link task will still hide its uniform. That is the same + // bargain every mg_-prefixed rewrite in this pipeline strikes. + inline const char* NUM_SAMPLES_UNIFORM_NAME = "mg_NumSamples"; // glslang's Vulkan-relaxed parse rewrites every atomic_uint into a member of a // synthesized storage block named "_" // (ParseContextBase::growAtomicCounterBlock). That block IS the GL atomic counter