Skip to content

Commit f13e3cd

Browse files
mbaetgen-wupmbaetgen-wup
committed
Fix: MilkdropShader - ignore code comments (#958)
1 parent 3649044 commit f13e3cd

4 files changed

Lines changed: 256 additions & 19 deletions

File tree

‎src/libprojectM/MilkdropPreset/MilkdropShader.cpp‎

Lines changed: 71 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -492,6 +492,63 @@ void PS(float4 _vDiffuse : COLOR,
492492
program = fullSource;
493493
}
494494

495+
namespace {
496+
/**
497+
* @brief Strips C/C++ style comments from shader source code.
498+
*
499+
* Replaces // line comments and block comments with spaces (preserving
500+
* string length and newline positions) so that subsequent text searches
501+
* do not match identifiers inside comments.
502+
*/
503+
auto StripComments(const std::string& source) -> std::string
504+
{
505+
std::string result = source;
506+
size_t i = 0;
507+
508+
while (i < result.size())
509+
{
510+
if (i + 1 < result.size() && result.at(i) == '/' && result.at(i + 1) == '/')
511+
{
512+
// Line comment: replace until end of line
513+
while (i < result.size() && result.at(i) != '\n' && result.at(i) != '\r')
514+
{
515+
result.at(i) = ' ';
516+
i++;
517+
}
518+
}
519+
else if (i + 1 < result.size() && result.at(i) == '/' && result.at(i + 1) == '*')
520+
{
521+
// Block comment: replace until closing */
522+
result.at(i) = ' ';
523+
result.at(i + 1) = ' ';
524+
i += 2;
525+
while (i < result.size())
526+
{
527+
if (i + 1 < result.size() && result.at(i) == '*' && result.at(i + 1) == '/')
528+
{
529+
result.at(i) = ' ';
530+
result.at(i + 1) = ' ';
531+
i += 2;
532+
break;
533+
}
534+
// Preserve newlines to keep line structure intact
535+
if (result.at(i) != '\n' && result.at(i) != '\r')
536+
{
537+
result.at(i) = ' ';
538+
}
539+
i++;
540+
}
541+
}
542+
else
543+
{
544+
i++;
545+
}
546+
}
547+
548+
return result;
549+
}
550+
} // namespace
551+
495552
void MilkdropShader::GetReferencedSamplers(const std::string& program)
496553
{
497554
// Look up samplers referenced in the shader program
@@ -500,40 +557,43 @@ void MilkdropShader::GetReferencedSamplers(const std::string& program)
500557
// "main" should always be present.
501558
m_samplerNames.insert("main");
502559

560+
// Strip comments so that commented-out sampler/texsize declarations are not matched.
561+
std::string const stripped = StripComments(program);
562+
503563
// Search for sampler usage
504-
auto found = program.find("sampler_", 0);
564+
auto found = stripped.find("sampler_", 0);
505565
while (found != std::string::npos)
506566
{
507567
found += 8;
508-
size_t const end = program.find_first_of(" ;,\n\r)", found);
568+
size_t const end = stripped.find_first_of(" ;,\n\r)", found);
509569

510570
if (end != std::string::npos)
511571
{
512-
std::string const sampler = program.substr(static_cast<int>(found), static_cast<int>(end - found));
572+
std::string const sampler = stripped.substr(static_cast<int>(found), static_cast<int>(end - found));
513573
// Skip "sampler_state", as it's a reserved word and not a sampler.
514574
if (sampler != "state")
515575
{
516576
m_samplerNames.insert(sampler);
517577
}
518578
}
519579

520-
found = program.find("sampler_", found);
580+
found = stripped.find("sampler_", found);
521581
}
522582

523583
// Also search for texsize usage, some presets don't reference the sampler.
524-
found = program.find("texsize_", 0);
584+
found = stripped.find("texsize_", 0);
525585
while (found != std::string::npos)
526586
{
527587
found += 8;
528-
size_t const end = program.find_first_of(" ;,.\n\r)", found);
588+
size_t const end = stripped.find_first_of(" ;,.\n\r)", found);
529589

530590
if (end != std::string::npos)
531591
{
532-
std::string const sampler = program.substr(static_cast<int>(found), static_cast<int>(end - found));
592+
std::string const sampler = stripped.substr(static_cast<int>(found), static_cast<int>(end - found));
533593
m_samplerNames.insert(sampler);
534594
}
535595

536-
found = program.find("texsize_", found);
596+
found = stripped.find("texsize_", found);
537597
}
538598

539599
{
@@ -563,15 +623,15 @@ void MilkdropShader::GetReferencedSamplers(const std::string& program)
563623
}
564624
}
565625

566-
if (program.find("GetBlur3") != std::string::npos)
626+
if (stripped.find("GetBlur3") != std::string::npos)
567627
{
568628
UpdateMaxBlurLevel(BlurTexture::BlurLevel::Blur3);
569629
}
570-
else if (program.find("GetBlur2") != std::string::npos)
630+
else if (stripped.find("GetBlur2") != std::string::npos)
571631
{
572632
UpdateMaxBlurLevel(BlurTexture::BlurLevel::Blur2);
573633
}
574-
else if (program.find("GetBlur1") != std::string::npos)
634+
else if (stripped.find("GetBlur1") != std::string::npos)
575635
{
576636
UpdateMaxBlurLevel(BlurTexture::BlurLevel::Blur1);
577637
}

‎src/libprojectM/MilkdropPreset/MilkdropShader.hpp‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -67,18 +67,21 @@ class MilkdropShader
6767
*/
6868
auto Shader() -> Renderer::Shader&;
6969

70-
private:
70+
protected:
7171
/**
72-
* @brief Prepares the shader code to be translated into GLSL.
72+
* @brief Searches for sampler references in the program and stores them in m_samplerNames.
7373
* @param program The program code to work on.
7474
*/
75-
void PreprocessPresetShader(std::string& program);
75+
void GetReferencedSamplers(const std::string& program);
76+
77+
std::set<std::string> m_samplerNames; //!< All sampler names referenced in the shader code.
7678

79+
private:
7780
/**
78-
* @brief Searches for sampler references in the program and stores them in m_samplerNames.
81+
* @brief Prepares the shader code to be translated into GLSL.
7982
* @param program The program code to work on.
8083
*/
81-
void GetReferencedSamplers(const std::string& program);
84+
void PreprocessPresetShader(std::string& program);
8285

8386
/**
8487
* @brief Translates the HLSL shader into GLSL.
@@ -98,9 +101,8 @@ class MilkdropShader
98101
std::string m_fragmentShaderCode; //!< The original preset fragment shader code.
99102
std::string m_preprocessedCode; //!< The preprocessed preset shader code.
100103

101-
std::set<std::string> m_samplerNames; //!< All sampler names referenced in the shader code.
102-
std::vector<Renderer::TextureSamplerDescriptor> m_mainTextureDescriptors; //!< Descriptors for all main texture references.
103-
std::vector<Renderer::TextureSamplerDescriptor> m_textureSamplerDescriptors; //!< Descriptors of all referenced samplers in the shader code.
104+
std::vector<Renderer::TextureSamplerDescriptor> m_mainTextureDescriptors; //!< Descriptors for all main texture references.
105+
std::vector<Renderer::TextureSamplerDescriptor> m_textureSamplerDescriptors; //!< Descriptors of all referenced samplers in the shader code.
104106
BlurTexture::BlurLevel m_maxBlurLevelRequired{BlurTexture::BlurLevel::None}; //!< Max blur level of main texture required by this shader.
105107

106108
std::array<float, 4> m_randValues{}; //!< Random values which don't change every frame.

‎tests/libprojectM/CMakeLists.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ test_api_headers(TestMainAPIHeaders
2929
add_executable(projectM-unittest
3030
HLSLParserTest.cpp
3131
LoggingTest.cpp
32+
MilkdropShaderSamplerParsingTest.cpp
3233
PresetFileParserTest.cpp
3334
WaveformAlignerTest.cpp
3435

‎tests/libprojectM/MilkdropShaderSamplerParsingTest.cpp‎

Lines changed: 174 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,174 @@
1+
#include <gtest/gtest.h>
2+
3+
#include <MilkdropPreset/MilkdropShader.hpp>
4+
5+
using libprojectM::MilkdropPreset::MilkdropShader;
6+
7+
/**
8+
* Subclass to expose protected members for testing.
9+
* Provides public wrappers around the protected GetReferencedSamplers()
10+
* method and m_samplerNames member so that test helpers can call them.
11+
*/
12+
class MilkdropShaderTestable : public MilkdropShader
13+
{
14+
public:
15+
MilkdropShaderTestable()
16+
: MilkdropShader(ShaderType::CompositeShader)
17+
{
18+
}
19+
20+
void CallGetReferencedSamplers(const std::string& program)
21+
{
22+
GetReferencedSamplers(program);
23+
}
24+
25+
auto GetSamplerNames() const -> const std::set<std::string>&
26+
{
27+
return m_samplerNames;
28+
}
29+
};
30+
31+
/**
32+
* Helper: create a testable shader and call GetReferencedSamplers on the given program text.
33+
* Returns the resulting set of sampler names (always includes "main").
34+
*/
35+
static auto ParseSamplers(const std::string& program) -> std::set<std::string>
36+
{
37+
MilkdropShaderTestable shader;
38+
shader.CallGetReferencedSamplers(program);
39+
return shader.GetSamplerNames();
40+
}
41+
42+
// --- Basic positive case ---
43+
44+
TEST(MilkdropShaderSamplerParsing, FindsUncommentedSampler)
45+
{
46+
auto names = ParseSamplers("uniform sampler2D sampler_mytex;\n");
47+
EXPECT_TRUE(names.count("main"));
48+
EXPECT_TRUE(names.count("mytex"));
49+
}
50+
51+
// --- Line comment cases ---
52+
53+
TEST(MilkdropShaderSamplerParsing, IgnoresLineCommentedSampler)
54+
{
55+
auto names = ParseSamplers("// sampler sampler_rand00;\n");
56+
EXPECT_TRUE(names.count("main"));
57+
EXPECT_FALSE(names.count("rand00"))
58+
<< "sampler_rand00 inside a // comment should not be extracted";
59+
}
60+
61+
TEST(MilkdropShaderSamplerParsing, NoSpaceAfterSlashesStillCommented)
62+
{
63+
// This is the exact pattern from the crash-triggering preset
64+
auto names = ParseSamplers("//sampler sampler_rand00;\n");
65+
EXPECT_TRUE(names.count("main"));
66+
EXPECT_FALSE(names.count("rand00"))
67+
<< "sampler_rand00 after //sampler (no space) should not be extracted";
68+
}
69+
70+
// --- Block comment cases ---
71+
72+
TEST(MilkdropShaderSamplerParsing, IgnoresBlockCommentedSampler)
73+
{
74+
auto names = ParseSamplers("/* sampler_rand00 */\n");
75+
EXPECT_TRUE(names.count("main"));
76+
EXPECT_FALSE(names.count("rand00"))
77+
<< "sampler_rand00 inside /* */ should not be extracted";
78+
}
79+
80+
TEST(MilkdropShaderSamplerParsing, BlockCommentSpansMultipleLines)
81+
{
82+
std::string program =
83+
"/*\n"
84+
" sampler_foo;\n"
85+
" texsize_bar;\n"
86+
"*/\n"
87+
"uniform sampler2D sampler_real;\n";
88+
auto names = ParseSamplers(program);
89+
EXPECT_FALSE(names.count("foo"))
90+
<< "sampler_foo inside multi-line block comment should not be extracted";
91+
EXPECT_FALSE(names.count("bar"))
92+
<< "texsize_bar inside multi-line block comment should not be extracted";
93+
EXPECT_TRUE(names.count("real"))
94+
<< "sampler_real outside the block comment should be found";
95+
}
96+
97+
TEST(MilkdropShaderSamplerParsing, SamplerAfterBlockCommentCloseIsFound)
98+
{
99+
std::string program = "/* comment */ sampler_visible;\n";
100+
auto names = ParseSamplers(program);
101+
EXPECT_TRUE(names.count("visible"))
102+
<< "sampler_visible after a closed block comment should be found";
103+
}
104+
105+
// --- texsize comment cases ---
106+
107+
TEST(MilkdropShaderSamplerParsing, IgnoresLineCommentedTexsize)
108+
{
109+
auto names = ParseSamplers("// texsize_rand00;\n");
110+
EXPECT_TRUE(names.count("main"));
111+
EXPECT_FALSE(names.count("rand00"))
112+
<< "texsize_rand00 inside // comment should not be extracted";
113+
}
114+
115+
TEST(MilkdropShaderSamplerParsing, IgnoresBlockCommentedTexsize)
116+
{
117+
auto names = ParseSamplers("/* texsize_rand00; */\n");
118+
EXPECT_TRUE(names.count("main"));
119+
EXPECT_FALSE(names.count("rand00"))
120+
<< "texsize_rand00 inside /* */ should not be extracted";
121+
}
122+
123+
// --- Mixed cases ---
124+
125+
TEST(MilkdropShaderSamplerParsing, MixedCommentedAndUncommented)
126+
{
127+
std::string program =
128+
"//sampler sampler_rand00;\n"
129+
"uniform sampler2D sampler_tex1;\n"
130+
"/* sampler_hidden; */\n"
131+
"uniform sampler2D sampler_tex2;\n";
132+
auto names = ParseSamplers(program);
133+
EXPECT_FALSE(names.count("rand00"))
134+
<< "commented sampler_rand00 should be ignored";
135+
EXPECT_FALSE(names.count("hidden"))
136+
<< "block-commented sampler_hidden should be ignored";
137+
EXPECT_TRUE(names.count("tex1"))
138+
<< "uncommented sampler_tex1 should be found";
139+
EXPECT_TRUE(names.count("tex2"))
140+
<< "uncommented sampler_tex2 should be found";
141+
}
142+
143+
// --- Edge / special cases ---
144+
145+
TEST(MilkdropShaderSamplerParsing, SkipsSamplerState)
146+
{
147+
auto names = ParseSamplers("sampler_state { Filter = LINEAR; };\n");
148+
EXPECT_TRUE(names.count("main"));
149+
EXPECT_FALSE(names.count("state"))
150+
<< "sampler_state is a reserved word and should be skipped";
151+
}
152+
153+
TEST(MilkdropShaderSamplerParsing, EmptyProgram)
154+
{
155+
auto names = ParseSamplers("");
156+
EXPECT_EQ(names.size(), 1u);
157+
EXPECT_TRUE(names.count("main"))
158+
<< "Empty program should still contain 'main'";
159+
}
160+
161+
TEST(MilkdropShaderSamplerParsing, ReproducesCrashPresetPattern)
162+
{
163+
// This reproduces the exact pattern from the crash-triggering preset line:
164+
// comp_1=`//sampler sampler_rand00; // this will choose a random texture from disk!
165+
// After the backtick is stripped by the preset parser, the shader program
166+
// contains the rest of the line.
167+
std::string program = "//sampler sampler_rand00; // this will choose a random texture from disk!\n";
168+
auto names = ParseSamplers(program);
169+
EXPECT_EQ(names.size(), 1u)
170+
<< "Only 'main' should be present; the entire line is a comment";
171+
EXPECT_TRUE(names.count("main"));
172+
EXPECT_FALSE(names.count("rand00"))
173+
<< "rand00 from the crash-triggering preset comment must not be extracted";
174+
}

0 commit comments

Comments
 (0)
⚡