Fix TopologyPreservingSimplifier to prevent jumping components causin…

…g incorrect topology (#1012)

Fixes a long-standing bug in TopologyPreservingSimplifier which created incorrect output due to simplified edges "jumping" over components in the input geometry.

The fix handles both polygonal and linear inputs. (Previously, polygonal inputs with "jumps" produced invalid results. Linear inputs produced results whose topology did not match the input).

See locationtech/jts#1024

include/geos/algorithm/RayCrossingCounter.h

@@ -66,7 +66,7 @@ class GEOS_DLL RayCrossingCounter {
 private:
     const geom::CoordinateXY& point;
 
-    int crossingCount;
+    std::size_t crossingCount;
 
     // true if the test point lies on an input segment
     bool isPointOnSegment;
@@ -145,6 +145,8 @@ class GEOS_DLL RayCrossingCounter {
      */
     bool isPointInPolygon() const;
 
+    std::size_t getCount() const { return crossingCount; };
+
 };
 
 } // geos::algorithm

include/geos/simplify/ComponentJumpChecker.h

@@ -0,0 +1,120 @@
+/**********************************************************************
+ *
+ * GEOS - Geometry Engine Open Source
+ * http://libgeos.org
+ *
+ * Copyright (C) 2006 Refractions Research Inc.
+ * Copyright (C) 2023 Martin Davis <mtnclimb@gmail.com>
+ * Copyright (C) 2023 Paul Ramsey <pramsey@cleverelephant.ca>
+ *
+ * This is free software; you can redistribute and/or modify it under
+ * the terms of the GNU Lesser General Licence as published
+ * by the Free Software Foundation.
+ * See the COPYING file for more information.
+ *
+ **********************************************************************/
+
+#pragma once
+
+#include <geos/export.h>
+#include <vector>
+#include <memory>
+
+
+// Forward declarations
+namespace geos {
+namespace geom {
+class Coordinate;
+class CoordinateSequence;
+class Envelope;
+class LineSegment;
+}
+namespace simplify {
+class TaggedLineString;
+}
+}
+
+using geos::geom::Coordinate;
+using geos::geom::Envelope;
+using geos::geom::LineSegment;
+
+namespace geos {
+namespace simplify { // geos::simplify
+
+
+class GEOS_DLL ComponentJumpChecker {
+
+private:
+
+    const std::vector<TaggedLineString*>& components;
+
+    static bool hasJumpAtComponent(
+        const Coordinate& compPt,
+        const TaggedLineString* line,
+        std::size_t start, std::size_t end,
+        const LineSegment& seg);
+
+    static bool hasJumpAtComponent(
+        const Coordinate& compPt,
+        const LineSegment* seg1, const LineSegment* seg2,
+        const LineSegment& seg);
+
+    static std::size_t crossingCount(
+        const Coordinate& compPt,
+        const LineSegment& seg);
+
+    static std::size_t crossingCount(
+        const Coordinate& compPt,
+        const LineSegment* seg1, const LineSegment* seg2);
+
+    std::size_t static  crossingCount(
+        const Coordinate& compPt,
+        const TaggedLineString* line,
+        std::size_t start, std::size_t end);
+
+    static Envelope computeEnvelope(
+        const LineSegment* seg1, const LineSegment* seg2);
+
+    static Envelope computeEnvelope(
+        const TaggedLineString* line,
+        std::size_t start, std::size_t end);
+
+
+public:
+
+    ComponentJumpChecker(const std::vector<TaggedLineString*>& taggedLines)
+        : components(taggedLines)
+    {}
+
+    bool hasJump(
+        const TaggedLineString* line,
+        std::size_t start, std::size_t end,
+        const LineSegment& seg) const;
+
+    /**
+    * Checks if two consecutive segments jumps a component if flattened.
+    * The segments are assumed to be consecutive.
+    * (so the seg1.p1 = seg2.p0).
+    * The flattening segment must be the segment between seg1.p0 and seg2.p1.
+    *
+    * @param line the line containing the section being flattened
+    * @param seg1 the first replaced segment
+    * @param seg2 the next replaced segment
+    * @param seg the flattening segment
+    * @return true if the flattened segment jumps a component
+    */
+    bool hasJump(
+        const TaggedLineString* line,
+        const LineSegment* seg1,
+        const LineSegment* seg2,
+        const LineSegment& seg) const;
+
+};
+
+} // namespace geos::simplify
+} // namespace geos
+
+
+
+
+

include/geos/simplify/TaggedLineString.h

@@ -47,6 +47,9 @@ class TaggedLineSegment;
 }
 }
 
+using geos::geom::Coordinate;
+using geos::geom::CoordinateSequence;
+
 namespace geos {
 namespace simplify { // geos::simplify
 
@@ -58,30 +61,36 @@ class GEOS_DLL TaggedLineString {
 
 public:
 
-    typedef std::vector<geom::Coordinate> CoordVect;
+    typedef std::vector<Coordinate> CoordVect;
 
     typedef std::unique_ptr<CoordVect> CoordVectPtr;
 
-    typedef geom::CoordinateSequence CoordSeq;
+    typedef CoordinateSequence CoordSeq;
 
-    typedef std::unique_ptr<geom::CoordinateSequence> CoordSeqPtr;
+    typedef std::unique_ptr<CoordinateSequence> CoordSeqPtr;
 
     TaggedLineString(const geom::LineString* nParentLine,
                      std::size_t minimumSize,
-                     bool preserveEndpoint);
+                     bool bIsRing);
 
     ~TaggedLineString();
 
     std::size_t getMinimumSize() const;
 
-    bool getPreserveEndpoint() const;
+    bool isRing() const;
 
     const geom::LineString* getParent() const;
 
     const CoordSeq* getParentCoordinates() const;
 
     CoordSeqPtr getResultCoordinates() const;
 
+    const Coordinate& getCoordinate(std::size_t i) const;
+
+    std::size_t size() const;
+
+    const Coordinate& getComponentPoint() const;
+
     std::size_t getResultSize() const;
 
     TaggedLineSegment* getSegment(std::size_t i);
@@ -114,11 +123,11 @@ class GEOS_DLL TaggedLineString {
 
     std::size_t minimumSize;
 
-    bool preserveEndpoint;
+    bool m_isRing;
 
     void init();
 
-    static std::unique_ptr<geom::CoordinateSequence> extractCoordinates(
+    static std::unique_ptr<CoordinateSequence> extractCoordinates(
         const std::vector<TaggedLineSegment*>& segs);
 
     // Copying is turned off

include/geos/simplify/TaggedLineStringSimplifier.h

@@ -40,15 +40,22 @@ class LineIntersector;
 }
 namespace geom {
 class CoordinateSequence;
+class Coordinate;
 class LineSegment;
 }
 namespace simplify {
 class TaggedLineSegment;
 class TaggedLineString;
 class LineSegmentIndex;
+class ComponentJumpChecker;
 }
 }
 
+using geos::geom::CoordinateSequence;
+using geos::geom::Coordinate;
+using geos::geom::LineSegment;
+
+
 namespace geos {
 namespace simplify { // geos::simplify
 
@@ -64,25 +71,17 @@ class GEOS_DLL TaggedLineStringSimplifier {
 public:
 
     TaggedLineStringSimplifier(LineSegmentIndex* inputIndex,
-                               LineSegmentIndex* outputIndex);
-
-    /** \brief
-     * Sets the distance tolerance for the simplification.
-     *
-     * All vertices in the simplified geometry will be within this
-     * distance of the original geometry.
-     *
-     * @param d the approximation tolerance to use
-     */
-    void setDistanceTolerance(double d);
+                               LineSegmentIndex* outputIndex,
+                               const ComponentJumpChecker* jumpChecker);
 
     /**
      * Simplifies the given {@link TaggedLineString}
      * using the distance tolerance specified.
      *
      * @param line the linestring to simplify
+     * @param distanceTolerance simplification tolerance
      */
-    void simplify(TaggedLineString* line);
+    void simplify(TaggedLineString* line, double distanceTolerance);
 
 
 private:
@@ -93,37 +92,49 @@ class GEOS_DLL TaggedLineStringSimplifier {
     // externally owned
     LineSegmentIndex* outputIndex;
 
+    const ComponentJumpChecker* jumpChecker;
+
     std::unique_ptr<algorithm::LineIntersector> li;
 
     /// non-const as segments are possibly added to it
     TaggedLineString* line;
 
-    const geom::CoordinateSequence* linePts;
-
-    double distanceTolerance;
+    const CoordinateSequence* linePts;
 
-    void simplifySection(std::size_t i, std::size_t j,
-                         std::size_t depth);
+    void simplifySection(std::size_t i, std::size_t j, std::size_t depth, double distanceTolerance);
 
-    void simplifyRingEndpoint();
+    void simplifyRingEndpoint(double distanceTolerance);
 
     static std::size_t findFurthestPoint(
-        const geom::CoordinateSequence* pts,
+        const CoordinateSequence* pts,
         std::size_t i, std::size_t j,
         double& maxDistance);
 
-    bool hasBadIntersection(const TaggedLineString* parentLine,
-                            const size_t excludeStart, const size_t excludeEnd,
-                            const geom::LineSegment& candidateSeg);
+    bool isTopologyValid(
+        const TaggedLineString* lineIn,
+        std::size_t sectionStart, std::size_t sectionEnd,
+        const LineSegment& flatSeg);
+
+    bool isTopologyValid(
+        const TaggedLineString* lineIn,
+        const LineSegment* seg1, const LineSegment* seg2,
+        const LineSegment& flatSeg);
+
+    bool hasInputIntersection(const LineSegment& flatSeg);
 
-    bool hasBadInputIntersection(const TaggedLineString* parentLine,
-                                const size_t excludeStart, const size_t excludeEnd,
-                                const geom::LineSegment& candidateSeg);
+    bool hasInputIntersection(
+        const TaggedLineString* lineIn,
+        std::size_t excludeStart, std::size_t excludeEnd,
+        const LineSegment& flatSeg);
 
-    bool hasBadOutputIntersection(const geom::LineSegment& candidateSeg);
+    bool isCollinear(const Coordinate& pt, const LineSegment& seg) const;
+
+    bool hasOutputIntersection(const LineSegment& flatSeg);
+
+    bool hasInvalidIntersection(
+        const LineSegment& seg0,
+        const LineSegment& seg1) const;
 
-    bool hasInteriorIntersection(const geom::LineSegment& seg0,
-                                 const geom::LineSegment& seg1) const;
 
     std::unique_ptr<TaggedLineSegment> flatten(
         std::size_t start, std::size_t end);
@@ -142,7 +153,7 @@ class GEOS_DLL TaggedLineStringSimplifier {
      */
     static bool isInLineSection(
         const TaggedLineString* line,
-        const size_t excludeStart, const size_t excludeEnd,
+        const std::size_t excludeStart, const std::size_t excludeEnd,
         const TaggedLineSegment* seg);
 
     /** \brief
@@ -158,11 +169,6 @@ class GEOS_DLL TaggedLineStringSimplifier {
 
 };
 
-inline void
-TaggedLineStringSimplifier::setDistanceTolerance(double d)
-{
-    distanceTolerance = d;
-}
 
 } // namespace geos::simplify
 } // namespace geos

include/geos/simplify/TaggedLinesSimplifier.h

@@ -68,44 +68,15 @@ class GEOS_DLL TaggedLinesSimplifier {
      */
     void setDistanceTolerance(double tolerance);
 
-    /** \brief
-     * Simplify a set of {@link TaggedLineString}s
-     *
-     * @tparam iterator_type an iterator, must support assignment, increment,
-     *                       inequality and dereference operators. Dereference
-     *                       operator must return a `TaggedLineString*`.
-     * @param begin iterator to the first element to be simplified.
-     * @param end an iterator to one-past-last element to be simplified.
-     */
-    template <class iterator_type>
-    void
-    simplify(
-        iterator_type begin,
-        iterator_type end)
-    {
-        // add lines to the index
-        for(iterator_type it = begin; it != end; ++it) {
-            assert(*it);
-            inputIndex->add(*(*it));
-        }
-
-        // Simplify lines
-        for(iterator_type it = begin; it != end; ++it) {
-            assert(*it);
-            simplify(*(*it));
-        }
-    }
-
+    void simplify(std::vector<TaggedLineString*>& tlsVector);
 
 private:
 
-    void simplify(TaggedLineString& line);
-
     std::unique_ptr<LineSegmentIndex> inputIndex;
 
     std::unique_ptr<LineSegmentIndex> outputIndex;
 
-    std::unique_ptr<TaggedLineStringSimplifier> taggedlineSimplifier;
+    double distanceTolerance;
 };
 
 } // namespace geos::simplify