Changeset 13436

Show
Ignore:
Timestamp:
09/04/15 17:35:24 (12 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.
"

Location:
OpenSceneGraph/trunk
Files:
2 modified

Legend:

Unmodified
Added
Removed
  • OpenSceneGraph/trunk/include/osg/Camera

    r13041 r13436  
    2525 
    2626#include <OpenThreads/Mutex> 
     27 
     28 
     29#if defined(OSG_GLES1_AVAILABLE) || defined(OSG_GLES2_AVAILABLE) && !defined(OSG_GL3_AVAILABLE) 
     30    #define GL_FRONT_LEFT       0x0400 
     31    #define GL_FRONT_RIGHT      0x0401 
     32    #define GL_BACK_LEFT        0x0402 
     33    #define GL_BACK_RIGHT       0x0403 
     34#endif 
    2735 
    2836namespace osg { 
  • OpenSceneGraph/trunk/src/osgUtil/SceneView.cpp

    r13373 r13436  
    3232using namespace osg; 
    3333using namespace osgUtil; 
    34  
    35 #if defined(OSG_GLES1_AVAILABLE) || defined(OSG_GLES2_AVAILABLE) && !defined(OSG_GL3_AVAILABLE) 
    36     // define under GLES to keep the main SceneView.cpp clean. 
    37     #define GL_BACK_LEFT        0x0402 
    38     #define GL_BACK_RIGHT       0x0403 
    39 #endif 
    4034 
    4135static const GLubyte patternVertEven[] = {