Show
Ignore:
Timestamp:
09/04/15 17:35:24 (60 minutes ago)
Author:
robert
Message:

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.
setUseVertexBufferObjects(false);
setUseVertexBufferObjects(true);

}

}

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.
"

Files:
1 modified

Legend:

Unmodified
Added
Removed
  • OpenSceneGraph/trunk/src/osgViewer/Viewer.cpp

    r13876 r13890  
    890890                ++itr) 
    891891            { 
    892                 osgGA::GUIEventAdapter* event = itr->get(); 
     892                osgGA::GUIEventAdapter* event = (*itr)->asGUIEventAdapter(); 
     893                if (!event) continue; 
    893894 
    894895                event->setGraphicsContext(gw); 
     
    944945                ++itr) 
    945946            { 
    946                 osgGA::GUIEventAdapter* event = itr->get(); 
     947                osgGA::GUIEventAdapter* event = (*itr)->asGUIEventAdapter(); 
     948                if (!event) continue; 
    947949                switch(event->getEventType()) 
    948950                { 
     
    982984            ++itr) 
    983985        { 
    984             osgGA::GUIEventAdapter* event = itr->get(); 
     986            osgGA::GUIEventAdapter* event = (*itr)->asGUIEventAdapter(); 
     987            if (!event) continue; 
    985988            switch(event->getEventType()) 
    986989            { 
     
    10101013            ++itr) 
    10111014        { 
    1012             osgGA::GUIEventAdapter* event = itr->get(); 
     1015            osgGA::GUIEventAdapter* event = (*itr)->asGUIEventAdapter(); 
     1016            if (!event) continue; 
    10131017 
    10141018            _eventVisitor->reset(); 
     
    10561060        ++itr) 
    10571061    { 
    1058         osgGA::GUIEventAdapter* event = itr->get(); 
    1059  
     1062        osgGA::Event* event = itr->get(); 
    10601063        for(EventHandlers::iterator hitr = _eventHandlers.begin(); 
    10611064            hitr != _eventHandlers.end(); 
    10621065            ++hitr) 
    10631066        { 
    1064             (*hitr)->handleWithCheckAgainstIgnoreHandledEventsMask( *event, *this, 0, _eventVisitor.get()); 
     1067            (*hitr)->handle( event, 0, _eventVisitor.get()); 
    10651068        } 
    10661069 
     
    10711074        ++itr) 
    10721075    { 
    1073         osgGA::GUIEventAdapter* event = itr->get(); 
    1074         if (_cameraManipulator.valid()) 
    1075         { 
    1076             _cameraManipulator->handleWithCheckAgainstIgnoreHandledEventsMask( *event, *this); 
     1076        osgGA::Event* event = itr->get(); 
     1077        if (event && _cameraManipulator.valid()) 
     1078        { 
     1079            _cameraManipulator->handle( event, 0, _eventVisitor.get()); 
    10771080        } 
    10781081    }