SimpleCurve / SegmentString: Store coordinates as std::shared_ptr (#1329)

Author: dbaston

include/geos/coverage/CoverageCleaner.h

@@ -323,7 +323,7 @@ class GEOS_DLL CoverageCleaner {
      */
     std::vector<const Polygon*> getMergedGaps();
 
-    std::unique_ptr<Geometry> toGeometry(
+    static std::unique_ptr<Geometry> toGeometry(
         std::vector<std::unique_ptr<SegmentString>>& segStrings,
         const GeometryFactory* geomFact);
 

include/geos/coverage/CoverageEdge.h

@@ -56,7 +56,7 @@ class GEOS_DLL CoverageEdge {
 private:
 
     // Members
-    std::unique_ptr<CoordinateSequence> m_pts;
+    std::shared_ptr<const CoordinateSequence> m_pts;
     std::size_t m_ringCount ;
     bool m_isFreeRing = true;
 
@@ -119,7 +119,7 @@ class GEOS_DLL CoverageEdge {
         const GeometryFactory* geomFactory);
 
     std::unique_ptr<LineString> toLineString(
-        const GeometryFactory* geomFactory);
+        const GeometryFactory* geomFactory) const;
 
     /* public */
     void incRingCount()
@@ -144,14 +144,14 @@ class GEOS_DLL CoverageEdge {
         return m_isFreeRing;
     }
 
-    void setCoordinates(const CoordinateSequence* pts)
+    void setCoordinates(const std::shared_ptr<const CoordinateSequence>& pts)
     {
-        m_pts = pts->clone();
+        m_pts = pts;
     }
 
-    const CoordinateSequence* getCoordinates() const
+    const std::shared_ptr<const CoordinateSequence>& getCoordinates() const
     {
-        return m_pts.get();
+        return m_pts;
     }
 
     const Coordinate& getEndCoordinate() const

include/geos/coverage/CoveragePolygonValidator.h

@@ -204,7 +204,6 @@ class GEOS_DLL CoveragePolygonValidator {
     double gapWidth = 0.0;
     std::vector<std::unique_ptr<CoveragePolygon>> m_adjCovPolygons;
     std::deque<CoverageRing> coverageRingStore;
-    std::vector<std::unique_ptr<CoordinateSequence>> localCoordinateSequences;
     std::deque<CoverageRingSegment> coverageRingSegmentStore;
 
     typedef std::unordered_map<CoverageRingSegment*, CoverageRingSegment*, CoverageRingSegment::CoverageRingSegHash, CoverageRingSegment::CoverageRingSegEq> CoverageRingSegmentMap;

include/geos/coverage/CoverageRing.h

@@ -78,7 +78,7 @@ class GEOS_DLL CoverageRing : public noding::BasicSegmentString {
 
 public:
 
-    CoverageRing(CoordinateSequence* pts, bool interiorOnRight);
+    CoverageRing(const std::shared_ptr<const CoordinateSequence>& pts, bool interiorOnRight);
 
     CoverageRing(const LinearRing* ring, bool isShell);
 

include/geos/geom/CircularString.h

@@ -58,6 +58,9 @@ class GEOS_DLL CircularString : public SimpleCurve {
     CircularString(std::unique_ptr<CoordinateSequence>&& pts,
                    const GeometryFactory& newFactory);
 
+    CircularString(const std::shared_ptr<const CoordinateSequence>& pts,
+                   const GeometryFactory& newFactory);
+
     CircularString* cloneImpl() const override
     {
         return new CircularString(*this);

include/geos/geom/GeometryFactory.h

@@ -236,6 +236,9 @@ class GEOS_DLL GeometryFactory {
     std::unique_ptr<LinearRing> createLinearRing(
         std::unique_ptr<CoordinateSequence> && newCoords) const;
 
+    std::unique_ptr<LinearRing> createLinearRing(
+        const std::shared_ptr<const CoordinateSequence>& newCoords) const;
+
     /// Construct a LinearRing with a deep-copy of given arguments
     std::unique_ptr<LinearRing> createLinearRing(
         const CoordinateSequence& coordinates) const;
@@ -312,6 +315,10 @@ class GEOS_DLL GeometryFactory {
     std::unique_ptr<LineString> createLineString(
         const CoordinateSequence& coordinates) const;
 
+    /// Construct a LineString with a reference to shared coordinates
+    std::unique_ptr<LineString> createLineString(
+        const std::shared_ptr<const CoordinateSequence>&) const;
+
     /// Construct an EMPTY CircularString
     std::unique_ptr<CircularString> createCircularString(bool hasZ, bool hasM) const;
 
@@ -322,6 +329,10 @@ class GEOS_DLL GeometryFactory {
     std::unique_ptr<CircularString> createCircularString(
         std::unique_ptr<CoordinateSequence> && coordinates) const;
 
+    /// Construct a CircularStrign with a reference to shared coordinates
+    std::unique_ptr<CircularString> createCircularString(
+        const std::shared_ptr<const CoordinateSequence>& coordinates) const;
+
     /// Construct a CircularString with a deep-copy of given argument
     std::unique_ptr<CircularString> createCircularString(
         const CoordinateSequence& coordinates) const;

include/geos/geom/LineString.h

@@ -114,6 +114,9 @@ class GEOS_DLL LineString: public SimpleCurve {
     LineString(CoordinateSequence::Ptr && pts,
                const GeometryFactory& newFactory);
 
+    LineString(const std::shared_ptr<const CoordinateSequence> & pts,
+               const GeometryFactory& newFactory);
+
     LineString* cloneImpl() const override { return new LineString(*this); }
 
     LineString* reverseImpl() const override;

include/geos/geom/LinearRing.h

@@ -78,6 +78,9 @@ class GEOS_DLL LinearRing : public LineString {
     LinearRing(CoordinateSequence::Ptr && points,
             const GeometryFactory& newFactory);
 
+    LinearRing(const std::shared_ptr<const CoordinateSequence>& points,
+            const GeometryFactory& newFactory);
+
     std::unique_ptr<LinearRing> clone() const
     {
         return std::unique_ptr<LinearRing>(cloneImpl());

include/geos/geom/SimpleCurve.h

@@ -61,6 +61,9 @@ class GEOS_DLL SimpleCurve : public Curve {
     /// Returns a read-only pointer to internal CoordinateSequence
     const CoordinateSequence* getCoordinatesRO() const;
 
+    /// Returns a read-only pointer to internal CoordinateSequence
+    const std::shared_ptr<const CoordinateSequence>& getSharedCoordinates() const;
+
     const SimpleCurve* getCurveN(std::size_t) const override;
 
     /// \brief
@@ -107,19 +110,14 @@ class GEOS_DLL SimpleCurve : public Curve {
      */
     void normalize() override;
 
-    /**
-     * \brief
-     * Take ownership of the CoordinateSequence managed by this geometry.
-     * After releasing the coordinates, the geometry should be considered
-     * in a moved-from state and should not be accessed.
-     * @return this Geometry's CoordinateSequence.
-     */
-    std::unique_ptr<CoordinateSequence> releaseCoordinates();
-
 protected:
 
     SimpleCurve(const SimpleCurve& other);
 
+    SimpleCurve(const std::shared_ptr<const CoordinateSequence>& newCoords,
+                bool isLinear,
+                const GeometryFactory& factory);
+
     SimpleCurve(std::unique_ptr<CoordinateSequence>&& newCoords,
                 bool isLinear,
                 const GeometryFactory& factory);
@@ -128,8 +126,7 @@ class GEOS_DLL SimpleCurve : public Curve {
 
     Envelope computeEnvelopeInternal(bool isLinear) const;
 
-    // TODO: hold value or shared_ptr instead of unique_ptr?
-    std::unique_ptr<CoordinateSequence> points;
+    std::shared_ptr<const CoordinateSequence> points;
     mutable Envelope envelope;
 
 

include/geos/geomgraph/EdgeNodingValidator.h

@@ -62,12 +62,6 @@ class GEOS_DLL EdgeNodingValidator final {
     // in turn expects this member to be initialized
     std::vector<std::unique_ptr<noding::SegmentString>> segStr;
 
-    // Make sure this member is initialized *before*
-    // the NodingValidator, as initialization of
-    // NodingValidator will use toSegmentString(), that
-    // in turn expects this member to be initialized
-    std::vector<geom::CoordinateSequence*> newCoordSeq;
-
     noding::FastNodingValidator nv;
 
 public:
@@ -91,7 +85,6 @@ class GEOS_DLL EdgeNodingValidator final {
     EdgeNodingValidator(std::vector<Edge*>& edges)
         :
         segStr(),
-        newCoordSeq(),
         nv(toSegmentStrings(edges))
     {}