Fix convex hull numerical precision issues causing non-convex outputs

[Copilot]

The Convex_Hull_3 implementation had three numerical precision issues causing non-convex surface meshes:

1. Coordinate quantization: truncate_vert rounded all coordinates to 0.01 unit precision, destroying geometric accuracy
2. Scale-invariant epsilon: Fixed tolerance didn't adapt to point cloud scale (fails for tiny or huge coordinates)
3. Exact zero visibility tests: offset >= 0 comparisons without epsilon margin misclassified vertices due to floating-point errors

Changes

Core algorithm (src/YgorMath.cc):

• Removed truncate_vert quantization entirely
• Compute bounding box and derive scale-aware epsilon: machine_eps = bbox_size * sqrt(ε)
• Visibility test now uses margin: offset > visibility_eps where visibility_eps = 10 * machine_eps
• Added degenerate case handling (no finite vertices, zero bbox fallback to absolute epsilon)

Testing (tests/Test_Math_27.cc, tests/Test_Math_28.cc):

• Added convexity validation using normalized face normals
• Added edge case coverage: 1e-8 scale, 1e10 scale, exact shapes, duplicate vertices

Example

Before (fails on large/small scales):

const auto truncate_vert = [&](const vec3<T> &v){
    return vec3<T>(static_cast<T>(static_cast<int64_t>(std::round(100.0 * v.x))), ...);
};
const auto machine_eps = std::sqrt(10 * std::numeric_limits<T>::epsilon());
const auto is_visible = (offset_A >= 0) && (offset_B >= 0) && (offset_C >= 0);

After (scale-adaptive):

const auto bbox_size = (bbox_max - bbox_min).length();
const auto machine_eps = (bbox_size > abs_eps) ? (bbox_size * rel_eps) : abs_eps;
const auto visibility_eps = static_cast<T>(10) * machine_eps;
const auto is_visible = (offset_A > visibility_eps) && (offset_B > visibility_eps) && ...;

All tests pass with verified convex hulls across 8 orders of magnitude scale range.

Original prompt

The convex hull implementation in src/YgorMath.cc sometimes creates surface meshes that are not convex. Evaluate the implementation for correctness and suggest improvements that will improve reliability. Pay special attention to the use of floating point data types and numerical details.

---

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

[Copilot]

Pull request overview

Improves numerical robustness of Convex_Hull_3 to prevent floating-point precision issues from producing non-convex hull meshes across a wide range of coordinate scales.

Changes:

• Removed vertex coordinate quantization and introduced scale-aware epsilons derived from the input point cloud bounding box.
• Updated visibility tests to use an epsilon margin instead of exact >= 0 comparisons.
• Added/extended tests to validate convexity across tiny/huge scales and common edge cases (cube, tetrahedron, duplicates).

Reviewed changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
src/YgorMath.cc	Makes tolerances scale-aware and removes coordinate truncation; updates visibility predicate to use epsilon margin.
tests/Test_Math_27.cc	Adds convexity validation by checking all vertices against all face planes.
tests/Test_Math_28.cc	Adds new scale/edge-case regression tests + shared convexity validation helper.
.gitignore	Stops ignoring CodeQL artifacts; starts ignoring *.ply and bin/Test_*.
_codeql_detected_source_root	Adds a CodeQL-generated marker file to the repo.

Comments suppressed due to low confidence (1)

_codeql_detected_source_root:1

• _codeql_detected_source_root appears to be a CodeQL-generated artifact and is typically not intended to be committed. Consider removing it from the repo and re-adding ignore rules for CodeQL artifacts (or otherwise documenting why this file should be versioned).

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

bbox_size can become non-finite (e.g., overflow to inf for huge coordinates). If machine_eps becomes inf, distinctness/area/volume checks (dist < machine_eps, area < machine_eps, etc.) become effectively always-true, which can prevent finding a seed tetrahedron and/or suppress hull expansion. Consider explicitly handling !std::isfinite(bbox_size) (and/or !bbox_diag.isfinite()), falling back to a finite scale estimate (e.g., max component extent, max absolute coordinate, or abs_eps) and/or clamping machine_eps to a sane finite upper bound.

[Copilot]

Using three offsets against the same plane and requiring all three to exceed visibility_eps is unnecessarily strict and can misclassify truly-visible points as not visible due to floating-point drift (since offset_A/B/C should be identical in exact arithmetic, but won’t be with a computed unit normal). Prefer computing a single offset against one point on the plane (e.g., just v_A), or use a more numerically stable predicate like testing min(offset_A, offset_B, offset_C) > visibility_eps only if you can justify why the three offsets differ meaningfully.

Suggested change

	// Use epsilon margin to account for floating-point errors in visibility test
	// A vertex is visible if it's clearly outside the face (offset > visibility_eps)
	// This prevents numerical errors from causing incorrect hull expansion
	const auto is_visible = ( std::isfinite(offset_A) && (visibility_eps < offset_A) )
	&& ( std::isfinite(offset_B) && (visibility_eps < offset_B) )
	&& ( std::isfinite(offset_C) && (visibility_eps < offset_C) );
	// Use epsilon margin to account for floating-point errors in visibility test.
	// A vertex is visible if it's clearly outside the face (offset > visibility_eps)
	// as measured from a single point on the face plane (here, v_A). Using multiple
	// redundant offsets against the same plane can cause misclassification due
	// solely due to floating-point drift.
	const auto is_visible = std::isfinite(offset_A) && (visibility_eps < offset_A);

[Copilot]

This test will report “PASSED” even if Convex_Hull_3 throws for reasons unrelated to degeneracy/regression (masking failures at small scale). If the intent is to validate correctness at 1e-8, the test should fail on exception. If the intent is “may be degenerate,” consider constructing a guaranteed non-degenerate shape at 1e-8 (e.g., a tetrahedron/cube scaled down) and requiring success, or explicitly detecting degeneracy (e.g., too few distinct points / near-zero bbox) and treating that case as a deliberate skip.

[Copilot]

Two issues reduce the diagnostic power of this validation: (1) indexing all_verts[face[k]] without bounds checks can crash the test if the hull returns an invalid index (better to early-return failure with a clear message), and (2) if(!face_normal.isfinite()) continue; silently skips degenerate/invalid faces, allowing a potentially broken hull (degenerate triangles) to pass convexity validation. Consider treating non-finite/near-zero normals as a validation failure (or at least counting them as violations) so the test catches degenerate face output.

[Copilot]

This new test uses std::vector and std::min/std::max, but doesn’t directly include <vector> and <algorithm>. Relying on transitive includes from project headers can cause fragile builds; include the standard headers you use explicitly.

[Copilot]

Similar to Test_Math_28, the validation will (a) crash rather than fail gracefully if any face indices are out of range, and (b) silently skip degenerate faces when the computed unit normal is non-finite. For a convex hull regression test, degenerate/invalid triangles should generally be treated as a failure (or at least logged and counted as violations) to avoid false passes.