From Miha Ravselj, "Regarding previous submission it was only partial solution. After further testing I found similar bug also in ClearNode serializer.

//GLbitfield mask = GL_COLOR_BUFFER_BIT|GL_DEPTH_BUFFER_BIT;
This line was problematic since it produced incorrect result when let's say COLOR flag is serialized
it should be null as in Camera serializer or in a proposed BitFlagsSerializer


This line of code caused that whenever only GL_COLOR_BUFFER_BIT bit was written and on value read GL_COLOR_BUFFER_BIT|GL_DEPTH_BUFFER_BIT was restored instead of GL_COLOR_BUFFER_BIT only.

//GLbitfield mask = 0; //this resolves the issue same as in camera
Also same bit-wise comparison bug was also present in write method.
-------------------------------------------------------------------------------------

As you can see there are total 3 bit mask serializers in OSG and all 3 had bugs so I decided to add ADD_BITFLAGS_SERIALIZER and replace USER serializers in osg::Camera, osg::ClearNode and osgText::TextBase. I have made sure that bitflags serializer does not break backwards-compatibility since it uses same code as user serializer does in all 3 cases. (see tester.cpp on how compatibility test was performed)"



git-svn-id: http://svn.openscenegraph.org/osg/OpenSceneGraph/trunk@14752 16af8721-9629-0410-8352-f15c8da7e697
This commit is contained in:
Robert Osfield 2015-03-03 12:03:23 +00:00
parent 4c4738eced
commit ec6edf535d
5 changed files with 123 additions and 150 deletions

View File

@ -21,9 +21,6 @@
namespace osgDB
{
typedef std::vector<std::string> StringList;
extern OSGDB_EXPORT void split( const std::string& src, StringList& list, char separator=' ' );
struct MethodObject : public osg::Referenced
{
typedef std::vector< osg::ref_ptr<osg::Object> > Parameters;

View File

@ -28,6 +28,10 @@
namespace osgDB
{
typedef std::vector<std::string> StringList;
extern OSGDB_EXPORT void split( const std::string& src, StringList& list, char separator=' ' );
#ifndef OBJECT_CAST
#define OBJECT_CAST static_cast
#endif
@ -1421,6 +1425,96 @@ public:
Setter _setter;
};
template<typename C, typename P=unsigned int>
class BitFlagsSerializer : public osgDB::TemplateSerializer<P>
{
public:
typedef TemplateSerializer<P> ParentType;
typedef P (C::*Getter)() const;
typedef void (C::*Setter)( P );
BitFlagsSerializer( const char* name, P def, Getter gf, Setter sf )
: ParentType(name, def), _getter(gf), _setter(sf) {}
void add( const char* str, P value )
{
_lookup.add(str, static_cast<osgDB::IntLookup::Value>(value));
}
P getValue( const char* str )
{ return static_cast<P>(_lookup.getValue(str)); }
const std::string& getString( P value )
{ return _lookup.getString(static_cast<osgDB::IntLookup::Value>(value)); }
virtual bool read( osgDB::InputStream& is, osg::Object& obj )
{
C& object = OBJECT_CAST<C&>(obj);
if ( is.isBinary() )
{
bool ok = false; is >> ok; //code from user serialized ensuring backwards-compatibility
if ( !ok ) return true;
P mask;
is >> mask;
(object.*_setter)( mask );
}
else
{
if ( !is.matchString(ParentType::_name) ) //code from user serialized ensuring backwards-compatibility
return true;
P mask=P();
std::string maskSetString;
is >> maskSetString;
osgDB::StringList maskList;
osgDB::split( maskSetString, maskList, '|' );
for ( unsigned int i=0; i<maskList.size(); ++i )
mask |= _lookup.getValue( maskList[i].c_str());
(object.*_setter)( mask );
}
return true;
}
virtual bool write( osgDB::OutputStream& os, const osg::Object& obj )
{
const C& object = OBJECT_CAST<const C&>(obj);
const P mask = (object.*_getter)();
bool ok = ParentType::_defaultValue!=static_cast<P>(mask);
if ( os.isBinary() )
{
os << ok;
if ( !ok )
return true;
os << (int)mask; //just write int value in binary case
}
else
{
if ( !ok )
return true;
os << os.PROPERTY(ParentType::_name.c_str());
std::string maskString;
const osgDB::IntLookup::ValueToString& v2sm = _lookup.getValueToString();
for( osgDB::IntLookup::ValueToString::const_iterator itr = v2sm.begin() ; itr != v2sm.end() ; itr++)
if( (mask & itr->first) != 0 )
maskString += std::string(itr->second + "|");
if ( !maskString.size() )
maskString = std::string("NONE|");
maskString.erase(maskString.size()-1,1);
os << maskString << std::endl; //remove last "|"
}
return true;
}
public:
Getter _getter;
Setter _setter;
protected:
osgDB::IntLookup _lookup;
};
// ADDING MANIPULATORS
#define ADD_SERIALIZER(S) \
wrapper->addSerializer( (S) )
@ -1745,6 +1839,17 @@ public:
#define END_ENUM_SERIALIZER() \
wrapper->addSerializer(serializer.get(), osgDB::BaseSerializer::RW_ENUM); }
#define BEGIN_BITFLAGS_SERIALIZER(PROP, DEF) \
{ typedef osgDB::BitFlagsSerializer<MyClass> MySerializer; \
osg::ref_ptr<MySerializer> serializer = new MySerializer( \
#PROP, DEF, &MyClass::get##PROP, &MyClass::set##PROP)
#define ADD_BITFLAG_VALUE(VALUE_NAME, VALUE) \
serializer->add(#VALUE_NAME, VALUE)
#define END_BITFLAGS_SERIALIZER() \
wrapper->addSerializer(serializer.get(), osgDB::BaseSerializer::RW_INT); }
// VERSION CONTROL OPERATORS
#define UPDATE_TO_VERSION(VER) \
wrapper->setUpdatedVersion( (VER) );

View File

@ -104,55 +104,6 @@ static void writeBufferAttachment( osgDB::OutputStream& os, const osg::Camera::A
os << os.PROPERTY("MultisampleColorSamples") << attachment._multisampleColorSamples << std::endl;
}
// _clearMask
static bool checkClearMask( const osg::Camera& node )
{
return node.getClearMask()!=(GL_COLOR_BUFFER_BIT|GL_DEPTH_BUFFER_BIT);
}
static bool readClearMask( osgDB::InputStream& is, osg::Camera& node )
{
GLbitfield mask = 0;
if ( is.isBinary() )
{
int maskValue; is >> maskValue;
mask = (GLbitfield)maskValue;
}
else
{
std::string maskSetString; is >> maskSetString;
osgDB::StringList maskList; osgDB::split( maskSetString, maskList, '|' );
for ( unsigned int i=0; i<maskList.size(); ++i )
{
const std::string& maskValue = maskList[i];
if ( maskValue=="COLOR" ) mask |= GL_COLOR_BUFFER_BIT;
else if ( maskValue=="DEPTH" ) mask |= GL_DEPTH_BUFFER_BIT;
else if ( maskValue=="ACCUM" ) mask |= GL_ACCUM_BUFFER_BIT;
else if ( maskValue=="STENCIL" ) mask |= GL_STENCIL_BUFFER_BIT;
}
}
node.setClearMask( mask );
return true;
}
static bool writeClearMask( osgDB::OutputStream& os, const osg::Camera& node )
{
GLbitfield mask = node.getClearMask();
if ( os.isBinary() )
os << (int)mask;
else
{
std::string maskString;
if ( (mask & GL_COLOR_BUFFER_BIT)!=0 ) maskString += std::string("COLOR|");
if ( (mask & GL_DEPTH_BUFFER_BIT)!=0 ) maskString += std::string("DEPTH|");
if ( (mask & GL_ACCUM_BUFFER_BIT)!=0 ) maskString += std::string("ACCUM|");
if ( (mask & GL_STENCIL_BUFFER_BIT)!=0 ) maskString += std::string("STENCIL|");
if ( !maskString.size() ) maskString = std::string("NONE|");
os << maskString.substr(0, maskString.size()-1) << std::endl;
}
return true;
}
// _renderOrder & _renderOrderNum
static bool checkRenderOrder( const osg::Camera& node )
{
@ -234,7 +185,12 @@ REGISTER_OBJECT_WRAPPER( Camera,
"osg::Object osg::Node osg::Group osg::Transform osg::Camera" )
{
ADD_BOOL_SERIALIZER( AllowEventFocus, true ); // _allowEventFocus
ADD_USER_SERIALIZER( ClearMask ); // _clearMask
BEGIN_BITFLAGS_SERIALIZER(ClearMask,GL_COLOR_BUFFER_BIT|GL_DEPTH_BUFFER_BIT);
ADD_BITFLAG_VALUE(COLOR, GL_COLOR_BUFFER_BIT);
ADD_BITFLAG_VALUE(DEPTH, GL_DEPTH_BUFFER_BIT);
ADD_BITFLAG_VALUE(ACCUM, GL_ACCUM_BUFFER_BIT);
ADD_BITFLAG_VALUE(STENCIL, GL_STENCIL_BUFFER_BIT);
END_BITFLAGS_SERIALIZER();
ADD_VEC4_SERIALIZER( ClearColor, osg::Vec4() ); // _clearColor
ADD_VEC4_SERIALIZER( ClearAccum, osg::Vec4() ); // _clearAccum
ADD_DOUBLE_SERIALIZER( ClearDepth, 1.0 ); // _clearDepth

View File

@ -7,55 +7,6 @@
#define GL_ACCUM_BUFFER_BIT 0x00000200
#endif
// _clearMask
static bool checkClearMask( const osg::ClearNode& node )
{
return node.getClearMask()!=(GL_COLOR_BUFFER_BIT|GL_DEPTH_BUFFER_BIT);
}
static bool readClearMask( osgDB::InputStream& is, osg::ClearNode& node )
{
GLbitfield mask = GL_COLOR_BUFFER_BIT|GL_DEPTH_BUFFER_BIT;
if ( is.isBinary() )
{
int maskValue; is >> maskValue;
mask = (GLbitfield)maskValue;
}
else
{
std::string maskSetString; is >> maskSetString;
osgDB::StringList maskList; osgDB::split( maskSetString, maskList, '|' );
for ( unsigned int i=0; i<maskList.size(); ++i )
{
const std::string& maskValue = maskList[i];
if ( maskValue=="COLOR" ) mask |= GL_COLOR_BUFFER_BIT;
else if ( maskValue=="DEPTH" ) mask |= GL_DEPTH_BUFFER_BIT;
else if ( maskValue=="ACCUM" ) mask |= GL_ACCUM_BUFFER_BIT;
else if ( maskValue=="STENCIL" ) mask |= GL_STENCIL_BUFFER_BIT;
}
}
node.setClearMask( mask );
return true;
}
static bool writeClearMask( osgDB::OutputStream& os, const osg::ClearNode& node )
{
GLbitfield mask = node.getClearMask();
if ( os.isBinary() )
os << (int)mask;
else
{
std::string maskString;
if ( mask==GL_COLOR_BUFFER_BIT ) maskString += std::string("COLOR|");
if ( mask==GL_DEPTH_BUFFER_BIT ) maskString += std::string("DEPTH|");
if ( mask==GL_ACCUM_BUFFER_BIT ) maskString += std::string("ACCUM|");
if ( mask==GL_STENCIL_BUFFER_BIT ) maskString += std::string("STENCIL|");
if ( !maskString.size() ) maskString = std::string("NONE|");
os << maskString.substr(0, maskString.size()-1) << std::endl;
}
return true;
}
REGISTER_OBJECT_WRAPPER( ClearNode,
new osg::ClearNode,
osg::ClearNode,
@ -63,5 +14,10 @@ REGISTER_OBJECT_WRAPPER( ClearNode,
{
ADD_BOOL_SERIALIZER( RequiresClear, true ); // _requiresClear
ADD_VEC4_SERIALIZER( ClearColor, osg::Vec4(0.0f, 0.0f, 0.0f, 1.0f) ); // _clearColor
ADD_USER_SERIALIZER( ClearMask ); // _clearMask
BEGIN_BITFLAGS_SERIALIZER(ClearMask,GL_COLOR_BUFFER_BIT|GL_DEPTH_BUFFER_BIT);
ADD_BITFLAG_VALUE(COLOR, GL_COLOR_BUFFER_BIT);
ADD_BITFLAG_VALUE(DEPTH, GL_DEPTH_BUFFER_BIT);
ADD_BITFLAG_VALUE(ACCUM, GL_ACCUM_BUFFER_BIT);
ADD_BITFLAG_VALUE(STENCIL, GL_STENCIL_BUFFER_BIT);
END_BITFLAGS_SERIALIZER();
}

View File

@ -123,52 +123,6 @@ static bool writeText( osgDB::OutputStream& os, const osgText::TextBase& text )
return true;
}
// _drawMode
static bool checkDrawMode( const osgText::TextBase& text )
{
return text.getDrawMode()!=osgText::TextBase::TEXT;
}
static bool readDrawMode( osgDB::InputStream& is, osgText::TextBase& text )
{
unsigned int mask = osgText::TextBase::TEXT;
if ( is.isBinary() )
is >> mask;
else
{
std::string maskSetString; is >> maskSetString;
osgDB::StringList maskList; osgDB::split( maskSetString, maskList, '|' );
for ( unsigned int i=0; i<maskList.size(); ++i )
{
const std::string& maskValue = maskList[i];
if ( maskValue=="TEXT" ) mask |= osgText::TextBase::TEXT;
else if ( maskValue=="BOUND" ) mask |= osgText::TextBase::BOUNDINGBOX;
else if ( maskValue=="FILLED" ) mask |= osgText::TextBase::FILLEDBOUNDINGBOX;
else if ( maskValue=="ALIGNMENT" ) mask |= osgText::TextBase::ALIGNMENT;
}
}
text.setDrawMode( mask );
return true;
}
static bool writeDrawMode( osgDB::OutputStream& os, const osgText::TextBase& text )
{
unsigned int mask = text.getDrawMode();
if ( os.isBinary() )
os << mask;
else
{
std::string maskString;
if ( (mask&osgText::TextBase::TEXT)!=0 ) maskString += std::string("TEXT|");
if ( (mask&osgText::TextBase::BOUNDINGBOX)!=0 ) maskString += std::string("BOUND|");
if ( (mask&osgText::TextBase::FILLEDBOUNDINGBOX)!=0 ) maskString += std::string("FILLED|");
if ( (mask&osgText::TextBase::ALIGNMENT)!=0 ) maskString += std::string("ALIGNMENT|");
if ( !maskString.size() ) maskString = std::string("NONE|");
os << maskString.substr(0, maskString.size()-1) << std::endl;
}
return true;
}
REGISTER_OBJECT_WRAPPER( osgText_TextBase,
/*new osgText::TextBase*/NULL,
osgText::TextBase,
@ -228,7 +182,12 @@ REGISTER_OBJECT_WRAPPER( osgText_TextBase,
ADD_ENUM_VALUE( VERTICAL );
END_ENUM_SERIALIZER(); // _layout
ADD_USER_SERIALIZER( DrawMode ); // _drawMode
BEGIN_BITFLAGS_SERIALIZER(DrawMode,osgText::TextBase::TEXT);
ADD_BITFLAG_VALUE(TEXT, osgText::TextBase::TEXT);
ADD_BITFLAG_VALUE(BOUND, osgText::TextBase::BOUNDINGBOX);
ADD_BITFLAG_VALUE(FILLED, osgText::TextBase::FILLEDBOUNDINGBOX);
ADD_BITFLAG_VALUE(ALIGNMENT, osgText::TextBase::ALIGNMENT);
END_BITFLAGS_SERIALIZER();
ADD_FLOAT_SERIALIZER( BoundingBoxMargin, 0.0f ); // _textBBMargin
ADD_VEC4_SERIALIZER( BoundingBoxColor, osg::Vec4() ); // _textBBColor
}