Add geodesic coverage mode to polygonToCellsExperimental#1178
Open
holoskii wants to merge 6 commits into
Open
Conversation
Adds geodesic (great-circle) polygon-to-cells support to the polygonToCellsExperimental API via a FLAG_GEODESIC_MASK flag bit. Squashed from 39 commits on geodesic_coverage branch for clean rebase onto upstream/master.
gcc -O2 inlines _ijkToHex2d and sees the uninitialized read. Initialize to zero like the non-pentagon counterpart already does.
The docs stated that only FULL and OVERLAPPING are valid when the geodesic flag is set. This contradicts the public API, which permits CENTER as well: h3api.h.in documents it, validatePolygonFlags accepts it (polygon.c), geodesicIteratorStep implements it (geodesicIterator.c), and testGeodesicPolygonToCellsExperimental.c exercises it. Only OVERLAPPING_BBOX is invalid in geodesic mode. Update the umbrella statement and all per-binding sections (Java, JavaScript, Python, Go, DuckDB) to list CENTER as valid.
Contributor
Author
|
Continuation of PR #1052 |
Contributor
Author
|
@ajfriend this took some time, but here we are, should be good to merge |
isaacbrodsky
requested review from
ajfriend,
dfellis,
dmitryzv,
isaacbrodsky and
nrabinowitz
July 1, 2026 16:26
dfellis
reviewed
Jul 3, 2026
|
|
||
| // If the axis is nearly aligned with the normal, there's no unique | ||
| // extremum to find. | ||
| if (extremum_mag_sq < EPSILON * EPSILON) { |
Collaborator
There was a problem hiding this comment.
Is Epsilon squared a value above 0? I am wondering if this branch runs at all.
Contributor
Author
There was a problem hiding this comment.
Yes, far above minimum positive double value DBL_MIN = ~2.2e-308. So there can be a tiny arc exercising this branch. I created a small UT to verify it, but there is not enough value to commit it
- Fix copyright years on new files (benchmark + fuzzer geodesic) - Clarify fuzzer duplicate-run skip comment - Promote sideEps to file-scope SIDE_EPS constant in geodesicPolygon.c Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Addressed all of @dfellis commits |
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.
Introduce a geodesic flag for polygonToCellsExperimental that interprets
polygon edges as great-circle arcs instead of planar segments. This is
critical for accurate coverage of very large polygons spanning hundreds
or thousands of kilometers, where planar interpolation diverges
significantly from the true spherical geometry.
The implementation operates entirely on the unit sphere using 3D
Cartesian coordinates, with sphere-cap and AABB acceleration structures
for efficient cell pruning. Supports CONTAINMENT_CENTER, CONTAINMENT_FULL,
and CONTAINMENT_OVERLAPPING modes. This mode is slower than the planar
algorithm, so lower resolutions are recommended.
Note: geodesic mode rejects polygons that don't fit within a hemisphere
Key changes:
Developed at FloeDB and contributed upstream