diff --git a/MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.cpp b/MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.cpp index 23d8a66f..80d35d77 100644 --- a/MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.cpp +++ b/MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.cpp @@ -579,8 +579,9 @@ namespace MobileGL::MG_State::GLState { if (!ValidateAttachedShaders()) return; - // The two merges below read the COMPILE snapshots only - no parsed shader - so they - // run before the L1 probe, which needs the merged opaque bindings in its key. + // Reads the COMPILE snapshots only - no parsed shader - so it runs before the L1 + // probe: a conflicting explicit uniform location must fail the link whether or not + // the memo has an answer for this program's sources. MergeShaderSideChannels(); if (!artifacts.infoLog.empty()) return; // a conflicting explicit uniform location @@ -617,11 +618,16 @@ namespace MobileGL::MG_State::GLState { } } + // The last two are OUT parameters that mapIO fills, not requests it honours: the IO + // mapper's collect callback is the last point at which a resource's qualifier still + // says what the SHADER declared rather than what glslang assigned, so both captures + // have to be taken from inside the link. See TMglGlslIoResolver::reserverResourceSlot. ProgramAttrib attrib{.shaders = Move(shaders), .explicitVertexInLocations = in.explicitAttribLocations, .explicitFragmentOutLocations = in.explicitFragDataLocation, .explicitFragmentOutIndices = in.explicitFragDataIndex, - .explicitOpaqueUniformBindings = &artifacts.explicitOpaqueUniformBindings}; + .explicitOpaqueUniformBindings = &artifacts.explicitOpaqueUniformBindings, + .storageBlocksWithoutBinding = &artifacts.storageBlocksWithoutBinding}; MGLOG_D("ProgramObject %u: Calling ShaderCompiler::LinkProgram", in.externalIndex); auto result = ShaderCompiler::LinkProgram(attrib); @@ -639,6 +645,7 @@ namespace MobileGL::MG_State::GLState { return; } + // A compute program must have a fixed local group size, and GL states that as a // property of the PROGRAM: "at least one" of its compute shaders declares it (GL 4.6 // core 7.13 / GLSL 4.30 4.4.1.4). MobileGL used to answer that question per SHADER, @@ -814,7 +821,6 @@ namespace MobileGL::MG_State::GLState { keyInputs.explicitVertexInLocations = &in.explicitAttribLocations; keyInputs.explicitFragmentOutLocations = &in.explicitFragDataLocation; keyInputs.explicitFragmentOutIndices = &in.explicitFragDataIndex; - keyInputs.explicitOpaqueUniformBindings = &artifacts.explicitOpaqueUniformBindings; // In the key ONLY because the payload now carries the reflection: transform feedback // is resolved by reading the linked intermediates and never perturbs the generated // SPIR-V, but it does shape xfbVaryings / xfbStrides / xfbBufferMode / @@ -826,17 +832,18 @@ namespace MobileGL::MG_State::GLState { return BuildSpirvTranslationKey(keyInputs); } - // The link rejections that need nothing but the compile snapshots. They run before the + // The one link rejection that needs nothing but the compile snapshots. It runs before the // L1 memo is consulted, so a hit can never paper over a program that must fail to link. - // The two lexical side channels the relaxed parse cannot provide, merged across stages: - // explicit default-block uniform locations (which must agree, or the link fails) and - // sampler/image layout(binding = N) initial units. Reads the COMPILE snapshots only, so - // it is legal - and necessary - before any shader is parsed: the merged bindings are part - // of the L1 memo key. + // + // Only the explicit default-block uniform locations are merged here, and only because they + // are the one piece of relaxed-parse wreckage that has to be recovered at COMPILE time: + // the snapshot is taken inside the parse, so it is per-shader by construction, and the + // same uniform declared in several stages must agree or the program cannot link. The + // opaque bindings and the unqualified storage blocks used to be merged alongside them; + // both now arrive from mapIO during LinkProgram below, straight into `artifacts`, which is + // both later and strictly better informed - the IO mapper sees macro-expanded declarations + // and a per-shader lexer never could. void ProgramLinkTask::MergeShaderSideChannels() { - // Merge the shaders' lexically extracted explicit uniform locations. The same - // uniform declared in several stages must agree on its location (config-A glslang - // enforced this at mapIO; the relaxed parse no longer sees the qualifiers). for (const auto& shader : in.shaders) { const ShaderCompileArtifacts& compiled = CompiledArtifacts(shader.compiled); for (const auto& [name, location] : compiled.explicitUniformLocations) { @@ -850,21 +857,7 @@ namespace MobileGL::MG_State::GLState { return; } } - // Sampler/image layout(binding = N) initial units, likewise invisible to the - // relaxed parse. Stage order matches the old per-stage mapIO capture, so a - // name declared in several stages keeps the last stage's binding as before. - 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()); } - } // An L1 hit: the entire front end, published without constructing a TShader or a @@ -1117,7 +1110,7 @@ namespace MobileGL::MG_State::GLState { artifacts.activeUniformCount, tProgramUniformCount); // Effective explicit location per TProgram uniform, from two sources: - // - the lexical side-channel for default-block uniforms - the relaxed parse + // - the parse-time snapshot for default-block uniforms - the relaxed parse // dropped their layout(location = N) qualifiers when collecting them into // MGL_GLOBAL_UBO, so reflection cannot provide them ("source-explicit"); // - glslang's layoutLocation() for opaque uniforms, where the qualifier @@ -1681,9 +1674,13 @@ namespace MobileGL::MG_State::GLState { // 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 blocks are named by TMglGlslIoResolver at mapIO's collect callback, which runs over + // every declared block of every stage BEFORE the write-back above happens - so "declared no + // binding" is a fact read off the AST, not a guess made about the text. The lexical scanner + // this replaced could only report positively, dropping any declaration whose grammar it did + // not fully recognise, and could not read `binding = SOME_MACRO` at all (it ran on + // macro-unexpanded source, and reading "no literal" as "no binding" once aliased eight + // Flywheel storage blocks onto 0). // // 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 diff --git a/MobileGL/MG_State/GLState/ProgramState/ProgramObject.h b/MobileGL/MG_State/GLState/ProgramState/ProgramObject.h index 6928d6c4..f9e33a26 100644 --- a/MobileGL/MG_State/GLState/ProgramState/ProgramObject.h +++ b/MobileGL/MG_State/GLState/ProgramState/ProgramObject.h @@ -1205,9 +1205,10 @@ namespace MobileGL::MG_State::GLState { // 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). + // Per-link merged snapshot of the layout(location = N) qualifiers the attached + // shaders' default-block uniforms declared, as glslang recorded them at the point + // its relaxed remap dropped them (the relaxed parse drops them from reflection; the + // DoReflection assigner restores them from here). UnorderedMap linkedExplicitUniformLocations; // Per-link snapshot of the default-block uniform INITIALIZERS the attached shaders // declared ("uniform int i = 1;"). Desktop GLSL says that value is what the uniform @@ -1232,6 +1233,10 @@ namespace MobileGL::MG_State::GLState { Vector uniformIndexInTProgram; // ditto. Will be set at glUniform1i Vector uniformSamplerOrImageUnitIndex; + // Sampler/image layout(binding = N) initial texture/image units, captured by + // TMglGlslIoResolver at mapIO's collect callback - the last point at which the + // qualifier still says what the shader declared. An OUTPUT of the link, not an + // input to it: nothing supplies this map, the resolver fills it. UnorderedMap explicitOpaqueUniformBindings; // Ordered by uniform block index @@ -1256,9 +1261,10 @@ namespace MobileGL::MG_State::GLState { // overwrites it - default and rebind travel one path. UnorderedMap 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. + // layout(binding = N). Input to the seeding above; filled during mapIO by + // TMglGlslIoResolver, which is the last observer that can still tell a declared + // binding from an invented one - and, unlike the per-shader lexer this replaced, + // sees the declaration with its macros expanded. std::set storageBlocksWithoutBinding; Uint activeUniformCount = 0; diff --git a/MobileGL/MG_State/GLState/ProgramState/ShaderCompileTask.cpp b/MobileGL/MG_State/GLState/ProgramState/ShaderCompileTask.cpp index 69d403d6..bc34f77d 100644 --- a/MobileGL/MG_State/GLState/ProgramState/ShaderCompileTask.cpp +++ b/MobileGL/MG_State/GLState/ProgramState/ShaderCompileTask.cpp @@ -154,10 +154,16 @@ namespace { } // The half of a compile that depends on nothing but the source text, the stage and the - // environment snapshot: preprocessing, the three lexical rejections, and the two lexical - // side-channel extractions. Split out so P0b layer 2 can memoize exactly this and - // nothing else - the glslang parse stays per-object because its TShader is consume-once. - // Deliberately free of any per-object state so the memo is sound. + // environment snapshot: preprocessing and the three lexical rejections. Split out so P0b + // layer 2 can memoize exactly this and nothing else - the glslang parse stays per-object + // because its TShader is consume-once. Deliberately free of any per-object state so the + // memo is sound. + // + // The side-channel EXTRACTIONS that used to live here are gone: what the relaxed parse + // destroys is now recovered from glslang itself, at the two points where it is destroyed + // (see ShaderCompileArtifacts::explicitUniformLocations and + // TMglGlslIoResolver::reserverResourceSlot). They could not stay here anyway - none of + // them is a function of the unexpanded source text, which is all this half can see. // // The compute local-size verdict reads `env` rather than the live backend, and // env.fingerprint is part of the P0b cache key, so a memo can never be returned against @@ -195,28 +201,21 @@ namespace { return result; } - if (const std::optional counterOffsetError = - FindAtomicCounterOffsetViolation(result.preprocessedSource)) { - result.outcome = ShaderPreprocessOutcome::AtomicCounterOffsetRejected; - result.infoLog = *counterOffsetError; - return result; - } + // NO ATOMIC-COUNTER OFFSET SCAN HERE ANY MORE: glslang raises both rules itself now, at + // the site where its relaxed remap folds the counter into a synthesized block + // (ParseHelper.cpp atomicCounterOffsetCheck, called from vkRelaxedRemapUniformVariable). + // A violation is an ordinary parse failure, so it reaches GL through the same path every + // other compile error does - and, unlike a scan of unexpanded text, it sees an offset + // spelled as a macro or a const expression. // The parse this feeds runs in the link-compatible configuration (Vulkan-client // env with relaxed rules): the TShader it produces is what glLinkProgram links and // what the backends' SPIR-V is generated from - there is no second, GL-client // parse. The GL frontend semantics the relaxed parse cannot provide are restored - // on top: explicit default-block uniform locations through the lexical - // side-channels below, dead-uniform/global-UBO filtering in + // on top, all of them out of glslang: explicit default-block uniform locations from + // the snapshot the parse takes, opaque bindings and unqualified storage blocks from + // the IO mapper's collect callback, dead-uniform/global-UBO filtering in // 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; } @@ -319,10 +318,14 @@ namespace MobileGL::MG_State::GLState { Bool parsedOk = false; String parseLog; SharedPtr parsedShader; + UnorderedMap explicitUniformLocations; if (verdict) { parsedOk = verdict->parsed; parseLog = verdict->infoLog; + // From the verdict, not from a parse - see ShaderParseVerdict for why they had to + // move into the payload when their origin moved into glslang. + explicitUniformLocations = verdict->explicitUniformLocations; MGLOG_D("ShaderCompileTask: shader %u (stage %d) L1c hit - the glslang parse was skipped; " "compileStatus = %d", externalIndex, static_cast(stage), static_cast(parsedOk)); @@ -335,6 +338,7 @@ namespace MobileGL::MG_State::GLState { parsedOk = result.has_value(); if (parsedOk) { parsedShader = result.value(); + explicitUniformLocations = CollectExplicitUniformLocations(*parsedShader); } else { parseLog = result.error().log; } @@ -344,6 +348,7 @@ namespace MobileGL::MG_State::GLState { // Empty on success by construction, matching what the publish below does with // the artifacts' own log; the diagnostic the application reads on failure. freshVerdict->infoLog = parseLog; + freshVerdict->explicitUniformLocations = explicitUniformLocations; const SizeT verdictBytes = ShaderParseVerdictBytes(*freshVerdict); GetShaderParseVerdictCache().Insert(parseKey, ShaderParseVerdictPtr(Move(freshVerdict)), verdictBytes); @@ -359,9 +364,7 @@ namespace MobileGL::MG_State::GLState { // `fresh` is about to be handed to the cache. Populated on the hit path too - it // is what ClaimParsedShader's deferred parse consumes. artifacts.preprocessedSource = shared.preprocessedSource; - artifacts.explicitUniformLocations = shared.explicitUniformLocations; - artifacts.explicitOpaqueBindings = shared.explicitOpaqueBindings; - artifacts.storageBlocksWithoutBinding = shared.storageBlocksWithoutBinding; + artifacts.explicitUniformLocations = Move(explicitUniformLocations); artifacts.infoLog.clear(); if (shouldPopulateCache) { cache->Insert(stage, sourceHash, *source, compileEnv.fingerprint, Move(fresh)); @@ -386,9 +389,6 @@ namespace MobileGL::MG_State::GLState { if (shouldPopulateCache) { fresh->outcome = ShaderPreprocessOutcome::ParseFailed; fresh->infoLog = artifacts.infoLog; - fresh->explicitUniformLocations.clear(); - fresh->explicitOpaqueBindings.clear(); - fresh->storageBlocksWithoutBinding.clear(); cache->Insert(stage, sourceHash, *source, compileEnv.fingerprint, Move(fresh)); } } diff --git a/MobileGL/MG_State/GLState/ProgramState/ShaderCompileTask.h b/MobileGL/MG_State/GLState/ProgramState/ShaderCompileTask.h index 2c819010..a95bd11c 100644 --- a/MobileGL/MG_State/GLState/ProgramState/ShaderCompileTask.h +++ b/MobileGL/MG_State/GLState/ProgramState/ShaderCompileTask.h @@ -60,19 +60,23 @@ namespace MobileGL::MG_State::GLState { // ClaimParsedShader's re-parse so a later link never depends on the preprocessor // being deterministic across backend-state changes. String preprocessedSource; + // The explicit layout(location = N) qualifiers this stage's default-block uniforms + // declared, as glslang recorded them at the point its Vulkan-relaxed remap dropped + // them (CollectExplicitUniformLocations). + // + // Populated on the L1c HIT path too, out of the cached verdict rather than out of a + // parse - which is why the verdict carries them. Everything else the relaxed parse + // destroys is recovered at LINK instead, from the IO mapper's collect callback, and so + // has no field here at all. UnorderedMap explicitUniformLocations; - UnorderedMap 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 storageBlocksWithoutBinding; String infoLog; Bool compileStatus = false; }; // The unit of asynchronous shader compilation: one glCompileShader's worth of pure CPU - // work - preprocess, the two lexical rejections, the two lexical extractions, and (unless - // the translation memo's compile half already knows the answer) the glslang parse - with - // every input it needs owned by the node itself. + // work - preprocess, the lexical rejections, and (unless the translation memo's compile + // half already knows the answer) the glslang parse plus the explicit-uniform-location + // snapshot it yields - with every input it needs owned by the node itself. // // That ownership is the whole point. The node reads no GL-thread state (the source is a // SharedPtr snapshot, the device limits come from the CompileEnv snapshot, diff --git a/MobileGL/MG_State/GLState/ProgramState/ShaderObject.h b/MobileGL/MG_State/GLState/ProgramState/ShaderObject.h index 9901149f..fb2828fd 100644 --- a/MobileGL/MG_State/GLState/ProgramState/ShaderObject.h +++ b/MobileGL/MG_State/GLState/ProgramState/ShaderObject.h @@ -101,24 +101,11 @@ namespace MobileGL { const SharedPtr& GetCompiledShader() const { return Compiled().shader; } const String& GetInfoLog() const { return Compiled().infoLog; } // Explicit layout(location = N) qualifiers on this shader's default-block - // uniforms, captured lexically at Compile() because the relaxed parse drops - // them from reflection (see ExtractExplicitUniformLocations). + // uniforms, as glslang recorded them at the point its Vulkan-relaxed remap + // discarded them (see CollectExplicitUniformLocations). const UnorderedMap& GetExplicitUniformLocations() const { return Compiled().explicitUniformLocations; } - // Explicit layout(binding = N) on sampler/image uniforms - their initial - // texture/image units - captured lexically for the same reason (see - // ExtractExplicitOpaqueBindings). - const UnorderedMap& 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& GetStorageBlocksWithoutBinding() const { - return Compiled().storageBlocksWithoutBinding; - } Bool GetCompileStatus() const { return Compiled().compileStatus; } Bool GetDeleteStatus() const { return m_deleteStatus; } diff --git a/MobileGL/MG_State/GLState/ProgramState/ShaderPreprocessCache.h b/MobileGL/MG_State/GLState/ProgramState/ShaderPreprocessCache.h index 52a38155..d4f158cf 100644 --- a/MobileGL/MG_State/GLState/ProgramState/ShaderPreprocessCache.h +++ b/MobileGL/MG_State/GLState/ProgramState/ShaderPreprocessCache.h @@ -29,9 +29,6 @@ namespace MobileGL::MG_State::GLState { // FindShaderStorageBindingViolation rejected it: a storage block declared a binding at or // past GL_MAX_SHADER_STORAGE_BUFFER_BINDINGS. ResourceBindingRejected, - // FindAtomicCounterOffsetViolation rejected it: an atomic counter declared a - // layout(offset =) that is misaligned or reaches past GL_MAX_ATOMIC_COUNTER_BUFFER_SIZE. - AtomicCounterOffsetRejected, // The source-only half was clean but glslang rejected the preprocessed source. // Memoizing this saves the parse itself on every later object with that source. ParseFailed, @@ -39,18 +36,23 @@ namespace MobileGL::MG_State::GLState { // Everything ShaderObject::Compile() derives from the source text alone, i.e. // everything that is identical for two shader objects holding byte-identical source. + // + // "The source text alone" is now literally true: the preprocessed text, an accept/reject + // verdict, and the log that explains a rejection. Anything that needs to know what the + // shader MEANS is derived from the parse instead - see the note on the missing fields. struct ShaderPreprocessResult { ShaderPreprocessOutcome outcome = ShaderPreprocessOutcome::Preprocessed; // Valid unless the preprocessor itself never ran; kept even for the rejection // outcomes because that is the text the diagnostics refer to. String preprocessedSource; - UnorderedMap explicitUniformLocations; - UnorderedMap 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 storageBlocksWithoutBinding; + // NO EXTRACTED SIDE CHANNELS ANY MORE, and their absence is the point. Explicit + // uniform locations, explicit opaque bindings and unqualified storage blocks used to be + // lexed out of the text here, which meant reading MACRO-UNEXPANDED source: MobileGL's + // preprocessor rewrites the text, it does not run the C preprocessor, so + // `binding = SOME_MACRO` reached the scanners verbatim. All three now come from + // glslang - the first from a snapshot taken inside the parse, the other two from the + // IO mapper's collect callback - and none of them is a function of the source text + // ALONE any more, which is the only thing this struct is allowed to hold. // The compile info log to publish; empty when outcome == Preprocessed. String infoLog; diff --git a/MobileGL/MG_Test/Program/ProgramUtilTest.cpp b/MobileGL/MG_Test/Program/ProgramUtilTest.cpp index 3c924ab6..a188f3cc 100644 --- a/MobileGL/MG_Test/Program/ProgramUtilTest.cpp +++ b/MobileGL/MG_Test/Program/ProgramUtilTest.cpp @@ -2131,9 +2131,11 @@ void main() { } // The case the old masker actually broke: an apostrophe in real (non-comment) text. Everything after -// it looked like string interior, so ExtractExplicitUniformLocations tokenized a blank source and -// handed the GL location assigner an empty map - the uniform silently lost its explicit location. -TEST_F(ProgramUtilTest, PreprocessApostropheInDirectiveKeepsLaterCodeVisibleToExtractors) { +// it looked like string interior, so the rewriter's own scans went blind past it - which is still +// what this pins, now that the explicit location itself is recovered from the parse rather than +// from a scan. The two halves have to agree end to end: the preprocessed text must still declare +// the uniform, AND the parse must still hand its location back. +TEST_F(ProgramUtilTest, PreprocessApostropheInDirectiveKeepsLaterCodeVisibleToTheParse) { using namespace MG_Util::ShaderTranspiler; String source = R"(#version 460 core @@ -2148,15 +2150,15 @@ void main() { )"; PreprocessShaderSource(ShaderStage::Fragment, source); - const UnorderedMap locations = ExtractExplicitUniformLocations(source); - ASSERT_EQ(locations.count("tint"), 1u) << "extractor went blind past the apostrophe:\n" << source; - EXPECT_EQ(locations.at("tint"), 7); - ShaderAttrib attrib{.shaderType = GL_FRAGMENT_SHADER, .sourceStr = source}; auto res = ShaderCompiler::CompileShader(attrib); if (!res) { FAIL() << "errc: " << res.error().errc << "\nlog: " << res.error().log << "\nsource:\n" << source; } + + const UnorderedMap locations = CollectExplicitUniformLocations(*res.value()); + ASSERT_EQ(locations.count("tint"), 1u) << "the rewriter went blind past the apostrophe:\n" << source; + EXPECT_EQ(locations.at("tint"), 7); } // PreprocessShaderSource used to rediscover "where does the #version directive end?" once per @@ -2340,8 +2342,7 @@ namespace { auto result = MakeShared(); result->outcome = ShaderPreprocessOutcome::Preprocessed; result->preprocessedSource = preprocessed; - result->explicitUniformLocations["uMarker"] = 7; - result->explicitOpaqueBindings["sMarker"] = 3; + result->infoLog = "marker:" + preprocessed; return result; } } // namespace @@ -2358,12 +2359,9 @@ TEST_F(ProgramUtilTest, ShaderPreprocessCacheRoundTripsAndSeparatesStages) { ASSERT_NE(hit, nullptr); EXPECT_TRUE(hit->Preprocessed()); EXPECT_EQ(hit->preprocessedSource, "vertex-preprocessed"); - const auto uniformIt = hit->explicitUniformLocations.find("uMarker"); - ASSERT_NE(uniformIt, hit->explicitUniformLocations.end()); - EXPECT_EQ(uniformIt->second, 7); - const auto bindingIt = hit->explicitOpaqueBindings.find("sMarker"); - ASSERT_NE(bindingIt, hit->explicitOpaqueBindings.end()); - EXPECT_EQ(bindingIt->second, 3u); + // The whole payload round-trips, not just the text: every field the entry carries has to + // come back, or a hit would publish a half-populated result. + EXPECT_EQ(hit->infoLog, "marker:vertex-preprocessed"); // Byte-identical source, different stage: a different key, so still a miss. Two // stages sharing one entry would hand a fragment shader a vertex preprocess. @@ -4418,267 +4416,6 @@ TEST_F(ProgramUtilTest, StorageBlockBindingCeilingIsCheckedAtItsExactBoundary) { EXPECT_FALSE(FindShaderStorageBindingViolation("layout(binding = 36) buffer B { int x; };\n", 0).has_value()); } -// KHR-GL43.shader_atomic_counters.negative-offset-1: an atomic counter whose layout(offset = N) -// puts its last byte past GL_MAX_ATOMIC_COUNTER_BUFFER_SIZE is a COMPILE-time error, and the CTS -// never links the shader at all. MobileGL only had the rule at link, because the Vulkan-relaxed -// parse never reaches glslang's fixOffset(). -TEST_F(ProgramUtilTest, AtomicCounterOffsetCeilingIsCheckedAtCompile) { - using namespace MG_Util::ShaderTranspiler; - - const auto violation = [](const String& body) { - return FindAtomicCounterOffsetViolation("#version 430 core\n" + body + "void main() {}\n"); - }; - const String maxSize = std::to_string(MAX_ATOMIC_COUNTER_BUFFER_SIZE); - const String lastLegal = std::to_string(MAX_ATOMIC_COUNTER_BUFFER_SIZE - 4); - - // The boundary itself: the last counter that still fits, and the first that does not. - EXPECT_FALSE(violation("layout(binding = 0, offset = " + lastLegal + ") uniform atomic_uint c;\n").has_value()); - EXPECT_TRUE(violation("layout(binding = 0, offset = " + maxSize + ") uniform atomic_uint c;\n").has_value()); - - // An array occupies one word per element, so what has to fit is the LAST one. - EXPECT_FALSE(violation("layout(offset = " + std::to_string(MAX_ATOMIC_COUNTER_BUFFER_SIZE - 16) + - ") uniform atomic_uint c[4];\n") - .has_value()); - EXPECT_TRUE(violation("layout(offset = " + std::to_string(MAX_ATOMIC_COUNTER_BUFFER_SIZE - 8) + - ") uniform atomic_uint c[4];\n") - .has_value()); - - // An offset that is not a multiple of 4 (GL 4.6 core 7.7), and one that is. - EXPECT_TRUE(violation("layout(offset = 2) uniform atomic_uint c;\n").has_value()); - EXPECT_FALSE(violation("layout(offset = 8) uniform atomic_uint c;\n").has_value()); - - // Things the scanner must NOT judge: a counter with no explicit offset, an `offset` that is - // an ordinary identifier rather than a layout qualifier, an array sized by an expression, - // and an offset qualifier that belongs to a different declaration. - EXPECT_FALSE(violation("uniform atomic_uint c;\nconst int offset = 99999;\n").has_value()); - EXPECT_FALSE(violation("const int kCount = 4;\nlayout(offset = " + maxSize + - ") uniform atomic_uint c[kCount];\n") - .has_value()); - EXPECT_FALSE(violation("layout(offset = " + maxSize + ") uniform Block { int x; };\n" - "uniform atomic_uint c;\n") - .has_value()); - // A source with no counter at all never pays for the scan and never reports one. - EXPECT_FALSE(FindAtomicCounterOffsetViolation("#version 430 core\nvoid main() {}\n").has_value()); -} - -// KHR-GL43.explicit_uniform_location.uniform-loc-nondecimal: GLSL integer literals are C-style, so -// layout(location = 0xA) is 10 and layout(location = 010) is OCTAL 8. The extractor used to accept -// a base-10 digit run and nothing else: the hex spelling failed the test entirely and the -// declaration silently lost its explicit location, while the octal one was read as decimal 10. -// The identical defect sat on every array dimension and on layout(binding = N). -TEST_F(ProgramUtilTest, ExtractExplicitUniformLocationsReadsNonDecimalIntegerLiterals) { - using namespace MG_Util::ShaderTranspiler; - - const String source = R"(#version 430 core -layout(location = 0xA) uniform vec4 hexLower; -layout(location = 0X1f) uniform vec4 hexUpper; -layout(location = 010) uniform vec4 octal; -layout(location = 3u) uniform vec4 unsignedSuffix; -layout(location = 0x2) uniform float hexArray[0x3]; -layout(location = 1.0) uniform vec4 notAnInteger; -layout(location = 7f) uniform vec4 unknownSuffix; -void main() {} -)"; - - const UnorderedMap locations = ExtractExplicitUniformLocations(source); - ASSERT_EQ(locations.count("hexLower"), 1u); - EXPECT_EQ(locations.at("hexLower"), 10); - ASSERT_EQ(locations.count("hexUpper"), 1u); - EXPECT_EQ(locations.at("hexUpper"), 31); - ASSERT_EQ(locations.count("octal"), 1u); - EXPECT_EQ(locations.at("octal"), 8) << "a leading zero is octal in GLSL, not decimal"; - ASSERT_EQ(locations.count("unsignedSuffix"), 1u); - EXPECT_EQ(locations.at("unsignedSuffix"), 3); - ASSERT_EQ(locations.count("hexArray"), 1u); - EXPECT_EQ(locations.at("hexArray"), 2); - - // Still never guessed at: a float and an unknown suffix are skipped, not rounded. - EXPECT_EQ(locations.count("notAnInteger"), 0u); - EXPECT_EQ(locations.count("unknownSuffix"), 0u); -} - -// A hexadecimal array dimension has to size the declarator's span too, or the declarator after it -// in the same statement starts at the wrong location. -TEST_F(ProgramUtilTest, ExtractExplicitUniformLocationsSpansANonDecimalArrayDimension) { - using namespace MG_Util::ShaderTranspiler; - - const UnorderedMap locations = ExtractExplicitUniformLocations( - "#version 430 core\nlayout(location = 50) uniform float first[0x3], second;\nvoid main() {}\n"); - ASSERT_EQ(locations.count("first"), 1u); - EXPECT_EQ(locations.at("first"), 50); - ASSERT_EQ(locations.count("second"), 1u); - EXPECT_EQ(locations.at("second"), 53) << "0x3 is three elements, not zero and not three hundred"; -} - -// KHR-GL43.explicit_uniform_location.uniform-loc-array-of-arrays: glslang reflects -// `float u[2][3]` as "u[0][0]" and "u[1][0]", and the linker resolves such a name by stripping the -// single trailing "[0]" - so the map has to answer "u[1]", not just "u". Without the pre-flattened -// keys both records missed the map entirely and were first-fitted from location 0. -TEST_F(ProgramUtilTest, ExtractExplicitUniformLocationsExpandsArrayOfArraysElements) { - using namespace MG_Util::ShaderTranspiler; - - const String source = R"(#version 430 core -layout(location = 2) uniform float two_d[2][3]; -layout(location = 20) uniform float three_d[2][2][4]; -layout(location = 40) uniform float one_d[3]; -void main() {} -)"; - - const UnorderedMap locations = ExtractExplicitUniformLocations(source); - - // The root entry is unchanged - the synthesized keys are additional, never a replacement. - ASSERT_EQ(locations.count("two_d"), 1u); - EXPECT_EQ(locations.at("two_d"), 2); - // One key per outer index, each starting a run of the innermost dimension (3 here). - ASSERT_EQ(locations.count("two_d[0]"), 1u); - EXPECT_EQ(locations.at("two_d[0]"), 2); - ASSERT_EQ(locations.count("two_d[1]"), 1u); - EXPECT_EQ(locations.at("two_d[1]"), 5); - - // Three dimensions: glslang expands all but the innermost, so both outer indices are spelled. - ASSERT_EQ(locations.count("three_d"), 1u); - EXPECT_EQ(locations.at("three_d"), 20); - ASSERT_EQ(locations.count("three_d[0][0]"), 1u); - EXPECT_EQ(locations.at("three_d[0][0]"), 20); - ASSERT_EQ(locations.count("three_d[0][1]"), 1u); - EXPECT_EQ(locations.at("three_d[0][1]"), 24); - ASSERT_EQ(locations.count("three_d[1][0]"), 1u); - EXPECT_EQ(locations.at("three_d[1][0]"), 28); - ASSERT_EQ(locations.count("three_d[1][1]"), 1u); - EXPECT_EQ(locations.at("three_d[1][1]"), 32); - - // A 1-D array needs no expansion: stripping "[0]" already reaches the root. - ASSERT_EQ(locations.count("one_d"), 1u); - EXPECT_EQ(locations.at("one_d"), 40); - EXPECT_EQ(locations.count("one_d[0]"), 0u); - - // The declarator after an array-of-arrays still advances by the WHOLE element count. - const UnorderedMap pair = ExtractExplicitUniformLocations( - "#version 430 core\nlayout(location = 0) uniform float a[2][3], b;\nvoid main() {}\n"); - ASSERT_EQ(pair.count("b"), 1u); - EXPECT_EQ(pair.at("b"), 6); -} - -// KHR-GL43.explicit_uniform_location: layout(binding = 0x2) on a sampler is the same literal defect -// as the location one, and losing it costs the sampler its initial texture unit. -TEST_F(ProgramUtilTest, ExtractExplicitOpaqueBindingsReadsNonDecimalIntegerLiterals) { - using namespace MG_Util::ShaderTranspiler; - - const String source = R"(#version 430 core -layout(binding = 0x2) uniform sampler2D hexUnit; -layout(binding = 012) uniform sampler2D octalUnit; -layout(binding = 1u) uniform sampler2D suffixedUnit; -void main() {} -)"; - - const UnorderedMap bindings = ExtractExplicitOpaqueBindings(source); - ASSERT_EQ(bindings.count("hexUnit"), 1u); - EXPECT_EQ(bindings.at("hexUnit"), 2u); - ASSERT_EQ(bindings.count("octalUnit"), 1u); - EXPECT_EQ(bindings.at("octalUnit"), 10u) << "012 is octal ten, not twelve"; - 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 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 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 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 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"; -} - // KHR-GL43.shader_image_size.advanced-nonMS-* is nothing but its passing twin basic-nonMS-* plus a // GLSL subroutine, and glslang refuses the keyword outright when the target is SPIR-V ("subroutine // : not allowed when generating SPIR-V"), so every stage of those shaders failed to compile. The @@ -4819,64 +4556,3 @@ subroutine(FuncType) void Func0(int coord) { fragColor = vec4(float(coord)); } << "an inactive #if arm must not have an unconditional forwarding body appended for it"; } -// THE TEXT THIS SCANS IS NOT MACRO-EXPANDED. MobileGL's preprocessing rewrites the source, it -// does not run the C preprocessor, so a `#define` and every use of it both survive into what -// RunSourceOnlyPipeline hands here. `binding = SOME_MACRO` is therefore the ordinary spelling in -// real shader packs - Flywheel's indirect engine writes every one of its storage blocks that way -// - and reading "no integer literal" as "no binding" defaulted all of them onto binding 0 at -// once, where they aliased and the whole engine drew nothing -// (minecraft-1.21.1-neoforge-create-indirect-in-world, both backends). -TEST_F(ProgramUtilTest, ExtractStorageBlocksWithoutExplicitBindingKeepsAMacroSpelledBinding) { - using namespace MG_Util::ShaderTranspiler; - - // Flywheel's own shape, verbatim in structure: the binding is a macro, the block carries - // memory qualifiers, and the macro's definition is still sitting in the text above it. - const String source = R"(#version 460 core -#define _FLW_MODEL_BUFFER_BINDING 3 -#define _FLW_DRAW_BUFFER_BINDING 4 -layout(local_size_x = 32) in; -layout(std430, binding = _FLW_MODEL_BUFFER_BINDING) restrict readonly buffer ModelBuffer { - uint models[]; -}; -layout(std430, binding = _FLW_DRAW_BUFFER_BINDING) restrict buffer DrawBuffer { - uint draws[]; -}; -layout(std430) buffer ReallyUnqualified { uint u; } reallyUnqualified; -void main() { draws[0] = models[0] + reallyUnqualified.u; } -)"; - - const std::set unqualified = ExtractStorageBlocksWithoutExplicitBinding(source); - EXPECT_EQ(unqualified.count("ModelBuffer"), 0u) - << "a binding spelled as a macro is still a declared binding, never an absent one"; - EXPECT_EQ(unqualified.count("DrawBuffer"), 0u) - << "every block of the engine would otherwise be defaulted onto 0 together"; - EXPECT_EQ(unqualified.count("ReallyUnqualified"), 1u) - << "a block that truly declares no binding is still recognised in the same shader"; -} - -// The other two ways an unexpanded macro can hide a binding: as a whole layout entry, and as the -// whole qualifier run. Both have to read as doubt for the same reason the macro-valued binding -// does - what the scanner cannot expand, it must not claim is absent. -TEST_F(ProgramUtilTest, ExtractStorageBlocksWithoutExplicitBindingDoubtsAMacroQualifier) { - using namespace MG_Util::ShaderTranspiler; - - const String source = R"(#version 430 core -#define FLW_BINDING binding = 2 -#define SSBO_QUALIFIER layout(std430, binding = 6) restrict -layout(local_size_x = 1) in; -layout(std430, FLW_BINDING) buffer EntryMacro { uint a; } entryMacro; -SSBO_QUALIFIER buffer RunMacro { uint b; } runMacro; -layout(std430, row_major) buffer PlainLayout { uint c; } plainLayout; -void main() { plainLayout.c = entryMacro.a + runMacro.b; } -)"; - - const std::set unqualified = ExtractStorageBlocksWithoutExplicitBinding(source); - EXPECT_EQ(unqualified.count("EntryMacro"), 0u) - << "an unreadable layout entry may itself be the binding"; - EXPECT_EQ(unqualified.count("RunMacro"), 0u) - << "a macro standing in for the whole qualifier run may carry the binding"; - // The counterweight: recognising doubt must not swallow the layout identifiers a buffer - // block legally carries, or nothing would ever be defaulted again. - EXPECT_EQ(unqualified.count("PlainLayout"), 1u) - << "std430/row_major are GLSL, not macros, and leave no doubt behind"; -} diff --git a/MobileGL/MG_Test/ShaderTranspiler/CMakeLists.txt b/MobileGL/MG_Test/ShaderTranspiler/CMakeLists.txt index 8e92d409..23e2c031 100644 --- a/MobileGL/MG_Test/ShaderTranspiler/CMakeLists.txt +++ b/MobileGL/MG_Test/ShaderTranspiler/CMakeLists.txt @@ -15,6 +15,7 @@ add_executable( LegalizeResourceArrayIndexTest.cpp FlattenAtomicCounterBlockTest.cpp WidenImageFormatsTest.cpp + GlslangCaptureTest.cpp ) target_include_directories(SpirvPassTest PRIVATE diff --git a/MobileGL/MG_Test/ShaderTranspiler/GlslangCaptureTest.cpp b/MobileGL/MG_Test/ShaderTranspiler/GlslangCaptureTest.cpp new file mode 100644 index 00000000..6f75486f --- /dev/null +++ b/MobileGL/MG_Test/ShaderTranspiler/GlslangCaptureTest.cpp @@ -0,0 +1,678 @@ +// MobileGL - MobileGL/MG_Test/ShaderTranspiler/GlslangCaptureTest.cpp +// Copyright (c) 2025-2026 MobileGL-Dev +// Licensed under the GNU Lesser General Public License v3.0: +// https://www.gnu.org/licenses/gpl-3.0.txt +// https://www.gnu.org/licenses/lgpl-3.0.txt +// SPDX-License-Identifier: LGPL-3.0-only +// End of Source File Header + +// WHAT SURVIVES MOBILEGL'S PARSE, ASKED OF GLSLANG ITSELF. +// +// Every shader is parsed as an EShClientVulkan client under +// setEnvInputVulkanRulesRelaxed(), which destroys some of the GL declarations MobileGL +// still has to answer for. Which ones it destroys - and WHERE - decides whether a piece of +// information can be captured from glslang at all or has to be reconstructed. That question +// used to be answered by comments; these cases answer it by running the real pipeline and +// reading the real qualifiers back. +// +// The probe drives ShaderCompiler::CompileShader (the production parse configuration, byte +// for byte) and then the production mapIO, with a resolver that snapshots every entity's +// qualifier AT THE COLLECT CALLBACK - which is before glslang's IO mapper writes its +// auto-assigned bindings back into the types (iomapper.cpp:240). That callback is the last +// moment at which "the shader declared this" and "glslang invented this" are still +// different statements. + +#include + +#include +#include +#include + +#include "Includes.h" +#include "Init.h" +#include +#include +#include +#include + +using namespace MobileGL; +using namespace MobileGL::MG_Util::ShaderTranspiler; + +namespace { + // One entity as the collect callback sees it. + struct ProbedEntity { + Bool hasBinding = false; + Uint binding = 0; + Bool hasLocation = false; + Int location = 0; + Bool isBlock = false; + Bool isBufferBlock = false; + Bool isSampler = false; + }; + + // A pass-through resolver that records instead of deciding. It derives from the SAME + // base MobileGL ships (TDefaultGlslIoResolver) so the callbacks fire in the same order + // and with the same arguments the production resolver sees. + class ProbeResolver : public glslang::TDefaultGlslIoResolver { + public: + explicit ProbeResolver(const glslang::TProgram& program, const EShLanguage stage) + : TDefaultGlslIoResolver(*program.getIntermediate(stage)) {} + + void reserverResourceSlot(glslang::TVarEntryInfo& ent, TInfoSink& infoSink) override { + Record(ent); + TDefaultGlslIoResolver::reserverResourceSlot(ent, infoSink); + } + + void reserverStorageSlot(glslang::TVarEntryInfo& ent, TInfoSink& infoSink) override { + Record(ent); + TDefaultGlslIoResolver::reserverStorageSlot(ent, infoSink); + } + + std::map probed; + + private: + void Record(const glslang::TVarEntryInfo& ent) { + const glslang::TType& type = ent.symbol->getType(); + const glslang::TQualifier& qualifier = type.getQualifier(); + ProbedEntity& record = probed[ent.symbol->getAccessName().c_str()]; + record.hasBinding = qualifier.hasBinding(); + record.binding = qualifier.hasBinding() ? qualifier.layoutBinding : 0u; + record.hasLocation = qualifier.hasLocation(); + record.location = qualifier.hasLocation() ? static_cast(qualifier.layoutLocation) : -1; + record.isBlock = type.getBasicType() == glslang::EbtBlock; + record.isBufferBlock = record.isBlock && qualifier.storage == glslang::EvqBuffer; + record.isSampler = type.getBasicType() == glslang::EbtSampler; + } + }; + + // Parses `source` exactly as production does, links it, and returns what the collect + // callback saw. Fails the calling test (through the ASSERT_* the caller applies to the + // optional) rather than throwing. + std::optional> ProbeShader(const GLenum stage, const String& source, + String& outLog) { + ShaderAttrib shaderAttrib{.shaderType = stage, .sourceStr = source}; + auto shaderResult = ShaderCompiler::CompileShader(shaderAttrib); + if (!shaderResult) { + outLog = shaderResult.error().log; + return std::nullopt; + } + + auto program = MakeShared(); + program->addShader(shaderResult.value().get()); + if (!program->link(EShMsgDefault)) { + outLog = program->getInfoLog(); + return std::nullopt; + } + + const EShLanguage lang = MG_Util::ConvertGLEnumToEShLanguage(stage); + ProbeResolver resolver(*program, lang); + auto ioMapper = UniquePtr(glslang::GetGlslIoMapper()); + if (!program->mapIO(&resolver, ioMapper.get())) { + outLog = program->getInfoLog(); + return std::nullopt; + } + return resolver.probed; + } + + // What ONE production link captures, taken through the real entry points rather than + // through a probe: ShaderCompiler::CompileShader and ShaderCompiler::LinkProgram with the + // same ProgramAttrib ProgramLinkTask builds. `captureEnabled` false leaves both OUT + // pointers null, which is the negative control every capture case below pairs itself with. + struct LinkCapture { + Bool linked = false; + String log; + UnorderedMap opaqueBindings; + std::set storageBlocksWithoutBinding; + UnorderedMap uniformLocations; + }; + + LinkCapture CaptureFromLink(const Vector>& stages, const Bool captureEnabled = true) { + LinkCapture capture; + ProgramAttrib programAttrib; + for (const auto& [stage, source] : stages) { + ShaderAttrib shaderAttrib{.shaderType = stage, .sourceStr = source}; + auto shaderResult = ShaderCompiler::CompileShader(shaderAttrib); + if (!shaderResult) { + capture.log = shaderResult.error().log; + return capture; + } + for (const auto& [name, location] : CollectExplicitUniformLocations(*shaderResult.value())) { + capture.uniformLocations.emplace(name, location); + } + programAttrib.shaders.push_back(shaderResult.value()); + } + + if (captureEnabled) { + programAttrib.explicitOpaqueUniformBindings = &capture.opaqueBindings; + programAttrib.storageBlocksWithoutBinding = &capture.storageBlocksWithoutBinding; + } + + auto programResult = ShaderCompiler::LinkProgram(programAttrib); + if (!programResult) { + capture.log = programResult.error().log; + return capture; + } + capture.linked = true; + return capture; + } + + LinkCapture CaptureFromCompute(const String& source, const Bool captureEnabled = true) { + return CaptureFromLink({{GL_COMPUTE_SHADER, source}}, captureEnabled); + } +} // namespace + +class GlslangCaptureProbeTest : public ::testing::Test { +protected: + void SetUp() override { MobileGL::Initialize(); } +}; + +// THE HEADLINE ANSWER, and it contradicts what ExtractExplicitOpaqueBindings' header claimed +// for years ("the Vulkan-client relaxed parse strips these before mapIO can capture them"). +// +// A PLAIN sampler/image uniform never enters vkRelaxedRemapUniformVariable's body at all: the +// guard at ParseHelper.cpp:8255-8259 admits only types that containsNonOpaque(), atomic_uint, +// or a sampler inside a STRUCT. So the binding is still on the qualifier when the IO mapper +// collects it, and it is glslang - not a lexer - that knows the answer. +// +// The default-block uniform LOCATION is the opposite verdict, and this case pins both halves +// side by side so neither can be assumed from the other: it is stripped inside that same +// function (ParseHelper.cpp:8261-8263, `layoutLocation = layoutLocationEnd`), which is why +// recovering it needs a snapshot taken INSIDE glslang rather than a resolver callback. +TEST_F(GlslangCaptureProbeTest, OpaqueBindingsSurviveTheRelaxedParseButPlainUniformLocationsDoNot) { + const String source = R"(#version 430 core +layout(local_size_x = 1) in; +layout(binding = 3) uniform sampler2D probeSampler; +layout(binding = 5, rgba32f) uniform image2D probeImage; +layout(location = 7) uniform vec4 probeUniform; +layout(std430, binding = 2) buffer BoundBlock { uint bound; } boundInstance; +layout(std430) buffer UnboundBlock { uint unbound; } unboundInstance; +void main() { + unboundInstance.unbound = boundInstance.bound + uint(texture(probeSampler, vec2(0)).x) + + uint(imageLoad(probeImage, ivec2(0)).x) + uint(probeUniform.x); +} +)"; + + String log; + const auto probed = ProbeShader(GL_COMPUTE_SHADER, source, log); + ASSERT_TRUE(probed.has_value()) << log; + + ASSERT_TRUE(probed->contains("probeSampler")); + EXPECT_TRUE(probed->at("probeSampler").hasBinding) + << "a plain sampler's layout(binding=) is NOT stripped by the relaxed parse"; + EXPECT_EQ(probed->at("probeSampler").binding, 3u); + + ASSERT_TRUE(probed->contains("probeImage")); + EXPECT_TRUE(probed->at("probeImage").hasBinding) + << "images take the same path as samplers (both are EbtSampler)"; + EXPECT_EQ(probed->at("probeImage").binding, 5u); + + // The default-block uniform is gone from the entity list entirely - it was swept into + // MGL_GLOBAL_UBO - and even if it were here it would carry layoutLocationEnd. That is + // exactly why the location capture has to happen inside glslang. + if (probed->contains("probeUniform")) { + EXPECT_FALSE(probed->at("probeUniform").hasLocation) + << "vkRelaxedRemapUniformVariable strips a default-block uniform's location"; + } +} + +// The other half of the storage-block question: at the collect callback, "declared no +// binding" is still distinguishable from "glslang picked one", which is what makes +// TMglGlslIoResolver the right place to recover GL's binding-0 default. Ten lines later +// (iomapper.cpp:240) both blocks carry a number and nothing can tell them apart. +TEST_F(GlslangCaptureProbeTest, StorageBlockBindingPresenceIsStillTruthfulAtTheCollectCallback) { + const String source = R"(#version 430 core +layout(local_size_x = 1) in; +layout(std430, binding = 2) buffer BoundBlock { uint bound; } boundInstance; +layout(std430) buffer UnboundBlock { uint unbound; } unboundInstance; +layout(std140) uniform UniformBlock { uint u; } uniformInstance; +void main() { unboundInstance.unbound = boundInstance.bound + uniformInstance.u; } +)"; + + String log; + const auto probed = ProbeShader(GL_COMPUTE_SHADER, source, log); + ASSERT_TRUE(probed.has_value()) << log; + + ASSERT_TRUE(probed->contains("BoundBlock")); + EXPECT_TRUE(probed->at("BoundBlock").isBufferBlock); + EXPECT_TRUE(probed->at("BoundBlock").hasBinding); + EXPECT_EQ(probed->at("BoundBlock").binding, 2u); + + ASSERT_TRUE(probed->contains("UnboundBlock")); + EXPECT_TRUE(probed->at("UnboundBlock").isBufferBlock); + EXPECT_FALSE(probed->at("UnboundBlock").hasBinding) + << "an unqualified storage block must still read as unqualified here"; + + // A uniform block is a different binding space with its own default path; the capture + // must be able to tell the two apart, which storage == EvqBuffer does. + ASSERT_TRUE(probed->contains("UniformBlock")); + EXPECT_FALSE(probed->at("UniformBlock").isBufferBlock); +} + +// =========================================================================================== +// THE CAPTURES THEMSELVES. +// +// Every case below is the SCENARIO of a scan these captures replaced, re-pointed at the new +// mechanism. Keeping the scenarios is the point: the interesting inputs were found the +// expensive way (a production regression, a CTS failure), and they are still the inputs that +// decide whether the recovery is right - what changed is only who answers. +// +// Every capture also has a NEGATIVE CONTROL: the same shader with the capture switched off, +// asserting the answer disappears. Without one, a case that passes proves only that SOMETHING +// produced the number. +// =========================================================================================== + +// KHR-GL43.explicit_uniform_location.uniform-loc-nondecimal: GLSL integer literals are C-style, +// so layout(location = 0xA) is 10 and layout(location = 010) is OCTAL 8. The lexical extractor +// this replaces had to implement that rule itself, got it wrong for both spellings, and was +// then fixed - twice. glslang has always had it, because it is the GLSL lexer. +TEST_F(GlslangCaptureProbeTest, UniformLocationsCarryNonDecimalIntegerLiterals) { + const String source = R"(#version 430 core +layout(location = 0xA) uniform vec4 hexLower; +layout(location = 0X1f) uniform vec4 hexUpper; +layout(location = 010) uniform vec4 octal; +layout(location = 3u) uniform vec4 unsignedSuffix; +layout(location = 0x2) uniform float hexArray[0x3]; +void main() { + gl_Position = hexLower + hexUpper + octal + unsignedSuffix + vec4(hexArray[2]); +} +)"; + + ShaderAttrib shaderAttrib{.shaderType = GL_VERTEX_SHADER, .sourceStr = source}; + auto shaderResult = ShaderCompiler::CompileShader(shaderAttrib); + ASSERT_TRUE(shaderResult) << shaderResult.error().log; + + const UnorderedMap locations = CollectExplicitUniformLocations(*shaderResult.value()); + ASSERT_EQ(locations.count("hexLower"), 1u); + EXPECT_EQ(locations.at("hexLower"), 10); + ASSERT_EQ(locations.count("hexUpper"), 1u); + EXPECT_EQ(locations.at("hexUpper"), 31); + ASSERT_EQ(locations.count("octal"), 1u); + EXPECT_EQ(locations.at("octal"), 8) << "a leading zero is octal in GLSL, not decimal"; + ASSERT_EQ(locations.count("unsignedSuffix"), 1u); + EXPECT_EQ(locations.at("unsignedSuffix"), 3); + ASSERT_EQ(locations.count("hexArray"), 1u); + EXPECT_EQ(locations.at("hexArray"), 2); +} + +// The counterweight the lexical version needed a rule for: a location that is not an integer +// literal at all. glslang REJECTS those outright rather than skipping them, which is what GLSL +// says should happen - the scanner could only decline to record them and let the declaration +// compile with no location. +TEST_F(GlslangCaptureProbeTest, ANonIntegralUniformLocationIsRejectedRatherThanIgnored) { + ShaderAttrib attrib{.shaderType = GL_VERTEX_SHADER, + .sourceStr = "#version 430 core\nlayout(location = 1.0) uniform vec4 notAnInteger;\n" + "void main() { gl_Position = notAnInteger; }\n"}; + EXPECT_FALSE(ShaderCompiler::CompileShader(attrib).has_value()) + << "a float location is a compile-time error, not a declaration without a location"; +} + +// KHR-GL43.explicit_uniform_location.uniform-loc-array-of-arrays: glslang reflects +// `float u[2][3]` as "u[0][0]" and "u[1][0]", and the location assigner resolves such a name by +// stripping the single trailing "[0]" - so the map has to answer "u[1]", not just "u". The +// synthesized keys are the one piece of the old extractor that survived the migration, because +// they are a REFLECTION-NAME mapping rather than a reading of the source. +TEST_F(GlslangCaptureProbeTest, UniformLocationsExpandArrayOfArraysElements) { + const String source = R"(#version 430 core +layout(location = 2) uniform float two_d[2][3]; +layout(location = 20) uniform float three_d[2][2][4]; +layout(location = 40) uniform float one_d[3]; +void main() { gl_Position = vec4(two_d[1][2] + three_d[1][1][3] + one_d[2]); } +)"; + + ShaderAttrib shaderAttrib{.shaderType = GL_VERTEX_SHADER, .sourceStr = source}; + auto shaderResult = ShaderCompiler::CompileShader(shaderAttrib); + ASSERT_TRUE(shaderResult) << shaderResult.error().log; + const UnorderedMap locations = CollectExplicitUniformLocations(*shaderResult.value()); + + // The root entry is unchanged - the synthesized keys are additional, never a replacement. + ASSERT_EQ(locations.count("two_d"), 1u); + EXPECT_EQ(locations.at("two_d"), 2); + // One key per outer index, each starting a run of the innermost dimension (3 here). + ASSERT_EQ(locations.count("two_d[0]"), 1u); + EXPECT_EQ(locations.at("two_d[0]"), 2); + ASSERT_EQ(locations.count("two_d[1]"), 1u); + EXPECT_EQ(locations.at("two_d[1]"), 5); + + // Three dimensions: glslang expands all but the innermost, so both outer indices are spelled. + ASSERT_EQ(locations.count("three_d"), 1u); + EXPECT_EQ(locations.at("three_d"), 20); + ASSERT_EQ(locations.count("three_d[0][0]"), 1u); + EXPECT_EQ(locations.at("three_d[0][0]"), 20); + ASSERT_EQ(locations.count("three_d[0][1]"), 1u); + EXPECT_EQ(locations.at("three_d[0][1]"), 24); + ASSERT_EQ(locations.count("three_d[1][0]"), 1u); + EXPECT_EQ(locations.at("three_d[1][0]"), 28); + ASSERT_EQ(locations.count("three_d[1][1]"), 1u); + EXPECT_EQ(locations.at("three_d[1][1]"), 32); + + // A 1-D array needs no expansion: stripping "[0]" already reaches the root. + ASSERT_EQ(locations.count("one_d"), 1u); + EXPECT_EQ(locations.at("one_d"), 40); + EXPECT_EQ(locations.count("one_d[0]"), 0u); +} + +// A DELIBERATE BEHAVIOUR CHANGE, recorded here because it is the one place the migration does +// not reproduce the old answer. +// +// The lexical extractor advanced the location across the declarators of one statement, so +// `layout(location = 50) uniform float first[3], second;` gave second = 53. GLSL has no such +// rule: 4.60 4.4 says a layout qualifier applies to THE DECLARATION, i.e. identically to every +// declarator in it, and 4.4.3 then makes two uniforms sharing a location an error. glslang - the +// reference front end - assigns 50 to both, and the CTS never exercises the form at all (its +// generator emits one uniform per declaration, es31cExplicitUniformLocationTest.cpp +// streamDefinition). The advance was an invention of the scanner; this is what the parser says. +TEST_F(GlslangCaptureProbeTest, EveryDeclaratorOfOneStatementCarriesTheQualifiersLocation) { + const String source = R"(#version 430 core +layout(location = 50) uniform float first[0x3], second; +void main() { gl_Position = vec4(first[2] + second); } +)"; + + ShaderAttrib shaderAttrib{.shaderType = GL_VERTEX_SHADER, .sourceStr = source}; + auto shaderResult = ShaderCompiler::CompileShader(shaderAttrib); + ASSERT_TRUE(shaderResult) << shaderResult.error().log; + const UnorderedMap locations = CollectExplicitUniformLocations(*shaderResult.value()); + + ASSERT_EQ(locations.count("first"), 1u); + EXPECT_EQ(locations.at("first"), 50) << "0x3 is three elements, not zero and not three hundred"; + ASSERT_EQ(locations.count("second"), 1u); + EXPECT_EQ(locations.at("second"), 50); +} + +// THE NEGATIVE CONTROL for the uniform-location capture: nothing else in the parsed module +// knows the number. If the snapshot inside vkRelaxedRemapUniformVariable were removed, this is +// the state the location assigner would be left with - no qualifier, no reflection entry, and +// therefore a first-fit location that has nothing to do with what the shader declared. +TEST_F(GlslangCaptureProbeTest, WithoutTheSnapshotAPlainUniformsLocationIsNowhereInTheModule) { + const String source = R"(#version 430 core +layout(location = 7) uniform vec4 tint; +void main() { gl_Position = tint; } +)"; + + ShaderAttrib shaderAttrib{.shaderType = GL_VERTEX_SHADER, .sourceStr = source}; + auto shaderResult = ShaderCompiler::CompileShader(shaderAttrib); + ASSERT_TRUE(shaderResult) << shaderResult.error().log; + + // The capture, on. + const UnorderedMap locations = CollectExplicitUniformLocations(*shaderResult.value()); + ASSERT_EQ(locations.count("tint"), 1u); + EXPECT_EQ(locations.at("tint"), 7); + + // The capture, off - i.e. everything the module itself can still say. The uniform is a + // member of MGL_GLOBAL_UBO by now, and no symbol in the module carries location 7. + auto program = MakeShared(); + program->addShader(shaderResult.value().get()); + ASSERT_TRUE(program->link(EShMsgDefault)) << program->getInfoLog(); + ASSERT_TRUE(program->buildReflection(EShReflectionStrictArraySuffix | EShReflectionBasicArraySuffix | + EShReflectionAllBlockVariables | EShReflectionSharedStd140UBO)); + for (Int i = 0; i < program->getNumUniformVariables(); ++i) { + const auto& uniform = program->getUniform(i); + if (uniform.name != "tint") continue; + // Copied out: layoutLocationEnd is a static const with no out-of-line definition, so + // binding it to EXPECT_EQ's const reference would ODR-use it and fail to link. + const Uint noLocation = glslang::TQualifier::layoutLocationEnd; + EXPECT_EQ(uniform.layoutLocation(), noLocation) + << "if reflection could answer this, the glslang patch would be unnecessary"; + } +} + +// KHR-GL43.explicit_uniform_location: layout(binding = 0x2) on a sampler is its initial texture +// unit. Same scenario the lexical extractor carried, now answered by the IO resolver - which +// gets the C-style literal rules for free, and sees a binding no scanner could have read. +TEST_F(GlslangCaptureProbeTest, OpaqueBindingsAreCapturedIncludingNonDecimalAndMacroSpellings) { + const String source = R"(#version 430 core +#define UNIT_FROM_A_MACRO 5 +layout(local_size_x = 1) in; +layout(binding = 0x2) uniform sampler2D hexUnit; +layout(binding = 012) uniform sampler2D octalUnit; +layout(binding = 1u) uniform sampler2D suffixedUnit; +layout(binding = UNIT_FROM_A_MACRO) uniform sampler2D macroUnit; +layout(binding = 6) uniform sampler2D arrayUnits[3]; +uniform sampler2D noUnit; +layout(std430, binding = 0) buffer Out { vec4 v; } o; +void main() { + o.v = texture(hexUnit, vec2(0)) + texture(octalUnit, vec2(0)) + texture(suffixedUnit, vec2(0)) + + texture(macroUnit, vec2(0)) + texture(arrayUnits[1], vec2(0)) + texture(noUnit, vec2(0)); +} +)"; + + const LinkCapture capture = CaptureFromCompute(source); + ASSERT_TRUE(capture.linked) << capture.log; + + ASSERT_EQ(capture.opaqueBindings.count("hexUnit"), 1u); + EXPECT_EQ(capture.opaqueBindings.at("hexUnit"), 2u); + ASSERT_EQ(capture.opaqueBindings.count("octalUnit"), 1u); + EXPECT_EQ(capture.opaqueBindings.at("octalUnit"), 10u) << "012 is octal ten, not twelve"; + ASSERT_EQ(capture.opaqueBindings.count("suffixedUnit"), 1u); + EXPECT_EQ(capture.opaqueBindings.at("suffixedUnit"), 1u); + // The whole reason the capture moved: the AST sees expanded text. + ASSERT_EQ(capture.opaqueBindings.count("macroUnit"), 1u) + << "a unit spelled as a macro is a declared unit like any other"; + EXPECT_EQ(capture.opaqueBindings.at("macroUnit"), 5u); + // An array is keyed by its declared name, which is what the reflection lookup strips "[0]" + // to reach. + ASSERT_EQ(capture.opaqueBindings.count("arrayUnits"), 1u); + EXPECT_EQ(capture.opaqueBindings.at("arrayUnits"), 6u); + // Reported POSITIVELY: a sampler that declared no unit must not appear at all, or it would + // be given one it never asked for. + EXPECT_EQ(capture.opaqueBindings.count("noUnit"), 0u); +} + +// THE NEGATIVE CONTROL for the opaque-binding capture. +TEST_F(GlslangCaptureProbeTest, OpaqueBindingsDisappearWhenTheResolverCaptureIsOff) { + const String source = R"(#version 430 core +layout(local_size_x = 1) in; +layout(binding = 3) uniform sampler2D unit; +layout(std430, binding = 0) buffer Out { vec4 v; } o; +void main() { o.v = texture(unit, vec2(0)); } +)"; + + ASSERT_EQ(CaptureFromCompute(source).opaqueBindings.count("unit"), 1u); + const LinkCapture off = CaptureFromCompute(source, /*captureEnabled=*/false); + ASSERT_TRUE(off.linked) << off.log; + EXPECT_TRUE(off.opaqueBindings.empty()) + << "nothing but the resolver fills this map; a non-empty result would mean the capture " + "is being shadowed by a leftover path"; +} + +// 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. GL 4.3 core 7.8 puts such a +// block on binding ZERO; by the time reflection is built glslang has invented a number and +// written it into the qualifier, so this capture is the only surviving record. +TEST_F(GlslangCaptureProbeTest, UnqualifiedStorageBlocksAreNamedAndQualifiedOnesAreNot) { + 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] + g_bound.data1[0] + g_bound_first.data2[0]; +} +)"; + + const LinkCapture capture = CaptureFromCompute(source); + ASSERT_TRUE(capture.linked) << capture.log; + + EXPECT_EQ(capture.storageBlocksWithoutBinding.count("OutputBuffer"), 1u) + << "the block the test binds at 0 with glBindBufferBase must be recognised"; + EXPECT_EQ(capture.storageBlocksWithoutBinding.count("BoundBlock"), 0u) + << "a declared binding must never be defaulted away"; + EXPECT_EQ(capture.storageBlocksWithoutBinding.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(capture.storageBlocksWithoutBinding.count("InputBuffer"), 0u) + << "uniform blocks are out of scope"; +} + +// The capture must not mistake a buffer-typed SAMPLER or a member qualifier for a block, and +// memory qualifiers in either order must not cost a block its binding - the dangerous +// direction, because a false positive here DEFAULTS AWAY a binding the shader really declared. +TEST_F(GlslangCaptureProbeTest, StorageBlockCaptureSurvivesMemoryQualifiersAndIgnoresBufferSamplers) { + const String source = R"(#version 430 core +layout(local_size_x = 1) in; +uniform samplerBuffer texelSampler; +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 + uint(texelFetch(texelSampler, 0).x); } +)"; + + const LinkCapture capture = CaptureFromCompute(source); + ASSERT_TRUE(capture.linked) << capture.log; + + EXPECT_EQ(capture.storageBlocksWithoutBinding.count("AfterLayout"), 0u) + << "coherent/restrict must not break the qualifier run and lose the binding"; + EXPECT_EQ(capture.storageBlocksWithoutBinding.count("BeforeLayout"), 0u) + << "a qualifier may precede the layout list too"; + EXPECT_EQ(capture.storageBlocksWithoutBinding.count("NoBindingAtAll"), 1u) + << "a memory-qualified block with no binding is still an unqualified block"; + EXPECT_EQ(capture.storageBlocksWithoutBinding.count("texelSampler"), 0u) + << "a samplerBuffer is not a buffer block"; +} + +// THE REGRESSION THIS MIGRATION EXISTS FOR (7de7cfc6, +// minecraft-1.21.1-neoforge-create-indirect-in-world, both backends). Flywheel's indirect +// engine spells every storage-block binding as a macro, and the scan that used to answer this +// question ran on MACRO-UNEXPANDED text: MobileGL's preprocessing rewrites the source, it does +// not run the C preprocessor, so `binding = _FLW_MODEL_BUFFER_BINDING` reached the scanner +// verbatim and "no integer literal" was read as "no binding". All eight blocks were defaulted +// onto binding 0 at once, aliased there, and the engine drew nothing. +// +// It passes here for a structural reason rather than a grammatical one: the IO mapper sees the +// declaration the PARSER built, and the parser ran the preprocessor first. No rule about macro +// spellings exists anywhere in this path, and none can be forgotten. +TEST_F(GlslangCaptureProbeTest, AMacroSpelledStorageBlockBindingIsADeclaredBinding) { + // Flywheel's own shape, verbatim in structure: the binding is a macro, the block carries + // memory qualifiers, and the macro's definition is still sitting in the text above it. + const String source = R"(#version 460 core +#define _FLW_MODEL_BUFFER_BINDING 3 +#define _FLW_DRAW_BUFFER_BINDING 4 +#define FLW_BINDING binding = 2 +#define SSBO_QUALIFIER layout(std430, binding = 6) restrict +layout(local_size_x = 32) in; +layout(std430, binding = _FLW_MODEL_BUFFER_BINDING) restrict readonly buffer ModelBuffer { + uint models[]; +}; +layout(std430, binding = _FLW_DRAW_BUFFER_BINDING) restrict buffer DrawBuffer { + uint draws[]; +}; +layout(std430, FLW_BINDING) buffer EntryMacro { uint a; } entryMacro; +SSBO_QUALIFIER buffer RunMacro { uint b; } runMacro; +layout(std430, row_major) buffer PlainLayout { uint c; } plainLayout; +layout(std430) buffer ReallyUnqualified { uint u; } reallyUnqualified; +void main() { + draws[0] = models[0] + reallyUnqualified.u + entryMacro.a + runMacro.b + plainLayout.c; +} +)"; + + const LinkCapture capture = CaptureFromCompute(source); + ASSERT_TRUE(capture.linked) << capture.log; + + EXPECT_EQ(capture.storageBlocksWithoutBinding.count("ModelBuffer"), 0u) + << "a binding spelled as a macro is still a declared binding, never an absent one"; + EXPECT_EQ(capture.storageBlocksWithoutBinding.count("DrawBuffer"), 0u) + << "every block of the engine would otherwise be defaulted onto 0 together"; + // The two shapes the scanner could only treat as DOUBT - a macro standing in for a whole + // layout entry, and one standing in for the whole qualifier run - are now ordinary. + EXPECT_EQ(capture.storageBlocksWithoutBinding.count("EntryMacro"), 0u) + << "a macro that expands to `binding = N` declares a binding"; + EXPECT_EQ(capture.storageBlocksWithoutBinding.count("RunMacro"), 0u) + << "a macro standing in for the whole qualifier run carries its binding too"; + // The counterweight: doubt must not swallow the layout identifiers a buffer block legally + // carries, or nothing would ever be defaulted again. + EXPECT_EQ(capture.storageBlocksWithoutBinding.count("PlainLayout"), 1u) + << "std430/row_major are layout identifiers, not bindings"; + EXPECT_EQ(capture.storageBlocksWithoutBinding.count("ReallyUnqualified"), 1u) + << "a block that truly declares no binding is still recognised in the same shader"; +} + +// THE NEGATIVE CONTROL for the storage-block capture. +TEST_F(GlslangCaptureProbeTest, UnqualifiedStorageBlocksDisappearWhenTheResolverCaptureIsOff) { + const String source = R"(#version 430 core +layout(local_size_x = 1) in; +layout(std430) buffer Unbound { uint u; } unbound; +void main() { unbound.u = 1u; } +)"; + + ASSERT_EQ(CaptureFromCompute(source).storageBlocksWithoutBinding.count("Unbound"), 1u); + const LinkCapture off = CaptureFromCompute(source, /*captureEnabled=*/false); + ASSERT_TRUE(off.linked) << off.log; + EXPECT_TRUE(off.storageBlocksWithoutBinding.empty()) + << "nothing but the resolver fills this set; a non-empty result would mean the capture " + "is being shadowed by a leftover path"; +} + +// A block declared in two stages contributes ONCE, and the capture is a union across them - +// which is what one resolver serving the whole program gives for free. GLSL requires every +// stage that declares a block to declare it identically, so the stages cannot disagree. +TEST_F(GlslangCaptureProbeTest, TheStorageBlockCaptureIsAUnionAcrossStages) { + const String vertex = R"(#version 430 core +layout(std430) buffer SharedBlock { uint u; } sharedInstance; +layout(std430) buffer VertexOnly { uint v; } vertexOnly; +void main() { gl_Position = vec4(float(sharedInstance.u + vertexOnly.v)); } +)"; + const String fragment = R"(#version 430 core +layout(std430) buffer SharedBlock { uint u; } sharedInstance; +layout(std430, binding = 4) buffer FragmentBound { uint f; } fragmentBound; +out vec4 colour; +void main() { colour = vec4(float(sharedInstance.u + fragmentBound.f)); } +)"; + + const LinkCapture capture = + CaptureFromLink({{GL_VERTEX_SHADER, vertex}, {GL_FRAGMENT_SHADER, fragment}}); + ASSERT_TRUE(capture.linked) << capture.log; + + EXPECT_EQ(capture.storageBlocksWithoutBinding.count("SharedBlock"), 1u); + EXPECT_EQ(capture.storageBlocksWithoutBinding.count("VertexOnly"), 1u); + EXPECT_EQ(capture.storageBlocksWithoutBinding.count("FragmentBound"), 0u); +} + +// KHR-GL43.shader_atomic_counters.negative-offset-1: an atomic counter at a misaligned offset, +// or one whose last byte passes GL_MAX_ATOMIC_COUNTER_BUFFER_SIZE, is a COMPILE-time error +// (GL 4.6 core 7.7). glslang enforces the alignment rule in fixOffset(), which the relaxed +// parse never reaches - vkRelaxedRemapUniformVariable folds the counter into a synthesized +// block and returns from declareVariable() first - and it never enforced the size ceiling at +// all. Both now run at that fold (ParseHelper.cpp atomicCounterOffsetCheck), so a violation is +// an ordinary parse failure. +TEST_F(GlslangCaptureProbeTest, AtomicCounterOffsetRulesAreRaisedByTheParse) { + const auto compiles = [](const String& body) { + const String source = "#version 430 core\n" + body + "void main() {}\n"; + ShaderAttrib attrib{.shaderType = GL_VERTEX_SHADER, .sourceStr = source}; + return ShaderCompiler::CompileShader(attrib).has_value(); + }; + const String maxSize = std::to_string(MAX_ATOMIC_COUNTER_BUFFER_SIZE); + const String lastLegal = std::to_string(MAX_ATOMIC_COUNTER_BUFFER_SIZE - 4); + + // The boundary itself: the last counter that still fits, and the first that does not. + EXPECT_TRUE(compiles("layout(binding = 0, offset = " + lastLegal + ") uniform atomic_uint c;\n")); + EXPECT_FALSE(compiles("layout(binding = 0, offset = " + maxSize + ") uniform atomic_uint c;\n")); + + // An array occupies one word per element, so what has to fit is the LAST one. + EXPECT_TRUE(compiles("layout(binding = 0, offset = " + std::to_string(MAX_ATOMIC_COUNTER_BUFFER_SIZE - 16) + + ") uniform atomic_uint c[4];\n")); + EXPECT_FALSE(compiles("layout(binding = 0, offset = " + std::to_string(MAX_ATOMIC_COUNTER_BUFFER_SIZE - 8) + + ") uniform atomic_uint c[4];\n")); + + // An offset that is not a multiple of 4, and one that is. + EXPECT_FALSE(compiles("layout(binding = 0, offset = 2) uniform atomic_uint c;\n")); + EXPECT_TRUE(compiles("layout(binding = 0, offset = 8) uniform atomic_uint c;\n")); + + // A counter with no explicit offset has nothing to judge, and neither has a shader with no + // counter at all. + EXPECT_TRUE(compiles("layout(binding = 0) uniform atomic_uint c;\n")); + EXPECT_TRUE(compiles("")); + + // The gain over the scan this replaces: an array sized by a constant EXPRESSION, and an + // offset spelled as a macro, are now both judged. The scanner declined both - it read + // unexpanded text and only understood integer literals. + EXPECT_FALSE(compiles("const int kCount = 4;\nlayout(binding = 0, offset = " + + std::to_string(MAX_ATOMIC_COUNTER_BUFFER_SIZE - 8) + + ") uniform atomic_uint c[kCount];\n")); + EXPECT_FALSE(compiles("#define BAD_OFFSET 2\nlayout(binding = 0, offset = BAD_OFFSET) uniform atomic_uint c;\n")); + + // The counterweight: `offset` as an ordinary identifier is not a layout qualifier, and an + // offset qualifier on an unrelated declaration must not reach the counter. + EXPECT_TRUE(compiles("layout(binding = 0) uniform atomic_uint c;\nconst int offset = 99999;\n")); +} diff --git a/MobileGL/MG_Test/ShaderTranspiler/TranslationCacheTest.cpp b/MobileGL/MG_Test/ShaderTranspiler/TranslationCacheTest.cpp index e68f0af1..f4851cdf 100644 --- a/MobileGL/MG_Test/ShaderTranspiler/TranslationCacheTest.cpp +++ b/MobileGL/MG_Test/ShaderTranspiler/TranslationCacheTest.cpp @@ -475,11 +475,11 @@ TEST_F(TranslationCacheTest, L1KeyMovesWithEveryInputThatMovesTheSpirv) { v.explicitFragmentOutIndices = &fragIndex; variants.emplace_back("explicitFragmentOutIndices", BuildSpirvTranslationKey(v)); } - { // the merged layout(binding = N) opaque units - SpirvTranslationKeyInputs v = base; - v.explicitOpaqueUniformBindings = &opaque; - variants.emplace_back("explicitOpaqueUniformBindings", BuildSpirvTranslationKey(v)); - } + // NOT the merged layout(binding = N) opaque units, which used to be a variant here: that + // map is an OUTPUT of mapIO (TMglGlslIoResolver writes it and never reads it), so it is a + // pure function of the stage sources this key already carries in full. It was dropped from + // SpirvTranslationKeyInputs with the glslang-capture migration; kKeyLayoutVersion moved to + // 4 so no blob written under the old shape can be honoured. { // ShaderCompileBits (0 on both production parse paths; keyed so a future value // cannot alias a module parsed without it) SpirvTranslationKeyInputs v = base; diff --git a/MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.cpp b/MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.cpp index 25555ee4..1367f9c2 100644 --- a/MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.cpp +++ b/MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.cpp @@ -358,6 +358,107 @@ namespace MobileGL { glslang::SetThreadPoolAllocator(nullptr); } + namespace { + // glslang reflects an array-of-arrays default-block uniform as ONE RECORD PER + // outer-index tuple, carrying the innermost array type: `float u[2][3]` becomes + // "u[0][0]" and "u[1][0]" (that last "[0]" is EShReflectionBasicArraySuffix). The + // linker resolves such a name by stripping the single trailing "[0]", so it looks + // up "u[1]" - a key the root entry alone cannot answer, and the whole declaration + // silently loses its explicit location. + // + // Emit those pre-flattened keys next to the root, so the result is + // order-independent: each carries the location its own element starts at (element + // i of `float u[2][3]` at location L starts at L + i*3). Identifiers cannot + // contain brackets, so a synthesized key never collides with a real uniform name, + // and a 1-D array needs none of this - stripping "[0]" already reaches the root. + void RecordArrayOfArraysElementLocations(const String& name, const std::vector& dimensions, + const long long baseLocation, + UnorderedMap& locations) { + if (dimensions.size() < 2) return; + // A pathological declaration must not be able to blow up the map; past the cap + // only the root entry stands, which is what every case used to get. + constexpr long long kMaxSynthesizedKeys = 4096; + const long long innerSpan = dimensions.back(); + const SizeT outerDimensions = dimensions.size() - 1; + long long elementCount = 1; + for (SizeT d = 0; d < outerDimensions; ++d) { + elementCount *= dimensions[d]; + if (elementCount > kMaxSynthesizedKeys) return; + } + for (long long element = 0; element < elementCount; ++element) { + String key = name; + long long remainder = element; + for (SizeT d = 0; d < outerDimensions; ++d) { + long long stride = 1; + for (SizeT inner = d + 1; inner < outerDimensions; ++inner) stride *= dimensions[inner]; + key += "[" + std::to_string(remainder / stride) + "]"; + remainder %= stride; + } + locations.emplace(key, static_cast(std::min(baseLocation + element * innerSpan, + static_cast(INT_MAX / 2)))); + } + } + } // namespace + + UnorderedMap CollectExplicitUniformLocations(const glslang::TShader& shader) { + UnorderedMap locations; + const glslang::TIntermediate* intermediate = shader.getIntermediate(); + if (intermediate == nullptr) return locations; + + // Half one: the uniforms the relaxed remap swallowed, out of the snapshot it + // takes on the way past. + for (const glslang::TIntermediate::TUniformLocation& record : + intermediate->getUniformLocations()) { + if (record.location < 0) continue; + // Keep the first sighting. Two records for one name mean the parser saw the + // declaration twice, and the first is the one the symbol table kept. + locations.emplace(record.name, record.location); + RecordArrayOfArraysElementLocations(record.name, record.arraySizes, record.location, + locations); + } + + // Half two: the OPAQUE uniforms, which the remap never touches (the guard in + // vkRelaxedRemapUniformVariable admits only types containing something + // non-opaque, atomic_uint, or a sampler inside a struct) and which therefore + // still carry their qualifier here. + // + // They belong in the same map even though reflection could also answer for them, + // and the distinction is not cosmetic: this map is what marks a location as + // SOURCE-EXPLICIT, i.e. API contract under ARB_explicit_uniform_location. A + // location that only reaches DoReflection through glslang's own layoutLocation() + // is treated as implementation-chosen and quietly moved on a collision, which is + // the wrong answer for one the shader declared. + // + // Read BEFORE any link: mapIO writes its own choices into these same qualifiers + // (iomapper.cpp:240), so this is only truthful while the shader is unlinked - + // which is exactly where ShaderCompileTask calls it. + const glslang::TIntermAggregate* linkerObjects = intermediate->findLinkerObjects(); + if (linkerObjects == nullptr) return locations; + for (TIntermNode* node : linkerObjects->getSequence()) { + const glslang::TIntermSymbol* symbol = node ? node->getAsSymbolNode() : nullptr; + if (symbol == nullptr) continue; + const glslang::TType& type = symbol->getType(); + const glslang::TQualifier& qualifier = type.getQualifier(); + if (qualifier.storage != glslang::EvqUniform || !qualifier.hasLocation()) continue; + // A BLOCK has no glGetUniformLocation of its own, and its members are + // addressed through the block. Only loose uniforms take locations. + if (type.getBasicType() == glslang::EbtBlock || type.isBuiltIn()) continue; + + std::vector arraySizes; + if (type.isArray() && type.getArraySizes() != nullptr) { + const glslang::TArraySizes& sizes = *type.getArraySizes(); + for (int dim = 0; dim < sizes.getNumDims(); ++dim) { + arraySizes.push_back(sizes.getDimSize(dim)); + } + } + const String name = symbol->getAccessName().c_str(); + const Int location = static_cast(qualifier.layoutLocation); + locations.emplace(name, location); + RecordArrayOfArraysElementLocations(name, arraySizes, location, locations); + } + return locations; + } + Result> ShaderCompiler::LinkProgram(const ProgramAttrib& attrib) { SharedPtr program = MakeShared(); for (auto& s : attrib.shaders) { @@ -383,7 +484,8 @@ namespace MobileGL { MakeUnique(*program, (EShLanguage)stage, attrib.explicitVertexInLocations, attrib.explicitFragmentOutLocations, attrib.explicitFragmentOutIndices, - attrib.explicitOpaqueUniformBindings); + attrib.explicitOpaqueUniformBindings, + attrib.storageBlocksWithoutBinding); break; } auto ioMapper = UniquePtr(glslang::GetGlslIoMapper()); diff --git a/MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.h b/MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.h index 41840591..a7d7a63f 100644 --- a/MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.h +++ b/MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.h @@ -445,6 +445,24 @@ namespace MobileGL { // identical question for the identical decision. static Bool ModuleReadsLocatedInput(const Vector& spirv); }; + + // The explicit layout(location = N) qualifiers this shader's DEFAULT-BLOCK uniforms + // declared, keyed the way glslang's own reflection will later spell them. + // + // They cannot be read back off the parsed module, and that is not an oversight of + // this function: MobileGL parses every shader as a Vulkan client under relaxed + // rules, which sweeps plain uniforms into MGL_GLOBAL_UBO - where a location + // qualifier has no meaning - and DROPS the qualifier on the way past + // (ParseHelper.cpp vkRelaxedRemapUniformVariable). What this reads is the snapshot + // glslang takes at that exact site, handed over through TIntermediate; the GL + // location assigner in ProgramLinkTask::DoReflection is the only party left that + // can honour the number. + // + // Keyed by declared name (no "[0]" suffix), plus one synthesized key per outer + // index of an array-of-arrays - see the note in the implementation for why + // reflection needs those spelled out. A uniform declared in several stages must + // agree, which the caller enforces across stages. + UnorderedMap CollectExplicitUniformLocations(const glslang::TShader& shader); } // namespace ShaderTranspiler } // namespace MG_Util } // namespace MobileGL diff --git a/MobileGL/MG_Util/ShaderTranspiler/ShaderSourceProcessor.cpp b/MobileGL/MG_Util/ShaderTranspiler/ShaderSourceProcessor.cpp index 3055f223..3b7f8c7a 100644 --- a/MobileGL/MG_Util/ShaderTranspiler/ShaderSourceProcessor.cpp +++ b/MobileGL/MG_Util/ShaderTranspiler/ShaderSourceProcessor.cpp @@ -1621,22 +1621,6 @@ namespace MobileGL { } namespace { - // The layout qualifiers a shader storage block may legally carry WITHOUT a - // value. GLSL 4.60 4.4.5: a buffer block's layout list holds the memory-layout - // and matrix-order identifiers below, plus binding/offset/align, which are - // spelled `name = value` and so never reach this test. Anything else bare in - // such a list is not GLSL - on the unexpanded text this scanner reads, it is a - // macro, and a macro may expand to the binding itself. - bool IsBufferBlockLayoutIdentifier(const String& text) { - static const char* kLayoutIdentifiers[] = { - "shared", "packed", "std140", "std430", "row_major", "column_major", - }; - for (const char* identifier : kLayoutIdentifiers) { - if (text == identifier) return true; - } - return false; - } - bool IsNonLayoutQualifierKeyword(const String& text) { static const char* kQualifiers[] = { "highp", "mediump", "lowp", "precise", "const", "flat", @@ -1669,266 +1653,8 @@ namespace MobileGL { return true; } - // glslang reflects an array-of-arrays default-block uniform as ONE RECORD PER - // outer-index tuple, carrying the innermost array type: `float u[2][3]` becomes - // "u[0][0]" and "u[1][0]" (that last "[0]" is EShReflectionBasicArraySuffix). The - // linker resolves such a name by stripping the single trailing "[0]", so it looks - // up "u[1]" - a key the root entry alone cannot answer, and the whole declaration - // silently loses its explicit location. - // - // Emit those pre-flattened keys here, next to the root, so the result is - // order-independent: each carries the location its own element starts at (element - // i of `float u[2][3]` at location L starts at L + i*3). Identifiers cannot - // contain brackets, so a synthesized key never collides with a real uniform name, - // and a 1-D array needs none of this - stripping "[0]" already reaches the root. - void RecordArrayOfArraysElementLocations(const String& name, const Vector& dimensions, - long long baseLocation, - MobileGL::UnorderedMap& locations) { - if (dimensions.size() < 2) return; - // A pathological declaration must not be able to blow up the map; past the cap - // only the root entry stands, which is what every case used to get. - constexpr long long kMaxSynthesizedKeys = 4096; - const long long innerSpan = dimensions.back(); - const SizeT outerDimensions = dimensions.size() - 1; - long long elementCount = 1; - for (SizeT d = 0; d < outerDimensions; ++d) { - elementCount *= dimensions[d]; - if (elementCount > kMaxSynthesizedKeys) return; - } - for (long long element = 0; element < elementCount; ++element) { - String key = name; - long long remainder = element; - for (SizeT d = 0; d < outerDimensions; ++d) { - long long stride = 1; - for (SizeT inner = d + 1; inner < outerDimensions; ++inner) stride *= dimensions[inner]; - key += "[" + std::to_string(remainder / stride) + "]"; - remainder %= stride; - } - locations.emplace(key, static_cast( - std::min(baseLocation + element * innerSpan, - static_cast(INT_MAX / 2)))); - } - } - - // Parses one brace-free depth-0 statement [begin, end) and records its - // declarators when it is a uniform declaration carrying an integral - // layout(location = N). Multi-declarator statements assign consecutive - // locations, each declarator advancing by its array element count - // (ARB_explicit_uniform_location rules). Anything the narrow grammar does - // not recognize is skipped, never guessed at. - void RecordUniformDeclarationLocations(const Vector& tokens, SizeT begin, SizeT end, - MobileGL::UnorderedMap& locations) { - using MobileGL::Int; - long long location = -1; - long long literal = 0; - bool sawUniform = false; - SizeT declaratorBegin = end; - - for (SizeT k = begin; k < end;) { - const String& text = tokens[k].text; - if (text == "layout" && k + 1 < end && tokens[k + 1].text == "(") { - SizeT j = k + 2; - Int parenDepth = 1; - while (j < end && parenDepth > 0) { - const String& layoutToken = tokens[j].text; - if (layoutToken == "(") { - ++parenDepth; - } else if (layoutToken == ")") { - --parenDepth; - } else if (parenDepth == 1 && layoutToken == "location" && j + 2 < end && - tokens[j + 1].text == "=" && - ParseGlslIntegerLiteral(tokens[j + 2].text, literal)) { - location = std::min(literal, static_cast(INT_MAX / 2)); - j += 2; - } - ++j; - } - k = j; - continue; - } - if (text == "uniform") { - sawUniform = true; - ++k; - continue; - } - if (sawUniform && location >= 0 && IsIdentifierToken(tokens[k]) && - !IsNonLayoutQualifierKeyword(text)) { - declaratorBegin = k + 1; // 'text' is the type; declarators follow - break; - } - ++k; - } - - if (!sawUniform || location < 0 || declaratorBegin >= end) return; - - long long nextLocation = location; - for (SizeT k = declaratorBegin; k < end;) { - if (!IsIdentifierToken(tokens[k])) return; // malformed; record nothing further - const String& name = tokens[k].text; - ++k; - long long span = 1; - Vector dimensions; - while (k < end && tokens[k].text == "[") { - ++k; - long long dimension = 1; - if (k < end && ParseGlslIntegerLiteral(tokens[k].text, literal)) { - dimension = literal; - ++k; - } - if (k >= end || tokens[k].text != "]") return; // sized by expression; bail out - ++k; - dimensions.push_back( - std::max(1ll, std::min(dimension, static_cast(INT_MAX / 2)))); - span *= dimensions.back(); - } - // Keep the first sighting: a duplicate can only come from alternative - // preprocessor branches declaring the same name. - locations.emplace(name, static_cast(std::min( - nextLocation, static_cast(INT_MAX / 2)))); - RecordArrayOfArraysElementLocations(name, dimensions, nextLocation, locations); - nextLocation += span; - if (k >= end) break; - if (tokens[k].text == "=") { // skip an initializer up to the declarator comma - Int nestingDepth = 0; - ++k; - while (k < end) { - const String& initializerToken = tokens[k].text; - if (initializerToken == "(" || initializerToken == "[") { - ++nestingDepth; - } else if (initializerToken == ")" || initializerToken == "]") { - --nestingDepth; - } else if (initializerToken == "," && nestingDepth == 0) { - break; - } - ++k; - } - } - if (k >= end) break; - if (tokens[k].text != ",") return; - ++k; - } - } - // Parses one brace-free depth-0 statement [begin, end) and records its - // declarators when it is a sampler/image uniform declaration carrying an - // integral layout(binding = N). Such a binding is a GL texture/image unit, - // which the Vulkan-client relaxed parse strips before mapIO can observe it - // (it is not a valid descriptor binding there), so it is extracted lexically - // and restored as the uniform's initial unit. Every declarator in the - // statement shares the qualifier's binding, matching what the GL-client - // mapIO used to capture from the shared type qualifier. Anything the narrow - // grammar does not recognize is skipped, never guessed at. - void RecordOpaqueDeclarationBindings(const Vector& tokens, SizeT begin, SizeT end, - MobileGL::UnorderedMap& bindings) { - using MobileGL::Int; - long long binding = -1; - long long literal = 0; - bool sawUniform = false; - SizeT declaratorBegin = end; - - for (SizeT k = begin; k < end;) { - const String& text = tokens[k].text; - if (text == "layout" && k + 1 < end && tokens[k + 1].text == "(") { - SizeT j = k + 2; - Int parenDepth = 1; - while (j < end && parenDepth > 0) { - const String& layoutToken = tokens[j].text; - if (layoutToken == "(") { - ++parenDepth; - } else if (layoutToken == ")") { - --parenDepth; - } else if (parenDepth == 1 && layoutToken == "binding" && j + 2 < end && - tokens[j + 1].text == "=" && - ParseGlslIntegerLiteral(tokens[j + 2].text, literal)) { - binding = std::min(literal, static_cast(INT_MAX / 2)); - j += 2; - } - ++j; - } - k = j; - continue; - } - if (text == "uniform") { - sawUniform = true; - ++k; - continue; - } - if (sawUniform && binding >= 0 && IsIdentifierToken(tokens[k]) && - !IsNonLayoutQualifierKeyword(text)) { - // 'text' is the type. Only sampler/image opaques carry unit - // bindings; on anything else (e.g. atomic_uint, whose binding - // is a counter-buffer index) record nothing. - if (text.find("sampler") == String::npos && text.find("image") == String::npos) return; - declaratorBegin = k + 1; - break; - } - ++k; - } - - if (!sawUniform || binding < 0 || declaratorBegin >= end) return; - - for (SizeT k = declaratorBegin; k < end;) { - if (!IsIdentifierToken(tokens[k])) return; // malformed; record nothing further - const String& name = tokens[k].text; - ++k; - while (k < end && tokens[k].text == "[") { - ++k; - if (k < end && ParseGlslIntegerLiteral(tokens[k].text, literal)) ++k; - if (k >= end || tokens[k].text != "]") return; // sized by expression; bail out - ++k; - } - bindings[name] = static_cast(binding); - if (k >= end) break; - if (tokens[k].text != ",") return; // opaque declarators cannot take initializers - ++k; - } - } } // namespace - UnorderedMap ExtractExplicitOpaqueBindings(const String& source) { - UnorderedMap bindings; - // Fast path: without the qualifier keyword there is nothing to extract. - if (source.find("binding") == String::npos) return bindings; - - const Vector tokens = TokenizeCode(source); - const SizeT count = tokens.size(); - Int braceDepth = 0; - SizeT pos = 0; - while (pos < count) { - const String& text = tokens[pos].text; - if (text == "{") { - ++braceDepth; - ++pos; - continue; - } - if (text == "}") { - if (braceDepth > 0) --braceDepth; - ++pos; - continue; - } - if (braceDepth != 0 || text == ";") { - ++pos; - continue; - } - - // A depth-0 statement runs to its ';'. One that opens a brace instead is - // a function definition or an interface/uniform block: a block's binding - // is a buffer binding point, not a texture unit, so skip both alike. - SizeT statementEnd = pos; - while (statementEnd < count && tokens[statementEnd].text != ";" && - tokens[statementEnd].text != "{") { - ++statementEnd; - } - if (statementEnd >= count || tokens[statementEnd].text == "{") { - pos = statementEnd; - continue; - } - - RecordOpaqueDeclarationBindings(tokens, pos, statementEnd, bindings); - pos = statementEnd + 1; - } - return bindings; - } - namespace { // Binding points a storage-block declaration starting at `bufferPos` occupies. // One for a scalar instance (and for the "layout(...) buffer;" default-qualifier @@ -1967,129 +1693,6 @@ namespace MobileGL { } } // namespace - std::set ExtractStorageBlocksWithoutExplicitBinding(const String& source) { - std::set 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 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 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; - // Set when the run carries something this scanner cannot read as a binding but - // which MAY BE ONE. THE TEXT SCANNED HERE IS NOT MACRO-EXPANDED - MobileGL's - // preprocessing rewrites the source, it does not run the C preprocessor, so - // `#define`s and their uses both survive into it. `binding = SOME_MACRO` is - // therefore the common spelling in real shader packs, not an exotic one - // (Flywheel's indirect engine writes every one of its blocks that way), and - // reading "no literal" as "no binding" DEFAULTS AWAY a binding the shader - // really declared: every such block would be seeded to 0 and alias there. Doubt - // is resolved by dropping the name, which restores the pre-seeding behaviour - // for exactly the declarations this scanner cannot read. - bool unreadableQualifier = false; - long long literal = 0; - const auto endRun = [&binding, &unreadableQualifier]() { - binding = -1; - unreadableQualifier = false; - }; - for (SizeT pos = 0; pos < count; ++pos) { - const String& text = tokens[pos].text; - if (text == "{") { - ++braceDepth; - endRun(); - 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) { - // Nested parentheses belong to some entry's value expression, - // which is not a literal - the entry itself is judged below. - } else if (layoutToken == "binding" && j + 2 < count && tokens[j + 1].text == "=") { - if (ParseGlslIntegerLiteral(tokens[j + 2].text, literal)) { - binding = std::min(literal, static_cast(INT_MAX / 2)); - } else { - // A binding IS declared here; its value is just spelled as - // something this scanner does not evaluate (a macro, a - // const, an expression). The one thing it certainly is not - // is absent. - unreadableQualifier = true; - } - j += 2; - } else if (IsIdentifierToken(tokens[j]) && !IsBufferBlockLayoutIdentifier(layoutToken) && - !(j + 1 < count && tokens[j + 1].text == "=")) { - // A bare identifier that is none of the layout qualifiers a - // buffer block may legally carry. On unexpanded text that is a - // macro, and a macro may well be the binding - // (`layout(std430, MY_BINDING) buffer B {...}`). - unreadableQualifier = true; - } - ++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 == "{") { - // The token immediately before `buffer` is the last of the run. A - // non-keyword identifier there is a macro standing in for the whole - // qualifier list (`SSBO_QUALIFIER buffer B {...}`), so it may carry - // the binding just as an unreadable layout entry may. - const bool macroQualifier = pos > 0 && IsIdentifierToken(tokens[pos - 1]) && - !IsNonLayoutQualifierKeyword(tokens[pos - 1].text); - const bool unqualified = binding < 0 && !unreadableQualifier && !macroQualifier; - (unqualified ? names : qualified).insert(tokens[pos + 1].text); - } - endRun(); - 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. - // Ending the run clears the doubt with it: the unreadable qualifier belonged - // to the declaration that just ended, not to whatever follows. - if (!IsNonLayoutQualifierKeyword(text)) endRun(); - } - for (const String& name : qualified) { - names.erase(name); - } - return names; - } - std::optional FindShaderStorageBindingViolation(const String& source, Int maxBindings) { // A backend that advertises nothing has no ceiling to enforce. if (maxBindings <= 0) return std::nullopt; @@ -2102,8 +1705,8 @@ namespace MobileGL { const Vector tokens = TokenizeCode(source); const SizeT count = tokens.size(); // The binding the qualifier run currently being scanned declared, -1 for none. - // Several layout(...) lists may precede one declaration and the later one wins, - // which is the same accumulate-then-consume shape the extractors above use. + // Several layout(...) lists may precede one declaration and the later one wins: + // accumulate, then consume at the `buffer` keyword. long long binding = -1; long long literal = 0; for (SizeT pos = 0; pos < count; ++pos) { @@ -2146,137 +1749,6 @@ namespace MobileGL { return std::nullopt; } - std::optional FindAtomicCounterOffsetViolation(const String& source) { - // Fast path: both keywords are required for a violation to exist, and the pair is - // absent from every shader-pack source. - if (source.find("atomic_uint") == String::npos || source.find("offset") == String::npos) { - return std::nullopt; - } - - constexpr long long kAtomicCounterSize = 4; // one 32-bit word per counter - const long long maxBufferSize = static_cast(MAX_ATOMIC_COUNTER_BUFFER_SIZE); - const Vector tokens = TokenizeCode(source); - const SizeT count = tokens.size(); - // The offset the qualifier run currently being scanned declared, -1 for none. - // Same accumulate-then-consume shape as the storage-binding scan above. - long long offset = -1; - long long literal = 0; - for (SizeT pos = 0; pos < count; ++pos) { - const String& text = tokens[pos].text; - 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 == "offset" && j + 2 < count && - tokens[j + 1].text == "=" && - ParseGlslIntegerLiteral(tokens[j + 2].text, literal)) { - offset = literal; - j += 2; - } - ++j; - } - pos = j - 1; - continue; - } - if (text == "atomic_uint") { - // How far the declaration reaches: `atomic_uint c[N]` occupies N words - // from the offset. An unparsable or absent declarator (an expression-sized - // array, or the "layout(...) uniform atomic_uint;" default-qualifier form, - // which declares no counter at all) is left alone rather than guessed at - - // over-rejection here would be a compile failure the application cannot - // work around. - long long elements = 1; - SizeT k = pos + 1; - if (k < count && IsIdentifierToken(tokens[k])) { - ++k; - if (k < count && tokens[k].text == "[") { - elements = (k + 2 < count && tokens[k + 2].text == "]" && - ParseGlslIntegerLiteral(tokens[k + 1].text, literal)) - ? std::max(1, literal) - : -1; - } - } else { - elements = -1; - } - // Clamped so the byte arithmetic below cannot overflow on an absurd - // literal; any element count at or past the ceiling already fails. - elements = std::min(elements, maxBufferSize); - - if (offset >= 0 && elements > 0) { - if (offset % kAtomicCounterSize != 0) { - return "ERROR: invalid value " + std::to_string(offset) + - " for layout specifier 'offset': an atomic counter offset must be a " - "multiple of 4."; - } - if (offset > maxBufferSize - elements * kAtomicCounterSize) { - return "ERROR: invalid value " + std::to_string(offset) + - " for layout specifier 'offset': an atomic counter ending at byte " + - std::to_string(offset + elements * kAtomicCounterSize) + - " passes GL_MAX_ATOMIC_COUNTER_BUFFER_SIZE (" + - std::to_string(maxBufferSize) + ")."; - } - } - offset = -1; - continue; - } - // `uniform` and the precision/auxiliary qualifiers may sit between the layout - // list and the type keyword; anything else ends the run, so an offset never - // leaks onto an unrelated declaration. - if (text != "uniform" && !IsNonLayoutQualifierKeyword(text)) offset = -1; - } - return std::nullopt; - } - - UnorderedMap ExtractExplicitUniformLocations(const String& source) { - UnorderedMap locations; - // Fast path: without the qualifier keyword there is nothing to extract. - if (source.find("location") == String::npos) return locations; - - const Vector tokens = TokenizeCode(source); - const SizeT count = tokens.size(); - Int braceDepth = 0; - SizeT pos = 0; - while (pos < count) { - const String& text = tokens[pos].text; - if (text == "{") { - ++braceDepth; - ++pos; - continue; - } - if (text == "}") { - if (braceDepth > 0) --braceDepth; - ++pos; - continue; - } - if (braceDepth != 0 || text == ";") { - ++pos; - continue; - } - - // A depth-0 statement runs to its ';'. One that opens a brace instead is a - // function definition or an interface/uniform block: neither can declare a - // default-block uniform location, so hand the '{' back to the depth tracker. - SizeT statementEnd = pos; - while (statementEnd < count && tokens[statementEnd].text != ";" && - tokens[statementEnd].text != "{") { - ++statementEnd; - } - if (statementEnd >= count || tokens[statementEnd].text == "{") { - pos = statementEnd; - continue; - } - - RecordUniformDeclarationLocations(tokens, pos, statementEnd, locations); - pos = statementEnd + 1; - } - return locations; - } - } // namespace ShaderTranspiler } // namespace MG_Util } // namespace MobileGL diff --git a/MobileGL/MG_Util/ShaderTranspiler/ShaderSourceProcessor.h b/MobileGL/MG_Util/ShaderTranspiler/ShaderSourceProcessor.h index 3b45a3ac..28369aef 100644 --- a/MobileGL/MG_Util/ShaderTranspiler/ShaderSourceProcessor.h +++ b/MobileGL/MG_Util/ShaderTranspiler/ShaderSourceProcessor.h @@ -45,68 +45,44 @@ namespace MobileGL { // compile-error text for the first violation, or nullopt for a clean source. std::optional FindReservedIdentifierViolation(const String& source); - // Explicit layout(location = N) qualifiers on default-block uniform declarations, - // keyed by declared name (no "[0]" suffix). Multi-declarator statements assign - // consecutive locations, advancing by the array element count. - // - // Exists because the single link-compatible parse runs under relaxed Vulkan rules, - // where glslang's vkRelaxedRemapUniformVariable moves plain uniforms into - // MGL_GLOBAL_UBO and DISCARDS their location qualifiers ("ignoring layout qualifier - // for uniform location"); opaque uniforms keep theirs. This lexical side-channel - // restores the discarded locations to the GL location assigner - // (ProgramObject::DoReflection). It scans preprocessor-visible text, so a - // declaration inside an inactive #if branch is still recorded - harmless unless a - // pack declares the same uniform with different explicit locations in alternative - // branches (none observed; explicit uniform locations have zero incidence in the - // shader-pack corpus, this is an ARB_explicit_uniform_location conformance surface). - UnorderedMap ExtractExplicitUniformLocations(const String& source); - - // Explicit layout(binding = N) on sampler/image uniforms, i.e. their initial - // texture/image units. The Vulkan-client relaxed parse strips these before - // mapIO can capture them, so they are recovered lexically (same narrow - // grammar discipline as ExtractExplicitUniformLocations). - UnorderedMap 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 ExtractStorageBlocksWithoutExplicitBinding(const String& source); + // NO SIDE-CHANNEL EXTRACTORS LIVE HERE ANY MORE. Three of them did - explicit + // default-block uniform locations, explicit sampler/image bindings, and the storage + // blocks that declared no binding - each recovering something MobileGL's + // Vulkan-client relaxed parse destroys. All three are now taken from glslang at the + // point of destruction instead: + // * uniform locations: a snapshot inside vkRelaxedRemapUniformVariable, read back + // through CollectExplicitUniformLocations (ShaderCompiler.h); + // * opaque bindings and unqualified storage blocks: + // TMglGlslIoResolver::reserverResourceSlot, which mapIO calls while the + // qualifier still says what the shader declared. + // The rewrites below stay lexical by construction - they exist to make glslang + // ACCEPT input it would otherwise reject, so they cannot be built on its parse. // 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 - // has to fit. glslang cannot raise it for MobileGL: every shader is parsed as a Vulkan - // client under relaxed rules, where the GL ceilings do not apply, and TBuiltInResource - // has no storage-buffer binding field to check against in the first place. Returns the - // compile-error text for the first violation, or nullopt for a clean source. - // `maxBindings` is what glGetIntegerv answers for that pname; a non-positive value - // means "nothing to check against" and every declaration passes. + // has to fit. Returns the compile-error text for the first violation, or nullopt for + // a clean source. `maxBindings` is what glGetIntegerv answers for that pname; a + // non-positive value means "nothing to check against" and every declaration passes. + // + // THE ONE SCAN THAT COULD NOT MOVE TO GLSLANG, and the reason is structural rather + // than a matter of where the check is written. glslang has no resource limit for this + // ceiling at all - Include/ResourceLimits.h carries maxAtomicCounterBindings, + // maxCombinedTextureImageUnits and forty others, but nothing for uniform-block or + // storage-block binding points - so there is no number for a parse-time check to + // compare against, and the relaxed Vulkan rules MobileGL parses under would exempt it + // anyway (ParseHelper.cpp layoutTypeCheck gates its binding ceilings on + // `spvVersion.vulkan == 0`). Reading the AST post-parse from MobileGL is possible and + // would be strictly better - a macro-spelled binding would finally be checked - but + // the limit is a per-device number that CompileEnv deliberately keeps OUT of + // frontendFingerprint (see its classification), so the L1c parse-verdict key would + // have to grow it before any such verdict could be memoized. That is a cache-key + // change in exchange for a new REJECTION surface, which is the one direction that + // cannot be validated without device time. + // + // Consequence, and it is deliberate: a binding this scanner cannot read as a literal + // is not judged. Under-rejection, never over-rejection. std::optional FindShaderStorageBindingViolation(const String& source, Int maxBindings); - - // GL 4.6 core 7.7 / ARB_shader_atomic_counters makes it a COMPILE-time error to - // declare an atomic counter at an offset that is not a multiple of 4, or whose last - // byte passes GL_MAX_ATOMIC_COUNTER_BUFFER_SIZE. glslang enforces both in fixOffset(), - // which the Vulkan-relaxed parse never reaches (vkRelaxedRemapUniformVariable folds - // the atomic_uint into a synthesized storage block and returns from declareVariable() - // first), so MobileGL only caught them at LINK - and - // KHR-GL43.shader_atomic_counters.negative-offset-1 never links at all. The - // cross-stage rule (two counters sharing a binding must not overlap) stays at link: - // a single-stage source cannot see it. Returns the compile-error text for the first - // violation, or nullopt for a clean source. - std::optional FindAtomicCounterOffsetViolation(const String& source); } // namespace ShaderTranspiler } // namespace MG_Util } // namespace MobileGL diff --git a/MobileGL/MG_Util/ShaderTranspiler/TranslationCache.cpp b/MobileGL/MG_Util/ShaderTranspiler/TranslationCache.cpp index 00c58c50..54f33007 100644 --- a/MobileGL/MG_Util/ShaderTranspiler/TranslationCache.cpp +++ b/MobileGL/MG_Util/ShaderTranspiler/TranslationCache.cpp @@ -26,7 +26,11 @@ namespace MobileGL::MG_Util::ShaderTranspiler { // 2: L2 gained atomicCounterEsslBindingTop (wave3's atomic-counter block rebinding // prints it into the emitted ESSL), and L1c was added. // 3: L2 gained the two interface-block rename maps (wave4's UniquifyIoBlockNames). - constexpr Uint32 kKeyLayoutVersion = 3u; + // 4: the glslang-capture migration. L1 DROPPED explicitOpaqueUniformBindings from its + // key (that map is an output of mapIO, not an input to it), and L1c's PAYLOAD gained + // the explicit uniform locations - so a blob written under 3 describes a differently + // shaped answer at both levels even where the bytes would have matched. + constexpr Uint32 kKeyLayoutVersion = 4u; // The repo's existing cache epoch (MG_Config::CacheVersion, the seed // ProgramFactory::ComputeHash uses). Strictly redundant for an in-memory @@ -128,7 +132,6 @@ namespace MobileGL::MG_Util::ShaderTranspiler { builder.NameMap(inputs.explicitVertexInLocations ? *inputs.explicitVertexInLocations : kEmpty); builder.NameMap(inputs.explicitFragmentOutLocations ? *inputs.explicitFragmentOutLocations : kEmpty); builder.NameMap(inputs.explicitFragmentOutIndices ? *inputs.explicitFragmentOutIndices : kEmpty); - builder.NameMap(inputs.explicitOpaqueUniformBindings ? *inputs.explicitOpaqueUniformBindings : kEmpty); static const Vector kNoXfb; builder.TextList(inputs.requestedXfbVaryings ? *inputs.requestedXfbVaryings : kNoXfb); builder.Value(inputs.xfbBufferMode); @@ -146,7 +149,13 @@ namespace MobileGL::MG_Util::ShaderTranspiler { return MakeTranslationCacheKey(builder); } - SizeT ShaderParseVerdictBytes(const ShaderParseVerdict& verdict) { return verdict.infoLog.size(); } + SizeT ShaderParseVerdictBytes(const ShaderParseVerdict& verdict) { + SizeT bytes = verdict.infoLog.size(); + for (const auto& [name, location] : verdict.explicitUniformLocations) { + bytes += name.size() + sizeof(Int); + } + return bytes; + } // Leaked for the same exit-order reason as the other two; see the note below. BoundedTranslationCache& GetShaderParseVerdictCache() { diff --git a/MobileGL/MG_Util/ShaderTranspiler/TranslationCache.h b/MobileGL/MG_Util/ShaderTranspiler/TranslationCache.h index 43f989fa..d4645fa0 100644 --- a/MobileGL/MG_Util/ShaderTranspiler/TranslationCache.h +++ b/MobileGL/MG_Util/ShaderTranspiler/TranslationCache.h @@ -365,11 +365,14 @@ namespace MobileGL::MG_Util::ShaderTranspiler { // covers backend identity and the advertised extension vector; // * per stage, in link order: the GL stage enum and the FULL preprocessed // source, which is literally the text ParseShaderSource was given; - // * the four link-time request maps mapIO resolves against + // * the three link-time request maps mapIO resolves against // (glBindAttribLocation / glBindFragDataLocation / - // glBindFragDataLocationIndexed, and the merged layout(binding=) opaque - // units) - these steer TMglGlslIoResolver and therefore the Locations and - // Bindings baked into every module; + // glBindFragDataLocationIndexed) - these steer TMglGlslIoResolver and + // therefore the Locations and Bindings baked into every module. NOT the + // merged layout(binding=) opaque units, which used to sit here: they are + // an OUTPUT of mapIO (TMglGlslIoResolver writes that map and never reads + // it), so they are a pure function of the stage sources already in this + // key and keying on them discriminated nothing; // * the ShaderCompileBits the parse ran under (always 0 in production; in // the key so a future non-zero value cannot alias); // * the SPIR-V validation switch (byte-identical output either way, but it @@ -393,7 +396,6 @@ namespace MobileGL::MG_Util::ShaderTranspiler { const UnorderedMap* explicitVertexInLocations = nullptr; const UnorderedMap* explicitFragmentOutLocations = nullptr; const UnorderedMap* explicitFragmentOutIndices = nullptr; - const UnorderedMap* explicitOpaqueUniformBindings = nullptr; Uint32 shaderCompileFlags = 0; Bool enableSpirvValidation = false; // ---- inputs that only matter because the PAYLOAD now carries the reflection ---- @@ -476,6 +478,16 @@ namespace MobileGL::MG_Util::ShaderTranspiler { // successful parse, so a successful compile's observable log is empty no matter what // glslang wrote into it. Stored rather than assumed so the two cannot drift. String infoLog; + // The explicit default-block uniform locations the parse recovered + // (CollectExplicitUniformLocations), empty when `parsed` is false. + // + // IN THE PAYLOAD BECAUSE A HIT SKIPS THE PARSE. These used to come from a lexical scan + // of the source, which ran in the half a hit still executes; they now come from the + // glslang snapshot, which a hit never produces. They belong to the same key as the + // verdict itself - a pure function of (front-end env, stage, preprocessed source) - so + // no key widening is needed, only this field. Without it an L1c hit would publish a + // shader with no explicit locations at all and the program would first-fit them from 0. + UnorderedMap explicitUniformLocations; }; using ShaderParseVerdictPtr = SharedPtr; diff --git a/MobileGL/MG_Util/ShaderTranspiler/Types.h b/MobileGL/MG_Util/ShaderTranspiler/Types.h index 3eda5f31..1269ce2e 100644 --- a/MobileGL/MG_Util/ShaderTranspiler/Types.h +++ b/MobileGL/MG_Util/ShaderTranspiler/Types.h @@ -69,7 +69,13 @@ namespace MobileGL { // Dual-source blend color index per fragment output (glBindFragDataLocationIndexed) -> // emitted as layout(index = N). UnorderedMap explicitFragmentOutIndices; + // ---- OUT parameters, written by TMglGlslIoResolver during mapIO ---- + // Neither is an input: the resolver only ever writes them. They exist because + // the IO mapper's collect callback is the last point at which a resource's + // qualifier still says what the SHADER declared rather than what glslang + // assigned - see the comment on TMglGlslIoResolver::reserverResourceSlot. UnorderedMap* explicitOpaqueUniformBindings = nullptr; + std::set* storageBlocksWithoutBinding = nullptr; }; struct ProgramBinaryAttrib { diff --git a/MobileGL/MG_Util/ShaderTranspiler/glslang/TMglGlslIoResolver.cpp b/MobileGL/MG_Util/ShaderTranspiler/glslang/TMglGlslIoResolver.cpp index 4aef3a06..7febc3f0 100644 --- a/MobileGL/MG_Util/ShaderTranspiler/glslang/TMglGlslIoResolver.cpp +++ b/MobileGL/MG_Util/ShaderTranspiler/glslang/TMglGlslIoResolver.cpp @@ -12,6 +12,10 @@ #include "TMglGlslIoResolver.h" +#include + +#include + namespace MobileGL { bool TMglGlslIoResolver::ShouldAssignPlainUniformLocation(const glslang::TType& type) const { if (!doAutoLocationMapping()) { @@ -149,12 +153,47 @@ namespace MobileGL { return TDefaultGlslIoResolver::resolveInOutLocation(stage, ent); } + // THE COLLECT CALLBACK IS THE CAPTURE POINT, and the reason is a matter of ten lines of + // glslang. mapIO gathers every declared symbol of every stage and calls this on each of + // them (iomapper.cpp addStage -> TSlotCollector) BEFORE it resolves anything; only + // afterwards, in doMap(), does it write the slots it chose back into the types + // (iomapper.cpp:240, `layoutBinding = at->second.newBinding`). Up to here + // `qualifier.hasBinding()` still answers "did the SHADER say so?"; past it, every resource + // carries a number and the question can no longer be asked at all. + // + // Both captures below used to be lexical scans of the shader source, which had to run + // before the preprocessor's macros were expanded and therefore could not read + // `binding = SOME_MACRO` - the spelling Flywheel's indirect engine uses for every one of + // its storage blocks. Asking the AST instead makes the macro case ordinary. void TMglGlslIoResolver::reserverResourceSlot(glslang::TVarEntryInfo& ent, TInfoSink& infoSink) { const glslang::TType& type = ent.symbol->getType(); + const glslang::TQualifier& qualifier = type.getQualifier(); + // getAccessName() is the BLOCK TYPE name for a block and the declared name for + // everything else (IntermTraverse.cpp TIntermSymbol::getAccessName) - which is exactly + // the key both consumers want. + const glslang::TString& name = ent.symbol->getAccessName(); + if (m_explicitOpaqueUniformBindings != nullptr && type.getBasicType() == glslang::EbtSampler && - type.getQualifier().hasBinding()) { - const glslang::TString& name = ent.symbol->getAccessName(); - (*m_explicitOpaqueUniformBindings)[name.c_str()] = type.getQualifier().layoutBinding; + qualifier.hasBinding()) { + (*m_explicitOpaqueUniformBindings)[name.c_str()] = qualifier.layoutBinding; + } + + // A storage block that declared no binding. UNION across stages by construction - one + // resolver serves the whole program - which is what GLSL's "every stage must declare + // the same block identically" rule makes correct. + // + // NOT the atomic-counter blocks glslang SYNTHESIZES, which are storage blocks by every + // structural test available here and are still not what this set means. Relaxed parsing + // folds each atomic_uint into a "gl_AtomicCounterBlock_" block + // (ParseContextBase::growAtomicCounterBlock) and leaves it unbound because MobileGL asks + // for auto-mapped bindings - so it arrives looking exactly like an unqualified + // application block. Seeding one to GL binding 0 would overwrite the counter buffer's + // real binding, which is the trailing number in that very name. + if (m_storageBlocksWithoutBinding != nullptr && type.getBasicType() == glslang::EbtBlock && + qualifier.storage == glslang::EvqBuffer && !qualifier.hasBinding() && + name.compare(0, std::strlen(MG_Util::ShaderTranspiler::ATOMIC_COUNTER_BLOCK_PREFIX), + MG_Util::ShaderTranspiler::ATOMIC_COUNTER_BLOCK_PREFIX) != 0) { + m_storageBlocksWithoutBinding->insert(name.c_str()); } TDefaultGlslIoResolver::reserverResourceSlot(ent, infoSink); diff --git a/MobileGL/MG_Util/ShaderTranspiler/glslang/TMglGlslIoResolver.h b/MobileGL/MG_Util/ShaderTranspiler/glslang/TMglGlslIoResolver.h index d1c22683..d4bd8dad 100644 --- a/MobileGL/MG_Util/ShaderTranspiler/glslang/TMglGlslIoResolver.h +++ b/MobileGL/MG_Util/ShaderTranspiler/glslang/TMglGlslIoResolver.h @@ -12,6 +12,7 @@ #pragma once +#include #include #include #include @@ -27,14 +28,17 @@ namespace MobileGL { using ExplicitVarSlotMap = UnorderedMap; TMglGlslIoResolver(const glslang::TIntermediate& intermediate, const ExplicitVarSlotMap& vertexIns, const ExplicitVarSlotMap& fragOuts, const ExplicitVarSlotMap& fragOutIndices, - ExplicitVarSlotMap* opaqueUniformBindings) + ExplicitVarSlotMap* opaqueUniformBindings, + std::set* storageBlocksWithoutBinding = nullptr) : TDefaultGlslIoResolver(intermediate), m_explicitVertexIns(vertexIns), m_explicitFragOuts(fragOuts), - m_explicitFragOutIndices(fragOutIndices), m_explicitOpaqueUniformBindings(opaqueUniformBindings) {} + m_explicitFragOutIndices(fragOutIndices), m_explicitOpaqueUniformBindings(opaqueUniformBindings), + m_storageBlocksWithoutBinding(storageBlocksWithoutBinding) {} TMglGlslIoResolver(const glslang::TProgram& program, const EShLanguage stage, const ExplicitVarSlotMap& vertexIns, const ExplicitVarSlotMap& fragOuts, - const ExplicitVarSlotMap& fragOutIndices, ExplicitVarSlotMap* opaqueUniformBindings) + const ExplicitVarSlotMap& fragOutIndices, ExplicitVarSlotMap* opaqueUniformBindings, + std::set* storageBlocksWithoutBinding = nullptr) : TMglGlslIoResolver(*program.getIntermediate(stage), vertexIns, fragOuts, fragOutIndices, - opaqueUniformBindings) {} + opaqueUniformBindings, storageBlocksWithoutBinding) {} void reserverStorageSlot(glslang::TVarEntryInfo& ent, TInfoSink& infoSink) override; void reserverResourceSlot(glslang::TVarEntryInfo& ent, TInfoSink& infoSink) override; int resolveInOutLocation(EShLanguage stage, glslang::TVarEntryInfo& ent) override; @@ -47,7 +51,17 @@ namespace MobileGL { const ExplicitVarSlotMap& m_explicitVertexIns; const ExplicitVarSlotMap& m_explicitFragOuts; const ExplicitVarSlotMap& m_explicitFragOutIndices; + // Two OUT channels, both filled from reserverResourceSlot and never read back by this + // resolver. They exist because the collect callback is the LAST place the shader's own + // declaration is still legible: ten lines later (iomapper.cpp:240) mapIO writes its + // auto-assigned binding into the very qualifier that says whether the shader declared + // one. Anything downstream that needs "as DECLARED" rather than "as ASSIGNED" has to be + // handed it from here. ExplicitVarSlotMap* m_explicitOpaqueUniformBindings = nullptr; + // Block TYPE names of the shader storage blocks that reached mapIO carrying NO + // layout(binding = N). GL 4.3 core 7.8 gives such a block binding ZERO; see + // ProgramLinkTask::SeedDefaultStorageBlockBindings for what is done with them. + std::set* m_storageBlocksWithoutBinding = nullptr; std::map m_plainUniformLocationSizeByName; std::map m_plainUniformLocationByName; bool m_plainUniformLocationsAssigned = false;