09/04/15 17:35:24 (25 hours ago)

From Jannik Heller, "I've hit what I believe to be a bug (or at the very least, an unintuitive behaviour) in the osg::Geometry copy constructor. I noticed it when using osg::clone on a Geometry with vertex buffer objects, and the copy flags DEEP_COPY_ARRAYS. To be precise, I add a Geometry to an osgUtil::IncrementalCompileOperation?, then osg::clone the Geometry. I was getting reports from users of random crashes happening.

I believe the offending lines are in the osg::Geometry copy constructor:

if ((copyop.getCopyFlags() & osg::CopyOp::DEEP_COPY_ARRAYS))

if (_useVertexBufferObjects)

// copying of arrays doesn't set up buffer objects so we'll need to force
// Geometry to assign these, we'll do this by switching off VBO's then renabling them.



Toggling the vertex buffer objects off then on again actually touches not only the arrays controlled by DEEP_COPY_ARRAYS, but also the PrimitiveSets? which are controlled by DEEP_COPY_PRIMITIVES. This means if the user has copyflags of only DEEP_COPY_ARRAYS, we are modifying arrays that belong to the original const Geometry& we are copying from. I believe this shouldn't be allowed to happen because we are using a const& specifier for the original Geometry.

In my case the osgUtil::IncrementalCompileOperation? was trying to compile the geometry, while in the main thread a clone operation toggled the VBO's off and on, a crash ensues.

In the attached patch, you will find a more efficient handling of VBO's in the osg::Geometry copy constructor, so that only the Arrays that were actually deep copied have their VBO assigned, and no changes are made to Arrays that already had a valid VBO assigned. In addition, the DEEP_COPY_PRIMITIVES flag is now honored so that VBO's are set up correctly should a user copy a Geometry with only that flag.

1 modified


  • OpenSceneGraph/trunk/include/osgDB/ReadFile

    r13041 r13172  
    4444inline osg::Object* readObjectFile(const std::string& filename) 
    46     return readObjectFile(filename,Registry::instance()->getOptions()); 
    47 } 
     46    return readObjectFile(filename, Registry::instance()->getOptions()); 
     49template<typename T> 
     50inline T* readFile(const std::string& filename, const Options* options) 
     52    osg::ref_ptr<osg::Object> object = readObjectFile(filename, options); 
     53    osg::ref_ptr<T> t = dynamic_cast<T*>(object.get()); 
     54    object = 0;     
     55    return t.release(); 
     58template<typename T> 
     59inline T* readFile(const std::string& filename) 
     61    return readFile<T>(filename, Registry::instance()->getOptions()); 
    4965/** Read an osg::Image from file.