From c27c737f4b7d67306bbf2c432538a325b655e246 Mon Sep 17 00:00:00 2001 From: Kurt Smith Date: Wed, 11 Sep 2019 16:19:51 +0000 Subject: [PATCH 1/7] trivial fix to bboxHexRadius to guarantee containment --- src/h3lib/lib/bbox.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/h3lib/lib/bbox.c b/src/h3lib/lib/bbox.c index 90196b35d..19ed49dcb 100644 --- a/src/h3lib/lib/bbox.c +++ b/src/h3lib/lib/bbox.c @@ -117,5 +117,5 @@ int bboxHexRadius(const BBox* bbox, int res) { // any orientation of the GeoJSON encased in a circle defined by the // bounding box radius and center, it is guaranteed to fit in this k-ring // Rounded *up* to guarantee containment - return (int)ceil(bboxRadiusKm / (1.5 * centerHexRadiusKm)); + return (int)ceil(bboxRadiusKm / (1.0 * centerHexRadiusKm)); } From 83a39ccc94052e93fd0bde051ea9e8d8a46ecf2a Mon Sep 17 00:00:00 2001 From: Kurt Smith Date: Thu, 12 Sep 2019 13:11:09 +0000 Subject: [PATCH 2/7] fix failing test --- src/apps/testapps/testPolyfill.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/apps/testapps/testPolyfill.c b/src/apps/testapps/testPolyfill.c index df9db2e90..7a303a0d0 100644 --- a/src/apps/testapps/testPolyfill.c +++ b/src/apps/testapps/testPolyfill.c @@ -64,10 +64,10 @@ SUITE(polyfill) { TEST(maxPolyfillSize) { int numHexagons = H3_EXPORT(maxPolyfillSize)(&sfGeoPolygon, 9); - t_assert(numHexagons == 3169, "got expected max polyfill size"); + t_assert(numHexagons == 7057, "got expected max polyfill size"); numHexagons = H3_EXPORT(maxPolyfillSize)(&holeGeoPolygon, 9); - t_assert(numHexagons == 3169, "got expected max polyfill size (hole)"); + t_assert(numHexagons == 7057, "got expected max polyfill size (hole)"); numHexagons = H3_EXPORT(maxPolyfillSize)(&emptyGeoPolygon, 9); t_assert(numHexagons == 1, "got expected max polyfill size (empty)"); From 56d2ff59e2698f1ac236f3e0f9756116df3f4cc8 Mon Sep 17 00:00:00 2001 From: Kurt Smith Date: Fri, 13 Sep 2019 16:45:46 +0000 Subject: [PATCH 3/7] update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c486b16a9..aeab10506 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ The public API of this library consists of the functions declared in file ## [Unreleased] ### Fixed - `compact` handles zero length input correctly. (#278) +- `bboxHexRadius` scaling factor adjusted to guarantee containment for `polyfill`. (#279) ## [3.6.0] - 2019-08-12 ### Added From 9166882b53ade3cd524a7626b5a7b2bc440acb95 Mon Sep 17 00:00:00 2001 From: Kurt Smith Date: Sat, 14 Sep 2019 16:04:24 -0500 Subject: [PATCH 4/7] simplify code and update comment based on #279 discussion. --- src/h3lib/lib/bbox.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/h3lib/lib/bbox.c b/src/h3lib/lib/bbox.c index 19ed49dcb..a54022a4b 100644 --- a/src/h3lib/lib/bbox.c +++ b/src/h3lib/lib/bbox.c @@ -112,10 +112,14 @@ int bboxHexRadius(const BBox* bbox, int res) { // Determine the radius of the center hexagon double centerHexRadiusKm = _hexRadiusKm(H3_EXPORT(geoToH3)(¢er, res)); - // The closest point along a hexagon drawn through the center points - // of a k-ring aggregation is exactly 1.5 radii of the hexagon. For - // any orientation of the GeoJSON encased in a circle defined by the - // bounding box radius and center, it is guaranteed to fit in this k-ring - // Rounded *up* to guarantee containment - return (int)ceil(bboxRadiusKm / (1.0 * centerHexRadiusKm)); + // We use centerHexRadiusKm un-scaled and rounded *up* to guarantee + // containment ot the bbox. Ideal, undistorted hexagons could scale + // centerHexRadiusKm by a factor of up to 1.5, reducing bboxHexRadius. + // This is because the closest point along an undistorted hexagon drawn + // through the center points of a k-ring aggregation is exactly 1.5 radii + // of the hexagon. But there is distortion near pentagons, and for those + // cases, the scaling needs to be less than 1.5. Using the un-scaled value + // conservatively guarantees containment for all cases, at the expense of a + // larger bboxHexRadius. + return (int)ceil(bboxRadiusKm / centerHexRadiusKm); } From fc93d829cac2fb4266243386596aa37b8989ddc3 Mon Sep 17 00:00:00 2001 From: Kurt Smith Date: Sat, 14 Sep 2019 19:03:43 -0500 Subject: [PATCH 5/7] regression test for #136. --- CMakeLists.txt | 2 + src/apps/testapps/testPolyfill_GH136.c | 59 ++++++++++++++++++++++++++ 2 files changed, 61 insertions(+) create mode 100644 src/apps/testapps/testPolyfill_GH136.c diff --git a/CMakeLists.txt b/CMakeLists.txt index 68beac0f5..e4f1c6f4a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -143,6 +143,7 @@ set(OTHER_SOURCE_FILES src/apps/testapps/testVertexGraph.c src/apps/testapps/testCompact.c src/apps/testapps/testPolyfill.c + src/apps/testapps/testPolyfill_GH136.c src/apps/testapps/testPentagonIndexes.c src/apps/testapps/testKRing.c src/apps/testapps/testH3ToGeoBoundary.c @@ -506,6 +507,7 @@ if(BUILD_TESTING) add_h3_test(testH3SetToVertexGraph src/apps/testapps/testH3SetToVertexGraph.c) add_h3_test(testLinkedGeo src/apps/testapps/testLinkedGeo.c) add_h3_test(testPolyfill src/apps/testapps/testPolyfill.c) + add_h3_test(testPolyfill_GH136 src/apps/testapps/testPolyfill_GH136.c) add_h3_test(testVertexGraph src/apps/testapps/testVertexGraph.c) add_h3_test(testH3UniEdge src/apps/testapps/testH3UniEdge.c) add_h3_test(testGeoCoord src/apps/testapps/testGeoCoord.c) diff --git a/src/apps/testapps/testPolyfill_GH136.c b/src/apps/testapps/testPolyfill_GH136.c new file mode 100644 index 000000000..6d25276bd --- /dev/null +++ b/src/apps/testapps/testPolyfill_GH136.c @@ -0,0 +1,59 @@ +/* + * Copyright 2017-2018 Uber Technologies, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include +#include "algos.h" +#include "constants.h" +#include "geoCoord.h" +#include "h3Index.h" +#include "test.h" + +// https://github.com/uber/h3/issues/136 + +static GeoCoord testVerts[] = {{0.10068990369902957, 0.8920772174196191}, + {0.10032914690616246, 0.8915914753447348}, + {0.10033349237998787, 0.8915860128746426}, + {0.10069496685903621, 0.8920742194546231}}; +static Geofence testGeofence = {.numVerts = 4, .verts = testVerts}; +static GeoPolygon testPolygon; + +static int countActualHexagons(H3Index* hexagons, int numHexagons) { + int actualNumHexagons = 0; + for (int i = 0; i < numHexagons; i++) { + if (hexagons[i] != 0) { + actualNumHexagons++; + } + } + return actualNumHexagons; +} + +SUITE(polyfill_gh136) { + + testPolygon.geofence = testGeofence; + testPolygon.numHoles = 0; + + TEST(gh136) { + int res = 13; + int numHexagons = H3_EXPORT(maxPolyfillSize)(&testPolygon, res); + H3Index* hexagons = calloc(numHexagons, sizeof(H3Index)); + + H3_EXPORT(polyfill)(&testPolygon, res, hexagons); + int actualNumHexagons = countActualHexagons(hexagons, numHexagons); + + t_assert(actualNumHexagons == 4353, "got expected polyfill size"); + free(hexagons); + } +} From dcb5446e036531898242dd5af0b298e4f03533b3 Mon Sep 17 00:00:00 2001 From: Kurt Smith Date: Sat, 14 Sep 2019 19:22:47 -0500 Subject: [PATCH 6/7] fix formatting --- src/apps/testapps/testPolyfill_GH136.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/apps/testapps/testPolyfill_GH136.c b/src/apps/testapps/testPolyfill_GH136.c index 6d25276bd..08bae86cb 100644 --- a/src/apps/testapps/testPolyfill_GH136.c +++ b/src/apps/testapps/testPolyfill_GH136.c @@ -41,7 +41,6 @@ static int countActualHexagons(H3Index* hexagons, int numHexagons) { } SUITE(polyfill_gh136) { - testPolygon.geofence = testGeofence; testPolygon.numHoles = 0; From 2f5a7010cef2340125db8dd4b2d22945da03fe46 Mon Sep 17 00:00:00 2001 From: Kurt Smith Date: Mon, 16 Sep 2019 16:50:33 -0500 Subject: [PATCH 7/7] update copyright date --- src/apps/testapps/testPolyfill_GH136.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/apps/testapps/testPolyfill_GH136.c b/src/apps/testapps/testPolyfill_GH136.c index 08bae86cb..9c6504ce1 100644 --- a/src/apps/testapps/testPolyfill_GH136.c +++ b/src/apps/testapps/testPolyfill_GH136.c @@ -1,5 +1,5 @@ /* - * Copyright 2017-2018 Uber Technologies, Inc. + * Copyright 2017-2019 Uber Technologies, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License.