Commit Graph
2 Commits
Author SHA1 Message Date
CodeCraftsman 5020ed3c55 Merge pull request #29714 from Thebinary110:fix-convexhull-type-punning
geometry: fix strict-aliasing UB in convexHull - #29714

Fixes #26952.

### What
`convexHull()` allocated a single `AutoBuffer<Point*>` and obtained a `Point2f**` view of the *same storage* via `reinterpret_cast` to share the sort/`Sklansky_` code between the `CV_32S` and `CV_32F` cases, and unconditionally read the input via `points.ptr<Point>()` regardless of the Mat's actual depth. Both are strict-aliasing violations -- the exact mechanism the issue title describes ("bogus C-cast and illegal assumptions of object layout") -- and are undefined behavior independent of `Point` and `Point2f` happening to be the same size.

### Why this approach
Two prior attempts (#26975, #27020, both by @kallaballa, both open for months before stalling) tried to fix this by rewriting input/output handling around `InputArray::copyTo` and `std::vector`. That cascaded into unrelated scope -- undocumented-input-format questions, new overloaded signatures that broke Java bindings, uncertainty about what the "authoritative spec" for accepted inputs even is -- and neither landed. @asmorkalov's own diagnosis in that thread was narrower: *"the same data is casted as `Point*` and `Point2f*` and used as integer or floating point"* -- i.e. the casting itself, not the input/output handling, is the bug.

This PR makes only that minimal change. `convexHull`'s core is now a function template on the point type (`convexHull_<_Tp,_DotTp>`), so `pointer`/`data0` are always genuinely `Point_<_Tp>*`-typed for whichever branch is active -- no cast is ever needed, because the two instantiations never share storage. `Sklansky_` and `CHullCmpPoints` needed no changes at all; they were already correctly templated -- the UB was only in how the non-template `convexHull()` constructed and reinterpreted the buffers it passed to them. The public signature, input parsing, and output writing are otherwise unchanged, and no other function needed touching (`convexityDefects`/`isContourConvex` already dispatch on depth correctly).

### Testing
- All 55 existing tests covering `convexHull`, the self-intersection index-monotonicity fixup (`!returnPoints`), `minAreaRect`, `minEnclosingTriangle`, and `convexityDefects` pass unchanged -- these call `convexHull` internally, which is exactly what broke under the prior attempts' more invasive rewrites.
- **Performance** (the other thing that sank a prior attempt): my first design (switching the internal representation from a pointer array to an int index array) measured a real, consistent ~10-13% slowdown at n=10000/100000 vs pristine 5.x (A/B via `git stash`, 7-trial medians) -- traced to the extra `index * stride` address computation an index array requires on every dereference that a direct pointer dereference doesn't. Redesigned to keep the *pointer*-array representation (just correctly typed per template instantiation instead of cast), which recovered performance matching baseline within trial-to-trial noise at every size tested. Added `modules/geometry/perf/perf_convhull.cpp` as a permanent regression guard (100/1k/10k/100k points, `CV_32S`/`CV_32F`) -- no such perf test existed before, despite this being exactly the kind of change that regressed performance in prior attempts.
- Full `opencv_test_geometry` suite: no failures attributable to this change (the 3 present are pre-existing, needing `opencv_extra` test data not configured in this scoped build).

### PR checklist
- [x] I agree to contribute to the project under Apache 2 License.
- [x] To the best of my knowledge, the proposed patch is not based on code under GPL or another incompatible license.
- [x] The PR is proposed to the proper branch (5.x).
- [x] Accuracy and performance tests included/added (see above).
- [x] No public API/behavior change, so no documentation or sample updates needed.
2026-08-19 14:05:37 +03:00
Alexander Smorkalov 59218f9edd Merge pull request #29175 from asmorkalov:as/geometry2
Geometry module #29175

OpenCV Contrib: https://github.com/opencv/opencv_contrib/pull/4129
CI changes: https://github.com/opencv/ci-gha-workflow/pull/313

Continues
- https://github.com/opencv/opencv/pull/28804
- https://github.com/opencv/opencv/pull/29101
- https://github.com/opencv/opencv/pull/29108
- https://github.com/opencv/opencv/pull/28810

Todo for followup PRs:
- [x] Rename doxygen groups
- [x] Fix JS modules layout and whitelists
- [ ] Sort tutorials code/snippets

### Pull Request Readiness Checklist

See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request

- [x] I agree to contribute to the project under Apache 2 License.
- [x] To the best of my knowledge, the proposed patch is not based on a code under GPL or another license that is incompatible with OpenCV
- [x] The PR is proposed to the proper branch
- [ ] There is a reference to the original bug report and related work
- [ ] There is accuracy test, performance test and test data in opencv_extra repository, if applicable
      Patch to opencv_extra has the same branch name.
- [ ] The feature is well documented and sample code can be built with the project CMake
2026-05-31 14:23:15 +03:00