diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.cpp b/MobileGL/MG_Backend/DirectGLES/Managers.cpp index f622dfb6..1a2d120e 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.cpp +++ b/MobileGL/MG_Backend/DirectGLES/Managers.cpp @@ -2732,6 +2732,17 @@ namespace MobileGL::MG_Backend::DirectGLES { ResolveBackendEsslVersion()); spvc_compiler_options_set_bool(options, SPVC_COMPILER_OPTION_GLSL_ES, SPVC_TRUE); spvc_compiler_options_set_bool(options, SPVC_COMPILER_OPTION_GLSL_VULKAN_SEMANTICS, SPVC_FALSE); + // Emit against highp default precision in every stage. SPIRV-Cross's fragment + // default is mediump, under which a RelaxedPrecision struct member prints with + // NO qualifier; ForceSupporterOutput later swaps the header to highp, silently + // flipping such members to highp. A uniform-block member that stays explicitly + // "mediump" in the vertex stage then mismatches, and the ES driver refuses to + // link ("definitions of uniform block ... do not match"). With highp defaults + // every relaxed member is printed with an explicit qualifier in both stages. + spvc_compiler_options_set_bool(options, SPVC_COMPILER_OPTION_GLSL_ES_DEFAULT_FLOAT_PRECISION_HIGHP, + SPVC_TRUE); + spvc_compiler_options_set_bool(options, SPVC_COMPILER_OPTION_GLSL_ES_DEFAULT_INT_PRECISION_HIGHP, + SPVC_TRUE); spvcSession.SetOptions(options); diff --git a/MobileGL/MG_Impl/GLImpl/Program/GL_Program.cpp b/MobileGL/MG_Impl/GLImpl/Program/GL_Program.cpp index ea8a1201..cc36b65f 100644 --- a/MobileGL/MG_Impl/GLImpl/Program/GL_Program.cpp +++ b/MobileGL/MG_Impl/GLImpl/Program/GL_Program.cpp @@ -339,6 +339,9 @@ namespace MobileGL::MG_Impl::GLImpl { MakeUnique("MG_Impl/GLImpl", __func__, "Shader is not attached to program.")); return; } + // A shader flagged with glDeleteShader lives on while attached; this detach may + // have been its last GL-visible attachment. + MG_State::pGLContext->ReleaseShaderNameIfOrphaned(shader); } void GetActiveAttrib_State(GLuint program, GLuint index, GLsizei bufSize, GLsizei* length, GLint* size, @@ -1497,9 +1500,13 @@ namespace MobileGL::MG_Impl::GLImpl { MGLOG_D("%s: GL_UNIFORM_BLOCK_REFERENCED_BY_COMPUTE_SHADER = %d", __func__, *params); break; case GL_UNIFORM_BLOCK_ACTIVE_UNIFORM_INDICES: { + // Member entries of an arrayed block are recorded against the first instance; + // every instance of the array reports that shared member set (matches + // GL_UNIFORM_BLOCK_ACTIVE_UNIFORMS, which scans with the same owner index). + const Int ownerIndex = static_cast(programObject->GetUniformBlockMemberOwnerIndex(uniformBlockIndex)); GLint uniformIndexCount = 0; for (Uint uniformIndex = 0; uniformIndex < programObject->GetUniformCount(); ++uniformIndex) { - if (programObject->GetActiveUniformBlockIndex(uniformIndex) != static_cast(uniformBlockIndex)) { + if (programObject->GetActiveUniformBlockIndex(uniformIndex) != ownerIndex) { continue; } params[uniformIndexCount++] = static_cast(uniformIndex); diff --git a/MobileGL/MG_State/GLState/Core.cpp b/MobileGL/MG_State/GLState/Core.cpp index d17bff3f..fecbec3e 100644 --- a/MobileGL/MG_State/GLState/Core.cpp +++ b/MobileGL/MG_State/GLState/Core.cpp @@ -279,6 +279,10 @@ namespace MobileGL::MG_State { return m_programState.MarkShaderObjectForDeletion(index); } + void GLContext::ReleaseShaderNameIfOrphaned(const Uint index) { + return m_programState.ReleaseShaderNameIfOrphaned(index); + } + Bool GLContext::ValidateProgramName(const Uint index) const { return m_programState.ValidateProgramObject(index); } diff --git a/MobileGL/MG_State/GLState/Core.h b/MobileGL/MG_State/GLState/Core.h index ad65734a..cb0a3c86 100644 --- a/MobileGL/MG_State/GLState/Core.h +++ b/MobileGL/MG_State/GLState/Core.h @@ -116,6 +116,9 @@ namespace MobileGL { Uint CreateShader(ShaderStage stage); void MarkProgramForDeletion(Uint index); void MarkShaderForDeletion(Uint index); + // Frees a deletion-flagged shader's name once it lost its last GL-visible + // attachment (call after glDetachShader). + void ReleaseShaderNameIfOrphaned(Uint index); Bool ValidateProgramName(Uint index) const; Bool ValidateShaderName(Uint index) const; const SharedPtr& GetProgramObject(Uint index); diff --git a/MobileGL/MG_State/GLState/ProgramState/ProgramObject.cpp b/MobileGL/MG_State/GLState/ProgramState/ProgramObject.cpp index 5acd0bd6..2de41eda 100644 --- a/MobileGL/MG_State/GLState/ProgramState/ProgramObject.cpp +++ b/MobileGL/MG_State/GLState/ProgramState/ProgramObject.cpp @@ -360,7 +360,18 @@ namespace MobileGL::MG_State::GLState { } MGLOG_D("ProgramObject %u: DoReflection - building reflection", m_externalIndex); - if (!m_program->buildReflection()) { + // GL-style reflection naming (GL CTS uniform_block relies on all four): + // - BasicArraySuffix: an array uniform is reported as "arr[0]" per the GL spec. + // - StrictArraySuffix: named-block struct arrays expand per element ("s[0].a", + // "s[1].a", ...) following ARB_program_interface_query rules. Default-block + // (loose) uniforms already expand per element without this option. + // - AllBlockVariables: every member of an active named block is active even when + // no shader statement reads it (ES 3.0/GL 3.3 named-block semantics). + // - SharedStd140UBO: a DECLARED uniform block is active even when no member is + // ever read (reflected from the linker objects). PreprocessShaderSource coerces + // every block to std140, so this covers all of them. + if (!m_program->buildReflection(EShReflectionStrictArraySuffix | EShReflectionBasicArraySuffix | + EShReflectionAllBlockVariables | EShReflectionSharedStd140UBO)) { m_linkStatus = false; m_infoLog = "Build reflection failed."; MGLOG_E("ProgramObject %u: DoReflection - buildReflection() returned false", m_externalIndex); @@ -483,7 +494,14 @@ namespace MobileGL::MG_State::GLState { continue; } - const auto explicitBinding = m_explicitOpaqueUniformBindings.find(uniform.name); + // Reflection names an array "texs[0]" while the layout(binding = N) map from the IO + // resolver is keyed by the declared name ("texs"); look up both spellings. + auto explicitBinding = m_explicitOpaqueUniformBindings.find(uniform.name); + if (explicitBinding == m_explicitOpaqueUniformBindings.end() && uniform.name.length() > 3 && + uniform.name.compare(uniform.name.length() - 3, 3, "[0]") == 0) { + explicitBinding = + m_explicitOpaqueUniformBindings.find(uniform.name.substr(0, uniform.name.length() - 3)); + } const int initialUnit = explicitBinding != m_explicitOpaqueUniformBindings.end() ? static_cast(explicitBinding->second) : 0; const Int locationSpan = GetUniformLocationSpan(uniform); @@ -685,7 +703,13 @@ namespace MobileGL::MG_State::GLState { m_globalUboScratch.resize(size); } for (const auto& [name, offset] : meta.plainUniformOffsetsInUBO) { - const auto locationIt = m_uniformLocations.find(name); + // 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. + auto locationIt = m_uniformLocations.find(name); + if (locationIt == m_uniformLocations.end()) { + locationIt = m_uniformLocations.find(name + "[0]"); + } if (locationIt == m_uniformLocations.end()) { MGLOG_D("ProgramObject %u: GenerateBinary - uniform '%s' offset=%u but not found in " "m_uniformLocations", diff --git a/MobileGL/MG_State/GLState/ProgramState/ProgramObject.h b/MobileGL/MG_State/GLState/ProgramState/ProgramObject.h index 715f56a4..82bbfe3a 100644 --- a/MobileGL/MG_State/GLState/ProgramState/ProgramObject.h +++ b/MobileGL/MG_State/GLState/ProgramState/ProgramObject.h @@ -18,6 +18,13 @@ namespace MobileGL::MG_State::GLState { public: ProgramObject(Uint externalIndex) : m_externalIndex(externalIndex), m_lifetimeId(AllocateLifetimeId()) {} bool ShaderIsAttached(const SharedPtr& shader); + // GL-visible attachment: in the attach list and not pending detach (glDetachShader + // defers the actual removal to the next link). + Bool ShaderIsAttachedGLVisible(const SharedPtr& shader) const { + const auto matches = [&shader](const SharedPtr& s) { return s.get() == shader.get(); }; + if (std::none_of(m_shaders.begin(), m_shaders.end(), matches)) return false; + return std::none_of(m_detachedShaders.begin(), m_detachedShaders.end(), matches); + } bool AttachShader(const SharedPtr& shader); SizeT DetachShader(const SharedPtr& shader); SizeT RemoveShader(const SharedPtr& shader); @@ -45,10 +52,16 @@ namespace MobileGL::MG_State::GLState { const auto it = m_uniformLocations.find(name); if (it != m_uniformLocations.end()) return (Int)it->second; - // "arr[k]" resolves to the location of element k: glslang reflection stores - // arrays under their base name (no "[0]" suffix), and DoReflection reserves - // one location per array element, so element k lives at base + k. - if (name.length() < 4 || name.back() != ']') return -1; + // Reflection stores GL-style names: an array uniform is keyed "arr[0]" (its base + // location). A bare "arr" query resolves to that entry; an "arr[k]" query resolves + // to base + k because DoReflection reserves one location per array element. + if (name.empty()) return -1; + if (name.back() != ']') { + const auto suffixedIt = m_uniformLocations.find(name + "[0]"); + if (suffixedIt != m_uniformLocations.end()) return (Int)suffixedIt->second; + return -1; + } + if (name.length() < 4) return -1; const SizeT bracket = name.rfind('['); // Require at least one digit between the brackets. if (bracket == String::npos || bracket + 1 >= name.length() - 1) return -1; @@ -58,8 +71,13 @@ namespace MobileGL::MG_State::GLState { element = element * 10 + static_cast(name[i] - '0'); if (element > 0x0FFFFFFFu) return -1; } - const auto baseIt = m_uniformLocations.find(name.substr(0, bracket)); - if (baseIt == m_uniformLocations.end()) return -1; + auto baseIt = m_uniformLocations.find(name.substr(0, bracket) + "[0]"); + if (baseIt == m_uniformLocations.end()) { + // Legacy key without the "[0]" suffix (defensive; reflection normally + // stores the suffixed form for arrays). + baseIt = m_uniformLocations.find(name.substr(0, bracket)); + if (baseIt == m_uniformLocations.end()) return -1; + } const Int base = (Int)baseIt->second; if (!IsValidUniformLocation(base)) return -1; const Int index = m_uniformIndexInTProgram[base]; @@ -86,6 +104,19 @@ namespace MobileGL::MG_State::GLState { return uniformIndex; } + // Reflection stores an array uniform under "arr[0]"; accept the bare "arr" + // spelling too. The reverse ("arr[0]" against a bare "arr" entry) is kept for + // robustness against non-suffixed reflection entries. + if (!name.empty() && name.back() != ']') { + const String suffixedName = name + "[0]"; + const Int suffixedIndex = m_program->getUniformIndex(suffixedName.c_str()); + if (suffixedIndex >= 0 && suffixedIndex < m_activeUniformCount && + m_program->getUniform(suffixedIndex).name == suffixedName) { + return suffixedIndex; + } + return -1; + } + if (name.length() <= 3 || name.compare(name.length() - 3, 3, "[0]") != 0) return -1; const String baseName = name.substr(0, name.length() - 3); const Int baseIndex = m_program->getUniformIndex(baseName.c_str()); @@ -136,11 +167,24 @@ namespace MobileGL::MG_State::GLState { } // GL_UNIFORM_ARRAY_STRIDE: byte stride of an array member in a named block; 0 for a non-array - // block member; -1 for a default-block uniform. glslang yields arrayStride==0 for the - // default-block case, so gate on block membership to return the spec-mandated -1. + // block member; -1 for a default-block uniform (glslang yields arrayStride==0 there, so gate + // on block membership for the spec-mandated -1). The stride itself is derived from the type + // instead of glslang's reflected arrayStride: for an array nested inside a struct member, + // glslang computes that field against the enclosing STRUCT's (unset) packing and reports a + // tight std430-like stride (ivec2 a[7] -> 8), even though its own member offsets and the + // generated SPIR-V lay the array out with std140 16-byte-rounded strides. MobileGL's UBO + // layout is always std140, where every array element stride rounds up to a vec4. GLint GetActiveUniformArrayStride(Uint index) const { const auto& uniform = m_program->getUniform(static_cast(index)); - return (uniform.index < 0) ? -1 : uniform.arrayStride; + if (uniform.index < 0) return -1; + const glslang::TType* type = uniform.getType(); + if (type == nullptr || !type->isArray()) return 0; + if (type->isMatrix()) { + const bool rowMajor = GetActiveUniformIsRowMajor(index) != 0; + const int vectors = rowMajor ? type->getMatrixRows() : type->getMatrixCols(); + return GetActiveUniformMatrixStride(index) * vectors; + } + return 16; // scalars and vectors: std140 rounds the element stride up to a vec4 } // GL_UNIFORM_IS_ROW_MAJOR: 1 only for a row-major matrix in a named block, else 0. The @@ -327,6 +371,11 @@ namespace MobileGL::MG_State::GLState { Uint GetUniformBlockIndex(const char* name) const { auto it = m_uniformBlockIndexByName.find(name); if (it != m_uniformBlockIndexByName.end()) return it->second; + // Instances of an arrayed block are reflected as "Block[0]".."Block[N-1]"; + // a bare "Block" query resolves to the first instance per GL semantics. + const String suffixedName = String(name) + "[0]"; + it = m_uniformBlockIndexByName.find(suffixedName); + if (it != m_uniformBlockIndexByName.end()) return it->second; return 0xFFFFFFFFu; // GL_INVALID_INDEX } Bool IsActiveUniformBlock(Uint index) const { @@ -335,7 +384,11 @@ namespace MobileGL::MG_State::GLState { } Uint GetUBOSizeAt(Uint index) const { if (!IsActiveUniformBlock(index)) return 0; - return m_program->getUniformBlock((Int)index).size; + // glslang reports the unpadded end offset of the last member, but a std140 block + // (like a std140 struct) occupies a vec4-rounded size, and that is what the + // backend compiles: ES drivers reject draws whose bound UBO range is smaller + // than the block (a block ending in ivec3 reported 12 while the driver needs 16). + return (m_program->getUniformBlock((Int)index).size + 15u) & ~15u; } const String& GetUniformBlockName(Uint index) const { @@ -343,8 +396,30 @@ namespace MobileGL::MG_State::GLState { return ubo.name; } + // Uniform entries that belong to an arrayed uniform block are reflected once, against + // the first instance ("Block[0]"); per GL semantics every other instance shares that + // member set. Maps any instance's block index to the index owning the member entries. + Uint GetUniformBlockMemberOwnerIndex(Uint index) const { + const String& name = GetUniformBlockName(index); + if (name.empty() || name.back() != ']') return index; + const SizeT bracket = name.rfind('['); + if (bracket == String::npos) return index; + const auto it = m_uniformBlockIndexByName.find(name.substr(0, bracket) + "[0]"); + if (it != m_uniformBlockIndexByName.end()) return it->second; + return index; + } + + // GL_UNIFORM_BLOCK_ACTIVE_UNIFORMS: derived from the same active-uniform scan that + // fills GL_UNIFORM_BLOCK_ACTIVE_UNIFORM_INDICES, so the two queries always agree + // (glslang's numMembers counts declared members, which diverges from the reflected + // entry list for struct arrays and arrayed block instances). Int GetUniformBlockActiveUniformCount(Uint index) const { - return m_program->getUniformBlock((Int)index).numMembers; + const Int ownerIndex = static_cast(GetUniformBlockMemberOwnerIndex(index)); + Int count = 0; + for (Uint uniformIndex = 0; uniformIndex < m_activeUniformCount; ++uniformIndex) { + if (GetActiveUniformBlockIndex(uniformIndex) == ownerIndex) ++count; + } + return count; } Bool IsUniformBlockReferencedByStage(Uint index, EShLanguage stage) const { diff --git a/MobileGL/MG_State/GLState/ProgramState/ProgramState.cpp b/MobileGL/MG_State/GLState/ProgramState/ProgramState.cpp index 53e19880..1197f7aa 100644 --- a/MobileGL/MG_State/GLState/ProgramState/ProgramState.cpp +++ b/MobileGL/MG_State/GLState/ProgramState/ProgramState.cpp @@ -29,9 +29,18 @@ namespace MobileGL::MG_State::GLState { if (!CheckIndexAvail(program, m_programObjects)) return; // FIXME: add error reporting here auto& programObject = m_programObjects[program]; if (programObject != nullptr) { + // Snapshot the attachments: deleting the program is a detach point for shaders + // that were flagged with glDeleteShader while still attached. + const Vector> attachedShaders = programObject->GetAttachedShaders(); programObject->MarkAsDeleted(); programObject.reset(); m_programIndexGenerator.Delete(program); + for (const auto& shader : attachedShaders) { + const Uint shaderName = shader->GetExternalIndex(); + if (CheckIndexAvail(shaderName, m_shaderObjects) && m_shaderObjects[shaderName] == shader) { + ReleaseShaderNameIfOrphaned(shaderName); + } + } } } @@ -66,12 +75,35 @@ namespace MobileGL::MG_State::GLState { if (!CheckIndexAvail(shader, m_shaderObjects)) return; auto& shaderObject = m_shaderObjects[shader]; if (shaderObject != nullptr) { - m_shaderObjects[shader]->MarkAsDeleted(); - m_shaderObjects[shader].reset(); - m_shaderIndexGenerator.Delete(shader); + // glDeleteShader on an attached shader only FLAGS it; the name stays valid (and + // glShaderSource/glCompileShader keep working on it) until the shader is detached + // from every program. The GL CTS compiles shaders through exactly this + // create-attach-delete-source-compile sequence (uniform_block.common.name_matching). + shaderObject->MarkAsDeleted(); + ReleaseShaderNameIfOrphaned(shader); } } + Bool ProgramState::ShaderHasGLVisibleAttachment(const SharedPtr& shaderObject) const { + for (const auto& programObject : m_programObjects) { + if (programObject != nullptr && programObject->ShaderIsAttachedGLVisible(shaderObject)) { + return true; + } + } + // A program deleted while current vacates its table slot but stays alive as the + // current program; its attachments still count. + return m_currentProgram != nullptr && m_currentProgram->ShaderIsAttachedGLVisible(shaderObject); + } + + void ProgramState::ReleaseShaderNameIfOrphaned(Uint shader) { + if (!CheckIndexAvail(shader, m_shaderObjects)) return; + auto& shaderObject = m_shaderObjects[shader]; + if (shaderObject == nullptr || !shaderObject->GetDeleteStatus()) return; + if (ShaderHasGLVisibleAttachment(shaderObject)) return; + shaderObject.reset(); + m_shaderIndexGenerator.Delete(shader); + } + Bool ProgramState::ValidateShaderObject(Uint shader) const { return CheckIndexAvail(shader, m_shaderObjects) && m_shaderObjects[shader] != nullptr; } diff --git a/MobileGL/MG_State/GLState/ProgramState/ProgramState.h b/MobileGL/MG_State/GLState/ProgramState/ProgramState.h index afdfe78f..c48ec59f 100644 --- a/MobileGL/MG_State/GLState/ProgramState/ProgramState.h +++ b/MobileGL/MG_State/GLState/ProgramState/ProgramState.h @@ -26,11 +26,16 @@ namespace MobileGL::MG_State::GLState { Uint CreateShader(ShaderStage stage); const SharedPtr& GetShaderObject(Uint shader); void MarkShaderObjectForDeletion(Uint shader); + // Frees a deletion-flagged shader's name once no program holds a GL-visible + // attachment to it (the deferred half of glDeleteShader-while-attached). + void ReleaseShaderNameIfOrphaned(Uint shader); Bool ValidateShaderObject(Uint shader) const; const SharedPtr& GetCurrentProgram() const { return m_currentProgram; } private: + Bool ShaderHasGLVisibleAttachment(const SharedPtr& shaderObject) const; + template static Bool CheckIndexAvail(const SizeT idx, const Vector& vec) { return idx < vec.size(); diff --git a/MobileGL/MG_Test/Program/ProgramTest.cpp b/MobileGL/MG_Test/Program/ProgramTest.cpp index 015445a1..a26699bc 100644 --- a/MobileGL/MG_Test/Program/ProgramTest.cpp +++ b/MobileGL/MG_Test/Program/ProgramTest.cpp @@ -2053,3 +2053,232 @@ void main() { EXPECT_EQ(readback, 9.0f); // untouched by the overlong write EXPECT_EQ(GetError(), GL_NO_ERROR); } + +// --------------------------------------------------------------------------- +// GL CTS KHR-GL33.shaders.uniform_block regression pack. MobileGL's SPIR-V +// pipeline lays every uniform block out as std140; the frontend implements the +// GL-visible consequences of that choice: packed/shared qualifiers compile (as +// std140), reflection uses GL naming ("arr[0]", per-element struct arrays), +// unused block members stay active, block sizes are vec4-padded, and array +// strides are std140 even for arrays nested inside struct members. +// --------------------------------------------------------------------------- + +TEST_F(ProgramTest, UniformBlockPackedAndSharedLayoutsCompileAsStd140) { + const char* fsSource = R"(#version 330 +layout(packed) uniform PackedBlock { + vec4 pv; +}; +layout(shared, row_major) uniform SharedBlock { + float sf; + mat4 sm; +}; +out vec4 o_color; +void main() { + o_color = pv + vec4(sf) + vec4(sm[0][0]); +})"; + GLuint program = LinkVsFsProgram(kPassthroughCoordsVs, fsSource); + + // The blocks land on the implementation's chosen layout: std140 offsets. + const GLuint pv = UniformIndexByName(program, "pv"); + const GLuint sf = UniformIndexByName(program, "sf"); + const GLuint sm = UniformIndexByName(program, "sm"); + ASSERT_NE(pv, GL_INVALID_INDEX); + ASSERT_NE(sf, GL_INVALID_INDEX); + ASSERT_NE(sm, GL_INVALID_INDEX); + EXPECT_EQ(QueryUniformiv(program, pv, GL_UNIFORM_OFFSET), 0); + EXPECT_EQ(QueryUniformiv(program, sf, GL_UNIFORM_OFFSET), 0); + EXPECT_EQ(QueryUniformiv(program, sm, GL_UNIFORM_OFFSET), 16); + // The remaining qualifiers in the rewritten layout() list survive. + EXPECT_EQ(QueryUniformiv(program, sm, GL_UNIFORM_IS_ROW_MAJOR), 1); + EXPECT_EQ(GetError(), GL_NO_ERROR); +} + +TEST_F(ProgramTest, UniformBlockReflectsUnusedMembersWithGLNamesAndPaddedSize) { + const char* fsSource = R"(#version 330 +layout(std140) uniform Blk { + float used; + vec4 unusedArr[3]; + ivec3 tail; +}; +out vec4 o_color; +void main() { + o_color = vec4(used); +})"; + GLuint program = LinkVsFsProgram(kPassthroughCoordsVs, fsSource); + + const GLuint blockIndex = GetUniformBlockIndex(program, "Blk"); + ASSERT_NE(blockIndex, GL_INVALID_INDEX); + + // All three members are active (unusedArr and tail are never read), the array is + // reported under its GL name "unusedArr[0]", and both spellings resolve. + const GLuint used = UniformIndexByName(program, "used"); + const GLuint unusedSuffixed = UniformIndexByName(program, "unusedArr[0]"); + const GLuint unusedBare = UniformIndexByName(program, "unusedArr"); + const GLuint tail = UniformIndexByName(program, "tail"); + ASSERT_NE(used, GL_INVALID_INDEX); + ASSERT_NE(unusedSuffixed, GL_INVALID_INDEX); + ASSERT_NE(tail, GL_INVALID_INDEX); + EXPECT_EQ(unusedSuffixed, unusedBare); + + char nameBuf[64] = ""; + GLsizei nameLen = 0; + GLint arraySize = 0; + GLenum type = 0; + GetActiveUniform(program, unusedSuffixed, sizeof(nameBuf), &nameLen, &arraySize, &type, nameBuf); + EXPECT_STREQ(nameBuf, "unusedArr[0]"); + EXPECT_EQ(arraySize, 3); + EXPECT_EQ(type, static_cast(GL_FLOAT_VEC4)); + + // std140 layout of the unused members. + EXPECT_EQ(QueryUniformiv(program, unusedSuffixed, GL_UNIFORM_OFFSET), 16); + EXPECT_EQ(QueryUniformiv(program, unusedSuffixed, GL_UNIFORM_ARRAY_STRIDE), 16); + EXPECT_EQ(QueryUniformiv(program, tail, GL_UNIFORM_OFFSET), 64); + + // GL_UNIFORM_BLOCK_ACTIVE_UNIFORMS agrees with the INDICES list and counts all members. + GLint activeInBlock = 0; + GetActiveUniformBlockiv(program, blockIndex, GL_UNIFORM_BLOCK_ACTIVE_UNIFORMS, &activeInBlock); + ASSERT_EQ(activeInBlock, 3); + GLint indices[3] = {-1, -1, -1}; + GetActiveUniformBlockiv(program, blockIndex, GL_UNIFORM_BLOCK_ACTIVE_UNIFORM_INDICES, indices); + for (GLint index : indices) { + EXPECT_TRUE(index == static_cast(used) || index == static_cast(unusedSuffixed) || + index == static_cast(tail)); + } + + // The block ends with an ivec3 at offset 64 (unpadded end 76); the backend compiles + // the std140 block at its vec4-padded size, and the reported size must cover it or + // buffers sized from this query are too small to draw with. + GLint dataSize = 0; + GetActiveUniformBlockiv(program, blockIndex, GL_UNIFORM_BLOCK_DATA_SIZE, &dataSize); + EXPECT_EQ(dataSize, 80); + EXPECT_EQ(GetError(), GL_NO_ERROR); +} + +TEST_F(ProgramTest, UniformBlockStructArrayExpandsPerElementWithStd140Strides) { + const char* fsSource = R"(#version 330 +struct S { + ivec2 v[2]; + float f; +}; +layout(std140) uniform Blk2 { + S s[2]; +} inst; +out vec4 o_color; +void main() { + o_color = vec4(inst.s[0].f); +})"; + GLuint program = LinkVsFsProgram(kPassthroughCoordsVs, fsSource); + + // ARB_program_interface_query naming: one entry per struct array element, prefixed + // with the BLOCK name (not the instance name), basic arrays suffixed with "[0]". + const GLuint v0 = UniformIndexByName(program, "Blk2.s[0].v[0]"); + const GLuint f0 = UniformIndexByName(program, "Blk2.s[0].f"); + const GLuint v1 = UniformIndexByName(program, "Blk2.s[1].v[0]"); + const GLuint f1 = UniformIndexByName(program, "Blk2.s[1].f"); + ASSERT_NE(v0, GL_INVALID_INDEX); + ASSERT_NE(f0, GL_INVALID_INDEX); + ASSERT_NE(v1, GL_INVALID_INDEX); + ASSERT_NE(f1, GL_INVALID_INDEX); + + // std140: ivec2 v[2] rounds each element up to a vec4 (stride 16, NOT the tight 8 + // glslang reflects for arrays nested inside a struct member); struct size rounds to + // 48, giving s[1] members a 48-byte bias. + EXPECT_EQ(QueryUniformiv(program, v0, GL_UNIFORM_OFFSET), 0); + EXPECT_EQ(QueryUniformiv(program, v0, GL_UNIFORM_ARRAY_STRIDE), 16); + EXPECT_EQ(QueryUniformiv(program, v0, GL_UNIFORM_SIZE), 2); + EXPECT_EQ(QueryUniformiv(program, f0, GL_UNIFORM_OFFSET), 32); + EXPECT_EQ(QueryUniformiv(program, v1, GL_UNIFORM_OFFSET), 48); + EXPECT_EQ(QueryUniformiv(program, f1, GL_UNIFORM_OFFSET), 80); + + GLint dataSize = 0; + const GLuint blockIndex = GetUniformBlockIndex(program, "Blk2"); + ASSERT_NE(blockIndex, GL_INVALID_INDEX); + GetActiveUniformBlockiv(program, blockIndex, GL_UNIFORM_BLOCK_DATA_SIZE, &dataSize); + EXPECT_EQ(dataSize, 96); + EXPECT_EQ(GetError(), GL_NO_ERROR); +} + +TEST_F(ProgramTest, UniformBlockInstanceArrayReportsPerInstanceBlocks) { + const char* fsSource = R"(#version 330 +layout(std140) uniform ArrBlk { + vec4 av; +} insts[2]; +out vec4 o_color; +void main() { + o_color = insts[0].av + insts[1].av; +})"; + GLuint program = LinkVsFsProgram(kPassthroughCoordsVs, fsSource); + + const GLuint inst0 = GetUniformBlockIndex(program, "ArrBlk[0]"); + const GLuint inst1 = GetUniformBlockIndex(program, "ArrBlk[1]"); + ASSERT_NE(inst0, GL_INVALID_INDEX); + ASSERT_NE(inst1, GL_INVALID_INDEX); + EXPECT_NE(inst0, inst1); + // A bare block name resolves to the first instance. + EXPECT_EQ(GetUniformBlockIndex(program, "ArrBlk"), inst0); + + // Every instance of the array shares the single reflected member set. + GLint count0 = 0; + GLint count1 = 0; + GetActiveUniformBlockiv(program, inst0, GL_UNIFORM_BLOCK_ACTIVE_UNIFORMS, &count0); + GetActiveUniformBlockiv(program, inst1, GL_UNIFORM_BLOCK_ACTIVE_UNIFORMS, &count1); + EXPECT_EQ(count0, 1); + EXPECT_EQ(count1, 1); + GLint index0 = -1; + GLint index1 = -1; + GetActiveUniformBlockiv(program, inst0, GL_UNIFORM_BLOCK_ACTIVE_UNIFORM_INDICES, &index0); + GetActiveUniformBlockiv(program, inst1, GL_UNIFORM_BLOCK_ACTIVE_UNIFORM_INDICES, &index1); + EXPECT_EQ(index0, index1); + EXPECT_EQ(static_cast(index0), UniformIndexByName(program, "ArrBlk.av")); + EXPECT_EQ(GetError(), GL_NO_ERROR); +} + +TEST_F(ProgramTest, DeleteShaderWhileAttachedKeepsNameUsableUntilDetach) { + // GL CTS compiles through exactly this sequence (create, attach, DELETE, source, + // compile): glDeleteShader on an attached shader only flags it, and the name must + // keep working until the last detach. + const char* vsSource = R"(#version 330 +void main() { gl_Position = vec4(0.0); } +)"; + const char* fsSource = R"(#version 330 +out vec4 o_color; +void main() { o_color = vec4(1.0); } +)"; + + GLuint program = CreateProgram(); + GLuint vs = CreateShader(GL_VERTEX_SHADER); + AttachShader(program, vs); + DeleteShader(vs); + EXPECT_EQ(IsShader(vs), GL_TRUE); // still alive: attached + + ShaderSource(vs, 1, &vsSource, nullptr); + CompileShader(vs); + GLint status = GL_FALSE; + GetShaderiv(vs, GL_COMPILE_STATUS, &status); + EXPECT_EQ(status, GL_TRUE); + status = GL_FALSE; + GetShaderiv(vs, GL_DELETE_STATUS, &status); + EXPECT_EQ(status, GL_TRUE); + + GLuint fs = CreateShader(GL_FRAGMENT_SHADER); + AttachShader(program, fs); + DeleteShader(fs); + ShaderSource(fs, 1, &fsSource, nullptr); + CompileShader(fs); + + LinkProgram(program); + GLint linkStatus = GL_FALSE; + char infoLog[1024] = ""; + GetProgramiv(program, GL_LINK_STATUS, &linkStatus); + GetProgramInfoLog(program, sizeof(infoLog), nullptr, infoLog); + EXPECT_EQ(linkStatus, GL_TRUE) << infoLog; + + // The last GL-visible detach releases the flagged shader's name. + DetachShader(program, vs); + EXPECT_EQ(IsShader(vs), GL_FALSE); + + // Deleting the program releases the other flagged shader. + DeleteProgram(program); + EXPECT_EQ(IsShader(fs), GL_FALSE); + EXPECT_EQ(GetError(), GL_NO_ERROR); +} diff --git a/MobileGL/MG_Test/Program/ProgramUtilTest.cpp b/MobileGL/MG_Test/Program/ProgramUtilTest.cpp index 652826f6..40cbd817 100644 --- a/MobileGL/MG_Test/Program/ProgramUtilTest.cpp +++ b/MobileGL/MG_Test/Program/ProgramUtilTest.cpp @@ -1017,3 +1017,57 @@ void main() { ASSERT_TRUE(ShaderCompiler::SanitizeAndOptimizeBinary(binRes->at(0), optimized)); } + +TEST_F(ProgramUtilTest, PreprocessCoercesBlockPackingQualifiersToStd140) { + using namespace MG_Util::ShaderTranspiler; + + // glslang rejects `packed`/`shared` outright when generating SPIR-V, and MobileGL's + // UBO layout is always std140 anyway; the preprocessor rewrites the qualifiers so the + // validation compile, reflection, and generated SPIR-V all agree on std140 (GL CTS + // KHR-GL33.shaders.uniform_block.*.packed/shared). + String source = R"(#version 330 +layout(packed) uniform PackedBlock { vec4 pv; }; +layout(shared, row_major) uniform SharedBlock { mat4 sm; }; +layout ( shared ) uniform SpacedBlock { float sx; }; +layout(std140) uniform KeptBlock { float kx; }; +// A non-layout use of the identifier stays untouched (compute storage qualifier). +void main() { + gl_Position = pv + vec4(sm[0][0]) + vec4(sx) + vec4(kx); +})"; + + PreprocessShaderSource(ShaderStage::Vertex, source); + + EXPECT_EQ(source.find("packed"), String::npos); + EXPECT_EQ(source.find("layout(shared"), String::npos); + EXPECT_NE(source.find("layout(std140) uniform PackedBlock"), String::npos); + EXPECT_NE(source.find("layout(std140, row_major) uniform SharedBlock"), String::npos); + EXPECT_NE(source.find("layout ( std140 ) uniform SpacedBlock"), String::npos); + EXPECT_NE(source.find("layout(std140) uniform KeptBlock"), String::npos); + + ShaderAttrib attrib{.shaderType = GL_VERTEX_SHADER, + .sourceStr = source, + .flags = ShaderCompileBits::CompileForOpenGL}; + auto res = ShaderCompiler::CompileShader(attrib); + if (!res) { + FAIL() << "errc: " << res.error().errc << "\nlog: " << res.error().log << "\nsource:\n" << source; + } +} + +TEST_F(ProgramUtilTest, PreprocessLeavesComputeSharedStorageQualifierAlone) { + using namespace MG_Util::ShaderTranspiler; + + // `shared` is only a packing qualifier inside layout(...); the compute-shader storage + // qualifier of the same spelling must survive. + String source = R"(#version 430 +layout(local_size_x = 8) in; +shared float sharedScratch[8]; +layout(shared) uniform Blk { float bx; }; +void main() { + sharedScratch[gl_LocalInvocationIndex] = bx; +})"; + + PreprocessShaderSource(ShaderStage::Compute, source); + + EXPECT_NE(source.find("shared float sharedScratch[8];"), String::npos); + EXPECT_NE(source.find("layout(std140) uniform Blk"), String::npos); +} diff --git a/MobileGL/MG_Util/ShaderTranspiler/ShaderSourceProcessor.cpp b/MobileGL/MG_Util/ShaderTranspiler/ShaderSourceProcessor.cpp index 43c2d5aa..70e3f62e 100644 --- a/MobileGL/MG_Util/ShaderTranspiler/ShaderSourceProcessor.cpp +++ b/MobileGL/MG_Util/ShaderTranspiler/ShaderSourceProcessor.cpp @@ -261,6 +261,65 @@ namespace { ReplaceIdentifier(source, "GL_ARB_gpu_shader_int64", "MG_DISABLED_GL_ARB_gpu_shader_int64"); } + // Rewrite the `packed` / `shared` block-packing qualifiers inside layout(...) declarations to + // `std140`. Desktop GL leaves the memory layout of such blocks to the implementation and the + // app must query member offsets; MobileGL's SPIR-V pipeline always lays uniform blocks out as + // std140 (glslang under a SPIR-V target rejects `packed`/`shared` outright and SPIRV-Cross has + // no other packing for UBOs), so std140 IS this implementation's chosen layout. Rewriting at + // the source level keeps the validation compile, the reflection the app queries, and the + // generated SPIR-V all agreeing on that choice. Both replacement tokens are 6 characters, so + // the rewrite is done in place. + void CoerceUniformBlockPackingToStd140(MobileGL::String& source) { + constexpr const char* layoutToken = "layout"; + constexpr SizeT layoutLen = 6; + + SizeT pos = 0; + while ((pos = source.find(layoutToken, pos)) != MobileGL::String::npos) { + const bool hasLeftBoundary = pos == 0 || !IsIdentifierChar(source[pos - 1]); + SizeT probe = pos + layoutLen; + const bool hasRightBoundary = probe >= source.size() || !IsIdentifierChar(source[probe]); + if (!hasLeftBoundary || !hasRightBoundary) { + pos = probe; + continue; + } + + while (probe < source.size() && std::isspace(static_cast(source[probe]))) { + probe++; + } + if (probe >= source.size() || source[probe] != '(') { + pos = probe; + continue; + } + + // Scan the qualifier list; layout qualifier values may contain parenthesized + // constant expressions, so track nesting until the matching ')'. + SizeT cursor = probe + 1; + int depth = 1; + while (cursor < source.size() && depth > 0) { + const char ch = source[cursor]; + if (ch == '(') { + depth++; + } else if (ch == ')') { + depth--; + } else if (IsIdentifierChar(ch) && (cursor == 0 || !IsIdentifierChar(source[cursor - 1]))) { + SizeT identifierEnd = cursor; + while (identifierEnd < source.size() && IsIdentifierChar(source[identifierEnd])) { + identifierEnd++; + } + const SizeT identifierLen = identifierEnd - cursor; + if (identifierLen == 6 && (source.compare(cursor, 6, "packed") == 0 || + source.compare(cursor, 6, "shared") == 0)) { + source.replace(cursor, 6, "std140"); + } + cursor = identifierEnd; + continue; + } + cursor++; + } + pos = cursor; + } + } + void ModernizeLegacyGLSL(MobileGL::ShaderStage stage, MobileGL::String& source) { // Precision qualifiers (highp/mediump/lowp and default-precision statements) are legal and // ignored in the forced "#version 460 core" profile, so glslang handles them natively. @@ -383,6 +442,7 @@ namespace MobileGL { } FilterUnsupportedGpuShaderInt64(source); + CoerceUniformBlockPackingToStd140(source); // Some shader packs define helpers with built-in GLSL names such as round(), tanh(), or fma(). // These may pass OpenGL-style validation but fail when recompiled for Vulkan/SPIR-V generation.