From de8e7a4606ac9f4603cc2ef3651a5352a2aafb69 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Thu, 20 Aug 2026 13:24:04 -0400 Subject: [PATCH] [Fix, Test] (ShaderTranspiler): parse layout literals in every GLSL base and key array-of-arrays uniforms per element --- MobileGL/MG_Test/Program/ProgramUtilTest.cpp | 119 ++++++++++++++++++ .../ShaderSourceProcessor.cpp | 103 ++++++++++++--- 2 files changed, 203 insertions(+), 19 deletions(-) diff --git a/MobileGL/MG_Test/Program/ProgramUtilTest.cpp b/MobileGL/MG_Test/Program/ProgramUtilTest.cpp index 34092c20..8c82509e 100644 --- a/MobileGL/MG_Test/Program/ProgramUtilTest.cpp +++ b/MobileGL/MG_Test/Program/ProgramUtilTest.cpp @@ -3946,3 +3946,122 @@ TEST_F(ProgramUtilTest, StorageBlockBindingCeilingIsCheckedAtItsExactBoundary) { // A backend that advertises no binding points has no ceiling to enforce. EXPECT_FALSE(FindShaderStorageBindingViolation("layout(binding = 36) buffer B { int x; };\n", 0).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); +} diff --git a/MobileGL/MG_Util/ShaderTranspiler/ShaderSourceProcessor.cpp b/MobileGL/MG_Util/ShaderTranspiler/ShaderSourceProcessor.cpp index 87fdedd8..1b15eb1e 100644 --- a/MobileGL/MG_Util/ShaderTranspiler/ShaderSourceProcessor.cpp +++ b/MobileGL/MG_Util/ShaderTranspiler/ShaderSourceProcessor.cpp @@ -10,6 +10,7 @@ #include #include +#include #include #include #include @@ -1228,10 +1229,65 @@ namespace MobileGL { return false; } - bool IsDecimalIntegerToken(const String& text) { - if (text.empty()) return false; - return std::all_of(text.begin(), text.end(), - [](char ch) { return ch >= '0' && ch <= '9'; }); + // One GLSL integer literal, spelled the C way: "0x"/"0X" is hexadecimal, a leading + // '0' is OCTAL, everything else decimal, and a single trailing 'u'/'U' is legal. + // strtoll with base 0 already implements exactly that detection, so the only work + // here is deciding what the tail is allowed to be. + // + // Never guesses, which is the discipline every caller depends on: a float ("1.0"), + // an unknown suffix ("3f"), an out-of-range run and a negative value all return + // false, and the caller skips the declaration rather than recording a wrong number. + bool ParseGlslIntegerLiteral(const String& text, long long& out) { + if (text.empty() || text.front() < '0' || text.front() > '9') return false; + errno = 0; + char* tail = nullptr; + const long long value = std::strtoll(text.c_str(), &tail, 0); + if (tail == text.c_str() || errno == ERANGE || value < 0) return false; + const String suffix = text.substr(static_cast(tail - text.c_str())); + if (!suffix.empty() && suffix != "u" && suffix != "U") return false; + out = value; + 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 @@ -1244,6 +1300,7 @@ namespace MobileGL { MobileGL::UnorderedMap& locations) { using MobileGL::Int; long long location = -1; + long long literal = 0; bool sawUniform = false; SizeT declaratorBegin = end; @@ -1259,9 +1316,9 @@ namespace MobileGL { } else if (layoutToken == ")") { --parenDepth; } else if (parenDepth == 1 && layoutToken == "location" && j + 2 < end && - tokens[j + 1].text == "=" && IsDecimalIntegerToken(tokens[j + 2].text)) { - location = std::min(std::strtoll(tokens[j + 2].text.c_str(), nullptr, 10), - static_cast(INT_MAX / 2)); + tokens[j + 1].text == "=" && + ParseGlslIntegerLiteral(tokens[j + 2].text, literal)) { + location = std::min(literal, static_cast(INT_MAX / 2)); j += 2; } ++j; @@ -1290,21 +1347,25 @@ namespace MobileGL { 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 && IsDecimalIntegerToken(tokens[k].text)) { - dimension = std::strtoll(tokens[k].text.c_str(), nullptr, 10); + if (k < end && ParseGlslIntegerLiteral(tokens[k].text, literal)) { + dimension = literal; ++k; } if (k >= end || tokens[k].text != "]") return; // sized by expression; bail out ++k; - span *= std::max(1ll, std::min(dimension, static_cast(INT_MAX / 2))); + 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 @@ -1340,6 +1401,7 @@ namespace MobileGL { MobileGL::UnorderedMap& bindings) { using MobileGL::Int; long long binding = -1; + long long literal = 0; bool sawUniform = false; SizeT declaratorBegin = end; @@ -1355,9 +1417,9 @@ namespace MobileGL { } else if (layoutToken == ")") { --parenDepth; } else if (parenDepth == 1 && layoutToken == "binding" && j + 2 < end && - tokens[j + 1].text == "=" && IsDecimalIntegerToken(tokens[j + 2].text)) { - binding = std::min(std::strtoll(tokens[j + 2].text.c_str(), nullptr, 10), - static_cast(INT_MAX / 2)); + tokens[j + 1].text == "=" && + ParseGlslIntegerLiteral(tokens[j + 2].text, literal)) { + binding = std::min(literal, static_cast(INT_MAX / 2)); j += 2; } ++j; @@ -1390,7 +1452,7 @@ namespace MobileGL { ++k; while (k < end && tokens[k].text == "[") { ++k; - if (k < end && IsDecimalIntegerToken(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; } @@ -1476,8 +1538,10 @@ namespace MobileGL { if (k < count && IsIdentifierToken(tokens[k])) ++k; // instance name if (k >= count || tokens[k].text != "[") return 1; - if (k + 2 < count && IsDecimalIntegerToken(tokens[k + 1].text) && tokens[k + 2].text == "]") { - return std::max(1, std::strtoll(tokens[k + 1].text.c_str(), nullptr, 10)); + long long elementCount = 0; + if (k + 2 < count && ParseGlslIntegerLiteral(tokens[k + 1].text, elementCount) && + tokens[k + 2].text == "]") { + return std::max(1, elementCount); } return -1; // sized by an expression, or unsized } @@ -1498,6 +1562,7 @@ namespace MobileGL { // Several layout(...) lists may precede one declaration and the later one wins, // which is the same accumulate-then-consume shape the extractors above use. long long binding = -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 == "(") { @@ -1510,9 +1575,9 @@ namespace MobileGL { } else if (layoutToken == ")") { --parenDepth; } else if (parenDepth == 1 && layoutToken == "binding" && j + 2 < count && - tokens[j + 1].text == "=" && IsDecimalIntegerToken(tokens[j + 2].text)) { - binding = std::min(std::strtoll(tokens[j + 2].text.c_str(), nullptr, 10), - static_cast(INT_MAX / 2)); + tokens[j + 1].text == "=" && + ParseGlslIntegerLiteral(tokens[j + 2].text, literal)) { + binding = std::min(literal, static_cast(INT_MAX / 2)); j += 2; } ++j;