Repository navigation
Conversation
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
approved these changes
Oct 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #6499.
std::numeric_limits<T>::min()is the smallest positive value forfloat/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 tolowest():2d/include/pcl/2d/impl/keypoint.hpp(hessianBlob,SCALED): the cornerness can be negative, somaxstayed atmin()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_indexand themaxy/maxzreductions start frommin(), 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 atFLT_MINinstead 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(equaleps),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(intsentinel),outofcore/src/cJSON.cpp,apps/src/openni_grab_images.cppandtools/openni_image.cpp(unsigned short),gpu/people/.../NCVPixelOperations.hppinteger specializations: integral types, wheremin()is the lowest value.common/distances.hgetMaxSegment(+ 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.hppmulti-scalehessianBlob,scale_max: withlowest()every non-positive local maximum would be reported as a blob, somin()currently acts as a "response > 0" threshold. Changing it would change the output.recognition/src/cg/hough_3d.cppfindMaxima: votes are non-negative, and withlowest()an empty Hough space would give a threshold of 0 and report every bin as a maximum, so I keptmin().recognition/impl/hv/hv_go.hpp: the values are non-negative distances, so the result doesn't change.visualization/impl/pcl_visualizer.hppscalar range[min(), max()]and themin_valuedefaults invisualization/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 belowest(), but I couldn't check how the NCV code uses it.Tests: built
common,2d,features(and their deps) on Linux with GCC and rantest_2d,test_2d_keypoint_instantiation_{with,without}_precompile,test_centroidandtest_moment_of_inertia_estimation; all pass. I could not build the modeler app (Qt/VTK) or the CUDA code here; those changes only swapmin()forlowest()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.