Ancient signiture loss forces users to use pointer arrays

[sariug]

Hello.
Currently the new SetUpdata3DPointsData, forces people to use Data3DPointsData which is less safe for the users. The ancient call was proving the entry points for each data. Simply, I was happy by passing data.x.data() to vector. Cant we just bring that back ?

   CompressedVectorReader Reader::SetUpData3DPointsData( int64_t dataIndex, size_t pointCount,
                                                         const Data3DPointsData_d &buffers ) const;

virtual CompressedVectorReader	SetUpData3DPointsData(
						int32_t		dataIndex,			//!< data block index given by the NewData3D
						int64_t		pointCount,			//!< size of each element buffer.
						double*		cartesianX,			//!< pointer to a buffer with the X coordinate (in meters) of the point in Cartesian coordinates
						double*		cartesianY,			//!< pointer to a buffer with the Y coordinate (in meters) of the point in Cartesian coordinates
						double*		cartesianZ,			//!< pointer to a buffer with the Z coordinate (in meters) of the point in Cartesian coordinates
						int8_t*		cartesianInvalidState = NULL,	//!< Value = 0 if the point is considered valid, 1 otherwise

						double*		intensity = NULL,	//!< pointer to a buffer with the Point response intensity. Unit is unspecified
						int8_t*		isIntensityInvalid = NULL,	//!< Value = 0 if the intensity is considered valid, 1 otherwise

						uint16_t*	colorRed = NULL,	//!< pointer to a buffer with the Red color coefficient. Unit is unspecified
						uint16_t*	colorGreen = NULL,	//!< pointer to a buffer with the Green color coefficient. Unit is unspecified
						uint16_t*	colorBlue = NULL,	//!< pointer to a buffer with the Blue color coefficient. Unit is unspecified
						int8_t*		isColorInvalid = NULL,	//!< Value = 0 if the color is considered valid, 1 otherwise

						double*		sphericalRange = NULL,		//!< pointer to a buffer with the range (in meters) of points in spherical coordinates. Shall be non-negative
						double*		sphericalAzimuth = NULL,	//!< pointer to a buffer with the Azimuth angle (in radians) of point in spherical coordinates
						double*		sphericalElevation = NULL,	//!< pointer to a buffer with the Elevation angle (in radians) of point in spherical coordinates
						int8_t*		sphericalInvalidState = NULL, //!< Value = 0 if the range is considered valid, 1 otherwise

						int32_t*	rowIndex = NULL,	//!< pointer to a buffer with the row number of point (zero based). This is useful for data that is stored in a regular grid.Shall be in the interval (0, 2^63).
						int32_t*	columnIndex = NULL,	//!< pointer to a buffer with the column number of point (zero based). This is useful for data that is stored in a regular grid. Shall be in the interval (0, 2^63).
						int8_t*		returnIndex = NULL,	//!< pointer to a buffer with the number of this return (zero based). That is, 0 is the first return, 1 is the second, and so on. Shall be in the interval (0, returnCount). Only for multi-return sensors. 
						int8_t*		returnCount = NULL,	//!< pointer to a buffer with the total number of returns for the pulse that this corresponds to. Shall be in the interval (0, 2^63). Only for multi-return sensors. 

						double*		timeStamp = NULL,	//!< pointer to a buffer with the time (in seconds) since the start time for the data, which is given by acquisitionStart in the parent Data3D Structure. Shall be non-negative
						int8_t*		isTimeStampInvalid = NULL	//!< Value = 0 if the timeStamp is considered valid, 1 otherwise
						);

[asmaloney]

Hi @sariug.

I don't understand what you mean by the current API being "less safe for the users" or how this forces users to use pointer arrays.

I find the "old API" you present here to be poor style and error-prone and would rather not expand the API with overloads, but maybe you could explain some more about what problem you're seeing?

I don't use this part of the API, so maybe I'm missing some detail of the issue you're raising.

[sariug]

Hello.

Firstly thank you for the quick answer.

In order to engage with the current code, one "must" have Data3DPointsData_d &buffers nothing but a struct of double* etc. Therefore, heap reserving by new seems to be still necessary for this.

And in the end, one shall again delete the buffers explicitly.

Please correct if anything is wrong up to here.

However, with structs such as vector, one can actually pass the vector for pointer array, thus, not so need to worry about deleting. Furthermore, one shall not may have smaller structs.

Example:
I am writing a code just to collect z values. Such small structures to skip unnecessary data seems not available to me(except of cource directly working with raw data and nodes.

struct compZ
{
 std::vector<float>z:
};

...
eReader->SetUpData3DPointsData(dataIndex,pointCount,	nullptr,nullptr, myComp.z.data(),nullptr... /* and so on.*/);
...

Please correct me if I have a fundamental wrong though.

[asmaloney]

   auto pointCount = myComp.z.size();
   
   e57::Data3DPointsData   buffers;
   
   buffers.cartesianZ = myComp.z.data();

   eReader->SetUpData3DPointsData( dataIndex, pointCount, buffers );

Data3DPointsData is really just bundling all those parameters into a struct. It's actually safer and (I would argue) makes it easier to use.

[sariug]

So the way to go is a temp buffer object with passing pointers around :)

[asmaloney]

It sounds like I still don't understand your objection. If I'm still missing something, please reopen and clarify.