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): stop blanking block comments in the source handed to glslang

Browse Source 
- BlankBlockComments replaced comment chars with spaces but preserved interior newlines, so a block comment spanning a newline inside a #define truncated the macro body (VALUE became empty)
- glslang has a conformant preprocessor and collapses a block comment to one space across newlines, so the delivered source now keeps comments intact and lets glslang handle them
- Fixes KHR-GL3x.shaders.preprocessor multiline_comment_define / redefine_object_multiline_comment / function_redefinition_3 (6 cases, both devices)
- FilterUnsupportedGpuShaderInt64 relied on the blanking to skip commented-out #extension lines; it now masks comments locally (MaskCommentsAndQuotedText) like the sibling passes, collecting edits and applying them back-to-front
- conditional_inclusion.basic_2 (defined() via macro expansion) stays failing by design: glslang rejects it as UB and working around it would mean re-running preprocessing MobileGL defers to glslang
This commit is contained in:
swung0x48
2026-07-21 05:27:01 -04:00
parent 2b3850b76b
commit 3049c4b82b
2 changed files with 103 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files
MobileGL/MG_Test/Program/ProgramUtilTest.cpp
+71
View File
@@ -238,6 +238,77 @@ void main() {
}
}
// KHR-GL33.shaders.preprocessor.* — a block comment is one preprocessing token that the C/GLSL
// preprocessor replaces with a single space, even when it spans newlines inside a directive. glslang
// handles this natively, so MobileGL must not mangle it. These reproduce the CTS cases that failed
// because comment blanking preserved the interior newline, truncating multi-line #define bodies.
static void ExpectCompiles(MobileGL::ShaderStage stage, GLenum glStage, MobileGL::String source) {
using namespace MG_Util::ShaderTranspiler;
PreprocessShaderSource(stage, source);
ShaderAttrib attrib{.shaderType = glStage, .sourceStr = source};
auto res = ShaderCompiler::CompileShader(attrib);
if (!res) {
FAIL() << "errc: " << res.error().errc << "\nlog: " << res.error().log << "\nsource:\n" << source;
}
}
TEST_F(ProgramUtilTest, PreprocessMultilineCommentInDefineBodyCompiles) {
ExpectCompiles(ShaderStage::Fragment, GL_FRAGMENT_SHADER,
R"(#version 330
precision mediump float;
out float out0;
#define VALUE /* current
value */ 4.2
void main()
{
out0 = VALUE;
})");
}
TEST_F(ProgramUtilTest, PreprocessRedefineObjectMultilineCommentCompiles) {
ExpectCompiles(ShaderStage::Fragment, GL_FRAGMENT_SHADER,
R"(#version 330
precision mediump float;
out float out0;
# define VAL1 1.0
#define VAL2 2.0
#define RES2 /* fdsjklfdsjkl
dsfjkhfdsjkh
fdsjklhfdsjkh */ (RES1 * VAL2)
#define RES1 (VAL2 / VAL1)
#define RES2 /* ewrlkjhsadf */ (RES1 * VAL2)
#define VALUE (RES2 + RES1)
void main()
{
out0 = VALUE;
})");
}
TEST_F(ProgramUtilTest, PreprocessFunctionMacroRedefinitionMultilineCommentCompiles) {
ExpectCompiles(ShaderStage::Fragment, GL_FRAGMENT_SHADER,
R"(#version 330
precision mediump float;
out float out0;
# define FUNC(a,b) (a +b)
# define FUNC(a,b)(a /* comment
*/ +b)
void main()
{
out0 = FUNC(1.0, 2.0);
})");
}
// Note: KHR-GL3x.shaders.preprocessor.conditional_inclusion.basic_2 (`#define AAA defined(BBB)` used
// in `#if !AAA`) is intentionally NOT handled here. Generating the `defined` operator via macro
// expansion is undefined per the C/GLSL preprocessor spec, and glslang deliberately rejects it
// ("'defined' : cannot use in preprocessor expression when expanded from macros"). Making it pass
// would require MobileGL to run its own macro expansion ahead of glslang, which is exactly the
// preprocessing we defer to glslang; the two cases stay failing by design.
TEST_F(ProgramUtilTest, PreprocessLegacyFragmentShaderModernizesGlmarkStyleSource) {
using namespace MG_Util::ShaderTranspiler;
MobileGL/MG_Util/ShaderTranspiler/ShaderSourceProcessor.cpp
+32 -14
View File
@@ -956,15 +956,28 @@ namespace {
return;
}
// Detect the directive on a comment/string-masked copy so a commented-out
// "#extension GL_ARB_gpu_shader_int64" is never turned into a synthesized #error. Comments are
// no longer blanked in the delivered source (glslang handles them), so this pass must mask
// locally like its siblings. Masking preserves offsets, so edits collected against the scan
// apply verbatim to `source`; they are applied back-to-front to keep earlier offsets valid.
const MobileGL::String scan = MaskCommentsAndQuotedText(source);
struct DirectiveEdit {
SizeT pos;
SizeT len;
MobileGL::String replacement;
};
Vector<DirectiveEdit> edits;
SizeT lineStart = 0;
while (lineStart < source.size()) {
SizeT lineEnd = source.find('\n', lineStart);
while (lineStart < scan.size()) {
SizeT lineEnd = scan.find('\n', lineStart);
const bool hasLineBreak = lineEnd != MobileGL::String::npos;
if (!hasLineBreak) {
lineEnd = source.size();
lineEnd = scan.size();
}
const MobileGL::String line = source.substr(lineStart, lineEnd - lineStart);
const MobileGL::String line = scan.substr(lineStart, lineEnd - lineStart);
SizeT probe = 0;
while (probe < line.size() && std::isspace(static_cast<unsigned char>(line[probe]))) {
probe++;
@@ -1006,16 +1019,12 @@ namespace {
const MobileGL::String behavior = TrimDirectiveToken(line.substr(probe));
const SizeT replaceLen = lineEnd - lineStart + (hasLineBreak ? 1 : 0);
if (behavior == "require") {
const MobileGL::String replacement =
"#error GL_ARB_gpu_shader_int64 is not advertised by MobileGL\n";
source.replace(lineStart, replaceLen, replacement);
lineStart += replacement.size();
edits.push_back({lineStart, replaceLen,
"#error GL_ARB_gpu_shader_int64 is not advertised by MobileGL\n"});
} else if (behavior == "enable" || behavior == "warn") {
source.replace(lineStart, replaceLen, "\n");
lineStart++;
} else {
lineStart = lineEnd + (hasLineBreak ? 1 : 0);
edits.push_back({lineStart, replaceLen, "\n"});
}
lineStart = lineEnd + (hasLineBreak ? 1 : 0);
continue;
}
}
@@ -1025,6 +1034,10 @@ namespace {
lineStart = lineEnd + (hasLineBreak ? 1 : 0);
}
for (auto it = edits.rbegin(); it != edits.rend(); ++it) {
source.replace(it->pos, it->len, it->replacement);
}
ReplaceIdentifier(source, "GL_ARB_gpu_shader_int64", "MG_DISABLED_GL_ARB_gpu_shader_int64");
}
@@ -1247,8 +1260,13 @@ namespace MobileGL {
const ShaderLanguageInfo originalLanguage = InspectShaderLanguage(source);
NormalizeVersionDirective(source, originalLanguage);
BlankBlockComments(source);
// Comments are left intact for glslang's own preprocessor: a block comment is a single
// preprocessing token that collapses to one space even across newlines and inside a
// directive, so blanking it here (which preserved the interior newlines) truncated
// multi-line #define bodies and broke otherwise-valid shaders (KHR-GL3x.shaders.
// preprocessor multiline_comment_define / redefine_object / function_redefinition).
// Every MobileGL pass that must ignore comment/string text already masks them locally
// via MaskCommentsAndQuotedText/TokenizeCode, so the source we hand glslang keeps them.
NormalizeLineDirectives(source);
// noperspective is intentionally NOT touched here. It is core in desktop GLSL (1.30+)