From f5a0779385382d6b3e6f12212c4bf6f93a8e3776 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Fri, 21 Aug 2026 11:54:45 -0400 Subject: [PATCH 1/2] [Fix, Test] (GLState): keep the atomic-counter and storage blocks out of the GL uniform-block list --- .../MG_Impl/GLImpl/Program/GL_Program.cpp | 54 ++++-- .../GLImpl/Program/ProgramInterface.cpp | 24 ++- .../GLState/ProgramState/ProgramLinkTask.cpp | 65 ++++++- .../GLState/ProgramState/ProgramObject.cpp | 2 + .../GLState/ProgramState/ProgramObject.h | 100 +++++++++-- MobileGL/MG_Test/Program/ProgramTest.cpp | 166 ++++++++++++++++++ 6 files changed, 368 insertions(+), 43 deletions(-) diff --git a/MobileGL/MG_Impl/GLImpl/Program/GL_Program.cpp b/MobileGL/MG_Impl/GLImpl/Program/GL_Program.cpp index 9fe2ef1e..9097eb78 100644 --- a/MobileGL/MG_Impl/GLImpl/Program/GL_Program.cpp +++ b/MobileGL/MG_Impl/GLImpl/Program/GL_Program.cpp @@ -680,7 +680,9 @@ namespace MobileGL::MG_Impl::GLImpl { MGLOG_D("%s: %s = %d", __func__, MG_Util::ConvertGLEnumToString(pname).c_str(), *params); break; case GL_ACTIVE_UNIFORM_BLOCKS: // GL >= 3.1 - *params = programObject->GetActiveUniformBlocksCount(); + // Uniform blocks only. GetActiveUniformBlocksCount() is the internal block space, + // which also carries the storage blocks and the synthesized atomic counter blocks. + *params = programObject->GetGlUniformBlockCount(); MGLOG_D("%s: %s = %d", __func__, MG_Util::ConvertGLEnumToString(pname).c_str(), *params); break; case GL_ACTIVE_UNIFORM_BLOCK_MAX_NAME_LENGTH: // ditto. @@ -1716,7 +1718,10 @@ namespace MobileGL::MG_Impl::GLImpl { return GL_INVALID_INDEX; } - const auto& index = programObject->GetUniformBlockIndex(uniformBlockName); + // GetGlUniformBlockIndex, not GetUniformBlockIndex: the latter answers in the internal + // block space, which also resolves storage blocks and the synthesized atomic counter + // blocks. Neither is a uniform block (GL 4.6 core 7.6), so both are GL_INVALID_INDEX here. + const auto index = programObject->GetGlUniformBlockIndex(uniformBlockName); MGLOG_D("GBI prog=%u name='%s' -> %d", program, uniformBlockName ? uniformBlockName : "(null)", (Int)index); return index; } @@ -1730,7 +1735,7 @@ namespace MobileGL::MG_Impl::GLImpl { "Program object" + std::to_string(program) + " that has been linked.")); return; } - if (!programObject->IsActiveUniformBlock(uniformBlockIndex)) { + if (!programObject->IsActiveGlUniformBlock(uniformBlockIndex)) { MG_State::pGLContext->RecordError( ErrorCode::InvalidValue, MakeUnique( @@ -1741,8 +1746,11 @@ namespace MobileGL::MG_Impl::GLImpl { std::to_string(program) + ".")); return; } + // The GL_UNIFORM_BLOCK index space skips the storage and atomic counter blocks the + // block-keyed tables still carry; translate before touching them. + const Uint blockIndex = static_cast(programObject->BlockIndexFromGlUniformBlock(uniformBlockIndex)); MGLOG_D("UBB prog=%u idx=%u binding=%u", program, uniformBlockIndex, uniformBlockBinding); - programObject->SetUniformBlockBinding(uniformBlockIndex, uniformBlockBinding); + programObject->SetUniformBlockBinding(blockIndex, uniformBlockBinding); } void GetActiveUniformBlockiv_State(GLuint program, GLuint uniformBlockIndex, GLenum pname, GLint* params) { @@ -1754,7 +1762,7 @@ namespace MobileGL::MG_Impl::GLImpl { "Program object" + std::to_string(program) + " that has been linked.")); return; } - if (!programObject->IsActiveUniformBlock(uniformBlockIndex)) { + if (!programObject->IsActiveGlUniformBlock(uniformBlockIndex)) { MG_State::pGLContext->RecordError( ErrorCode::InvalidValue, MakeUnique( @@ -1765,61 +1773,68 @@ namespace MobileGL::MG_Impl::GLImpl { std::to_string(program) + ".")); return; } + // The GL_UNIFORM_BLOCK index space skips the storage and atomic counter blocks the + // block-keyed tables still carry; every accessor below is indexed by the block space. + const Uint blockIndex = static_cast(programObject->BlockIndexFromGlUniformBlock(uniformBlockIndex)); switch (pname) { case GL_UNIFORM_BLOCK_DATA_SIZE: { - *params = (GLint)programObject->GetUBOSizeAt(uniformBlockIndex); + *params = (GLint)programObject->GetUBOSizeAt(blockIndex); MGLOG_D("%s: GL_UNIFORM_BLOCK_DATA_SIZE = %d", __func__, *params); break; } case GL_UNIFORM_BLOCK_NAME_LENGTH: { - *params = (GLint)programObject->GetUniformBlockName(uniformBlockIndex).length() + 1; + *params = (GLint)programObject->GetUniformBlockName(blockIndex).length() + 1; MGLOG_D("%s: GL_UNIFORM_BLOCK_NAME_LENGTH = %d", __func__, *params); break; } case GL_UNIFORM_BLOCK_ACTIVE_UNIFORMS: { - *params = programObject->GetUniformBlockActiveUniformCount(uniformBlockIndex); + *params = programObject->GetUniformBlockActiveUniformCount(blockIndex); MGLOG_D("%s: GL_UNIFORM_BLOCK_ACTIVE_UNIFORMS = %d", __func__, *params); break; } case GL_UNIFORM_BLOCK_BINDING: { - *params = static_cast(programObject->GetUniformBlockBinding(uniformBlockIndex)); + *params = static_cast(programObject->GetUniformBlockBinding(blockIndex)); MGLOG_D("%s: GL_UNIFORM_BLOCK_BINDING = %d", __func__, *params); break; } case GL_UNIFORM_BLOCK_REFERENCED_BY_VERTEX_SHADER: - *params = BoolToGLInt(programObject->IsUniformBlockReferencedByStage(uniformBlockIndex, EShLangVertex)); + *params = BoolToGLInt(programObject->IsUniformBlockReferencedByStage(blockIndex, EShLangVertex)); MGLOG_D("%s: GL_UNIFORM_BLOCK_REFERENCED_BY_VERTEX_SHADER = %d", __func__, *params); break; case GL_UNIFORM_BLOCK_REFERENCED_BY_TESS_CONTROL_SHADER: *params = - BoolToGLInt(programObject->IsUniformBlockReferencedByStage(uniformBlockIndex, EShLangTessControl)); + BoolToGLInt(programObject->IsUniformBlockReferencedByStage(blockIndex, EShLangTessControl)); MGLOG_D("%s: GL_UNIFORM_BLOCK_REFERENCED_BY_TESS_CONTROL_SHADER = %d", __func__, *params); break; case GL_UNIFORM_BLOCK_REFERENCED_BY_TESS_EVALUATION_SHADER: *params = - BoolToGLInt(programObject->IsUniformBlockReferencedByStage(uniformBlockIndex, EShLangTessEvaluation)); + BoolToGLInt(programObject->IsUniformBlockReferencedByStage(blockIndex, EShLangTessEvaluation)); MGLOG_D("%s: GL_UNIFORM_BLOCK_REFERENCED_BY_TESS_EVALUATION_SHADER = %d", __func__, *params); break; case GL_UNIFORM_BLOCK_REFERENCED_BY_GEOMETRY_SHADER: - *params = BoolToGLInt(programObject->IsUniformBlockReferencedByStage(uniformBlockIndex, EShLangGeometry)); + *params = BoolToGLInt(programObject->IsUniformBlockReferencedByStage(blockIndex, EShLangGeometry)); MGLOG_D("%s: GL_UNIFORM_BLOCK_REFERENCED_BY_GEOMETRY_SHADER = %d", __func__, *params); break; case GL_UNIFORM_BLOCK_REFERENCED_BY_FRAGMENT_SHADER: - *params = BoolToGLInt(programObject->IsUniformBlockReferencedByStage(uniformBlockIndex, EShLangFragment)); + *params = BoolToGLInt(programObject->IsUniformBlockReferencedByStage(blockIndex, EShLangFragment)); MGLOG_D("%s: GL_UNIFORM_BLOCK_REFERENCED_BY_FRAGMENT_SHADER = %d", __func__, *params); break; case GL_UNIFORM_BLOCK_REFERENCED_BY_COMPUTE_SHADER: - *params = BoolToGLInt(programObject->IsUniformBlockReferencedByStage(uniformBlockIndex, EShLangCompute)); + *params = BoolToGLInt(programObject->IsUniformBlockReferencedByStage(blockIndex, EShLangCompute)); 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)); + // + // Both sides of the comparison are BLOCK indices: GetUniformBlockMemberOwnerIndex + // answers in that space, so the scan uses GetActiveUniformOwnerBlockIndex rather + // than the GL_UNIFORM_BLOCK-space GetActiveUniformBlockIndex. + const Int ownerIndex = static_cast(programObject->GetUniformBlockMemberOwnerIndex(blockIndex)); GLint uniformIndexCount = 0; for (Uint uniformIndex = 0; uniformIndex < programObject->GetUniformCount(); ++uniformIndex) { - if (programObject->GetActiveUniformBlockIndex(uniformIndex) != ownerIndex) { + if (programObject->GetActiveUniformOwnerBlockIndex(uniformIndex) != ownerIndex) { continue; } params[uniformIndexCount++] = static_cast(uniformIndex); @@ -1849,7 +1864,7 @@ namespace MobileGL::MG_Impl::GLImpl { " is not a program object that has been linked.")); return; } - if (!programObject->IsActiveUniformBlock(uniformBlockIndex)) { + if (!programObject->IsActiveGlUniformBlock(uniformBlockIndex)) { MG_State::pGLContext->RecordError( ErrorCode::InvalidValue, MakeUnique( @@ -1859,7 +1874,8 @@ namespace MobileGL::MG_Impl::GLImpl { "not the index of an active uniform block in program.")); return; } - const auto& name = programObject->GetUniformBlockName(uniformBlockIndex); + const auto& name = programObject->GetUniformBlockName( + static_cast(programObject->BlockIndexFromGlUniformBlock(uniformBlockIndex))); CopyStr(bufSize, length, uniformBlockName, name.c_str(), (GLsizei)name.length()); MGLOG_D("%s: \"%s\" at uniformBlockIndex %02d, length = %d", __func__, uniformBlockName, uniformBlockIndex, length ? *length : 0); diff --git a/MobileGL/MG_Impl/GLImpl/Program/ProgramInterface.cpp b/MobileGL/MG_Impl/GLImpl/Program/ProgramInterface.cpp index 092b2ea3..a6834dd6 100644 --- a/MobileGL/MG_Impl/GLImpl/Program/ProgramInterface.cpp +++ b/MobileGL/MG_Impl/GLImpl/Program/ProgramInterface.cpp @@ -305,13 +305,17 @@ namespace MobileGL::MG_Impl::GLImpl::ProgramInterface { // GL_UNIFORM_BLOCK keeps the index space glUniformBlockBinding and // glGetActiveUniformBlockiv already use, so an index handed out here is usable // with them (which is exactly what the CTS does). - const Int glBlockCount = program.GetActiveUniformBlocksCount(); + const Int glBlockCount = program.GetGlUniformBlockCount(); for (Int glIndex = 0; glIndex < glBlockCount; ++glIndex) { + // The block-space index the block-keyed accessors want; the two spaces differ + // whenever the program also has a storage or atomic counter block, which + // glslang files under the same reflection list (no EShReflectionSeparateBuffers). + const Int blockIndex = program.BlockIndexFromGlUniformBlock(static_cast(glIndex)); Resource resource; - resource.name = program.GetUniformBlockName(glIndex); - resource.bufferBinding = static_cast(program.GetUniformBlockBinding(glIndex)); - resource.bufferDataSize = static_cast(program.GetUBOSizeAt(glIndex)); - const Int tIndex = program.TProgramBlockIndex(static_cast(glIndex)); + resource.name = program.GetUniformBlockName(static_cast(blockIndex)); + resource.bufferBinding = static_cast(program.GetUniformBlockBinding(static_cast(blockIndex))); + resource.bufferDataSize = static_cast(program.GetUBOSizeAt(static_cast(blockIndex))); + const Int tIndex = program.TProgramBlockIndex(static_cast(blockIndex)); if (tIndex >= 0 && tIndex < blockCount) { resource.stages = UniformBlockStages(reflection.blockReflection[tIndex], stagesFromMembers, tIndex); @@ -388,12 +392,16 @@ namespace MobileGL::MG_Impl::GLImpl::ProgramInterface { static_cast(i)); } } - for (SizeT blockIndex = 0; blockIndex < model.uniformBlocks.size(); ++blockIndex) { + for (SizeT glBlockIndex = 0; glBlockIndex < model.uniformBlocks.size(); ++glBlockIndex) { // Members of an arrayed block are reflected once, against instance [0]. - const Int owner = static_cast(program.GetUniformBlockMemberOwnerIndex(static_cast(blockIndex))); + // GetUniformBlockMemberOwnerIndex takes and answers BLOCK indices, while + // Resource::blockIndex is a GL_UNIFORM_BLOCK index, so translate both ways. + const Int blockIndex = program.BlockIndexFromGlUniformBlock(static_cast(glBlockIndex)); + const Int owner = program.GlUniformBlockIndexFromBlock( + static_cast(program.GetUniformBlockMemberOwnerIndex(static_cast(blockIndex)))); for (SizeT i = 0; i < model.uniforms.size(); ++i) { if (model.uniforms[i].blockIndex == owner) { - model.uniformBlocks[blockIndex].activeVariables.push_back(static_cast(i)); + model.uniformBlocks[glBlockIndex].activeVariables.push_back(static_cast(i)); } } } diff --git a/MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.cpp b/MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.cpp index 3860b355..7189ed0e 100644 --- a/MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.cpp +++ b/MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.cpp @@ -129,6 +129,34 @@ namespace { return element; } + // Blocks come out of reflection in three kinds and only one of them is a GL uniform block. + // The same split ProgramInterface::ClassifyBlock makes (it reads the flattened + // TypeFacts::isBuffer, which is this very qualifier), reachable here from the live TProgram + // because the block index spaces are built before the reflection snapshot exists. + + // The transpiler lowers every atomic_uint onto a synthesized "gl_AtomicCounterBlock_" + // buffer block, which reflection then reports as an ordinary block. It is not one: GL + // enumerates it through GL_ACTIVE_ATOMIC_COUNTER_BUFFERS instead. + static MobileGL::Bool IsAtomicCounterBlockName(const MobileGL::String& name) { + namespace Transpiler = MobileGL::MG_Util::ShaderTranspiler; + const MobileGL::SizeT prefixLength = std::strlen(Transpiler::ATOMIC_COUNTER_BLOCK_PREFIX); + return name.compare(0, prefixLength, Transpiler::ATOMIC_COUNTER_BLOCK_PREFIX) == 0; + } + + // A shader storage block: GL enumerates it through GL_SHADER_STORAGE_BLOCK and its members + // through GL_BUFFER_VARIABLE. The counter blocks above are buffer blocks too, hence the + // exclusion. A block whose type reflection did not survive is treated as a uniform block, + // which is what every caller assumed before this classification existed. + static MobileGL::Bool IsStorageBlock(const glslang::TObjectReflection& block) { + if (IsAtomicCounterBlockName(block.name)) return false; + const glslang::TType* type = block.getType(); + return type != nullptr && type->getQualifier().storage == glslang::EvqBuffer; + } + + static MobileGL::Bool IsGlUniformBlock(const glslang::TObjectReflection& block) { + return !IsAtomicCounterBlockName(block.name) && !IsStorageBlock(block); + } + // GL 4.6 core 7.7 / ARB_shader_atomic_counters: within one binding no two atomic counters // may occupy the same bytes, every offset is a multiple of 4, and no counter may reach past // GL_MAX_ATOMIC_COUNTER_BUFFER_SIZE. glslang enforces all three in fixOffset(), which the @@ -1009,6 +1037,30 @@ namespace MobileGL::MG_State::GLState { artifacts.glBlockIndexToTProgram.push_back(i); } + // The GL_UNIFORM_BLOCK subsequence of that space. MobileGL does not pass + // EShReflectionSeparateBuffers to buildReflection above, so glslang files BUFFER blocks + // under indexToUniformBlock as well and the list just built also holds every shader + // storage block and every synthesized gl_AtomicCounterBlock_N. GL 4.6 core 7.6 says + // GL_ACTIVE_UNIFORM_BLOCKS / glGetActiveUniformBlockiv / glGetUniformBlockIndex see + // uniform blocks and nothing else; an atomic counter buffer is enumerated by + // GL_ACTIVE_ATOMIC_COUNTER_BUFFERS and a storage block by GL_SHADER_STORAGE_BLOCK. + // + // A SECOND space rather than a filter of the first, deliberately: the block space is + // what the backends walk (DirectGLES hands out one ESSL uniform-buffer binding point per + // entry as it goes) and what "tProgramBlockIndexToGl[i] < 0 means MGL_GLOBAL_UBO" reads, + // and neither may move. + artifacts.blockIndexToGlUniformBlock.assign(artifacts.glBlockIndexToTProgram.size(), -1); + artifacts.glUniformBlockIndexToBlock.clear(); + for (SizeT blockIndex = 0; blockIndex < artifacts.glBlockIndexToTProgram.size(); ++blockIndex) { + const auto& block = artifacts.program->getUniformBlock(artifacts.glBlockIndexToTProgram[blockIndex]); + if (!IsGlUniformBlock(block)) continue; + artifacts.blockIndexToGlUniformBlock[blockIndex] = + static_cast(artifacts.glUniformBlockIndexToBlock.size()); + artifacts.glUniformBlockIndexToBlock.push_back(static_cast(blockIndex)); + } + MGLOG_D("ProgramObject %u: Reflection - %zu block(s), %zu of them GL uniform blocks", in.externalIndex, + artifacts.glBlockIndexToTProgram.size(), artifacts.glUniformBlockIndexToBlock.size()); + // ------------ Uniforms (GL Plain) ---------------- // The relaxed parse sweeps every DECLARED default-block uniform into // MGL_GLOBAL_UBO whether or not any stage reads it. GL requires a @@ -1472,14 +1524,21 @@ namespace MobileGL::MG_State::GLState { } // ---------- UBO ---------- - // GL-visible blocks only (MGL_GLOBAL_UBO was filtered out above). + // The BLOCK space (MGL_GLOBAL_UBO was filtered out above, storage and atomic counter + // blocks were not): these tables are what the backends index, and what the GL + // uniform-block entry points reach after translating out of the GL_UNIFORM_BLOCK space. const Int uboCount = static_cast(artifacts.glBlockIndexToTProgram.size()); MGLOG_D("ProgramObject %u: Reflection - uniform block count (UBO) = %d", in.externalIndex, uboCount); artifacts.uniformBlockBinding.resize(uboCount, -1); for (Int i = 0; i < uboCount; i++) { auto& ubo = artifacts.program->getUniformBlock(artifacts.glBlockIndexToTProgram[i]); - artifacts.uniformBlockNameMaxLength = - std::max(artifacts.uniformBlockNameMaxLength, (Int)ubo.name.length()); + // GL_ACTIVE_UNIFORM_BLOCK_MAX_NAME_LENGTH is measured over the names + // glGetActiveUniformBlockName can report, so only the GL uniform blocks count - + // a long storage-block name must not size the caller's buffer. + if (artifacts.blockIndexToGlUniformBlock[i] >= 0) { + artifacts.uniformBlockNameMaxLength = + std::max(artifacts.uniformBlockNameMaxLength, (Int)ubo.name.length()); + } artifacts.uniformBlockIndexByName[ubo.name] = i; // if there's binding defined in shader as layout(binding = ...), // retrieve it here. diff --git a/MobileGL/MG_State/GLState/ProgramState/ProgramObject.cpp b/MobileGL/MG_State/GLState/ProgramState/ProgramObject.cpp index e5da0fea..687387ea 100644 --- a/MobileGL/MG_State/GLState/ProgramState/ProgramObject.cpp +++ b/MobileGL/MG_State/GLState/ProgramState/ProgramObject.cpp @@ -331,6 +331,8 @@ namespace MobileGL::MG_State::GLState { artifacts.tProgramUniformIndexToGl.clear(); artifacts.glBlockIndexToTProgram.clear(); artifacts.tProgramBlockIndexToGl.clear(); + artifacts.glUniformBlockIndexToBlock.clear(); + artifacts.blockIndexToGlUniformBlock.clear(); artifacts.linkedExplicitUniformLocations.clear(); artifacts.uniformInitialValues.clear(); artifacts.uniformIndexInTProgram.clear(); diff --git a/MobileGL/MG_State/GLState/ProgramState/ProgramObject.h b/MobileGL/MG_State/GLState/ProgramState/ProgramObject.h index ee92e265..6928d6c4 100644 --- a/MobileGL/MG_State/GLState/ProgramState/ProgramObject.h +++ b/MobileGL/MG_State/GLState/ProgramState/ProgramObject.h @@ -279,12 +279,12 @@ namespace MobileGL::MG_State::GLState { if (tIndex < 0 || tIndex >= static_cast(Artifacts().tProgramUniformIndexToGl.size())) return -1; return Artifacts().tProgramUniformIndexToGl[tIndex]; } - // GL uniform-block index -> glslang TProgram block index (the inverse of + // Block index -> glslang TProgram block index (the inverse of // GlBlockIndexFromTProgram). The interface-query layer needs it to reach block // properties glslang exposes but no typed getter here does. - Int TProgramBlockIndex(Uint glBlockIndex) const { - return glBlockIndex < Artifacts().glBlockIndexToTProgram.size() - ? Artifacts().glBlockIndexToTProgram[glBlockIndex] + Int TProgramBlockIndex(Uint blockIndex) const { + return blockIndex < Artifacts().glBlockIndexToTProgram.size() + ? Artifacts().glBlockIndexToTProgram[blockIndex] : -1; } Int GlBlockIndexFromTProgram(Int tBlockIndex) const { @@ -292,6 +292,41 @@ namespace MobileGL::MG_State::GLState { return Artifacts().tProgramBlockIndexToGl[tBlockIndex]; } + // ---- GL_UNIFORM_BLOCK index <-> block index translation ---- + // The block index space above carries the storage blocks and the synthesized atomic + // counter blocks as well; GL_ACTIVE_UNIFORM_BLOCKS counts only actual uniform blocks + // (GL 4.6 core 7.6). Every glGetActiveUniformBlock* / glGetUniformBlockIndex / + // glUniformBlockBinding entry point speaks THIS space and translates into the block + // space before touching any of the block-keyed tables; the backends keep speaking the + // block space directly. See LinkArtifacts::glUniformBlockIndexToBlock. + Int GetGlUniformBlockCount() const { + return static_cast(Artifacts().glUniformBlockIndexToBlock.size()); + } + Bool IsActiveGlUniformBlock(Uint glUniformBlockIndex) const { + return glUniformBlockIndex < Artifacts().glUniformBlockIndexToBlock.size(); + } + Int BlockIndexFromGlUniformBlock(Uint glUniformBlockIndex) const { + return glUniformBlockIndex < Artifacts().glUniformBlockIndexToBlock.size() + ? Artifacts().glUniformBlockIndexToBlock[glUniformBlockIndex] + : -1; + } + Int GlUniformBlockIndexFromBlock(Int blockIndex) const { + if (blockIndex < 0 || blockIndex >= static_cast(Artifacts().blockIndexToGlUniformBlock.size())) { + return -1; + } + return Artifacts().blockIndexToGlUniformBlock[blockIndex]; + } + // glGetUniformBlockIndex: GL_INVALID_INDEX for a name that is not an active UNIFORM + // block, which includes every storage block and every atomic counter block even though + // GetUniformBlockIndex() below resolves them (it answers in the block space, which the + // backends need to keep reaching them by name). + Uint GetGlUniformBlockIndex(const char* name) const { + const Uint blockIndex = GetUniformBlockIndex(name); + if (blockIndex == 0xFFFFFFFFu) return 0xFFFFFFFFu; + const Int glIndex = GlUniformBlockIndexFromBlock(static_cast(blockIndex)); + return glIndex < 0 ? 0xFFFFFFFFu : static_cast(glIndex); + } + Int GetActiveUniformIndex(const String& name) const { // uniformIndexByName is keyed by the REFLECTED name, so a lookup that hits is // already the exact-match the old code re-verified with a string compare after @@ -340,7 +375,10 @@ namespace MobileGL::MG_State::GLState { return GetUniformArraySizeByTIndex(TProgramUniformIndex(index)); } - Int GetActiveUniformBlockIndex(Uint index) const { + // The BLOCK index of the block owning this active uniform, or -1 when it owns none as + // far as GL is concerned. Internal: pair it with another block-space index, never with + // a GL_UNIFORM_BLOCK one (GetActiveUniformBlockIndex below is that one). + Int GetActiveUniformOwnerBlockIndex(Uint index) const { // An atomic counter is a DEFAULT-BLOCK uniform to GL, whatever block the // transpiler lowered it onto (GL 4.6 core 7.6, table 7.6): -1. if (IsActiveUniformAtomicCounter(index)) return -1; @@ -348,6 +386,13 @@ namespace MobileGL::MG_State::GLState { return GlBlockIndexFromTProgram(UniformAt(TProgramUniformIndex(index)).index); } + // GL_UNIFORM_BLOCK_INDEX: an index into the GL_ACTIVE_UNIFORM_BLOCKS list, or -1. A + // buffer variable owns a storage block, which is not in that list, so it answers -1 too + // (and after the enumeration filter it is not an active uniform in the first place). + Int GetActiveUniformBlockIndex(Uint index) const { + return GlUniformBlockIndexFromBlock(GetActiveUniformOwnerBlockIndex(index)); + } + // The transpiler lowers every atomic_uint onto a synthesized gl_AtomicCounterBlock_N // block, but GL keeps seeing an atomic counter as a default-block uniform of type // GL_UNSIGNED_INT_ATOMIC_COUNTER that points at an atomic-counter BUFFER. These two @@ -875,14 +920,19 @@ namespace MobileGL::MG_State::GLState { Int GetActiveAttributesCount() const { return static_cast(Artifacts().pipeInputReflection.size()); } - // GL-visible uniform blocks only: the synthesized MGL_GLOBAL_UBO the relaxed parse - // materializes for default-block uniforms is filtered out by DoReflection. + // Size of the BLOCK index space - every block the relaxed parse produced except the + // synthesized MGL_GLOBAL_UBO, which DoReflection filters out. NOT the answer to + // glGetProgramiv(GL_ACTIVE_UNIFORM_BLOCKS): storage blocks and atomic counter blocks + // live in here too, and GetGlUniformBlockCount() is the one that excludes them. Int GetActiveUniformBlocksCount() const { return static_cast(Artifacts().glBlockIndexToTProgram.size()); } GLuint GetComputeLocalSize(Uint dim) const { return dim < 3u ? Artifacts().computeLocalSize[dim] : 0u; } Int GetActiveAttributesMaxLength() const { return Artifacts().attribInNameMaxLength; } Int GetActiveUniformBlocksMaxNameLength() const { return Artifacts().uniformBlockNameMaxLength; } + // Answers in the BLOCK space, so it resolves storage and atomic counter blocks too - + // the backends reach those by name. glGetUniformBlockIndex must NOT: use + // GetGlUniformBlockIndex() for the GL entry point. Uint GetUniformBlockIndex(const char* name) const { auto it = Artifacts().uniformBlockIndexByName.find(name); if (it != Artifacts().uniformBlockIndexByName.end()) return it->second; @@ -893,12 +943,11 @@ namespace MobileGL::MG_State::GLState { if (it != Artifacts().uniformBlockIndexByName.end()) return it->second; return 0xFFFFFFFFu; // GL_INVALID_INDEX } - Bool IsActiveUniformBlock(Uint index) const { - if (index >= GetActiveUniformBlocksCount()) return false; - return true; - } + // Takes a BLOCK index. The GL entry points validate their argument against the + // GL_UNIFORM_BLOCK space with IsActiveGlUniformBlock() first and translate; the bound + // test here is only the range of the space this index actually lives in. Uint GetUBOSizeAt(Uint index) const { - if (!IsActiveUniformBlock(index)) return 0; + if (index >= Artifacts().glBlockIndexToTProgram.size()) return 0; // 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 @@ -928,11 +977,14 @@ namespace MobileGL::MG_State::GLState { // 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). + // Takes a BLOCK index, and scans in the block space: GetUniformBlockMemberOwnerIndex + // answers there, so pairing it with the GL_UNIFORM_BLOCK-space + // GetActiveUniformBlockIndex would compare two different numberings. Int GetUniformBlockActiveUniformCount(Uint index) const { const Int ownerIndex = static_cast(GetUniformBlockMemberOwnerIndex(index)); Int count = 0; for (Uint uniformIndex = 0; uniformIndex < Artifacts().activeUniformCount; ++uniformIndex) { - if (GetActiveUniformBlockIndex(uniformIndex) == ownerIndex) ++count; + if (GetActiveUniformOwnerBlockIndex(uniformIndex) == ownerIndex) ++count; } return count; } @@ -1131,6 +1183,28 @@ namespace MobileGL::MG_State::GLState { Vector tProgramUniformIndexToGl; Vector glBlockIndexToTProgram; Vector tProgramBlockIndexToGl; + // GL_UNIFORM_BLOCK index space: ACTUAL uniform blocks only, a strict subsequence of + // glBlockIndexToTProgram above. + // + // That list is the BLOCK space - everything the relaxed parse produced except + // MGL_GLOBAL_UBO - and it is what the backends walk and what every block-keyed table + // here (uniformBlockBinding, uniformBlockIndexByName, blockReflection ordering) is + // indexed by. It is NOT the GL uniform-block list: MobileGL does not pass + // EShReflectionSeparateBuffers to buildReflection, so glslang routes BUFFER blocks + // through indexToUniformBlock too, and the list therefore also carries every shader + // storage block and every synthesized gl_AtomicCounterBlock_N. GL 4.6 core 7.6 gives + // those their own enumerations (GL_SHADER_STORAGE_BLOCK and + // GL_ACTIVE_ATOMIC_COUNTER_BUFFERS respectively), and GL_ACTIVE_UNIFORM_BLOCKS / + // glGetActiveUniformBlock*/glGetUniformBlockIndex must not see either. + // + // Kept as a SECOND space rather than filtering the first in place: DirectGLES assigns + // one ESSL uniform-buffer binding point per entry of the block list as it walks it + // (Managers.cpp CacheResourceLocations and the matching per-draw loop in + // DirectGLES.cpp), so compacting that list would renumber every backend binding + // point, and tProgramBlockIndexToGl[i] < 0 is what DoReflection and + // BuildGlobalUboRouting read as "member of the synthesized global UBO". + Vector glUniformBlockIndexToBlock; // GL uniform-block index -> block index + Vector blockIndexToGlUniformBlock; // block index -> GL uniform-block index (-1) // Per-link merged snapshot of the attached shaders' lexically extracted // layout(location = N) default-block uniform qualifiers (the relaxed parse drops // them from reflection; the DoReflection assigner restores them from here). diff --git a/MobileGL/MG_Test/Program/ProgramTest.cpp b/MobileGL/MG_Test/Program/ProgramTest.cpp index da9c2e1b..df93cf2b 100644 --- a/MobileGL/MG_Test/Program/ProgramTest.cpp +++ b/MobileGL/MG_Test/Program/ProgramTest.cpp @@ -2413,6 +2413,172 @@ void main() { EXPECT_EQ(GetError(), GL_NO_ERROR); } +namespace { + // One program carrying all four block/uniform kinds at once: a real uniform block, a + // shader storage block, an atomic counter (which the transpiler lowers onto a synthesized + // gl_AtomicCounterBlock_N buffer block) and plain default-block uniforms. + // + // MobileGL does not pass EShReflectionSeparateBuffers to glslang's buildReflection, so + // glslang routes BUFFER blocks through indexToUniformBlock alongside the uniform blocks - + // which is why every one of these has to be classified explicitly rather than taken at + // face value from the reflection list. + // The storage block and the counter are declared FIRST on purpose: that pushes both + // uniform blocks off the front of the block list, so the GL uniform-block index and the + // internal block index of every one of them differ. A translation that quietly reused one + // space for the other would answer with the storage block's name, size and binding here. + const char* kMixedBlockKindsFs = R"(#version 430 +layout(std430, binding = 0) buffer AVeryLongStorageBlockName { + vec4 storageVec; +}; +layout(binding = 1, offset = 0) uniform atomic_uint counter; +layout(std140) uniform Blk { + vec4 uboVec; +}; +layout(std140) uniform Blk2 { + vec4 uboVec2[3]; +}; +uniform float uScale; +out vec4 o_color; +void main() { + o_color = uboVec * uScale + uboVec2[1] + storageVec + vec4(float(atomicCounterIncrement(counter))); +})"; + + const char* kMixedBlockKindsVs = R"(#version 430 +void main() { gl_Position = vec4(0.0); })"; +} // namespace + +// GL 4.6 core 7.6: GL_ACTIVE_UNIFORM_BLOCKS and the glGetActiveUniformBlock* / +// glGetUniformBlockIndex family enumerate ACTUAL uniform blocks. An atomic counter buffer is +// enumerated by GL_ACTIVE_ATOMIC_COUNTER_BUFFERS and a shader storage block by the +// GL_SHADER_STORAGE_BLOCK program interface; neither may appear in the uniform-block list. +TEST_F(ProgramTest, UniformBlockListExcludesStorageAndAtomicCounterBlocks) { + GLuint program = LinkVsFsProgram(kMixedBlockKindsVs, kMixedBlockKindsFs); + + GLint activeBlocks = -1; + GetProgramiv(program, GL_ACTIVE_UNIFORM_BLOCKS, &activeBlocks); + ASSERT_EQ(activeBlocks, 2) << "only 'Blk' and 'Blk2' are GL uniform blocks"; + + // GL_ACTIVE_UNIFORM_BLOCK_MAX_NAME_LENGTH is measured over that same list, so the far + // longer storage-block name must not raise it. + GLint maxBlockNameLength = -1; + GetProgramiv(program, GL_ACTIVE_UNIFORM_BLOCK_MAX_NAME_LENGTH, &maxBlockNameLength); + EXPECT_EQ(maxBlockNameLength, static_cast(std::strlen("Blk2") + 1)); + + const GLuint blk = GetUniformBlockIndex(program, "Blk"); + const GLuint blk2 = GetUniformBlockIndex(program, "Blk2"); + ASSERT_NE(blk, GL_INVALID_INDEX); + ASSERT_NE(blk2, GL_INVALID_INDEX); + EXPECT_LT(blk, 2u); + EXPECT_LT(blk2, 2u); + EXPECT_NE(blk, blk2); + EXPECT_EQ(GetUniformBlockIndex(program, "AVeryLongStorageBlockName"), GL_INVALID_INDEX); + EXPECT_EQ(GetUniformBlockIndex(program, "gl_AtomicCounterBlock_1"), GL_INVALID_INDEX); + EXPECT_EQ(GetError(), GL_NO_ERROR); + + // Every index in the list names one of the two, and each index answers with ITS OWN + // block's properties - the storage block sits ahead of both in the internal block space, + // so a query answered in the wrong space reports "AVeryLongStorageBlockName" here. + char nameBuf[128] = ""; + GLsizei nameLen = 0; + GetActiveUniformBlockName(program, blk, sizeof(nameBuf), &nameLen, nameBuf); + EXPECT_STREQ(nameBuf, "Blk"); + GetActiveUniformBlockName(program, blk2, sizeof(nameBuf), &nameLen, nameBuf); + EXPECT_STREQ(nameBuf, "Blk2"); + + GLint dataSize = -1; + GetActiveUniformBlockiv(program, blk, GL_UNIFORM_BLOCK_DATA_SIZE, &dataSize); + EXPECT_EQ(dataSize, 16) << "Blk is one vec4"; + GetActiveUniformBlockiv(program, blk2, GL_UNIFORM_BLOCK_DATA_SIZE, &dataSize); + EXPECT_EQ(dataSize, 48) << "Blk2 is a vec4[3]"; + + GLint nameLengthProp = -1; + GetActiveUniformBlockiv(program, blk2, GL_UNIFORM_BLOCK_NAME_LENGTH, &nameLengthProp); + EXPECT_EQ(nameLengthProp, static_cast(std::strlen("Blk2") + 1)); + + // glUniformBlockBinding lands on the block the GL index names, and reads back through the + // same index. + UniformBlockBinding(program, blk2, 7); + GLint binding = -1; + GetActiveUniformBlockiv(program, blk2, GL_UNIFORM_BLOCK_BINDING, &binding); + EXPECT_EQ(binding, 7); + GetActiveUniformBlockiv(program, blk, GL_UNIFORM_BLOCK_BINDING, &binding); + EXPECT_NE(binding, 7) << "the rebind must not have leaked onto the neighbouring block"; + EXPECT_EQ(GetError(), GL_NO_ERROR); + + // An index past the end of the (now shorter) list is GL_INVALID_VALUE, not a silently + // answered query about a storage block. + GLint sink = -12345; + GetActiveUniformBlockiv(program, static_cast(activeBlocks), GL_UNIFORM_BLOCK_BINDING, &sink); + EXPECT_EQ(GetError(), GL_INVALID_VALUE); + EXPECT_EQ(sink, -12345); + UniformBlockBinding(program, static_cast(activeBlocks), 1); + EXPECT_EQ(GetError(), GL_INVALID_VALUE); + GetActiveUniformBlockName(program, static_cast(activeBlocks), sizeof(nameBuf), &nameLen, nameBuf); + EXPECT_EQ(GetError(), GL_INVALID_VALUE); + + // Each block's own member resolves against the block index this list hands out. + const GLuint uboVec = UniformIndexByName(program, "uboVec"); + const GLuint uboVec2 = UniformIndexByName(program, "uboVec2[0]"); + ASSERT_NE(uboVec, GL_INVALID_INDEX); + ASSERT_NE(uboVec2, GL_INVALID_INDEX); + EXPECT_EQ(QueryUniformiv(program, uboVec, GL_UNIFORM_BLOCK_INDEX), static_cast(blk)); + EXPECT_EQ(QueryUniformiv(program, uboVec2, GL_UNIFORM_BLOCK_INDEX), static_cast(blk2)); + + GLint blockMemberCount = -1; + GetActiveUniformBlockiv(program, blk, GL_UNIFORM_BLOCK_ACTIVE_UNIFORMS, &blockMemberCount); + EXPECT_EQ(blockMemberCount, 1); + GLint blockMemberIndex = -1; + GetActiveUniformBlockiv(program, blk, GL_UNIFORM_BLOCK_ACTIVE_UNIFORM_INDICES, &blockMemberIndex); + EXPECT_EQ(static_cast(blockMemberIndex), uboVec); + GetActiveUniformBlockiv(program, blk2, GL_UNIFORM_BLOCK_ACTIVE_UNIFORM_INDICES, &blockMemberIndex); + EXPECT_EQ(static_cast(blockMemberIndex), uboVec2); + EXPECT_EQ(GetError(), GL_NO_ERROR); +} + +// The GL_UNIFORM_BLOCK program interface hands out indices that are usable with +// glUniformBlockBinding / glGetActiveUniformBlockiv (ARB_program_interface_query), so it has +// to enumerate exactly the same list - not the internal block space that also carries the +// storage and atomic counter blocks. +TEST_F(ProgramTest, UniformBlockProgramInterfaceMatchesTheUniformBlockList) { + GLuint program = LinkVsFsProgram(kMixedBlockKindsVs, kMixedBlockKindsFs); + + GLint interfaceBlocks = -1; + GetProgramInterfaceiv(program, GL_UNIFORM_BLOCK, GL_ACTIVE_RESOURCES, &interfaceBlocks); + GLint activeBlocks = -1; + GetProgramiv(program, GL_ACTIVE_UNIFORM_BLOCKS, &activeBlocks); + EXPECT_EQ(interfaceBlocks, activeBlocks); + ASSERT_EQ(interfaceBlocks, 2); + + // The storage block is enumerated by its OWN interface instead. + GLint storageBlocks = -1; + GetProgramInterfaceiv(program, GL_SHADER_STORAGE_BLOCK, GL_ACTIVE_RESOURCES, &storageBlocks); + EXPECT_EQ(storageBlocks, 1); + EXPECT_EQ(GetProgramResourceIndex(program, GL_UNIFORM_BLOCK, "AVeryLongStorageBlockName"), GL_INVALID_INDEX); + EXPECT_NE(GetProgramResourceIndex(program, GL_SHADER_STORAGE_BLOCK, "AVeryLongStorageBlockName"), + GL_INVALID_INDEX); + + for (const char* blockName : {"Blk", "Blk2"}) { + const GLuint interfaceIndex = GetProgramResourceIndex(program, GL_UNIFORM_BLOCK, blockName); + ASSERT_NE(interfaceIndex, GL_INVALID_INDEX) << blockName; + EXPECT_EQ(interfaceIndex, GetUniformBlockIndex(program, blockName)) << blockName; + + // GL_NUM_ACTIVE_VARIABLES / GL_ACTIVE_VARIABLES must reach the same member the + // glGetActiveUniformBlockiv spelling does. + const GLenum numActive = GL_NUM_ACTIVE_VARIABLES; + GLint memberCount = -1; + GetProgramResourceiv(program, GL_UNIFORM_BLOCK, interfaceIndex, 1, &numActive, 1, nullptr, &memberCount); + ASSERT_EQ(memberCount, 1) << blockName; + const GLenum activeVariables = GL_ACTIVE_VARIABLES; + GLint memberIndex = -1; + GetProgramResourceiv(program, GL_UNIFORM_BLOCK, interfaceIndex, 1, &activeVariables, 1, nullptr, + &memberIndex); + GLint viaBlockiv = -1; + GetActiveUniformBlockiv(program, interfaceIndex, GL_UNIFORM_BLOCK_ACTIVE_UNIFORM_INDICES, &viaBlockiv); + EXPECT_EQ(memberIndex, viaBlockiv) << blockName; + } + 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 From 6e2a3b349644f050d089dcdd60a917f934a76b46 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Fri, 21 Aug 2026 11:55:29 -0400 Subject: [PATCH 2/2] [Fix, Test] (GLState): stop enumerating buffer variables as GL uniforms --- .../GLImpl/Program/ProgramInterface.cpp | 27 +++++--- .../GLState/ProgramState/ProgramLinkTask.cpp | 20 ++++++ MobileGL/MG_Test/Program/ProgramTest.cpp | 63 +++++++++++++++++++ 3 files changed, 102 insertions(+), 8 deletions(-) diff --git a/MobileGL/MG_Impl/GLImpl/Program/ProgramInterface.cpp b/MobileGL/MG_Impl/GLImpl/Program/ProgramInterface.cpp index a6834dd6..3ecf1e5f 100644 --- a/MobileGL/MG_Impl/GLImpl/Program/ProgramInterface.cpp +++ b/MobileGL/MG_Impl/GLImpl/Program/ProgramInterface.cpp @@ -328,15 +328,25 @@ namespace MobileGL::MG_Impl::GLImpl::ProgramInterface { const ProgramObject::LinkArtifacts& reflection, Model& model, const Vector& blockKind, const Vector& blockInterfaceIndex) { - const Uint uniformCount = program.GetUniformCount(); - for (Uint glIndex = 0; glIndex < uniformCount; ++glIndex) { - const Int tIndex = program.TProgramUniformIndex(glIndex); + // Walks the TPROGRAM uniform space, not the GL one. A buffer variable is not a GL + // uniform (GL 4.6 core 7.3.1) and DoReflection therefore keeps it out of the GL + // active-uniform index space - but GL_BUFFER_VARIABLE still has to enumerate it, and + // this is the only place that does. GL uniforms keep their GL index as their + // GL_UNIFORM resource index: the GL space is a subsequence of this one, so pushing + // the GL-visible entries in this order preserves the correspondence. + const Int tUniformCount = static_cast(reflection.uniformReflection.size()); + for (Int tIndex = 0; tIndex < tUniformCount; ++tIndex) { const auto& refl = ProgramObject::UniformAtIn(reflection, tIndex); const auto& type = refl.type; const Int owner = refl.index; const BlockKind kind = (owner >= 0 && owner < static_cast(blockKind.size())) ? blockKind[owner] : BlockKind::GlobalUbo; + const Int glIndex = program.GlUniformIndexFromTProgram(tIndex); + // Everything except a buffer variable is enumerated through the GL space, so a + // uniform the relaxed parse swept out of it (a declared-but-dead default-block + // one) stays out of GL_UNIFORM too. + if (kind != BlockKind::Storage && glIndex < 0) continue; Resource resource; resource.name = refl.name; @@ -369,11 +379,12 @@ namespace MobileGL::MG_Impl::GLImpl::ProgramInterface { resource.atomicCounterBufferIndex = blockInterfaceIndex[owner]; resource.location = -1; } else { - resource.blockIndex = program.GetActiveUniformBlockIndex(glIndex); - resource.offset = program.GetActiveUniformOffset(glIndex); - resource.arrayStride = program.GetActiveUniformArrayStride(glIndex); - resource.matrixStride = program.GetActiveUniformMatrixStride(glIndex); - resource.isRowMajor = program.GetActiveUniformIsRowMajor(glIndex); + const Uint glUniformIndex = static_cast(glIndex); + resource.blockIndex = program.GetActiveUniformBlockIndex(glUniformIndex); + resource.offset = program.GetActiveUniformOffset(glUniformIndex); + resource.arrayStride = program.GetActiveUniformArrayStride(glUniformIndex); + resource.matrixStride = program.GetActiveUniformMatrixStride(glUniformIndex); + resource.isRowMajor = program.GetActiveUniformIsRowMajor(glUniformIndex); // A member of a named uniform block has no location, whatever the // frontend's own location table says (it hands one out to every uniform // so glUniform* can address block members through the global UBO). diff --git a/MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.cpp b/MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.cpp index 7189ed0e..23d8a66f 100644 --- a/MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.cpp +++ b/MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.cpp @@ -1081,6 +1081,20 @@ namespace MobileGL::MG_State::GLState { const auto isNamedBlockMember = [&isGlobalUboMember](const glslang::TObjectReflection& uniform) { return uniform.index >= 0 && !isGlobalUboMember(uniform); }; + // A member of a BUFFER block is a buffer variable, not a uniform: GL 4.6 core 7.3.1 + // gives it the GL_BUFFER_VARIABLE interface and 7.6 keeps it out of GL_ACTIVE_UNIFORMS, + // glGetActiveUniform, glGetUniformIndices and glGetActiveUniformsiv. The relaxed parse + // reflects it as a uniform anyway (no EShReflectionSeparateBuffers), so drop it from the + // GL index space here - the same place the dead default-block uniforms are dropped, and + // the counterpart of the location half already handled by isNamedBlockMember below. + // + // Atomic counters are NOT in this set even though their synthesized owner is a buffer + // block: an atomic_uint IS a uniform (of type GL_UNSIGNED_INT_ATOMIC_COUNTER), and + // KHR-GL43.shader_atomic_counters.basic-program-query enumerates it as one. + const auto isBufferVariable = [this](const glslang::TObjectReflection& uniform) { + if (uniform.index < 0 || uniform.index >= artifacts.program->getNumUniformBlocks()) return false; + return IsStorageBlock(artifacts.program->getUniformBlock(uniform.index)); + }; for (Int i = 0; i < tProgramUniformCount; i++) { const auto& uniform = artifacts.program->getUniform(i); if (isGlobalUboMember(uniform) && uniform.stages == 0) { @@ -1089,6 +1103,12 @@ namespace MobileGL::MG_State::GLState { in.externalIndex, uniform.name.c_str()); continue; } + if (isBufferVariable(uniform)) { + MGLOG_D("ProgramObject %u: Reflection - buffer variable '%s' filtered from the GL uniform " + "surface", + in.externalIndex, uniform.name.c_str()); + continue; + } artifacts.tProgramUniformIndexToGl[i] = static_cast(artifacts.glUniformIndexToTProgram.size()); artifacts.glUniformIndexToTProgram.push_back(i); } diff --git a/MobileGL/MG_Test/Program/ProgramTest.cpp b/MobileGL/MG_Test/Program/ProgramTest.cpp index df93cf2b..0827ab6e 100644 --- a/MobileGL/MG_Test/Program/ProgramTest.cpp +++ b/MobileGL/MG_Test/Program/ProgramTest.cpp @@ -2556,6 +2556,9 @@ TEST_F(ProgramTest, UniformBlockProgramInterfaceMatchesTheUniformBlockList) { EXPECT_EQ(GetProgramResourceIndex(program, GL_UNIFORM_BLOCK, "AVeryLongStorageBlockName"), GL_INVALID_INDEX); EXPECT_NE(GetProgramResourceIndex(program, GL_SHADER_STORAGE_BLOCK, "AVeryLongStorageBlockName"), GL_INVALID_INDEX); + // ... and the buffer variable by GL_BUFFER_VARIABLE, not GL_UNIFORM. + EXPECT_NE(GetProgramResourceIndex(program, GL_BUFFER_VARIABLE, "storageVec"), GL_INVALID_INDEX); + EXPECT_EQ(GetProgramResourceIndex(program, GL_UNIFORM, "storageVec"), GL_INVALID_INDEX); for (const char* blockName : {"Blk", "Blk2"}) { const GLuint interfaceIndex = GetProgramResourceIndex(program, GL_UNIFORM_BLOCK, blockName); @@ -2579,6 +2582,66 @@ TEST_F(ProgramTest, UniformBlockProgramInterfaceMatchesTheUniformBlockList) { EXPECT_EQ(GetError(), GL_NO_ERROR); } +// GL 4.6 core 7.3.1 / 7.6: a buffer variable is not a uniform - it lives in the +// GL_BUFFER_VARIABLE interface - so it must not appear in GL_ACTIVE_UNIFORMS, +// glGetActiveUniform, glGetUniformIndices or glGetActiveUniformsiv. An ATOMIC COUNTER, by +// contrast, IS a uniform (of type GL_UNSIGNED_INT_ATOMIC_COUNTER) and must stay enumerated. +TEST_F(ProgramTest, ActiveUniformsExcludeBufferVariablesButKeepAtomicCounters) { + GLuint program = LinkVsFsProgram(kMixedBlockKindsVs, kMixedBlockKindsFs); + + GLint activeUniforms = -1; + GetProgramiv(program, GL_ACTIVE_UNIFORMS, &activeUniforms); + ASSERT_EQ(activeUniforms, 4) + << "uboVec, uboVec2[0], uScale and counter - storageVec is a buffer variable"; + + // Neither spelling of the buffer variable is a uniform index. + EXPECT_EQ(UniformIndexByName(program, "storageVec"), GL_INVALID_INDEX); + EXPECT_EQ(UniformIndexByName(program, "AVeryLongStorageBlockName.storageVec"), GL_INVALID_INDEX); + // The location half of the same rule (already landed) must stay consistent with it. + EXPECT_EQ(GetUniformLocation(program, "storageVec"), -1); + + char nameBuf[128] = ""; + for (GLint i = 0; i < activeUniforms; ++i) { + GLsizei nameLen = 0; + GLint size = 0; + GLenum type = 0; + GetActiveUniform(program, static_cast(i), sizeof(nameBuf), &nameLen, &size, &type, nameBuf); + EXPECT_EQ(std::string(nameBuf).find("storageVec"), std::string::npos) + << "buffer variable enumerated as active uniform " << i << ": " << nameBuf; + } + + // The counter is still a uniform, still reports the atomic-counter type, has no owning + // uniform block, and still points at its atomic counter BUFFER. + const GLuint counter = UniformIndexByName(program, "counter"); + ASSERT_NE(counter, GL_INVALID_INDEX); + EXPECT_EQ(QueryUniformiv(program, counter, GL_UNIFORM_TYPE), + static_cast(GL_UNSIGNED_INT_ATOMIC_COUNTER)); + EXPECT_EQ(QueryUniformiv(program, counter, GL_UNIFORM_BLOCK_INDEX), -1); + EXPECT_EQ(QueryUniformiv(program, counter, GL_UNIFORM_ATOMIC_COUNTER_BUFFER_INDEX), 0); + EXPECT_EQ(QueryUniformiv(program, counter, GL_UNIFORM_OFFSET), 0); + EXPECT_EQ(GetUniformLocation(program, "counter"), -1); + + // GL_ACTIVE_ATOMIC_COUNTER_BUFFERS indexes into the GL uniform index space, so the + // counter index it reports has to be the one glGetUniformIndices just handed out. + GLint counterBuffers = -1; + GetProgramiv(program, GL_ACTIVE_ATOMIC_COUNTER_BUFFERS, &counterBuffers); + ASSERT_EQ(counterBuffers, 1); + GLint counterCount = -1; + GetActiveAtomicCounterBufferiv(program, 0, GL_ATOMIC_COUNTER_BUFFER_ACTIVE_ATOMIC_COUNTERS, &counterCount); + ASSERT_EQ(counterCount, 1); + GLint counterIndex = -1; + GetActiveAtomicCounterBufferiv(program, 0, GL_ATOMIC_COUNTER_BUFFER_ACTIVE_ATOMIC_COUNTER_INDICES, + &counterIndex); + EXPECT_EQ(static_cast(counterIndex), counter); + GLint counterBinding = -1; + GetActiveAtomicCounterBufferiv(program, 0, GL_ATOMIC_COUNTER_BUFFER_BINDING, &counterBinding); + EXPECT_EQ(counterBinding, 1); + + // The default-block uniform is untouched by either filter. + EXPECT_NE(GetUniformLocation(program, "uScale"), -1); + 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