Simplify Data3DPointsData_t memory management

Adds a constructor & destructor that will allocate and deallocate memory based on fields set in a Data3D struct. Also changes the extension field names in PointStandardizedFieldsAvailable to be consistent with the others.
asmaloney · Oct 27, 2022 · 25387c6 · 25387c6
1 parent 15fb83c
commit 25387c6
Show file tree

Hide file tree

Showing 6 changed files with 209 additions and 59 deletions.
diff --git a/include/E57SimpleData.h b/include/E57SimpleData.h
@@ -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
@@ -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;
 
@@ -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>;

diff --git a/src/E57SimpleData.cpp b/src/E57SimpleData.cpp
@@ -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()
    {
@@ -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
diff --git a/src/ReaderImpl.cpp b/src/ReaderImpl.cpp
@@ -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;

diff --git a/src/WriterImpl.cpp b/src/WriterImpl.cpp
@@ -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;
@@ -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. ) );
       }

diff --git a/test/src/test_SimpleReader.cpp b/test/src/test_SimpleReader.cpp
@@ -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 );
 
@@ -109,10 +106,6 @@ TEST( SimpleReaderData, ReadBunnyDouble )
 
    EXPECT_EQ( cNumRead, cNumPoints );
 
-   delete[] pointsData.cartesianX;
-   delete[] pointsData.cartesianY;
-   delete[] pointsData.cartesianZ;
-
    delete reader;
 }
 
@@ -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 );
 
@@ -153,9 +143,5 @@ TEST( SimpleReaderData, ReadBunnyInt32 )
 
    EXPECT_EQ( cNumRead, cNumPoints );
 
-   delete[] pointsData.cartesianX;
-   delete[] pointsData.cartesianY;
-   delete[] pointsData.cartesianZ;
-
    delete reader;
 }