swung0x48/MobileGL
1
0
Fork 0
mirror of https://github.com/MobileGL-Dev/MobileGL synced 2026-09-09 04:38:30 +09:00
Code Issues Packages Projects Releases Wiki Activity

[Fix] (ShaderTranspiler): fan #extension implications out to both new gates and bound the mid-line #version probe to its line

Browse Source
This commit is contained in:
swung0x48
2026-08-27 03:35:58 -04:00
parent d52a3b2196
commit 07669aacd4
2 changed files with 248 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files
MobileGL/MG_Test/Program/ProgramUtilTest.cpp
+140 -1
View File
@@ -4689,19 +4689,43 @@ TEST_F(ProgramUtilTest, NumSamplesShimHonoursTheVersionAndExtensionGate) {
Bool expectShim;
};
// Mirrors glslang's own gate (Initialize.cpp): desktop from 4.00, or from 1.30 with
// ARB_sample_shading; ESSL from 3.20, or from 3.10 with OES_sample_variables.
// ARB_sample_shading; ESSL from 3.20, or from 3.10 with OES_sample_variables - which
// GL_ANDROID_extension_pack_es31a and `#extension all : warn` also turn on
// (TParseVersions::updateExtensionBehavior).
const Case cases[] = {
{"desktop 460 core", "#version 460 core\n", true},
{"desktop 400 core", "#version 400 core\n", true},
{"desktop 330 core, no extension", "#version 330 core\n", false},
{"desktop 330 core + ARB_sample_shading",
"#version 330 core\n#extension GL_ARB_sample_shading : require\n", true},
{"desktop 330 core + all : warn",
"#version 330 core\n#extension all : warn\n", true},
{"desktop 120, no extension", "#version 120\n", false},
{"desktop 120 + all : warn (below the 1.30 floor)",
"#version 120\n#extension all : warn\n", false},
{"ESSL 320", "#version 320 es\n", true},
{"ESSL 310, no extension", "#version 310 es\n", false},
{"ESSL 310 + OES_sample_variables",
"#version 310 es\n#extension GL_OES_sample_variables : require\n", true},
// The AEP spellings. glslang applies the directive's behavior to all twelve AEP members,
// GL_OES_sample_variables among them, so these are legal ES 3.1 shaders.
{"ESSL 310 + AEP : require",
"#version 310 es\n#extension GL_ANDROID_extension_pack_es31a : require\n", true},
{"ESSL 310 + AEP : enable",
"#version 310 es\n#extension GL_ANDROID_extension_pack_es31a : enable\n", true},
{"ESSL 310 + AEP : warn",
"#version 310 es\n#extension GL_ANDROID_extension_pack_es31a : warn\n", true},
// ...but `disable` is not an opt-in, and the implication carries the behavior with it.
{"ESSL 310 + AEP : disable",
"#version 310 es\n#extension GL_ANDROID_extension_pack_es31a : disable\n", false},
{"ESSL 310 + all : warn",
"#version 310 es\n#extension all : warn\n", true},
// An AEP member that does NOT imply sample variables must not open the gate.
{"ESSL 310 + EXT_geometry_shader only",
"#version 310 es\n#extension GL_EXT_geometry_shader : require\n", false},
{"ESSL 300", "#version 300 es\n", false},
{"ESSL 300 + AEP (below the 3.10 floor)",
"#version 300 es\n#extension GL_ANDROID_extension_pack_es31a : require\n", false},
};
for (const Case& testCase : cases) {
@@ -4873,3 +4897,118 @@ TEST_F(ProgramUtilTest, OnlyAnExactVersionRepeatIsElided) {
EXPECT_NE(source.find("#version 330 foobar"), String::npos) << source;
}
}
// glslang applies an #extension directive's behavior to every extension the named one IMPLIES
// (TParseVersions::updateExtensionBehavior, Versions.cpp:1039-1064). Both consumers of the
// extension sets have to see that expansion or the AEP spelling of a shader behaves differently
// from the byte-equivalent one that names its members directly.
TEST_F(ProgramUtilTest, AepFansOutToItsMemberExtensionMacros) {
using namespace MG_Util::ShaderTranspiler;
// The CTS-shaped guard, opted in the AEP way. Before the fan-out this took the broken arm.
String source = R"(#version 310 es
#extension GL_ANDROID_extension_pack_es31a : require
precision highp float;
out vec4 fragColor;
#if !GL_OES_sample_variables
this is broken
#endif
#if !GL_OES_shader_multisample_interpolation
this is also broken
#endif
void main() { fragColor = vec4(float(gl_NumSamples)); }
)";
PreprocessShaderSource(ShaderStage::Fragment, source);
// Both halves of the AEP path: the built-in shim AND the restored member macros.
EXPECT_TRUE(HasNumSamplesShim(source)) << source;
const String defines = CollectEsPreambleMacroDefines(source);
for (const char* member : {"GL_ANDROID_extension_pack_es31a", "GL_OES_sample_variables",
"GL_OES_shader_image_atomic", "GL_OES_shader_multisample_interpolation",
"GL_OES_texture_storage_multisample_2d_array", "GL_EXT_geometry_shader",
"GL_EXT_gpu_shader5", "GL_EXT_primitive_bounding_box",
"GL_EXT_shader_io_blocks", "GL_EXT_tessellation_shader",
"GL_EXT_texture_buffer", "GL_EXT_texture_cube_map_array"}) {
EXPECT_NE(defines.find(String("#define ") + member + " 1\n"), String::npos)
<< member << " missing from:\n" << defines;
}
// GL_KHR_blend_equation_advanced is an AEP member glslang propagates to, but its macro is in
// the DESKTOP preamble too - so the rewrite never took it away and it must not be restored.
EXPECT_EQ(defines.find("GL_KHR_blend_equation_advanced"), String::npos) << defines;
ExpectShaderCompiles(GL_FRAGMENT_SHADER, source);
}
TEST_F(ProgramUtilTest, ExtensionImplicationIsTransitiveAndStaysNamed) {
using namespace MG_Util::ShaderTranspiler;
{
SCOPED_TRACE("geometry/tessellation imply the matching io_blocks");
// glslang re-enters updateExtensionBehavior for each implication, so the graph is walked
// to a fixed point rather than one level deep.
String source = R"(#version 310 es
#extension GL_OES_geometry_shader : require
out vec4 fragColor;
void main() { fragColor = vec4(1.0); }
)";
PreprocessShaderSource(ShaderStage::Fragment, source);
const String defines = CollectEsPreambleMacroDefines(source);
EXPECT_NE(defines.find("#define GL_OES_geometry_shader 1\n"), String::npos) << defines;
EXPECT_NE(defines.find("#define GL_OES_shader_io_blocks 1\n"), String::npos) << defines;
// The EXT spelling is a different extension and must not come along.
EXPECT_EQ(defines.find("GL_EXT_shader_io_blocks"), String::npos) << defines;
}
{
SCOPED_TRACE("a source that names nothing implied still gets nothing");
String source = R"(#version 310 es
#extension GL_OES_sample_variables : enable
out vec4 fragColor;
void main() { fragColor = vec4(1.0); }
)";
PreprocessShaderSource(ShaderStage::Fragment, source);
const String defines = CollectEsPreambleMacroDefines(source);
EXPECT_EQ(defines, String("#define GL_OES_sample_variables 1\n")) << defines;
}
{
SCOPED_TRACE("`all` opens the built-in gate but does not define every ES macro");
// The two questions differ: `all : warn` really does turn every extension on in glslang,
// but the preamble macros are defined before any #extension line runs, so `all` says
// nothing about which ones the ES -> desktop rewrite took away.
String source = R"(#version 310 es
#extension all : warn
out vec4 fragColor;
void main() { fragColor = vec4(float(gl_NumSamples)); }
)";
PreprocessShaderSource(ShaderStage::Fragment, source);
EXPECT_TRUE(HasNumSamplesShim(source)) << source;
EXPECT_EQ(source.find("mobilegl-es-preamble"), String::npos) << source;
EXPECT_TRUE(CollectEsPreambleMacroDefines(source).empty());
}
}
// The mid-line #version probe must search THE LINE, not the rest of the file: an unbounded
// std::string::find makes InspectShaderLanguage quadratic on the ordinary resolved-shader-pack
// shape (one leading #version, no further '#' anywhere). This pins both halves - the detection
// still fires, and it fires on a source whose only other content is a long directive-free body.
TEST_F(ProgramUtilTest, MidLineVersionDetectionSurvivesALongDirectiveFreeBody) {
using namespace MG_Util::ShaderTranspiler;
String body;
body.reserve(64 * 1024);
for (int line = 0; line < 2000; ++line) {
body += " float v" + std::to_string(line) + " = 0.0;\n";
}
// The CTS concatenation shape, followed by a body with no '#' in it at all.
String source = "#version 310 es\nprecision highp float;#version 310 es\nout vec4 fragColor;\nvoid main() {\n" +
body + " fragColor = vec4(1.0);\n}\n";
const SizeT lineCountBefore = static_cast<SizeT>(std::count(source.begin(), source.end(), '\n'));
PreprocessShaderSource(ShaderStage::Fragment, source);
EXPECT_EQ(source.find("#version", source.find("#version") + 1), String::npos) << source.substr(0, 200);
EXPECT_EQ(static_cast<SizeT>(std::count(source.begin(), source.end(), '\n')), lineCountBefore);
ExpectShaderCompiles(GL_FRAGMENT_SHADER, source);
}
MobileGL/MG_Util/ShaderTranspiler/ShaderSourceProcessor.cpp
+108 -15
View File
@@ -13,6 +13,7 @@
#include <cerrno>
#include <climits>
#include <cstdlib>
#include <cstring>
#include <initializer_list>
#include <utility>
#include <Config.h>
@@ -255,6 +256,50 @@ namespace {
bool isValid = false;
};
// glslang's #extension implication graph, transcribed from
// TParseVersions::updateExtensionBehavior (Versions.cpp:1039-1064). Naming one of these
// extensions applies the SAME behavior to every name it implies, so a source that says
// `#extension GL_ANDROID_extension_pack_es31a : require` has really required all twelve AEP
// members - and glslang's ES gl_NumSamples gate reads GL_OES_sample_variables, one of them.
//
// Transcribed rather than approximated: the AEP membership list is glslang's, and a guess that
// drifts from it would make MobileGL accept or reject a shader glslang does not.
// GL_KHR_blend_equation_advanced is in the list for completeness even though it has no ES
// preamble macro - IsEsOnlyPreambleExtensionMacro filters it out on its own.
const Vector<std::pair<const char*, Vector<const char*>>>& GetExtensionImplications() {
static const Vector<std::pair<const char*, Vector<const char*>>> kImplications = {
{"GL_ANDROID_extension_pack_es31a",
{"GL_KHR_blend_equation_advanced", "GL_OES_sample_variables", "GL_OES_shader_image_atomic",
"GL_OES_shader_multisample_interpolation", "GL_OES_texture_storage_multisample_2d_array",
"GL_EXT_geometry_shader", "GL_EXT_gpu_shader5", "GL_EXT_primitive_bounding_box",
"GL_EXT_shader_io_blocks", "GL_EXT_tessellation_shader", "GL_EXT_texture_buffer",
"GL_EXT_texture_cube_map_array"}},
// geometry / tessellation to io_blocks
{"GL_EXT_geometry_shader", {"GL_EXT_shader_io_blocks"}},
{"GL_OES_geometry_shader", {"GL_OES_shader_io_blocks"}},
{"GL_EXT_tessellation_shader", {"GL_EXT_shader_io_blocks"}},
{"GL_OES_tessellation_shader", {"GL_OES_shader_io_blocks"}},
};
return kImplications;
}
// Closes `extensions` under the graph above. glslang propagates by RE-ENTERING
// updateExtensionBehavior, so the propagation is transitive (AEP -> GL_EXT_geometry_shader ->
// GL_EXT_shader_io_blocks); the fixed-point loop below is that re-entry.
void AddImpliedExtensions(std::set<MobileGL::String>& extensions) {
if (extensions.empty()) return;
bool grew = true;
while (grew) {
grew = false;
for (const auto& [source, implied] : GetExtensionImplications()) {
if (extensions.count(source) == 0) continue;
for (const char* name : implied) {
grew |= extensions.insert(name).second;
}
}
}
}
// Reads "<digits> [profile]" out of a "#version" directive whose keyword ends at `probe`, and
// decides whether it is one MobileGL is willing to rewrite. `code` must be the masked source,
// so a trailing comment has already become blanks.
@@ -337,12 +382,21 @@ namespace {
// BlankRedundantVersionDirectives for why that one is tolerated and nothing else.
//
// Gated on a directive having been accepted already, so an ordinary shader - which
// has none of these - pays one memchr per line past its #version line and nothing
// before it.
if (info.hasValidVersionDirective) {
const SizeT hashPos = code.find('#', probe);
if (hashPos != MobileGL::String::npos && hashPos < lineEnd) {
recordIfRedundant(hashPos, lineEnd);
// has none of these - pays nothing at all before its #version line.
//
// BOUNDED TO THE LINE, and that is not a detail. std::string::find(char, pos) has
// no end bound, so a `code.find('#', probe)` filtered afterwards by
// `hashPos < lineEnd` scans from this line to the END OF THE SOURCE whenever no
// '#' follows - which is the ordinary shape of a resolved shader-pack source (one
// leading #version, nothing after it), and it makes this whole sweep quadratic in
// shader size. A 131 KB glsl-transformer output in .trace-work has exactly one '#'
// in the file. Searching the line span is behaviour-identical: every hashPos the
// unbounded form could accept already had to satisfy hashPos < lineEnd.
if (info.hasValidVersionDirective && probe < lineEnd) {
const void* hash = std::memchr(code.data() + probe, '#', lineEnd - probe);
if (hash != nullptr) {
recordIfRedundant(static_cast<SizeT>(static_cast<const char*>(hash) - code.data()),
lineEnd);
}
}
} else {
@@ -390,6 +444,19 @@ namespace {
lineStart = lineEnd + (hasLineBreak ? 1 : 0);
}
// Both extension sets are closed under glslang's implication graph BEFORE anyone reads
// them, so every consumer sees the same expansion and none of them can forget it. Applied
// here rather than at the directive because an implication may be named before its source
// (`#extension GL_EXT_shader_io_blocks : disable` then `... AEP : require`), and the
// fixed point of the whole set is what glslang's re-entrant propagation ends up at.
//
// enablesGpuShader5 is deliberately NOT recomputed from the expanded set: it gates the
// 460 version escalation on the DESKTOP ARB/NV spellings, and AEP implies the ESSL
// GL_EXT_gpu_shader5, a different extension. An ES source is rewritten to 460 core
// anyway, so there is nothing for the escalation to do there.
AddImpliedExtensions(info.namedExtensions);
AddImpliedExtensions(info.enabledExtensions);
return info;
}
@@ -1653,21 +1720,47 @@ namespace {
if (!info.hasValidVersionDirective) return;
if (info.namedExtensions.empty()) return;
MobileGL::String macros;
for (const MobileGL::String& extension : info.namedExtensions) {
if (!IsEsOnlyPreambleExtensionMacro(extension, info.version)) continue;
macros += extension;
}
// namedExtensions is already closed under glslang's implication graph, so a source that
// names only GL_ANDROID_extension_pack_es31a marks its twelve members too - glslang's ES
// preamble defines all of them, and the CTS-shaped "#if !GL_OES_sample_variables" guard
// reads one of them.
//
// `#extension all : warn` is deliberately NOT honoured here, unlike in the built-in gate.
// The two answer different questions: the gate asks "would glslang have this extension
// turned on", where `all` genuinely says yes, while this asks "which preamble macros did
// the ES -> desktop rewrite take away". glslang's preamble runs BEFORE any #extension line
// and defines the ES macros regardless of behavior, so `all` adds no information - and
// emitting all thirty-five for a source that named nothing is exactly the broad rewrite
// the named-extensions-only policy exists to avoid.
const bool hasMacroToRestore =
std::any_of(info.namedExtensions.begin(), info.namedExtensions.end(),
[&info](const MobileGL::String& extension) {
return IsEsOnlyPreambleExtensionMacro(extension, info.version);
});
// Nothing the desktop preamble is missing: leave the source byte-identical.
if (macros.empty()) return;
if (!hasMacroToRestore) return;
source.insert(afterVersion.Get(source),
MobileGL::String(kEsPreambleMarkerPrefix) + std::to_string(info.version) + "*/\n");
}
// "Would glslang have this extension turned on?", mirroring TParseVersions::extensionTurnedOn.
//
// Two spellings besides the name itself reach it. The implication graph is already folded into
// enabledExtensions (AddImpliedExtensions), so only `#extension all : <behavior>` is left:
// glslang applies that behavior to EVERY registered extension at once, and rejects `all` with
// require/enable outright (Versions.cpp:1136-1141) - so the only spellings that survive are
// `all : warn`, which turns everything ON (behavior != EBhDisable), and `all : disable`.
// InspectShaderLanguage only records a name in enabledExtensions for enable/require/warn, so
// the literal "all" appearing here means `all : warn` and nothing else.
bool ExtensionTurnedOn(const ShaderLanguageInfo& info, const char* extension) {
return info.enabledExtensions.count(extension) != 0 || info.enabledExtensions.count("all") != 0;
}
// gl_NumSamples is legal in this source only where glslang would have declared it with a
// non-SPIR-V target (Initialize.cpp): desktop from 4.00 core, or from 1.30 with
// ARB_sample_shading; ESSL from 3.20, or from 3.10 with OES_sample_variables.
// ARB_sample_shading; ESSL from 3.20, or from 3.10 with OES_sample_variables - the last of
// which GL_ANDROID_extension_pack_es31a also turns on, via the implication graph.
//
// The gate matters because the shim ends in "#define gl_NumSamples mg_NumSamples", and a
// #define is not scoped by anything: defining it for a source where the built-in does not
@@ -1676,10 +1769,10 @@ namespace {
if (!info.HasVersionDirective() || !info.hasValidVersionDirective) return false;
if (info.profile == MobileGL::ShaderProfile::ES) {
if (info.version >= 320) return true;
return info.version >= 310 && info.enabledExtensions.count("GL_OES_sample_variables") != 0;
return info.version >= 310 && ExtensionTurnedOn(info, "GL_OES_sample_variables");
}
if (info.version >= 400) return true;
return info.version >= 130 && info.enabledExtensions.count("GL_ARB_sample_shading") != 0;
return info.version >= 130 && ExtensionTurnedOn(info, "GL_ARB_sample_shading");
}
// gl_NumSamples has no SPIR-V built-in to lower to, so glslang declares it only when it is NOT