Skip to content

Commit f246ce7

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

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
@@ -484,6 +484,63 @@ void PS(float4 _vDiffuse : COLOR,
484484
program = fullSource;
485485
}
486486

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

552+
// Strip comments so that commented-out sampler/texsize declarations are not matched.
553+
std::string const stripped = StripComments(program);
554+
495555
// Search for sampler usage
496-
auto found = program.find("sampler_", 0);
556+
auto found = stripped.find("sampler_", 0);
497557
while (found != std::string::npos)
498558
{
499559
found += 8;
500-
size_t const end = program.find_first_of(" ;,\n\r)", found);
560+
size_t const end = stripped.find_first_of(" ;,\n\r)", found);
501561

502562
if (end != std::string::npos)
503563
{
504-
std::string const sampler = program.substr(static_cast<int>(found), static_cast<int>(end - found));
564+
std::string const sampler = stripped.substr(static_cast<int>(found), static_cast<int>(end - found));
505565
// Skip "sampler_state", as it's a reserved word and not a sampler.
506566
if (sampler != "state")
507567
{
508568
m_samplerNames.insert(sampler);
509569
}
510570
}
511571

512-
found = program.find("sampler_", found);
572+
found = stripped.find("sampler_", found);
513573
}
514574

515575
// Also search for texsize usage, some presets don't reference the sampler.
516-
found = program.find("texsize_", 0);
576+
found = stripped.find("texsize_", 0);
517577
while (found != std::string::npos)
518578
{
519579
found += 8;
520-
size_t const end = program.find_first_of(" ;,.\n\r)", found);
580+
size_t const end = stripped.find_first_of(" ;,.\n\r)", found);
521581

522582
if (end != std::string::npos)
523583
{
524-
std::string const sampler = program.substr(static_cast<int>(found), static_cast<int>(end - found));
584+
std::string const sampler = stripped.substr(static_cast<int>(found), static_cast<int>(end - found));
525585
m_samplerNames.insert(sampler);
526586
}
527587

528-
found = program.find("texsize_", found);
588+
found = stripped.find("texsize_", found);
529589
}
530590

531591
{
@@ -555,15 +615,15 @@ void MilkdropShader::GetReferencedSamplers(const std::string& program)
555615
}
556616
}
557617

558-
if (program.find("GetBlur3") != std::string::npos)
618+
if (stripped.find("GetBlur3") != std::string::npos)
559619
{
560620
UpdateMaxBlurLevel(BlurTexture::BlurLevel::Blur3);
561621
}
562-
else if (program.find("GetBlur2") != std::string::npos)
622+
else if (stripped.find("GetBlur2") != std::string::npos)
563623
{
564624
UpdateMaxBlurLevel(BlurTexture::BlurLevel::Blur2);
565625
}
566-
else if (program.find("GetBlur1") != std::string::npos)
626+
else if (stripped.find("GetBlur1") != std::string::npos)
567627
{
568628
UpdateMaxBlurLevel(BlurTexture::BlurLevel::Blur1);
569629
}

‎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
@@ -2,6 +2,7 @@ find_package(GTest 1.10 REQUIRED NO_MODULE)
22

33
add_executable(projectM-unittest
44
WaveformAlignerTest.cpp
5+
MilkdropShaderSamplerParsingTest.cpp
56
PresetFileParserTest.cpp
67

78
$<TARGET_OBJECTS:Audio>

‎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)