Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify Data3DPointsData_t memory management #149

Merged
merged 1 commit into from
Oct 27, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 19 additions & 4 deletions include/E57SimpleData.h
Original file line number Diff line number Diff line change
Expand Up @@ -375,9 +375,9 @@ namespace e57
bool colorBlueField = false; //!< Indicates that the PointRecord colorBlue field is active
bool isColorInvalidField = false; //!< Indicates that the PointRecord isColorInvalid field is active

bool normalX = false; //!< Indicates that the PointRecord nor:normalX field is active
bool normalY = false; //!< Indicates that the PointRecord nor:normalY field is active
bool normalZ = false; //!< Indicates that the PointRecord nor:normalZ field is active
bool normalXField = false; //!< Indicates that the PointRecord nor:normalX field is active
bool normalYField = false; //!< Indicates that the PointRecord nor:normalY field is active
bool normalZField = false; //!< Indicates that the PointRecord nor:normalZ field is active
};

//! @brief Stores the top-level information for a single lidar scan
Expand Down Expand Up @@ -442,10 +442,21 @@ namespace e57
};

//! @brief Stores pointers to user-provided buffers
template <typename COORDTYPE = float> struct Data3DPointsData_t
template <typename COORDTYPE = float> struct E57_DLL Data3DPointsData_t
{
static_assert( std::is_floating_point<COORDTYPE>::value, "Floating point type required." );

//! @brief Default constructor does not manage any memory
Data3DPointsData_t() = default;

//! @brief Constructor which allocates buffers for all valid fields in the given Data3D header
//! @param [in] data3D Completed header which indicates the fields wee are using
explicit Data3DPointsData_t( const e57::Data3D &data3D );

//! @brief Destructor will delete any memory allocated using the Data3DPointsData_t( const e57::Data3D & )
//! constructor
~Data3DPointsData_t();

//! Pointer to a buffer with the X coordinate (in meters) of the point in Cartesian coordinates
COORDTYPE *cartesianX = nullptr;

Expand Down Expand Up @@ -508,6 +519,10 @@ namespace e57
float *normalX = nullptr; //!< The X component of a surface normal vector (E57_EXT_surface_normals extension).
float *normalY = nullptr; //!< The Y component of a surface normal vector (E57_EXT_surface_normals extension).
float *normalZ = nullptr; //!< The Z component of a surface normal vector (E57_EXT_surface_normals extension).

private:
//! Keeps track of whether we used the Data3D constructor or not so we can free our memory.
bool _selfAllocated = false;
};

using Data3DPointsData = Data3DPointsData_t<float>;
Expand Down
177 changes: 176 additions & 1 deletion src/E57SimpleData.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,11 @@

#include "E57SimpleData.h"

#include "Common.h"
#include "StringFunctions.h"

namespace e57
{

// To avoid exposing M_PI, we define the constructor here.
SphericalBounds::SphericalBounds()
{
Expand All @@ -22,4 +24,177 @@ namespace e57
elevationMaximum = M_PI / 2.;
}

template <typename COORDTYPE>
Data3DPointsData_t<COORDTYPE>::Data3DPointsData_t( const Data3D &data3D ) : _selfAllocated( true )
{
const auto cSize = data3D.pointsSize;

if ( cSize < 1 )
{
throw E57_EXCEPTION2( E57_ERROR_VALUE_OUT_OF_BOUNDS, "pointsSize=" + toString( cSize ) + " minimum=1" );
}

if ( data3D.pointFields.cartesianXField )
{
cartesianX = new COORDTYPE[cSize];
}

if ( data3D.pointFields.cartesianYField )
{
cartesianY = new COORDTYPE[cSize];
}

if ( data3D.pointFields.cartesianZField )
{
cartesianZ = new COORDTYPE[cSize];
}

if ( data3D.pointFields.cartesianInvalidStateField )
{
cartesianInvalidState = new int8_t[cSize];
}

if ( data3D.pointFields.intensityField )
{
intensity = new float[cSize];
}

if ( data3D.pointFields.isIntensityInvalidField )
{
isIntensityInvalid = new int8_t[cSize];
}

if ( data3D.pointFields.colorRedField )
{
colorRed = new uint8_t[cSize];
}

if ( data3D.pointFields.colorGreenField )
{
colorGreen = new uint8_t[cSize];
}

if ( data3D.pointFields.colorBlueField )
{
colorBlue = new uint8_t[cSize];
}

if ( data3D.pointFields.isColorInvalidField )
{
isColorInvalid = new int8_t[cSize];
}

if ( data3D.pointFields.sphericalRangeField )
{
sphericalRange = new COORDTYPE[cSize];
}

if ( data3D.pointFields.sphericalAzimuthField )
{
sphericalAzimuth = new COORDTYPE[cSize];
}

if ( data3D.pointFields.sphericalElevationField )
{
sphericalElevation = new COORDTYPE[cSize];
}

if ( data3D.pointFields.sphericalInvalidStateField )
{
sphericalInvalidState = new int8_t[cSize];
}

if ( data3D.pointFields.rowIndexField )
{
rowIndex = new int32_t[cSize];
}

if ( data3D.pointFields.columnIndexField )
{
columnIndex = new int32_t[cSize];
}

if ( data3D.pointFields.returnIndexField )
{
returnIndex = new int8_t[cSize];
}

if ( data3D.pointFields.returnCountField )
{
returnCount = new int8_t[cSize];
}

if ( data3D.pointFields.timeStampField )
{
timeStamp = new double[cSize];
}

if ( data3D.pointFields.isTimeStampInvalidField )
{
isTimeStampInvalid = new int8_t[cSize];
}

if ( data3D.pointFields.normalXField )
{
normalX = new float[cSize];
}

if ( data3D.pointFields.normalYField )
{
normalY = new float[cSize];
}

if ( data3D.pointFields.normalZField )
{
normalZ = new float[cSize];
}
}

template <typename COORDTYPE> Data3DPointsData_t<COORDTYPE>::~Data3DPointsData_t()
{
if ( !_selfAllocated )
{
return;
}

delete[] cartesianX;
delete[] cartesianY;
delete[] cartesianZ;
delete[] cartesianInvalidState;

delete[] intensity;
delete[] isIntensityInvalid;

delete[] colorRed;
delete[] colorGreen;
delete[] colorBlue;
delete[] isColorInvalid;

delete[] sphericalRange;
delete[] sphericalAzimuth;
delete[] sphericalElevation;
delete[] sphericalInvalidState;

delete[] rowIndex;
delete[] columnIndex;

delete[] returnIndex;
delete[] returnCount;

delete[] timeStamp;
delete[] isTimeStampInvalid;

delete[] normalX;
delete[] normalY;
delete[] normalZ;

// Set them all to nullptr.
*this = Data3DPointsData_t<COORDTYPE>();
}

template Data3DPointsData_t<float>::Data3DPointsData_t( const Data3D &data3D );
template Data3DPointsData_t<double>::Data3DPointsData_t( const Data3D &data3D );

template Data3DPointsData_t<float>::~Data3DPointsData_t();
template Data3DPointsData_t<double>::~Data3DPointsData_t();
} // end namespace e57
6 changes: 3 additions & 3 deletions src/ReaderImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1108,9 +1108,9 @@ namespace e57
ustring norExtUri;
if ( imf_.extensionsLookupPrefix( "nor", norExtUri ) )
{
data3DHeader.pointFields.normalX = proto.isDefined( "nor:normalX" );
data3DHeader.pointFields.normalY = proto.isDefined( "nor:normalY" );
data3DHeader.pointFields.normalZ = proto.isDefined( "nor:normalZ" );
data3DHeader.pointFields.normalXField = proto.isDefined( "nor:normalX" );
data3DHeader.pointFields.normalYField = proto.isDefined( "nor:normalY" );
data3DHeader.pointFields.normalZField = proto.isDefined( "nor:normalZ" );
}

return true;
Expand Down
9 changes: 5 additions & 4 deletions src/WriterImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -910,7 +910,8 @@ namespace e57

// E57_EXT_surface_normals
// See: http://www.libe57.org/E57_EXT_surface_normals.txt
if ( data3DHeader.pointFields.normalX || data3DHeader.pointFields.normalY || data3DHeader.pointFields.normalZ )
if ( data3DHeader.pointFields.normalXField || data3DHeader.pointFields.normalYField ||
data3DHeader.pointFields.normalZField )
{
// make sure we declare the extension before using the fields with prefix
ustring norExtUri;
Expand All @@ -921,15 +922,15 @@ namespace e57
}

// currently we support writing normals only as float32
if ( data3DHeader.pointFields.normalX )
if ( data3DHeader.pointFields.normalXField )
{
proto.set( "nor:normalX", FloatNode( imf_, 0., E57_SINGLE, -1., 1. ) );
}
if ( data3DHeader.pointFields.normalY )
if ( data3DHeader.pointFields.normalYField )
{
proto.set( "nor:normalY", FloatNode( imf_, 0., E57_SINGLE, -1., 1. ) );
}
if ( data3DHeader.pointFields.normalZ )
if ( data3DHeader.pointFields.normalZField )
{
proto.set( "nor:normalZ", FloatNode( imf_, 0., E57_SINGLE, -1., 1. ) );
}
Expand Down
18 changes: 2 additions & 16 deletions test/src/test_SimpleReader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -96,10 +96,7 @@ TEST( SimpleReaderData, ReadBunnyDouble )

const uint64_t cNumPoints = data3DHeader.pointsSize;

e57::Data3DPointsData pointsData;
pointsData.cartesianX = new float[cNumPoints];
pointsData.cartesianY = new float[cNumPoints];
pointsData.cartesianZ = new float[cNumPoints];
e57::Data3DPointsData pointsData( data3DHeader );

auto vectorReader = reader->SetUpData3DPointsData( 0, cNumPoints, pointsData );

Expand All @@ -109,10 +106,6 @@ TEST( SimpleReaderData, ReadBunnyDouble )

EXPECT_EQ( cNumRead, cNumPoints );

delete[] pointsData.cartesianX;
delete[] pointsData.cartesianY;
delete[] pointsData.cartesianZ;

delete reader;
}

Expand Down Expand Up @@ -140,10 +133,7 @@ TEST( SimpleReaderData, ReadBunnyInt32 )

const uint64_t cNumPoints = data3DHeader.pointsSize;

e57::Data3DPointsData pointsData;
pointsData.cartesianX = new float[cNumPoints];
pointsData.cartesianY = new float[cNumPoints];
pointsData.cartesianZ = new float[cNumPoints];
e57::Data3DPointsData pointsData( data3DHeader );

auto vectorReader = reader->SetUpData3DPointsData( 0, cNumPoints, pointsData );

Expand All @@ -153,9 +143,5 @@ TEST( SimpleReaderData, ReadBunnyInt32 )

EXPECT_EQ( cNumRead, cNumPoints );

delete[] pointsData.cartesianX;
delete[] pointsData.cartesianY;
delete[] pointsData.cartesianZ;

delete reader;
}
Loading