stop correcting geometries at decode time

Dane Springmeyer · Dane Springmeyer · commit 124362d2d48c · 2015-04-22T14:27:43.000+02:00
- the original motivation for this was to avoid needing to
   mutate a copy later on (for operations needing correct winding order)
 - but mutating a copy is looking feasible, so removing this now.
diff --git a/include/mapnik/json/geometry_util.hpp b/include/mapnik/json/geometry_util.hpp
@@ -25,8 +25,6 @@
 
 #include <mapnik/geometry.hpp>
 #include <mapnik/geometry_adapters.hpp>
-// boost.geometry
-#include <boost/geometry/algorithms/correct.hpp>
 
 namespace mapnik { namespace json {
 
@@ -86,7 +84,9 @@ struct create_polygon
         mapnik::geometry::polygon<double> poly;
         std::size_t num_rings = rings.size();
         if (num_rings > 1)
+        {
             poly.interior_rings.reserve(num_rings - 1);
+        }
 
         for ( std::size_t i = 0; i < num_rings; ++i)
         {
@@ -100,8 +100,6 @@ struct create_polygon
             if (i == 0) poly.set_exterior_ring(std::move(ring));
             else poly.add_hole(std::move(ring));
         }
-        // correct oriantations etc
-        boost::geometry::correct(poly);
         geom_ = std::move(poly);
     }
 
diff --git a/plugins/input/shape/shape_io.cpp b/plugins/input/shape/shape_io.cpp
@@ -27,7 +27,6 @@
 #include <mapnik/make_unique.hpp>
 #include <mapnik/datasource.hpp>
 #include <mapnik/util/is_clockwise.hpp>
-#include <mapnik/geometry_correct.hpp>
 
 using mapnik::datasource_exception;
 const std::string shape_io::SHP = ".shp";
@@ -196,6 +195,5 @@ mapnik::geometry::geometry<double> shape_io::read_polygon(shape_file::record_typ
     } else {
         geom = std::move(poly);
     }
-    mapnik::geometry::correct(geom);
     return geom;
 }
diff --git a/src/wkb.cpp b/src/wkb.cpp
@@ -28,7 +28,6 @@
 #include <mapnik/feature.hpp>
 #include <mapnik/util/noncopyable.hpp>
 #include <mapnik/geometry_adapters.hpp>
-#include <mapnik/geometry_correct.hpp>
 
 namespace mapnik
 {
@@ -411,7 +410,6 @@ mapnik::geometry::geometry<double> geometry_utils::from_wkb(const char* wkb,
 {
     wkb_reader reader(wkb, size, format);
     mapnik::geometry::geometry<double> geom(reader.read());
-    mapnik::geometry::correct(geom);
     return geom;
 }
 
diff --git a/tests/cpp_tests/wkb_formats_test.cpp b/tests/cpp_tests/wkb_formats_test.cpp
@@ -5,6 +5,7 @@
 #include <mapnik/feature.hpp>
 #include <mapnik/geometry_is_valid.hpp>
 #include <mapnik/geometry_is_simple.hpp>
+#include <mapnik/geometry_correct.hpp>
 #include <mapnik/feature_factory.hpp>
 #include <vector>
 #include <algorithm>
@@ -71,15 +72,16 @@ int main(int argc, char** argv)
         mapnik::geometry::geometry<double> geom = mapnik::geometry_utils::from_wkb((const char*)sp_valid_blob,
                                                                                sizeof(sp_valid_blob) / sizeof(sp_valid_blob[0]),
                                                                                mapnik::wkbSpatiaLite);
+        // winding order is not correct per OGC so we'll fix it
+        mapnik::geometry::correct(geom);
         BOOST_TEST(mapnik::geometry::is_valid(geom) && mapnik::geometry::is_simple(geom));
 
         geom = mapnik::geometry_utils::from_wkb((const char*)sp_valid_blob,
                                                 sizeof(sp_valid_blob) / sizeof(sp_valid_blob[0]),
                                                 mapnik::wkbAuto);
+        mapnik::geometry::correct(geom);
         BOOST_TEST(mapnik::geometry::is_valid(geom) && mapnik::geometry::is_simple(geom));
 
-
-
         geom = mapnik::geometry_utils::from_wkb((const char*)sp_invalid_blob,
                                                 sizeof(sp_invalid_blob) / sizeof(sp_invalid_blob[0]),
                                                 mapnik::wkbAuto);
diff --git a/tests/python_tests/feature_test.py b/tests/python_tests/feature_test.py
@@ -38,12 +38,13 @@ def test_python_extended_constructor():
 def test_add_geom_wkb():
 # POLYGON ((30 10, 10 20, 20 40, 40 40, 30 10))
     wkb = '010300000001000000050000000000000000003e4000000000000024400000000000002440000000000000344000000000000034400000000000004440000000000000444000000000000044400000000000003e400000000000002440'
-    context = mapnik.Context()
-    f = mapnik.Feature(context,1)
-    f.geometry = mapnik.Geometry.from_wkb(unhexlify(wkb))
-    eq_(f.geometry.is_valid(), True)
-    eq_(f.geometry.is_simple(), True)
-    eq_(f.geometry.envelope(), f.envelope())
+    geometry = mapnik.Geometry.from_wkb(unhexlify(wkb))
+    eq_(geometry.is_valid(), False) # False because winding order is wrong according to OGC or because end point != first point
+    eq_(geometry.is_simple(), True)
+    eq_(geometry.envelope(), mapnik.Box2d(10.0,10.0,40.0,40.0))
+    geometry.correct()
+    # valid after calling correct
+    eq_(geometry.is_valid(), True)
 
 def test_feature_expression_evaluation():
     context = mapnik.Context()

Original file line number	Diff line number	Diff line change
`@@ -27,7 +27,6 @@`
`27`	`27`	`#include <mapnik/make_unique.hpp>`
`28`	`28`	`#include <mapnik/datasource.hpp>`
`29`	`29`	`#include <mapnik/util/is_clockwise.hpp>`
`30`		`-#include <mapnik/geometry_correct.hpp>`
`31`	`30`
`32`	`31`	`using mapnik::datasource_exception;`
`33`	`32`	`const std::string shape_io::SHP = ".shp";`
`@@ -196,6 +195,5 @@ mapnik::geometry::geometry<double> shape_io::read_polygon(shape_file::record_typ`
`196`	`195`	`} else {`
`197`	`196`	`geom = std::move(poly);`
`198`	`197`	`}`
`199`		`- mapnik::geometry::correct(geom);`
`200`	`198`	`return geom;`
`201`	`199`	`}`
Original file line number	Diff line number	Diff line change
`@@ -28,7 +28,6 @@`
`28`	`28`	`#include <mapnik/feature.hpp>`
`29`	`29`	`#include <mapnik/util/noncopyable.hpp>`
`30`	`30`	`#include <mapnik/geometry_adapters.hpp>`
`31`		`-#include <mapnik/geometry_correct.hpp>`
`32`	`31`
`33`	`32`	`namespace mapnik`
`34`	`33`	`{`
`@@ -411,7 +410,6 @@ mapnik::geometry::geometry<double> geometry_utils::from_wkb(const char* wkb,`
`411`	`410`	`{`
`412`	`411`	`wkb_reader reader(wkb, size, format);`
`413`	`412`	`mapnik::geometry::geometry<double> geom(reader.read());`
`414`		`- mapnik::geometry::correct(geom);`
`415`	`413`	`return geom;`
`416`	`414`	`}`
`417`	`415`