mirror of
https://github.com/MobileGL-Dev/MobileGL
synced 2026-09-12 06:08:30 +09:00
[Fix] (MG_Util/ShaderTranspiler): blank block comments lexically instead of erasing them - a '//*** banner ***' line opened a comment the old scanner never closed, so it deleted the rest of the shader, and a commented-out builtin definition renamed every genuine call to a name nothing defines
This commit is contained in:
@@ -455,6 +455,77 @@ void main() {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// A banner line like "//*** NOTE ***" contains "/*" at offset 1 and no "*/" anywhere after it. The
|
||||||
|
// old hand-rolled comment stripper searched for "/*" with no lexical state, found that, failed to
|
||||||
|
// find a terminator, and erased everything from there to the end of the file - deleting the entire
|
||||||
|
// shader. Banner comments in that exact shape are common in Iris and OptiFine packs.
|
||||||
|
TEST_F(ProgramUtilTest, PreprocessKeepsShaderBodyAfterAStarredLineComment) {
|
||||||
|
using namespace MG_Util::ShaderTranspiler;
|
||||||
|
|
||||||
|
String source = R"(#version 330 core
|
||||||
|
//*** lighting pass ***
|
||||||
|
out vec4 fragColor;
|
||||||
|
void main() {
|
||||||
|
fragColor = vec4(1.0);
|
||||||
|
}
|
||||||
|
)";
|
||||||
|
PreprocessShaderSource(ShaderStage::Fragment, source);
|
||||||
|
|
||||||
|
EXPECT_NE(source.find("void main()"), String::npos) << "shader body was truncated:\n" << source;
|
||||||
|
EXPECT_NE(source.find("fragColor = vec4(1.0);"), String::npos);
|
||||||
|
|
||||||
|
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;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// The builtin-shadowing rename only fires when the shader really defines its own round/tanh/etc.
|
||||||
|
// Deciding that from a commented-out definition renames every genuine call to the builtin to a
|
||||||
|
// mg_ name that nothing defines, which fails to link.
|
||||||
|
TEST_F(ProgramUtilTest, PreprocessIgnoresCommentedOutBuiltinShadowingDefinition) {
|
||||||
|
using namespace MG_Util::ShaderTranspiler;
|
||||||
|
|
||||||
|
String source = R"(#version 330 core
|
||||||
|
// float round(float x) { return floor(x + 0.5); }
|
||||||
|
out vec4 fragColor;
|
||||||
|
void main() {
|
||||||
|
fragColor = vec4(round(1.25));
|
||||||
|
}
|
||||||
|
)";
|
||||||
|
PreprocessShaderSource(ShaderStage::Fragment, source);
|
||||||
|
|
||||||
|
EXPECT_NE(source.find("round(1.25)"), String::npos) << "call was renamed from a comment:\n" << source;
|
||||||
|
EXPECT_EQ(source.find("mg_round"), String::npos);
|
||||||
|
}
|
||||||
|
|
||||||
|
// A block-commented extension directive must not be treated as a real one - the int64 filter turns
|
||||||
|
// unsupported directives into #error, so reading one out of a comment manufactures a compile
|
||||||
|
// failure for a shader that never asked for the extension.
|
||||||
|
TEST_F(ProgramUtilTest, PreprocessIgnoresBlockCommentedExtensionDirectives) {
|
||||||
|
using namespace MG_Util::ShaderTranspiler;
|
||||||
|
|
||||||
|
String source = R"(#version 330 core
|
||||||
|
/*
|
||||||
|
#extension GL_ARB_gpu_shader_int64 : require
|
||||||
|
*/
|
||||||
|
out vec4 fragColor;
|
||||||
|
void main() {
|
||||||
|
fragColor = vec4(1.0);
|
||||||
|
}
|
||||||
|
)";
|
||||||
|
PreprocessShaderSource(ShaderStage::Fragment, source);
|
||||||
|
|
||||||
|
EXPECT_EQ(source.find("#error"), String::npos) << "#error synthesized from a comment:\n" << source;
|
||||||
|
|
||||||
|
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;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
TEST_F(ProgramUtilTest, PreprocessModernSampleQualifierStaysAtVersion460) {
|
TEST_F(ProgramUtilTest, PreprocessModernSampleQualifierStaysAtVersion460) {
|
||||||
using namespace MG_Util::ShaderTranspiler;
|
using namespace MG_Util::ShaderTranspiler;
|
||||||
|
|
||||||
|
|||||||
@@ -96,6 +96,77 @@ namespace {
|
|||||||
return masked;
|
return masked;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Blank out block comments in place, leaving line comments and every other byte where it is.
|
||||||
|
//
|
||||||
|
// The passes that follow scan the source as raw text, so block comments have to stop being
|
||||||
|
// visible to them - but they must not be *deleted*: replacing the bytes with spaces keeps every
|
||||||
|
// later offset valid and keeps newlines, so glslang's diagnostics still point at the line the
|
||||||
|
// application wrote. It also has to be lexically aware. A banner line such as
|
||||||
|
//
|
||||||
|
// //*** lighting pass ***
|
||||||
|
//
|
||||||
|
// contains "/*" one byte in, and a naive search for that opener treats the rest of the file as
|
||||||
|
// an unterminated comment.
|
||||||
|
void BlankBlockComments(MobileGL::String& source) {
|
||||||
|
enum class Region { Code, SingleLineComment, MultiLineComment, QuotedText };
|
||||||
|
|
||||||
|
Region region = Region::Code;
|
||||||
|
char quote = '\0';
|
||||||
|
bool escaped = false;
|
||||||
|
|
||||||
|
for (SizeT pos = 0; pos < source.size(); pos++) {
|
||||||
|
const char ch = source[pos];
|
||||||
|
const char next = pos + 1 < source.size() ? source[pos + 1] : '\0';
|
||||||
|
|
||||||
|
if (region == Region::Code) {
|
||||||
|
if (ch == '/' && next == '/') {
|
||||||
|
pos++;
|
||||||
|
region = Region::SingleLineComment;
|
||||||
|
} else if (ch == '/' && next == '*') {
|
||||||
|
source[pos] = ' ';
|
||||||
|
source[pos + 1] = ' ';
|
||||||
|
pos++;
|
||||||
|
region = Region::MultiLineComment;
|
||||||
|
} else if (ch == '"' || ch == '\'') {
|
||||||
|
quote = ch;
|
||||||
|
escaped = false;
|
||||||
|
region = Region::QuotedText;
|
||||||
|
}
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
if (region == Region::SingleLineComment) {
|
||||||
|
if (ch == '\n' || ch == '\r') region = Region::Code;
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
if (region == Region::MultiLineComment) {
|
||||||
|
if (ch == '*' && next == '/') {
|
||||||
|
source[pos] = ' ';
|
||||||
|
source[pos + 1] = ' ';
|
||||||
|
pos++;
|
||||||
|
region = Region::Code;
|
||||||
|
} else if (ch != '\n' && ch != '\r') {
|
||||||
|
source[pos] = ' ';
|
||||||
|
}
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
// GLSL has no multi-line string literals, so a quote that reaches end of line was never
|
||||||
|
// a literal to begin with - most likely an apostrophe in a #error or #pragma message.
|
||||||
|
// Ending the region here keeps one stray apostrophe from swallowing the rest of the file.
|
||||||
|
if (ch == '\n' || ch == '\r') {
|
||||||
|
region = Region::Code;
|
||||||
|
} else if (escaped) {
|
||||||
|
escaped = false;
|
||||||
|
} else if (ch == '\\') {
|
||||||
|
escaped = true;
|
||||||
|
} else if (ch == quote) {
|
||||||
|
region = Region::Code;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
struct CodeToken {
|
struct CodeToken {
|
||||||
String text;
|
String text;
|
||||||
SizeT begin = 0;
|
SizeT begin = 0;
|
||||||
@@ -686,7 +757,10 @@ namespace {
|
|||||||
|
|
||||||
void RenameBuiltinShadowingFunction(MobileGL::String& source, const char* from, const char* to) {
|
void RenameBuiltinShadowingFunction(MobileGL::String& source, const char* from, const char* to) {
|
||||||
const MobileGL::String fromName = from;
|
const MobileGL::String fromName = from;
|
||||||
if (!HasSingleLineFunctionDefinition(source, fromName)) {
|
// Decide from a comment-free view. A commented-out definition is not a definition, and
|
||||||
|
// acting on one renames every genuine call to the builtin to a name nothing defines - which
|
||||||
|
// then fails to resolve. Line comments survive BlankBlockComments, so this matters.
|
||||||
|
if (!HasSingleLineFunctionDefinition(MaskCommentsAndQuotedText(source), fromName)) {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -1078,18 +1152,7 @@ namespace MobileGL {
|
|||||||
const ShaderLanguageInfo originalLanguage = InspectShaderLanguage(source);
|
const ShaderLanguageInfo originalLanguage = InspectShaderLanguage(source);
|
||||||
NormalizeVersionDirective(source, originalLanguage);
|
NormalizeVersionDirective(source, originalLanguage);
|
||||||
|
|
||||||
// remove multi-line comment
|
BlankBlockComments(source);
|
||||||
size_t commentStartPos = source.find("/*");
|
|
||||||
while (commentStartPos != String::npos) {
|
|
||||||
size_t commentEndPos = source.find("*/", commentStartPos);
|
|
||||||
if (commentEndPos == String::npos) {
|
|
||||||
source.erase(commentStartPos);
|
|
||||||
break;
|
|
||||||
}
|
|
||||||
// + length of "*/"
|
|
||||||
source = source.replace(commentStartPos, commentEndPos - commentStartPos + 2, "");
|
|
||||||
commentStartPos = source.find("/*", commentStartPos);
|
|
||||||
}
|
|
||||||
|
|
||||||
// remove #line directives
|
// remove #line directives
|
||||||
SizeT linedirPos = source.find("#line");
|
SizeT linedirPos = source.find("#line");
|
||||||
|
|||||||
Reference in New Issue
Block a user