From 755cdcfa6ca33b838bbbec79f132c5149e003f42 Mon Sep 17 00:00:00 2001 From: Ulysse Buonomo Date: Thu, 10 Nov 2022 11:25:43 +0100 Subject: [PATCH] proposal 2: multilinesting Improve consistency of GEOSVoronoiDiagram **change** the getDiagramEdges now only returns a MultiLineString, whether there is one or many lines. Which reduces to two the number of possible kind of geometries given by `GEOSVoronoiDiagram` **reason** I've been working on adding `voronoi_diagram` to RGeo. And looking at the [Shapely PR for voronoi diagram][1] and [mine in RGeo][2], it seems like the `GEOSVoronoiDiagram` CAPI method causes confusion by returning three possible kind of geometries: - `GeometryCollection` (when `onlyEdges` is false) - `MultiLineString` (`onlyEdges` is true and there are at least two lines) - `LineString` (`onlyEdges` is true and there is only one line) The parameter `onlyEdges` changes the outcome, which is ok to reason about. However, the fact that we could have either a LineString or a MultiLineString is way harder to work around in my opinion. [1]: shapely/shapely#881 (comment) [2]: rgeo/rgeo#334 (comment) --- .../geos/triangulate/VoronoiDiagramBuilder.h | 4 ++-- src/triangulate/VoronoiDiagramBuilder.cpp | 18 ++++++++---------- tests/unit/capi/GEOSVoronoiDiagramTest.cpp | 12 ++++++------ tests/unit/triangulate/VoronoiTest.cpp | 16 +++++----------- 4 files changed, 21 insertions(+), 29 deletions(-) diff --git a/include/geos/triangulate/VoronoiDiagramBuilder.h b/include/geos/triangulate/VoronoiDiagramBuilder.h index 24813f1588..fdb7c6d4a9 100644 --- a/include/geos/triangulate/VoronoiDiagramBuilder.h +++ b/include/geos/triangulate/VoronoiDiagramBuilder.h @@ -107,13 +107,13 @@ class GEOS_DLL VoronoiDiagramBuilder { std::unique_ptr getDiagram(const geom::GeometryFactory& geomFact); /** \brief - * Gets the faces of the computed diagram as a {@link geom::GeometryCollection} + * Gets the faces of the computed diagram as a {@link geom::MultiLineString} * of {@link geom::LineString}s, clipped as specified. * * @param geomFact the geometry factory to use to create the output * @return the faces of the diagram */ - std::unique_ptr getDiagramEdges(const geom::GeometryFactory& geomFact); + std::unique_ptr getDiagramEdges(const geom::GeometryFactory& geomFact); private: diff --git a/src/triangulate/VoronoiDiagramBuilder.cpp b/src/triangulate/VoronoiDiagramBuilder.cpp index 74689bff78..5e8a336d1c 100644 --- a/src/triangulate/VoronoiDiagramBuilder.cpp +++ b/src/triangulate/VoronoiDiagramBuilder.cpp @@ -123,33 +123,31 @@ VoronoiDiagramBuilder::getDiagram(const geom::GeometryFactory& geomFact) return ret; } -std::unique_ptr +std::unique_ptr VoronoiDiagramBuilder::getDiagramEdges(const geom::GeometryFactory& geomFact) { create(); if (!subdiv) { - return geomFact.createGeometryCollection(); + return geomFact.createMultiLineString(); } std::unique_ptr edges = subdiv->getVoronoiDiagramEdges(geomFact); if(edges->isEmpty()) { - return geomFact.createGeometryCollection(); + return edges; } std::unique_ptr clipPoly(geomFact.toGeometry(&diagramEnv)); std::unique_ptr clipped(clipPoly->intersection(edges.get())); switch (clipped->getGeometryTypeId()) { case GEOS_LINESTRING: { - std::vector> vec; - vec.reserve(1); - vec.emplace_back(clipped.release()); - return geomFact.createGeometryCollection(std::move(vec)); + std::vector> lines; + lines.emplace_back(static_cast(clipped.release())); + return geomFact.createMultiLineString(std::move(lines)); } case GEOS_MULTILINESTRING: { - return geomFact.createGeometryCollection( - static_cast(clipped.get())->releaseGeometries() - ); + std::unique_ptr mls(static_cast(clipped.release())); + return mls; } default: { throw util::GEOSException("Unknown state"); diff --git a/tests/unit/capi/GEOSVoronoiDiagramTest.cpp b/tests/unit/capi/GEOSVoronoiDiagramTest.cpp index 4eb621d16a..19f02c9bc5 100644 --- a/tests/unit/capi/GEOSVoronoiDiagramTest.cpp +++ b/tests/unit/capi/GEOSVoronoiDiagramTest.cpp @@ -47,7 +47,7 @@ void object::test<1> GEOSGeom_destroy(geom2_); geom2_ = GEOSVoronoiDiagram(geom1_, nullptr, 0, 1); - ensure_geometry_equals(geom2_, "GEOMETRYCOLLECTION EMPTY"); + ensure_geometry_equals(geom2_, "MULTILINESTRING EMPTY"); } //More points: @@ -65,7 +65,7 @@ void object::test<2> GEOSGeom_destroy(geom2_); geom2_ = GEOSVoronoiDiagram(geom1_, nullptr, 0, 1); ensure_geometry_equals(geom2_, - "GEOMETRYCOLLECTION (LINESTRING (310.3571428571428 500, 353.515625 298.59375), LINESTRING (353.515625 298.59375, 306.875 231.9642857142857), LINESTRING (306.875 231.9642857142857, 110 175.7142857142857), LINESTRING (589.1666666666666 -10, 306.875 231.9642857142857), LINESTRING (353.515625 298.59375, 590 204))"); + "MULTILINESTRING ((310.3571428571428 500, 353.515625 298.59375), (353.515625 298.59375, 306.875 231.9642857142857), (306.875 231.9642857142857, 110 175.7142857142857), (589.1666666666666 -10, 306.875 231.9642857142857), (353.515625 298.59375, 590 204))"); } //Larger number of points: template<> @@ -82,7 +82,7 @@ void object::test<3> GEOSGeom_destroy(geom2_); geom2_ = GEOSVoronoiDiagram(geom1_, nullptr, 0, 1); ensure_geometry_equals(geom2_, - "GEOMETRYCOLLECTION (LINESTRING (190 510, 213.9473684210526 342.3684210526316), LINESTRING (213.9473684210526 342.3684210526316, 195.625 296.5625), LINESTRING (195.625 296.5625, 0 329.1666666666667), LINESTRING (195.625 296.5625, 216 266), LINESTRING (216 266, 88.33333333333333 138.3333333333333), LINESTRING (88.33333333333333 138.3333333333333, 0 76.50000000000001), LINESTRING (213.9473684210526 342.3684210526316, 267 307), LINESTRING (267 307, 225 265), LINESTRING (225 265, 216 266), LINESTRING (245 245, 225 265), LINESTRING (267 307, 275.9160583941606 309.5474452554744), LINESTRING (275.9160583941606 309.5474452554744, 303.1666666666667 284), LINESTRING (303.1666666666667 284, 296.6666666666667 245), LINESTRING (296.6666666666667 245, 245 245), LINESTRING (245 245, 245 201), LINESTRING (245 201, 88.33333333333333 138.3333333333333), LINESTRING (245 201, 380 120), LINESTRING (380 120, 500 0), LINESTRING (343.7615384615385 510, 275.9160583941606 309.5474452554744), LINESTRING (296.6666666666667 245, 380 120), LINESTRING (500 334.9051724137931, 303.1666666666667 284))"); + "MULTILINESTRING ((190 510, 213.9473684210526 342.3684210526316), (213.9473684210526 342.3684210526316, 195.625 296.5625), (195.625 296.5625, 0 329.1666666666667), (195.625 296.5625, 216 266), (216 266, 88.33333333333333 138.3333333333333), (88.33333333333333 138.3333333333333, 0 76.50000000000001), (213.9473684210526 342.3684210526316, 267 307), (267 307, 225 265), (225 265, 216 266), (245 245, 225 265), (267 307, 275.9160583941606 309.5474452554744), (275.9160583941606 309.5474452554744, 303.1666666666667 284), (303.1666666666667 284, 296.6666666666667 245), (296.6666666666667 245, 245 245), (245 245, 245 201), (245 201, 88.33333333333333 138.3333333333333), (245 201, 380 120), (380 120, 500 0), (343.7615384615385 510, 275.9160583941606 309.5474452554744), (296.6666666666667 245, 380 120), (500 334.9051724137931, 303.1666666666667 284))"); } //Test with non-zero Tolerance value template<> @@ -99,7 +99,7 @@ void object::test<4> GEOSGeom_destroy(geom2_); geom2_ = GEOSVoronoiDiagram(geom1_, nullptr, 10, 1); ensure_geometry_equals(geom2_, - "GEOMETRYCOLLECTION (LINESTRING (185 215, 187.9268292682927 235.4878048780488), LINESTRING (187.9268292682927 235.4878048780488, 290 252.5), LINESTRING (185 140, 185 215), LINESTRING (185 215, 80 215), LINESTRING (100.8333333333334 340, 187.9268292682927 235.4878048780488))"); + "MULTILINESTRING ((185 215, 187.9268292682927 235.4878048780488), (187.9268292682927 235.4878048780488, 290 252.5), (185 140, 185 215), (185 215, 80 215), (100.8333333333334 340, 187.9268292682927 235.4878048780488))"); } template<> template<> @@ -115,7 +115,7 @@ void object::test<5> GEOSGeom_destroy(geom2_); geom2_ = GEOSVoronoiDiagram(geom1_, nullptr, 10, 1); ensure_geometry_equals(geom2_, - "GEOMETRYCOLLECTION (LINESTRING (45 770, 45 263.6473684210526), LINESTRING (45 263.6473684210526, -310 146.559649122807), LINESTRING (45 263.6473684210526, 59.16911764705881 267.8235294117647), LINESTRING (59.16911764705881 267.8235294117647, 239.4350649350649 179.4350649350649), LINESTRING (239.4350649350649 179.4350649350649, 241.3415637860082 151.9814814814815), LINESTRING (241.3415637860082 151.9814814814815, -310 -153.376923076923), LINESTRING (266.2 770, 181.9432314410481 418.9301310043668), LINESTRING (181.9432314410481 418.9301310043668, 59.16911764705881 267.8235294117647), LINESTRING (181.9432314410481 418.9301310043668, 311.875 251.875), LINESTRING (311.875 251.875, 239.4350649350649 179.4350649350649), LINESTRING (241.3415637860082 151.9814814814815, 433.3333333333333 -280), LINESTRING (701 344.5238095238096, 311.875 251.875))"); + "MULTILINESTRING ((45 770, 45 263.6473684210526), (45 263.6473684210526, -310 146.559649122807), (45 263.6473684210526, 59.16911764705881 267.8235294117647), (59.16911764705881 267.8235294117647, 239.4350649350649 179.4350649350649), (239.4350649350649 179.4350649350649, 241.3415637860082 151.9814814814815), (241.3415637860082 151.9814814814815, -310 -153.376923076923), (266.2 770, 181.9432314410481 418.9301310043668), (181.9432314410481 418.9301310043668, 59.16911764705881 267.8235294117647), (181.9432314410481 418.9301310043668, 311.875 251.875), (311.875 251.875, 239.4350649350649 179.4350649350649), (241.3415637860082 151.9814814814815, 433.3333333333333 -280), (701 344.5238095238096, 311.875 251.875))"); } template<> template<> @@ -125,7 +125,7 @@ void object::test<6> geom1_ = GEOSGeomFromWKT("MULTIPOINT ((123 245), (165 313), (240 310), (260 260), (180 210), (240 210))"); geom2_ = GEOSVoronoiDiagram(geom1_, nullptr, 0, 1); ensure_geometry_equals(geom2_, - "GEOMETRYCOLLECTION (LINESTRING (-14 376.5882352941176, 172.3651328095773 261.4803591470258), LINESTRING (172.3651328095773 261.4803591470258, 56.63157894736844 73), LINESTRING (172.3651328095773 261.4803591470258, 200.6640625 265.6015625), LINESTRING (200.6640625 265.6015625, 201 265.4), LINESTRING (201 265.4, 210 251), LINESTRING (210 251, 210 73), LINESTRING (208.04 450, 200.6640625 265.6015625), LINESTRING (397 343.8, 201 265.4), LINESTRING (210 251, 397 176.2))"); + "MULTILINESTRING ((-14 376.5882352941176, 172.3651328095773 261.4803591470258), (172.3651328095773 261.4803591470258, 56.63157894736844 73), (172.3651328095773 261.4803591470258, 200.6640625 265.6015625), (200.6640625 265.6015625, 201 265.4), (201 265.4, 210 251), (210 251, 210 73), (208.04 450, 200.6640625 265.6015625), (397 343.8, 201 265.4), (210 251, 397 176.2))"); } template<> diff --git a/tests/unit/triangulate/VoronoiTest.cpp b/tests/unit/triangulate/VoronoiTest.cpp index 6007eb2364..791a3d8ab7 100644 --- a/tests/unit/triangulate/VoronoiTest.cpp +++ b/tests/unit/triangulate/VoronoiTest.cpp @@ -237,25 +237,19 @@ void object::test<10> runVoronoi(wkt, expected, 0); } -// Consistency in the return value. +// Consistency in the return value for edges template<> template<> void object::test<11> () { - const char* wkt0 = "MULTIPOINT ((150 200))"; - const char* expected0 = "GEOMETRYCOLLECTION EMPTY"; const char* wkt1 = "LINESTRING (10 10, 20 20)"; - const char* expected1 = "GEOMETRYCOLLECTION (POLYGON ((0 30, 30 30, 30 0, 0 30)), POLYGON ((0 0, 0 30, 30 0, 0 0)))"; - const char* expected2 = "GEOMETRYCOLLECTION (LINESTRING (0 30, 30 0))"; + const char* expected1 = "MULTILINESTRING ((0 30, 30 0))"; const char* wkt2 = "LINESTRING (10 10, 20 20, 30 30)"; - const char* expected3 = "GEOMETRYCOLLECTION (LINESTRING (0 50, 50 0), LINESTRING (-10 40, 40 -10))"; + const char* expected2 = "MULTILINESTRING ((0 50, 50 0), (-10 40, 40 -10))"; - runVoronoi(wkt0, expected0, 0, false); - runVoronoi(wkt0, expected0, 0, true); - runVoronoi(wkt1, expected1, 0, false); - runVoronoi(wkt1, expected2, 0, true); - runVoronoi(wkt2, expected3, 0, true); + runVoronoi(wkt1, expected1, 0, true); + runVoronoi(wkt2, expected2, 0, true); } } // namespace tut