Markus Vieth

Results 390 comments of Markus Vieth
trafficstars

So maybe something like this? ```CMake set(EXT_DEPS flann) if(PCL_NO_PRECOMPILE) add_library(${LIB_NAME} INTERFACE) target_link_libraries("${LIB_NAME}" INTERFACE pcl_common FLANN::FLANN) PCL_MAKE_PKGCONFIG(${LIB_NAME} COMPONENT ${SUBSYS_NAME} DESC ${SUBSYS_DESC} PCL_DEPS ${SUBSYS_DEPS} EXT_DEPS ${EXT_DEPS} HEADER_ONLY) else() PCL_ADD_LIBRARY(${LIB_NAME} COMPONENT ${SUBSYS_NAME} SOURCES...

You are welcome to open a pull request to add new features to PCL. Note however, that simply adding that new entry to model_types.h is not enough, you also have...

Yes, using `setModelConstraints` with `SACSegmentation` or `SACSegmentationFromNormals` is currently not easy. But it is when using [this other way for sample consensus](https://pcl.readthedocs.io/projects/tutorials/en/master/random_sample_consensus.html).

Yes, but that is well documented, see for example [here](https://pointclouds.org/documentation/classpcl_1_1_sample_consensus_model_normal_parallel_plane.html): `... In addition, the plane normal must lie parallel to a user-specified axis. This means that the plane itself will...

> i dont know what a beautiful solution would be, maybe adding SAC_MODEL_NORMAL_PARALLEL2 and making/marking SAC_MODEL_NORMAL_PARALLEL as depricated and remove in future releses and make the naming consistent.... That would...

> Would it be possible to explain me from which math calculations are calculations of coefficients createPlane coming from? For example for a plane given by the equation a x...

I wouldn't change all `assert`s to `if`s and `PCL_ERROR`. In functions that are called often, I would keep the `assert` for performance reasons. E.g. in `addPointsFromInputCloud` and `addPointIdx`, I would...

> @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...

> 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...

I still found instances of `std::copy` changed to `std::copy_n` in - [apps/point_cloud_editor/src/common.cpp](https://github.com/PointCloudLibrary/pcl/pull/4299/files#diff-0c49354954886e2d46af2c15d882478ffa588cd1d11929b682a7375b84c2b99c) - [apps/point_cloud_editor/src/selectionTransformTool.cpp](https://github.com/PointCloudLibrary/pcl/pull/4299/files#diff-ee71ba8209222b6dee70797c58572473a3a7d7f6216f5191b742cf8a90d285b2) - [apps/point_cloud_editor/src/transformCommand.cpp](https://github.com/PointCloudLibrary/pcl/pull/4299/files#diff-e127d8b085c0f9e9bbf9c7f10633b1ef87051cecb75c1b77d27bf0cd1ef06a77) - [doc/tutorials/content/sources/gpu/people_detect/src/people_detect.cpp](https://github.com/PointCloudLibrary/pcl/pull/4299/files#diff-cb56112c6099325573da0fced8c3497efac66d6907add05975e4b05c4a76264f) - [features/include/pcl/features/impl/gasd.hpp](https://github.com/PointCloudLibrary/pcl/pull/4299/files#diff-0fe88e460337462d3127b8589f41c76e80ae0cb3c3237e1ace1435954fe840b1) - [gpu/kinfu/src/kinfu.cpp](https://github.com/PointCloudLibrary/pcl/pull/4299/files#diff-f5ef9271b8c6d349962c47113148422ce919c15e5cd4c23f2fe39023e3b924c1) - [gpu/kinfu_large_scale/src/kinfu.cpp](https://github.com/PointCloudLibrary/pcl/pull/4299/files#diff-d049fd1d14f932b3519964b7fa5f7ba3be303161353b587288e1a9d6506619f6) - [gpu/people/tools/people_app.cpp](https://github.com/PointCloudLibrary/pcl/pull/4299/files#diff-271d6ef1b7c26e2aec19ad6f4168117cf377d3c3e10ca61d12401347a7a74828) - [gpu/utils/src/repacks.cu](https://github.com/PointCloudLibrary/pcl/pull/4299/files#diff-62421c25b834115b8f4f0197bc463e45c1c312f69da27907f45b044255e539d8) - [io/src/libpng_wrapper.cpp](https://github.com/PointCloudLibrary/pcl/pull/4299/files#diff-a0b1dec7027b06856377c5db7f3efe0c597fd7f59e50f39eacc22adcbef976be)...