[geos-commits] [SCM] GEOS branch master updated. dc65705ca78ab009df92ed8aea6f4724b557c3a9
git at osgeo.org
git at osgeo.org
Fri Dec 13 08:58:57 PST 2019
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, master has been updated
via dc65705ca78ab009df92ed8aea6f4724b557c3a9 (commit)
via 128a5d6b68a7f81d32b9b4b510b409779893a43f (commit)
via e4501a221faffbc3294cecda7a170445312476f7 (commit)
via a3814e30a45ad8d5eb931af20cf4f0d27f62de28 (commit)
via 8c87d30fe43767469a60b4e4290bdf659eb800b8 (commit)
via 1bf16cdf5a4827b483a1f712e0597ccb243f58cb (commit)
via 56cc946a9d9b0969fb2a9a3cedda58554d92e554 (commit)
via 8c58bfb1460ddc0cb9db81dad8c1f344647470b7 (commit)
via 8b0f0cdbcec74441d008dc144a771e102718be20 (commit)
via 1e7fc88cfbbd2c22f896aebe0b3db31a9d88c24b (commit)
via 09bdf26f086796273556bb5149452bf53de50f50 (commit)
via adb7ca818bfd93e64acd7c2efda9ac9b0c982d02 (commit)
via 767cb5fe0fd11e20a4b91c875c14160beb0f3ee0 (commit)
from e5655619f9164c7e914cd66fda801c64b108425d (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 dc65705ca78ab009df92ed8aea6f4724b557c3a9
Merge: 128a5d6 56cc946
Author: Daniel Baston <dbaston at gmail.com>
Date: Fri Dec 13 11:58:42 2019 -0500
Merge branch 'indexed-nested-shell-tester'
commit 128a5d6b68a7f81d32b9b4b510b409779893a43f
Merge: e4501a2 a3814e3
Author: Daniel Baston <dbaston at gmail.com>
Date: Fri Dec 13 11:58:03 2019 -0500
Merge branch 'valid-perf2'
commit e4501a221faffbc3294cecda7a170445312476f7
Author: Daniel Baston <dbaston at gmail.com>
Date: Fri Dec 13 11:57:40 2019 -0500
Disable Travis scan-build job
Can re-enable in future if allow_failures is configured.
diff --git a/.travis.yml b/.travis.yml
index fb90667..e3f609c 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -103,17 +103,17 @@ matrix:
env:
- E="TOOL=cmake && BUILD_TYPE=Release && CXX=clang++ && ARCH=-m64 && CC=clang"
- - os: linux
- cache:
- apt: true
- directories:
- - $HOME/.ccache
- addons:
- apt:
- sources: *sources
- packages: ['clang','ccache','doxygen']
- env:
- - E="TOOL=autotools_scanbuild && BUILD_TYPE=Release && ARCH=-m64
+# - os: linux
+# cache:
+# apt: true
+# directories:
+# - $HOME/.ccache
+# addons:
+# apt:
+# sources: *sources
+# packages: ['clang','ccache','doxygen']
+# env:
+# - E="TOOL=autotools_scanbuild && BUILD_TYPE=Release && ARCH=-m64
before_install:
- eval "${E}"
commit a3814e30a45ad8d5eb931af20cf4f0d27f62de28
Author: Daniel Baston <dbaston at gmail.com>
Date: Tue Dec 10 07:44:00 2019 -0500
Remove using namespace std
diff --git a/src/geomgraph/EdgeRing.cpp b/src/geomgraph/EdgeRing.cpp
index 5680337..f99b661 100644
--- a/src/geomgraph/EdgeRing.cpp
+++ b/src/geomgraph/EdgeRing.cpp
@@ -41,7 +41,6 @@
#include <cassert>
#include <iostream> // for operator<<
-using namespace std;
using namespace geos::algorithm;
using namespace geos::geom;
@@ -179,7 +178,7 @@ EdgeRing::computeRing()
}
/*public*/
-vector<DirectedEdge*>&
+std::vector<DirectedEdge*>&
EdgeRing::getEdges()
{
testInvariant();
commit 8c87d30fe43767469a60b4e4290bdf659eb800b8
Author: Daniel Baston <dbaston at gmail.com>
Date: Sat Dec 7 23:00:03 2019 -0500
Build up EdgeRing points in std::vector, not CoordinateSequence
Improves performance by allowing use of vector::reserve() and
CoordinateSequence::toVector().
diff --git a/include/geos/geomgraph/EdgeRing.h b/include/geos/geomgraph/EdgeRing.h
index c117967..fa0312b 100644
--- a/include/geos/geomgraph/EdgeRing.h
+++ b/include/geos/geomgraph/EdgeRing.h
@@ -176,7 +176,7 @@ private:
/// the DirectedEdges making up this EdgeRing
std::vector<DirectedEdge*> edges;
- std::unique_ptr<geom::CoordinateArraySequence> pts;
+ std::vector<geom::Coordinate> pts;
// label stores the locations of each geometry on the
// face surrounded by this ring
diff --git a/src/geomgraph/EdgeRing.cpp b/src/geomgraph/EdgeRing.cpp
index 9e3c802..5680337 100644
--- a/src/geomgraph/EdgeRing.cpp
+++ b/src/geomgraph/EdgeRing.cpp
@@ -30,7 +30,6 @@
#include <geos/geomgraph/Label.h>
#include <geos/geomgraph/Position.h>
#include <geos/geom/CoordinateSequenceFactory.h>
-#include <geos/geom/CoordinateArraySequence.h>
#include <geos/geom/CoordinateSequence.h>
#include <geos/geom/GeometryFactory.h>
#include <geos/geom/LinearRing.h>
@@ -57,7 +56,6 @@ EdgeRing::EdgeRing(DirectedEdge* newStart,
holes(),
maxNodeDegree(-1),
edges(),
- pts(new CoordinateArraySequence()),
label(Location::UNDEF), // new Label(Location::UNDEF)),
ring(nullptr),
isHoleVar(false),
@@ -173,8 +171,9 @@ EdgeRing::computeRing()
if(ring != nullptr) {
return; // don't compute more than once
}
- isHoleVar = Orientation::isCCW(pts.get());
- ring = geometryFactory->createLinearRing(std::move(pts));
+ auto coordSeq = geometryFactory->getCoordinateSequenceFactory()->create(std::move(pts));
+ ring = geometryFactory->createLinearRing(std::move(coordSeq));
+ isHoleVar = Orientation::isCCW(ring->getCoordinatesRO());
testInvariant();
}
@@ -319,15 +318,15 @@ EdgeRing::addPoints(Edge* edge, bool isForward, bool isFirstEdge)
assert(edgePts);
size_t numEdgePts = edgePts->getSize();
- assert(pts);
-
+ pts.reserve(pts.size() + numEdgePts);
if(isForward) {
- size_t startIndex = 1;
if(isFirstEdge) {
- startIndex = 0;
- }
- for(size_t i = startIndex; i < numEdgePts; ++i) {
- pts->add(edgePts->getAt(i));
+ edgePts->toVector(pts);
+ return;
+ } else {
+ for(size_t i = 1; i < numEdgePts; ++i) {
+ pts.push_back(edgePts->getAt(i));
+ }
}
}
@@ -336,21 +335,18 @@ EdgeRing::addPoints(Edge* edge, bool isForward, bool isFirstEdge)
if(isFirstEdge) {
startIndex = numEdgePts;
}
- //for (int i=startIndex;i>=0;i--)
for(size_t i = startIndex; i > 0; --i) {
- pts->add(edgePts->getAt(i - 1));
+ pts.push_back(edgePts->getAt(i - 1));
}
}
testInvariant();
-
}
/*public*/
bool
EdgeRing::containsPoint(const Coordinate& p)
{
-
testInvariant();
assert(ring);
@@ -378,8 +374,6 @@ std::ostream&
operator<< (std::ostream& os, const EdgeRing& er)
{
os << "EdgeRing[" << &er << "]: "
- << std::endl
- << "Points: " << er.pts.get()
<< std::endl;
return os;
}
commit 1bf16cdf5a4827b483a1f712e0597ccb243f58cb
Author: Daniel Baston <dbaston at gmail.com>
Date: Fri Dec 6 11:34:29 2019 -0500
Optimize IndexedNesterRingTester
- Use IndexedPointInAreaLocator for ring containment tests
- Use envelope covers test instead of intersection test
- Reuse results vector across iterations
- Presize ring vector
The first change can bring a significant performance improvement
polygons that have many or complex rings. An example is a parcel or land
cover dataset that models the street network as a single polygon with
many holes. It's possible that the indexed locator could perform worse
in some cases but I did not find any in my testing.
The last three changes should have no adverse effect in any situation.
diff --git a/src/operation/valid/IndexedNestedRingTester.cpp b/src/operation/valid/IndexedNestedRingTester.cpp
index 6ce774a..c1a03cc 100644
--- a/src/operation/valid/IndexedNestedRingTester.cpp
+++ b/src/operation/valid/IndexedNestedRingTester.cpp
@@ -19,9 +19,10 @@
#include "IndexedNestedRingTester.h"
#include <geos/geom/LinearRing.h> // for use
-#include <geos/algorithm/PointLocation.h> // for use
+#include <geos/algorithm/locate/IndexedPointInAreaLocator.h>
#include <geos/operation/valid/IsValidOp.h> // for use (findPtNotNode)
#include <geos/index/strtree/STRtree.h> // for use
+#include <geos/geom/Location.h>
// Forward declarations
namespace geos {
@@ -40,32 +41,35 @@ IndexedNestedRingTester::isNonNested()
{
buildIndex();
+ std::vector<void*> results;
for(size_t i = 0, n = rings.size(); i < n; ++i) {
- const geom::LinearRing* innerRing = rings[i];
- const geom::CoordinateSequence* innerRingPts = innerRing->getCoordinatesRO();
- std::vector<void*> results;
- index->query(innerRing->getEnvelopeInternal(), results);
- for(size_t j = 0, jn = results.size(); j < jn; ++j) {
- const geom::LinearRing* searchRing = static_cast<const geom::LinearRing*>(results[j]);
- const geom::CoordinateSequence* searchRingPts = searchRing->getCoordinatesRO();
-
- if(innerRing == searchRing) {
+ results.clear();
+
+ const geom::LinearRing* outerRing = rings[i];
+
+ geos::algorithm::locate::IndexedPointInAreaLocator locator(*outerRing);
+
+ index->query(outerRing->getEnvelopeInternal(), results);
+ for(const auto& result : results) {
+ const geom::LinearRing* possibleInnerRing = static_cast<const geom::LinearRing*>(result);
+ const geom::CoordinateSequence* possibleInnerRingPts = possibleInnerRing->getCoordinatesRO();
+
+ if(outerRing == possibleInnerRing) {
continue;
}
- if(!innerRing->getEnvelopeInternal()->intersects(
- searchRing->getEnvelopeInternal())) {
+ if(!outerRing->getEnvelopeInternal()->covers(possibleInnerRing->getEnvelopeInternal())) {
continue;
}
const geom::Coordinate* innerRingPt =
- IsValidOp::findPtNotNode(innerRingPts,
- searchRing,
+ IsValidOp::findPtNotNode(possibleInnerRingPts,
+ outerRing,
graph);
/*
* If no non-node pts can be found, this means
- * that the searchRing touches ALL of the innerRing vertices.
+ * that the possibleInnerRing touches ALL of the outerRing vertices.
* This indicates an invalid polygon, since either
* the two holes create a disconnected interior,
* or they touch in an infinite number of points
@@ -77,12 +81,7 @@ IndexedNestedRingTester::isNonNested()
continue;
}
- // Unable to find a ring point not a node of
- // the search ring
- assert(innerRingPt != nullptr);
-
- bool isInside = algorithm::PointLocation::isInRing(
- *innerRingPt, searchRingPts);
+ bool isInside = locator.locate(innerRingPt) != geom::Location::EXTERIOR;
if(isInside) {
nestedPt = innerRingPt;
diff --git a/src/operation/valid/IndexedNestedRingTester.h b/src/operation/valid/IndexedNestedRingTester.h
index f7b8c33..3e07e3e 100644
--- a/src/operation/valid/IndexedNestedRingTester.h
+++ b/src/operation/valid/IndexedNestedRingTester.h
@@ -19,6 +19,7 @@
#ifndef GEOS_OP_VALID_OFFSETCURVEVERTEXLIST_H
#define GEOS_OP_VALID_OFFSETCURVEVERTEXLIST_H
+#include <cstddef>
#include <vector> // for composition
// Forward declarations
@@ -49,13 +50,13 @@ namespace valid { // geos.operation.valid
class IndexedNestedRingTester {
public:
// @param newGraph : ownership retained by caller
- IndexedNestedRingTester(geomgraph::GeometryGraph* newGraph)
+ IndexedNestedRingTester(geomgraph::GeometryGraph* newGraph, size_t initialCapacity)
:
graph(newGraph),
- //totalEnv(0),
index(nullptr),
nestedPt(nullptr)
{
+ rings.reserve(initialCapacity);
}
~IndexedNestedRingTester();
@@ -89,9 +90,6 @@ private:
/// Ownership of this vector elements are externally owned
std::vector<const geom::LinearRing*> rings;
- // CHECK: Owned by (seems unused)?
- //geom::Envelope* totalEnv;
-
// Owned by us (use unique_ptr ?)
geos::index::SpatialIndex* index; // 'index' in JTS
diff --git a/src/operation/valid/IsValidOp.cpp b/src/operation/valid/IsValidOp.cpp
index 2416333..1d0e4a0 100644
--- a/src/operation/valid/IsValidOp.cpp
+++ b/src/operation/valid/IsValidOp.cpp
@@ -466,9 +466,9 @@ IsValidOp::checkHolesNotNested(const Polygon* p, GeometryGraph* graph)
//SimpleNestedRingTester nestedTester(graph);
//SweeplineNestedRingTester nestedTester(graph);
//QuadtreeNestedRingTester nestedTester(graph);
- IndexedNestedRingTester nestedTester(graph);
-
auto nholes = p->getNumInteriorRing();
+
+ IndexedNestedRingTester nestedTester(graph, nholes);
for (size_t i = 0; i < nholes; ++i) {
const LinearRing* innerHole = p->getInteriorRingN(i);
commit 56cc946a9d9b0969fb2a9a3cedda58554d92e554
Author: Daniel Baston <dbaston at gmail.com>
Date: Sat Dec 7 23:00:26 2019 -0500
Avoid nested shell check for single-part MultiPolygons
diff --git a/src/operation/valid/IsValidOp.cpp b/src/operation/valid/IsValidOp.cpp
index d38deba..5ebd4a0 100644
--- a/src/operation/valid/IsValidOp.cpp
+++ b/src/operation/valid/IsValidOp.cpp
@@ -326,7 +326,9 @@ IsValidOp::checkValid(const MultiPolygon* g)
}
}
- checkShellsNotNested(g, &graph);
+ if (ngeoms > 1) {
+ checkShellsNotNested(g, &graph);
+ }
if(validErr != nullptr) {
return;
}
commit 8c58bfb1460ddc0cb9db81dad8c1f344647470b7
Author: Daniel Baston <dbaston at gmail.com>
Date: Fri Dec 6 15:27:04 2019 -0500
Use spatial index to check shell nesting in IsValidOp
This commit introduces a IndexedNestedShellTester class, modelled after
IndexedNestedRingTester, to check shell nesting as part of MultiPolygon
validity checking. Indexes are used both to locate rings relative to
each other (STRtree) and to check point containment within a ring
(IndexedPointOnAreaLocator).
This commit reduces GEOSisValid runtime for a test geometry (GADM
boundary of Australia) from about 85 seconds to less than 1 second. It
does not appreciably change runtime for TIGER counties, HydroBASINS
Level 5, or a sample parcel dataset.
diff --git a/include/geos/algorithm/locate/IndexedPointInAreaLocator.h b/include/geos/algorithm/locate/IndexedPointInAreaLocator.h
index 10541a1..89e2b69 100644
--- a/include/geos/algorithm/locate/IndexedPointInAreaLocator.h
+++ b/include/geos/algorithm/locate/IndexedPointInAreaLocator.h
@@ -104,6 +104,10 @@ public:
*/
IndexedPointInAreaLocator(const geom::Geometry& g);
+ const geom::Geometry& getGeometry() const {
+ return areaGeom;
+ }
+
/** \brief
* Determines the [Location](@ref geom::Location) of a point in an areal
* [Geometry](@ref geom::Geometry).
diff --git a/include/geos/geomgraph/GeometryGraph.h b/include/geos/geomgraph/GeometryGraph.h
index 00bd4df..1136bbb 100644
--- a/include/geos/geomgraph/GeometryGraph.h
+++ b/include/geos/geomgraph/GeometryGraph.h
@@ -187,7 +187,7 @@ public:
/// Returned object is owned by this GeometryGraph
geom::CoordinateSequence* getBoundaryPoints();
- Edge* findEdge(const geom::LineString* line);
+ Edge* findEdge(const geom::LineString* line) const;
void computeSplitEdges(std::vector<Edge*>* edgelist);
diff --git a/include/geos/operation/valid/IndexedNestedShellTester.h b/include/geos/operation/valid/IndexedNestedShellTester.h
new file mode 100644
index 0000000..5626ef3
--- /dev/null
+++ b/include/geos/operation/valid/IndexedNestedShellTester.h
@@ -0,0 +1,108 @@
+/**********************************************************************
+ *
+ * GEOS - Geometry Engine Open Source
+ * http://geos.osgeo.org
+ *
+ * Copyright (C) 2001-2002 Vivid Solutions Inc.
+ * Copyright (C) 2005 Refractions Research Inc.
+ * Copyright (C) 2010 Safe Software Inc.
+ * Copyright (C) 2010 Sandro Santilli <strk at kbt.io>
+ * Copyright (C) 2019 Daniel Baston <dbaston at gmail.com>
+ *
+ * This is free software; you can redistribute and/or modify it under
+ * the terms of the GNU Lesser General Public Licence as published
+ * by the Free Software Foundation.
+ * See the COPYING file for more information.
+ *
+ **********************************************************************/
+
+#ifndef GEOS_OP_VALID_INDEXEDNESTEDSHELLTESTER_H
+#define GEOS_OP_VALID_INDEXEDNESTEDSHELLTESTER_H
+
+#include <geos/geom/Polygon.h>
+#include <geos/index/SpatialIndex.h>
+
+#include <cstddef>
+#include <memory>
+
+// Forward declarations
+namespace geos {
+namespace algorithm {
+namespace locate {
+ class IndexedPointInAreaLocator;
+}
+}
+namespace geom {
+ class Polygon;
+}
+namespace geomgraph {
+ class GeometryGraph;
+}
+namespace operation {
+namespace valid {
+ class PolygonIndexedLocators;
+}
+}
+}
+
+namespace geos {
+namespace operation {
+namespace valid {
+
+class IndexedNestedShellTester {
+
+public:
+ IndexedNestedShellTester(const geomgraph::GeometryGraph& g, size_t initialCapacity);
+
+ void add(const geom::Polygon& p) {
+ polys.push_back(&p);
+ }
+
+ const geom::Coordinate* getNestedPoint();
+
+ bool isNonNested();
+
+private:
+ void compute();
+
+ /**
+ * Check if a shell is incorrectly nested within a polygon.
+ * This is the case if the shell is inside the polygon shell,
+ * but not inside a polygon hole.
+ * (If the shell is inside a polygon hole, the nesting is valid.)
+ *
+ * The algorithm used relies on the fact that the rings must be
+ * properly contained.
+ * E.g. they cannot partially overlap (this has been previously
+ * checked by <code>checkRelateConsistency</code>
+ */
+ void checkShellNotNested(const geom::LinearRing* shell, PolygonIndexedLocators & locs);
+
+ /**
+ * This routine checks to see if a shell is properly contained
+ * in a hole.
+ * It assumes that the edges of the shell and hole do not
+ * properly intersect.
+ *
+ * @return <code>null</code> if the shell is properly contained, or
+ * a Coordinate which is not inside the hole if it is not
+ */
+ const geom::Coordinate* checkShellInsideHole(const geom::LinearRing* shell,
+ algorithm::locate::IndexedPointInAreaLocator & holeLoc);
+
+ /// Externally owned
+ const geomgraph::GeometryGraph& graph;
+
+ std::vector<const geom::Polygon*> polys;
+
+ // Externally owned, if not null
+ const geom::Coordinate* nestedPt;
+
+ bool processed;
+};
+
+}
+}
+}
+
+#endif //GEOS_INDEXEDNESTEDSHELLTESTER_H
diff --git a/include/geos/operation/valid/IsValidOp.h b/include/geos/operation/valid/IsValidOp.h
index 0756870..8795290 100644
--- a/include/geos/operation/valid/IsValidOp.h
+++ b/include/geos/operation/valid/IsValidOp.h
@@ -149,35 +149,6 @@ private:
void checkShellsNotNested(const geom::MultiPolygon* mp,
geomgraph::GeometryGraph* graph);
- /**
- * Check if a shell is incorrectly nested within a polygon.
- * This is the case if the shell is inside the polygon shell,
- * but not inside a polygon hole.
- * (If the shell is inside a polygon hole, the nesting is valid.)
- *
- * The algorithm used relies on the fact that the rings must be
- * properly contained.
- * E.g. they cannot partially overlap (this has been previously
- * checked by <code>checkRelateConsistency</code>
- */
- void checkShellNotNested(const geom::LinearRing* shell,
- const geom::Polygon* p,
- geomgraph::GeometryGraph* graph);
-
- /**
- * This routine checks to see if a shell is properly contained
- * in a hole.
- * It assumes that the edges of the shell and hole do not
- * properly intersect.
- *
- * @return <code>null</code> if the shell is properly contained, or
- * a Coordinate which is not inside the hole if it is not
- *
- */
- const geom::Coordinate* checkShellInsideHole(
- const geom::LinearRing* shell,
- const geom::LinearRing* hole,
- geomgraph::GeometryGraph* graph);
void checkConnectedInteriors(geomgraph::GeometryGraph& graph);
@@ -201,7 +172,7 @@ public:
static const geom::Coordinate* findPtNotNode(
const geom::CoordinateSequence* testCoords,
const geom::LinearRing* searchRing,
- geomgraph::GeometryGraph* graph);
+ const geomgraph::GeometryGraph* graph);
/** \brief
* Checks whether a coordinate is valid for processing.
diff --git a/include/geos/operation/valid/Makefile.am b/include/geos/operation/valid/Makefile.am
index f608323..aed363d 100644
--- a/include/geos/operation/valid/Makefile.am
+++ b/include/geos/operation/valid/Makefile.am
@@ -11,6 +11,7 @@ geos_HEADERS = \
ConnectedInteriorTester.h \
ConsistentAreaTester.h \
IsValidOp.h \
+ IndexedNestedShellTester.h \
MakeValid.h \
QuadtreeNestedRingTester.h \
RepeatedPointRemover.h \
diff --git a/src/geomgraph/GeometryGraph.cpp b/src/geomgraph/GeometryGraph.cpp
index 7354953..9aaaae5 100644
--- a/src/geomgraph/GeometryGraph.cpp
+++ b/src/geomgraph/GeometryGraph.cpp
@@ -147,7 +147,7 @@ GeometryGraph::getBoundaryPoints()
}
Edge*
-GeometryGraph::findEdge(const LineString* line)
+GeometryGraph::findEdge(const LineString* line) const
{
return lineEdgeMap.find(line)->second;
}
diff --git a/src/operation/valid/IndexedNestedRingTester.h b/src/operation/valid/IndexedNestedRingTester.h
index f7b8c33..47c292f 100644
--- a/src/operation/valid/IndexedNestedRingTester.h
+++ b/src/operation/valid/IndexedNestedRingTester.h
@@ -19,6 +19,7 @@
#ifndef GEOS_OP_VALID_OFFSETCURVEVERTEXLIST_H
#define GEOS_OP_VALID_OFFSETCURVEVERTEXLIST_H
+#include <cstddef>
#include <vector> // for composition
// Forward declarations
diff --git a/src/operation/valid/IndexedNestedShellTester.cpp b/src/operation/valid/IndexedNestedShellTester.cpp
new file mode 100644
index 0000000..87dabe8
--- /dev/null
+++ b/src/operation/valid/IndexedNestedShellTester.cpp
@@ -0,0 +1,214 @@
+/**********************************************************************
+ *
+ * GEOS - Geometry Engine Open Source
+ * http://geos.osgeo.org
+ *
+ * Copyright (C) 2019 Daniel Baston <dbaston at gmail.com>
+ *
+ * This is free software; you can redistribute and/or modify it under
+ * the terms of the GNU Lesser General Public Licence as published
+ * by the Free Software Foundation.
+ * See the COPYING file for more information.
+ *
+ **********************************************************************/
+
+#include <geos/algorithm/PointLocation.h>
+#include <geos/algorithm/locate/IndexedPointInAreaLocator.h>
+#include <geos/geom/Polygon.h>
+#include <geos/index/strtree/STRtree.h>
+#include <geos/operation/valid/IndexedNestedShellTester.h>
+#include <geos/operation/valid/IsValidOp.h>
+
+#include <deque>
+
+namespace geos {
+namespace operation {
+namespace valid {
+
+class PolygonIndexedLocators {
+
+public:
+ using Locator = algorithm::locate::IndexedPointInAreaLocator;
+
+ PolygonIndexedLocators(const geom::Polygon & p) :
+ poly(p),
+ shellLoc(*poly.getExteriorRing())
+ {
+ auto n = poly.getNumInteriorRing();
+ for (size_t i = 0; i < n; i++) {
+ ringLoc.emplace_back(*poly.getInteriorRingN(i));
+ }
+ }
+
+ Locator& getShellLocator() {
+ return shellLoc;
+ }
+
+ Locator& getHoleLocator(size_t holeNum) {
+ return ringLoc[holeNum];
+ }
+
+ const geom::Polygon* getPolygon() const {
+ return &poly;
+ }
+
+ const geom::LinearRing* getInteriorRingN(size_t n) const {
+ return poly.getInteriorRingN(n);
+ }
+
+private:
+ const geom::Polygon& poly;
+ Locator shellLoc;
+ std::deque<Locator> ringLoc;
+};
+
+IndexedNestedShellTester::IndexedNestedShellTester(const geos::geomgraph::GeometryGraph &g, size_t initialCapacity) :
+ graph(g),
+ nestedPt(nullptr),
+ processed(false)
+{
+ polys.reserve(initialCapacity);
+}
+
+bool
+IndexedNestedShellTester::isNonNested() {
+ return getNestedPoint() == nullptr;
+}
+
+const geom::Coordinate*
+IndexedNestedShellTester::getNestedPoint() {
+ compute();
+
+ return nestedPt;
+}
+
+void
+IndexedNestedShellTester::compute() {
+ if (processed) {
+ return;
+ }
+
+ processed = true;
+
+ index::strtree::STRtree tree;
+ for (const auto& p : polys) {
+ tree.insert(p->getEnvelopeInternal(), (void*) p->getExteriorRing());
+ }
+
+ std::vector<void*> hits;
+ for (const auto& outerPoly : polys) {
+ hits.clear();
+
+ PolygonIndexedLocators locs(*outerPoly);
+ const geom::LinearRing* outerShell = outerPoly->getExteriorRing();
+
+ tree.query(outerShell->getEnvelopeInternal(), hits);
+
+ for (const auto& hit : hits) {
+ const geom::LinearRing* potentialInnerShell = static_cast<const geom::LinearRing*>(hit);
+
+ if (potentialInnerShell == outerShell) {
+ continue;
+ }
+
+ // check if p1 can possibly by inside p2
+ if (!outerShell->getEnvelopeInternal()->covers(potentialInnerShell->getEnvelopeInternal())) {
+ continue;
+ }
+
+ checkShellNotNested(potentialInnerShell, locs);
+
+ if (nestedPt != nullptr) {
+ return;
+ }
+ }
+
+ }
+}
+
+/*private*/
+void
+IndexedNestedShellTester::checkShellNotNested(const geom::LinearRing* shell, PolygonIndexedLocators & locs)
+{
+ const geom::CoordinateSequence* shellPts = shell->getCoordinatesRO();
+
+ // test if shell is inside polygon shell
+ const geom::LinearRing* polyShell = locs.getPolygon()->getExteriorRing();
+ const geom::Coordinate* shellPt = IsValidOp::findPtNotNode(shellPts, polyShell, &graph);
+
+ // if no point could be found, we can assume that the shell
+ // is outside the polygon
+ if(shellPt == nullptr) {
+ return;
+ }
+
+ bool insidePolyShell = locs.getShellLocator().locate(shellPt) != geom::Location::EXTERIOR;
+ if(!insidePolyShell) {
+ return;
+ }
+
+ auto nholes = locs.getPolygon()->getNumInteriorRing();
+ if (nholes == 0) {
+ nestedPt = shellPt;
+ return;
+ }
+
+ // Check if the shell is inside one of the holes.
+ // This is the case if one of the calls to checkShellInsideHole
+ // returns a null coordinate.
+ // Otherwise, the shell is not properly contained in a hole, which is
+ // an error.
+ const geom::Coordinate* badNestedPt = nullptr;
+ for (size_t i = 0; i < nholes; i++) {
+ const geom::LinearRing* hole = locs.getPolygon()->getInteriorRingN(i);
+
+ if (hole->getEnvelopeInternal()->covers(shell->getEnvelopeInternal())) {
+ badNestedPt = checkShellInsideHole(shell, locs.getHoleLocator(i));
+ if(badNestedPt == nullptr) {
+ return;
+ }
+
+ }
+ }
+
+ nestedPt = badNestedPt;
+}
+
+
+const geom::Coordinate*
+IndexedNestedShellTester::checkShellInsideHole(const geom::LinearRing* shell,
+ algorithm::locate::IndexedPointInAreaLocator & holeLoc) {
+
+ const geom::CoordinateSequence* shellPts = shell->getCoordinatesRO();
+ const geom::LinearRing* hole = static_cast<const geom::LinearRing*>(&holeLoc.getGeometry());
+ const geom::CoordinateSequence* holePts = hole->getCoordinatesRO();
+
+ const geom::Coordinate* shellPtNotOnHole = IsValidOp::findPtNotNode(shellPts, hole, &graph);
+
+ if (shellPtNotOnHole) {
+ // Found a point not on the hole boundary. Is it outside the hole?
+ if (holeLoc.locate(shellPtNotOnHole) == geom::Location::EXTERIOR) {
+ return shellPtNotOnHole;
+ }
+ }
+
+ const geom::Coordinate* holePt = IsValidOp::findPtNotNode(holePts, shell, &graph);
+ // if point is on hole but not on shell, check that the hole is outside the shell
+
+ if (holePt != nullptr) {
+ if (algorithm::PointLocation::isInRing(*holePt, shellPts)) {
+ return holePt;
+ }
+
+ return nullptr;
+ }
+
+ // should never reach here: points in hole and shell appear to be equal
+ // TODO throw?
+ return nullptr;
+}
+
+}
+}
+}
+
diff --git a/src/operation/valid/IsValidOp.cpp b/src/operation/valid/IsValidOp.cpp
index dcbaa11..d38deba 100644
--- a/src/operation/valid/IsValidOp.cpp
+++ b/src/operation/valid/IsValidOp.cpp
@@ -41,6 +41,7 @@
#include <geos/operation/valid/ConnectedInteriorTester.h>
#include <geos/operation/valid/ConsistentAreaTester.h>
#include <geos/operation/valid/IsValidOp.h>
+#include <geos/operation/valid/IndexedNestedShellTester.h>
#include <geos/util/UnsupportedOperationException.h>
@@ -66,7 +67,7 @@ namespace valid { // geos.operation.valid
*/
const Coordinate*
IsValidOp::findPtNotNode(const CoordinateSequence* testCoords,
- const LinearRing* searchRing, GeometryGraph* graph)
+ const LinearRing* searchRing, const GeometryGraph* graph)
{
// find edge corresponding to searchRing.
Edge* searchEdge = graph->findEdge(searchRing);
@@ -488,121 +489,21 @@ IsValidOp::checkHolesNotNested(const Polygon* p, GeometryGraph* graph)
void
IsValidOp::checkShellsNotNested(const MultiPolygon* mp, GeometryGraph* graph)
{
- for (size_t i = 0, ngeoms = mp->getNumGeometries(); i < ngeoms; ++i) {
- const Polygon* p = mp->getGeometryN(i);
+ auto ngeoms = mp->getNumGeometries();
- const LinearRing* shell = p->getExteriorRing();
+ IndexedNestedShellTester tester(*graph, ngeoms);
- if (shell->isEmpty()) return;
-
- for (size_t j = 0; j < ngeoms; ++j) {
- if (i == j) {
- continue;
- }
-
- const Polygon* p2 = mp->getGeometryN(j);
-
- if (p2->isEmpty()) {
- continue;
- }
-
- checkShellNotNested(shell, p2, graph);
-
- if (validErr != nullptr) {
- return;
- }
- }
+ for (size_t i = 0; i < ngeoms; ++i) {
+ tester.add(*mp->getGeometryN(i));
}
-}
-/*private*/
-void
-IsValidOp::checkShellNotNested(const LinearRing* shell, const Polygon* p,
- GeometryGraph* graph)
-{
- const CoordinateSequence* shellPts = shell->getCoordinatesRO();
-
- // test if shell is inside polygon shell
- const LinearRing* polyShell = p->getExteriorRing();
- const CoordinateSequence* polyPts = polyShell->getCoordinatesRO();
- const Coordinate* shellPt = findPtNotNode(shellPts, polyShell, graph);
-
- // if no point could be found, we can assume that the shell
- // is outside the polygon
- if(shellPt == nullptr) {
- return;
+ if (!tester.isNonNested()) {
+ validErr = new TopologyValidationError(TopologyValidationError::eNestedShells,
+ *tester.getNestedPoint());
}
- bool insidePolyShell = PointLocation::isInRing(*shellPt, polyPts);
- if(!insidePolyShell) {
- return;
- }
-
- // if no holes, this is an error!
- auto nholes = p->getNumInteriorRing();
- if(nholes <= 0) {
- validErr = new TopologyValidationError(
- TopologyValidationError::eNestedShells,
- *shellPt);
- return;
- }
-
- /*
- * Check if the shell is inside one of the holes.
- * This is the case if one of the calls to checkShellInsideHole
- * returns a null coordinate.
- * Otherwise, the shell is not properly contained in a hole, which is
- * an error.
- */
- const Coordinate* badNestedPt = nullptr;
- for(size_t i = 0; i < nholes; ++i) {
- const LinearRing* hole = p->getInteriorRingN(i);
- badNestedPt = checkShellInsideHole(shell, hole, graph);
- if(badNestedPt == nullptr) {
- return;
- }
- }
- validErr = new TopologyValidationError(
- TopologyValidationError::eNestedShells, *badNestedPt
- );
}
-/*private*/
-const Coordinate*
-IsValidOp::checkShellInsideHole(const LinearRing* shell,
- const LinearRing* hole,
- GeometryGraph* graph)
-{
- const CoordinateSequence* shellPts = shell->getCoordinatesRO();
- const CoordinateSequence* holePts = hole->getCoordinatesRO();
-
- // TODO: improve performance of this - by sorting pointlists
- // for instance?
- const Coordinate* shellPt = findPtNotNode(shellPts, hole, graph);
-
- // if point is on shell but not hole, check that the shell is
- // inside the hole
- if(shellPt) {
- bool insideHole = PointLocation::isInRing(*shellPt, holePts);
- if(!insideHole) {
- return shellPt;
- }
- }
-
- const Coordinate* holePt = findPtNotNode(holePts, shell, graph);
-
- // if point is on hole but not shell, check that the hole is
- // outside the shell
- if(holePt) {
- bool insideShell = PointLocation::isInRing(*holePt, shellPts);
- if(insideShell) {
- return holePt;
- }
- return nullptr;
- }
- assert(0); // points in shell and hole appear to be equal
- return nullptr;
-}
/*private*/
void
diff --git a/src/operation/valid/Makefile.am b/src/operation/valid/Makefile.am
index 7d66258..e5523fe 100644
--- a/src/operation/valid/Makefile.am
+++ b/src/operation/valid/Makefile.am
@@ -23,6 +23,7 @@ libopvalid_la_SOURCES = \
TopologyValidationError.cpp \
IndexedNestedRingTester.cpp \
IndexedNestedRingTester.h \
+ IndexedNestedShellTester.cpp \
MakeValid.cpp
libopvalid_la_LIBADD =
commit 8b0f0cdbcec74441d008dc144a771e102718be20
Author: Daniel Baston <dbaston at gmail.com>
Date: Fri Dec 6 11:29:20 2019 -0500
Lazily build index in IndexedPointInAreaLocator
diff --git a/src/algorithm/locate/IndexedPointInAreaLocator.cpp b/src/algorithm/locate/IndexedPointInAreaLocator.cpp
index bbe6890..c9e6c08 100644
--- a/src/algorithm/locate/IndexedPointInAreaLocator.cpp
+++ b/src/algorithm/locate/IndexedPointInAreaLocator.cpp
@@ -105,13 +105,15 @@ IndexedPointInAreaLocator::IndexedPointInAreaLocator(const geom::Geometry& g)
&& areaGeomId != typeid(geom::LinearRing)) {
throw util::IllegalArgumentException("Argument must be Polygonal or LinearRing");
}
-
- buildIndex(areaGeom);
}
geom::Location
IndexedPointInAreaLocator::locate(const geom::Coordinate* /*const*/ p)
{
+ if (index == nullptr) {
+ buildIndex(areaGeom);
+ }
+
algorithm::RayCrossingCounter rcc(*p);
IndexedPointInAreaLocator::SegmentVisitor visitor(&rcc);
commit 1e7fc88cfbbd2c22f896aebe0b3db31a9d88c24b
Author: Daniel Baston <dbaston at gmail.com>
Date: Fri Dec 6 11:20:27 2019 -0500
Add generic perf test for unary operations
diff --git a/benchmarks/capi/CMakeLists.txt b/benchmarks/capi/CMakeLists.txt
index 5f37deb..886a808 100644
--- a/benchmarks/capi/CMakeLists.txt
+++ b/benchmarks/capi/CMakeLists.txt
@@ -22,3 +22,6 @@ target_link_libraries(perf_geospreparedcontains PRIVATE geos geos_c)
add_executable(perf_intersection IntersectionPerfTest.cpp)
target_link_libraries(perf_intersection PRIVATE geos geos_c)
+
+add_executable(perf_unary UnaryOpPerfTest.cpp)
+target_link_libraries(perf_unary PRIVATE geos geos_c)
diff --git a/benchmarks/capi/UnaryOpPerfTest.cpp b/benchmarks/capi/UnaryOpPerfTest.cpp
new file mode 100644
index 0000000..e5f733c
--- /dev/null
+++ b/benchmarks/capi/UnaryOpPerfTest.cpp
@@ -0,0 +1,80 @@
+/**********************************************************************
+ *
+ * GEOS - Geometry Engine Open Source
+ * http://geos.osgeo.org
+ *
+ * Copyright (C) 2019 Daniel Baston <dbaston at gmail.com>
+ *
+ * This is free software; you can redistribute and/or modify it under
+ * the terms of the GNU Lesser General Public Licence as published
+ * by the Free Software Foundation.
+ * See the COPYING file for more information.
+ *
+ **********************************************************************/
+
+#include <geos/profiler.h>
+#include <geos_c.h>
+
+#include <fstream>
+#include <iostream>
+#include <sstream>
+
+int main(int argc, char** argv) {
+ if (argc != 3 && argc != 4) {
+ std::cout << "perf_unary reads geometries from a WKT file and" << std::endl;
+ std::cout << "performs a unary operation on each. The number of" << std::endl;
+ std::cout << "geometries processed can be limited by specifying n." << std::endl;
+ std::cout << std::endl;
+ std::cout << "The following operations are supported:" << std::endl;
+ std::cout << "- valid" << std::endl;
+ std::cout << std::endl;
+ std::cout << "Usage: perf_unary [wktfile] [operation] [n]" << std::endl;
+ return 0;
+ }
+
+ initGEOS(nullptr, nullptr);
+
+ std::string fname{argv[1]};
+ std::string op{argv[2]};
+
+ long max_geoms;
+ if (argc == 4) {
+ max_geoms = std::atol(argv[3]);
+ std::cout << "Reading up to " << max_geoms << " geometries from " << fname << std::endl;
+ } else {
+ std::cout << "Reading geometries from " << fname << std::endl;
+ max_geoms = -1;
+ }
+
+ std::vector<GEOSGeometry*> geoms;
+
+ std::ifstream f(fname);
+ std::string line;
+ long i = 0;
+ while(std::getline(f, line) && (max_geoms < 0 || i < max_geoms)) {
+ auto g = GEOSGeomFromWKT(line.c_str());
+ if (g != nullptr) {
+ geoms.push_back(g);
+ i++;
+ }
+ }
+ f.close();
+
+ std::cout << "Read " << geoms.size() << " geometries." << std::endl;
+
+ geos::util::Profile sw(op);
+ sw.start();
+
+ if (op == "valid") {
+ for (const auto& g : geoms) {
+ GEOSisValid(g);
+ }
+ }
+
+ sw.stop();
+ std::cout << sw.getTotFormatted() << std::endl;
+
+ for (auto& g : geoms) {
+ GEOSGeom_destroy(g);
+ }
+}
commit 09bdf26f086796273556bb5149452bf53de50f50
Author: Daniel Baston <dbaston at gmail.com>
Date: Sat Oct 5 22:08:14 2019 -0400
Improve performance of PlanarGraph::findEdgeInSameDirection
Use the nodeMap instead of performing a complete scan of all edges.
diff --git a/src/geomgraph/PlanarGraph.cpp b/src/geomgraph/PlanarGraph.cpp
index b4ebaa2..3ad84c7 100644
--- a/src/geomgraph/PlanarGraph.cpp
+++ b/src/geomgraph/PlanarGraph.cpp
@@ -319,11 +319,16 @@ Edge*
PlanarGraph::findEdgeInSameDirection(const Coordinate& p0,
const Coordinate& p1)
{
- for(size_t i = 0, n = edges->size(); i < n; i++) {
- Edge* e = (*edges)[i];
- assert(e);
+ Node* node = getNodeMap()->find(p0);
+ if (node == nullptr) {
+ return nullptr;
+ }
+
+ for (const auto& ee : *(node->getEdges())) {
+ Edge* e = ee->getEdge();
const CoordinateSequence* eCoord = e->getCoordinates();
+
assert(eCoord);
size_t nCoords = eCoord->size();
commit adb7ca818bfd93e64acd7c2efda9ac9b0c982d02
Author: Daniel Baston <dbaston at gmail.com>
Date: Sat Oct 5 22:09:53 2019 -0400
Use unordered_map for GeometryGraph::lineEdgeMap
Improves lookup performance.
diff --git a/include/geos/geomgraph/GeometryGraph.h b/include/geos/geomgraph/GeometryGraph.h
index 00bd4df..dd3f45a 100644
--- a/include/geos/geomgraph/GeometryGraph.h
+++ b/include/geos/geomgraph/GeometryGraph.h
@@ -24,6 +24,7 @@
#include <geos/export.h>
#include <map>
+#include <unordered_map>
#include <vector>
#include <memory>
@@ -86,8 +87,7 @@ private:
* Following the above description there's no need to
* compare LineStrings other then by pointer value.
*/
-//std::map<const geom::LineString*,Edge*,geom::LineStringLT> lineEdgeMap;
- std::map<const geom::LineString*, Edge*> lineEdgeMap;
+ std::unordered_map<const geom::LineString*, Edge*> lineEdgeMap;
/**
* If this flag is true, the Boundary Determination Rule will
commit 767cb5fe0fd11e20a4b91c875c14160beb0f3ee0
Author: Daniel Baston <dbaston at gmail.com>
Date: Thu Oct 3 15:23:14 2019 -0400
Avoid indexing polygon shell if we have no holes to test
diff --git a/src/operation/valid/IsValidOp.cpp b/src/operation/valid/IsValidOp.cpp
index dcbaa11..2416333 100644
--- a/src/operation/valid/IsValidOp.cpp
+++ b/src/operation/valid/IsValidOp.cpp
@@ -426,12 +426,16 @@ IsValidOp::checkNoSelfIntersectingRing(EdgeIntersectionList& eiList)
void
IsValidOp::checkHolesInShell(const Polygon* p, GeometryGraph* graph)
{
+ auto nholes = p->getNumInteriorRing();
+ if (nholes == 0) {
+ return;
+ }
+
const LinearRing* shell = p->getExteriorRing();
bool isShellEmpty = shell->isEmpty();
locate::IndexedPointInAreaLocator ipial(*shell);
- auto nholes = p->getNumInteriorRing();
for(size_t i = 0; i < nholes; ++i) {
const LinearRing* hole = p->getInteriorRingN(i);
-----------------------------------------------------------------------
Summary of changes:
.travis.yml | 22 +--
benchmarks/capi/CMakeLists.txt | 3 +
...ntersectionPerfTest.cpp => UnaryOpPerfTest.cpp} | 51 +++--
.../algorithm/locate/IndexedPointInAreaLocator.h | 4 +
include/geos/geomgraph/EdgeRing.h | 2 +-
include/geos/geomgraph/GeometryGraph.h | 6 +-
.../operation/valid/IndexedNestedShellTester.h | 108 +++++++++++
include/geos/operation/valid/IsValidOp.h | 31 +--
include/geos/operation/valid/Makefile.am | 1 +
src/algorithm/locate/IndexedPointInAreaLocator.cpp | 6 +-
src/geomgraph/EdgeRing.cpp | 31 ++-
src/geomgraph/GeometryGraph.cpp | 2 +-
src/geomgraph/PlanarGraph.cpp | 11 +-
src/operation/valid/IndexedNestedRingTester.cpp | 41 ++--
src/operation/valid/IndexedNestedRingTester.h | 8 +-
src/operation/valid/IndexedNestedShellTester.cpp | 214 +++++++++++++++++++++
src/operation/valid/IsValidOp.cpp | 131 ++-----------
src/operation/valid/Makefile.am | 1 +
18 files changed, 436 insertions(+), 237 deletions(-)
copy benchmarks/capi/{IntersectionPerfTest.cpp => UnaryOpPerfTest.cpp} (52%)
create mode 100644 include/geos/operation/valid/IndexedNestedShellTester.h
create mode 100644 src/operation/valid/IndexedNestedShellTester.cpp
hooks/post-receive
--
GEOS
More information about the geos-commits
mailing list