Fix MVT large geometry processing

If MVT uses extent is larger than 4096, is_area_positive test panics due to an integer overflow. The solution seems to use i64 area computation instead - the parsing seem to work ok.
georust · Aug 8, 2023 · 523fefb · 523fefb
1 parent 9132a59
commit 523fefb
Show file tree

Hide file tree

Showing 2 changed files with 48 additions and 2 deletions.
diff --git a/geozero/src/mvt/mvt_reader.rs b/geozero/src/mvt/mvt_reader.rs
@@ -278,7 +278,7 @@ fn process_polygons<P: GeomProcessor>(
 // using surveyor's formula
 fn is_area_positive(mut cursor: [i32; 2], first: &[u32], rest: &[u32]) -> bool {
     let nb = 1 + rest.len() / 2;
-    let mut area = 0;
+    let mut area = 0_i64;
     let mut coords = first
         .iter()
         .chain(rest)
@@ -290,7 +290,7 @@ fn is_area_positive(mut cursor: [i32; 2], first: &[u32], rest: &[u32]) -> bool {
         let [x0, y0] = cursor;
         cursor[0] += coords.next().unwrap();
         cursor[1] += coords.next().unwrap();
-        area += x0 * cursor[1] - cursor[0] * y0;
+        area += (x0 as i64) * (cursor[1] as i64) - (y0 as i64) * (cursor[0] as i64);
     }
     area > 0
 }
@@ -496,4 +496,30 @@ mod test {
             })
         );
     }
+
+    #[test]
+    fn big_number_geom() {
+        // In some cases, if the extent is large enough, the coordinate parsing threw an error
+        // This geometry is using 65536 extent
+        let mut mvt_feature = tile::Feature::default();
+        mvt_feature.set_type(GeomType::Polygon);
+        mvt_feature.geometry = [
+            9, 69752, 75236, 250, 4342, 2820, 1418, 912, 2046, 1334, 936, 600, 708, 442, 1660,
+            1020, 1158, 686, 1648, 940, 712, 396, 714, 388, 13, 30, 121, 228, 117, 222, 127, 244,
+            23, 42, 13, 28, 1215, 645, 1947, 1069, 1273, 725, 1365, 803, 933, 557, 939, 573, 2595,
+            1617, 1305, 807, 3999, 2493, 16, 25, 80, 125, 120, 189, 142, 217, 138, 219, 136, 213,
+            15,
+        ]
+        .to_vec();
+
+        let geojson = mvt_feature.to_json().unwrap();
+
+        assert_eq!(
+            serde_json::from_str::<serde_json::Value>(&geojson).unwrap(),
+            json!({
+                "type": "Polygon",
+                "coordinates": [[[34876,37618],[37047,39028],[37756,39484],[38779,40151],[39247,40451],[39601,40672],[40431,41182],[41010,41525],[41834,41995],[42190,42193],[42547,42387],[42540,42402],[42479,42516],[42420,42627],[42356,42749],[42344,42770],[42337,42784],[41729,42461],[40755,41926],[40118,41563],[39435,41161],[38968,40882],[38498,40595],[37200,39786],[36547,39382],[34547,38135],[34555,38122],[34595,38059],[34655,37964],[34726,37855],[34795,37745],[34863,37638],[34876,37618]]]
+            })
+        );
+    }
 }
diff --git a/geozero/src/mvt/mvt_writer.rs b/geozero/src/mvt/mvt_writer.rs
@@ -453,6 +453,26 @@ mod test {
         );
     }
 
+    #[test]
+    fn big_number_geom() {
+        let geojson = r#"{
+            "type": "Polygon",
+            "coordinates": [[[34876,37618],[37047,39028],[37756,39484],[38779,40151],[39247,40451],[39601,40672],[40431,41182],[41010,41525],[41834,41995],[42190,42193],[42547,42387],[42540,42402],[42479,42516],[42420,42627],[42356,42749],[42344,42770],[42337,42784],[41729,42461],[40755,41926],[40118,41563],[39435,41161],[38968,40882],[38498,40595],[37200,39786],[36547,39382],[34547,38135],[34555,38122],[34595,38059],[34655,37964],[34726,37855],[34795,37745],[34863,37638],[34876,37618]]]
+        }"#;
+        let geojson = GeoJson(geojson);
+        let mvt = geojson.to_mvt().unwrap();
+        assert_eq!(
+            mvt.geometry,
+            [
+                9, 69752, 75236, 250, 4342, 2820, 1418, 912, 2046, 1334, 936, 600, 708, 442, 1660,
+                1020, 1158, 686, 1648, 940, 712, 396, 714, 388, 13, 30, 121, 228, 117, 222, 127,
+                244, 23, 42, 13, 28, 1215, 645, 1947, 1069, 1273, 725, 1365, 803, 933, 557, 939,
+                573, 2595, 1617, 1305, 807, 3999, 2493, 16, 25, 80, 125, 120, 189, 142, 217, 138,
+                219, 136, 213, 15,
+            ]
+        );
+    }
+
     #[test]
     #[cfg(feature = "with-geo")]
     fn geo_to_mvt() -> Result<()> {