Add comprehensive geometry algorithms and examples

[lrleon]

Summary by CodeRabbit

• New Features

  • 60+ new geometry example programs and a runner script; many produce TikZ/LaTeX visualizations, CSV and SVG outputs.

• Documentation

  • Expanded English and Spanish READMEs with a new "Computational Geometry" section and example usage details.

• Refactor

  • Public API renames for clarity (distance_with → distance_to, contains_to → contains, is_to_left_from → is_left_of, Segment.size → length).

• Tests

  • Vastly expanded test coverage, many re-enabled tests, and new SVG golden-output support.

• Chores

  • CI workflows improved to collect per-job visual artifacts and adjusted Coverity trigger conditions.

[coderabbitai]

🧹 Nitpick comments (1)

.github/workflows/ci.yml (1)

188-190: Inconsistent ALEPH_TEST_GOLDEN_DIR setup vs. sanitizers/tsan jobs.

build-and-test and coverage set the variable via export inside the run: block, while sanitizers and tsan (lines 410, 480) declare it in the step-level env: block — the idiomatic GitHub Actions approach. Both are functionally equivalent here since ctest runs in the same shell, but aligning all four jobs improves readability.

♻️ Proposed refactor for `build-and-test` Test step (same pattern applies to `coverage`)

     - name: Test
+      env:
+        ALEPH_TEST_GOLDEN_DIR: ${{ github.workspace }}/build/test_artifacts/golden_svg
       run: |
-        mkdir -p build/test_artifacts/golden_svg
-        export ALEPH_TEST_GOLDEN_DIR="$PWD/build/test_artifacts/golden_svg"
+        mkdir -p "$ALEPH_TEST_GOLDEN_DIR"
         ctest --test-dir build --output-on-failure --parallel 2 --timeout 600

Apply the same change to coverage using ${{ github.workspace }}/build-coverage/test_artifacts/golden_svg.

Also applies to: 282-287

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.github/workflows/ci.yml around lines 188 - 190, The CI step currently
exports ALEPH_TEST_GOLDEN_DIR inside the run block; make it consistent with the
sanitizers/tsan jobs by moving ALEPH_TEST_GOLDEN_DIR into the step-level env for
the build-and-test and coverage test steps (use ALEPH_TEST_GOLDEN_DIR: ${{
github.workspace }}/build/test_artifacts/golden_svg for build-and-test and ${{
github.workspace }}/build-coverage/test_artifacts/golden_svg for coverage),
remove the inline export from the run body, and leave the mkdir and ctest
invocation unchanged; target the test step that currently runs "mkdir -p
build/test_artifacts/golden_svg" / "ctest ..." and update its env rather than
exporting in the shell.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 188-190: The CI step currently exports ALEPH_TEST_GOLDEN_DIR
inside the run block; make it consistent with the sanitizers/tsan jobs by moving
ALEPH_TEST_GOLDEN_DIR into the step-level env for the build-and-test and
coverage test steps (use ALEPH_TEST_GOLDEN_DIR: ${{ github.workspace
}}/build/test_artifacts/golden_svg for build-and-test and ${{ github.workspace
}}/build-coverage/test_artifacts/golden_svg for coverage), remove the inline
export from the run body, and leave the mkdir and ctest invocation unchanged;
target the test step that currently runs "mkdir -p
build/test_artifacts/golden_svg" / "ctest ..." and update its env rather than
exporting in the shell.

[Copilot]

Pull request overview

Copilot reviewed 75 out of 107 changed files in this pull request and generated 5 comments.

[Copilot]

The variable name changed from vertex0 to vertex2 at line 946, but the logic expects the last segment to connect vertex N-1 to vertex 0 (closing the polygon). For a square (4 vertices), the last segment should connect vertex 3 to vertex 0, not vertex 2 to vertex 3. This appears to be testing the wrong segment.

[Copilot]

The anchor IDs in the Spanish README were changed to remove the {#...} suffix (e.g., {#readme-es-contenido}). While this is valid Markdown, removing explicit anchor IDs may break internal links if other documents reference these specific anchors. Verify that no other documents link to these anchors with the old format.

[Copilot]

The condition github.event.label.name == 'run-coverity' should use github.event.action == 'labeled' && contains(github.event.pull_request.labels.*.name, 'run-coverity') to properly check if the label exists. The current syntax is incorrect for checking PR labels in GitHub Actions.

Suggested change

	(github.event_name == 'pull_request' && github.event.label.name == 'run-coverity')
	(github.event_name == 'pull_request' && github.event.action == 'labeled' && contains(github.event.pull_request.labels.*.name, 'run-coverity'))

[Copilot]

Pull request overview

Copilot reviewed 75 out of 108 changed files in this pull request and generated 3 comments.

[Copilot]

This header is included by multiple split geom_algorithms_test_part*.cc translation units. If the project builds tests with -Wunused-function (often promoted to errors in CI), helpers like sort3() (and also polygon_signed_area() below) may trigger unused-function warnings in TUs that don't use them. Mark such helpers [[maybe_unused]] (as you did for others) and/or make them inline to reduce warning noise across split test files.

[Copilot]

Failing to open the golden SVG output file is silently ignored (the function just returns the path). That makes visual-regression diagnostics much harder because tests can appear to 'pass' without generating artifacts. Consider surfacing this as a test failure (e.g., via a gtest failure macro from the calling test), or returning an error/optional so callers can explicitly assert that IO succeeded.

[Copilot]

M_PI is not standard C++ and is not guaranteed to be available on all toolchains (notably MSVC unless specific macros are defined). Since these are new examples, prefer a portable constant (e.g., std::numbers::pi / std::numbers::pi_v<double> from <numbers>, or a library-provided PI constant if one exists) to avoid example build failures on some platforms.

[Copilot]

Pull request overview

Copilot reviewed 75 out of 109 changed files in this pull request and generated 4 comments.

[Copilot]

M_PI is non-standard and can break compilation on some platforms. Please switch to std::numbers::pi (add #include <numbers>) or an existing library constant so these examples build portably.

[Copilot]

This example relies on M_PI, which is not guaranteed by the C++ standard library. Use std::numbers::pi (C++20 <numbers>) or a project constant to prevent cross-platform build failures.

[Copilot]

Using --gcov-ignore-parse-errors all is very broad and can hide genuine coverage parsing issues (including regressions). Prefer a narrower set of ignore modes (or adding new specific ones as needed) so coverage failures remain actionable.

Suggested change

--gcov-ignore-parse-errors all

[Copilot]

Pull request overview

Copilot reviewed 75 out of 109 changed files in this pull request and generated 3 comments.

[Copilot]

This assertion is stricter than the previous test, which allowed either π/2 or (2π - π/2). Unless counterclockwise_angle_with() is documented to always return an angle normalized specifically to π/2 for this case, the new expectation can make the test fail for equally valid angle conventions (e.g., [0, 2π) vs. signed/alternate normalization). Consider either reinstating the alternate acceptable value or asserting the expected normalization contract explicitly (and documenting it).

[Copilot]

This header is included by multiple geometry test translation units. Unlike surrounding helpers, polygon_signed_area is not marked [[maybe_unused]]. If any TU includes this header but doesn’t reference polygon_signed_area, some toolchains/configurations with -Wunused-function (often promoted to errors) may fail the build. Mark it [[maybe_unused]] (consistent with other helpers here) or move the helper into a .cc where it’s actually used.

Suggested change

	Geom_Number polygon_signed_area(const Polygon & poly)
	[[maybe_unused]] Geom_Number polygon_signed_area(const Polygon & poly)

[Copilot]

M_PI is not guaranteed by standard C++ and may be unavailable depending on platform/compiler settings (notably MSVC unless specific macros are defined). Prefer std::numbers::pi_v<double> (include <numbers>) to keep these examples portable under C++20.