diff --git a/.github/workflows/test-macos.yml b/.github/workflows/test-macos.yml index 2f61248546..c12e21d950 100644 --- a/.github/workflows/test-macos.yml +++ b/.github/workflows/test-macos.yml @@ -12,6 +12,7 @@ jobs: runs-on: ${{ matrix.os }} strategy: + fail-fast: false matrix: os: [macos-12, macos-13, macos-latest] build_type: ["Debug", "Release"] diff --git a/src/apps/testapps/testCellsToLinkedMultiPolygon.c b/src/apps/testapps/testCellsToLinkedMultiPolygon.c index 5c903386e6..c3f9a003df 100644 --- a/src/apps/testapps/testCellsToLinkedMultiPolygon.c +++ b/src/apps/testapps/testCellsToLinkedMultiPolygon.c @@ -53,7 +53,7 @@ SUITE(cellsToLinkedMultiPolygon) { int numHexes = ARRAY_SIZE(set); t_assert(H3_EXPORT(cellsToLinkedMultiPolygon)( - set, numHexes, &polygon) == E_CELL_INVALID, + set, numHexes, &polygon) != E_SUCCESS, "Invalid set fails"); } @@ -72,6 +72,24 @@ SUITE(cellsToLinkedMultiPolygon) { H3_EXPORT(destroyLinkedMultiPolygon)(&polygon); } + TEST(pentagonChildren) { + // children of pentagon 0x80ebfffffffffff + H3Index kids[] = {0x81ea3ffffffffff, 0x81eabffffffffff, + 0x81eafffffffffff, 0x81eb3ffffffffff, + 0x81eb7ffffffffff, 0x81ebbffffffffff}; + int numCells = ARRAY_SIZE(kids); + + LinkedGeoPolygon polygon; + t_assertSuccess( + H3_EXPORT(cellsToLinkedMultiPolygon)(kids, numCells, &polygon)); + + // Since these are the children of a cell, we expect a single loop with + // no holes. + t_assert(countLinkedLoops(&polygon) == 1, "1 loop added to polygon"); + + H3_EXPORT(destroyLinkedMultiPolygon)(&polygon); + } + // TODO: This test asserts incorrect behavior - we should be creating // multiple polygons, each with their own single loop. Update when the // algorithm is corrected. diff --git a/src/h3lib/include/linkedGeo.h b/src/h3lib/include/linkedGeo.h index 1abc341d51..fee45ac920 100644 --- a/src/h3lib/include/linkedGeo.h +++ b/src/h3lib/include/linkedGeo.h @@ -51,7 +51,7 @@ H3Error normalizeMultiPolygon(LinkedGeoPolygon *root); LinkedGeoPolygon *addNewLinkedPolygon(LinkedGeoPolygon *polygon); LinkedGeoLoop *addNewLinkedLoop(LinkedGeoPolygon *polygon); LinkedGeoLoop *addLinkedLoop(LinkedGeoPolygon *polygon, LinkedGeoLoop *loop); -LinkedLatLng *addLinkedCoord(LinkedGeoLoop *loop, const LatLng *vertex); +LinkedLatLng *addLinkedCoord(LinkedGeoLoop *loop, H3Index vertex); int countLinkedPolygons(LinkedGeoPolygon *polygon); int countLinkedLoops(LinkedGeoPolygon *polygon); int countLinkedCoords(LinkedGeoLoop *loop); diff --git a/src/h3lib/include/vertexGraph.h b/src/h3lib/include/vertexGraph.h index 5d17f59a67..bf95448ddf 100644 --- a/src/h3lib/include/vertexGraph.h +++ b/src/h3lib/include/vertexGraph.h @@ -30,8 +30,8 @@ */ typedef struct VertexNode VertexNode; struct VertexNode { - LatLng from; - LatLng to; + H3Index from; + H3Index to; VertexNode *next; }; @@ -49,21 +49,19 @@ void initVertexGraph(VertexGraph *graph, int numBuckets, int res); void destroyVertexGraph(VertexGraph *graph); -VertexNode *addVertexNode(VertexGraph *graph, const LatLng *fromVtx, - const LatLng *toVtx); +VertexNode *addVertexNode(VertexGraph *graph, H3Index fromVtx, H3Index toVtx); int removeVertexNode(VertexGraph *graph, VertexNode *node); -VertexNode *findNodeForEdge(const VertexGraph *graph, const LatLng *fromVtx, - const LatLng *toVtx); +VertexNode *findNodeForEdge(const VertexGraph *graph, H3Index fromVtx, + H3Index toVtx); -VertexNode *findNodeForVertex(const VertexGraph *graph, const LatLng *fromVtx); +VertexNode *findNodeForVertex(const VertexGraph *graph, H3Index fromVtx); VertexNode *firstVertexNode(const VertexGraph *graph); // Internal functions -uint32_t _hashVertex(const LatLng *vertex, int res, int numBuckets); -void _initVertexNode(VertexNode *node, const LatLng *fromVtx, - const LatLng *toVtx); +uint32_t _hashVertex(H3Index vertex, int numBuckets); +void _initVertexNode(VertexNode *node, H3Index fromVtx, H3Index toVtx); #endif diff --git a/src/h3lib/lib/algos.c b/src/h3lib/lib/algos.c index 1b20bf5efc..1d7589a22e 100644 --- a/src/h3lib/lib/algos.c +++ b/src/h3lib/lib/algos.c @@ -1077,9 +1077,9 @@ H3Error H3_EXPORT(polygonToCells)(const GeoPolygon *geoPolygon, int res, */ H3Error h3SetToVertexGraph(const H3Index *h3Set, const int numHexes, VertexGraph *graph) { - CellBoundary vertices; - LatLng *fromVtx; - LatLng *toVtx; + H3Index vertices[NUM_HEX_VERTS] = {0}; + H3Index fromVtx; + H3Index toVtx; VertexNode *edge; if (numHexes < 1) { // We still need to init the graph, or calls to destroyVertexGraph will @@ -1094,17 +1094,26 @@ H3Error h3SetToVertexGraph(const H3Index *h3Set, const int numHexes, initVertexGraph(graph, numBuckets, res); // Iterate through every hexagon for (int i = 0; i < numHexes; i++) { - H3Error boundaryErr = H3_EXPORT(cellToBoundary)(h3Set[i], &vertices); - if (boundaryErr) { + H3Error vertexesError = H3_EXPORT(cellToVertexes)(h3Set[i], vertices); + if (vertexesError) { // Destroy vertex graph as caller will not know to do so. destroyVertexGraph(graph); - return boundaryErr; + return vertexesError; } // iterate through every edge - for (int j = 0; j < vertices.numVerts; j++) { - fromVtx = &vertices.verts[j]; - toVtx = &vertices.verts[(j + 1) % vertices.numVerts]; + for (int j = 0; j < NUM_HEX_VERTS; j++) { + if (vertices[j] == H3_NULL) { + continue; + } + int next = j + 1; + if (vertices[next % NUM_HEX_VERTS] == H3_NULL) { + // There can only be one deleted vertex, for pentagons, so just advance past it. + next++; + } + fromVtx = vertices[j]; + toVtx = vertices[next % NUM_HEX_VERTS]; // If we've seen this edge already, it will be reversed + // TODO: Wrong order of parameters edge = findNodeForEdge(graph, toVtx, fromVtx); if (edge != NULL) { // If we've seen it, drop it. No edge is shared by more than 2 @@ -1132,17 +1141,17 @@ void _vertexGraphToLinkedGeo(VertexGraph *graph, LinkedGeoPolygon *out) { *out = (LinkedGeoPolygon){0}; LinkedGeoLoop *loop; VertexNode *edge; - LatLng nextVtx; + H3Index nextVtx; // Find the next unused entry point while ((edge = firstVertexNode(graph)) != NULL) { loop = addNewLinkedLoop(out); // Walk the graph to get the outline do { - addLinkedCoord(loop, &edge->from); + addLinkedCoord(loop, edge->from); nextVtx = edge->to; // Remove frees the node, so we can't use edge after this removeVertexNode(graph, edge); - edge = findNodeForVertex(graph, &nextVtx); + edge = findNodeForVertex(graph, nextVtx); } while (edge); } } diff --git a/src/h3lib/lib/linkedGeo.c b/src/h3lib/lib/linkedGeo.c index 85308e8a4c..4b6c41d050 100644 --- a/src/h3lib/lib/linkedGeo.c +++ b/src/h3lib/lib/linkedGeo.c @@ -23,6 +23,7 @@ #include #include "alloc.h" +#include "h3Assert.h" #include "h3api.h" #include "latLng.h" @@ -73,10 +74,16 @@ LinkedGeoLoop *addLinkedLoop(LinkedGeoPolygon *polygon, LinkedGeoLoop *loop) { * @param vertex Coordinate to add * @return Pointer to the coordinate */ -LinkedLatLng *addLinkedCoord(LinkedGeoLoop *loop, const LatLng *vertex) { +LinkedLatLng *addLinkedCoord(LinkedGeoLoop *loop, H3Index vertex) { LinkedLatLng *coord = H3_MEMORY(malloc)(sizeof(*coord)); assert(coord != NULL); - *coord = (LinkedLatLng){.vertex = *vertex, .next = NULL}; + LatLng vertexLatLng; + // TODO: Also add the distrortion vertex here + H3Error err = H3_EXPORT(vertexToLatLng)(vertex, &vertexLatLng); + if (NEVER(err)) { + return NULL; + } + *coord = (LinkedLatLng){.vertex = vertexLatLng, .next = NULL}; LinkedLatLng *last = loop->last; if (last == NULL) { assert(loop->first == NULL); diff --git a/src/h3lib/lib/vertexGraph.c b/src/h3lib/lib/vertexGraph.c index fd7c5299d0..e1bcb1c25b 100644 --- a/src/h3lib/lib/vertexGraph.c +++ b/src/h3lib/lib/vertexGraph.c @@ -62,25 +62,17 @@ void destroyVertexGraph(VertexGraph *graph) { /** * Get an integer hash for a lat/lng point, at a precision determined * by the current hexagon resolution. - * TODO: Light testing suggests this might not be sufficient at resolutions - * finer than 10. Design a better hash function if performance and collisions - * seem to be an issue here. * @param vertex Lat/lng vertex to hash - * @param res Resolution of the hexagon the vertex belongs to * @param numBuckets Number of buckets in the graph * @return Integer hash */ -uint32_t _hashVertex(const LatLng *vertex, int res, int numBuckets) { - // Simple hash: Take the sum of the lat and lng with a precision level - // determined by the resolution, converted to int, modulo bucket count. - return (uint32_t)fmod(fabs((vertex->lat + vertex->lng) * pow(10, 15 - res)), - numBuckets); +uint32_t _hashVertex(H3Index vertex, int numBuckets) { + return (uint32_t)(vertex % numBuckets); } -void _initVertexNode(VertexNode *node, const LatLng *fromVtx, - const LatLng *toVtx) { - node->from = *fromVtx; - node->to = *toVtx; +void _initVertexNode(VertexNode *node, H3Index fromVtx, H3Index toVtx) { + node->from = fromVtx; + node->to = toVtx; node->next = NULL; } @@ -91,14 +83,13 @@ void _initVertexNode(VertexNode *node, const LatLng *fromVtx, * @param toVtx End vertex * @return Pointer to the new node */ -VertexNode *addVertexNode(VertexGraph *graph, const LatLng *fromVtx, - const LatLng *toVtx) { +VertexNode *addVertexNode(VertexGraph *graph, H3Index fromVtx, H3Index toVtx) { // Make the new node VertexNode *node = H3_MEMORY(malloc)(sizeof(VertexNode)); assert(node != NULL); _initVertexNode(node, fromVtx, toVtx); // Determine location - uint32_t index = _hashVertex(fromVtx, graph->res, graph->numBuckets); + uint32_t index = _hashVertex(fromVtx, graph->numBuckets); // Check whether there's an existing node in that spot VertexNode *currentNode = graph->buckets[index]; if (currentNode == NULL) { @@ -108,8 +99,7 @@ VertexNode *addVertexNode(VertexGraph *graph, const LatLng *fromVtx, // Find the end of the list do { // Check the the edge we're adding doesn't already exist - if (geoAlmostEqual(¤tNode->from, fromVtx) && - geoAlmostEqual(¤tNode->to, toVtx)) { + if (currentNode->from == fromVtx && currentNode->to == toVtx) { // already exists, bail H3_MEMORY(free)(node); return currentNode; @@ -134,7 +124,7 @@ VertexNode *addVertexNode(VertexGraph *graph, const LatLng *fromVtx, */ int removeVertexNode(VertexGraph *graph, VertexNode *node) { // Determine location - uint32_t index = _hashVertex(&node->from, graph->res, graph->numBuckets); + uint32_t index = _hashVertex(node->from, graph->numBuckets); VertexNode *currentNode = graph->buckets[index]; int found = 0; if (currentNode != NULL) { @@ -168,17 +158,17 @@ int removeVertexNode(VertexGraph *graph, VertexNode *node) { * @param toVtx End vertex, or NULL if we don't care * @return Pointer to the vertex node, if found */ -VertexNode *findNodeForEdge(const VertexGraph *graph, const LatLng *fromVtx, - const LatLng *toVtx) { +VertexNode *findNodeForEdge(const VertexGraph *graph, H3Index fromVtx, + H3Index toVtx) { // Determine location - uint32_t index = _hashVertex(fromVtx, graph->res, graph->numBuckets); + uint32_t index = _hashVertex(fromVtx, graph->numBuckets); // Check whether there's an existing node in that spot VertexNode *node = graph->buckets[index]; if (node != NULL) { // Look through the list and see if we find the edge do { - if (geoAlmostEqual(&node->from, fromVtx) && - (toVtx == NULL || geoAlmostEqual(&node->to, toVtx))) { + if (node->from == fromVtx && + (toVtx == H3_NULL || node->to == toVtx)) { return node; } node = node->next; @@ -194,8 +184,8 @@ VertexNode *findNodeForEdge(const VertexGraph *graph, const LatLng *fromVtx, * @param fromVtx Start vertex * @return Pointer to the vertex node, if found */ -VertexNode *findNodeForVertex(const VertexGraph *graph, const LatLng *fromVtx) { - return findNodeForEdge(graph, fromVtx, NULL); +VertexNode *findNodeForVertex(const VertexGraph *graph, H3Index fromVtx) { + return findNodeForEdge(graph, fromVtx, H3_NULL); } /**