Add fixes and tests for 0-, 1-, and 2-vertex geoloops and holes #816
Conversation
| TEST(polygonToCellsNullPolygon) { | ||
| int64_t numHexagons; | ||
| t_assertSuccess(H3_EXPORT(maxPolygonToCellsSizeExperimental)( | ||
| &nullGeoPolygon, 9, CONTAINMENT_CENTER, &numHexagons)); | ||
| t_assert(numHexagons == 0, "got expected estimated size"); | ||
| H3Index *hexagons = calloc(numHexagons, sizeof(H3Index)); | ||
|
|
||
| t_assertSuccess(H3_EXPORT(polygonToCellsExperimental)( | ||
| &nullGeoPolygon, 9, CONTAINMENT_CENTER, hexagons)); | ||
| int64_t actualNumIndexes = countNonNullIndexes(hexagons, numHexagons); | ||
|
|
||
| t_assert(actualNumIndexes == 0, "got expected polygonToCells size"); | ||
| free(hexagons); | ||
| } | ||
|
|
||
| TEST(polygonToCellsNullPolygon_full) { | ||
| int64_t numHexagons; | ||
| t_assertSuccess(H3_EXPORT(maxPolygonToCellsSizeExperimental)( | ||
| &nullGeoPolygon, 9, CONTAINMENT_FULL, &numHexagons)); | ||
| t_assert(numHexagons == 0, "got expected estimated size"); | ||
| H3Index *hexagons = calloc(numHexagons, sizeof(H3Index)); | ||
|
|
||
| t_assertSuccess(H3_EXPORT(polygonToCellsExperimental)( | ||
| &nullGeoPolygon, 9, CONTAINMENT_FULL, hexagons)); | ||
| int64_t actualNumIndexes = countNonNullIndexes(hexagons, numHexagons); | ||
|
|
||
| t_assert(actualNumIndexes == 0, "got expected polygonToCells size"); | ||
| free(hexagons); | ||
| } | ||
|
|
||
| TEST(polygonToCellsNullPolygon_overlapping) { | ||
| int64_t numHexagons; | ||
| t_assertSuccess(H3_EXPORT(maxPolygonToCellsSizeExperimental)( | ||
| &nullGeoPolygon, 9, CONTAINMENT_OVERLAPPING, &numHexagons)); | ||
| t_assert(numHexagons == 0, "got expected estimated size"); | ||
| H3Index *hexagons = calloc(numHexagons, sizeof(H3Index)); | ||
|
|
||
| t_assertSuccess(H3_EXPORT(polygonToCellsExperimental)( | ||
| &nullGeoPolygon, 9, CONTAINMENT_OVERLAPPING, hexagons)); | ||
| int64_t actualNumIndexes = countNonNullIndexes(hexagons, numHexagons); | ||
|
|
||
| t_assert(actualNumIndexes == 0, "got expected polygonToCells size"); | ||
| free(hexagons); | ||
| } |
There was a problem hiding this comment.
maybe these could be collapsed to the same test and just loop through which flags to test? Might be easier to read that way?
There was a problem hiding this comment.
True, that's what you did in #800. I can collapse this one, and the null hole, but not the other two (where OVERLAPPING and FULL behave differently)
There was a problem hiding this comment.
Oh, actually this is the only one I can collapse, the others have different results per mode
|
|
||
| t_assert(actualNumIndexes == 1, "got expected polygonToCells size"); | ||
| free(hexagons); | ||
| } |
There was a problem hiding this comment.
There is a test missing in this PR for polygonToCellsPointPolygon_overlappingBbox (and same for the below), which is why the fuzzer is still failing
| // Special case: 1-vertex polygon | ||
| if (polygon->geoloop.numVerts == 1) { | ||
| *out = 1; | ||
| return E_SUCCESS; | ||
| } |
There was a problem hiding this comment.
In overlapping BBOX mode, this will not be correct as more than one cell bounding box may overlap a vertex.
There was a problem hiding this comment.
Gotcha. I'll have a new PR up for this soon.
This includes the fixes from #800, plus one more fix for estimating a 1-vertex polygon. Includes the following tests:
Hopefully #800 will be green once rebased on top of this.