Changed memcpy/memset for non-POD objects [PCL-3368]

[gnawme]

Per C++ Coding Standards item 96:

memcpy and memcmp violate the type system. Using memcpy to copy objects is like making money using a photocopier. Using memcmp to compare objects is like comparing leopards by counting their spots. The tools and methods might appear to do the job, but they are too coarse to do it acceptably.

memset is also considered harmful.

For this PR:

• For the most part, memset was replaced by std::fill_n or, in many cases, by zero initialization

• There are many, many instances of memcpy in PCL that are used to copy various kinds of Point data as uint8_t to float; I left these alone.

• Otherwise, where possible, memcpy was replaced by std::copy or std::copy_n

• The only instances of memcmp (outside of third-party code) were in CUDA code; I left those alone.

#3368

[kunaltyagi]

PS: Builds are failing (Ignore MSVC for a while)

[gnawme]

PS: Builds are failing (Ignore MSVC for a while)

Yes, unit tests are failing in CovarianceSampling.Filters. So far, I'm only able to resolve it to something in Eigen.

[kunaltyagi]

Hey, sorry but I've not seen the past few updates due to GSoC rush. I'm marking this as "needs more work", but not marking this as draft so you can use the CI to iron out the issues. Please ping me once the CI is green. (PS: windows CI sometimes goes Out of Memory, ignore those errors)

[gnawme]

Hey, sorry but I've not seen the past few updates due to GSoC rush. I'm marking this as "needs more work", but not marking this as draft so you can use the CI to iron out the issues. Please ping me once the CI is green. (PS: windows CI sometimes goes Out of Memory, ignore those errors)

No worries, I finally figured out how to run the tests in my IDE. Plowing through the failures now.

[gnawme]

@kunaltyagi Do you know how to debug sections under #pragma omp parallel?

I'm down to one failing unit test (test_poisson). It's failing because in surface/include/pcl/surface/3rdparty/poisson4/multi_grid_octree_data.hpp, between lines 2843 and 2841, isoValue is being calculated as nan.

[kunaltyagi]

Tagging @SunBlack coz of the magic touch

I'm sorry, but I haven't seen enough of surface to comment something specific.

If the error is present with just 1 thread, something fundamental is wrong about the implementation. If the error is present with more than 1 thread, I'd look for incorrectly used critical sections and shared variables

[gnawme]

Tagging @SunBlack coz of the magic touch

I'm sorry, but I haven't seen enough of surface to comment something specific.

If the error is present with just 1 thread, something fundamental is wrong about the implementation. If the error is present with more than 1 thread, I'd look for incorrectly used critical sections and shared variables

@kunaltyagi Update (this PR is dragging on, I know): test_poisson passes in my IDE, but not on the command line. Kinda scratching my head.

[gnawme]

Hey, sorry but I've not seen the past few updates due to GSoC rush. I'm marking this as "needs more work", but not marking this as draft so you can use the CI to iron out the issues. Please ping me once the CI is green. (PS: windows CI sometimes goes Out of Memory, ignore those errors)

@kunaltyagi CI is green on this PR

[gnawme]

@mvieth Kunal Tyagi completely reviewed this PR, and was waiting for unit testing to be green before approving it. Would you be comfortable approving it?

[mvieth]

@mvieth Kunal Tyagi completely reviewed this PR, and was waiting for unit testing to be green before approving it. Would you be comfortable approving it?

I will review again and then approve it, but might take a bit

[mvieth]

Maybe could make the std::array chnges also in another MR - just saw in this MR, that we could modernize them here.

I would prefer to not make those changes here since this is already quite big and dragging on for two years :slightly_smiling_face:

[mvieth]

I still found instances of std::copy changed to std::copy_n in

• apps/point_cloud_editor/src/common.cpp
• apps/point_cloud_editor/src/selectionTransformTool.cpp
• apps/point_cloud_editor/src/transformCommand.cpp
• doc/tutorials/content/sources/gpu/people_detect/src/people_detect.cpp
• features/include/pcl/features/impl/gasd.hpp
• gpu/kinfu/src/kinfu.cpp
• gpu/kinfu_large_scale/src/kinfu.cpp
• gpu/people/tools/people_app.cpp
• gpu/utils/src/repacks.cu
• io/src/libpng_wrapper.cpp
• visualization/include/pcl/visualization/impl/pcl_visualizer.hpp
• visualization/src/pcl_visualizer.cpp

And pcl:: removed in

• filters/src/project_inliers.cpp
• registration/include/pcl/registration/impl/icp.hpp
• registration/include/pcl/registration/impl/joint_icp.hpp
• surface/include/pcl/surface/impl/concave_hull.hpp
• surface/include/pcl/surface/impl/convex_hull.hpp
• surface/include/pcl/surface/impl/marching_cubes.hpp

Please check again and undo.

[gnawme]

I still found instances of std::copy changed to std::copy_n

Sorry, I didn't realize that I had a filter set on my Find in Files...

[gnawme]

Please check and undo if you agree

I retained std::copy if it was like-type to like-type, otherwise I reverted