diff --git a/H3_VERSION b/H3_VERSION index 857572f..82f24fd 100644 --- a/H3_VERSION +++ b/H3_VERSION @@ -1 +1 @@ -v4.0.0 +v4.0.1 diff --git a/h3_algos.c b/h3_algos.c index 6626b3b..aae6914 100644 --- a/h3_algos.c +++ b/h3_algos.c @@ -151,6 +151,12 @@ static const Direction NEW_ADJUSTMENT_III[7][7] = { {CENTER_DIGIT, CENTER_DIGIT, IJ_AXES_DIGIT, CENTER_DIGIT, I_AXES_DIGIT, CENTER_DIGIT, IJ_AXES_DIGIT}}; +/** + * k value which will encompass all cells at resolution 15. + * This is the largest possible k in the H3 grid system. + */ +static const int K_ALL_CELLS_AT_RES_15 = 13780510; + /** * Maximum number of cells that result from the gridDisk algorithm with the * given k. Formula source and proof: https://oeis.org/A003215 @@ -162,6 +168,17 @@ H3Error H3_EXPORT(maxGridDiskSize)(int k, int64_t *out) { if (k < 0) { return E_DOMAIN; } + if (k >= K_ALL_CELLS_AT_RES_15) { + // If a k value of this value or above is provided, this function will + // estimate more cells than exist in the H3 grid at the finest + // resolution. This is a problem since the function does signed integer + // arithmetic on `k`, which could overflow. To prevent that, instead + // substitute the maximum number of cells in the grid, as it should not + // be possible for the gridDisk functions to exceed that. Note this is + // not resolution specific. So, when resolution < 15, this function may + // still estimate a size larger than the number of cells in the grid. + return H3_EXPORT(getNumCells)(MAX_H3_RES, out); + } *out = 3 * (int64_t)k * ((int64_t)k + 1) + 1; return E_SUCCESS; } @@ -1149,9 +1166,10 @@ H3Error H3_EXPORT(cellsToLinkedMultiPolygon)(const H3Index *h3Set, return err; } _vertexGraphToLinkedGeo(&graph, out); - if (normalizeMultiPolygon(out)) { - return E_FAILED; - } destroyVertexGraph(&graph); - return E_SUCCESS; + H3Error normalizeResult = normalizeMultiPolygon(out); + if (normalizeResult) { + H3_EXPORT(destroyLinkedMultiPolygon)(out); + } + return normalizeResult; } diff --git a/h3_coordijk.c b/h3_coordijk.c index 922d33d..77e9640 100644 --- a/h3_coordijk.c +++ b/h3_coordijk.c @@ -245,11 +245,12 @@ void _ijkNormalize(CoordIJK *c) { } /** - * Determines the H3 digit corresponding to a unit vector in ijk coordinates. + * Determines the H3 digit corresponding to a unit vector or the zero vector + * in ijk coordinates. * - * @param ijk The ijk coordinates; must be a unit vector. - * @return The H3 digit (0-6) corresponding to the ijk unit vector, or - * INVALID_DIGIT on failure. + * @param ijk The ijk coordinates; must be a unit vector or zero vector. + * @return The H3 digit (0-6) corresponding to the ijk unit vector, zero vector, + * or INVALID_DIGIT (7) on failure. */ Direction _unitIjkToDigit(const CoordIJK *ijk) { CoordIJK c = *ijk; diff --git a/h3_directedEdge.c b/h3_directedEdge.c index 7b0c08f..2acff27 100644 --- a/h3_directedEdge.c +++ b/h3_directedEdge.c @@ -75,6 +75,20 @@ H3Error H3_EXPORT(areNeighborCells)(H3Index origin, H3Index destination, *out = 1; return E_SUCCESS; } + if (originResDigit >= INVALID_DIGIT) { + // Prevent indexing off the end of the array below + return E_CELL_INVALID; + } + if ((originResDigit == K_AXES_DIGIT || + destinationResDigit == K_AXES_DIGIT) && + H3_EXPORT(isPentagon)(originParent)) { + // If these are invalid cells, fail rather than incorrectly + // reporting neighbors. For pentagon cells that are actually + // neighbors across the deleted subsequence, they will fail the + // optimized check below, but they will be accepted by the + // gridDisk check below that. + return E_CELL_INVALID; + } // These sets are the relevant neighbors in the clockwise // and counter-clockwise const Direction neighborSetClockwise[] = { diff --git a/h3_faceijk.c b/h3_faceijk.c index d31713e..6534296 100644 --- a/h3_faceijk.c +++ b/h3_faceijk.c @@ -747,8 +747,8 @@ void _faceIjkToCellBoundary(const FaceIJK *h, int res, int start, int length, adjacent hexagon edge will lie completely on a single icosahedron face, and no additional vertex is required. */ - bool isIntersectionAtVertex = - _v2dEquals(&orig2d0, &inter) || _v2dEquals(&orig2d1, &inter); + bool isIntersectionAtVertex = _v2dAlmostEquals(&orig2d0, &inter) || + _v2dAlmostEquals(&orig2d1, &inter); if (!isIntersectionAtVertex) { _hex2dToGeo(&inter, centerIJK.face, adjRes, 1, &g->verts[g->numVerts]); diff --git a/h3_h3Index.c b/h3_h3Index.c index 2784405..97b1a63 100644 --- a/h3_h3Index.c +++ b/h3_h3Index.c @@ -19,7 +19,6 @@ */ #include "h3_h3Index.h" - #include #include #include @@ -104,7 +103,7 @@ int H3_EXPORT(isValidCell)(H3Index h) { int res = H3_GET_RESOLUTION(h); if (res < 0 || res > MAX_H3_RES) { // LCOV_EXCL_BR_LINE // Resolutions less than zero can not be represented in an index - return 0; + return 0; // LCOV_EXCL_LINE } bool foundFirstNonZeroDigit = false; @@ -323,75 +322,84 @@ H3Error H3_EXPORT(compactCells)(const H3Index *h3Set, H3Index *compactedSet, while (numRemainingHexes) { res = H3_GET_RESOLUTION(remainingHexes[0]); int parentRes = res - 1; - // Put the parents of the hexagons into the temp array - // via a hashing mechanism, and use the reserved bits - // to track how many times a parent is duplicated - for (int i = 0; i < numRemainingHexes; i++) { - H3Index currIndex = remainingHexes[i]; - if (currIndex != 0) { - // If the reserved bits were set by the caller, the - // algorithm below may encounter undefined behavior - // because it expects to have set the reserved bits - // itself. - if (H3_GET_RESERVED_BITS(currIndex) != 0) { - H3_MEMORY(free)(remainingHexes); - H3_MEMORY(free)(hashSetArray); - return E_CELL_INVALID; - } - H3Index parent; - H3Error parentError = - H3_EXPORT(cellToParent)(currIndex, parentRes, &parent); - // Should never be reachable as a result of the compact - // algorithm. Can happen if cellToParent errors e.g. - // because of incompatible resolutions. - if (parentError) { - H3_MEMORY(free)(remainingHexes); - H3_MEMORY(free)(hashSetArray); - return parentError; - } - // Modulus hash the parent into the temp array - int loc = (int)(parent % numRemainingHexes); - int loopCount = 0; - while (hashSetArray[loc] != 0) { - if (loopCount > numRemainingHexes) { // LCOV_EXCL_BR_LINE - // LCOV_EXCL_START - // This case should not be possible because at most one - // index is placed into hashSetArray per - // numRemainingHexes. + // If parentRes is less than zero, we've compacted all the way up to the + // base cells. Time to process the remaining cells. + if (parentRes >= 0) { + // Put the parents of the hexagons into the temp array + // via a hashing mechanism, and use the reserved bits + // to track how many times a parent is duplicated + for (int i = 0; i < numRemainingHexes; i++) { + H3Index currIndex = remainingHexes[i]; + if (currIndex != 0) { + // If the reserved bits were set by the caller, the + // algorithm below may encounter undefined behavior + // because it expects to have set the reserved bits + // itself. + if (H3_GET_RESERVED_BITS(currIndex) != 0) { H3_MEMORY(free)(remainingHexes); H3_MEMORY(free)(hashSetArray); - return E_FAILED; - // LCOV_EXCL_STOP + return E_CELL_INVALID; } - H3Index tempIndex = - hashSetArray[loc] & H3_RESERVED_MASK_NEGATIVE; - if (tempIndex == parent) { - int count = H3_GET_RESERVED_BITS(hashSetArray[loc]) + 1; - int limitCount = 7; - if (H3_EXPORT(isPentagon)(tempIndex & - H3_RESERVED_MASK_NEGATIVE)) { - limitCount--; - } - // One is added to count for this check to match one - // being added to count later in this function when - // checking for all children being present. - if (count + 1 > limitCount) { - // Only possible on duplicate input + + H3Index parent; + H3Error parentError = + H3_EXPORT(cellToParent)(currIndex, parentRes, &parent); + // Should never be reachable as a result of the compact + // algorithm. Can happen if cellToParent errors e.g. + // because of incompatible resolutions. + if (parentError) { + H3_MEMORY(free)(remainingHexes); + H3_MEMORY(free)(hashSetArray); + return parentError; + } + // Modulus hash the parent into the temp array + int loc = (int)(parent % numRemainingHexes); + int loopCount = 0; + while (hashSetArray[loc] != 0) { + if (loopCount > // LCOV_EXCL_BR_LINE + numRemainingHexes) { + // LCOV_EXCL_START + // This case should not be possible because at + // most one index is placed into hashSetArray + // per numRemainingHexes. H3_MEMORY(free)(remainingHexes); H3_MEMORY(free)(hashSetArray); - return E_DUPLICATE_INPUT; + return E_FAILED; + // LCOV_EXCL_STOP } - H3_SET_RESERVED_BITS(parent, count); - hashSetArray[loc] = H3_NULL; - } else { - loc = (loc + 1) % numRemainingHexes; + H3Index tempIndex = + hashSetArray[loc] & H3_RESERVED_MASK_NEGATIVE; + if (tempIndex == parent) { + int count = + H3_GET_RESERVED_BITS(hashSetArray[loc]) + 1; + int limitCount = 7; + if (H3_EXPORT(isPentagon)( + tempIndex & H3_RESERVED_MASK_NEGATIVE)) { + limitCount--; + } + // One is added to count for this check to match + // one being added to count later in this + // function when checking for all children being + // present. + if (count + 1 > limitCount) { + // Only possible on duplicate input + H3_MEMORY(free)(remainingHexes); + H3_MEMORY(free)(hashSetArray); + return E_DUPLICATE_INPUT; + } + H3_SET_RESERVED_BITS(parent, count); + hashSetArray[loc] = H3_NULL; + } else { + loc = (loc + 1) % numRemainingHexes; + } + loopCount++; } - loopCount++; + hashSetArray[loc] = parent; } - hashSetArray[loc] = parent; } } + // Determine which parent hexagons have a complete set // of children and put them in the compactableHexes array int compactableCount = 0; @@ -438,16 +446,12 @@ H3Error H3_EXPORT(compactCells)(const H3Index *h3Set, H3Index *compactedSet, H3Index parent; H3Error parentError = H3_EXPORT(cellToParent)(currIndex, parentRes, &parent); - // LCOV_EXCL_START - // Should never be reachable as a result of the compact - // algorithm. if (parentError) { - // TODO: Determine if this is somehow reachable. + H3_MEMORY(free)(compactableHexes); H3_MEMORY(free)(remainingHexes); H3_MEMORY(free)(hashSetArray); return parentError; } - // LCOV_EXCL_STOP // Modulus hash the parent into the temp array // to determine if this index was included in // the compactableHexes array diff --git a/h3_linkedGeo.c b/h3_linkedGeo.c index b660002..f8d489a 100644 --- a/h3_linkedGeo.c +++ b/h3_linkedGeo.c @@ -290,20 +290,20 @@ static const LinkedGeoPolygon *findPolygonForHole( * @param root Root polygon including all loops * @return 0 on success, or an error code > 0 for invalid input */ -int normalizeMultiPolygon(LinkedGeoPolygon *root) { +H3Error normalizeMultiPolygon(LinkedGeoPolygon *root) { // We assume that the input is a single polygon with loops; // if it has multiple polygons, don't touch it if (root->next) { - return NORMALIZATION_ERR_MULTIPLE_POLYGONS; + return E_FAILED; } // Count loops, exiting early if there's only one int loopCount = countLinkedLoops(root); if (loopCount <= 1) { - return NORMALIZATION_SUCCESS; + return E_SUCCESS; } - int resultCode = NORMALIZATION_SUCCESS; + H3Error resultCode = E_SUCCESS; LinkedGeoPolygon *polygon = NULL; LinkedGeoLoop *next; int innerCount = 0; @@ -355,7 +355,7 @@ int normalizeMultiPolygon(LinkedGeoPolygon *root) { // a way to destroy it with destroyLinkedMultiPolygon. destroyLinkedGeoLoop(innerLoops[i]); H3_MEMORY(free)(innerLoops[i]); - resultCode = NORMALIZATION_ERR_UNASSIGNED_HOLES; + resultCode = E_FAILED; } } diff --git a/h3_linkedGeo.h b/h3_linkedGeo.h index d8ba08b..d60956b 100644 --- a/h3_linkedGeo.h +++ b/h3_linkedGeo.h @@ -26,11 +26,6 @@ #include "h3_h3api.h" #include "h3_latLng.h" -// Error codes for normalizeMultiPolygon -#define NORMALIZATION_SUCCESS 0 -#define NORMALIZATION_ERR_MULTIPLE_POLYGONS 1 -#define NORMALIZATION_ERR_UNASSIGNED_HOLES 2 - // Macros for use with polygonAlgos.h /** Macro: Init iteration vars for LinkedGeoLoop */ #define INIT_ITERATION_LINKED_LOOP \ @@ -52,7 +47,7 @@ /** Macro: Whether a LinkedGeoLoop is empty */ #define IS_EMPTY_LINKED_LOOP(loop) loop->first == NULL -int normalizeMultiPolygon(LinkedGeoPolygon *root); +H3Error normalizeMultiPolygon(LinkedGeoPolygon *root); LinkedGeoPolygon *addNewLinkedPolygon(LinkedGeoPolygon *polygon); LinkedGeoLoop *addNewLinkedLoop(LinkedGeoPolygon *polygon); LinkedGeoLoop *addLinkedLoop(LinkedGeoPolygon *polygon, LinkedGeoLoop *loop); diff --git a/h3_localij.c b/h3_localij.c index 72f7712..ac9dbc3 100644 --- a/h3_localij.c +++ b/h3_localij.c @@ -317,12 +317,12 @@ H3Error localIjkToCell(H3Index origin, const CoordIJK *ijk, H3Index *out) { // check for res 0/base cell if (res == 0) { - if (ijk->i > 1 || ijk->j > 1 || ijk->k > 1) { - // out of range input + const Direction dir = _unitIjkToDigit(ijk); + if (dir == INVALID_DIGIT) { + // out of range input - not a unit vector or zero vector return E_FAILED; } - const Direction dir = _unitIjkToDigit(ijk); const int newBaseCell = _getBaseCellNeighbor(originBaseCell, dir); if (newBaseCell == INVALID_BASE_CELL) { // Moving in an invalid direction off a pentagon. diff --git a/h3_vec2d.c b/h3_vec2d.c index 8dcb4f0..a179e06 100644 --- a/h3_vec2d.c +++ b/h3_vec2d.c @@ -19,6 +19,7 @@ #include "h3_vec2d.h" +#include #include #include @@ -46,7 +47,7 @@ void _v2dIntersect(const Vec2d *p0, const Vec2d *p1, const Vec2d *p2, s2.x = p3->x - p2->x; s2.y = p3->y - p2->y; - float t; + double t; t = (s2.x * (p0->y - p2->y) - s2.y * (p0->x - p2->x)) / (-s2.x * s1.y + s1.x * s2.y); @@ -55,12 +56,12 @@ void _v2dIntersect(const Vec2d *p0, const Vec2d *p1, const Vec2d *p2, } /** - * Whether two 2D vectors are equal. Does not consider possible false - * negatives due to floating-point errors. + * Whether two 2D vectors are almost equal, within some threshold * @param v1 First vector to compare * @param v2 Second vector to compare - * @return Whether the vectors are equal + * @return Whether the vectors are almost equal */ -bool _v2dEquals(const Vec2d *v1, const Vec2d *v2) { - return v1->x == v2->x && v1->y == v2->y; +bool _v2dAlmostEquals(const Vec2d *v1, const Vec2d *v2) { + return fabs(v1->x - v2->x) < FLT_EPSILON && + fabs(v1->y - v2->y) < FLT_EPSILON; } diff --git a/h3_vec2d.h b/h3_vec2d.h index 059e6f2..8ba0e9b 100644 --- a/h3_vec2d.h +++ b/h3_vec2d.h @@ -35,6 +35,6 @@ typedef struct { double _v2dMag(const Vec2d *v); void _v2dIntersect(const Vec2d *p0, const Vec2d *p1, const Vec2d *p2, const Vec2d *p3, Vec2d *inter); -bool _v2dEquals(const Vec2d *p0, const Vec2d *p1); +bool _v2dAlmostEquals(const Vec2d *p0, const Vec2d *p1); #endif diff --git a/h3_vertex.c b/h3_vertex.c index e78d068..deec56a 100644 --- a/h3_vertex.c +++ b/h3_vertex.c @@ -224,7 +224,9 @@ H3Error H3_EXPORT(cellToVertex)(H3Index cell, int vertexNum, H3Index *out) { if (left == INVALID_DIGIT) return E_FAILED; int lRotations = 0; H3Index leftNeighbor; - h3NeighborRotations(cell, left, &lRotations, &leftNeighbor); + H3Error leftNeighborError = + h3NeighborRotations(cell, left, &lRotations, &leftNeighbor); + if (leftNeighborError) return leftNeighborError; // Set to owner if lowest index if (leftNeighbor < owner) owner = leftNeighbor; @@ -238,7 +240,9 @@ H3Error H3_EXPORT(cellToVertex)(H3Index cell, int vertexNum, H3Index *out) { if (right == INVALID_DIGIT) return E_FAILED; // LCOV_EXCL_LINE int rRotations = 0; H3Index rightNeighbor; - h3NeighborRotations(cell, right, &rRotations, &rightNeighbor); + H3Error rightNeighborError = + h3NeighborRotations(cell, right, &rRotations, &rightNeighbor); + if (rightNeighborError) return rightNeighborError; // Set to owner if lowest index if (rightNeighbor < owner) { owner = rightNeighbor;