[geos-commits] [SCM] GEOS branch main updated. 82633dd536e15b7b9842da81f6458d01611f71a1
git at osgeo.org
git at osgeo.org
Thu Jun 29 16:14:34 PDT 2023
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "GEOS".
The branch, main has been updated
via 82633dd536e15b7b9842da81f6458d01611f71a1 (commit)
from 29b8f8e55bcce33992d64d9e1231dbb833268034 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit 82633dd536e15b7b9842da81f6458d01611f71a1
Author: Paul Ramsey <pramsey at cleverelephant.ca>
Date: Thu Jun 29 16:13:12 2023 -0700
Improve Convex Hull performance by avoiding duplicate uniquing
Port of https://github.com/locationtech/jts/pull/985
Added a short-circuit facility to CoordinateFilter (isDone)
to avoid rebuilding unique coordinate facility.
diff --git a/NEWS.md b/NEWS.md
index df0b6d7d3..b3b2d3fa3 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -8,6 +8,7 @@
- Fixes/Improvements:
- WKTReader: Points with all-NaN coordinates are not considered empty anymore (GH-927, Casper van der Wel)
- WKTWriter: Points with all-NaN coordinates are written as such (GH-927, Casper van der Wel)
+ - ConvexHull: Performance improvement for larger geometries (JTS-985, Martin Davis)
## Changes in 3.12.0
diff --git a/include/geos/algorithm/ConvexHull.h b/include/geos/algorithm/ConvexHull.h
index a3dfb7a8a..048d3aa71 100644
--- a/include/geos/algorithm/ConvexHull.h
+++ b/include/geos/algorithm/ConvexHull.h
@@ -31,6 +31,7 @@
#include <geos/geom/CoordinateSequence.h>
#include <geos/geom/Geometry.h>
#include <geos/util/UniqueCoordinateArrayFilter.h>
+#include <geos/util/CoordinateArrayFilter.h>
#ifdef _MSC_VER
#pragma warning(push)
@@ -60,27 +61,26 @@ namespace algorithm { // geos::algorithm
*/
class GEOS_DLL ConvexHull {
private:
+
+ static constexpr std::size_t TUNING_REDUCE_SIZE = 50;
+
+ const geom::Geometry* inputGeom;
const geom::GeometryFactory* geomFactory;
geom::Coordinate::ConstVect inputPts;
- void
- extractCoordinates(const geom::Geometry* geom)
- {
- util::UniqueCoordinateArrayFilter filter(inputPts);
- geom->apply_ro(&filter);
- }
-
/// Create a CoordinateSequence from the Coordinate::ConstVect
/// This is needed to construct the geometries.
/// Here coordinate copies happen
/// The returned object is newly allocated !NO EXCEPTION SAFE!
std::unique_ptr<geom::CoordinateSequence> toCoordinateSequence(geom::Coordinate::ConstVect& cv);
- void computeOctPts(const geom::Coordinate::ConstVect& src,
- geom::Coordinate::ConstVect& tgt);
+ void computeInnerOctolateralPts(
+ const geom::Coordinate::ConstVect& src,
+ geom::Coordinate::ConstVect& tgt);
- bool computeOctRing(const geom::Coordinate::ConstVect& src,
- geom::Coordinate::ConstVect& tgt);
+ bool computeInnerOctolateralRing(
+ const geom::Coordinate::ConstVect& src,
+ geom::Coordinate::ConstVect& tgt);
/**
* Uses a heuristic to reduce the number of points scanned
@@ -132,7 +132,8 @@ private:
* equal to or greater than q
*/
int polarCompare(const geom::Coordinate& o,
- const geom::Coordinate& p, const geom::Coordinate& q);
+ const geom::Coordinate& p,
+ const geom::Coordinate& q);
void grahamScan(const geom::Coordinate::ConstVect& c,
geom::Coordinate::ConstVect& ps);
@@ -159,7 +160,15 @@ private:
* @return whether the three coordinates are collinear
* and c2 lies between c1 and c3 inclusive
*/
- bool isBetween(const geom::Coordinate& c1, const geom::Coordinate& c2, const geom::Coordinate& c3);
+ bool isBetween(
+ const geom::Coordinate& c1,
+ const geom::Coordinate& c2,
+ const geom::Coordinate& c3);
+
+ bool extractUnique(geom::Coordinate::ConstVect& pts, std::size_t maxPts);
+ std::unique_ptr<geom::Geometry> createFewPointsResult();
+
+
public:
@@ -167,10 +176,9 @@ public:
* Create a new convex hull construction for the input Geometry.
*/
ConvexHull(const geom::Geometry* newGeometry)
- : geomFactory(newGeometry->getFactory())
- {
- extractCoordinates(newGeometry);
- };
+ : inputGeom(newGeometry)
+ , geomFactory(newGeometry->getFactory())
+ {};
~ConvexHull() {};
diff --git a/include/geos/geom/CoordinateFilter.h b/include/geos/geom/CoordinateFilter.h
index 47b30d806..2555a79b2 100644
--- a/include/geos/geom/CoordinateFilter.h
+++ b/include/geos/geom/CoordinateFilter.h
@@ -45,6 +45,11 @@ public:
virtual
~CoordinateFilter() {}
+ virtual bool isDone() const
+ {
+ return false;
+ }
+
/** \brief
* Performs an operation on `coord`.
*
diff --git a/include/geos/geom/CoordinateSequence.h b/include/geos/geom/CoordinateSequence.h
index 34a725bd3..b49b8693f 100644
--- a/include/geos/geom/CoordinateSequence.h
+++ b/include/geos/geom/CoordinateSequence.h
@@ -621,10 +621,30 @@ public:
template<typename Filter>
void apply_rw(const Filter* filter) {
switch(getCoordinateType()) {
- case CoordinateType::XY: for (auto& c : items<CoordinateXY>()) { filter->filter_rw(&c); } break;
- case CoordinateType::XYZ: for (auto& c : items<Coordinate>()) { filter->filter_rw(&c); } break;
- case CoordinateType::XYM: for (auto& c : items<CoordinateXYM>()) { filter->filter_rw(&c); } break;
- case CoordinateType::XYZM: for (auto& c : items<CoordinateXYZM>()) { filter->filter_rw(&c); } break;
+ case CoordinateType::XY:
+ for (auto& c : items<CoordinateXY>()) {
+ if (filter->isDone()) break;
+ filter->filter_rw(&c);
+ }
+ break;
+ case CoordinateType::XYZ:
+ for (auto& c : items<Coordinate>()) {
+ if (filter->isDone()) break;
+ filter->filter_rw(&c);
+ }
+ break;
+ case CoordinateType::XYM:
+ for (auto& c : items<CoordinateXYM>()) {
+ if (filter->isDone()) break;
+ filter->filter_rw(&c);
+ }
+ break;
+ case CoordinateType::XYZM:
+ for (auto& c : items<CoordinateXYZM>()) {
+ if (filter->isDone()) break;
+ filter->filter_rw(&c);
+ }
+ break;
}
m_hasdim = m_hasz = false; // re-check (see http://trac.osgeo.org/geos/ticket/435)
}
@@ -632,10 +652,30 @@ public:
template<typename Filter>
void apply_ro(Filter* filter) const {
switch(getCoordinateType()) {
- case CoordinateType::XY: for (const auto& c : items<CoordinateXY>()) { filter->filter_ro(&c); } break;
- case CoordinateType::XYZ: for (const auto& c : items<Coordinate>()) { filter->filter_ro(&c); } break;
- case CoordinateType::XYM: for (const auto& c : items<CoordinateXYM>()) { filter->filter_ro(&c); } break;
- case CoordinateType::XYZM: for (const auto& c : items<CoordinateXYZM>()) { filter->filter_ro(&c); } break;
+ case CoordinateType::XY:
+ for (const auto& c : items<CoordinateXY>()) {
+ if (filter->isDone()) break;
+ filter->filter_ro(&c);
+ }
+ break;
+ case CoordinateType::XYZ:
+ for (const auto& c : items<Coordinate>()) {
+ if (filter->isDone()) break;
+ filter->filter_ro(&c);
+ }
+ break;
+ case CoordinateType::XYM:
+ for (const auto& c : items<CoordinateXYM>()) {
+ if (filter->isDone()) break;
+ filter->filter_ro(&c);
+ }
+ break;
+ case CoordinateType::XYZM:
+ for (const auto& c : items<CoordinateXYZM>()) {
+ if (filter->isDone()) break;
+ filter->filter_ro(&c);
+ }
+ break;
}
}
diff --git a/include/geos/util/CoordinateArrayFilter.h b/include/geos/util/CoordinateArrayFilter.h
index 9ef7e4242..0bed5c406 100644
--- a/include/geos/util/CoordinateArrayFilter.h
+++ b/include/geos/util/CoordinateArrayFilter.h
@@ -17,46 +17,69 @@
#include <geos/export.h>
#include <cassert>
+#include <set>
+#include <vector>
#include <geos/geom/CoordinateFilter.h>
#include <geos/geom/CoordinateSequence.h>
#include <geos/geom/Coordinate.h>
+#ifdef _MSC_VER
+#pragma warning(push)
+#pragma warning(disable: 4251) // warning C4251: needs to have dll-interface to be used by clients of class
+#endif
+
namespace geos {
namespace util { // geos::util
-/** \brief
- * A CoordinateFilter that adds read-only pointers
- * to every Coordinate in a Geometry to a given
- * vector.
- *
- * Last port: util/CoordinateArrayFilter.java rev. 1.15
+/*
+ * A CoordinateFilter that fills a vector of Coordinate const pointers.
*/
-class GEOS_DLL CoordinateArrayFilter: public geom::CoordinateFilter {
-private:
- geom::Coordinate::ConstVect& pts; // target vector reference
+class GEOS_DLL CoordinateArrayFilter : public geom::CoordinateInspector<CoordinateArrayFilter> {
public:
- /** \brief
- * Constructs a CoordinateArrayFilter.
- *
- * @param target The destination vector.
- */
- CoordinateArrayFilter(geom::Coordinate::ConstVect& target)
- :
- pts(target)
+
+ CoordinateArrayFilter(std::vector<const geom::Coordinate*>& target)
+ : pts(target)
{}
- virtual
- ~CoordinateArrayFilter() {}
+ /**
+ * Destructor.
+ * Virtual dctor promises appropriate behaviour when someone will
+ * delete a derived-class object via a base-class pointer.
+ * http://www.parashift.com/c++-faq-lite/virtual-functions.html#faq-20.7
+ */
+ ~CoordinateArrayFilter() override {}
- virtual void
- filter_ro(const geom::Coordinate* coord)
+ template<typename CoordType>
+ void filter(const CoordType* coord)
{
pts.push_back(coord);
}
+
+ void filter(const geom::CoordinateXY*) {
+ assert(0); // not supported
+ }
+
+ void filter(const geom::CoordinateXYM*) {
+ assert(0); // not supported
+ }
+
+private:
+ std::vector<const geom::Coordinate*>& pts; // target set reference
+
+ // Declare type as noncopyable
+ CoordinateArrayFilter(const CoordinateArrayFilter& other) = delete;
+ CoordinateArrayFilter& operator=(const CoordinateArrayFilter& rhs) = delete;
};
-} // namespace geos.util
+
+
+
+} // namespace geos::util
} // namespace geos
+#ifdef _MSC_VER
+#pragma warning(pop)
+#endif
+
diff --git a/include/geos/util/UniqueCoordinateArrayFilter.h b/include/geos/util/UniqueCoordinateArrayFilter.h
index 17371169a..5b0afb025 100644
--- a/include/geos/util/UniqueCoordinateArrayFilter.h
+++ b/include/geos/util/UniqueCoordinateArrayFilter.h
@@ -47,6 +47,12 @@ public:
*/
UniqueCoordinateArrayFilter(std::vector<const geom::Coordinate*>& target)
: pts(target)
+ , maxUnique(NO_COORD_INDEX)
+ {}
+
+ UniqueCoordinateArrayFilter(std::vector<const geom::Coordinate*>& target, std::size_t p_maxUnique)
+ : pts(target)
+ , maxUnique(p_maxUnique)
{}
/**
@@ -69,6 +75,9 @@ public:
// TODO make `pts` a CoordinateSequence rather than coercing the type
pts.push_back(coord);
}
+ if(maxUnique != NO_COORD_INDEX && uniqPts.size() > maxUnique) {
+ done = true;
+ }
}
void filter(const geom::CoordinateXY*) {
@@ -80,15 +89,22 @@ public:
assert(0); // not supported
}
+ bool isDone() const override {
+ return done;
+ }
+
private:
std::vector<const geom::Coordinate*>& pts; // target set reference
std::set<const geom::CoordinateXY*, geom::CoordinateLessThan> uniqPts; // unique points set
+ std::size_t maxUnique; // stop visiting when we have this many unique coordinates
+ bool done = false;
// Declare type as noncopyable
UniqueCoordinateArrayFilter(const UniqueCoordinateArrayFilter& other) = delete;
UniqueCoordinateArrayFilter& operator=(const UniqueCoordinateArrayFilter& rhs) = delete;
};
+
} // namespace geos::util
} // namespace geos
diff --git a/src/algorithm/ConvexHull.cpp b/src/algorithm/ConvexHull.cpp
index 205ca1ad4..7d8e34374 100644
--- a/src/algorithm/ConvexHull.cpp
+++ b/src/algorithm/ConvexHull.cpp
@@ -126,7 +126,7 @@ ConvexHull::toCoordinateSequence(Coordinate::ConstVect& cv)
/* private */
void
-ConvexHull::computeOctPts(const Coordinate::ConstVect& p_inputPts,
+ConvexHull::computeInnerOctolateralPts(const Coordinate::ConstVect& p_inputPts,
Coordinate::ConstVect& pts)
{
// Initialize all slots with first input coordinate
@@ -164,10 +164,10 @@ ConvexHull::computeOctPts(const Coordinate::ConstVect& p_inputPts,
/* private */
bool
-ConvexHull::computeOctRing(const Coordinate::ConstVect& p_inputPts,
+ConvexHull::computeInnerOctolateralRing(const Coordinate::ConstVect& p_inputPts,
Coordinate::ConstVect& dest)
{
- computeOctPts(p_inputPts, dest);
+ computeInnerOctolateralPts(p_inputPts, dest);
// Remove consecutive equal Coordinates
// unique() returns an iterator to the end of the resulting
@@ -191,7 +191,7 @@ ConvexHull::reduce(Coordinate::ConstVect& pts)
{
Coordinate::ConstVect polyPts;
- if(! computeOctRing(pts, polyPts)) {
+ if(! computeInnerOctolateralRing(pts, polyPts)) {
// unable to compute interior polygon for some reason
return;
}
@@ -234,27 +234,23 @@ ConvexHull::padArray3(geom::Coordinate::ConstVect& pts)
std::unique_ptr<Geometry>
ConvexHull::getConvexHull()
{
- std::size_t nInputPts = inputPts.size();
-
- if(nInputPts == 0) { // Return an empty geometry
- return geomFactory->createEmptyGeometry();
- }
+ std::unique_ptr<Geometry> fewPointsGeom = createFewPointsResult();
+ if (fewPointsGeom != nullptr)
+ return fewPointsGeom;
- if(nInputPts == 1) { // Return a Point
- // Copy the Coordinate from the ConstVect
- return std::unique_ptr<Geometry>(geomFactory->createPoint(*(inputPts[0])));
- }
+ util::CoordinateArrayFilter filter(inputPts);
+ inputGeom->apply_ro(&filter);
- if(nInputPts == 2) { // Return a LineString
- // Copy all Coordinates from the ConstVect
- auto cs = toCoordinateSequence(inputPts);
- return geomFactory->createLineString(std::move(cs));
- }
+ std::size_t nInputPts = inputPts.size();
// use heuristic to reduce points, if large
- if(nInputPts > 50) {
+ if(nInputPts > TUNING_REDUCE_SIZE) {
reduce(inputPts);
}
+ else {
+ inputPts.clear();
+ extractUnique(inputPts, NO_COORD_INDEX);
+ }
GEOS_CHECK_FOR_INTERRUPTS();
@@ -399,5 +395,45 @@ ConvexHull::cleanRing(const Coordinate::ConstVect& original,
}
+/*
+* Returns true if the extraction is stopped
+* by the maxPts restriction. That is, if there
+* are yet more points to be processed.
+*/
+/* private */
+bool
+ConvexHull::extractUnique(Coordinate::ConstVect& pts, std::size_t maxPts)
+{
+ util::UniqueCoordinateArrayFilter filter(pts, maxPts);
+ inputGeom->apply_ro(&filter);
+ return filter.isDone();
+}
+
+
+/* private */
+std::unique_ptr<Geometry>
+ConvexHull::createFewPointsResult()
+{
+ Coordinate::ConstVect uniquePts;
+ bool ok = extractUnique(uniquePts, 2);
+
+ if (ok) {
+ return nullptr;
+ }
+
+ if(uniquePts.size() == 0) { // Return an empty geometry
+ return geomFactory->createEmptyGeometry();
+ }
+ else if(uniquePts.size() == 1) { // Return a Point
+ // Copy the Coordinate from the ConstVect
+ return std::unique_ptr<Geometry>(geomFactory->createPoint(*(uniquePts[0])));
+ }
+ else { // Return a LineString
+ auto cs = toCoordinateSequence(uniquePts);
+ return geomFactory->createLineString(std::move(cs));
+ }
+}
+
+
} // namespace geos.algorithm
} // namespace geos
diff --git a/tests/unit/algorithm/ConvexHullTest.cpp b/tests/unit/algorithm/ConvexHullTest.cpp
index 95ddf0e34..b265c2216 100644
--- a/tests/unit/algorithm/ConvexHullTest.cpp
+++ b/tests/unit/algorithm/ConvexHullTest.cpp
@@ -47,6 +47,8 @@ struct test_convexhull_data {
std::unique_ptr<Geometry> geom = rdr.read(wkt);
std::unique_ptr<Geometry> actual = geom->convexHull();
std::unique_ptr<Geometry> expected = rdr.read(wktExpected);
+ // std::cout << *actual << std::endl;
+ // std::cout << *expected << std::endl;
ensure_equals_geometry(expected.get(), actual.get());
}
@@ -71,6 +73,8 @@ void object::test<1>
"LINESTRING (30 220, 240 220)");
}
+
+
// 2 - Test convex hull of multipoint
template<>
template<>
@@ -167,4 +171,24 @@ void object::test<9>
);
}
+
+template<>
+template<>
+void object::test<10>
+()
+{
+ checkHull("LINESTRING (30 220, 240 220, 240 220, 240 220, 240 220)",
+ "LINESTRING (30 220, 240 220)");
+}
+
+template<>
+template<>
+void object::test<11>
+()
+{
+ checkHull("MULTIPOINT ((0 0), (5 1), (10 0), (5 0))",
+ "POLYGON ((0 0, 5 1, 10 0, 0 0))");
+}
+
+
} // namespace tut
-----------------------------------------------------------------------
Summary of changes:
NEWS.md | 1 +
include/geos/algorithm/ConvexHull.h | 42 ++++++++------
include/geos/geom/CoordinateFilter.h | 5 ++
include/geos/geom/CoordinateSequence.h | 56 ++++++++++++++++---
include/geos/util/CoordinateArrayFilter.h | 67 ++++++++++++++--------
include/geos/util/UniqueCoordinateArrayFilter.h | 16 ++++++
src/algorithm/ConvexHull.cpp | 74 ++++++++++++++++++-------
tests/unit/algorithm/ConvexHullTest.cpp | 24 ++++++++
8 files changed, 219 insertions(+), 66 deletions(-)
hooks/post-receive
--
GEOS
More information about the geos-commits
mailing list