Hi Andreas, I have adapted the code further, now checked into master and OSG-3.4 branch.
Robert. On 29 February 2016 at 09:17, Andreas Roth <[email protected]> wrote: > 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 > >
_______________________________________________ osg-submissions mailing list [email protected] http://lists.openscenegraph.org/listinfo.cgi/osg-submissions-openscenegraph.org
