Use vertex mode for cellsToLinkedMultiPolygon

.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"]

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.

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);

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

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);
     }
 }

src/h3lib/lib/linkedGeo.c

@@ -23,6 +23,7 @@
 #include <stdlib.h>
 
 #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);

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(&currentNode->from, fromVtx) &&
-                geoAlmostEqual(&currentNode->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);
 }
 
 /**