Hi Robert,
I tested your proposed change in Shader.cpp and i agree that your
solution is more suitable since it limits the search to the current
line.
Thanks for this prompt fix.
Andreas
------ Original Message ------
From: "Robert Osfield" <[email protected]>
To: "Andreas Roth" <[email protected]>; "OpenSceneGraph
Submissions" <[email protected]>
Sent: 2016-02-26 22:49:20
Subject: Re: [osg-submissions] Fix for #pragma handling in
Shader::_computeShaderDefines()
Hi Andeas,
I have just done a code review of your changes and the original code
and feel that the original code has more little problems that your fix
addresses, and this all stem to the way std::string::find_first_of and
find_first_not_of work, some wishful thinking in the code w.r.t how
they actually behave vs what the codes what them to behave like. To
address this issue I have begun experimenting with a local find_first
template function and couple of functors to help out with the character
testing.
The modified Shader.cpp is attached, this now behaves what I think is
correct for my test chase of a #pragma line without a (. Could you
test this in your application to see if it's working fine for you as
well? There are debug messages left at OSG_NOTIFY so you should see
more info output to the console, I'll remove this for the final
checkin.
I have also provide the git diff below so you can see what I've
changed.
Cheers,
Robert.
diff --git a/src/osg/Shader.cpp b/src/osg/Shader.cpp
index f772494..216163b 100644
--- a/src/osg/Shader.cpp
+++ b/src/osg/Shader.cpp
@@ -713,6 +713,62 @@ void Shader::_parseShaderDefines(const
std::string& str, ShaderDefines& defines)
} while (start_of_parameter<str.size());
}
+namespace
+{
+
+template<typename M>
+inline std::string::size_type find_first(const std::string& str, const
M& match, std::string::size_type startpos, std::string::size_type
endpos=std::string::npos)
+{
+ if (endpos==std::string::npos) endpos = str.size();
+ while(startpos<endpos)
+ {
+ if (match(str[startpos])) return startpos;
+
+ ++startpos;
+ }
+ return endpos;
+}
+
+struct CharCompare
+{
+ CharCompare(char c): _c(c) {}
+ bool operator() (char rhs) const { return rhs==_c; }
+ char _c;
+};
+
+struct StringCompare
+{
+ StringCompare(const char* str) : _str(str) {}
+ bool operator() (char rhs) const
+ {
+ const char* ptr = _str;
+ while(*ptr!=0 && rhs!=*ptr) ++ptr;
+ return (*ptr!=0);
+ }
+ const char* _str;
+};
+
+struct NotCharCompare
+{
+ NotCharCompare(char c): _c(c) {}
+ bool operator() (char rhs) const { return rhs!=_c; }
+ char _c;
+};
+
+struct NotStringCompare
+{
+ NotStringCompare(const char* str) : _str(str) {}
+ bool operator() (char rhs) const
+ {
+ const char* ptr = _str;
+ while(*ptr!=0 && rhs!=*ptr) ++ptr;
+ return (*ptr==0);
+ }
+ const char* _str;
+};
+
+}
+
void Shader::_computeShaderDefines()
{
if (_shaderDefinesMode==USE_MANUAL_SETTINGS) return;
@@ -726,21 +782,20 @@ void Shader::_computeShaderDefines()
{
// skip over #pragma characters
pos += 7;
- std::string::size_type first_chararcter =
_shaderSource.find_first_not_of(" \t", pos);
- std::string::size_type eol =
_shaderSource.find_first_of("\n\r", pos);
- if (eol==std::string::npos) eol = _shaderSource.size();
+ std::string::size_type eol = find_first(_shaderSource,
StringCompare("\n\r"), pos);
+
+ std::string::size_type first_chararcter =
find_first(_shaderSource, NotStringCompare(" \t"), pos, eol);
- OSG_INFO<<"\nFound pragma line
["<<_shaderSource.substr(first_chararcter,
eol-first_chararcter)<<"]"<<std::endl;
+ OSG_NOTICE<<"\nFound pragma line
["<<_shaderSource.substr(first_chararcter,
eol-first_chararcter)<<"]"<<std::endl;
if (first_chararcter<eol)
{
- std::string::size_type end_of_keyword =
_shaderSource.find_first_of(" \t(", first_chararcter);
+ std::string::size_type end_of_keyword =
find_first(_shaderSource, StringCompare(" \t("), first_chararcter,
eol);
std::string keyword =
_shaderSource.substr(first_chararcter,
end_of_keyword-first_chararcter);
- std::string::size_type open_brackets =
_shaderSource.find_first_of("(", end_of_keyword);
-
- if ((open_brackets!=std::string::npos))
+ std::string::size_type open_brackets =
find_first(_shaderSource, CharCompare('('), end_of_keyword, eol);
+ if (open_brackets<eol)
{
std::string str(_shaderSource, open_brackets+1,
eol-open_brackets-1);
@@ -748,7 +803,7 @@ void Shader::_computeShaderDefines()
if (keyword == "import_defines")
_parseShaderDefines(str, _shaderDefines);
else if (keyword == "requires")
_parseShaderDefines(str, _shaderRequirements);
else {
- //OSG_NOTICE<<" keyword not matched
["<<keyword<<"]"<<std::endl;
+ OSG_NOTICE<<" keyword not matched
["<<keyword<<"]"<<std::endl;
_parseShaderDefines(str, _shaderDefines);
}
#if 1
@@ -756,14 +811,14 @@ void Shader::_computeShaderDefines()
itr != _shaderDefines.end();
++itr)
{
- OSG_INFO<<" define ["<<*itr<<"]"<<std::endl;
+ OSG_NOTICE<<" define
["<<*itr<<"]"<<std::endl;
}
for(ShaderDefines::iterator itr =
_shaderRequirements.begin();
itr != _shaderRequirements.end();
++itr)
{
- OSG_INFO<<" requirements
["<<*itr<<"]"<<std::endl;
+ OSG_NOTICE<<" requirements
["<<*itr<<"]"<<std::endl;
}
#endif
@@ -771,7 +826,7 @@ void Shader::_computeShaderDefines()
#if 1
else
{
- OSG_INFO<<" Found keyword ["<<keyword<<"] but not
matched ()\n"<<std::endl;
+ OSG_NOTICE<<" Found keyword ["<<keyword<<"] but not
matched ()\n"<<std::endl;
}
#endif
}
_______________________________________________
osg-submissions mailing list
[email protected]
http://lists.openscenegraph.org/listinfo.cgi/osg-submissions-openscenegraph.org
