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/osgQt/CMakeLists.txt

    r12292 r13482  
    1313) 
    1414 
    15 QT4_WRAP_CPP( SOURCES_H_MOC ${SOURCES_H} OPTIONS "-f" ) 
     15IF ( Qt5Widgets_FOUND ) 
     16    QT5_WRAP_CPP( SOURCES_H_MOC ${SOURCES_H} OPTIONS "-f" ) 
     17ELSE() 
     18    QT4_WRAP_CPP( SOURCES_H_MOC ${SOURCES_H} OPTIONS "-f" ) 
     19ENDIF() 
    1620 
    1721SET(TARGET_H 
     
    4852ENDIF() 
    4953 
    50 SET(TARGET_LIBRARIES 
     54# FIXME: This should work but something with the LINK_WITH_VARIABLES macro is not working 
     55#SET(TARGET_LIBRARIES_VARS 
     56#    QT_QTCORE_LIBRARY  
     57#    QT_QTGUI_LIBRARY 
     58#    QT_QTOPENGL_LIBRARY 
     59#) 
     60 
     61IF( QT4_FOUND ) 
     62    INCLUDE_DIRECTORIES( ${QT_INCLUDE_DIR} ${QT_QTCORE_INCLUDE_DIR} ${QT_QTGUI_INCLUDE_DIR} 
     63             ${QT_QTOPENGL_INCLUDE_DIR} ) 
     64    SET(TARGET_LIBRARIES 
    5165    ${TARGET_LIBRARIES} 
    5266    ${QT_QTCORE_LIBRARY}  
     
    5569) 
    5670 
    57 # FIXME: This should work but something with the LINK_WITH_VARIABLES macro is not working 
    58 #SET(TARGET_LIBRARIES_VARS 
    59 #    QT_QTCORE_LIBRARY  
    60 #    QT_QTGUI_LIBRARY 
    61 #    QT_QTOPENGL_LIBRARY 
    62 #) 
    63 INCLUDE_DIRECTORIES(${QT_INCLUDE_DIR} ${QT_QTCORE_INCLUDE_DIR}) 
     71ENDIF( QT4_FOUND ) 
    6472 
    6573SETUP_LIBRARY(${LIB_NAME}) 
    6674 
     75IF ( Qt5Widgets_FOUND ) 
     76    qt5_use_modules( ${LIB_NAME} Widgets OpenGL ) 
     77ENDIF ( Qt5Widgets_FOUND )