[Fix, Test] (GLImpl, GLState): give a storage block with no binding qualifier GL's default binding of zero

This commit is contained in:
2026-08-21 05:13:58 -04:00
parent 21a4c8aa95
commit d9def5c1bb
11 changed files with 312 additions and 0 deletions
@@ -828,6 +828,13 @@ namespace MobileGL::MG_State::GLState {
for (const auto& [name, binding] : compiled.explicitOpaqueBindings) {
artifacts.explicitOpaqueUniformBindings[name] = binding;
}
// Storage blocks declared with NO layout(binding = N), which GL puts on binding 0.
// A UNION across stages, unlike the maps above: a block that any stage declared
// unqualified is unqualified, because GLSL requires every stage declaring the same
// block to declare it identically - so the stages cannot disagree, and a stage whose
// grammar the scanner did not recognise simply contributes nothing.
artifacts.storageBlocksWithoutBinding.insert(compiled.storageBlocksWithoutBinding.begin(),
compiled.storageBlocksWithoutBinding.end());
}
}
@@ -1505,6 +1512,7 @@ namespace MobileGL::MG_State::GLState {
for (Int i = 0; i < blockCount; ++i) {
artifacts.blockReflection.push_back(MakeResourceReflection(program.getUniformBlock(i)));
}
SeedDefaultStorageBlockBindings();
const Int uniformCount = program.getNumUniformVariables();
artifacts.uniformReflection.clear();
@@ -1553,6 +1561,63 @@ namespace MobileGL::MG_State::GLState {
artifacts.pipeInputReflection.size(), artifacts.pipeOutputReflection.size());
}
// GL 4.3 core 7.8: a shader storage block declared without a layout(binding = N) qualifier
// has a buffer binding of ZERO. MobileGL could not report that, because by the time this
// reflection is built the number in the block's qualifier is one glslang INVENTED.
//
// Every shader is parsed as a Vulkan client, so glslang's IO mapper takes the `set = openGl
// ? resource : ent.newSet` branch with openGl == 0 (iomapper.cpp resolveBinding) - i.e. it
// allocates out of ONE flat binding space shared by every sampler, image, uniform block,
// storage block and the synthesized MGL_GLOBAL_UBO - and then writes the result back into
// the type's qualifier (iomapper.cpp, `base->getWritableType().getQualifier().layoutBinding =
// at->second.newBinding`). getBinding() therefore answers with the auto-assigned slot and
// cannot be distinguished from a declared one. An unqualified block lands on 0 only when
// nothing else in the program claimed 0 first, which is why a lone storage block in a
// trivial shader looked correct and KHR-GL43.compute_shader.resource-ubo - whose shader also
// declares twelve uniform blocks - wrote everything to a binding nothing was bound at.
//
// THE FLAT SPACE IS LEFT ALONE. It is load-bearing: DirectVulkan indexes bindingKinds[],
// uniformBlockIndexByBinding[] and storageBlockIndexByBinding[] by that one number and
// asserts when two resources collide on it, so forcing the SPIR-V decoration to 0 would
// collide an unqualified block with the global UBO and take working programs down. What is
// repaired is the GL-VISIBLE binding, through the record GL already has for exactly this -
// the same per-name map glShaderStorageBlockBinding writes, which both backends already
// consult (ProgramInterface's GL_BUFFER_BINDING, DirectGLES's SPIRV-Cross binding rewrite,
// DirectVulkan's GetShaderStorageBlockBinding). Seeding it here means the default and a
// later rebind travel the same path, and basic-noBindingLayout - which rebinds all three of
// its unqualified blocks - keeps working because a rebind simply overwrites the seed.
//
// Seeded INSIDE `artifacts`, so an L1 translation-cache hit that republishes the artifacts
// wholesale carries it too; a seed applied outside them would silently vanish on a hit.
//
// Only blocks the lexical scanner recognised in full AND recognised as unqualified are
// seeded. A block whose grammar it did not understand keeps today's behaviour rather than
// being defaulted on a guess - the shape of the input decides, never an assumption about it.
//
// THE COLLISION IS DELIBERATE, and it is GL's. Several unqualified blocks all default to 0
// and alias there until the application rebinds them; a real GL driver does the same, which
// is why every program that has more than one either rebinds or uses one of them.
// basic-noBindingLayout is that regression test - it rebinds all three of its blocks
// immediately after linking, and the DirectGLES transpile is lazy (first use, not link), so
// the ESSL it eventually emits already carries the rebound 0/1/2 and never the aliased seed.
// What this replaces was not a safer arrangement, only an accidental one: the three blocks
// got glslang's 0/1/2 and an application that rebound them to anything else still wrote to
// the wrong buffers.
void ProgramLinkTask::SeedDefaultStorageBlockBindings() {
if (artifacts.storageBlocksWithoutBinding.empty()) return;
for (const ProgramObject::BlockReflection& block : artifacts.blockReflection) {
if (!block.type.isBuffer) continue;
// An instance array reflects as "B[0]", "B[1]", ... and each element is its own GL
// resource with its own binding; the scanner keys on the block TYPE name, so the
// subscript is stripped before the lookup. GL gives element k of an unqualified
// array binding 0 + k, the same base + element rule a declared binding follows.
const String base = StripArrayElementSuffix(block.name);
if (!artifacts.storageBlocksWithoutBinding.contains(base)) continue;
// First writer wins: never overwrite a binding the application has already chosen.
artifacts.shaderStorageBlockBinding.emplace(block.name, BlockArrayElement(block.name));
}
}
Bool ProgramLinkTask::ValidateFragmentOutputLocations() {
if (!artifacts.program) return false;
// The pipe-output list is the output interface of the program's LAST stage. Only a
@@ -189,6 +189,11 @@ namespace MobileGL::MG_State::GLState {
// Copies every reflection record the GL query surface reads out of the glslang
// TProgram into LinkArtifacts own owned tables. Runs at the tail of DoReflection.
void SnapshotGlslangReflection();
// Gives every storage block whose shader declared no layout(binding = N) the binding
// GL 4.3 core 7.8 says it has - zero - because glslang's IO mapper has by then invented
// one and overwritten the qualifier. See the definition for why the invented binding is
// deliberately left in place for the backends' own use.
void SeedDefaultStorageBlockBindings();
Bool ValidateFragmentOutputLocations();
Bool ResolveTransformFeedbackVaryings();
void ResolveGsTriangleStripCapture(const glslang::TIntermediate* captureIntermediate);
@@ -344,6 +344,10 @@ namespace MobileGL::MG_State::GLState {
artifacts.uniformBlockIndexByName.clear();
artifacts.uniformBlockBinding.clear();
artifacts.shaderStorageBlockBinding.clear();
// Cleared with it: the seed above is re-derived from the newly attached shaders on every
// link, so a stale set would otherwise default a block the new sources do declare a
// binding for.
artifacts.storageBlocksWithoutBinding.clear();
artifacts.attribs.clear();
artifacts.attribTypes.clear();
artifacts.activeUniformCount = 0;
@@ -1125,7 +1125,20 @@ namespace MobileGL::MG_State::GLState {
Vector<Int> uniformBlockBinding;
// glShaderStorageBlockBinding overrides, keyed by GL block name. See
// SetShaderStorageBlockBinding for why this one is by name and not by index.
//
// ALSO SEEDED AT LINK, by ProgramLinkTask::SeedDefaultStorageBlockBindings, with the
// GL-mandated binding 0 for every storage block whose shader declared no
// layout(binding = N). Those blocks have no other way to be told apart from a block
// that declared one: glslang's IO mapper invents a binding and writes it into the
// qualifier, so the reflection reports the invention. A seed is therefore "GL's
// default binding for this block", and a later glShaderStorageBlockBinding simply
// overwrites it - default and rebind travel one path.
UnorderedMap<String, Int> shaderStorageBlockBinding;
// Block type names of the storage blocks the program's shaders declared with NO
// layout(binding = N), merged across stages by MergeShaderSideChannels. Input to the
// seeding above; see ExtractStorageBlocksWithoutExplicitBinding for why it has to be
// captured lexically.
std::set<String> storageBlocksWithoutBinding;
Uint activeUniformCount = 0;
Uint maxUniformLocation = 0;
@@ -211,6 +211,12 @@ namespace {
// ProgramObject::DoReflection.
result.explicitUniformLocations = ExtractExplicitUniformLocations(result.preprocessedSource);
result.explicitOpaqueBindings = ExtractExplicitOpaqueBindings(result.preprocessedSource);
// The third side-channel, and the one that restores a GL DEFAULT rather than an
// application-declared value: a storage block with no layout(binding = N) has buffer
// binding 0 in GL, and by the time the reflection is built glslang's IO mapper has
// already invented one and written it into the qualifier.
result.storageBlocksWithoutBinding =
ExtractStorageBlocksWithoutExplicitBinding(result.preprocessedSource);
result.outcome = ShaderPreprocessOutcome::Preprocessed;
return result;
}
@@ -355,6 +361,7 @@ namespace MobileGL::MG_State::GLState {
artifacts.preprocessedSource = shared.preprocessedSource;
artifacts.explicitUniformLocations = shared.explicitUniformLocations;
artifacts.explicitOpaqueBindings = shared.explicitOpaqueBindings;
artifacts.storageBlocksWithoutBinding = shared.storageBlocksWithoutBinding;
artifacts.infoLog.clear();
if (shouldPopulateCache) {
cache->Insert(stage, sourceHash, *source, compileEnv.fingerprint, Move(fresh));
@@ -381,6 +388,7 @@ namespace MobileGL::MG_State::GLState {
fresh->infoLog = artifacts.infoLog;
fresh->explicitUniformLocations.clear();
fresh->explicitOpaqueBindings.clear();
fresh->storageBlocksWithoutBinding.clear();
cache->Insert(stage, sourceHash, *source, compileEnv.fingerprint, Move(fresh));
}
}
@@ -62,6 +62,9 @@ namespace MobileGL::MG_State::GLState {
String preprocessedSource;
UnorderedMap<String, Int> explicitUniformLocations;
UnorderedMap<String, Uint> explicitOpaqueBindings;
// Block type names of this stage's shader storage blocks that declared no
// layout(binding = N), i.e. the ones GL puts on binding 0.
std::set<String> storageBlocksWithoutBinding;
String infoLog;
Bool compileStatus = false;
};
@@ -112,6 +112,13 @@ namespace MobileGL {
const UnorderedMap<String, Uint>& GetExplicitOpaqueBindings() const {
return Compiled().explicitOpaqueBindings;
}
// Block type names of this shader's storage blocks that declared NO
// layout(binding = N), captured lexically because glslang's IO mapper invents one
// and overwrites the qualifier before anything can ask (see
// ExtractStorageBlocksWithoutExplicitBinding).
const std::set<String>& GetStorageBlocksWithoutBinding() const {
return Compiled().storageBlocksWithoutBinding;
}
Bool GetCompileStatus() const { return Compiled().compileStatus; }
Bool GetDeleteStatus() const { return m_deleteStatus; }
@@ -46,6 +46,11 @@ namespace MobileGL::MG_State::GLState {
String preprocessedSource;
UnorderedMap<String, Int> explicitUniformLocations;
UnorderedMap<String, Uint> explicitOpaqueBindings;
// Block type names of the shader storage blocks declared here with NO
// layout(binding = N). GL gives such a block binding 0; nothing downstream can still
// tell, because glslang's IO mapper auto-assigns one and writes it into the qualifier.
// See ExtractStorageBlocksWithoutExplicitBinding.
std::set<String> storageBlocksWithoutBinding;
// The compile info log to publish; empty when outcome == Preprocessed.
String infoLog;
@@ -4538,3 +4538,102 @@ void main() {}
ASSERT_EQ(bindings.count("suffixedUnit"), 1u);
EXPECT_EQ(bindings.at("suffixedUnit"), 1u);
}
// GL 4.3 core 7.8 puts a storage block with no layout(binding = N) on binding ZERO. Nothing
// downstream can still tell which blocks those are, because glslang's IO mapper auto-assigns a
// binding out of one flat space and writes it into the qualifier - so the reflection reports the
// invention. This scanner is the only surviving record, and it reports POSITIVELY: a block is
// named only when it was recognised in full AND recognised as unqualified.
TEST_F(ProgramUtilTest, ExtractStorageBlocksWithoutExplicitBindingNamesOnlyUnqualifiedBlocks) {
using namespace MG_Util::ShaderTranspiler;
// KHR-GL43.compute_shader.resource-ubo's own shape: an unqualified storage block alongside
// the uniform blocks whose presence is what pushes it off binding 0 today.
const String source = R"(#version 430 core
layout(local_size_x = 1) in;
layout(std140) uniform InputBuffer { vec4 data[4]; } g_in_buffer[12];
layout(std430) buffer OutputBuffer { vec4 data0[4]; } g_out_buffer;
layout(std430, binding = 3) buffer BoundBlock { vec4 data1[4]; } g_bound;
layout(binding = 5, std430) buffer BoundFirst { vec4 data2[4]; } g_bound_first;
void main() { g_out_buffer.data0[0] = g_in_buffer[0].data[0]; }
)";
const std::set<String> unqualified = ExtractStorageBlocksWithoutExplicitBinding(source);
EXPECT_EQ(unqualified.count("OutputBuffer"), 1u)
<< "the block the test binds at 0 with glBindBufferBase must be recognised";
EXPECT_EQ(unqualified.count("BoundBlock"), 0u)
<< "a declared binding must never be defaulted away";
EXPECT_EQ(unqualified.count("BoundFirst"), 0u)
<< "the binding may appear anywhere in the layout list, not only last";
// A UNIFORM block is a different binding space with its own glUniformBlockBinding path, and
// its default is already handled where uniformBlockBinding is seeded. Naming it here would
// make the seeder default a resource it does not own.
EXPECT_EQ(unqualified.count("InputBuffer"), 0u) << "uniform blocks are out of scope";
}
// The scanner must not mistake a member qualifier, a buffer-typed sampler, or the
// "layout(...) buffer;" default-qualifier form for a block declaration - and must record nothing
// at all for grammar it does not fully recognise, so that anything surprising keeps today's
// behaviour instead of being defaulted on a guess.
TEST_F(ProgramUtilTest, ExtractStorageBlocksWithoutExplicitBindingIgnoresNonBlockBufferTokens) {
using namespace MG_Util::ShaderTranspiler;
const String source = R"(#version 430 core
layout(local_size_x = 1) in;
uniform samplerBuffer texelSampler;
layout(std430) buffer;
layout(std430) buffer Real { vec4 v[4]; } realInstance;
void main() { realInstance.v[0] = texelFetch(texelSampler, 0); }
)";
const std::set<String> unqualified = ExtractStorageBlocksWithoutExplicitBinding(source);
EXPECT_EQ(unqualified.count("Real"), 1u);
// samplerBuffer is one identifier token, so it can never match the `buffer` keyword; and the
// default-qualifier form declares no block, so there is no name to record.
EXPECT_EQ(unqualified.size(), 1u)
<< "only the one real block declaration may be recorded";
}
// The dangerous direction, because a false positive here DEFAULTS AWAY a binding the shader
// really declared. Memory qualifiers may sit between the layout list and the `buffer` keyword in
// either order, and the binding has to survive them.
TEST_F(ProgramUtilTest, ExtractStorageBlocksWithoutExplicitBindingKeepsBindingsAcrossMemoryQualifiers) {
using namespace MG_Util::ShaderTranspiler;
const String source = R"(#version 430 core
layout(local_size_x = 1) in;
layout(std430, binding = 1) coherent restrict buffer AfterLayout { uint a; } afterLayout;
readonly layout(std430, binding = 2) buffer BeforeLayout { uint b; } beforeLayout;
writeonly buffer NoBindingAtAll { uint c; } noBinding;
void main() { noBinding.c = afterLayout.a + beforeLayout.b; }
)";
const std::set<String> unqualified = ExtractStorageBlocksWithoutExplicitBinding(source);
EXPECT_EQ(unqualified.count("AfterLayout"), 0u)
<< "coherent/restrict must not break the qualifier run and lose the binding";
EXPECT_EQ(unqualified.count("BeforeLayout"), 0u)
<< "a qualifier may precede the layout list too";
EXPECT_EQ(unqualified.count("NoBindingAtAll"), 1u)
<< "a memory-qualified block with no binding is still an unqualified block";
}
// This scans preprocessor-visible text, so a block can be declared twice - once with a binding
// and once without. Reporting it as unqualified would default away a binding the active
// declaration carries, so any name seen WITH a binding is dropped outright.
TEST_F(ProgramUtilTest, ExtractStorageBlocksWithoutExplicitBindingDropsNamesSeenBothWays) {
using namespace MG_Util::ShaderTranspiler;
const String source = R"(#version 430 core
layout(local_size_x = 1) in;
layout(std430, binding = 4) buffer Ambiguous { uint a; } bound;
layout(std430) buffer Ambiguous { uint a; } unbound;
layout(std430) buffer Clear { uint b; } clearInstance;
void main() { clearInstance.b = 0u; }
)";
const std::set<String> unqualified = ExtractStorageBlocksWithoutExplicitBinding(source);
EXPECT_EQ(unqualified.count("Ambiguous"), 0u)
<< "seen both ways is a doubt, and a doubt must not become a default";
EXPECT_EQ(unqualified.count("Clear"), 1u)
<< "the unambiguous block alongside it is still recognised";
}
@@ -1548,6 +1548,89 @@ namespace MobileGL {
}
} // namespace
std::set<String> ExtractStorageBlocksWithoutExplicitBinding(const String& source) {
std::set<String> names;
// Fast path: no storage block, nothing to record. `buffer` as a whole token is
// what declares one; samplerBuffer/imageBuffer/textureBuffer tokenize as single
// identifiers and so cannot match below, but this substring test is only a
// cheap pre-filter and is allowed to be generous.
if (source.find("buffer") == String::npos) return names;
const Vector<CodeToken> tokens = TokenizeCode(source);
const SizeT count = tokens.size();
Int braceDepth = 0;
// Block names seen WITH a binding. Subtracted at the end so a name that is
// declared unqualified in one place and qualified in another is never reported:
// this scans preprocessor-visible text, so both arms of a #if can be present,
// and defaulting a block the active arm binds explicitly would be a regression.
// A name is dropped whenever there is any doubt, never kept.
std::set<String> qualified;
// The binding the qualifier run currently being scanned declared, -1 for none.
// Several layout(...) lists may precede one declaration and the later one wins -
// the same accumulate-then-consume shape FindShaderStorageBindingViolation uses.
long long binding = -1;
long long literal = 0;
for (SizeT pos = 0; pos < count; ++pos) {
const String& text = tokens[pos].text;
if (text == "{") {
++braceDepth;
binding = -1;
continue;
}
if (text == "}") {
if (braceDepth > 0) --braceDepth;
continue;
}
// Only depth-0 declarations are block declarations; `buffer` inside a block
// body or a function is a member qualifier or an identifier.
if (braceDepth != 0) continue;
if (text == "layout" && pos + 1 < count && tokens[pos + 1].text == "(") {
SizeT j = pos + 2;
Int parenDepth = 1;
while (j < count && parenDepth > 0) {
const String& layoutToken = tokens[j].text;
if (layoutToken == "(") {
++parenDepth;
} else if (layoutToken == ")") {
--parenDepth;
} else if (parenDepth == 1 && layoutToken == "binding" && j + 2 < count &&
tokens[j + 1].text == "=" &&
ParseGlslIntegerLiteral(tokens[j + 2].text, literal)) {
binding = std::min(literal, static_cast<long long>(INT_MAX / 2));
j += 2;
}
++j;
}
pos = j - 1;
continue;
}
if (text == "buffer") {
// Recorded ONLY for the fully recognised shape: a block type name
// followed by the body's '{'. The "layout(...) buffer;"
// default-qualifier form declares no block and has no name to key on,
// and anything else here is grammar this scanner does not judge - both
// fall through and keep today's behaviour.
if (pos + 2 < count && IsIdentifierToken(tokens[pos + 1]) &&
tokens[pos + 2].text == "{") {
(binding < 0 ? names : qualified).insert(tokens[pos + 1].text);
}
binding = -1;
continue;
}
// Qualifiers may sit between the layout list and the `buffer` keyword;
// anything else ends the run, so a binding never leaks onto an unrelated
// declaration - and, just as importantly, the ABSENCE of one never does.
if (!IsNonLayoutQualifierKeyword(text)) binding = -1;
}
for (const String& name : qualified) {
names.erase(name);
}
return names;
}
std::optional<String> FindShaderStorageBindingViolation(const String& source, Int maxBindings) {
// A backend that advertises nothing has no ceiling to enforce.
if (maxBindings <= 0) return std::nullopt;
@@ -7,6 +7,8 @@
// End of Source File Header
#pragma once
#include <set>
#include <Includes.h>
#include <MG_State/GLState/ProgramState/ShaderObject.h>
#include <MG_Util/ShaderTranspiler/CompileEnv.h>
@@ -65,6 +67,24 @@ namespace MobileGL {
// grammar discipline as ExtractExplicitUniformLocations).
UnorderedMap<String, Uint> ExtractExplicitOpaqueBindings(const String& source);
// The BLOCK TYPE NAMES of the shader storage blocks this source declares WITHOUT a
// layout(binding = N) qualifier. GL 4.3 core 7.8 gives such a block a buffer binding
// of ZERO, which the application may then move with glShaderStorageBlockBinding.
//
// Exists because nothing downstream can still tell. Every shader is parsed as a
// Vulkan client, so glslang's IO mapper allocates a binding for the block out of one
// flat space shared with every sampler, image and uniform block in the program
// (iomapper.cpp resolveBinding: `set = openGl ? resource : ent.newSet`, and openGl is
// 0 here) - and then WRITES IT BACK into the type's qualifier, so the reflection
// reports an auto-assigned number as if the shader had declared it. An unqualified
// block therefore lands on 0 only when nothing else claimed 0 first.
//
// Reported POSITIVELY - only blocks the scanner recognised in full, and recognised as
// carrying no binding - so anything outside its narrow grammar is left to the
// existing behaviour rather than defaulted on a guess. Same discipline as
// ExtractExplicitOpaqueBindings.
std::set<String> ExtractStorageBlocksWithoutExplicitBinding(const String& source);
// A shader storage block whose layout(binding = N) reaches or passes
// GL_MAX_SHADER_STORAGE_BUFFER_BINDINGS is a compile-time error in GL 4.3 core 4.4.5,
// and an arrayed block instance takes CONSECUTIVE points, so the last element is what