Skip to content

Commit

Permalink
Improve consistency of GEOSVoronoiDiagram
Browse files Browse the repository at this point in the history 
**change**

the getDiagramEdges now only returns a GeometryCollection, whether there
is one or many lines. So `GEOSVoronoiDiagram` now can only return a
GeometryCollection itself.

**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 fact that we could have either a LineString or a MultiLineString is
quite confusing. And wrapping the whole result in a GeometryCollection
is actually [not a significant overhead][3]. Hence the choice of always
returning a GeometryCollection.

[1]: shapely/shapely#881 (comment)
[2]: rgeo/rgeo#334 (comment)
[3]: libgeos#702 (comment)
  • Loading branch information
@BuonOmo
BuonOmo committed Oct 17, 2022
1 parent d0b11bd commit dca2e40
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 24 deletions.
6 changes: 3 additions & 3 deletions include/geos/triangulate/VoronoiDiagramBuilder.h
View file
Expand Up @@ -107,13 +107,13 @@ class GEOS_DLL VoronoiDiagramBuilder {
std::unique_ptr<geom::GeometryCollection> getDiagram(const geom::GeometryFactory& geomFact);

/** \brief
* Gets the faces of the computed diagram as a {@link geom::MultiLineString},
* clipped as specified.
* Gets the faces of the computed diagram as a {@link geom::GeometryCollection}
* 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<geom::Geometry> getDiagramEdges(const geom::GeometryFactory& geomFact);
std::unique_ptr<geom::GeometryCollection> getDiagramEdges(const geom::GeometryFactory& geomFact);

private:

Expand Down
33 changes: 24 additions & 9 deletions src/triangulate/VoronoiDiagramBuilder.cpp
View file
Expand Up @@ -123,28 +123,43 @@ VoronoiDiagramBuilder::getDiagram(const geom::GeometryFactory& geomFact)
return ret;
}

std::unique_ptr<geom::Geometry>
std::unique_ptr<GeometryCollection>
VoronoiDiagramBuilder::getDiagramEdges(const geom::GeometryFactory& geomFact)
{
create();

if (!subdiv) {
return geomFact.createMultiLineString();
return geomFact.createGeometryCollection();
}

std::unique_ptr<geom::MultiLineString> edges = subdiv->getVoronoiDiagramEdges(geomFact);
if(edges->isEmpty()) {
return std::unique_ptr<Geometry>(edges.release());
return geomFact.createGeometryCollection();
}
std::unique_ptr<geom::Geometry> clipPoly(geomFact.toGeometry(&diagramEnv));
std::unique_ptr<Geometry> clipped(clipPoly->intersection(edges.get()));

if (clipped->getGeometryTypeId() == GEOS_LINESTRING) {
std::vector<std::unique_ptr<LineString>> lines;
lines.emplace_back(static_cast<LineString*>(clipped.release()));
return geomFact.createMultiLineString(std::move(lines));
} else {
return clipped;
switch (clipped->getGeometryTypeId()) {
case GEOS_LINESTRING: {
std::vector<std::unique_ptr<Geometry>> vec;
vec.reserve(1);
vec.emplace_back(clipped.release());
return geomFact.createGeometryCollection(std::move(vec));
}
case GEOS_MULTILINESTRING: {
auto mls = static_cast<MultiLineString*>(clipped.release());
size_t len = mls->getNumGeometries();
std::vector<std::unique_ptr<Geometry>> vec;
vec.reserve(len);
for (size_t i = 0; i < len; i++)
{
vec.emplace_back(mls->getGeometryN(i)->clone());
}
return geomFact.createGeometryCollection(std::move(vec));
}
default: {
throw util::GEOSException("Unknown state");
}
}
}

Expand Down
13 changes: 6 additions & 7 deletions tests/unit/capi/GEOSVoronoiDiagramTest.cpp
View file
Expand Up @@ -47,7 +47,7 @@ void object::test<1>

GEOSGeom_destroy(geom2_);
geom2_ = GEOSVoronoiDiagram(geom1_, nullptr, 0, 1);
ensure_geometry_equals(geom2_, "MULTILINESTRING EMPTY");
ensure_geometry_equals(geom2_, "GEOMETRYCOLLECTION EMPTY");
}

//More points:
Expand All @@ -65,7 +65,7 @@ void object::test<2>
GEOSGeom_destroy(geom2_);
geom2_ = GEOSVoronoiDiagram(geom1_, nullptr, 0, 1);
ensure_geometry_equals(geom2_,
"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))");
"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))");
}
//Larger number of points:
template<>
Expand All @@ -82,7 +82,7 @@ void object::test<3>
GEOSGeom_destroy(geom2_);
geom2_ = GEOSVoronoiDiagram(geom1_, nullptr, 0, 1);
ensure_geometry_equals(geom2_,
"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))");
"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))");
}
//Test with non-zero Tolerance value
template<>
Expand All @@ -99,7 +99,7 @@ void object::test<4>
GEOSGeom_destroy(geom2_);
geom2_ = GEOSVoronoiDiagram(geom1_, nullptr, 10, 1);
ensure_geometry_equals(geom2_,
"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))");
"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))");
}
template<>
template<>
Expand All @@ -115,7 +115,7 @@ void object::test<5>
GEOSGeom_destroy(geom2_);
geom2_ = GEOSVoronoiDiagram(geom1_, nullptr, 10, 1);
ensure_geometry_equals(geom2_,
"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))");
"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))");
}
template<>
template<>
Expand All @@ -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_,
"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))");
"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))");
}

template<>
Expand All @@ -140,4 +140,3 @@ void object::test<7>
}

} // namespace tut

16 changes: 11 additions & 5 deletions tests/unit/triangulate/VoronoiTest.cpp
View file
Expand Up @@ -236,19 +236,25 @@ void object::test<10>
runVoronoi(wkt, expected, 0);
}

// Consistency in the return value for edges
// Consistency in the return value.
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 = "MULTILINESTRING ((0 30, 30 0))";
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* wkt2 = "LINESTRING (10 10, 20 20, 30 30)";
const char* expected2 = "MULTILINESTRING ((0 50, 50 0), (-10 40, 40 -10))";
const char* expected3 = "GEOMETRYCOLLECTION (LINESTRING (0 50, 50 0), LINESTRING (-10 40, 40 -10))";

runVoronoi(wkt1, expected1, 0, true);
runVoronoi(wkt2, expected2, 0, true);
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);
}

} // namespace tut

0 comments on commit dca2e40

Please sign in to comment.