kicad-developers team mailing list archive
-
kicad-developers team
-
Mailing list archive
-
Message #17869
[PATCH] S3D Mesh Smart Pointer
Hi Guys,
Could someone who has access to both vrml1 and vrml2 models please
test the attached patch which uses a smart pointer rather than
S3D_MESH* to get rid of some coverity issues and feedback whether it
makes your computer explode, or carry on working normally?
Alternatively, could someone email a couple of vrml files that would
exercise it and I'll put it through it's paces...
Sorry, there's white space deletions going on in the patch so it's a bit noisy.
Thanks,
Brian.
=== modified file '3d-viewer/3d_mesh_model.cpp'
--- 3d-viewer/3d_mesh_model.cpp 2015-03-29 16:13:58 +0000
+++ 3d-viewer/3d_mesh_model.cpp 2015-04-01 21:58:33 +0000
@@ -68,13 +68,6 @@
S3D_MESH::~S3D_MESH()
{
- for( unsigned int idx = 0; idx < childs.size(); idx++ )
- {
- if( childs[idx] )
- {
- delete childs[idx];
- }
- }
}
@@ -82,7 +75,7 @@
{
if( !m_BBox.IsInitialized() )
calcBBoxAllChilds();
-
+
return m_BBox;
}
@@ -100,7 +93,7 @@
// Calc transformation matrix
glm::mat4 fullTransformMatrix;
glm::mat4 translationMatrix = glm::translate( glm::mat4(), m_translation );
-
+
if( m_rotation[3] != 0.0f )
{
glm::mat4 rotationMatrix = glm::rotate( translationMatrix, glm::radians( m_rotation[3] ),
@@ -110,7 +103,7 @@
else
fullTransformMatrix = glm::scale( translationMatrix, m_scale );
-
+
// Apply transformation
m_BBox.Set( S3D_VERTEX( fullTransformMatrix * glm::vec4( tmpBBox.Min(), 1.0f ) ),
S3D_VERTEX( fullTransformMatrix * glm::vec4( tmpBBox.Max(), 1.0f ) ) );
@@ -243,7 +236,7 @@
if ( m_MaterialIndex.size() > idx )
{
bool isTransparent = m_Materials->SetOpenGLMaterial( m_MaterialIndex[idx], useMaterial );
-
+
if( isTransparent && aIsRenderingJustNonTransparentObjects )
continue;
@@ -652,7 +645,7 @@
l,
(unsigned int)m_CoordIndex[idx].size()) );
*/
-
+
if( ( cross_prod.x > cross_prod.y ) && ( cross_prod.x > cross_prod.z ) )
{
cross_prod.x = 0.0f;
@@ -713,7 +706,7 @@
for( unsigned int each_face_A_idx = 0; each_face_A_idx < m_CoordIndex.size(); each_face_A_idx++ )
{
glm::vec3 initVertexFaceNormal = m_PerFaceNormalsRaw_X_PerFaceSquaredArea[each_face_A_idx];
-
+
std::vector< glm::vec3 >& face_A_normals = m_PerFaceVertexNormals[each_face_A_idx];
for( unsigned int each_vert_A_idx = 0; each_vert_A_idx < m_CoordIndex[each_face_A_idx].size(); each_vert_A_idx++ )
@@ -722,7 +715,7 @@
}
}
-
+
#ifdef USE_OPENMP
#pragma omp parallel for
#endif /* USE_OPENMP */
@@ -732,7 +725,7 @@
{
// n = face A facet normal
std::vector< glm::vec3 >& face_A_normals = m_PerFaceVertexNormals[each_face_A_idx];
-
+
// loop through all vertices
// for each vert in face A
for( unsigned int each_vert_A_idx = 0; each_vert_A_idx < m_CoordIndex[each_face_A_idx].size(); each_vert_A_idx++ )
=== modified file '3d-viewer/3d_mesh_model.h'
--- 3d-viewer/3d_mesh_model.h 2015-03-29 10:59:53 +0000
+++ 3d-viewer/3d_mesh_model.h 2015-04-10 22:50:03 +0000
@@ -24,12 +24,14 @@
/**
* @file 3d_mesh_model.h
- * @brief
+ * @brief
*/
#ifndef __3D_MESH_MODEL_H__
#define __3D_MESH_MODEL_H__
+#include <memory>
+#include <boost/shared_ptr.hpp>
#include <vector>
#define GLM_FORCE_RADIANS
#include <gal/opengl/glm/glm.hpp>
@@ -37,9 +39,14 @@
#include "3d_material.h"
#include "CBBox.h"
-
class S3D_MESH;
+/** A smart pointer to an S3D_MESH object */
+typedef boost::shared_ptr<S3D_MESH> S3D_MESH_PTR;
+
+/** A container of smar S3D_MESH object pointers */
+typedef std::vector<S3D_MESH_PTR> S3D_MESH_PTRS;
+
class S3D_MESH
{
public:
@@ -60,7 +67,7 @@
std::vector< S3D_VERTEX > m_PerFaceNormalsNormalized;
std::vector< S3D_VERTEX > m_PerVertexNormalsNormalized;
std::vector< int > m_MaterialIndex;
- std::vector< S3D_MESH * > childs;
+ S3D_MESH_PTRS childs;
S3D_VERTEX m_translation;
glm::vec4 m_rotation;
=== modified file '3d-viewer/modelparsers.h'
--- 3d-viewer/modelparsers.h 2015-03-29 16:13:58 +0000
+++ 3d-viewer/modelparsers.h 2015-04-01 23:08:49 +0000
@@ -30,6 +30,7 @@
#define MODELPARSERS_H
#include <map>
+#include <memory>
#include <vector>
#include <wx/string.h>
#include <3d_mesh_model.h>
@@ -71,11 +72,11 @@
* @param aFilename = the full file name of the file to load
* @return true if as succeeded
*/
- virtual bool Load( const wxString& aFilename ) {
+ virtual bool Load( const wxString& aFilename ) {
return false;
};
- std::vector< S3D_MESH* > childs;
+ S3D_MESH_PTRS childs;
private:
S3D_MASTER* master;
@@ -127,7 +128,7 @@
private:
wxString m_Filename;
- S3D_MESH* m_model;
+ S3D_MESH_PTR m_model;
std::vector< wxString > vrml_materials;
std::vector< wxString > vrml_points;
@@ -143,7 +144,7 @@
typedef std::map< std::string, std::vector< glm::vec3 > > VRML2_COORDINATE_MAP;
-typedef std::map< std::string, S3D_MESH* > VRML2_DEF_GROUP_MAP;
+typedef std::map< std::string, S3D_MESH_PTR > VRML2_DEF_GROUP_MAP;
/**
* class VRML2_MODEL_PARSER
@@ -164,7 +165,7 @@
* @param aTransformationModel a model with translation, rotation and scale to apply to default root
* @return bool - true if finnished with success
*/
- bool Load( const wxString& aFilename, S3D_MESH *aTransformationModel );
+ bool Load( const wxString& aFilename, S3D_MESH_PTR aTransformationModel );
/**
* Return string representing VRML2 file in vrml2 format
@@ -174,7 +175,7 @@
wxString VRML2_representation();
private:
- int loadFileModel( S3D_MESH *transformationModel );
+ int loadFileModel( S3D_MESH_PTR transformationModel );
int read_Transform();
int read_DEF();
int read_DEF_Coordinate();
@@ -211,7 +212,7 @@
bool m_normalPerVertex;
bool colorPerVertex;
- S3D_MESH* m_model; ///< It stores the current model that the parsing is adding data
+ S3D_MESH_PTR m_model; ///< It stores the current model that the parsing is adding data
FILE* m_file;
wxFileName m_Filename;
VRML2_COORDINATE_MAP m_defCoordinateMap;
@@ -266,7 +267,7 @@
bool m_normalPerVertex;
bool colorPerVertex;
- S3D_MESH* m_model;
+ S3D_MESH_PTR m_model;
FILE* m_file;
wxString m_Filename;
S3D_MODEL_PARSER* m_ModelParser;
=== modified file '3d-viewer/vrml_v1_modelparser.cpp'
--- 3d-viewer/vrml_v1_modelparser.cpp 2015-03-28 11:33:56 +0000
+++ 3d-viewer/vrml_v1_modelparser.cpp 2015-04-01 22:31:20 +0000
@@ -51,7 +51,7 @@
{
m_ModelParser = aModelParser;
m_Master = m_ModelParser->GetMaster();
- m_model = NULL;
+ m_model.reset();
m_file = NULL;
m_normalPerVertex = true;
colorPerVertex = true;
@@ -88,7 +88,7 @@
if( strcmp( text, "Separator" ) == 0 )
{
- m_model = new S3D_MESH();
+ m_model.reset( new S3D_MESH() );
m_ModelParser->childs.push_back( m_model );
read_separator();
}
@@ -122,18 +122,18 @@
}
else if( strcmp( text, "Separator" ) == 0 )
{
- S3D_MESH* parent = m_model;
+ S3D_MESH_PTR parent( m_model.get() );
- S3D_MESH* new_mesh_model = new S3D_MESH();
+ S3D_MESH_PTR new_mesh_model( new S3D_MESH() );
m_model->childs.push_back( new_mesh_model );
- m_model = new_mesh_model;
+ m_model.reset( new_mesh_model.get() );
// recursive
read_separator();
- m_model = parent;
+ m_model.reset( parent.get() );
}
else if( ( *text != '}' ) )
{
=== modified file '3d-viewer/vrml_v2_modelparser.cpp'
--- 3d-viewer/vrml_v2_modelparser.cpp 2015-04-02 11:18:19 +0000
+++ 3d-viewer/vrml_v2_modelparser.cpp 2015-04-09 21:16:53 +0000
@@ -54,7 +54,7 @@
{
m_ModelParser = aModelParser;
m_Master = m_ModelParser->GetMaster();
- m_model = NULL;
+ m_model.reset();
m_file = NULL;
m_normalPerVertex = true;
colorPerVertex = true;
@@ -98,7 +98,7 @@
// Switch the locale to standard C (needed to print floating point numbers)
LOCALE_IO toggle;
- loadFileModel( NULL );
+ loadFileModel( S3D_MESH_PTR() );
fclose( m_file );
@@ -107,7 +107,7 @@
}
-bool VRML2_MODEL_PARSER::Load( const wxString& aFilename, S3D_MESH *aTransformationModel )
+bool VRML2_MODEL_PARSER::Load( const wxString& aFilename, S3D_MESH_PTR aTransformationModel )
{
if( aTransformationModel )
{
@@ -141,7 +141,7 @@
}
-int VRML2_MODEL_PARSER::loadFileModel( S3D_MESH *aTransformationModel )
+int VRML2_MODEL_PARSER::loadFileModel( S3D_MESH_PTR aTransformationModel )
{
char text[BUFLINE_SIZE];
@@ -156,9 +156,9 @@
if( strcmp( text, "Transform" ) == 0 )
{
- m_model = new S3D_MESH();
+ m_model.reset( new S3D_MESH() );
- S3D_MESH* save_ptr = m_model;
+ S3D_MESH_PTR save_ptr( m_model );
if( read_Transform() == 0 )
{
@@ -167,12 +167,12 @@
if( ((m_model->m_Point.size() == 0) || (m_model->m_CoordIndex.size() == 0)) &&
(m_model->childs.size() == 0) )
{
- delete m_model;
+ m_model.reset();
wxLogTrace( traceVrmlV2Parser, m_debugSpacer + wxT( "loadFileModel: skipping model with no points or childs" ) );
}
else
{
- if( aTransformationModel )
+ if( aTransformationModel.get() != NULL )
{
m_model->m_translation = aTransformationModel->m_translation;
m_model->m_rotation = aTransformationModel->m_rotation;
@@ -187,14 +187,14 @@
}
else
{
- delete m_model;
+ m_model.reset();
}
}
else if( strcmp( text, "DEF" ) == 0 )
{
- m_model = new S3D_MESH();
+ m_model.reset( new S3D_MESH() );
- S3D_MESH* save_ptr = m_model;
+ S3D_MESH_PTR save_ptr( m_model );
if( read_DEF() == 0 )
{
@@ -203,12 +203,12 @@
if( ((m_model->m_Point.size() == 0) || (m_model->m_CoordIndex.size() == 0)) &&
(m_model->childs.size() == 0) )
{
- delete m_model;
+ m_model.reset();
wxLogTrace( traceVrmlV2Parser, m_debugSpacer + wxT( "loadFileModel: skipping model with no points or childs" ) );
}
else
{
- if( aTransformationModel )
+ if( aTransformationModel.get() != NULL )
{
m_model->m_translation = aTransformationModel->m_translation;
m_model->m_rotation = aTransformationModel->m_rotation;
@@ -223,14 +223,14 @@
}
else
{
- delete m_model;
+ m_model.reset();
}
}
else if( strcmp( text, "Shape" ) == 0 )
{
- m_model = new S3D_MESH();
+ m_model.reset( new S3D_MESH() );
- S3D_MESH* save_ptr = m_model;
+ S3D_MESH_PTR save_ptr = m_model;
if( read_Shape() == 0 )
{
@@ -239,12 +239,12 @@
if( ((m_model->m_Point.size() == 0) || (m_model->m_CoordIndex.size() == 0)) &&
(m_model->childs.size() == 0) )
{
- delete m_model;
+ m_model.reset();
wxLogTrace( traceVrmlV2Parser, m_debugSpacer + wxT( "loadFileModel: skipping model with no points or childs" ) );
}
else
{
- if( aTransformationModel )
+ if( aTransformationModel.get() != NULL )
{
m_model->m_translation = aTransformationModel->m_translation;
m_model->m_rotation = aTransformationModel->m_rotation;
@@ -261,7 +261,7 @@
}
else
{
- delete m_model;
+ m_model.reset();
}
}
}
@@ -279,7 +279,7 @@
for( VRML2_DEF_GROUP_MAP::iterator groupIt = m_defGroupMap.begin(); groupIt!=m_defGroupMap.end(); ++groupIt )
{
wxString groupName = groupIt->first;
- S3D_MESH* ptrModel = groupIt->second;
+ S3D_MESH_PTR ptrModel( groupIt->second );
if( ((ptrModel->m_Point.size() == 0) || (ptrModel->m_CoordIndex.size() == 0)) &&
@@ -326,9 +326,9 @@
if( strcmp( text, "Transform" ) == 0 )
{
- m_model = new S3D_MESH();
+ m_model.reset( new S3D_MESH() );
- S3D_MESH* save_ptr = m_model;
+ S3D_MESH_PTR save_ptr( m_model );
if( read_Transform() == 0 )
{
@@ -337,7 +337,7 @@
if( ((m_model->m_Point.size() == 0) || (m_model->m_CoordIndex.size() == 0)) &&
(m_model->childs.size() == 0) )
{
- delete m_model;
+ m_model.reset();
wxLogTrace( traceVrmlV2Parser, m_debugSpacer + wxT( "read_Transform: skipping model with no points or childs" ) );
}
else
@@ -352,7 +352,7 @@
}
else
{
- delete m_model;
+ m_model.reset();
}
}
else if( strcmp( text, "translation" ) == 0 )
@@ -463,14 +463,14 @@
else if( strcmp( text, "Shape" ) == 0 )
{
// Save the pointer
- S3D_MESH* parent = m_model;
+ S3D_MESH_PTR parent( m_model );
- S3D_MESH* new_mesh_model = new S3D_MESH();
+ S3D_MESH_PTR new_mesh_model( new S3D_MESH() );
// Assign the current pointer
m_model = new_mesh_model;
- S3D_MESH* save_ptr = m_model;
+ S3D_MESH_PTR save_ptr = m_model;
if( read_Shape() == 0 )
{
@@ -481,7 +481,7 @@
if( ((m_model->m_Point.size() == 0) || (m_model->m_CoordIndex.size() == 0)) &&
(m_model->childs.size() == 0) )
{
- delete m_model;
+ m_model.reset();
wxLogTrace( traceVrmlV2Parser, m_debugSpacer + wxT( "read_Transform: Shape, skipping model with no points or childs" ) );
}
else
@@ -496,14 +496,14 @@
}
else
{
- delete m_model;
+ m_model.reset();
wxLogTrace( traceVrmlV2Parser, m_debugSpacer + wxT( "read_Transform: Shape, discard child." ) );
}
}
else
{
- delete m_model;
+ m_model.reset();
}
m_model = parent;
@@ -540,7 +540,7 @@
return -1;
}
- S3D_MESH* ptrModel = groupIt->second;
+ S3D_MESH_PTR ptrModel( groupIt->second );
if( ((ptrModel->m_Point.size() == 0) || (ptrModel->m_CoordIndex.size() == 0)) &&
(ptrModel->childs.size() == 0) )
@@ -778,14 +778,14 @@
wxLogTrace( traceVrmlV2Parser, m_debugSpacer + wxT( "Shape" ) );
// Save the pointer
- S3D_MESH* parent = m_model;
+ S3D_MESH_PTR parent = m_model;
- S3D_MESH* new_mesh_model = new S3D_MESH();
+ S3D_MESH_PTR new_mesh_model( new S3D_MESH() );
// Assign the current pointer
m_model = new_mesh_model;
- S3D_MESH* save_ptr = m_model;
+ S3D_MESH_PTR save_ptr = m_model;
if( read_Shape() == 0 )
{
@@ -794,7 +794,7 @@
if( ((m_model->m_Point.size() == 0) || (m_model->m_CoordIndex.size() == 0)) &&
(m_model->childs.size() == 0) )
{
- delete m_model;
+ m_model.reset();
wxLogTrace( traceVrmlV2Parser, m_debugSpacer + wxT( "read_DEF: Shape, skipping model with no points or childs" ) );
}
else
@@ -809,7 +809,7 @@
}
else
{
- delete m_model;
+ m_model.reset();
}
m_model = parent;
@@ -824,9 +824,9 @@
wxLogTrace( traceVrmlV2Parser, m_debugSpacer + wxT( "Group %s" ), tagName );
// Save the pointer
- S3D_MESH* parent = m_model;
+ S3D_MESH_PTR parent = m_model;
- S3D_MESH* new_mesh_model = new S3D_MESH();
+ S3D_MESH_PTR new_mesh_model( new S3D_MESH() );
// Assign the current pointer
m_model = new_mesh_model;
@@ -848,7 +848,7 @@
}
else
{
- delete m_model;
+ m_model.reset();
}
// Restore current model pointer
=== modified file '3d-viewer/vrmlmodelparser.cpp'
--- 3d-viewer/vrmlmodelparser.cpp 2015-03-28 11:33:56 +0000
+++ 3d-viewer/vrmlmodelparser.cpp 2015-04-01 21:55:31 +0000
@@ -45,14 +45,6 @@
VRML_MODEL_PARSER::~VRML_MODEL_PARSER()
{
- for( unsigned int idx = 0; idx < childs.size(); idx++ )
- {
- if( childs[idx] )
- {
- delete childs[idx];
- childs[idx] = 0;
- }
- }
}
bool VRML_MODEL_PARSER::Load( const wxString& aFilename )
=== modified file '3d-viewer/x3dmodelparser.cpp'
--- 3d-viewer/x3dmodelparser.cpp 2015-03-28 11:33:56 +0000
+++ 3d-viewer/x3dmodelparser.cpp 2015-04-01 23:12:18 +0000
@@ -52,20 +52,12 @@
X3D_MODEL_PARSER::X3D_MODEL_PARSER( S3D_MASTER* aMaster ) :
S3D_MODEL_PARSER( aMaster )
{
- m_model = NULL;
+ m_model.reset();
}
X3D_MODEL_PARSER::~X3D_MODEL_PARSER()
{
- for( unsigned int idx = 0; idx < childs.size(); idx++ )
- {
- if( childs[idx] )
- {
- delete childs[idx];
- childs[idx] = 0;
- }
- }
}
@@ -102,7 +94,7 @@
node_it != transforms.end();
node_it++ )
{
- m_model = new S3D_MESH();
+ m_model.reset( new S3D_MESH() );
childs.push_back( m_model );
readTransform( *node_it );
Follow ups
