[geos-commits] [SCM] GEOS branch master updated. f1c5dcecc2daf47b76941919a60e3c06ace8f5a3

git at osgeo.org git at osgeo.org
Wed Sep 18 15:15:54 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  f1c5dcecc2daf47b76941919a60e3c06ace8f5a3 (commit)
       via  823467f36cb86c94a140351cfc2d7735fa1a713e (commit)
       via  a16006c14c56b7f68997f9c8c947cd9b64583dca (commit)
       via  3c5d8252a5a48e64685a76e0766fce68d7394275 (commit)
       via  628542854071fc8e8c314827b4b7697c877ce994 (commit)
      from  abaa838bb869419d178c96653c7d066422664bf5 (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 f1c5dcecc2daf47b76941919a60e3c06ace8f5a3
Author: Daniel Baston <dbaston at gmail.com>
Date:   Wed Sep 18 14:27:03 2019 -0400

    Use simpler GeometryFactory methods in GeometryTransformer

diff --git a/src/geom/util/GeometryTransformer.cpp b/src/geom/util/GeometryTransformer.cpp
index 49f3f08..67b9106 100644
--- a/src/geom/util/GeometryTransformer.cpp
+++ b/src/geom/util/GeometryTransformer.cpp
@@ -44,8 +44,6 @@
 #include <iostream>
 #endif
 
-using namespace std;
-
 namespace geos {
 namespace geom { // geos.geom
 namespace util { // geos.geom.util
@@ -74,7 +72,7 @@ GeometryTransformer::setSkipTransformedInvalidInteriorRings(bool b)
 }
 
 /*public*/
-unique_ptr<Geometry>
+std::unique_ptr<Geometry>
 GeometryTransformer::transform(const Geometry* nInputGeom)
 {
     using geos::util::IllegalArgumentException;
@@ -168,7 +166,7 @@ GeometryTransformer::transformMultiPoint(
               std::endl;
 #endif
 
-    vector<Geometry*>* transGeomList = new vector<Geometry*>();
+    std::vector<std::unique_ptr<Geometry>> transGeomList;
 
     for(size_t i = 0, n = geom->getNumGeometries(); i < n; i++) {
         const Point* p = dynamic_cast<const Point*>(geom->getGeometryN(i));
@@ -182,11 +180,10 @@ GeometryTransformer::transformMultiPoint(
             continue;
         }
 
-        // If an exception is thrown we'll leak
-        transGeomList->push_back(transformGeom.release());
+        transGeomList.push_back(std::move(transformGeom));
     }
 
-    return Geometry::Ptr(factory->buildGeometry(transGeomList));
+    return factory->buildGeometry(std::move(transGeomList));
 
 }
 
@@ -245,7 +242,7 @@ GeometryTransformer::transformMultiLineString(
               << std::endl;
 #endif
 
-    vector<Geometry*>* transGeomList = new vector<Geometry*>();
+    std::vector<std::unique_ptr<Geometry>> transGeomList;
 
     for(size_t i = 0, n = geom->getNumGeometries(); i < n; i++) {
         const LineString* l = dynamic_cast<const LineString*>(
@@ -260,11 +257,10 @@ GeometryTransformer::transformMultiLineString(
             continue;
         }
 
-        // If an exception is thrown we'll leak
-        transGeomList->push_back(transformGeom.release());
+        transGeomList.push_back(std::move(transformGeom));
     }
 
-    return Geometry::Ptr(factory->buildGeometry(transGeomList));
+    return factory->buildGeometry(std::move(transGeomList));
 
 }
 
@@ -292,7 +288,7 @@ GeometryTransformer::transformPolygon(
         isAllValidLinearRings = false;
     }
 
-    vector<LinearRing*>* holes = new vector<LinearRing*>();
+    std::vector<std::unique_ptr<LinearRing>> holes;
     for(size_t i = 0, n = geom->getNumInteriorRing(); i < n; i++) {
         const LinearRing* p_lr = geom->getInteriorRingN(i);
         assert(p_lr);
@@ -303,35 +299,32 @@ GeometryTransformer::transformPolygon(
             continue;
         }
 
-        if(! dynamic_cast<LinearRing*>(hole.get())) {
+        if(dynamic_cast<LinearRing*>(hole.get())) {
+            holes.emplace_back(dynamic_cast<LinearRing*>(hole.release()));
+        } else {
             if(skipTransformedInvalidInteriorRings) {
                 continue;
             }
             isAllValidLinearRings = false;
         }
 
-        holes->push_back(dynamic_cast<LinearRing*>(hole.release()));
     }
 
     if(isAllValidLinearRings) {
-        Geometry* sh = shell.release();
-        LinearRing* p_lr = dynamic_cast<LinearRing*>(sh);
-        assert(p_lr);
-        return Geometry::Ptr(factory->createPolygon(p_lr, holes));
+        std::unique_ptr<LinearRing> shell_lr(dynamic_cast<LinearRing*>(shell.release()));
+        return factory->createPolygon(std::move(shell_lr), std::move(holes));
     }
     else {
-        // would like to use a manager constructor here
-        vector<Geometry*>* components = new vector<Geometry*>();
+        std::vector<std::unique_ptr<Geometry>> components;
         if(shell.get() != nullptr) {
-            components->push_back(shell.release());
+            components.push_back(std::move(shell));
         }
 
-        components->insert(components->end(),
-                           holes->begin(), holes->end());
-
-        delete holes; // :(
+        for (auto& g : holes) {
+            components.push_back(std::move(g));
+        }
 
-        return Geometry::Ptr(factory->buildGeometry(components));
+        return factory->buildGeometry(std::move(components));
     }
 
 }
@@ -348,7 +341,7 @@ GeometryTransformer::transformMultiPolygon(
               std::endl;
 #endif
 
-    unique_ptr< vector<Geometry*> > transGeomList(new vector<Geometry*>());
+    std::vector<std::unique_ptr<Geometry>> transGeomList;
 
     for(std::size_t i = 0, n = geom->getNumGeometries(); i < n; i++) {
         const Polygon* p = dynamic_cast<const Polygon*>(
@@ -363,11 +356,10 @@ GeometryTransformer::transformMultiPolygon(
             continue;
         }
 
-        // If an exception is thrown we'll leak
-        transGeomList->push_back(transformGeom.release());
+        transGeomList.push_back(std::move(transformGeom));
     }
 
-    return Geometry::Ptr(factory->buildGeometry(transGeomList.release()));
+    return factory->buildGeometry(std::move(transGeomList));
 
 }
 
@@ -383,7 +375,7 @@ GeometryTransformer::transformGeometryCollection(
               << ");" << std::endl;
 #endif
 
-    vector<Geometry*>* transGeomList = new vector<Geometry*>();
+    std::vector<std::unique_ptr<Geometry>> transGeomList;
 
     for(std::size_t i = 0, n = geom->getNumGeometries(); i < n; i++) {
         Geometry::Ptr transformGeom = transform(
@@ -395,16 +387,14 @@ GeometryTransformer::transformGeometryCollection(
             continue;
         }
 
-        // If an exception is thrown we'll leak
-        transGeomList->push_back(transformGeom.release());
+        transGeomList.push_back(std::move(transformGeom));
     }
 
     if(preserveGeometryCollectionType) {
-        return Geometry::Ptr(factory->createGeometryCollection(
-                                 transGeomList));
+        return factory->createGeometryCollection(std::move(transGeomList));
     }
     else {
-        return Geometry::Ptr(factory->buildGeometry(transGeomList));
+        return factory->buildGeometry(std::move(transGeomList));
     }
 
 }

commit 823467f36cb86c94a140351cfc2d7735fa1a713e
Author: Daniel Baston <dbaston at gmail.com>
Date:   Wed Sep 18 12:07:14 2019 -0400

    Add GeometryFactory::buildGeometry signature and simplify implementations

diff --git a/include/geos/geom/GeometryFactory.h b/include/geos/geom/GeometryFactory.h
index 0c00b71..6393c67 100644
--- a/include/geos/geom/GeometryFactory.h
+++ b/include/geos/geom/GeometryFactory.h
@@ -328,6 +328,8 @@ public:
      */
     Geometry* buildGeometry(std::vector<Geometry*>* geoms) const;
 
+    std::unique_ptr<Geometry> buildGeometry(std::vector<std::unique_ptr<Geometry>> && geoms) const;
+
     /// See buildGeometry(std::vector<Geometry *>&) for semantics
     //
     /// Will clone the geometries accessible trough the iterator.
diff --git a/src/geom/GeometryFactory.cpp b/src/geom/GeometryFactory.cpp
index 7ba51e9..8925e2f 100644
--- a/src/geom/GeometryFactory.cpp
+++ b/src/geom/GeometryFactory.cpp
@@ -647,6 +647,32 @@ const
     return g;
 }
 
+template<typename T>
+GeometryTypeId commonType(const T& geoms) {
+    if (geoms.empty()) {
+        return GEOS_GEOMETRYCOLLECTION;
+    }
+
+    if (geoms.size() == 1) {
+        return geoms[0]->getGeometryTypeId();
+    }
+
+    GeometryTypeId type = geoms[0]->getGeometryTypeId();
+    for (size_t i = 1; i < geoms.size(); i++) {
+        if (geoms[i]->getGeometryTypeId() != type) {
+            return GEOS_GEOMETRYCOLLECTION;
+        }
+    }
+
+    switch(geoms[0]->getGeometryTypeId()) {
+        case GEOS_POINT: return GEOS_MULTIPOINT;
+        case GEOS_LINEARRING:
+        case GEOS_LINESTRING: return GEOS_MULTILINESTRING;
+        case GEOS_POLYGON: return GEOS_MULTIPOLYGON;
+        default: return GEOS_GEOMETRYCOLLECTION;
+    }
+}
+
 /*public*/
 Geometry*
 GeometryFactory::buildGeometry(vector<Geometry*>* newGeoms) const
@@ -657,91 +683,63 @@ GeometryFactory::buildGeometry(vector<Geometry*>* newGeoms) const
         return createGeometryCollection().release();
     }
 
-    bool isHeterogeneous = false;
-    bool hasGeometryCollection = false;
-    GeometryTypeId type = (*newGeoms)[0]->getGeometryTypeId();
+    if (newGeoms->size() == 1) {
+        Geometry* ret = (*newGeoms)[0];
+        delete newGeoms;
+        return ret;
+    }
 
-    for(Geometry* gp : *newGeoms) {
-        GeometryTypeId geometryType = gp->getGeometryTypeId();
-        if(type != geometryType) {
-            isHeterogeneous = true;
-        }
-        if(geometryType == GEOS_GEOMETRYCOLLECTION) {
-            hasGeometryCollection = true;
-        }
+    auto resultType = commonType(*newGeoms);
+
+    switch(resultType) {
+        case GEOS_MULTIPOINT: return createMultiPoint(newGeoms);
+        case GEOS_MULTILINESTRING: return createMultiLineString(newGeoms);
+        case GEOS_MULTIPOLYGON: return createMultiPolygon(newGeoms);
+        default: return createGeometryCollection(newGeoms);
     }
+}
 
-    if(isHeterogeneous || hasGeometryCollection) {
-        return createGeometryCollection(newGeoms);
+std::unique_ptr<Geometry>
+GeometryFactory::buildGeometry(std::vector<std::unique_ptr<Geometry>> && geoms) const
+{
+    if (geoms.empty()) {
+        return createGeometryCollection();
     }
 
-    // At this point we know the collection is not hetereogenous.
-    bool isCollection = newGeoms->size() > 1;
-    if(isCollection) {
-        if(type == GEOS_POLYGON) {
-            return createMultiPolygon(newGeoms);
-        }
-        else if(type == GEOS_LINESTRING) {
-            return createMultiLineString(newGeoms);
-        }
-        else if(type == GEOS_LINEARRING) {
-            return createMultiLineString(newGeoms);
-        }
-        else if(type == GEOS_POINT) {
-            return createMultiPoint(newGeoms);
-        }
-        else {
-            return createGeometryCollection(newGeoms);
-        }
+    if (geoms.size() == 1) {
+        return std::move(geoms[0]);
     }
 
-    // since this is not a collection we can delete vector
-    Geometry* geom0 = (*newGeoms)[0];
-    delete newGeoms;
-    return geom0;
+    auto resultType = commonType(geoms);
+
+    switch(resultType) {
+        case GEOS_MULTIPOINT: return createMultiPoint(std::move(geoms));
+        case GEOS_MULTILINESTRING: return createMultiLineString(std::move(geoms));
+        case GEOS_MULTIPOLYGON: return createMultiPolygon(std::move(geoms));
+        default: return createGeometryCollection(std::move(geoms));
+    }
 }
 
 /*public*/
 Geometry*
 GeometryFactory::buildGeometry(const vector<const Geometry*>& fromGeoms) const
 {
-    size_t geomsSize = fromGeoms.size();
-    if(geomsSize == 0) {
+    if(fromGeoms.empty()) {
         return createGeometryCollection().release();
     }
 
-    if(geomsSize == 1) {
+    if(fromGeoms.size() == 1) {
         return fromGeoms[0]->clone().release();
     }
 
-    bool isHeterogeneous = false;
-    GeometryTypeId type = fromGeoms[0]->getGeometryTypeId();
+    auto resultType = commonType(fromGeoms);
 
-    for(const Geometry* gp : fromGeoms) {
-        GeometryTypeId geometryType = gp->getGeometryTypeId();
-        if(type != geometryType) {
-            isHeterogeneous = true;
-        }
+    switch(resultType) {
+        case GEOS_MULTIPOINT: return createMultiPoint(fromGeoms);
+        case GEOS_MULTILINESTRING: return createMultiLineString(fromGeoms);
+        case GEOS_MULTIPOLYGON: return createMultiPolygon(fromGeoms);
+        default: return createGeometryCollection(fromGeoms);
     }
-
-    if(isHeterogeneous) {
-        return createGeometryCollection(fromGeoms);
-    }
-
-    if(type == GEOS_POLYGON) {
-        return createMultiPolygon(fromGeoms);
-    }
-    else if(type == GEOS_LINESTRING) {
-        return createMultiLineString(fromGeoms);
-    }
-    else if(type == GEOS_LINEARRING) {
-        return createMultiLineString(fromGeoms);
-    }
-    else if(type == GEOS_POINT) {
-        return createMultiPoint(fromGeoms);
-    }
-
-    throw geos::util::GEOSException("GeometryFactory::buildGeometry encountered an unknown geometry type!");
 }
 
 /*public*/

commit a16006c14c56b7f68997f9c8c947cd9b64583dca
Author: Daniel Baston <dbaston at gmail.com>
Date:   Wed Sep 18 11:13:33 2019 -0400

    Use simpler GeometryFactory methods in CoordinateOperation

diff --git a/src/geom/util/CoordinateOperation.cpp b/src/geom/util/CoordinateOperation.cpp
index 7676fb7..b3d383c 100644
--- a/src/geom/util/CoordinateOperation.cpp
+++ b/src/geom/util/CoordinateOperation.cpp
@@ -34,12 +34,12 @@ CoordinateOperation::edit(const Geometry* geometry,
         const CoordinateSequence* coords = ring->getCoordinatesRO();
         auto newCoords = edit(coords, geometry);
         // LinearRing instance takes over ownership of newCoords instance
-        return std::unique_ptr<Geometry>(factory->createLinearRing(newCoords.release()));
+        return factory->createLinearRing(std::move(newCoords));
     }
     if(const LineString* line = dynamic_cast<const LineString*>(geometry)) {
         const CoordinateSequence* coords = line->getCoordinatesRO();
         auto newCoords = edit(coords, geometry);
-        return std::unique_ptr<Geometry>(factory->createLineString(newCoords.release()));
+        return factory->createLineString(std::move(newCoords));
     }
     if(const Point* point = dynamic_cast<const Point*>(geometry)) {
         auto coords = point->getCoordinatesRO();

commit 3c5d8252a5a48e64685a76e0766fce68d7394275
Author: Daniel Baston <dbaston at gmail.com>
Date:   Wed Sep 18 11:11:09 2019 -0400

    Use simpler GeometryFactory methods in WKBReader

diff --git a/src/io/WKBReader.cpp b/src/io/WKBReader.cpp
index ff1e260..f264f38 100644
--- a/src/io/WKBReader.cpp
+++ b/src/io/WKBReader.cpp
@@ -291,7 +291,7 @@ WKBReader::readLineString()
     cout << "WKB npoints: " << size << endl;
 #endif
     auto pts = readCoordinateSequence(size);
-    return std::unique_ptr<LineString>(factory.createLineString(pts.release()));
+    return factory.createLineString(std::move(pts));
 }
 
 std::unique_ptr<LinearRing>
@@ -302,7 +302,7 @@ WKBReader::readLinearRing()
     cout << "WKB npoints: " << size << endl;
 #endif
     auto pts = readCoordinateSequence(size);
-    return std::unique_ptr<LinearRing>(factory.createLinearRing(pts.release()));
+    return factory.createLinearRing(std::move(pts));
 }
 
 std::unique_ptr<Polygon>

commit 628542854071fc8e8c314827b4b7697c877ce994
Author: Daniel Baston <dbaston at gmail.com>
Date:   Wed Sep 18 11:05:40 2019 -0400

    Avoid vector heap alloc in BuildArea

diff --git a/src/operation/polygonize/BuildArea.cpp b/src/operation/polygonize/BuildArea.cpp
index f120332..6b4b923 100644
--- a/src/operation/polygonize/BuildArea.cpp
+++ b/src/operation/polygonize/BuildArea.cpp
@@ -117,15 +117,15 @@ static void findFaceHoles(std::vector<std::unique_ptr<Face>>& faces) {
 
 static std::unique_ptr<geom::MultiPolygon> collectFacesWithEvenAncestors(
     std::vector<std::unique_ptr<Face>>& faces) {
-    std::vector<geom::Geometry*>* geoms = new std::vector<geom::Geometry*>();
+    std::vector<std::unique_ptr<geom::Geometry>> geoms;
     for( auto& face: faces ) {
         if( face->countParents() % 2 ) {
             continue; /* we skip odd parents geoms */
         }
-        geoms->push_back(face->poly->clone().release());
+        geoms.push_back(face->poly->clone());
     }
-    return std::unique_ptr<geom::MultiPolygon>(
-        GeometryFactory::create()->createMultiPolygon(geoms));
+    // TODO don't create new GeometryFactory here
+    return GeometryFactory::create()->createMultiPolygon(std::move(geoms));
 }
 
 #ifdef DUMP_GEOM
@@ -145,6 +145,7 @@ unique_ptr<geom::Geometry> BuildArea::build(const geom::Geometry* geom) {
 
     // No geometries in collection, early out
     if( polys->empty() ) {
+        // TODO don't create new GeometryFactory here
         auto emptyGeomCollection = unique_ptr<geom::Geometry>(
             GeometryFactory::create()->createGeometryCollection());
         emptyGeomCollection->setSRID(geom->getSRID());

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

Summary of changes:
 include/geos/geom/GeometryFactory.h    |   2 +
 src/geom/GeometryFactory.cpp           | 126 ++++++++++++++++-----------------
 src/geom/util/CoordinateOperation.cpp  |   4 +-
 src/geom/util/GeometryTransformer.cpp  |  62 +++++++---------
 src/io/WKBReader.cpp                   |   4 +-
 src/operation/polygonize/BuildArea.cpp |   9 +--
 6 files changed, 99 insertions(+), 108 deletions(-)


hooks/post-receive
-- 
GEOS


More information about the geos-commits mailing list