Skip to content

Use numeric_limits::lowest() instead of min() to seed running maxima - #6500

Merged
mvieth merged 1 commit into
PointCloudLibrary:masterfrom
ump45nose:fix/numeric-limits-lowest-max-seeds
Oct 21, 2026
Merged

mvieth merged 1 commit into
PointCloudLibrary:masterfrom
ump45nose:fix/numeric-limits-lowest-max-seeds

Conversation

@ump45nose

Copy link
Copy Markdown
Contributor

Part of #6499.

std::numeric_limits<T>::min() is the smallest positive value for float/double, so a running maximum seeded with it can never end up below zero. I went through the usages listed in the issue and changed the ones that seed a maximum over values that can be negative or zero to lowest():

  • 2d/include/pcl/2d/impl/keypoint.hpp (hessianBlob, SCALED): the cornerness can be negative, so max stayed at min() when all values were negative and the scaling was wrong.
  • apps/modeler/src/{icp_registration,normal_estimation,voxel_grid_downsample}_worker.cpp: x_max_/y_max_/z_max_ are the bounding box of the selected clouds; for a cloud with only negative coordinates the max stayed at ~0, so the ranges used to set up the parameter dialogs were wrong.
  • gpu/surface/src/cuda/convex_hull.cu: transform_reduce_max_index and the maxy/maxz reductions start from min(), so they return ~0 when all coordinates are negative.
  • features/include/pcl/features/impl/moment_of_inertia_estimation.hpp: OBB max seed. The projections are relative to the mean, so this only differs for degenerate clouds (e.g. all points identical, where the max stayed at FLT_MIN instead of 0).
  • common/include/pcl/common/impl/centroid.hpp (computeCentroidAndOBB, both overloads): the seed is overwritten by the first (finite) point, so no behavior change; changed for consistency with the min seed next to it.

Checked and left unchanged, because min() is used as a small positive threshold, an integer limit or an API sentinel:

  • common/impl/eigen.hpp, cuda/common/eigen.h, gpu/features/device/eigen.hpp, gpu/kinfu*/src/cuda/utils.hpp: scale <= min() division guards.
  • common/utils.h (equal eps), surface/organized_fast_mesh.h (> min() checks), test/common/test_type_traits.cpp (a tiny finite value on purpose).
  • common/src/parse.cpp, range_image (int sentinel), outofcore/src/cJSON.cpp, apps/src/openni_grab_images.cpp and tools/openni_image.cpp (unsigned short), gpu/people/.../NCVPixelOperations.hpp integer specializations: integral types, where min() is the lowest value.
  • common/distances.h getMaxSegment (+ its test and the stick_segmentation tutorial): min() is the documented return value for "no segment", and distances are non-negative.

Not changed, but someone may want to look at them:

  • keypoint.hpp multi-scale hessianBlob, scale_max: with lowest() every non-positive local maximum would be reported as a blob, so min() currently acts as a "response > 0" threshold. Changing it would change the output.
  • recognition/src/cg/hough_3d.cpp findMaxima: votes are non-negative, and with lowest() an empty Hough space would give a threshold of 0 and report every bin as a maximum, so I kept min().
  • recognition/impl/hv/hv_go.hpp: the values are non-negative distances, so the result doesn't change.
  • visualization/impl/pcl_visualizer.hpp scalar range [min(), max()] and the min_value defaults in visualization/image_viewer.h / tools/image_viewer.cpp: these are display ranges, and [lowest(), max()] would overflow the range width.
  • gpu/people/.../NCVPixelOperations.hpp _pixMinVal<Ncv32f/Ncv64f>: probably should be lowest(), but I couldn't check how the NCV code uses it.

Tests: built common, 2d, features (and their deps) on Linux with GCC and ran test_2d, test_2d_keypoint_instantiation_{with,without}_precompile, test_centroid and test_moment_of_inertia_estimation; all pass. I could not build the modeler app (Qt/VTK) or the CUDA code here; those changes only swap min() for lowest() in the initial values.

I used an AI assistant (Grok Bot) to help find and review these usages and to write this description; I checked the changes and ran the tests listed above.

std::numeric_limits<T>::min() is the smallest positive value for floating
point types, so a running maximum seeded with it never drops below zero.
Use lowest() in the places that compute a maximum of values that can be
negative or zero.

Part of PointCloudLibrary#6499.
@mvieth
mvieth merged commit efae3e8 into PointCloudLibrary:master Oct 21, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants