From a523fc65de144f4d54dbf056b13d90ae95bbf4c1 Mon Sep 17 00:00:00 2001 From: Nick Rabinowitz Date: Tue, 13 Feb 2024 13:59:58 -0800 Subject: [PATCH] Fix for edge cases with bbox overlap and out-of-bounds polygons (#817) * Adds tests for CONTAINMENT_OVERLAPPING_BBOX and fixes the out-of-bounds write when using bbox overlap on a one-vertex polygon * Fixes CONTAINMENT_OVERLAPPING results for polygons outside of the normal lat/lng range. The issue here was the test for cell-contains-polygon, which used latLngToCell, which will return results for out-of-bounds input. I updated this to include a bbox check first, which will exclude polygons with all points outside of the normal range. --- src/apps/testapps/testPolyfillInternal.c | 26 -- .../testapps/testPolygonToCellsExperimental.c | 289 ++++++++++++++---- src/h3lib/lib/polyfill.c | 50 +-- 3 files changed, 266 insertions(+), 99 deletions(-) diff --git a/src/apps/testapps/testPolyfillInternal.c b/src/apps/testapps/testPolyfillInternal.c index 7efeebf79..915378f8c 100644 --- a/src/apps/testapps/testPolyfillInternal.c +++ b/src/apps/testapps/testPolyfillInternal.c @@ -36,14 +36,6 @@ static GeoPolygon sfGeoPolygon = { {0.6599990002976, -2.1376771158464}}}, .numHoles = 0}; -static GeoPolygon invalidGeoPolygon = { - .geoloop = {.numVerts = 4, - .verts = (LatLng[]){{NAN, -2.1364398519396}, - {0.6595011102219, NAN}, - {NAN, -2.1354884206045}, - {0.6581220034068, NAN}}}, - .numHoles = 0}; - SUITE(polyfillInternal) { TEST(iterInitPolygonCompact_errors) { IterCellsPolygonCompact iter; @@ -142,24 +134,6 @@ SUITE(polyfillInternal) { t_assert(iter.cell == H3_NULL, "Got null output for invalid cell"); } - TEST(iterStepPolygonCompact_invalidPolygonErrors) { - IterCellsPolygonCompact iter; - - // Start with a good polygon, otherwise we error out early - iter = - iterInitPolygonCompact(&sfGeoPolygon, 5, CONTAINMENT_OVERLAPPING); - t_assertSuccess(iter.error); - - // Give the iterator a bad polygon and a cell at target res - iter._polygon = &invalidGeoPolygon; - iter.cell = 0x85283473fffffff; - - iterStepPolygonCompact(&iter); - t_assert(iter.error == E_LATLNG_DOMAIN, - "Got expected error for invalid polygon"); - t_assert(iter.cell == H3_NULL, "Got null output for invalid cell"); - } - TEST(iterDestroyPolygonCompact) { IterCellsPolygonCompact iter = iterInitPolygonCompact(&sfGeoPolygon, 9, CONTAINMENT_CENTER); diff --git a/src/apps/testapps/testPolygonToCellsExperimental.c b/src/apps/testapps/testPolygonToCellsExperimental.c index 654de0c4f..6c824f25e 100644 --- a/src/apps/testapps/testPolygonToCellsExperimental.c +++ b/src/apps/testapps/testPolygonToCellsExperimental.c @@ -50,6 +50,11 @@ static LatLng invalidVerts[] = {{INFINITY, INFINITY}, {-INFINITY, -INFINITY}}; static GeoLoop invalidGeoLoop = {.numVerts = 2, .verts = invalidVerts}; static GeoPolygon invalidGeoPolygon; +static LatLng outOfBoundsVert[] = {{-2000, -2000}}; +static GeoLoop outOfBoundsVertGeoLoop = {.numVerts = 1, + .verts = outOfBoundsVert}; +static GeoPolygon outOfBoundsVertGeoPolygon; + static LatLng invalid2Verts[] = {{NAN, NAN}, {-NAN, -NAN}}; static GeoLoop invalid2GeoLoop = {.numVerts = 2, .verts = invalid2Verts}; static GeoPolygon invalid2GeoPolygon; @@ -173,6 +178,9 @@ SUITE(polygonToCells) { invalid2GeoPolygon.geoloop = invalid2GeoLoop; invalid2GeoPolygon.numHoles = 0; + outOfBoundsVertGeoPolygon.geoloop = outOfBoundsVertGeoLoop; + outOfBoundsVertGeoPolygon.numHoles = 0; + nullGeoPolygon.geoloop = nullGeoLoop; nullGeoPolygon.numHoles = 0; @@ -182,7 +190,7 @@ SUITE(polygonToCells) { lineGeoPolygon.geoloop = lineGeoLoop; lineGeoPolygon.numHoles = 0; - TEST(polygonToCells) { + TEST(polygonToCells_CenterContainment) { int64_t numHexagons; t_assertSuccess(H3_EXPORT(maxPolygonToCellsSizeExperimental)( &sfGeoPolygon, 9, CONTAINMENT_CENTER, &numHexagons)); @@ -226,7 +234,22 @@ SUITE(polygonToCells) { free(hexagons); } - TEST(polygonToCellsHole) { + TEST(polygonToCells_OverlappingBBox) { + int64_t numHexagons; + t_assertSuccess(H3_EXPORT(maxPolygonToCellsSizeExperimental)( + &sfGeoPolygon, 9, CONTAINMENT_OVERLAPPING_BBOX, &numHexagons)); + H3Index *hexagons = calloc(numHexagons, sizeof(H3Index)); + + t_assertSuccess(H3_EXPORT(polygonToCellsExperimental)( + &sfGeoPolygon, 9, CONTAINMENT_OVERLAPPING_BBOX, hexagons)); + int64_t actualNumIndexes = countNonNullIndexes(hexagons, numHexagons); + + t_assert(actualNumIndexes == 1416, + "got expected polygonToCells size (overlapping bbox mode)"); + free(hexagons); + } + + TEST(polygonToCellsHole_CenterContainment) { int64_t numHexagons; t_assertSuccess(H3_EXPORT(maxPolygonToCellsSizeExperimental)( &holeGeoPolygon, 9, CONTAINMENT_CENTER, &numHexagons)); @@ -241,7 +264,7 @@ SUITE(polygonToCells) { free(hexagons); } - TEST(polygonToCellsHoleFullContainment) { + TEST(polygonToCellsHole_FullContainment) { int64_t numHexagons; t_assertSuccess(H3_EXPORT(maxPolygonToCellsSizeExperimental)( &holeGeoPolygon, 9, CONTAINMENT_FULL, &numHexagons)); @@ -257,7 +280,7 @@ SUITE(polygonToCells) { free(hexagons); } - TEST(polygonToCellsHoleOverlapping) { + TEST(polygonToCellsHole_Overlapping) { int64_t numHexagons; t_assertSuccess(H3_EXPORT(maxPolygonToCellsSizeExperimental)( &holeGeoPolygon, 9, CONTAINMENT_OVERLAPPING, &numHexagons)); @@ -272,6 +295,22 @@ SUITE(polygonToCells) { free(hexagons); } + TEST(polygonToCellsHole_OverlappingBBox) { + int64_t numHexagons; + t_assertSuccess(H3_EXPORT(maxPolygonToCellsSizeExperimental)( + &holeGeoPolygon, 9, CONTAINMENT_OVERLAPPING_BBOX, &numHexagons)); + H3Index *hexagons = calloc(numHexagons, sizeof(H3Index)); + + t_assertSuccess(H3_EXPORT(polygonToCellsExperimental)( + &holeGeoPolygon, 9, CONTAINMENT_OVERLAPPING_BBOX, hexagons)); + int64_t actualNumIndexes = countNonNullIndexes(hexagons, numHexagons); + + t_assert( + actualNumIndexes == 1403, + "got expected polygonToCells size (hole, overlapping bbox mode)"); + free(hexagons); + } + TEST(polygonToCellsHoleParentIssue) { // This checks a specific issue where the bounding box of the parent // cell fully contains the hole. @@ -341,7 +380,7 @@ SUITE(polygonToCells) { free(hexagons); } - TEST(polygonToCellsContainsPolygon_CenterContained) { + TEST(polygonToCellsContainsPolygon_CenterContainment) { // Contains the center point of a res 4 cell LatLng centerVerts[] = {{0.6595645, -2.1353315}, {0.6595645, -2.1353314}, @@ -398,6 +437,22 @@ SUITE(polygonToCells) { free(hexagons); } + TEST(polygonToCellsContainsPolygon_OverlappingBBox) { + int64_t numHexagons; + t_assertSuccess(H3_EXPORT(maxPolygonToCellsSizeExperimental)( + &sfGeoPolygon, 4, CONTAINMENT_OVERLAPPING_BBOX, &numHexagons)); + H3Index *hexagons = calloc(numHexagons, sizeof(H3Index)); + + t_assertSuccess(H3_EXPORT(polygonToCellsExperimental)( + &sfGeoPolygon, 4, CONTAINMENT_OVERLAPPING_BBOX, hexagons)); + int64_t actualNumIndexes = countNonNullIndexes(hexagons, numHexagons); + + t_assert(actualNumIndexes == 5, + "got expected polygonToCells size (overlapping bbox mode)"); + t_assert(hexagons[0] == 0x8428309ffffffff, "got expected hexagon"); + free(hexagons); + } + TEST(polygonToCellsExact) { LatLng somewhere = {1, 2}; H3Index origin; @@ -440,6 +495,15 @@ SUITE(polygonToCells) { // TODO: CONTAINMENT_OVERLAPPING yields 7 cells, presumably due to FPE // in the various cell boundaries + t_assertSuccess(H3_EXPORT(polygonToCellsExperimental)( + &someHexagon, 9, CONTAINMENT_OVERLAPPING_BBOX, hexagons)); + actualNumIndexes = countNonNullIndexes(hexagons, numHexagons); + // Overlapping BBox is very rough, so we get a couple of overlaps from + // non-neighboring cells + t_assert(actualNumIndexes == 9, + "got expected polygonToCells size for overlapping bbox " + "containment"); + free(hexagons); free(verts); } @@ -642,52 +706,108 @@ SUITE(polygonToCells) { } } - TEST(polygonToCellsPointPolygon) { - int64_t numHexagons; - t_assertSuccess(H3_EXPORT(maxPolygonToCellsSizeExperimental)( - &pointGeoPolygon, 9, CONTAINMENT_CENTER, &numHexagons)); - t_assert(numHexagons == 1, "got expected estimated size"); - H3Index *hexagons = calloc(numHexagons, sizeof(H3Index)); - - t_assertSuccess(H3_EXPORT(polygonToCellsExperimental)( - &pointGeoPolygon, 9, CONTAINMENT_CENTER, hexagons)); - int64_t actualNumIndexes = countNonNullIndexes(hexagons, numHexagons); - - t_assert(actualNumIndexes == 0, "got expected polygonToCells size"); - free(hexagons); + TEST(polygonToCellsPointPolygon_CenterContainment) { + for (int res = 0; res < MAX_H3_RES; res++) { + int64_t numHexagons; + t_assertSuccess(H3_EXPORT(maxPolygonToCellsSizeExperimental)( + &pointGeoPolygon, res, CONTAINMENT_CENTER, &numHexagons)); + t_assert(numHexagons >= 1 && numHexagons <= 5, + "got expected estimated size"); + H3Index *hexagons = calloc(numHexagons, sizeof(H3Index)); + + t_assertSuccess(H3_EXPORT(polygonToCellsExperimental)( + &pointGeoPolygon, res, CONTAINMENT_CENTER, hexagons)); + int64_t actualNumIndexes = + countNonNullIndexes(hexagons, numHexagons); + + t_assert(actualNumIndexes == 0, "got expected polygonToCells size"); + free(hexagons); + } } - TEST(polygonToCellsPointPolygon_full) { - int64_t numHexagons; - t_assertSuccess(H3_EXPORT(maxPolygonToCellsSizeExperimental)( - &pointGeoPolygon, 9, CONTAINMENT_FULL, &numHexagons)); - t_assert(numHexagons == 1, "got expected estimated size"); - H3Index *hexagons = calloc(numHexagons, sizeof(H3Index)); + TEST(polygonToCellsPointPolygon_FullContainment) { + for (int res = 0; res < MAX_H3_RES; res++) { + int64_t numHexagons; + t_assertSuccess(H3_EXPORT(maxPolygonToCellsSizeExperimental)( + &pointGeoPolygon, res, CONTAINMENT_FULL, &numHexagons)); + t_assert(numHexagons >= 1 && numHexagons <= 5, + "got expected estimated size"); + H3Index *hexagons = calloc(numHexagons, sizeof(H3Index)); + + t_assertSuccess(H3_EXPORT(polygonToCellsExperimental)( + &pointGeoPolygon, res, CONTAINMENT_FULL, hexagons)); + int64_t actualNumIndexes = + countNonNullIndexes(hexagons, numHexagons); + + t_assert(actualNumIndexes == 0, "got expected polygonToCells size"); + free(hexagons); + } + } - t_assertSuccess(H3_EXPORT(polygonToCellsExperimental)( - &pointGeoPolygon, 9, CONTAINMENT_FULL, hexagons)); - int64_t actualNumIndexes = countNonNullIndexes(hexagons, numHexagons); + TEST(polygonToCellsPointPolygon_Overlapping) { + for (int res = 0; res < MAX_H3_RES; res++) { + int64_t numHexagons; + t_assertSuccess(H3_EXPORT(maxPolygonToCellsSizeExperimental)( + &pointGeoPolygon, res, CONTAINMENT_OVERLAPPING, &numHexagons)); + t_assert(numHexagons >= 1 && numHexagons <= 5, + "got expected estimated size"); + H3Index *hexagons = calloc(numHexagons, sizeof(H3Index)); + + t_assertSuccess(H3_EXPORT(polygonToCellsExperimental)( + &pointGeoPolygon, res, CONTAINMENT_OVERLAPPING, hexagons)); + int64_t actualNumIndexes = + countNonNullIndexes(hexagons, numHexagons); + + t_assert(actualNumIndexes == 1, "got expected polygonToCells size"); + free(hexagons); + } + } - t_assert(actualNumIndexes == 0, "got expected polygonToCells size"); - free(hexagons); + TEST(polygonToCellsPointPolygon_OverlappingBBox) { + for (int res = 0; res < MAX_H3_RES; res++) { + int64_t numHexagons; + t_assertSuccess(H3_EXPORT(maxPolygonToCellsSizeExperimental)( + &pointGeoPolygon, res, CONTAINMENT_OVERLAPPING_BBOX, + &numHexagons)); + t_assert(numHexagons >= 1 && numHexagons <= 5, + "got expected estimated size"); + H3Index *hexagons = calloc(numHexagons, sizeof(H3Index)); + + t_assertSuccess(H3_EXPORT(polygonToCellsExperimental)( + &pointGeoPolygon, res, CONTAINMENT_OVERLAPPING_BBOX, hexagons)); + int64_t actualNumIndexes = + countNonNullIndexes(hexagons, numHexagons); + + t_assert(actualNumIndexes >= 1 && actualNumIndexes <= 5, + "got expected polygonToCells size"); + free(hexagons); + } } - TEST(polygonToCellsPointPolygon_overlapping) { - int64_t numHexagons; - t_assertSuccess(H3_EXPORT(maxPolygonToCellsSizeExperimental)( - &pointGeoPolygon, 9, CONTAINMENT_OVERLAPPING, &numHexagons)); - t_assert(numHexagons == 1, "got expected estimated size"); - H3Index *hexagons = calloc(numHexagons, sizeof(H3Index)); + TEST(polygonToCellsOutOfBoundsPolygon) { + for (int res = 0; res < MAX_H3_RES; res++) { + for (uint32_t flags = 0; flags < CONTAINMENT_INVALID; flags++) { + int64_t numHexagons; + t_assertSuccess(H3_EXPORT(maxPolygonToCellsSizeExperimental)( + &outOfBoundsVertGeoPolygon, res, flags, &numHexagons)); + t_assert(numHexagons == 0, "got expected estimated size"); + // Note: We're allocating more memory than the estimate to test + // for out-of-bounds writes here + H3Index *hexagons = calloc(10, sizeof(H3Index)); - t_assertSuccess(H3_EXPORT(polygonToCellsExperimental)( - &pointGeoPolygon, 9, CONTAINMENT_OVERLAPPING, hexagons)); - int64_t actualNumIndexes = countNonNullIndexes(hexagons, numHexagons); + t_assertSuccess(H3_EXPORT(polygonToCellsExperimental)( + &outOfBoundsVertGeoPolygon, res, flags, hexagons)); + int64_t actualNumIndexes = + countNonNullIndexes(hexagons, numHexagons); - t_assert(actualNumIndexes == 1, "got expected polygonToCells size"); - free(hexagons); + t_assert(actualNumIndexes == 0, + "got expected polygonToCells size"); + free(hexagons); + } + } } - TEST(polygonToCellsLinePolygon) { + TEST(polygonToCellsLinePolygon_CenterContainment) { int64_t numHexagons; t_assertSuccess(H3_EXPORT(maxPolygonToCellsSizeExperimental)( &lineGeoPolygon, 9, CONTAINMENT_CENTER, &numHexagons)); @@ -701,7 +821,7 @@ SUITE(polygonToCells) { free(hexagons); } - TEST(polygonToCellsLinePolygon_full) { + TEST(polygonToCellsLinePolygon_FullContainment) { int64_t numHexagons; t_assertSuccess(H3_EXPORT(maxPolygonToCellsSizeExperimental)( &lineGeoPolygon, 9, CONTAINMENT_FULL, &numHexagons)); @@ -715,7 +835,7 @@ SUITE(polygonToCells) { free(hexagons); } - TEST(polygonToCellsLinePolygon_overlapping) { + TEST(polygonToCellsLinePolygon_Overlapping) { int64_t numHexagons; t_assertSuccess(H3_EXPORT(maxPolygonToCellsSizeExperimental)( &lineGeoPolygon, 9, CONTAINMENT_OVERLAPPING, &numHexagons)); @@ -729,7 +849,21 @@ SUITE(polygonToCells) { free(hexagons); } - TEST(polygonToCellsNullHole) { + TEST(polygonToCellsLinePolygon_OverlappingBBox) { + int64_t numHexagons; + t_assertSuccess(H3_EXPORT(maxPolygonToCellsSizeExperimental)( + &lineGeoPolygon, 9, CONTAINMENT_OVERLAPPING_BBOX, &numHexagons)); + H3Index *hexagons = calloc(numHexagons, sizeof(H3Index)); + + t_assertSuccess(H3_EXPORT(polygonToCellsExperimental)( + &lineGeoPolygon, 9, CONTAINMENT_OVERLAPPING_BBOX, hexagons)); + int64_t actualNumIndexes = countNonNullIndexes(hexagons, numHexagons); + + t_assert(actualNumIndexes == 21, "got expected polygonToCells size"); + free(hexagons); + } + + TEST(polygonToCellsNullHole_CenterContainment) { int64_t numHexagons; t_assertSuccess(H3_EXPORT(maxPolygonToCellsSizeExperimental)( &nullHoleGeoPolygon, 9, CONTAINMENT_CENTER, &numHexagons)); @@ -745,7 +879,7 @@ SUITE(polygonToCells) { free(hexagons); } - TEST(polygonToCellsNullHole_full) { + TEST(polygonToCellsNullHole_FullContainment) { int64_t numHexagons; t_assertSuccess(H3_EXPORT(maxPolygonToCellsSizeExperimental)( &nullHoleGeoPolygon, 9, CONTAINMENT_FULL, &numHexagons)); @@ -761,7 +895,7 @@ SUITE(polygonToCells) { free(hexagons); } - TEST(polygonToCellsNullHole_overlapping) { + TEST(polygonToCellsNullHole_Overlapping) { int64_t numHexagons; t_assertSuccess(H3_EXPORT(maxPolygonToCellsSizeExperimental)( &nullHoleGeoPolygon, 9, CONTAINMENT_OVERLAPPING, &numHexagons)); @@ -777,7 +911,24 @@ SUITE(polygonToCells) { free(hexagons); } - TEST(polygonToCellsPointHole) { + TEST(polygonToCellsNullHole_OverlappingBBox) { + int64_t numHexagons; + t_assertSuccess(H3_EXPORT(maxPolygonToCellsSizeExperimental)( + &nullHoleGeoPolygon, 9, CONTAINMENT_OVERLAPPING_BBOX, + &numHexagons)); + H3Index *hexagons = calloc(numHexagons, sizeof(H3Index)); + + t_assertSuccess(H3_EXPORT(polygonToCellsExperimental)( + &nullHoleGeoPolygon, 9, CONTAINMENT_OVERLAPPING_BBOX, hexagons)); + int64_t actualNumIndexes = countNonNullIndexes(hexagons, numHexagons); + + // Same as without the hole + t_assert(actualNumIndexes == 1416, + "got expected polygonToCells size (null hole)"); + free(hexagons); + } + + TEST(polygonToCellsPointHole_CenterContainment) { int64_t numHexagons; t_assertSuccess(H3_EXPORT(maxPolygonToCellsSizeExperimental)( &pointHoleGeoPolygon, 9, CONTAINMENT_CENTER, &numHexagons)); @@ -793,7 +944,7 @@ SUITE(polygonToCells) { free(hexagons); } - TEST(polygonToCellsPointHole_full) { + TEST(polygonToCellsPointHole_FullContainment) { int64_t numHexagons; t_assertSuccess(H3_EXPORT(maxPolygonToCellsSizeExperimental)( &pointHoleGeoPolygon, 9, CONTAINMENT_FULL, &numHexagons)); @@ -809,7 +960,7 @@ SUITE(polygonToCells) { free(hexagons); } - TEST(polygonToCellsPointHole_overlapping) { + TEST(polygonToCellsPointHole_Overlapping) { int64_t numHexagons; t_assertSuccess(H3_EXPORT(maxPolygonToCellsSizeExperimental)( &pointHoleGeoPolygon, 9, CONTAINMENT_OVERLAPPING, &numHexagons)); @@ -825,7 +976,24 @@ SUITE(polygonToCells) { free(hexagons); } - TEST(polygonToCellsLineHole) { + TEST(polygonToCellsPointHole_OverlappingBBox) { + int64_t numHexagons; + t_assertSuccess(H3_EXPORT(maxPolygonToCellsSizeExperimental)( + &pointHoleGeoPolygon, 9, CONTAINMENT_OVERLAPPING_BBOX, + &numHexagons)); + H3Index *hexagons = calloc(numHexagons, sizeof(H3Index)); + + t_assertSuccess(H3_EXPORT(polygonToCellsExperimental)( + &pointHoleGeoPolygon, 9, CONTAINMENT_OVERLAPPING_BBOX, hexagons)); + int64_t actualNumIndexes = countNonNullIndexes(hexagons, numHexagons); + + // Same as without the hole + t_assert(actualNumIndexes == 1416, + "got expected polygonToCells size (point hole)"); + free(hexagons); + } + + TEST(polygonToCellsLineHole_CenterContainment) { int64_t numHexagons; t_assertSuccess(H3_EXPORT(maxPolygonToCellsSizeExperimental)( &lineHoleGeoPolygon, 9, CONTAINMENT_CENTER, &numHexagons)); @@ -841,7 +1009,7 @@ SUITE(polygonToCells) { free(hexagons); } - TEST(polygonToCellsLineHole_full) { + TEST(polygonToCellsLineHole_FullContainment) { int64_t numHexagons; t_assertSuccess(H3_EXPORT(maxPolygonToCellsSizeExperimental)( &lineHoleGeoPolygon, 9, CONTAINMENT_FULL, &numHexagons)); @@ -857,7 +1025,7 @@ SUITE(polygonToCells) { free(hexagons); } - TEST(polygonToCellsLineHole_overlapping) { + TEST(polygonToCellsLineHole_Overlapping) { int64_t numHexagons; t_assertSuccess(H3_EXPORT(maxPolygonToCellsSizeExperimental)( &lineHoleGeoPolygon, 9, CONTAINMENT_OVERLAPPING, &numHexagons)); @@ -873,6 +1041,23 @@ SUITE(polygonToCells) { free(hexagons); } + TEST(polygonToCellsLineHole_OverlappingBBox) { + int64_t numHexagons; + t_assertSuccess(H3_EXPORT(maxPolygonToCellsSizeExperimental)( + &lineHoleGeoPolygon, 9, CONTAINMENT_OVERLAPPING_BBOX, + &numHexagons)); + H3Index *hexagons = calloc(numHexagons, sizeof(H3Index)); + + t_assertSuccess(H3_EXPORT(polygonToCellsExperimental)( + &lineHoleGeoPolygon, 9, CONTAINMENT_OVERLAPPING_BBOX, hexagons)); + int64_t actualNumIndexes = countNonNullIndexes(hexagons, numHexagons); + + // Same as without the hole + t_assert(actualNumIndexes == 1416, + "got expected polygonToCells size (line hole)"); + free(hexagons); + } + TEST(invalidFlags) { int64_t numHexagons; for (uint32_t flags = CONTAINMENT_INVALID; flags <= 32; flags++) { diff --git a/src/h3lib/lib/polyfill.c b/src/h3lib/lib/polyfill.c index 7b771f22d..150dd2474 100644 --- a/src/h3lib/lib/polyfill.c +++ b/src/h3lib/lib/polyfill.c @@ -207,6 +207,8 @@ static BBox RES0_BBOXES[NUM_BASE_CELLS] = { {-1.20305471830087, -1.52480158339146, -0.60112285060716, 2.53494381704943}}; +static BBox VALID_RANGE_BBOX = {M_PI_2, -M_PI_2, M_PI, -M_PI}; + /** * For a given cell, return its bounding box. If coverChildren is true, the bbox * will be guaranteed to contain its children at any finer resolution. Note that @@ -248,7 +250,7 @@ H3Error cellToBBox(H3Index cell, BBox *out, bool coverChildren) { out->north = M_PI_2; } - // Cell that contains the north pole + // Cell that contains the south pole if (cell == SOUTH_POLE_CELLS[res]) { out->south = -M_PI_2; } @@ -448,20 +450,31 @@ void iterStepPolygonCompact(IterCellsPolygonCompact *iter) { } if (mode == CONTAINMENT_OVERLAPPING) { // For overlapping, we need to do a quick check to determine - // whether the polygon is wholly contained by the cell. We check - // the first polygon vertex, which if it is contained could also - // mean we simply intersect. - H3Index polygonCell; - H3Error polygonCellErr = H3_EXPORT(latLngToCell)( - &(iter->_polygon->geoloop.verts[0]), cellRes, &polygonCell); - if (polygonCellErr != E_SUCCESS) { - iterErrorPolygonCompact(iter, polygonCellErr); - return; - } - if (polygonCell == cell) { - // Set to next output - iter->cell = cell; - return; + // whether the polygon is wholly contained by the cell. We + // check the first polygon vertex, which if it is contained + // could also mean we simply intersect. + + // Deferencing verts[0] is safe because we check numVerts above + LatLng firstVertex = iter->_polygon->geoloop.verts[0]; + + // We have to check whether the point is in the expected range + // first, because out-of-bounds values will yield false + // positives with latLngToCell + if (bboxContains(&VALID_RANGE_BBOX, &firstVertex)) { + H3Index polygonCell; + H3Error polygonCellErr = H3_EXPORT(latLngToCell)( + &(iter->_polygon->geoloop.verts[0]), cellRes, + &polygonCell); + if (NEVER(polygonCellErr != E_SUCCESS)) { + // This should be unreachable with the bbox check + iterErrorPolygonCompact(iter, polygonCellErr); + return; + } + if (polygonCell == cell) { + // Set to next output + iter->cell = cell; + return; + } } } if (mode == CONTAINMENT_FULL || mode == CONTAINMENT_OVERLAPPING) { @@ -711,11 +724,6 @@ H3Error H3_EXPORT(maxPolygonToCellsSizeExperimental)(const GeoPolygon *polygon, *out = 0; return E_SUCCESS; } - // Special case: 1-vertex polygon - if (polygon->geoloop.numVerts == 1) { - *out = 1; - return E_SUCCESS; - } // Initialize the iterator without stepping, so we can adjust the res and // flags (after they are validated by the initialization) before we start @@ -757,4 +765,4 @@ H3Error H3_EXPORT(maxPolygonToCellsSizeExperimental)(const GeoPolygon *polygon, } return iter.error; -} \ No newline at end of file +}