[geos-commits] [SCM] GEOS branch master updated. 17b84d02d61d2958c9c1df33d6954cc956406380

git at osgeo.org git at osgeo.org
Mon Jun 3 05:31:53 PDT 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  17b84d02d61d2958c9c1df33d6954cc956406380 (commit)
       via  f055e97679f160a83077cc17ee18de8ec111c59a (commit)
      from  47c50306157577e9a923ab532a9361eb65bd2074 (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 17b84d02d61d2958c9c1df33d6954cc956406380
Merge: 47c5030 f055e97
Author: Daniel Baston <dbaston at gmail.com>
Date:   Mon Jun 3 08:31:03 2019 -0400

    Merge remote-tracking branch 'origin/distanceop-refactor'


commit f055e97679f160a83077cc17ee18de8ec111c59a
Author: Daniel Baston <dbaston at gmail.com>
Date:   Wed May 29 12:22:08 2019 -0400

    Resolve memory leaks in DistanceOp
    
    This commit takes a fairly invasive approach to resolving the leaks
    and removes all manual memory management from the DistanceOp class.

diff --git a/capi/geos_ts_c.cpp b/capi/geos_ts_c.cpp
index 896ce1d..ae36efe 100644
--- a/capi/geos_ts_c.cpp
+++ b/capi/geos_ts_c.cpp
@@ -1293,7 +1293,7 @@ extern "C" {
             if(g1->isEmpty() || g2->isEmpty()) {
                 return 0;
             }
-            return geos::operation::distance::DistanceOp::nearestPoints(g1, g2);
+            return geos::operation::distance::DistanceOp::nearestPoints(g1, g2).release();
         }
         catch(const std::exception& e) {
             handle->ERROR_MESSAGE("%s", e.what());
diff --git a/include/geos/geom/LineSegment.h b/include/geos/geom/LineSegment.h
index 3ef5d9c..2430c12 100644
--- a/include/geos/geom/LineSegment.h
+++ b/include/geos/geom/LineSegment.h
@@ -26,6 +26,7 @@
 
 #include <geos/inline.h>
 
+#include <array>
 #include <iostream> // for ostream
 #include <functional> // for std::hash
 #include <memory> // for unique_ptr
@@ -319,11 +320,10 @@ public:
      * @param line the line segment to find the closest points to
      * @return a pair of Coordinates which are the closest points on
      * the line segments.
-     * The returned CoordinateSequence must be deleted by caller
      */
-    CoordinateSequence* closestPoints(const LineSegment& line);
+    std::array<Coordinate, 2> closestPoints(const LineSegment& line);
 
-    CoordinateSequence* closestPoints(const LineSegment* line);
+    std::array<Coordinate, 2> closestPoints(const LineSegment* line);
 
     /**
      * Computes an intersection point between two segments,
diff --git a/include/geos/geom/LineSegment.inl b/include/geos/geom/LineSegment.inl
index 76724ee..85c2497 100644
--- a/include/geos/geom/LineSegment.inl
+++ b/include/geos/geom/LineSegment.inl
@@ -165,7 +165,7 @@ LineSegment::orientationIndex(const Coordinate& p) const
     return algorithm::Orientation::index(p0, p1, p);
 }
 
-INLINE CoordinateSequence*
+INLINE std::array<Coordinate, 2>
 LineSegment::closestPoints(const LineSegment* line)
 {
     assert(line);
diff --git a/include/geos/operation/distance/ConnectedElementLocationFilter.h b/include/geos/operation/distance/ConnectedElementLocationFilter.h
index 683a0e2..4703cbd 100644
--- a/include/geos/operation/distance/ConnectedElementLocationFilter.h
+++ b/include/geos/operation/distance/ConnectedElementLocationFilter.h
@@ -22,7 +22,9 @@
 #include <geos/export.h>
 
 #include <geos/geom/GeometryFilter.h> // for inheritance
+#include <geos/operation/distance/GeometryLocation.h>
 
+#include <memory>
 #include <vector>
 
 // Forward declarations
@@ -30,11 +32,6 @@ namespace geos {
 namespace geom {
 class Geometry;
 }
-namespace operation {
-namespace distance {
-class GeometryLocation;
-}
-}
 }
 
 
@@ -52,7 +49,8 @@ namespace distance { // geos::operation::distance
 class GEOS_DLL ConnectedElementLocationFilter: public geom::GeometryFilter {
 private:
 
-    std::vector<GeometryLocation*>* locations;
+    std::vector<std::unique_ptr<GeometryLocation>> locations;
+    ConnectedElementLocationFilter() = default;
 
 public:
     /**
@@ -61,12 +59,7 @@ public:
      * not a GeometryCollection, an empty list will be returned. The elements of the list
      * are {@link com.vividsolutions.jts.operation.distance.GeometryLocation}s.
      */
-    static std::vector<GeometryLocation*>* getLocations(const geom::Geometry* geom);
-
-    ConnectedElementLocationFilter(std::vector<GeometryLocation*>* newLocations)
-        :
-        locations(newLocations)
-    {}
+    static std::vector<std::unique_ptr<GeometryLocation>> getLocations(const geom::Geometry* geom);
 
     void filter_ro(const geom::Geometry* geom) override;
     void filter_rw(geom::Geometry* geom) override;
diff --git a/include/geos/operation/distance/DistanceOp.h b/include/geos/operation/distance/DistanceOp.h
index e8f45cc..f38d0df 100644
--- a/include/geos/operation/distance/DistanceOp.h
+++ b/include/geos/operation/distance/DistanceOp.h
@@ -23,9 +23,12 @@
 #include <geos/export.h>
 
 #include <geos/algorithm/PointLocator.h> // for composition
+#include <geos/operation/distance/GeometryLocation.h>
+#include <geos/geom/CoordinateSequence.h>
 
 #include <array>
 #include <vector>
+#include <memory>
 
 #ifdef _MSC_VER
 #pragma warning(push)
@@ -40,12 +43,6 @@ class Polygon;
 class LineString;
 class Point;
 class Geometry;
-class CoordinateSequence;
-}
-namespace operation {
-namespace distance {
-class GeometryLocation;
-}
 }
 }
 
@@ -117,24 +114,7 @@ public:
      *         A NULL return means one of the geometries is empty.
      *
      */
-    static geom::CoordinateSequence* nearestPoints(
-        const geom::Geometry* g0,
-        const geom::Geometry* g1);
-
-    /**
-     * Compute the the closest points of two geometries.
-     *
-     * The points are presented in the same order as the input Geometries.
-     *
-     * @param g0 a {@link Geometry}
-     * @param g1 another {@link Geometry}
-     *
-     * @return the closest points in the geometries, ownership to caller.
-     *         A NULL return means one of the geometries is empty.
-     *
-     * @deprecated renamed to nearestPoints
-     */
-    static geom::CoordinateSequence* closestPoints(
+    static std::unique_ptr<geom::CoordinateSequence> nearestPoints(
         const geom::Geometry* g0,
         const geom::Geometry* g1);
 
@@ -164,7 +144,7 @@ public:
     DistanceOp(const geom::Geometry& g0, const geom::Geometry& g1,
                double terminateDistance);
 
-    ~DistanceOp();
+    ~DistanceOp() = default;
 
     /**
      * Report the distance between the closest points on the input geometries.
@@ -174,17 +154,6 @@ public:
     double distance();
 
     /**
-     * Report the coordinates of the closest points in the input geometries.
-     * The points are presented in the same order as the input Geometries.
-     *
-     * @return a pair of {@link Coordinate}s of the closest points
-     *         as a newly allocated object (ownership to caller)
-     *
-     * @deprecated renamed to nearestPoints
-     */
-    geom::CoordinateSequence* closestPoints();
-
-    /**
      * Report the coordinates of the nearest points in the input geometries.
      * The points are presented in the same order as the input Geometries.
      *
@@ -192,52 +161,33 @@ public:
      *         as a newly allocated object (ownership to caller)
      *
      */
-    geom::CoordinateSequence* nearestPoints();
+    std::unique_ptr<geom::CoordinateSequence> nearestPoints();
 
 private:
 
-    /**
-     * Report the locations of the closest points in the input geometries.
-     * The locations are presented in the same order as the input
-     * Geometries.
-     *
-     * @return a pair of {@link GeometryLocation}s for the closest points.
-     *         Ownership of returned object is left to this instance and
-     *         it's reference will be alive for the whole lifetime of it.
-     *
-     * NOTE: this is public in JTS, but we aim at API reduction here...
-     *
-     */
-    std::vector<GeometryLocation*>* nearestLocations();
-
     // input
     std::array<geom::Geometry const*, 2> geom;
     double terminateDistance;
 
     // working
     algorithm::PointLocator ptLocator;
-    // TODO: use unique_ptr
-    std::vector<GeometryLocation*>* minDistanceLocation;
+    std::array<std::unique_ptr<GeometryLocation>, 2> minDistanceLocation;
     double minDistance;
+    bool computed = false;
 
-    // memory management
-    std::vector<geom::Coordinate*> newCoords;
-
-
-    void updateMinDistance(std::vector<GeometryLocation*>& locGeom,
-                           bool flip);
+    void updateMinDistance(std::array<std::unique_ptr<GeometryLocation>, 2> & locGeom, bool flip);
 
     void computeMinDistance();
 
     void computeContainmentDistance();
 
-    void computeInside(std::vector<GeometryLocation*>* locs,
+    void computeInside(std::vector<std::unique_ptr<GeometryLocation>> & locs,
                        const std::vector<const geom::Polygon*>& polys,
-                       std::vector<GeometryLocation*>* locPtPoly);
+                       std::array<std::unique_ptr<GeometryLocation>, 2> & locPtPoly);
 
-    void computeInside(GeometryLocation* ptLoc,
+    void computeInside(std::unique_ptr<GeometryLocation> & ptLoc,
                        const geom::Polygon* poly,
-                       std::vector<GeometryLocation*>* locPtPoly);
+                       std::array<std::unique_ptr<GeometryLocation>, 2> & locPtPoly);
 
     /**
      * Computes distance between facets (lines and points)
@@ -248,25 +198,25 @@ private:
     void computeMinDistanceLines(
         const std::vector<const geom::LineString*>& lines0,
         const std::vector<const geom::LineString*>& lines1,
-        std::vector<GeometryLocation*>& locGeom);
+        std::array<std::unique_ptr<GeometryLocation>, 2> & locGeom);
 
     void computeMinDistancePoints(
         const std::vector<const geom::Point*>& points0,
         const std::vector<const geom::Point*>& points1,
-        std::vector<GeometryLocation*>& locGeom);
+        std::array<std::unique_ptr<GeometryLocation>, 2> & locGeom);
 
     void computeMinDistanceLinesPoints(
         const std::vector<const geom::LineString*>& lines0,
         const std::vector<const geom::Point*>& points1,
-        std::vector<GeometryLocation*>& locGeom);
+        std::array<std::unique_ptr<GeometryLocation>, 2> & locGeom);
 
     void computeMinDistance(const geom::LineString* line0,
                             const geom::LineString* line1,
-                            std::vector<GeometryLocation*>& locGeom);
+                            std::array<std::unique_ptr<GeometryLocation>, 2> & locGeom);
 
     void computeMinDistance(const geom::LineString* line,
                             const geom::Point* pt,
-                            std::vector<GeometryLocation*>& locGeom);
+                            std::array<std::unique_ptr<GeometryLocation>, 2> & locGeom);
 };
 
 
diff --git a/src/geom/LineSegment.cpp b/src/geom/LineSegment.cpp
index 9c490e4..00072ce 100644
--- a/src/geom/LineSegment.cpp
+++ b/src/geom/LineSegment.cpp
@@ -193,22 +193,20 @@ LineSegment::orientationIndex(const LineSegment& seg) const
     return 0;
 }
 
-CoordinateSequence*
+std::array<Coordinate, 2>
 LineSegment::closestPoints(const LineSegment& line)
 {
     // test for intersection
     Coordinate intPt;
     if(intersection(line, intPt)) {
-        CoordinateSequence* cl = new CoordinateArraySequence(new vector<Coordinate>(2, intPt));
-        return cl;
+        return { intPt, intPt };
     }
 
     /*
      * if no intersection closest pair contains at least one endpoint.
      * Test each endpoint in turn.
      */
-    CoordinateSequence* closestPt = new CoordinateArraySequence(2);
-    //vector<Coordinate> *cv = new vector<Coordinate>(2);
+    std::array<Coordinate, 2> closestPt;
 
     double minDistance = DoubleMax;
     double dist;
@@ -217,18 +215,16 @@ LineSegment::closestPoints(const LineSegment& line)
     closestPoint(line.p0, close00);
     minDistance = close00.distance(line.p0);
 
-    closestPt->setAt(close00, 0);
-    closestPt->setAt(line.p0, 1);
+    closestPt[0] = close00;
+    closestPt[1] = line.p0;
 
     Coordinate close01;
     closestPoint(line.p1, close01);
     dist = close01.distance(line.p1);
     if(dist < minDistance) {
         minDistance = dist;
-        closestPt->setAt(close01, 0);
-        closestPt->setAt(line.p1, 1);
-        //(*cv)[0] = close01;
-        //(*cv)[1] = line.p1;
+        closestPt[0] = close01;
+        closestPt[1] = line.p1;
     }
 
     Coordinate close10;
@@ -236,10 +232,8 @@ LineSegment::closestPoints(const LineSegment& line)
     dist = close10.distance(p0);
     if(dist < minDistance) {
         minDistance = dist;
-        closestPt->setAt(p0, 0);
-        closestPt->setAt(close10, 1);
-        //(*cv)[0] = p0;
-        //(*cv)[1] = close10;
+        closestPt[0] = p0;
+        closestPt[1] = close10;
     }
 
     Coordinate close11;
@@ -247,10 +241,8 @@ LineSegment::closestPoints(const LineSegment& line)
     dist = close11.distance(p1);
     if(dist < minDistance) {
         minDistance = dist;
-        closestPt->setAt(p1, 0);
-        closestPt->setAt(close11, 1);
-        //(*cv)[0] = p1;
-        //(*cv)[1] = *close11;
+        closestPt[0] = p1;
+        closestPt[1] = close11;
     }
 
     return closestPt;
diff --git a/src/operation/distance/ConnectedElementLocationFilter.cpp b/src/operation/distance/ConnectedElementLocationFilter.cpp
index 08822b6..e12b892 100644
--- a/src/operation/distance/ConnectedElementLocationFilter.cpp
+++ b/src/operation/distance/ConnectedElementLocationFilter.cpp
@@ -35,13 +35,12 @@ namespace operation { // geos.operation
 namespace distance { // geos.operation.distance
 
 /*public*/
-vector<GeometryLocation*>*
+vector<unique_ptr<GeometryLocation>>
 ConnectedElementLocationFilter::getLocations(const Geometry* geom)
 {
-    vector<GeometryLocation*>* loc = new vector<GeometryLocation*>();
-    ConnectedElementLocationFilter c(loc);
+    ConnectedElementLocationFilter c;
     geom->apply_ro(&c);
-    return loc;
+    return std::move(c.locations);
 }
 
 void
@@ -51,7 +50,7 @@ ConnectedElementLocationFilter::filter_ro(const Geometry* geom)
             (typeid(*geom) == typeid(LineString)) ||
             (typeid(*geom) == typeid(LinearRing)) ||
             (typeid(*geom) == typeid(Polygon))) {
-        locations->push_back(new GeometryLocation(geom, 0, *(geom->getCoordinate())));
+        locations.emplace_back(new GeometryLocation(geom, 0, *(geom->getCoordinate())));
     }
 }
 
@@ -62,7 +61,7 @@ ConnectedElementLocationFilter::filter_rw(Geometry* geom)
             (typeid(*geom) == typeid(LineString)) ||
             (typeid(*geom) == typeid(LinearRing)) ||
             (typeid(*geom) == typeid(Polygon))) {
-        locations->push_back(new GeometryLocation(geom, 0, *(geom->getCoordinate())));
+        locations.emplace_back(new GeometryLocation(geom, 0, *(geom->getCoordinate())));
     }
 }
 
diff --git a/src/operation/distance/DistanceOp.cpp b/src/operation/distance/DistanceOp.cpp
index b3f38bd..0c038ef 100644
--- a/src/operation/distance/DistanceOp.cpp
+++ b/src/operation/distance/DistanceOp.cpp
@@ -71,16 +71,8 @@ DistanceOp::distance(const Geometry& g0, const Geometry& g1)
     return distOp.distance();
 }
 
-/*public static deprecated*/
-CoordinateSequence*
-DistanceOp::closestPoints(const Geometry* g0, const Geometry* g1)
-{
-    DistanceOp distOp(g0, g1);
-    return distOp.nearestPoints();
-}
-
 /*public static*/
-CoordinateSequence*
+std::unique_ptr<CoordinateSequence>
 DistanceOp::nearestPoints(const Geometry* g0, const Geometry* g1)
 {
     DistanceOp distOp(g0, g1);
@@ -90,14 +82,12 @@ DistanceOp::nearestPoints(const Geometry* g0, const Geometry* g1)
 DistanceOp::DistanceOp(const Geometry* g0, const Geometry* g1):
     geom{g0, g1},
     terminateDistance(0.0),
-    minDistanceLocation(nullptr),
     minDistance(DoubleMax)
 {}
 
 DistanceOp::DistanceOp(const Geometry& g0, const Geometry& g1):
     geom{&g0, &g1},
     terminateDistance(0.0),
-    minDistanceLocation(nullptr),
     minDistance(DoubleMax)
 {}
 
@@ -105,24 +95,9 @@ DistanceOp::DistanceOp(const Geometry& g0, const Geometry& g1, double tdist)
     :
     geom{&g0, &g1},
     terminateDistance(tdist),
-    minDistanceLocation(nullptr),
     minDistance(DoubleMax)
 {}
 
-DistanceOp::~DistanceOp()
-{
-    size_t i;
-    for(i = 0; i < newCoords.size(); i++) {
-        delete newCoords[i];
-    }
-    if(minDistanceLocation) {
-        for(i = 0; i < minDistanceLocation->size(); i++) {
-            delete(*minDistanceLocation)[i];
-        }
-        delete minDistanceLocation;
-    }
-}
-
 /**
 * Report the distance between the closest points on the input geometries.
 *
@@ -144,22 +119,13 @@ DistanceOp::distance()
 }
 
 /* public */
-CoordinateSequence*
-DistanceOp::closestPoints()
-{
-    return nearestPoints();
-}
-
-
-/* public */
-CoordinateSequence*
+std::unique_ptr<CoordinateSequence>
 DistanceOp::nearestPoints()
 {
     // lazily creates minDistanceLocation
     computeMinDistance();
 
-    assert(nullptr != minDistanceLocation);
-    std::vector<GeometryLocation*>& locs = *minDistanceLocation;
+    auto& locs = minDistanceLocation;
 
     // Empty input geometries result in this behaviour
     if(locs[0] == nullptr || locs[1] == nullptr) {
@@ -169,31 +135,16 @@ DistanceOp::nearestPoints()
         return nullptr;
     }
 
-    GeometryLocation* loc0 = locs[0];
-    GeometryLocation* loc1 = locs[1];
-    const Coordinate& c0 = loc0->getCoordinate();
-    const Coordinate& c1 = loc1->getCoordinate();
+    std::unique_ptr<std::vector<Coordinate>> nearestPts(new std::vector<Coordinate>(2));
+    (*nearestPts)[0] = locs[0]->getCoordinate();
+    (*nearestPts)[1] = locs[1]->getCoordinate();
 
-    CoordinateSequence* nearestPts = new CoordinateArraySequence();
-    nearestPts->add(c0);
-    nearestPts->add(c1);
-
-    return nearestPts;
-}
-
-/*private, unused!*/
-vector<GeometryLocation*>*
-DistanceOp::nearestLocations()
-{
-    computeMinDistance();
-    return minDistanceLocation;
+    return std::unique_ptr<CoordinateSequence>(new CoordinateArraySequence(nearestPts.release()));
 }
 
 void
-DistanceOp::updateMinDistance(vector<GeometryLocation*>& locGeom, bool flip)
+DistanceOp::updateMinDistance(array<unique_ptr<GeometryLocation>, 2> & locGeom, bool flip)
 {
-    assert(minDistanceLocation);
-
     // if not set then don't update
     if(locGeom[0] == nullptr) {
 #if GEOS_DEBUG
@@ -203,15 +154,13 @@ DistanceOp::updateMinDistance(vector<GeometryLocation*>& locGeom, bool flip)
         return;
     }
 
-    delete(*minDistanceLocation)[0];
-    delete(*minDistanceLocation)[1];
     if(flip) {
-        (*minDistanceLocation)[0] = locGeom[1];
-        (*minDistanceLocation)[1] = locGeom[0];
+        minDistanceLocation[0] = std::move(locGeom[1]);
+        minDistanceLocation[1] = std::move(locGeom[0]);
     }
     else {
-        (*minDistanceLocation)[0] = locGeom[0];
-        (*minDistanceLocation)[1] = locGeom[1];
+        minDistanceLocation[0] = std::move(locGeom[0]);
+        minDistanceLocation[1] = std::move(locGeom[1]);
     }
 }
 
@@ -220,7 +169,7 @@ void
 DistanceOp::computeMinDistance()
 {
     // only compute once!
-    if(minDistanceLocation) {
+    if(computed) {
         return;
     }
 
@@ -228,15 +177,15 @@ DistanceOp::computeMinDistance()
     std::cerr << "---Start: " << geom[0]->toString() << " - " << geom[1]->toString() << std::endl;
 #endif
 
-    minDistanceLocation = new vector<GeometryLocation*>(2);
-
     computeContainmentDistance();
 
     if(minDistance <= terminateDistance) {
+        computed = true;
         return;
     }
 
     computeFacetDistance();
+    computed = true;
 
 #if GEOS_DEBUG
     std::cerr << "---End " << std::endl;
@@ -261,32 +210,21 @@ DistanceOp::computeContainmentDistance()
     // Expected to fill minDistanceLocation items
     // if minDistance <= terminateDistance
 
-    vector<GeometryLocation*>* locPtPoly = new vector<GeometryLocation*>(2);
+    array<std::unique_ptr<GeometryLocation>, 2> locPtPoly;
     // test if either geometry has a vertex inside the other
     if(! polys1.empty()) {
-        vector<GeometryLocation*>* insideLocs0 =
-            ConnectedElementLocationFilter::getLocations(geom[0]);
+        auto insideLocs0 = ConnectedElementLocationFilter::getLocations(geom[0]);
         computeInside(insideLocs0, polys1, locPtPoly);
+
         if(minDistance <= terminateDistance) {
-            assert((*locPtPoly)[0]);
-            assert((*locPtPoly)[1]);
-            (*minDistanceLocation)[0] = (*locPtPoly)[0];
-            (*minDistanceLocation)[1] = (*locPtPoly)[1];
-            delete locPtPoly;
-            for(size_t i = 0; i < insideLocs0->size(); i++) {
-                GeometryLocation* l = (*insideLocs0)[i];
-                if(l != (*minDistanceLocation)[0] &&
-                        l != (*minDistanceLocation)[1]) {
-                    delete l;
-                }
-            }
-            delete insideLocs0;
+            assert(locPtPoly[0]);
+            assert(locPtPoly[1]);
+
+            minDistanceLocation[0] = std::move(locPtPoly[0]);
+            minDistanceLocation[1] = std::move(locPtPoly[1]);
+
             return;
         }
-        for(size_t i = 0; i < insideLocs0->size(); i++) {
-            delete(*insideLocs0)[i];
-        }
-        delete insideLocs0;
     }
 
     Polygon::ConstVect polys0;
@@ -298,50 +236,31 @@ DistanceOp::computeContainmentDistance()
 
 
     if(! polys0.empty()) {
-        vector<GeometryLocation*>* insideLocs1 = ConnectedElementLocationFilter::getLocations(geom[1]);
+        auto insideLocs1 = ConnectedElementLocationFilter::getLocations(geom[1]);
         computeInside(insideLocs1, polys0, locPtPoly);
         if(minDistance <= terminateDistance) {
-// flip locations, since we are testing geom 1 VS geom 0
-            assert((*locPtPoly)[0]);
-            assert((*locPtPoly)[1]);
-            (*minDistanceLocation)[0] = (*locPtPoly)[1];
-            (*minDistanceLocation)[1] = (*locPtPoly)[0];
-            delete locPtPoly;
-            for(size_t i = 0; i < insideLocs1->size(); i++) {
-                GeometryLocation* l = (*insideLocs1)[i];
-                if(l != (*minDistanceLocation)[0] &&
-                        l != (*minDistanceLocation)[1]) {
-                    delete l;
-                }
-            }
-            delete insideLocs1;
+            // flip locations, since we are testing geom 1 VS geom 0
+            assert(locPtPoly[0]);
+            assert(locPtPoly[1]);
+
+            minDistanceLocation[0] = std::move(locPtPoly[1]);
+            minDistanceLocation[1] = std::move(locPtPoly[0]);
+
             return;
         }
-        for(size_t i = 0; i < insideLocs1->size(); i++) {
-            delete(*insideLocs1)[i];
-        }
-        delete insideLocs1;
     }
-
-    delete locPtPoly;
-
-    // If minDistance <= terminateDistance we must have
-    // set minDistanceLocations to some non-null item
-    assert(minDistance > terminateDistance ||
-           ((*minDistanceLocation)[0] && (*minDistanceLocation)[1]));
 }
 
 
 /*private*/
 void
-DistanceOp::computeInside(vector<GeometryLocation*>* locs,
+DistanceOp::computeInside(vector<unique_ptr<GeometryLocation>> & locs,
                           const Polygon::ConstVect& polys,
-                          vector<GeometryLocation*>* locPtPoly)
+                          array<unique_ptr<GeometryLocation>, 2> & locPtPoly)
 {
-    for(size_t i = 0, ni = locs->size(); i < ni; ++i) {
-        GeometryLocation* loc = (*locs)[i];
-        for(size_t j = 0, nj = polys.size(); j < nj; ++j) {
-            computeInside(loc, polys[j], locPtPoly);
+    for(auto& loc : locs) {
+        for(const auto& poly : polys) {
+            computeInside(loc, poly, locPtPoly);
             if(minDistance <= terminateDistance) {
                 return;
             }
@@ -351,18 +270,17 @@ DistanceOp::computeInside(vector<GeometryLocation*>* locs,
 
 /*private*/
 void
-DistanceOp::computeInside(GeometryLocation* ptLoc,
+DistanceOp::computeInside(std::unique_ptr<GeometryLocation> & ptLoc,
                           const Polygon* poly,
-                          vector<GeometryLocation*>* locPtPoly)
+                          array<std::unique_ptr<GeometryLocation>, 2> & locPtPoly)
 {
     const Coordinate& pt = ptLoc->getCoordinate();
 
     // if pt is not in exterior, distance to geom is 0
     if(Location::EXTERIOR != ptLocator.locate(pt, static_cast<const Geometry*>(poly))) {
         minDistance = 0.0;
-        (*locPtPoly)[0] = ptLoc;
-        GeometryLocation* locPoly = new GeometryLocation(poly, pt);
-        (*locPtPoly)[1] = locPoly;
+        locPtPoly[0] = std::move(ptLoc);
+        locPtPoly[1].reset(new GeometryLocation(poly, pt));
         return;
     }
 }
@@ -374,12 +292,11 @@ DistanceOp::computeFacetDistance()
     using geom::util::LinearComponentExtracter;
     using geom::util::PointExtracter;
 
-    vector<GeometryLocation*> locGeom(2);
+    array<unique_ptr<GeometryLocation>, 2> locGeom;
 
     /**
-     * Geometries are not wholely inside, so compute distance from lines
-     * and points
-     * of one to lines and points of the other
+     * Geometries are not wholly inside, so compute distance from lines
+     * and points of one to lines and points of the other
      */
     LineString::ConstVect lines0;
     LineString::ConstVect lines1;
@@ -393,18 +310,6 @@ DistanceOp::computeFacetDistance()
               << std::endl;
 #endif
 
-    Point::ConstVect pts0;
-    Point::ConstVect pts1;
-    PointExtracter::getPoints(*(geom[0]), pts0);
-    PointExtracter::getPoints(*(geom[1]), pts1);
-
-#if GEOS_DEBUG
-    std::cerr << "PointExtracter found "
-              << pts0.size() << " points in geometry 1 and "
-              << pts1.size() << " points in geometry 2 "
-              << std::endl;
-#endif
-
     // exit whenever minDistance goes LE than terminateDistance
     computeMinDistanceLines(lines0, lines1, locGeom);
     updateMinDistance(locGeom, false);
@@ -413,7 +318,14 @@ DistanceOp::computeFacetDistance()
         std::cerr << "Early termination after line-line distance" << std::endl;
 #endif
         return;
-    };
+    }
+
+    Point::ConstVect pts1;
+    PointExtracter::getPoints(*(geom[1]), pts1);
+
+#if GEOS_DEBUG
+    std::cerr << "PointExtracter found " << pts1.size() << " points in geometry 2" << std::endl;
+#endif
 
     locGeom[0] = nullptr;
     locGeom[1] = nullptr;
@@ -424,7 +336,14 @@ DistanceOp::computeFacetDistance()
         std::cerr << "Early termination after lines0-points1 distance" << std::endl;
 #endif
         return;
-    };
+    }
+
+    Point::ConstVect pts0;
+    PointExtracter::getPoints(*(geom[0]), pts0);
+
+#if GEOS_DEBUG
+    std::cerr << "PointExtracter found " << pts0.size() << " points in geometry 1" << std::endl;
+#endif
 
     locGeom[0] = nullptr;
     locGeom[1] = nullptr;
@@ -435,7 +354,7 @@ DistanceOp::computeFacetDistance()
         std::cerr << "Early termination after lines1-points0 distance" << std::endl;
 #endif
         return;
-    };
+    }
 
     locGeom[0] = nullptr;
     locGeom[1] = nullptr;
@@ -452,12 +371,10 @@ void
 DistanceOp::computeMinDistanceLines(
     const LineString::ConstVect& lines0,
     const LineString::ConstVect& lines1,
-    vector<GeometryLocation*>& locGeom)
+    std::array<std::unique_ptr<GeometryLocation>, 2> & locGeom)
 {
-    for(size_t i = 0, ni = lines0.size(); i < ni; ++i) {
-        const LineString* line0 = lines0[i];
-        for(size_t j = 0, nj = lines1.size(); j < nj; ++j) {
-            const LineString* line1 = lines1[j];
+    for(const LineString* line0 : lines0) {
+        for(const LineString* line1 : lines1) {
             computeMinDistance(line0, line1, locGeom);
             if(minDistance <= terminateDistance) {
                 return;
@@ -471,12 +388,10 @@ void
 DistanceOp::computeMinDistancePoints(
     const Point::ConstVect& points0,
     const Point::ConstVect& points1,
-    vector<GeometryLocation*>& locGeom)
+    array<unique_ptr<GeometryLocation>, 2> & locGeom)
 {
-    for(size_t i = 0, ni = points0.size(); i < ni; ++i) {
-        const Point* pt0 = points0[i];
-        for(size_t j = 0, nj = points1.size(); j < nj; ++j) {
-            const Point* pt1 = points1[j];
+    for(const Point* pt0 : points0) {
+        for(const Point* pt1 : points1) {
             double dist = pt0->getCoordinate()->distance(*(pt1->getCoordinate()));
 
 #if GEOS_DEBUG
@@ -490,10 +405,8 @@ DistanceOp::computeMinDistancePoints(
             if(dist < minDistance) {
                 minDistance = dist;
                 // this is wrong - need to determine closest points on both segments!!!
-                delete locGeom[0];
-                locGeom[0] = new GeometryLocation(pt0, 0, *(pt0->getCoordinate()));
-                delete locGeom[1];
-                locGeom[1] = new GeometryLocation(pt1, 0, *(pt1->getCoordinate()));
+                locGeom[0].reset(new GeometryLocation(pt0, 0, *(pt0->getCoordinate())));
+                locGeom[1].reset(new GeometryLocation(pt1, 0, *(pt1->getCoordinate())));
             }
 
             if(minDistance <= terminateDistance) {
@@ -508,12 +421,10 @@ void
 DistanceOp::computeMinDistanceLinesPoints(
     const LineString::ConstVect& lines,
     const Point::ConstVect& points,
-    vector<GeometryLocation*>& locGeom)
+    std::array<std::unique_ptr<GeometryLocation>, 2> & locGeom)
 {
-    for(size_t i = 0; i < lines.size(); i++) {
-        const LineString* line = lines[i];
-        for(size_t j = 0; j < points.size(); j++) {
-            const Point* pt = points[j];
+    for(const LineString* line : lines) {
+        for(const Point* pt : points) {
             computeMinDistance(line, pt, locGeom);
             if(minDistance <= terminateDistance) {
                 return;
@@ -527,7 +438,7 @@ void
 DistanceOp::computeMinDistance(
     const LineString* line0,
     const LineString* line1,
-    vector<GeometryLocation*>& locGeom)
+    std::array<std::unique_ptr<GeometryLocation>, 2> & locGeom)
 {
     using geos::algorithm::Distance;
 
@@ -549,19 +460,16 @@ DistanceOp::computeMinDistance(
                           coord1->getAt(j), coord1->getAt(j + 1));
             if(dist < minDistance) {
                 minDistance = dist;
+
+                // TODO avoid copy from constructing segs, maybe
+                // by making a static closestPoints that takes four
+                // coordinate references
                 LineSegment seg0(coord0->getAt(i), coord0->getAt(i + 1));
                 LineSegment seg1(coord1->getAt(j), coord1->getAt(j + 1));
-                CoordinateSequence* closestPt = seg0.closestPoints(seg1);
-                Coordinate* c1 = new Coordinate(closestPt->getAt(0));
-                Coordinate* c2 = new Coordinate(closestPt->getAt(1));
-                newCoords.push_back(c1);
-                newCoords.push_back(c2);
-                delete closestPt;
-
-                delete locGeom[0];
-                locGeom[0] = new GeometryLocation(line0, i, *c1);
-                delete locGeom[1];
-                locGeom[1] = new GeometryLocation(line1, j, *c2);
+                auto closestPt = seg0.closestPoints(seg1);
+
+                locGeom[0].reset(new GeometryLocation(line0, i, closestPt[0]));
+                locGeom[1].reset(new GeometryLocation(line1, j, closestPt[1]));
             }
             if(minDistance <= terminateDistance) {
                 return;
@@ -574,7 +482,7 @@ DistanceOp::computeMinDistance(
 void
 DistanceOp::computeMinDistance(const LineString* line,
                                const Point* pt,
-                               vector<GeometryLocation*>& locGeom)
+                               std::array<std::unique_ptr<GeometryLocation>, 2> & locGeom)
 {
     using geos::algorithm::Distance;
 
@@ -584,8 +492,7 @@ DistanceOp::computeMinDistance(const LineString* line,
         return;
     }
     const CoordinateSequence* coord0 = line->getCoordinatesRO();
-    Coordinate* coord = new Coordinate(*(pt->getCoordinate()));
-    newCoords.push_back(coord);
+    const Coordinate* coord = pt->getCoordinate();
 
     // brute force approach!
     size_t npts0 = coord0->getSize();
@@ -593,14 +500,16 @@ DistanceOp::computeMinDistance(const LineString* line,
         double dist = Distance::pointToSegment(*coord, coord0->getAt(i), coord0->getAt(i + 1));
         if(dist < minDistance) {
             minDistance = dist;
+
+            // TODO avoid copy from constructing segs, maybe
+            // by making a static closestPoints that takes three
+            // coordinate references
             LineSegment seg(coord0->getAt(i), coord0->getAt(i + 1));
             Coordinate segClosestPoint;
             seg.closestPoint(*coord, segClosestPoint);
 
-            delete locGeom[0];
-            locGeom[0] = new GeometryLocation(line, i, segClosestPoint);
-            delete locGeom[1];
-            locGeom[1] = new GeometryLocation(pt, 0, *coord);
+            locGeom[0].reset(new GeometryLocation(line, i, segClosestPoint));
+            locGeom[1].reset(new GeometryLocation(pt, 0, *coord));
         }
         if(minDistance <= terminateDistance) {
             return;
diff --git a/src/operation/distance/FacetSequence.cpp b/src/operation/distance/FacetSequence.cpp
index bbc1c8a..5729ca4 100644
--- a/src/operation/distance/FacetSequence.cpp
+++ b/src/operation/distance/FacetSequence.cpp
@@ -200,16 +200,14 @@ FacetSequence::updateNearestLocationsLineLine(size_t i, const Coordinate& p0, co
 {
     LineSegment seg0(p0, p1);
     LineSegment seg1(q0, q1);
-    std::unique_ptr<CoordinateSequence> closestPts(seg0.closestPoints(seg1));
-    Coordinate c0, c1;
-    closestPts->getAt(0, c0);
-    closestPts->getAt(1, c1);
-    GeometryLocation gl0(geom, i, c0);
-    GeometryLocation gl1(facetSeq.geom, j, c1);
+
+    auto closestPts = seg0.closestPoints(seg1);
+    GeometryLocation gl0(geom, i, closestPts[0]);
+    GeometryLocation gl1(facetSeq.geom, j, closestPts[1]);
+
     locs->clear();
     locs->push_back(gl0);
     locs->push_back(gl1);
-    return;
 }
 
 void
diff --git a/tests/unit/operation/distance/DistanceOpTest.cpp b/tests/unit/operation/distance/DistanceOpTest.cpp
index 0b3fd82..1447b8c 100644
--- a/tests/unit/operation/distance/DistanceOpTest.cpp
+++ b/tests/unit/operation/distance/DistanceOpTest.cpp
@@ -64,7 +64,7 @@ void object::test<1>
 
     ensure_equals(dist.distance(), 10);
 
-    CSPtr cs(dist.closestPoints());
+    CSPtr cs(dist.nearestPoints());
     ensure_equals(cs->getAt(0), Coordinate(0, 0));
     ensure_equals(cs->getAt(1), Coordinate(10, 0));
 }
@@ -86,7 +86,7 @@ void object::test<2>
 
     ensure_equals(dist.distance(), 10);
 
-    CSPtr cs(dist.closestPoints());
+    CSPtr cs(dist.nearestPoints());
     ensure_equals(cs->getAt(0), Coordinate(0, 0));
     ensure_equals(cs->getAt(1), Coordinate(10, 0));
 
@@ -109,7 +109,7 @@ void object::test<3>
 
     ensure_equals(dist.distance(), 10);
 
-    CSPtr cs(dist.closestPoints());
+    CSPtr cs(dist.nearestPoints());
     ensure_equals(cs->getAt(0), Coordinate(3, 0));
     ensure_equals(cs->getAt(1), Coordinate(3, 10));
 
@@ -132,7 +132,7 @@ void object::test<4>
 
     ensure_equals(dist.distance(), 10);
 
-    CSPtr cs(dist.closestPoints());
+    CSPtr cs(dist.nearestPoints());
     ensure_equals(cs->getAt(0), Coordinate(3, 0));
     ensure_equals(cs->getAt(1), Coordinate(3, 10));
 
@@ -155,7 +155,7 @@ void object::test<5>
 
     ensure_equals(dist.distance(), 6);
 
-    CSPtr cs(dist.closestPoints());
+    CSPtr cs(dist.nearestPoints());
     ensure_equals(cs->getAt(0), Coordinate(35, 60));
     ensure_equals(cs->getAt(1), Coordinate(35, 54));
 
@@ -179,7 +179,7 @@ void object::test<6>
 
     ensure_equals(dist.distance(), 6);
 
-    CSPtr cs(dist.closestPoints());
+    CSPtr cs(dist.nearestPoints());
     ensure_equals(cs->getAt(0), Coordinate(35, 60));
     ensure_equals(cs->getAt(1), Coordinate(35, 54));
 
@@ -206,7 +206,7 @@ void object::test<7>
 
     ensure_equals(dist.distance(), 6);
 
-    CSPtr cs(dist.closestPoints());
+    CSPtr cs(dist.nearestPoints());
     ensure_equals(cs->getAt(0), Coordinate(35, 60));
     ensure_equals(cs->getAt(1), Coordinate(35, 54));
 }
@@ -231,7 +231,7 @@ void object::test<8>
 
     ensure_equals(dist.distance(), 0);
 
-    ensure_equals(dist.closestPoints(), (void*)nullptr);
+    ensure(dist.nearestPoints() == nullptr);
 }
 
 template<>
@@ -251,7 +251,7 @@ void object::test<9>
 
     ensure_equals(dist.distance(), 0);
 
-    CSPtr cs(dist.closestPoints());
+    CSPtr cs(dist.nearestPoints());
     ensure_equals(cs->getAt(0), Coordinate(10, 0));
     ensure_equals(cs->getAt(1), Coordinate(10, 0));
 
@@ -274,7 +274,7 @@ void object::test<10>
 
     ensure_equals(dist.distance(), 10);
 
-    CSPtr cs(dist.closestPoints());
+    CSPtr cs(dist.nearestPoints());
     ensure_equals(cs->getAt(0), Coordinate(10, 0));
     ensure_equals(cs->getAt(1), Coordinate(0, 0));
 
@@ -297,7 +297,7 @@ void object::test<11>
 
     ensure_equals(dist.distance(), 10);
 
-    CSPtr cs(dist.closestPoints());
+    CSPtr cs(dist.nearestPoints());
     ensure_equals(cs->getAt(0), Coordinate(3, 0));
     ensure_equals(cs->getAt(1), Coordinate(3, 10));
 
@@ -320,7 +320,7 @@ void object::test<12>
 
     ensure_equals(dist.distance(), 10);
 
-    CSPtr cs(dist.closestPoints());
+    CSPtr cs(dist.nearestPoints());
     ensure_equals(cs->getAt(0), Coordinate(3, 0));
     ensure_equals(cs->getAt(1), Coordinate(3, 10));
 
@@ -343,7 +343,7 @@ void object::test<13>
 
     ensure_equals(dist.distance(), 6);
 
-    CSPtr cs(dist.closestPoints());
+    CSPtr cs(dist.nearestPoints());
     ensure_equals(cs->getAt(0), Coordinate(35, 60));
     ensure_equals(cs->getAt(1), Coordinate(35, 54));
 
@@ -367,7 +367,7 @@ void object::test<14>
 
     ensure_equals(dist.distance(), 6);
 
-    CSPtr cs(dist.closestPoints());
+    CSPtr cs(dist.nearestPoints());
     ensure_equals(cs->getAt(0), Coordinate(35, 60));
     ensure_equals(cs->getAt(1), Coordinate(35, 54));
 }
@@ -393,7 +393,7 @@ void object::test<15>
 
     ensure_equals(dist.distance(), 6);
 
-    CSPtr cs(dist.closestPoints());
+    CSPtr cs(dist.nearestPoints());
     ensure_equals(cs->getAt(0), Coordinate(35, 60));
     ensure_equals(cs->getAt(1), Coordinate(35, 54));
 
@@ -419,8 +419,7 @@ void object::test<16>
 
     ensure_equals(dist.distance(), 0);
 
-    ensure_equals(dist.closestPoints(), (void*)nullptr);
-
+    ensure(dist.nearestPoints() == nullptr);
 }
 
 // Test for crash reported in Ticket #236:
@@ -442,7 +441,7 @@ void object::test<17>
     DistanceOp dist(g0.get(), g1.get());
     ensure_equals(dist.distance(), 0.25);
 
-    CSPtr cs(dist.closestPoints());
+    CSPtr cs(dist.nearestPoints());
     ensure_equals(cs->getAt(0), Coordinate(1, 0.25));
     ensure_equals(cs->getAt(1), Coordinate(1.25, 0.25));
 }
@@ -537,7 +536,7 @@ void object::test<20>()
     GeomPtr g1(gfact->createLineString(seq1.release()));
 
     DistanceOp dist(g0.get(), g1.get());
-    CSPtr seq(dist.closestPoints());
+    CSPtr seq(dist.nearestPoints());
 
     // input lines overlap, so generated point should intersect both geometries
     ensure(geos::geom::LineSegment(a0, a1).distance(seq->getAt(0)) < 1e-8);
@@ -546,8 +545,8 @@ void object::test<20>()
     ensure(geos::geom::LineSegment(b0, b1).distance(seq->getAt(1)) < 1e-8);
 
     // reverse argument order and check again
-    dist = DistanceOp(g1.get(), g0.get());
-    seq.reset(dist.closestPoints());
+    DistanceOp dist2(g1.get(), g0.get());
+    seq = dist2.nearestPoints();
 
     ensure(geos::geom::LineSegment(a0, a1).distance(seq->getAt(0)) < 1e-8);
     ensure(geos::geom::LineSegment(a0, a1).distance(seq->getAt(1)) < 1e-8);

-----------------------------------------------------------------------

Summary of changes:
 capi/geos_ts_c.cpp                                 |   2 +-
 include/geos/geom/LineSegment.h                    |   6 +-
 include/geos/geom/LineSegment.inl                  |   2 +-
 .../distance/ConnectedElementLocationFilter.h      |  17 +-
 include/geos/operation/distance/DistanceOp.h       |  86 ++-----
 src/geom/LineSegment.cpp                           |  30 +--
 .../distance/ConnectedElementLocationFilter.cpp    |  11 +-
 src/operation/distance/DistanceOp.cpp              | 269 +++++++--------------
 src/operation/distance/FacetSequence.cpp           |  12 +-
 tests/unit/operation/distance/DistanceOpTest.cpp   |  41 ++--
 10 files changed, 158 insertions(+), 318 deletions(-)


hooks/post-receive
-- 
GEOS


More information about the geos-commits mailing list