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 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)
  • Loading branch information
BuonOmo committed Oct 14, 2022
1 parent cc66726 commit c7daad6
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 8 deletions.
1 change: 0 additions & 1 deletion capi/geos_ts_c.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4019,4 +4019,3 @@ extern "C" {
}

} /* extern "C" */

5 changes: 2 additions & 3 deletions include/geos/triangulate/VoronoiDiagramBuilder.h
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,8 @@ class GEOS_DLL VoronoiDiagramBuilder {
std::unique_ptr<geom::GeometryCollection> getDiagram(const geom::GeometryFactory& geomFact);

/** \brief
* Gets the faces of the computed diagram as a geom::GeometryCollection
* of {@link geom::LineString}s, clipped as specified.
* Gets the faces of the computed diagram as a {@link geom::MultiLineString},
* clipped as specified.
*
* @param geomFact the geometry factory to use to create the output
* @return the faces of the diagram
Expand All @@ -132,4 +132,3 @@ class GEOS_DLL VoronoiDiagramBuilder {

} //namespace geos.triangulate
} //namespace geos

10 changes: 9 additions & 1 deletion src/triangulate/VoronoiDiagramBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,15 @@ VoronoiDiagramBuilder::getDiagramEdges(const geom::GeometryFactory& geomFact)
}
std::unique_ptr<geom::Geometry> clipPoly(geomFact.toGeometry(&diagramEnv));
std::unique_ptr<Geometry> clipped(clipPoly->intersection(edges.get()));
return clipped;

if (dynamic_cast<LineString*>(clipped.get())) {
std::unique_ptr<std::vector<Geometry*>> lines(new std::vector<Geometry*>());
lines->push_back(clipped.release());
std::unique_ptr<MultiLineString> multi(geomFact.createMultiLineString(lines.release()));
return multi;
} else {
return clipped;
}
}

std::unique_ptr<geom::GeometryCollection>
Expand Down
25 changes: 22 additions & 3 deletions tests/unit/triangulate/VoronoiTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,19 +58,23 @@ readTextOrHex(const char* wkt)

//helper function for running triangulation
void
runVoronoi(const char* sitesWkt, const char* expectedWkt, const double tolerance)
runVoronoi(const char* sitesWkt, const char* expectedWkt, const double tolerance, const bool onlyEdges = false)
{
WKTWriter writer;
geos::triangulate::VoronoiDiagramBuilder builder;
std::unique_ptr<Geometry> sites(readTextOrHex(sitesWkt));
std::unique_ptr<Geometry> expected(readTextOrHex(expectedWkt));
std::unique_ptr<GeometryCollection> results;
std::unique_ptr<Geometry> results;
const GeometryFactory& geomFact(*GeometryFactory::getDefaultInstance());
builder.setSites(*sites);

//set Tolerance:
builder.setTolerance(tolerance);
results = builder.getDiagram(geomFact);
if (onlyEdges) {
results = builder.getDiagramEdges(geomFact);
} else {
results = builder.getDiagram(geomFact);
}

results->normalize();
expected->normalize();
Expand Down Expand Up @@ -232,4 +236,19 @@ void object::test<10>
runVoronoi(wkt, expected, 0);
}

// Consistency in the return value for edges
template<>
template<>
void object::test<11>
()
{
const char* wkt1 = "LINESTRING (10 10, 20 20)";
const char* expected1 = "MULTILINESTRING ((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))";

runVoronoi(wkt1, expected1, 0, true);
runVoronoi(wkt2, expected2, 0, true);
}

} // namespace tut

0 comments on commit c7daad6

Please sign in to comment.