Refactor WKTWriter::writeTrimmedNumber for big and small values (#973)

libgeos · Oct 23, 2023 · 0dc1901 · 0dc1901
1 parent 28d70a2
commit 0dc1901
Show file tree

Hide file tree

Showing 5 changed files with 222 additions and 7 deletions.
diff --git a/NEWS.md b/NEWS.md
@@ -16,6 +16,7 @@
   - Fix PreparedLineStringDistance for lines within envelope and polygons (GH-959, Martin Davis)
   - Improve scale handling for PrecisionModel (GH-956, Martin Davis)
   - Fix error in CoordinateSequence::add when disallowing repeated points (GH-963, Dan Baston)
+  - Fix WKTWriter::writeTrimmedNumber for big and small values (GH-973, Mike Taves)
 
 
 ## Changes in 3.12.0

diff --git a/capi/geos_c.h.in b/capi/geos_c.h.in
@@ -1898,11 +1898,13 @@ extern void GEOS_DLL GEOSWKTWriter_setOld3D_r(
     GEOSWKTWriter *writer,
     int useOld3D);
 
-/** Print the shortest representation of a double using fixed notation.
- * Only works for numbers smaller than 1e17 (absolute value).
+/** Print the shortest representation of a double. Non-zero absolute values
+ * that are <1e-4 and >=1e+17 are formatted using scientific notation, and
+ * other values are formatted with positional notation with precision used for
+ * the max digits after decimal point.
  * \param d The number to format.
- * \param precision The desired precision. (max digits after decimal point)
- * \param result The buffer to write the result to.
+ * \param precision The desired precision.
+ * \param result The buffer to write the result to, with a suggested size 28.
  * \return the length of the written string.
  */
 extern int GEOS_DLL GEOS_printDouble(
@@ -5436,7 +5438,10 @@ extern char GEOS_DLL *GEOSWKTWriter_write(
 /**
 * Sets the number trimming option on a \ref GEOSWKTWriter.
 * With trim set to 1, the writer will strip trailing 0's from
-* the output coordinates. With 0, all coordinates will be
+* the output coordinates. With 1 (trimming enabled), big and small
+* absolute coordinates will use scientific notation, otherwise
+* positional notation is used; see \ref GEOS_printDouble for details.
+* With 0 (trimming disabled), all coordinates will be
 * padded with 0's out to the rounding precision.
 * Default since GEOS 3.12 is with trim set to 1 for 'on'.
 * \param writer A \ref GEOSWKTWriter.

diff --git a/src/io/WKTWriter.cpp b/src/io/WKTWriter.cpp
@@ -407,7 +407,21 @@ WKTWriter::appendSequenceText(const CoordinateSequence& seq,
 int 
 WKTWriter::writeTrimmedNumber(double d, uint32_t precision, char* buf)
 {
-    return geos_d2sfixed_buffered_n(d, precision, buf);
+    const auto da = std::fabs(d);
+    if ( !std::isfinite(d) || (da == 0.0) )
+        // non-finite or exactly zero
+        return geos_d2sfixed_buffered_n(d, precision, buf);
+    else if ( (da >= 1e+17) || (da < 1e-4) )
+        // very large or small numbers, use scientific notation
+        return geos_d2sexp_buffered_n(d, precision, buf);
+    else {
+        // most real-world coordinates, use positional notation
+        if ( (precision < 4) && (da < 1.0) ) {
+            // adjust precision to avoid rounding to zero
+            precision = static_cast<std::uint32_t>(-floor(log10(da)));
+        }
+        return geos_d2sfixed_buffered_n(d, precision, buf);
+    }
 }
 
 std::string
@@ -417,7 +431,7 @@ WKTWriter::writeNumber(double d, bool trim, uint32_t precision) {
     * the ryu library.
     */
     if (trim) {
-        char buf[128];
+        char buf[28];
         int len = writeTrimmedNumber(d, precision, buf);
         buf[len] = '\0';
         std::string s(buf);

diff --git a/tests/unit/capi/GEOS_printDoubleTest.cpp b/tests/unit/capi/GEOS_printDoubleTest.cpp
@@ -0,0 +1,84 @@
+#include <tut/tut.hpp>
+// geos
+#include <geos_c.h>
+
+#include "capi_test_utils.h"
+
+namespace tut {
+//
+// Test Group
+//
+
+struct test_geos_printdouble_data : public capitest::utility {};
+
+typedef test_group<test_geos_printdouble_data> group;
+typedef group::object object;
+
+group test_geos_printdouble("capi::GEOS_printDouble");
+
+template<>
+template<>
+void object::test<1>()
+{
+    struct TESTCASE {
+        TESTCASE(unsigned int p_p, double p_d, std::string p_expected)
+            : p(p_p), d(p_d), expected(p_expected) {}
+        unsigned int p;
+        double d;
+        std::string expected;
+    };
+    std::vector<TESTCASE> testcase_l{
+        TESTCASE(1, 0.0, "0"),
+        TESTCASE(1, std::nan("0"), "NaN"),
+        TESTCASE(1, std::numeric_limits<double>::infinity(), "Infinity"),
+        TESTCASE(1, -std::numeric_limits<double>::infinity(), "-Infinity"),
+        TESTCASE(16, 1.0, "1"),
+        TESTCASE(16, 1.2e+234, "1.2e+234"),
+        TESTCASE(2, -1.2e+234, "-1.2e+234"),
+        TESTCASE(16, 1.2e-234, "1.2e-234"),
+        TESTCASE(2, -1.2e-234, "-1.2e-234"),
+        TESTCASE(2, 1.1e-5, "1.1e-5"),
+        TESTCASE(0, 1e-4, "0.0001"),
+        TESTCASE(1, 1e-4, "0.0001"),
+        TESTCASE(2, 1e-4, "0.0001"),
+        TESTCASE(3, 1e-4, "0.0001"),
+        TESTCASE(4, 1e-4, "0.0001"),
+        TESTCASE(5, 1e-4, "0.0001"),
+        TESTCASE(0, 5.6e-4, "0.0006"),
+        TESTCASE(1, 5.6e-4, "0.0006"),
+        TESTCASE(2, 5.6e-4, "0.0006"),
+        TESTCASE(3, 5.6e-4, "0.0006"),
+        TESTCASE(4, 5.6e-4, "0.0006"),
+        TESTCASE(5, 5.6e-4, "0.00056"),
+        TESTCASE(0, 1.2345678901234e+15, "1234567890123400"),
+        TESTCASE(1, 1.2345678901234e+15, "1234567890123400"),
+        TESTCASE(0, 1.2345678901234e+16, "12345678901234000"),
+        TESTCASE(1, 1.2345678901234e+16, "12345678901234000"),
+        TESTCASE(0, 1.2345678901234e+17, "1e+17"),
+        TESTCASE(1, 1.2345678901234e+17, "1.2e+17"),
+        TESTCASE(2, 1.2345678901234e+17, "1.23e+17"),
+        TESTCASE(3, 1.2345678901234e+17, "1.235e+17"),
+        TESTCASE(4, 1.2345678901234e+17, "1.2346e+17"),
+        TESTCASE(5, 1.2345678901234e+17, "1.23457e+17"),
+        TESTCASE(6, 1.2345678901234e+17, "1.234568e+17"),
+        TESTCASE(7, 1.2345678901234e+17, "1.2345679e+17"),
+        TESTCASE(8, 1.2345678901234e+17, "1.23456789e+17"),
+        TESTCASE(9, 1.2345678901234e+17, "1.23456789e+17"),
+        TESTCASE(10, 1.2345678901234e+17, "1.2345678901e+17"),
+        TESTCASE(11, 1.2345678901234e+17, "1.23456789012e+17"),
+        TESTCASE(12, 1.2345678901234e+17, "1.234567890123e+17"),
+        TESTCASE(13, 1.2345678901234e+17, "1.2345678901234e+17"),
+        TESTCASE(14, 1.2345678901234e+17, "1.2345678901234e+17"),
+    };
+    for (const auto& testcase : testcase_l) {
+        char buf[28];
+        const auto len = GEOS_printDouble(testcase.d, testcase.p, buf);
+        buf[len] = '\0';
+        const auto res_str = std::string(buf);
+        ensure_equals(res_str, testcase.expected);
+        ensure_equals(len, static_cast<int>(testcase.expected.size()));
+    }
+
+}
+
+} // namespace tut
diff --git a/tests/unit/io/WKTWriterTest.cpp b/tests/unit/io/WKTWriterTest.cpp
@@ -464,4 +464,115 @@ void object::test<15>
 
 }
 
+// Test big, small, and non-finite values
+// https://github.com/libgeos/geos/issues/970
+template<>
+template<>
+void object::test<16>
+()
+{
+    PrecisionModel pmf(PrecisionModel::FLOATING);
+    GeometryFactory::Ptr gff(GeometryFactory::create(&pmf));
+    WKTReader wktreaderf(gff.get());
+
+    // Big values
+    auto big = wktreaderf.read("POINT (-1.234e+15 1.234e+16 1.234e+17 -1.234e+18)");
+
+    // Check precision from 0 to 5
+    wktwriter.setRoundingPrecision(0);
+    wktwriter.setTrim(true);
+    ensure_equals(wktwriter.write(*big), "POINT ZM (-1234000000000000 12340000000000000 1e+17 -1e+18)");
+    wktwriter.setTrim(false);
+    ensure_equals(wktwriter.write(*big), "POINT ZM (-1234000000000000 12340000000000000 123400000000000000 -1234000000000000000)");
+
+    wktwriter.setRoundingPrecision(1);
+    wktwriter.setTrim(true);
+    ensure_equals(wktwriter.write(*big), "POINT ZM (-1234000000000000 12340000000000000 1.2e+17 -1.2e+18)");
+    wktwriter.setTrim(false);
+    ensure_equals(wktwriter.write(*big), "POINT ZM (-1234000000000000.0 12340000000000000.0 123400000000000000.0 -1234000000000000000.0)");
+
+    wktwriter.setRoundingPrecision(2);
+    wktwriter.setTrim(true);
+    ensure_equals(wktwriter.write(*big), "POINT ZM (-1234000000000000 12340000000000000 1.23e+17 -1.23e+18)");
+    wktwriter.setTrim(false);
+    ensure_equals(wktwriter.write(*big), "POINT ZM (-1234000000000000.00 12340000000000000.00 123400000000000000.00 -1234000000000000000.00)");
+
+    wktwriter.setRoundingPrecision(3);
+    wktwriter.setTrim(true);
+    ensure_equals(wktwriter.write(*big), "POINT ZM (-1234000000000000 12340000000000000 1.234e+17 -1.234e+18)");
+    wktwriter.setTrim(false);
+    ensure_equals(wktwriter.write(*big), "POINT ZM (-1234000000000000.000 12340000000000000.000 123400000000000000.000 -1234000000000000000.000)");
+
+    wktwriter.setRoundingPrecision(4);
+    wktwriter.setTrim(true);
+    ensure_equals(wktwriter.write(*big), "POINT ZM (-1234000000000000 12340000000000000 1.234e+17 -1.234e+18)");
+    wktwriter.setTrim(false);
+    ensure_equals(wktwriter.write(*big), "POINT ZM (-1234000000000000.0000 12340000000000000.0000 123400000000000000.0000 -1234000000000000000.0000)");
+
+    wktwriter.setRoundingPrecision(5);
+    wktwriter.setTrim(true);
+    ensure_equals(wktwriter.write(*big), "POINT ZM (-1234000000000000 12340000000000000 1.234e+17 -1.234e+18)");
+    wktwriter.setTrim(false);
+    ensure_equals(wktwriter.write(*big), "POINT ZM (-1234000000000000.00000 12340000000000000.00000 123400000000000000.00000 -1234000000000000000.00000)");
+
+    // Small values
+    auto small = wktreaderf.read("POINT (-1.234e-3 2.234e-4 1.234e-5 -1.234e-6)");
+
+    // Check precision from 0 to 5
+    wktwriter.setRoundingPrecision(0);
+    wktwriter.setTrim(true);
+    ensure_equals(wktwriter.write(*small), "POINT ZM (-0.001 0.0002 1e-5 -1e-6)");
+    wktwriter.setTrim(false);
+    ensure_equals(wktwriter.write(*small), "POINT ZM (-0 0 0 -0)");
+
+    wktwriter.setRoundingPrecision(1);
+    wktwriter.setTrim(true);
+    ensure_equals(wktwriter.write(*small), "POINT ZM (-0.001 0.0002 1.2e-5 -1.2e-6)");
+    wktwriter.setTrim(false);
+    ensure_equals(wktwriter.write(*small), "POINT ZM (-0.0 0.0 0.0 -0.0)");
+
+    wktwriter.setRoundingPrecision(2);
+    wktwriter.setTrim(true);
+    ensure_equals(wktwriter.write(*small), "POINT ZM (-0.001 0.0002 1.23e-5 -1.23e-6)");
+    wktwriter.setTrim(false);
+    ensure_equals(wktwriter.write(*small), "POINT ZM (-0.00 0.00 0.00 -0.00)");
+
+    wktwriter.setRoundingPrecision(3);
+    wktwriter.setTrim(true);
+    ensure_equals(wktwriter.write(*small), "POINT ZM (-0.001 0.0002 1.234e-5 -1.234e-6)");
+    wktwriter.setTrim(false);
+    ensure_equals(wktwriter.write(*small), "POINT ZM (-0.001 0.000 0.000 -0.000)");
+
+    wktwriter.setRoundingPrecision(4);
+    wktwriter.setTrim(true);
+    ensure_equals(wktwriter.write(*small), "POINT ZM (-0.0012 0.0002 1.234e-5 -1.234e-6)");
+    wktwriter.setTrim(false);
+    ensure_equals(wktwriter.write(*small), "POINT ZM (-0.0012 0.0002 0.0000 -0.0000)");
+
+    wktwriter.setRoundingPrecision(5);
+    wktwriter.setTrim(true);
+    ensure_equals(wktwriter.write(*small), "POINT ZM (-0.00123 0.00022 1.234e-5 -1.234e-6)");
+    wktwriter.setTrim(false);
+    ensure_equals(wktwriter.write(*small), "POINT ZM (-0.00123 0.00022 0.00001 -0.00000)");
+
+    // Extremely small and big
+    auto extreme = wktreaderf.read("POINT (-1.2e-208 9.1e-191 3.8e+221 4.9e+154)");
+    wktwriter.setRoundingPrecision(5);
+    wktwriter.setTrim(true);
+    ensure_equals(wktwriter.write(*extreme), "POINT ZM (-1.2e-208 9.1e-191 3.8e+221 4.9e+154)");
+    // Skip non-trim, as this may vary between compilers
+    // wktwriter.setTrim(false);
+    // ensure_equals(wktwriter.write(*extreme), "POINT ZM (-0.00000 0.00000 ...)");
+
+    // Non-finite values
+    auto nonfinite = wktreaderf.read("POINT(-inf inf nan)");
+
+    wktwriter.setTrim(true);
+    ensure_equals(wktwriter.write(*nonfinite), "POINT Z (-Infinity Infinity NaN)");
+    // Skip non-trim, as this may vary between compilers
+    // wktwriter.setTrim(false);
+    // ensure_equals(wktwriter.write(*nonfinite), "POINT Z (-inf inf nan)");
+
+}
+
 } // namespace tut