Show
Ignore:
Timestamp:
09/04/15 17:35:24 (6 hours 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/osgSim/ScalarBar.cpp

    r14047 r14059  
    135135    else 
    136136    { 
    137         matrix = osg::Matrix::rotate(osg::DegreesToRadians(90.0f),0.0f,0.0f,-1.0f) * osg::Matrix::translate(_position); 
     137        matrix = osg::Matrix::rotate(osg::DegreesToRadians(90.0f),0.0f,0.0f,1.0f) * osg::Matrix::translate(_position); 
    138138    } 
    139139 
     
    207207    float labelIncr = (_numLabels>0) ? (_stc->getMax()-_stc->getMin())/(_numLabels-1) : 0.0f; 
    208208    float labelxIncr = (_numLabels>0) ? (_width)/(_numLabels-1) : 0.0f; 
    209     const float labely = arOffset + characterSize*CHARACTER_OFFSET_FACTOR; 
     209    const float labelStickStartY = _orientation==HORIZONTAL ? arOffset : 0; 
     210    const float labelY = labelStickStartY + 
     211        (_orientation==HORIZONTAL ?  characterSize : -characterSize) * CHARACTER_OFFSET_FACTOR; 
     212 
    210213 
    211214    for(i=0; i<_numLabels; ++i) 
     
    218221        text->setText(_sp->printScalar(_stc->getMin()+(i*labelIncr))); 
    219222 
    220         text->setPosition(osg::Vec3((i*labelxIncr), labely, 0.0f)*matrix); 
     223        text->setPosition(osg::Vec3((i*labelxIncr), labelY, 0.0f)*matrix); 
    221224        text->setAlignment( (_orientation==HORIZONTAL) ? osgText::Text::CENTER_BASE_LINE : osgText::Text::LEFT_CENTER); 
    222225 
     
    241244        if ( _orientation==HORIZONTAL ) 
    242245        { 
    243             const float titleY = (_numLabels>0) ? labely + characterSize : labely; 
     246            const float titleY = (_numLabels>0) ? labelY + characterSize : labelY; 
    244247            titlePos = osg::Vec3((_width/2.0f), titleY, 0.0f); 
    245248        } 
    246249        else 
    247250        { 
    248             titlePos = osg::Vec3( 0, arOffset/2, 0 ); 
     251            titlePos = osg::Vec3(_width+characterSize*0.5, arOffset*0.5, 0 ); 
    249252        } 
    250  
    251         float titleY = (_numLabels>0) ? labely + characterSize : labely; 
    252253 
    253254        // Position the title at the middle of the bar above any labels. 
     
    280281    for(i=0; i<_numLabels; ++i) 
    281282    { 
    282         const osg::Vec3 p1(osg::Vec3((i*labelxIncr), arOffset, 0.0f)*matrix); 
    283         const osg::Vec3 p2(osg::Vec3((i*labelxIncr), labely, 0.0f)*matrix); 
     283        const osg::Vec3 p1(osg::Vec3((i*labelxIncr), labelStickStartY, 0.0f)*matrix); 
     284        const osg::Vec3 p2(osg::Vec3((i*labelxIncr), labelY, 0.0f)*matrix); 
    284285        annotVertices->push_back( p1 ); 
    285286        annotVertices->push_back( p2 );