[geos-commits] [SCM] GEOS branch master updated. 7a7cbf84fc5d4369baaf4c6e0614f44d4207503f

git at osgeo.org git at osgeo.org
Wed Nov 27 17:59:58 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  7a7cbf84fc5d4369baaf4c6e0614f44d4207503f (commit)
       via  ec5d8b2a7ea8eed70450145a6d4f0da182b87dd8 (commit)
       via  695b30dd48d6371ce0882bf2537fe2fd8d73c6a7 (commit)
      from  bc180e14fab95c77044647525e0854fbf98a8200 (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 7a7cbf84fc5d4369baaf4c6e0614f44d4207503f
Author: Daniel Baston <dbaston at gmail.com>
Date:   Sun Nov 24 21:49:53 2019 -0500

    Simplify GEOSLineMerge_r

diff --git a/capi/geos_ts_c.cpp b/capi/geos_ts_c.cpp
index 5b4944a..34ec1e6 100644
--- a/capi/geos_ts_c.cpp
+++ b/capi/geos_ts_c.cpp
@@ -1890,26 +1890,15 @@ extern "C" {
         return execute(extHandle, [&]() {
             GEOSContextHandleInternal_t* handle = reinterpret_cast<GEOSContextHandleInternal_t*>(extHandle);
             const GeometryFactory* gf = handle->geomFactory;
-            Geometry* out;
             LineMerger lmrgr;
             lmrgr.add(g);
 
             std::vector<LineString*>* lines = lmrgr.getMergedLineStrings();
-            assert(0 != lines);
-
-            // TODO avoid "new" here
 
-            std::vector<Geometry*>* geoms = new std::vector<Geometry*>(lines->size());
-            for(std::vector<Geometry*>::size_type i = 0; i < lines->size(); ++i) {
-                (*geoms)[i] = (*lines)[i];
-            }
-            delete lines;
-            lines = 0;
-
-            out = gf->buildGeometry(geoms);
+            auto out = gf->buildGeometry(lines->begin(), lines->end());
             out->setSRID(g->getSRID());
 
-            return out;
+            return out.release();
         });
     }
 

commit ec5d8b2a7ea8eed70450145a6d4f0da182b87dd8
Author: Daniel Baston <dbaston at gmail.com>
Date:   Sun Nov 24 21:44:27 2019 -0500

    Remove heap alloc in GEOSGeom_createCollection_r

diff --git a/capi/geos_ts_c.cpp b/capi/geos_ts_c.cpp
index 736b12d..5b4944a 100644
--- a/capi/geos_ts_c.cpp
+++ b/capi/geos_ts_c.cpp
@@ -1285,7 +1285,6 @@ extern "C" {
         return execute(extHandle, [&]() {
             auto ret = g1->getInteriorPoint();
             if(ret == nullptr) {
-                // TODO check if it is really possible for getInteriorPoint to return null
                 const GeometryFactory* gf = g1->getFactory();
                 // return an empty point
                 ret = gf->createPoint();
@@ -1683,31 +1682,31 @@ extern "C" {
             GEOSContextHandleInternal_t* handle = reinterpret_cast<GEOSContextHandleInternal_t*>(extHandle);
 
             const GeometryFactory* gf = handle->geomFactory;
-            // TODO avoid new here
-            std::vector<Geometry*>* vgeoms = new std::vector<Geometry*>(geoms, geoms + ngeoms);
 
-            Geometry* g = 0;
+            std::vector<std::unique_ptr<Geometry>> vgeoms(ngeoms);
+            for (size_t i = 0; i < ngeoms; i++) {
+                vgeoms[i].reset(geoms[i]);
+            }
+
+            std::unique_ptr<Geometry> g;
             switch(type) {
             case GEOS_GEOMETRYCOLLECTION:
-                g = gf->createGeometryCollection(vgeoms);
+                g = gf->createGeometryCollection(std::move(vgeoms));
                 break;
             case GEOS_MULTIPOINT:
-                g = gf->createMultiPoint(vgeoms);
+                g = gf->createMultiPoint(std::move(vgeoms));
                 break;
             case GEOS_MULTILINESTRING:
-                g = gf->createMultiLineString(vgeoms);
+                g = gf->createMultiLineString(std::move(vgeoms));
                 break;
             case GEOS_MULTIPOLYGON:
-                g = gf->createMultiPolygon(vgeoms);
+                g = gf->createMultiPolygon(std::move(vgeoms));
                 break;
             default:
                 handle->ERROR_MESSAGE("Unsupported type request for PostGIS2GEOS_collection");
-                delete vgeoms;
-                g = 0;
-
             }
 
-            return g;
+            return g.release();
         });
     }
 

commit 695b30dd48d6371ce0882bf2537fe2fd8d73c6a7
Author: Daniel Baston <dbaston at gmail.com>
Date:   Sun Nov 24 21:12:55 2019 -0500

    Simplify CAPI polygonizer code

diff --git a/capi/geos_ts_c.cpp b/capi/geos_ts_c.cpp
index 66d9373..736b12d 100644
--- a/capi/geos_ts_c.cpp
+++ b/capi/geos_ts_c.cpp
@@ -1726,31 +1726,8 @@ extern "C" {
             }
 
             auto polys = plgnzr.getPolygons();
-            assert(0 != polys);
-
-            // We need a vector of Geometry pointers, not Polygon pointers.
-            // STL vector doesn't allow transparent upcast of this
-            // nature, so we explicitly convert.
-            // (it's just a waste of processor and memory, btw)
-            //
-            // XXX mloskot: Why not to extent GeometryFactory to accept
-            // vector of polygons or extend Polygonizer to return list of Geometry*
-            // or add a wrapper which semantic is similar to:
-            // std::vector<as_polygon<Geometry*> >
-
-            // TODO avoid new here
-            std::vector<Geometry*>* polyvec = new std::vector<Geometry*>(polys->size());
-
-            for(std::size_t i = 0; i < polys->size(); ++i) {
-                (*polyvec)[i] = (*polys)[i].release();
-            }
-            polys = 0;
-
             const GeometryFactory* gf = handle->geomFactory;
-
-            // The below takes ownership of the passed vector,
-            // so we must *not* delete it
-            return gf->createGeometryCollection(polyvec);
+            return gf->createGeometryCollection(std::move(polys)).release();
         });
     }
 
@@ -1772,18 +1749,12 @@ extern "C" {
             }
 
             auto polys = plgnzr.getPolygons();
-            if (polys->empty()) {
+            if (polys.empty()) {
                 out = handle->geomFactory->createGeometryCollection().release();
-            } else if (polys->size() == 1) {
-                return (*polys)[0].release();
+            } else if (polys.size() == 1) {
+                return polys[0].release();
             } else {
-                // TODO avoid new here
-                auto geoms = new std::vector<Geometry *>(polys->size());
-                for (size_t i = 0; i < polys->size(); i++) {
-                    (*geoms)[i] = (*polys)[i].release();
-                }
-
-                return handle->geomFactory->createMultiPolygon(geoms);
+                return handle->geomFactory->createMultiPolygon(std::move(polys)).release();
             }
 
             out->setSRID(srid);
@@ -1876,54 +1847,37 @@ extern "C" {
             const GeometryFactory* gf = g->getFactory();
 
             if(cuts) {
-                // TODO avoid "new" here
-
                 const std::vector<const LineString*>& lines = plgnzr.getCutEdges();
-                std::vector<Geometry*>* linevec = new std::vector<Geometry*>(lines.size());
+                std::vector<std::unique_ptr<Geometry>> linevec(lines.size());
                 for(std::size_t i = 0, n = lines.size(); i < n; ++i) {
-                    (*linevec)[i] = lines[i]->clone().release();
+                    linevec[i] = lines[i]->clone();
                 }
 
-                // The below takes ownership of the passed vector,
-                // so we must *not* delete it
-                *cuts = gf->createGeometryCollection(linevec);
+                *cuts = gf->createGeometryCollection(std::move(linevec)).release();
             }
 
             if(dangles) {
-                // TODO avoid "new" here
-
                 const std::vector<const LineString*>& lines = plgnzr.getDangles();
-                std::vector<Geometry*>* linevec = new std::vector<Geometry*>(lines.size());
+                std::vector<std::unique_ptr<Geometry>> linevec(lines.size());
                 for(std::size_t i = 0, n = lines.size(); i < n; ++i) {
-                    (*linevec)[i] = lines[i]->clone().release();
+                    linevec[i] = lines[i]->clone();
                 }
 
-                // The below takes ownership of the passed vector,
-                // so we must *not* delete it
-                *dangles = gf->createGeometryCollection(linevec);
+                *dangles = gf->createGeometryCollection(std::move(linevec)).release();
             }
 
             if(invalid) {
-                // TODO avoid "new" here
-
                 const std::vector<std::unique_ptr<LineString>>& lines = plgnzr.getInvalidRingLines();
-                std::vector<Geometry*>* linevec = new std::vector<Geometry*>(lines.size());
+                std::vector<std::unique_ptr<Geometry>> linevec(lines.size());
                 for(std::size_t i = 0, n = lines.size(); i < n; ++i) {
-                    (*linevec)[i] = lines[i]->clone().release();
+                    linevec[i] = lines[i]->clone();
                 }
 
-                // The below takes ownership of the passed vector,
-                // so we must *not* delete it
-                *invalid = gf->createGeometryCollection(linevec);
+                *invalid = gf->createGeometryCollection(std::move(linevec)).release();
             }
 
             auto polys = plgnzr.getPolygons();
-            std::vector<Geometry*>* polyvec = new std::vector<Geometry*>(polys->size());
-            for(std::size_t i = 0; i < polys->size(); ++i) {
-                (*polyvec)[i] = (*polys)[i].release();
-            }
-
-            Geometry* out = gf->createGeometryCollection(polyvec);
+            Geometry* out = gf->createGeometryCollection(std::move(polys)).release();
             out->setSRID(g->getSRID());
             return out;
         });
diff --git a/doc/example.cpp b/doc/example.cpp
index 0a6b32c..bc1f324 100644
--- a/doc/example.cpp
+++ b/doc/example.cpp
@@ -1100,8 +1100,8 @@ do_all()
     plgnzr.add(geoms);
     auto polys = plgnzr.getPolygons();
     newgeoms = new vector<const Geometry*>;
-    for(unsigned int i = 0; i < polys->size(); i++) {
-        newgeoms->push_back((*polys)[i].release());
+    for(unsigned int i = 0; i < polys.size(); i++) {
+        newgeoms->push_back(polys[i].release());
     }
 
     cout << endl << "----- HERE IS POLYGONIZE OUTPUT ------" << endl;
diff --git a/include/geos/geom/GeometryFactory.h b/include/geos/geom/GeometryFactory.h
index a33d03f..c961745 100644
--- a/include/geos/geom/GeometryFactory.h
+++ b/include/geos/geom/GeometryFactory.h
@@ -185,8 +185,12 @@ public:
     GeometryCollection* createGeometryCollection(
         std::vector<Geometry*>* newGeoms) const;
 
+    template<typename T>
     std::unique_ptr<GeometryCollection> createGeometryCollection(
-            std::vector<std::unique_ptr<Geometry>> && newGeoms) const;
+            std::vector<std::unique_ptr<T>> && newGeoms) const {
+        // Can't use make_unique because constructor is protected
+        return std::unique_ptr<GeometryCollection>(new GeometryCollection(std::move(newGeoms), *this));
+    }
 
     /// Constructs a GeometryCollection with a deep-copy of args
     GeometryCollection* createGeometryCollection(
diff --git a/include/geos/operation/polygonize/Polygonizer.h b/include/geos/operation/polygonize/Polygonizer.h
index ca4202b..04b2c72 100644
--- a/include/geos/operation/polygonize/Polygonizer.h
+++ b/include/geos/operation/polygonize/Polygonizer.h
@@ -118,9 +118,10 @@ private:
 
     static void findOuterShells(std::vector<EdgeRing*>& shellList);
 
-    static std::unique_ptr<std::vector<std::unique_ptr<geom::Polygon>>> extractPolygons(std::vector<EdgeRing*> & shellList, bool includeAll);
+    static std::vector<std::unique_ptr<geom::Polygon>> extractPolygons(std::vector<EdgeRing*> & shellList, bool includeAll);
 
     bool extractOnlyPolygonal;
+    bool computed;
 
 protected:
 
@@ -133,7 +134,7 @@ protected:
 
     std::vector<EdgeRing*> holeList;
     std::vector<EdgeRing*> shellList;
-    std::unique_ptr<std::vector<std::unique_ptr<geom::Polygon>>> polyList;
+    std::vector<std::unique_ptr<geom::Polygon>> polyList;
 
 public:
 
@@ -194,7 +195,7 @@ public:
      * calls will return NULL.
      * @return a collection of Polygons
      */
-    std::unique_ptr<std::vector<std::unique_ptr<geom::Polygon>>> getPolygons();
+    std::vector<std::unique_ptr<geom::Polygon>> getPolygons();
 
     /** \brief
      * Get the list of dangling lines found during polygonization.
diff --git a/src/geom/GeometryFactory.cpp b/src/geom/GeometryFactory.cpp
index fc692f6..6cfa396 100644
--- a/src/geom/GeometryFactory.cpp
+++ b/src/geom/GeometryFactory.cpp
@@ -398,12 +398,6 @@ GeometryFactory::createGeometryCollection(vector<Geometry*>* newGeoms) const
     return new GeometryCollection(newGeoms, this);
 }
 
-std::unique_ptr<GeometryCollection>
-GeometryFactory::createGeometryCollection(std::vector<std::unique_ptr<geos::geom::Geometry>> && newGeoms) const {
-    // Can't use make_unique because constructor is protected
-    return std::unique_ptr<GeometryCollection>(new GeometryCollection(std::move(newGeoms), *this));
-}
-
 /*public*/
 GeometryCollection*
 GeometryFactory::createGeometryCollection(const std::vector<const Geometry*>& fromGeoms) const
diff --git a/src/operation/polygonize/BuildArea.cpp b/src/operation/polygonize/BuildArea.cpp
index 6b4b923..2302d20 100644
--- a/src/operation/polygonize/BuildArea.cpp
+++ b/src/operation/polygonize/BuildArea.cpp
@@ -144,7 +144,7 @@ unique_ptr<geom::Geometry> BuildArea::build(const geom::Geometry* geom) {
     auto polys = polygonizer.getPolygons();
 
     // No geometries in collection, early out
-    if( polys->empty() ) {
+    if( polys.empty() ) {
         // TODO don't create new GeometryFactory here
         auto emptyGeomCollection = unique_ptr<geom::Geometry>(
             GeometryFactory::create()->createGeometryCollection());
@@ -153,8 +153,8 @@ unique_ptr<geom::Geometry> BuildArea::build(const geom::Geometry* geom) {
     }
 
     // Return first geometry if we only have one in collection
-    if( polys->size() == 1 ) {
-        auto ret = std::unique_ptr<geom::Geometry>((*polys)[0].release());
+    if( polys.size() == 1 ) {
+        std::unique_ptr<Geometry> ret = std::move(polys[0]);
         ret->setSRID(geom->getSRID());
         return ret;
     }
@@ -189,7 +189,7 @@ unique_ptr<geom::Geometry> BuildArea::build(const geom::Geometry* geom) {
 
     /* Prepare face structures for later analysis */
     std::vector<std::unique_ptr<Face>> faces;
-    for(auto& poly: *polys) {
+    for(const auto& poly: polys) {
         faces.emplace_back(newFace(poly.get()));
     }
 
diff --git a/src/operation/polygonize/Polygonizer.cpp b/src/operation/polygonize/Polygonizer.cpp
index 38bd0b1..e146bdc 100644
--- a/src/operation/polygonize/Polygonizer.cpp
+++ b/src/operation/polygonize/Polygonizer.cpp
@@ -63,13 +63,13 @@ Polygonizer::LineStringAdder::filter_ro(const Geometry* g)
 Polygonizer::Polygonizer(bool onlyPolygonal):
     lineStringAdder(this),
     extractOnlyPolygonal(onlyPolygonal),
+    computed(false),
     graph(nullptr),
     dangles(),
     cutEdges(),
     invalidRingLines(),
     holeList(),
-    shellList(),
-    polyList(nullptr)
+    shellList()
 {
 }
 
@@ -152,7 +152,7 @@ Polygonizer::add(const LineString* line)
  * Gets the list of polygons formed by the polygonization.
  * @return a collection of Polygons
  */
-unique_ptr<vector<unique_ptr<Polygon>>>
+std::vector<std::unique_ptr<Polygon>>
 Polygonizer::getPolygons()
 {
     polygonize();
@@ -215,13 +215,13 @@ void
 Polygonizer::polygonize()
 {
     // check if already computed
-    if(polyList != nullptr) {
+    if(computed) {
         return;
     }
 
     // if no geometries were supplied it's possible graph could be null
     if(graph == nullptr) {
-        polyList.reset(new std::vector<std::unique_ptr<Polygon>>());
+        polyList.clear();
         return;
     }
 
@@ -256,6 +256,8 @@ Polygonizer::polygonize()
         includeAll = false;
     }
     polyList = extractPolygons(shellList, includeAll);
+
+    computed = true;
 }
 
 /* private */
@@ -320,13 +322,13 @@ Polygonizer::findOuterShells(vector<EdgeRing*> & shells)
     }
 }
 
-std::unique_ptr<std::vector<std::unique_ptr<Polygon>>>
+std::vector<std::unique_ptr<Polygon>>
 Polygonizer::extractPolygons(vector<EdgeRing*> & shells, bool includeAll)
 {
-    std::unique_ptr<std::vector<std::unique_ptr<Polygon>>> polys(new std::vector<std::unique_ptr<Polygon>>);
+    std::vector<std::unique_ptr<Polygon>> polys;
     for (EdgeRing* er : shells) {
         if (includeAll || er->isIncluded()) {
-            polys->emplace_back(er->getPolygon());
+            polys.emplace_back(er->getPolygon());
         }
     }
 
diff --git a/src/operation/union/CoverageUnion.cpp b/src/operation/union/CoverageUnion.cpp
index 4988d5a..5e10ca4 100644
--- a/src/operation/union/CoverageUnion.cpp
+++ b/src/operation/union/CoverageUnion.cpp
@@ -97,11 +97,11 @@ std::unique_ptr<Geometry> CoverageUnion::polygonize(const GeometryFactory* gf) {
     auto polygons = p.getPolygons();
     segment_geoms.reset();
 
-    if (polygons->size() == 1) {
-        return std::unique_ptr<Geometry>((*polygons)[0].release());
+    if (polygons.size() == 1) {
+        return std::move(polygons[0]);
     }
 
-    return gf->createMultiPolygon(std::move(*polygons));
+    return gf->createMultiPolygon(std::move(polygons));
 }
 
 std::unique_ptr<geom::Geometry> CoverageUnion::Union(const geom::Geometry* geom) {
diff --git a/tests/unit/operation/polygonize/PolygonizeTest.cpp b/tests/unit/operation/polygonize/PolygonizeTest.cpp
index 62a35a5..a668aba 100644
--- a/tests/unit/operation/polygonize/PolygonizeTest.cpp
+++ b/tests/unit/operation/polygonize/PolygonizeTest.cpp
@@ -119,18 +119,17 @@ struct test_polygonizetest_data {
             polygonizer.add(p.get());
         }
 
-        std::unique_ptr<std::vector<std::unique_ptr<Poly>> > retGeoms;
-        retGeoms = polygonizer.getPolygons();
-        for (const auto& g : *retGeoms) {
+        auto retGeoms = polygonizer.getPolygons();
+        for (const auto& g : retGeoms) {
             g->normalize();
         }
 
-        bool ok = compare(expectGeoms, *retGeoms);
+        bool ok = compare(expectGeoms, retGeoms);
         if(! ok) {
             cout << "EXPECTED(" << expectGeoms.size() << "): " << std::endl;
             printAll(cout, expectGeoms);
-            cout << "OBTAINED(" << retGeoms->size() << "): " << std::endl;
-            printAll(cout, *retGeoms);
+            cout << "OBTAINED(" << retGeoms.size() << "): " << std::endl;
+            printAll(cout, retGeoms);
             cout << endl;
 
             ensure("not all expected geometries in the obtained set", 0);
diff --git a/tests/xmltester/XMLTester.cpp b/tests/xmltester/XMLTester.cpp
index 1c71baf..c133809 100644
--- a/tests/xmltester/XMLTester.cpp
+++ b/tests/xmltester/XMLTester.cpp
@@ -1512,8 +1512,8 @@ XMLTester::parseTest(const tinyxml2::XMLNode* node)
 
             auto polys = plgnzr.getPolygons();
             std::vector<geom::Geometry*>* newgeoms = new std::vector<geom::Geometry*>;
-            for(unsigned int i = 0; i < polys->size(); i++) {
-                newgeoms->push_back((*polys)[i].release());
+            for(unsigned int i = 0; i < polys.size(); i++) {
+                newgeoms->push_back(polys[i].release());
             }
 
             GeomPtr gRealRes(factory->createGeometryCollection(newgeoms));

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

Summary of changes:
 capi/geos_ts_c.cpp                                 | 114 +++++----------------
 doc/example.cpp                                    |   4 +-
 include/geos/geom/GeometryFactory.h                |   6 +-
 include/geos/operation/polygonize/Polygonizer.h    |   7 +-
 src/geom/GeometryFactory.cpp                       |   6 --
 src/operation/polygonize/BuildArea.cpp             |   8 +-
 src/operation/polygonize/Polygonizer.cpp           |  18 ++--
 src/operation/union/CoverageUnion.cpp              |   6 +-
 tests/unit/operation/polygonize/PolygonizeTest.cpp |  11 +-
 tests/xmltester/XMLTester.cpp                      |   4 +-
 10 files changed, 63 insertions(+), 121 deletions(-)


hooks/post-receive
-- 
GEOS


More information about the geos-commits mailing list