Remove NaN point special case logic (#927)

* Remove NaN point special case logic

NEWS.md

@@ -39,11 +39,13 @@
   - CoverageUnion now requires valid inputs to produce valid outputs
     and may return invalid outputs silently when fed invalid inputs.
     Use CoverageValidator first if you do not know the validity of your data.
+  - WKTReader: Points with all-NaN coordinates are not considered empty anymore (#..., Casper van der Wel)
 
 - Fixes/Improvements:
   - WKTReader: Fix parsing of Z and M flags in WKTReader (#676 and GH-669, Dan Baston)
   - WKTReader: Throw exception on inconsistent geometry dimension (#1080, Dan Baston)
   - WKTReader: Throw exception if WKT contains extra text after end of geometry (#1095, Dan Baston)
+  - WKTReader: Points with all-NaN coordinates are written as such (#..., Casper van der Wel)
   - GEOSIntersects: Fix crash with empty point inputs (#1110, Dan Baston)
   - GEOSIntersects: Improve performance/robustness by using PreparedGeometry algorithm (GH-775, Dan Baston)
   - LineMerger: Recursively collect all components from GeometryCollections (#401, Dan Baston)

capi/geos_ts_c.cpp

@@ -3785,7 +3785,7 @@ extern "C" {
             geos::linearref::LengthIndexedLine lil(g);
             geos::geom::Coordinate coord = lil.extractPoint(d);
             const GeometryFactory* gf = handle->geomFactory;
-            auto point = gf->createPoint(coord);
+            auto point = coord.isNull() ? gf->createPoint(g->getCoordinateDimension()) : gf->createPoint(coord);
             point->setSRID(g->getSRID());
             return point.release();
         });

include/geos/geom/CoordinateSequence.h

@@ -197,20 +197,6 @@ class GEOS_DLL CoordinateSequence {
         return m_vect.empty();
     }
 
-    /// Returns <code>true</code> if there is 1 coordinate and if it is null.
-    bool isNullPoint() const {
-        if (size() != 1) {
-            return false;
-        }
-        switch(getCoordinateType()) {
-            case CoordinateType::XY: return getAt<CoordinateXY>(0).isNull();
-            case CoordinateType::XYZ: return getAt<Coordinate>(0).isNull();
-            case CoordinateType::XYZM: return getAt<CoordinateXYZM>(0).isNull();
-            case CoordinateType::XYM: return getAt<CoordinateXYM>(0).isNull();
-            default: return false;
-        }
-    }
-
     /** \brief
     * Tests whether an a {@link CoordinateSequence} forms a ring,
     * by checking length and closure. Self-intersection is not checked.

src/geom/GeometryFactory.cpp

@@ -226,64 +226,39 @@ GeometryFactory::createPoint(std::unique_ptr<CoordinateSequence>&& coords) const
 {
     if (!coords) {
         return createPoint();
-    } else if ((*coords).isNullPoint()) {
-        return createPoint((*coords).getDimension());
     }
     return std::unique_ptr<Point>(new Point(std::move(*coords), this));
 }
 
 std::unique_ptr<Point>
 GeometryFactory::createPoint(const CoordinateXY& coordinate) const
 {
-    if(coordinate.isNull()) {
-        return createPoint(2);
-    }
-    else {
-        return std::unique_ptr<Point>(new Point(coordinate, this));
-    }
+    return std::unique_ptr<Point>(new Point(coordinate, this));
 }
 
 /*public*/
 std::unique_ptr<Point>
 GeometryFactory::createPoint(const Coordinate& coordinate) const
 {
-    if(coordinate.isNull()) {
-        return createPoint(3);
-    }
-    else {
-        return std::unique_ptr<Point>(new Point(coordinate, this));
-    }
+    return std::unique_ptr<Point>(new Point(coordinate, this));
 }
 
 std::unique_ptr<Point>
 GeometryFactory::createPoint(const CoordinateXYM& coordinate) const
 {
-    if(coordinate.isNull()) {
-        return createPoint(4);  // can't do XYM!
-    }
-    else {
-        return std::unique_ptr<Point>(new Point(coordinate, this));
-    }
+    return std::unique_ptr<Point>(new Point(coordinate, this));
 }
 
 std::unique_ptr<Point>
 GeometryFactory::createPoint(const CoordinateXYZM& coordinate) const
 {
-    if(coordinate.isNull()) {
-        return createPoint(4);
-    }
-    else {
-        return std::unique_ptr<Point>(new Point(coordinate, this));
-    }
+    return std::unique_ptr<Point>(new Point(coordinate, this));
 }
 
 /*public*/
 std::unique_ptr<Point>
 GeometryFactory::createPoint(const CoordinateSequence& fromCoords) const
 {
-    if (fromCoords.isNullPoint()) {
-        return createPoint(fromCoords.getDimension());
-    }
     CoordinateSequence newCoords(fromCoords);
     return std::unique_ptr<Point>(new Point(std::move(newCoords), this));
 

tests/unit/geom/PointTest.cpp

@@ -597,7 +597,7 @@ void object::test<46>
     coords->add(Coordinate(geos::DoubleNotANumber, geos::DoubleNotANumber));
 
     auto point = factory_->createPoint(std::move(coords));
-    ensure("point->isEmpty()", point->isEmpty());
+    ensure("point->isEmpty()", !point->isEmpty());
     ensure("point->getCoordinateDimension() == 2", point->getCoordinateDimension() == 2);
 }
 

tests/unit/io/GeoJSONWriterTest.cpp

@@ -306,5 +306,17 @@ void object::test<19>
     ensure_equals(result, "{\"type\":\"LineString\",\"coordinates\":[[0.0,0.0],[1.0,1.0],[1.0,0.0],[0.0,0.0]]}");
 }
 
+// Write a point with all-nan coordinates
+// https://github.com/libgeos/geos/issues/885
+template<>
+template<>
+void object::test<20>
+()
+{
+    GeomPtr geom(wktreader.read("POINT (NaN NaN)"));
+    std::string result = geojsonwriter.write(geom.get());
+    ensure_equals(result, "{\"type\":\"Point\",\"coordinates\":[null,null]}");
+}
+
 
 }

tests/unit/io/WKTReaderTest.cpp

@@ -447,4 +447,20 @@ void object::test<22>
     ensure_dimension("MULTIPOINT ZM ((0 0 4 3), (1 2 4 5))", true, true);
 }
 
+// accept NaN coordinates
+template<>
+template<>
+void object::test<23>
+()
+{
+    GeomPtr geom(wktreader.read("POINT(NaN NaN)"));
+    auto coords = geom->getCoordinates();
+
+    ensure(!coords->isEmpty());
+    ensure(coords->getDimension() == 2);
+    ensure(std::isnan(coords->getX(0)));
+    ensure(std::isnan(coords->getY(0)));
+}
+
+
 } // namespace tut

tests/unit/io/WKTWriterTest.cpp

@@ -223,7 +223,7 @@ void object::test<7>
     auto point = factory_->createPoint(std::move(coords));
 
     std::string result = wktwriter.write(point.get());
-    ensure_equals(result, std::string("POINT EMPTY"));
+    ensure_equals(result, std::string("POINT (NaN NaN)"));
 }
 
 
@@ -406,4 +406,3 @@ void object::test<14>
 }
 
 } // namespace tut
-