search

ros-drivers / ros2_ouster_drivers

ROS2 Drivers for the Ouster OS-0, OS-1, and OS-2 Lidars
https://ouster.com/
Apache License 2.0
139 stars 79 forks source link

Reorganizes in-memory data layout of point cloud #53

Closed tpanzarella closed 4 years ago

tpanzarella commented 4 years ago

This PR reorganizes the in-memory data layout of the point cloud to be consistent with the LiDAR geometry and in row-major order.

An analysis of this reorganization to include its motivation and validating the fix is available here.

An analysis of the performance implications (which are minimal) is available here.

Closes: #52

SteveMacenski commented 4 years ago

I see why you said that there's some overhead. Why not create a new version of the batch iteration function to input them in another order rather than reordering after the fact? If you have the data, you know the azimuth and elevation, and probably can get the operation mode. In that case, you should know the exact position on the pointcloud each column needs to go to. Under the hood a pcl::PointCloud<PointT> is just a std::vector so there's some really good caching even between columns as long as they're reasonably distanced.

tpanzarella commented 4 years ago

Without a test suite, I cannot guarantee correctness of the rest of the code base. I believe there are utility functions that make assumptions about the data layout that are used by other data processors. (I think). We only consume the point cloud, and I can validate that for correctness.

Also, in the PCL to ROS conversion, there was already an implicit copy, so, the only additional overhead here is the reordering.