Show
Ignore:
Timestamp:
09/04/15 17:35:24 (8 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/examples/osganimationviewer/AnimtkViewer.cpp

    r12292 r13576  
    1 /*  -*-c++-*-  
     1/*  -*-c++-*- 
    22 *  Copyright (C) 2008 Cedric Pinson <mornifle@plopbyte.net> 
    33 * 
    4  * This library is open source and may be redistributed and/or modified under   
    5  * the terms of the OpenSceneGraph Public License (OSGPL) version 0.0 or  
     4 * This library is open source and may be redistributed and/or modified under 
     5 * the terms of the OpenSceneGraph Public License (OSGPL) version 0.0 or 
    66 * (at your option) any later version.  The full license is in LICENSE file 
    77 * included with this distribution, and on the openscenegraph.org website. 
    8  *  
     8 * 
    99 * This library is distributed in the hope that it will be useful, 
    1010 * but WITHOUT ANY WARRANTY; without even the implied warranty of 
    11  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the  
     11 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the 
    1212 * OpenSceneGraph Public License for more details. 
    1313 * 
     
    3838 
    3939 
    40 osg::Geode* createAxis()  
     40osg::Geode* createAxis() 
    4141{ 
    42     osg::Geode*     geode    = new osg::Geode();   
     42    osg::Geode*     geode    = new osg::Geode(); 
    4343    osg::Geometry*  geometry = new osg::Geometry(); 
    4444    osg::Vec3Array* vertices = new osg::Vec3Array(); 
    4545    osg::Vec4Array* colors   = new osg::Vec4Array(); 
    46      
     46 
    4747    vertices->push_back(osg::Vec3(0.0f, 0.0f, 0.0f)); 
    4848    vertices->push_back(osg::Vec3(1.0f, 0.0f, 0.0f)); 
     
    5858    colors->push_back(osg::Vec4(0.0f, 0.0f, 1.0f, 1.0f)); 
    5959    colors->push_back(osg::Vec4(0.0f, 0.0f, 1.0f, 1.0f)); 
    60      
     60 
    6161    geometry->setVertexArray(vertices); 
    62     geometry->setColorArray(colors); 
    63     geometry->setColorBinding(osg::Geometry::BIND_PER_VERTEX);     
     62    geometry->setColorArray(colors, osg::Array::BIND_PER_VERTEX); 
    6463    geometry->addPrimitiveSet(new osg::DrawArrays(osg::PrimitiveSet::LINES, 0, 6)); 
    6564    geometry->getOrCreateStateSet()->setMode(GL_LIGHTING, false); 
     
    101100}; 
    102101 
    103 int main(int argc, char** argv)  
     102int main(int argc, char** argv) 
    104103{ 
    105104    osg::ArgumentParser arguments(&argc, argv); 
     
    154153    AnimtkViewerGUI* gui    = new AnimtkViewerGUI(&viewer, WIDTH, HEIGHT, 0x1234); 
    155154    osg::Camera*     camera = gui->createParentOrthoCamera(); 
    156      
     155 
    157156    node->setNodeMask(0x0001); 
    158157