CoordinateFilter: Support multiple CoordinateSequence dimensions (#799)

The interface of CoordinateFilter is changed so that a filter should
support operations on CoordinateXY* rather than Coordinate*. By default,
higher-dimension coordinates will default to base-class implementations
(e.g., a CoordinateXYZM will be processed as a CoordinateXY if no
implementation is available for Coordiante or CoordinateXYZM.)

To avoid the case where the filter implementation does not need to
consider Z or M but must preserve them (e.g., repeated point removal),
the CoordinateInspector and CoordinateMutator base classes are provided.
These allow a filter class to inherit from the base using CRTP and
provide a template for the filter method rather than an implementation
for each coordinate type.

include/geos/algorithm/distance/DiscreteHausdorffDistance.h

@@ -152,7 +152,7 @@ class GEOS_DLL DiscreteHausdorffDistance {
         {}
 
         void
-        filter_ro(const geom::Coordinate* pt) override
+        filter_ro(const geom::CoordinateXY* pt) override
         {
             minPtDist.initialize();
             DistanceToPoint::computeDistance(geom, *pt,

include/geos/geom/CoordinateFilter.h

@@ -15,14 +15,13 @@
 #pragma once
 
 #include <geos/export.h>
+#include <geos/geom/Coordinate.h>
 
 #include <cassert>
 
 namespace geos {
 namespace geom { // geos::geom
 
-class Coordinate;
-
 /** \brief
  * Geometry classes support the concept of applying a
  * coordinate filter to every coordinate in the Geometry.
@@ -34,8 +33,11 @@ class Coordinate;
  * used to implement such things as coordinate transformations, centroid and
  * envelope computation, and many other functions.
  *
- * TODO: provide geom::CoordinateInspector and geom::CoordinateMutator instead
- * of having the two versions of filter_rw and filter_ro
+ * A CoordinateFilter should be able to process a CoordinateXY object and can
+ * optionally provide specialized implementations for higher-dimensionality
+ * Coordinates. If the behavior can be templated on coordinate type, then
+ * a filter may inherit from CoordinateInspector or CoordinateMutator to
+ * generate these implementations from a template.
  *
  */
 class GEOS_DLL CoordinateFilter {
@@ -49,7 +51,7 @@ class GEOS_DLL CoordinateFilter {
      * **param** `coord` a Coordinate to which the filter is applied.
      */
     virtual void
-    filter_rw(Coordinate* /*coord*/) const
+    filter_rw(CoordinateXY* /*coord*/) const
     {
         assert(0);
     }
@@ -60,10 +62,66 @@ class GEOS_DLL CoordinateFilter {
      * **param** `coord`  a Coordinate to which the filter is applied.
      */
     virtual void
-    filter_ro(const Coordinate* /*coord*/)
+    filter_ro(const CoordinateXY* /*coord*/)
     {
         assert(0);
     }
+
+    virtual void
+    filter_rw(Coordinate* c) const
+    {
+        filter_rw(static_cast<CoordinateXY*>(c));
+    }
+
+    virtual void
+    filter_ro(const Coordinate* c)
+    {
+        filter_ro(static_cast<const CoordinateXY*>(c));
+    }
+
+    virtual void
+    filter_rw(CoordinateXYM* c) const
+    {
+        filter_rw(static_cast<CoordinateXY*>(c));
+    }
+
+    virtual void
+    filter_ro(const CoordinateXYM* c)
+    {
+        filter_ro(static_cast<const CoordinateXY*>(c));
+    }
+
+    virtual void
+    filter_rw(CoordinateXYZM* c) const
+    {
+        filter_rw(static_cast<Coordinate*>(c));
+    }
+
+    virtual void
+    filter_ro(const CoordinateXYZM* c)
+    {
+        filter_ro(static_cast<const Coordinate*>(c));
+    }
+};
+
+template<class Derived>
+class CoordinateInspector : public CoordinateFilter
+{
+public:
+    virtual void filter_ro(const CoordinateXY* c) override { static_cast<Derived*>(this)->filter(c); }
+    virtual void filter_ro(const Coordinate* c) override { static_cast<Derived*>(this)->filter(c); }
+    virtual void filter_ro(const CoordinateXYM* c) override { static_cast<Derived*>(this)->filter(c); }
+    virtual void filter_ro(const CoordinateXYZM* c) override { static_cast<Derived*>(this)->filter(c); }
+};
+
+template<class Derived>
+class CoordinateMutator : public CoordinateFilter
+{
+public:
+    virtual void filter_rw(CoordinateXY* c) const override { static_cast<const Derived*>(this)->filter(c); }
+    virtual void filter_rw(Coordinate* c) const override { static_cast<const Derived*>(this)->filter(c); }
+    virtual void filter_rw(CoordinateXYM* c) const override { static_cast<const Derived*>(this)->filter(c); }
+    virtual void filter_rw(CoordinateXYZM* c) const override { static_cast<const Derived*>(this)->filter(c); }
 };
 
 } // namespace geos::geom

include/geos/geom/CoordinateSequence.h

@@ -608,8 +608,26 @@ class GEOS_DLL CoordinateSequence {
     /// \defgroup iterate Iteration
     /// @{
 
-    void apply_rw(const CoordinateFilter* filter);
-    void apply_ro(CoordinateFilter* filter) const;
+    template<typename Filter>
+    void apply_rw(const Filter* filter) {
+        switch(getCoordinateType()) {
+            case CoordinateType::XY:    for (auto& c : items<CoordinateXY>())   { filter->filter_rw(&c); } break;
+            case CoordinateType::XYZ:   for (auto& c : items<Coordinate>())     { filter->filter_rw(&c); } break;
+            case CoordinateType::XYM:   for (auto& c : items<CoordinateXYM>())  { filter->filter_rw(&c); } break;
+            case CoordinateType::XYZM:  for (auto& c : items<CoordinateXYZM>()) { filter->filter_rw(&c); } break;
+        }
+        m_hasdim = m_hasz = false; // re-check (see http://trac.osgeo.org/geos/ticket/435)
+    }
+
+    template<typename Filter>
+    void apply_ro(Filter* filter) const {
+        switch(getCoordinateType()) {
+            case CoordinateType::XY:    for (const auto& c : items<CoordinateXY>())   { filter->filter_ro(&c); } break;
+            case CoordinateType::XYZ:   for (const auto& c : items<Coordinate>())     { filter->filter_ro(&c); } break;
+            case CoordinateType::XYM:   for (const auto& c : items<CoordinateXYM>())  { filter->filter_ro(&c); } break;
+            case CoordinateType::XYZM:  for (const auto& c : items<CoordinateXYZM>()) { filter->filter_ro(&c); } break;
+        }
+    }
 
     template<typename T, typename F>
     void forEach(F&& fun) const

include/geos/geom/Quadrant.h

@@ -95,7 +95,7 @@ class GEOS_DLL Quadrant {
      *
      * @throws IllegalArgumentException if the points are equal
      */
-    static int quadrant(const geom::Coordinate& p0, const geom::Coordinate& p1)
+    static int quadrant(const geom::CoordinateXY& p0, const geom::CoordinateXY& p1)
     {
         if(p1.x == p0.x && p1.y == p0.y) {
             throw util::IllegalArgumentException("Cannot compute the quadrant for two identical points " + p0.toString());

include/geos/util/UniqueCoordinateArrayFilter.h

@@ -38,14 +38,14 @@ namespace util { // geos::util
  *
  *  Last port: util/UniqueCoordinateArrayFilter.java rev. 1.17
  */
-class GEOS_DLL UniqueCoordinateArrayFilter: public geom::CoordinateFilter {
+class GEOS_DLL UniqueCoordinateArrayFilter : public geom::CoordinateInspector<UniqueCoordinateArrayFilter> {
 public:
     /**
      * Constructs a CoordinateArrayFilter.
      *
      * @param target The destination set.
      */
-    UniqueCoordinateArrayFilter(geom::Coordinate::ConstVect& target)
+    UniqueCoordinateArrayFilter(std::vector<const geom::Coordinate*>& target)
         : pts(target)
     {}
 
@@ -62,17 +62,27 @@ class GEOS_DLL UniqueCoordinateArrayFilter: public geom::CoordinateFilter {
      * @param coord The "read-only" Coordinate to which
      * 				the filter is applied.
      */
-    void
-    filter_ro(const geom::Coordinate* coord) override
+    template<typename CoordType>
+    void filter(const CoordType* coord)
     {
         if(uniqPts.insert(coord).second) {
+            // TODO make `pts` a CoordinateSequence rather than coercing the type
             pts.push_back(coord);
         }
     }
 
+    void filter(const geom::CoordinateXY*) {
+        assert(0); // not supported
+    }
+
+
+    void filter(const geom::CoordinateXYM*) {
+        assert(0); // not supported
+    }
+
 private:
-    geom::Coordinate::ConstVect& pts;	// target set reference
-    geom::Coordinate::ConstSet uniqPts; 	// unique points set
+    std::vector<const geom::Coordinate*>& pts;	// target set reference
+    std::set<const geom::CoordinateXY*, geom::CoordinateLessThen> uniqPts; 	// unique points set
 
     // Declare type as noncopyable
     UniqueCoordinateArrayFilter(const UniqueCoordinateArrayFilter& other) = delete;

src/geom/CoordinateSequence.cpp

@@ -239,25 +239,6 @@ CoordinateSequence::add(const CoordinateSequence& cl, bool allowRepeated, bool f
 
 /*public*/
 
-void
-CoordinateSequence::apply_rw(const CoordinateFilter* filter)
-{
-    // FIXME dispatch on dimension?
-    for(auto& c : items<Coordinate>()) {
-        filter->filter_rw(&c);
-    }
-    m_hasdim = m_hasz = false; // re-check (see http://trac.osgeo.org/geos/ticket/435)
-}
-
-void
-CoordinateSequence::apply_ro(CoordinateFilter* filter) const
-{
-    // FIXME dispatch on dimension?
-    for(const auto& c : items<Coordinate>()) {
-        filter->filter_ro(&c);
-    };
-}
-
 std::unique_ptr<CoordinateSequence>
 CoordinateSequence::clone() const
 {

src/index/chain/MonotoneChainBuilder.cpp

@@ -57,7 +57,7 @@ class ChainBuilder : public CoordinateFilter {
      m_context(context),
      m_list(list) {}
 
-    void filter_ro(const Coordinate* c) override final {
+    void filter_ro(const CoordinateXY* c) override {
         process(c);
 
         m_prev = c;
@@ -76,7 +76,7 @@ class ChainBuilder : public CoordinateFilter {
         m_start = chainEnd;
     }
 
-    void process(const Coordinate* curr) {
+    void process(const CoordinateXY* curr) {
         if (m_prev == nullptr || curr->equals2D(*m_prev)) {
             return;
         }
@@ -93,7 +93,7 @@ class ChainBuilder : public CoordinateFilter {
         }
     }
 
-    const Coordinate* m_prev;
+    const CoordinateXY* m_prev;
     std::size_t m_i;
     int m_quadrant;
     std::size_t m_start;

src/noding/ScaledNoder.cpp

@@ -89,7 +89,7 @@ class ScaledNoder::Scaler : public geom::CoordinateFilter {
     //void filter_ro(const geom::Coordinate* c) { assert(0); }
 
     void
-    filter_rw(geom::Coordinate* c) const override
+    filter_rw(geom::CoordinateXY* c) const override
     {
         c->x = util::round((c->x - sn.offsetX) * sn.scaleFactor);
         c->y = util::round((c->y - sn.offsetY) * sn.scaleFactor);
@@ -114,14 +114,14 @@ class ScaledNoder::ReScaler: public geom::CoordinateFilter {
     }
 
     void
-    filter_ro(const geom::Coordinate* c) override
+    filter_ro(const geom::CoordinateXY* c) override
     {
         ::geos::ignore_unused_variable_warning(c);
         assert(0);
     }
 
     void
-    filter_rw(geom::Coordinate* c) const override
+    filter_rw(geom::CoordinateXY* c) const override
     {
         c->x = c->x / sn.scaleFactor + sn.offsetX;
         c->y = c->y / sn.scaleFactor + sn.offsetY;

src/operation/valid/RepeatedPointRemover.cpp

@@ -36,20 +36,16 @@ namespace geos {
 namespace operation {
 namespace valid {
 
-class RepeatedPointFilter : public CoordinateFilter {
+class RepeatedPointFilter : public geom::CoordinateInspector<RepeatedPointFilter> {
     public:
 
-        RepeatedPointFilter()
-            : m_coords(detail::make_unique<CoordinateSequence>())
-            , m_prev(nullptr)
-            , sqTolerance(0.0) {}
-
-        RepeatedPointFilter(double tolerance)
-            : m_coords(detail::make_unique<CoordinateSequence>())
+        RepeatedPointFilter(bool has_z, bool has_m, double tolerance = 0.0)
+            : m_coords(detail::make_unique<CoordinateSequence>(0u, has_z, has_m))
             , m_prev(nullptr)
             , sqTolerance(tolerance*tolerance) {}
 
-        void filter_ro(const Coordinate* curr) override final {
+        template<typename CoordType>
+        void filter(const CoordType* curr) {
 
             // skip duplicate point or too-close poinnt
             if (m_prev != nullptr && (
@@ -70,7 +66,7 @@ class RepeatedPointFilter : public CoordinateFilter {
     private:
 
         std::unique_ptr<CoordinateSequence> m_coords;
-        const Coordinate* m_prev;
+        const geom::CoordinateXY* m_prev;
         double sqTolerance;
 };
 
@@ -90,26 +86,22 @@ RepeatedPointRemover::removeRepeatedPoints(const CoordinateSequence* seq, double
         return ret;
     }
 
-    RepeatedPointFilter filter(tolerance);
+    RepeatedPointFilter filter(seq->hasZ(), seq->hasM(), tolerance);
     seq->apply_ro(&filter);
     return filter.getCoords();
 }
 
 
-class RepeatedInvalidPointFilter : public CoordinateFilter {
+class RepeatedInvalidPointFilter : public geom::CoordinateInspector<RepeatedInvalidPointFilter> {
     public:
 
-        RepeatedInvalidPointFilter()
-            : m_coords(detail::make_unique<CoordinateSequence>())
-            , m_prev(nullptr)
-            , sqTolerance(0.0) {}
-
-        RepeatedInvalidPointFilter(double tolerance)
-            : m_coords(detail::make_unique<CoordinateSequence>())
+        RepeatedInvalidPointFilter(bool has_z, bool has_m, double tolerance = 0.0)
+            : m_coords(detail::make_unique<CoordinateSequence>(0u, has_z, has_m))
             , m_prev(nullptr)
             , sqTolerance(tolerance*tolerance) {}
 
-        void filter_ro(const Coordinate* curr) override final {
+        template<typename CoordType>
+        void filter(const CoordType* curr) {
 
             bool invalid = ! curr->isValid();
             // skip initial invalids
@@ -136,7 +128,7 @@ class RepeatedInvalidPointFilter : public CoordinateFilter {
     private:
 
         std::unique_ptr<CoordinateSequence> m_coords;
-        const Coordinate* m_prev;
+        const geom::CoordinateXY* m_prev;
         double sqTolerance;
 };
 
@@ -149,7 +141,7 @@ RepeatedPointRemover::removeRepeatedAndInvalidPoints(const CoordinateSequence* s
         return detail::make_unique<CoordinateSequence>(0u, seq->getDimension());
     }
 
-    RepeatedInvalidPointFilter filter(tolerance);
+    RepeatedInvalidPointFilter filter(seq->hasZ(), seq->hasM(), tolerance);
     seq->apply_ro(&filter);
     return filter.getCoords();
 }
@@ -203,7 +195,7 @@ class RepeatedPointCoordinateOperation : public CoordinateOperation
 
         // Create new vector of coordinates filtered to the
         // the tolerance.
-        RepeatedInvalidPointFilter filter(tolerance);
+        RepeatedInvalidPointFilter filter(coordinates->hasZ(), coordinates->hasM(), tolerance);
         coordinates->apply_ro(&filter);
         auto filtCoords = filter.getCoords();