Merge pull request #2820 from elsid/shader_parser_tests

Shader parser tests

CI/build_googletest.sh

@@ -1,6 +1,6 @@
 #!/bin/sh -e
 
-git clone -b release-1.8.1 https://github.com/google/googletest.git
+git clone -b release-1.10.0 https://github.com/google/googletest.git
 cd googletest
 mkdir build
 cd build

apps/openmw_test_suite/CMakeLists.txt

@@ -27,6 +27,10 @@ if (GTEST_FOUND AND GMOCK_FOUND)
         detournavigator/tilecachedrecastmeshmanager.cpp
 
         settings/parser.cpp
+
+        shader/parsedefines.cpp
+        shader/parsefors.cpp
+        shader/shadermanager.cpp
     )
 
     source_group(apps\\openmw_test_suite FILES openmw_test_suite.cpp ${UNITTEST_SRC_FILES})

apps/openmw_test_suite/shader/parsedefines.cpp

@@ -0,0 +1,191 @@
+#include <components/shader/shadermanager.hpp>
+
+#include <gtest/gtest.h>
+#include <gmock/gmock.h>
+
+namespace
+{
+    using namespace testing;
+    using namespace Shader;
+
+    using DefineMap = ShaderManager::DefineMap;
+
+    struct ShaderParseDefinesTest : Test
+    {
+        std::string mSource;
+        const std::string mName = "shader";
+        DefineMap mDefines;
+        DefineMap mGlobalDefines;
+    };
+
+    TEST_F(ShaderParseDefinesTest, empty_should_succeed)
+    {
+        ASSERT_TRUE(parseDefines(mSource, mDefines, mGlobalDefines, mName));
+        EXPECT_EQ(mSource, "");
+    }
+
+    TEST_F(ShaderParseDefinesTest, should_fail_for_absent_define)
+    {
+        mSource = "@foo\n";
+        ASSERT_FALSE(parseDefines(mSource, mDefines, mGlobalDefines, mName));
+        EXPECT_EQ(mSource, "@foo\n");
+    }
+
+    TEST_F(ShaderParseDefinesTest, should_replace_by_existing_define)
+    {
+        mDefines["foo"] = "42";
+        mSource = "@foo\n";
+        ASSERT_TRUE(parseDefines(mSource, mDefines, mGlobalDefines, mName));
+        EXPECT_EQ(mSource, "42\n");
+    }
+
+    TEST_F(ShaderParseDefinesTest, should_replace_by_existing_global_define)
+    {
+        mGlobalDefines["foo"] = "42";
+        mSource = "@foo\n";
+        ASSERT_TRUE(parseDefines(mSource, mDefines, mGlobalDefines, mName));
+        EXPECT_EQ(mSource, "42\n");
+    }
+
+    TEST_F(ShaderParseDefinesTest, should_prefer_define_over_global_define)
+    {
+        mDefines["foo"] = "13";
+        mGlobalDefines["foo"] = "42";
+        mSource = "@foo\n";
+        ASSERT_TRUE(parseDefines(mSource, mDefines, mGlobalDefines, mName));
+        EXPECT_EQ(mSource, "13\n");
+    }
+
+    namespace SupportedTerminals
+    {
+        struct ShaderParseDefinesTest : ::ShaderParseDefinesTest, WithParamInterface<char> {};
+
+        TEST_P(ShaderParseDefinesTest, support_defines_terminated_by)
+        {
+            mDefines["foo"] = "13";
+            mSource = "@foo" + std::string(1, GetParam());
+            ASSERT_TRUE(parseDefines(mSource, mDefines, mGlobalDefines, mName));
+            EXPECT_EQ(mSource, "13" + std::string(1, GetParam()));
+        }
+
+        INSTANTIATE_TEST_SUITE_P(
+            SupportedTerminals,
+            ShaderParseDefinesTest,
+            Values(' ', '\n', '\r', '(', ')', '[', ']', '.', ';', ',')
+        );
+    }
+
+    TEST_F(ShaderParseDefinesTest, should_not_support_define_ending_with_source)
+    {
+        mDefines["foo"] = "42";
+        mSource = "@foo";
+        ASSERT_FALSE(parseDefines(mSource, mDefines, mGlobalDefines, mName));
+        EXPECT_EQ(mSource, "@foo");
+    }
+
+    TEST_F(ShaderParseDefinesTest, should_replace_all_matched_values)
+    {
+        mDefines["foo"] = "42";
+        mSource = "@foo @foo ";
+        ASSERT_TRUE(parseDefines(mSource, mDefines, mGlobalDefines, mName));
+        EXPECT_EQ(mSource, "42 42 ");
+    }
+
+    TEST_F(ShaderParseDefinesTest, should_support_define_with_empty_name)
+    {
+        mDefines[""] = "42";
+        mSource = "@ ";
+        ASSERT_TRUE(parseDefines(mSource, mDefines, mGlobalDefines, mName));
+        EXPECT_EQ(mSource, "42 ");
+    }
+
+    TEST_F(ShaderParseDefinesTest, should_replace_all_found_defines)
+    {
+        mDefines["foo"] = "42";
+        mDefines["bar"] = "13";
+        mDefines["baz"] = "55";
+        mSource = "@foo @bar ";
+        ASSERT_TRUE(parseDefines(mSource, mDefines, mGlobalDefines, mName));
+        EXPECT_EQ(mSource, "42 13 ");
+    }
+
+    TEST_F(ShaderParseDefinesTest, should_fail_on_foreach_without_endforeach)
+    {
+        mSource = "@foreach ";
+        ASSERT_FALSE(parseDefines(mSource, mDefines, mGlobalDefines, mName));
+        EXPECT_EQ(mSource, "$foreach ");
+    }
+
+    TEST_F(ShaderParseDefinesTest, should_fail_on_endforeach_without_foreach)
+    {
+        mSource = "@endforeach ";
+        ASSERT_FALSE(parseDefines(mSource, mDefines, mGlobalDefines, mName));
+        EXPECT_EQ(mSource, "$endforeach ");
+    }
+
+    TEST_F(ShaderParseDefinesTest, should_replace_at_sign_by_dollar_for_foreach_endforeach)
+    {
+        mSource = "@foreach @endforeach ";
+        ASSERT_TRUE(parseDefines(mSource, mDefines, mGlobalDefines, mName));
+        EXPECT_EQ(mSource, "$foreach $endforeach ");
+    }
+
+    TEST_F(ShaderParseDefinesTest, should_succeed_on_unmatched_nested_foreach)
+    {
+        mSource = "@foreach @foreach @endforeach ";
+        ASSERT_TRUE(parseDefines(mSource, mDefines, mGlobalDefines, mName));
+        EXPECT_EQ(mSource, "$foreach $foreach $endforeach ");
+    }
+
+    TEST_F(ShaderParseDefinesTest, should_fail_on_unmatched_nested_endforeach)
+    {
+        mSource = "@foreach @endforeach @endforeach ";
+        ASSERT_FALSE(parseDefines(mSource, mDefines, mGlobalDefines, mName));
+        EXPECT_EQ(mSource, "$foreach $endforeach $endforeach ");
+    }
+
+    TEST_F(ShaderParseDefinesTest, should_support_nested_foreach)
+    {
+        mSource = "@foreach @foreach @endforeach @endforeach ";
+        ASSERT_TRUE(parseDefines(mSource, mDefines, mGlobalDefines, mName));
+        EXPECT_EQ(mSource, "$foreach $foreach $endforeach $endforeach ");
+    }
+
+    TEST_F(ShaderParseDefinesTest, should_support_foreach_variable)
+    {
+        mSource = "@foreach foo @foo @endforeach ";
+        ASSERT_TRUE(parseDefines(mSource, mDefines, mGlobalDefines, mName));
+        EXPECT_EQ(mSource, "$foreach foo $foo $endforeach ");
+    }
+
+    TEST_F(ShaderParseDefinesTest, should_not_replace_foreach_variable_by_define)
+    {
+        mDefines["foo"] = "42";
+        mSource = "@foreach foo @foo @endforeach ";
+        ASSERT_TRUE(parseDefines(mSource, mDefines, mGlobalDefines, mName));
+        EXPECT_EQ(mSource, "$foreach foo $foo $endforeach ");
+    }
+
+    TEST_F(ShaderParseDefinesTest, should_support_nested_foreach_with_variable)
+    {
+        mSource = "@foreach foo @foo @foreach bar @bar @endforeach @endforeach ";
+        ASSERT_TRUE(parseDefines(mSource, mDefines, mGlobalDefines, mName));
+        EXPECT_EQ(mSource, "$foreach foo $foo $foreach bar $bar $endforeach $endforeach ");
+    }
+
+    TEST_F(ShaderParseDefinesTest, should_not_support_single_line_comments_for_defines)
+    {
+        mDefines["foo"] = "42";
+        mSource = "@foo // @foo\n";
+        ASSERT_TRUE(parseDefines(mSource, mDefines, mGlobalDefines, mName));
+        EXPECT_EQ(mSource, "42 // 42\n");
+    }
+
+    TEST_F(ShaderParseDefinesTest, should_not_support_multiline_comments_for_defines)
+    {
+        mDefines["foo"] = "42";
+        mSource = "/* @foo */ @foo ";
+        ASSERT_TRUE(parseDefines(mSource, mDefines, mGlobalDefines, mName));
+        EXPECT_EQ(mSource, "/* 42 */ 42 ");
+    }
+}

apps/openmw_test_suite/shader/parsefors.cpp

@@ -0,0 +1,94 @@
+#include <components/shader/shadermanager.hpp>
+
+#include <gtest/gtest.h>
+#include <gmock/gmock.h>
+
+namespace
+{
+    using namespace testing;
+    using namespace Shader;
+
+    using DefineMap = ShaderManager::DefineMap;
+
+    struct ShaderParseForsTest : Test
+    {
+        std::string mSource;
+        const std::string mName = "shader";
+    };
+
+    TEST_F(ShaderParseForsTest, empty_should_succeed)
+    {
+        ASSERT_TRUE(parseFors(mSource, mName));
+        EXPECT_EQ(mSource, "");
+    }
+
+    TEST_F(ShaderParseForsTest, should_fail_for_single_escape_symbol)
+    {
+        mSource = "$";
+        ASSERT_FALSE(parseFors(mSource, mName));
+        EXPECT_EQ(mSource, "$");
+    }
+
+    TEST_F(ShaderParseForsTest, should_fail_on_first_found_escaped_not_foreach)
+    {
+        mSource = "$foo ";
+        ASSERT_FALSE(parseFors(mSource, mName));
+        EXPECT_EQ(mSource, "$foo ");
+    }
+
+    TEST_F(ShaderParseForsTest, should_fail_on_absent_foreach_variable)
+    {
+        mSource = "$foreach ";
+        ASSERT_FALSE(parseFors(mSource, mName));
+        EXPECT_EQ(mSource, "$foreach ");
+    }
+
+    TEST_F(ShaderParseForsTest, should_fail_on_unmatched_after_variable)
+    {
+        mSource = "$foreach foo ";
+        ASSERT_FALSE(parseFors(mSource, mName));
+        EXPECT_EQ(mSource, "$foreach foo ");
+    }
+
+    TEST_F(ShaderParseForsTest, should_fail_on_absent_newline_after_foreach_list)
+    {
+        mSource = "$foreach foo 1,2,3 ";
+        ASSERT_FALSE(parseFors(mSource, mName));
+        EXPECT_EQ(mSource, "$foreach foo 1,2,3 ");
+    }
+
+    TEST_F(ShaderParseForsTest, should_fail_on_absent_endforeach_after_newline)
+    {
+        mSource = "$foreach foo 1,2,3\n";
+        ASSERT_FALSE(parseFors(mSource, mName));
+        EXPECT_EQ(mSource, "$foreach foo 1,2,3\n");
+    }
+
+    TEST_F(ShaderParseForsTest, should_replace_complete_foreach_by_line_number)
+    {
+        mSource = "$foreach foo 1,2,3\n$endforeach";
+        ASSERT_TRUE(parseFors(mSource, mName));
+        EXPECT_EQ(mSource, "\n#line 3");
+    }
+
+    TEST_F(ShaderParseForsTest, should_replace_loop_variable)
+    {
+        mSource = "$foreach foo 1,2,3\n$foo\n$endforeach";
+        ASSERT_TRUE(parseFors(mSource, mName));
+        EXPECT_EQ(mSource, "1\n2\n3\n\n#line 4");
+    }
+
+    TEST_F(ShaderParseForsTest, should_count_line_number_from_existing)
+    {
+        mSource = "$foreach foo 1,2,3\n#line 10\n$foo\n$endforeach";
+        ASSERT_TRUE(parseFors(mSource, mName));
+        EXPECT_EQ(mSource, "#line 10\n1\n#line 10\n2\n#line 10\n3\n\n#line 12");
+    }
+
+    TEST_F(ShaderParseForsTest, should_not_support_nested_loops)
+    {
+        mSource = "$foreach foo 1,2\n$foo\n$foreach bar 1,2\n$bar\n$endforeach\n$endforeach";
+        ASSERT_FALSE(parseFors(mSource, mName));
+        EXPECT_EQ(mSource, "1\n1\n2\n$foreach bar 1,2\n1\n\n#line 6\n2\n2\n$foreach bar 1,2\n2\n\n#line 6\n\n#line 7");
+    }
+}

apps/openmw_test_suite/shader/shadermanager.cpp

@@ -0,0 +1,240 @@
+#include <components/shader/shadermanager.hpp>
+
+#include <boost/filesystem/fstream.hpp>
+
+#include <gtest/gtest.h>
+
+namespace
+{
+    using namespace testing;
+    using namespace Shader;
+
+    struct ShaderManagerTest : Test
+    {
+        ShaderManager mManager;
+        ShaderManager::DefineMap mDefines;
+
+        ShaderManagerTest()
+        {
+            mManager.setShaderPath(".");
+        }
+
+        template <class F>
+        void withShaderFile(const std::string& content, F&& f)
+        {
+            withShaderFile("", content, std::forward<F>(f));
+        }
+
+        template <class F>
+        void withShaderFile(const std::string& suffix, const std::string& content, F&& f)
+        {
+            const auto path = UnitTest::GetInstance()->current_test_info()->name() + suffix + ".glsl";
+
+            {
+                boost::filesystem::ofstream stream;
+                stream.open(path);
+                stream << content;
+                stream.close();
+            }
+
+            f(path);
+        }
+    };
+
+    TEST_F(ShaderManagerTest, get_shader_with_empty_content_should_succeed)
+    {
+        const std::string content;
+
+        withShaderFile(content, [this] (const std::string& templateName) {
+            EXPECT_TRUE(mManager.getShader(templateName, {}, osg::Shader::VERTEX));
+        });
+    }
+
+    TEST_F(ShaderManagerTest, get_shader_should_not_change_source_without_template_parameters)
+    {
+        const std::string content =
+            "#version 120\n"
+            "void main() {}\n";
+
+        withShaderFile(content, [&] (const std::string& templateName) {
+            const auto shader = mManager.getShader(templateName, mDefines, osg::Shader::VERTEX);
+            ASSERT_TRUE(shader);
+            EXPECT_EQ(shader->getShaderSource(), content);
+        });
+    }
+
+    TEST_F(ShaderManagerTest, get_shader_should_replace_includes_with_content)
+    {
+        const std::string content0 =
+            "void foo() {}\n";
+
+        withShaderFile("_0", content0, [&] (const std::string& templateName0) {
+            const std::string content1 =
+                "#include \"" + templateName0 + "\"\n"
+                "void bar() { foo() }\n";
+
+            withShaderFile("_1", content1, [&] (const std::string& templateName1) {
+                const std::string content2 =
+                    "#version 120\n"
+                    "#include \"" + templateName1 + "\"\n"
+                    "void main() { bar() }\n";
+
+                withShaderFile(content2, [&] (const std::string& templateName2) {
+                    const auto shader = mManager.getShader(templateName2, mDefines, osg::Shader::VERTEX);
+                    ASSERT_TRUE(shader);
+                    const std::string expected =
+                        "#version 120\n"
+                        "#line 0 1\n"
+                        "#line 0 2\n"
+                        "void foo() {}\n"
+                        "\n"
+                        "#line 0 0\n"
+                        "\n"
+                        "void bar() { foo() }\n"
+                        "\n"
+                        "#line 2 0\n"
+                        "\n"
+                        "void main() { bar() }\n";
+                    EXPECT_EQ(shader->getShaderSource(), expected);
+                });
+            });
+        });
+    }
+
+    TEST_F(ShaderManagerTest, get_shader_should_replace_defines)
+    {
+        const std::string content =
+            "#version 120\n"
+            "#define FLAG @flag\n"
+            "void main() {}\n"
+        ;
+
+        withShaderFile(content, [&] (const std::string& templateName) {
+            mDefines["flag"] = "1";
+            const auto shader = mManager.getShader(templateName, mDefines, osg::Shader::VERTEX);
+            ASSERT_TRUE(shader);
+            const std::string expected =
+                "#version 120\n"
+                "#define FLAG 1\n"
+                "void main() {}\n";
+            EXPECT_EQ(shader->getShaderSource(), expected);
+        });
+    }
+
+    TEST_F(ShaderManagerTest, get_shader_should_expand_loop)
+    {
+        const std::string content =
+            "#version 120\n"
+            "@foreach index @list\n"
+            "    varying vec4 foo@index;\n"
+            "@endforeach\n"
+            "void main() {}\n"
+        ;
+
+        withShaderFile(content, [&] (const std::string& templateName) {
+            mDefines["list"] = "1,2,3";
+            const auto shader = mManager.getShader(templateName, mDefines, osg::Shader::VERTEX);
+            ASSERT_TRUE(shader);
+            const std::string expected =
+                "#version 120\n"
+                "    varying vec4 foo1;\n"
+                "    varying vec4 foo2;\n"
+                "    varying vec4 foo3;\n"
+                "\n"
+                "#line 5\n"
+                "void main() {}\n";
+            EXPECT_EQ(shader->getShaderSource(), expected);
+        });
+    }
+
+    TEST_F(ShaderManagerTest, get_shader_should_replace_loops_with_conditions)
+    {
+        const std::string content =
+            "#version 120\n"
+            "@foreach index @list\n"
+            "    varying vec4 foo@index;\n"
+            "@endforeach\n"
+            "void main()\n"
+            "{\n"
+            "#ifdef BAR\n"
+            "@foreach index @list\n"
+            "    foo@index = vec4(1.0);\n"
+            "@endforeach\n"
+            "#elif BAZ\n"
+            "@foreach index @list\n"
+            "    foo@index = vec4(2.0);\n"
+            "@endforeach\n"
+            "#else\n"
+            "@foreach index @list\n"
+            "    foo@index = vec4(3.0);\n"
+            "@endforeach\n"
+            "#endif\n"
+            "}\n"
+        ;
+
+        withShaderFile(content, [&] (const std::string& templateName) {
+            mDefines["list"] = "1,2,3";
+            const auto shader = mManager.getShader(templateName, mDefines, osg::Shader::VERTEX);
+            ASSERT_TRUE(shader);
+            const std::string expected =
+                "#version 120\n"
+                "    varying vec4 foo1;\n"
+                "    varying vec4 foo2;\n"
+                "    varying vec4 foo3;\n"
+                "\n"
+                "#line 5\n"
+                "void main()\n"
+                "{\n"
+                "#ifdef BAR\n"
+                "    foo1 = vec4(1.0);\n"
+                "    foo2 = vec4(1.0);\n"
+                "    foo3 = vec4(1.0);\n"
+                "\n"
+                "#line 11\n"
+                "#elif BAZ\n"
+                "#line 12\n"
+                "    foo1 = vec4(2.0);\n"
+                "    foo2 = vec4(2.0);\n"
+                "    foo3 = vec4(2.0);\n"
+                "\n"
+                "#line 15\n"
+                "#else\n"
+                "#line 16\n"
+                "    foo1 = vec4(3.0);\n"
+                "    foo2 = vec4(3.0);\n"
+                "    foo3 = vec4(3.0);\n"
+                "\n"
+                "#line 19\n"
+                "#endif\n"
+                "#line 20\n"
+                "}\n";
+            EXPECT_EQ(shader->getShaderSource(), expected);
+        });
+    }
+
+    TEST_F(ShaderManagerTest, get_shader_should_fail_on_absent_template_parameters_in_single_line_comments)
+    {
+        const std::string content =
+            "#version 120\n"
+            "// #define FLAG @flag\n"
+            "void main() {}\n"
+        ;
+
+        withShaderFile(content, [&] (const std::string& templateName) {
+            EXPECT_FALSE(mManager.getShader(templateName, mDefines, osg::Shader::VERTEX));
+        });
+    }
+
+    TEST_F(ShaderManagerTest, get_shader_should_fail_on_absent_template_parameter_in_multi_line_comments)
+    {
+        const std::string content =
+            "#version 120\n"
+            "/* #define FLAG @flag */\n"
+            "void main() {}\n"
+        ;
+
+        withShaderFile(content, [&] (const std::string& templateName) {
+            EXPECT_FALSE(mManager.getShader(templateName, mDefines, osg::Shader::VERTEX));
+        });
+    }
+}

components/shader/shadermanager.cpp

@@ -58,7 +58,7 @@ namespace Shader
         return true;
     }
 
-    bool parseIncludes(boost::filesystem::path shaderPath, std::string& source, const std::string& shaderTemplate)
+    bool parseIncludes(boost::filesystem::path shaderPath, std::string& source, const std::string& templateName)
     {
         Misc::StringUtils::replaceAll(source, "\r\n", "\n");
 
@@ -70,13 +70,13 @@ namespace Shader
             size_t start = source.find('"', foundPos);
             if (start == std::string::npos || start == source.size()-1)
             {
-                Log(Debug::Error) << "Shader " << shaderTemplate << " error: Invalid #include";
+                Log(Debug::Error) << "Shader " << templateName << " error: Invalid #include";
                 return false;
             }
             size_t end = source.find('"', start+1);
             if (end == std::string::npos)
             {
-                Log(Debug::Error) << "Shader " << shaderTemplate << " error: Invalid #include";
+                Log(Debug::Error) << "Shader " << templateName << " error: Invalid #include";
                 return false;
             }
             std::string includeFilename = source.substr(start+1, end-(start+1));
@@ -85,7 +85,7 @@ namespace Shader
             includeFstream.open(includePath);
             if (includeFstream.fail())
             {
-                Log(Debug::Error) << "Shader " << shaderTemplate << " error: Failed to open include " << includePath.string();
+                Log(Debug::Error) << "Shader " << templateName << " error: Failed to open include " << includePath.string();
                 return false;
             }
 
@@ -120,14 +120,14 @@ namespace Shader
 
             if (includedFiles.insert(includePath).second == false)
             {
-                Log(Debug::Error) << "Shader " << shaderTemplate << " error: Detected cyclic #includes";
+                Log(Debug::Error) << "Shader " << templateName << " error: Detected cyclic #includes";
                 return false;
             }
         }
         return true;
     }
 
-    bool parseFors(std::string& source, const std::string& shaderTemplate)
+    bool parseFors(std::string& source, const std::string& templateName)
     {
         const char escapeCharacter = '$';
         size_t foundPos = 0;
@@ -136,13 +136,13 @@ namespace Shader
             size_t endPos = source.find_first_of(" \n\r()[].;,", foundPos);
             if (endPos == std::string::npos)
             {
-                Log(Debug::Error) << "Shader " << shaderTemplate << " error: Unexpected EOF";
+                Log(Debug::Error) << "Shader " << templateName << " error: Unexpected EOF";
                 return false;
             }
             std::string command = source.substr(foundPos + 1, endPos - (foundPos + 1));
             if (command != "foreach")
             {
-                Log(Debug::Error) << "Shader " << shaderTemplate << " error: Unknown shader directive: $" << command;
+                Log(Debug::Error) << "Shader " << templateName << " error: Unknown shader directive: $" << command;
                 return false;
             }
 
@@ -150,7 +150,7 @@ namespace Shader
             size_t iterNameEnd = source.find_first_of(" \n\r()[].;,", iterNameStart);
             if (iterNameEnd == std::string::npos)
             {
-                Log(Debug::Error) << "Shader " << shaderTemplate << " error: Unexpected EOF";
+                Log(Debug::Error) << "Shader " << templateName << " error: Unexpected EOF";
                 return false;
             }
             std::string iteratorName = "$" + source.substr(iterNameStart, iterNameEnd - iterNameStart);
@@ -159,7 +159,7 @@ namespace Shader
             size_t listEnd = source.find_first_of("\n\r", listStart);
             if (listEnd == std::string::npos)
             {
-                Log(Debug::Error) << "Shader " << shaderTemplate << " error: Unexpected EOF";
+                Log(Debug::Error) << "Shader " << templateName << " error: Unexpected EOF";
                 return false;
             }
             std::string list = source.substr(listStart, listEnd - listStart);
@@ -171,7 +171,7 @@ namespace Shader
             size_t contentEnd = source.find("$endforeach", contentStart);
             if (contentEnd == std::string::npos)
             {
-                Log(Debug::Error) << "Shader " << shaderTemplate << " error: Unexpected EOF";
+                Log(Debug::Error) << "Shader " << templateName << " error: Unexpected EOF";
                 return false;
             }
             std::string content = source.substr(contentStart, contentEnd - contentStart);
@@ -211,7 +211,7 @@ namespace Shader
     }
 
     bool parseDefines(std::string& source, const ShaderManager::DefineMap& defines,
-        const ShaderManager::DefineMap& globalDefines, const std::string& shaderTemplate)
+        const ShaderManager::DefineMap& globalDefines, const std::string& templateName)
     {
         const char escapeCharacter = '@';
         size_t foundPos = 0;
@@ -221,7 +221,7 @@ namespace Shader
             size_t endPos = source.find_first_of(" \n\r()[].;,", foundPos);
             if (endPos == std::string::npos)
             {
-                Log(Debug::Error) << "Shader " << shaderTemplate << " error: Unexpected EOF";
+                Log(Debug::Error) << "Shader " << templateName << " error: Unexpected EOF";
                 return false;
             }
             std::string define = source.substr(foundPos+1, endPos - (foundPos+1));
@@ -234,7 +234,7 @@ namespace Shader
                 size_t iterNameEnd = source.find_first_of(" \n\r()[].;,", iterNameStart);
                 if (iterNameEnd == std::string::npos)
                 {
-                    Log(Debug::Error) << "Shader " << shaderTemplate << " error: Unexpected EOF";
+                    Log(Debug::Error) << "Shader " << templateName << " error: Unexpected EOF";
                     return false;
                 }
                 forIterators.push_back(source.substr(iterNameStart, iterNameEnd - iterNameStart));
@@ -244,7 +244,7 @@ namespace Shader
                 source.replace(foundPos, 1, "$");
                 if (forIterators.empty())
                 {
-                    Log(Debug::Error) << "Shader " << shaderTemplate << " error: endforeach without foreach";
+                    Log(Debug::Error) << "Shader " << templateName << " error: endforeach without foreach";
                     return false;
                 }
                 else
@@ -264,22 +264,22 @@ namespace Shader
             }
             else
             {
-                Log(Debug::Error) << "Shader " << shaderTemplate << " error: Undefined " << define;
+                Log(Debug::Error) << "Shader " << templateName << " error: Undefined " << define;
                 return false;
             }
         }
         return true;
     }
 
-    osg::ref_ptr<osg::Shader> ShaderManager::getShader(const std::string &shaderTemplate, const ShaderManager::DefineMap &defines, osg::Shader::Type shaderType)
+    osg::ref_ptr<osg::Shader> ShaderManager::getShader(const std::string &templateName, const ShaderManager::DefineMap &defines, osg::Shader::Type shaderType)
     {
         OpenThreads::ScopedLock<OpenThreads::Mutex> lock(mMutex);
 
         // read the template if we haven't already
-        TemplateMap::iterator templateIt = mShaderTemplates.find(shaderTemplate);
+        TemplateMap::iterator templateIt = mShaderTemplates.find(templateName);
         if (templateIt == mShaderTemplates.end())
         {
-            boost::filesystem::path p = (boost::filesystem::path(mPath) / shaderTemplate);
+            boost::filesystem::path p = (boost::filesystem::path(mPath) / templateName);
             boost::filesystem::ifstream stream;
             stream.open(p);
             if (stream.fail())
@@ -293,20 +293,20 @@ namespace Shader
             // parse includes
             std::string source = buffer.str();
             if (!addLineDirectivesAfterConditionalBlocks(source)
-                    || !parseIncludes(boost::filesystem::path(mPath), source, shaderTemplate))
+                    || !parseIncludes(boost::filesystem::path(mPath), source, templateName))
                 return nullptr;
 
-            templateIt = mShaderTemplates.insert(std::make_pair(shaderTemplate, source)).first;
+            templateIt = mShaderTemplates.insert(std::make_pair(templateName, source)).first;
         }
 
-        ShaderMap::iterator shaderIt = mShaders.find(std::make_pair(shaderTemplate, defines));
+        ShaderMap::iterator shaderIt = mShaders.find(std::make_pair(templateName, defines));
         if (shaderIt == mShaders.end())
         {
             std::string shaderSource = templateIt->second;
-            if (!parseDefines(shaderSource, defines, mGlobalDefines, shaderTemplate) || !parseFors(shaderSource, shaderTemplate))
+            if (!parseDefines(shaderSource, defines, mGlobalDefines, templateName) || !parseFors(shaderSource, templateName))
             {
                 // Add to the cache anyway to avoid logging the same error over and over.
-                mShaders.insert(std::make_pair(std::make_pair(shaderTemplate, defines), nullptr));
+                mShaders.insert(std::make_pair(std::make_pair(templateName, defines), nullptr));
                 return nullptr;
             }
 
@@ -316,7 +316,7 @@ namespace Shader
             static unsigned int counter = 0;
             shader->setName(std::to_string(counter++));
 
-            shaderIt = mShaders.insert(std::make_pair(std::make_pair(shaderTemplate, defines), shader)).first;
+            shaderIt = mShaders.insert(std::make_pair(std::make_pair(templateName, defines), shader)).first;
         }
         return shaderIt->second;
     }

components/shader/shadermanager.hpp

@@ -30,7 +30,7 @@ namespace Shader
         /// @param shaderType The type of shader (usually vertex or fragment shader).
         /// @note May return nullptr on failure.
         /// @note Thread safe.
-        osg::ref_ptr<osg::Shader> getShader(const std::string& shaderTemplate, const DefineMap& defines, osg::Shader::Type shaderType);
+        osg::ref_ptr<osg::Shader> getShader(const std::string& templateName, const DefineMap& defines, osg::Shader::Type shaderType);
 
         osg::ref_ptr<osg::Program> getProgram(osg::ref_ptr<osg::Shader> vertexShader, osg::ref_ptr<osg::Shader> fragmentShader);
 
@@ -63,6 +63,10 @@ namespace Shader
         OpenThreads::Mutex mMutex;
     };
 
+    bool parseFors(std::string& source, const std::string& templateName);
+
+    bool parseDefines(std::string& source, const ShaderManager::DefineMap& defines,
+        const ShaderManager::DefineMap& globalDefines, const std::string& templateName);
 }
 
 #endif