Skip to content

Commit e93d54a

Browse files
committed
Altered input type, updated tests, altered CoordinateLayout terms.
1 parent 545078d commit e93d54a

File tree

3 files changed

+79
-77
lines changed

3 files changed

+79
-77
lines changed

src/vector/geometry.rs

Lines changed: 77 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ use crate::{errors::*, utils::_last_cpl_err};
1616

1717
#[derive(Debug, Clone, Copy)]
1818
#[repr(u8)]
19-
pub enum CoordinateComponent {
19+
pub enum CoordinateDim {
2020
X = 0b00001,
2121
Y = 0b00010,
2222
Z = 0b00100,
@@ -49,21 +49,22 @@ pub enum CoordinateLayout {
4949
}
5050

5151
impl CoordinateLayout {
52-
const COMPONENTS: [CoordinateComponent; 4] = [
53-
CoordinateComponent::X,
54-
CoordinateComponent::Y,
55-
CoordinateComponent::Z,
56-
CoordinateComponent::M,
52+
const COMPONENTS: [CoordinateDim; 4] = [
53+
CoordinateDim::X,
54+
CoordinateDim::Y,
55+
CoordinateDim::Z,
56+
CoordinateDim::M,
5757
];
5858

59-
pub fn has_component(&self, layout: CoordinateComponent) -> bool {
59+
pub fn has_dim(&self, layout: CoordinateDim) -> bool {
6060
(*self as u8 & layout as u8) != 0
6161
}
6262

63-
pub fn coordinate_size(&self) -> usize {
63+
/// Check the number of dimensions for the [`CoordinateLayout`].
64+
pub fn num_dims(&self) -> usize {
6465
Self::COMPONENTS
6566
.into_iter()
66-
.filter(|c| self.has_component(*c))
67+
.filter(|c| self.has_dim(*c))
6768
.count()
6869
}
6970
}
@@ -280,7 +281,7 @@ impl Geometry {
280281
///
281282
/// One-dimensional data is unsupported.
282283
pub fn set_points(&mut self, in_points: &[f64], layout: CoordinateLayout) -> Result<()> {
283-
let coord_size = layout.coordinate_size();
284+
let coord_size = layout.num_dims();
284285

285286
if in_points.len() % coord_size != 0 {
286287
return Err(
@@ -296,17 +297,16 @@ impl Geometry {
296297

297298
let component_size = size_of::<f64>();
298299

299-
let (stride, ptr_offset): (c_int, isize) =
300-
match layout.has_component(CoordinateComponent::AoS) {
301-
true => (
302-
(component_size * coord_size) as c_int,
303-
component_size as isize,
304-
),
305-
false => (
306-
component_size as c_int,
307-
(num_points * component_size) as isize,
308-
),
309-
};
300+
let (stride, ptr_offset): (c_int, isize) = match layout.has_dim(CoordinateDim::AoS) {
301+
true => (
302+
(component_size * coord_size) as c_int,
303+
component_size as isize,
304+
),
305+
false => (
306+
component_size as c_int,
307+
(num_points * component_size) as isize,
308+
),
309+
};
310310

311311
unsafe {
312312
gdal_sys::CPLErrorReset();
@@ -316,10 +316,10 @@ impl Geometry {
316316
let mut curr_ptr_offset: isize = 0;
317317

318318
let component_loc = |layout: &CoordinateLayout,
319-
component: CoordinateComponent,
319+
component: CoordinateDim,
320320
curr_ptr_offset: &mut isize|
321321
-> *mut c_void {
322-
match layout.has_component(component) {
322+
match layout.has_dim(component) {
323323
true => {
324324
let ret = data.byte_offset(*curr_ptr_offset);
325325
*curr_ptr_offset += ptr_offset;
@@ -329,10 +329,10 @@ impl Geometry {
329329
}
330330
};
331331

332-
let x_ptr = component_loc(&layout, CoordinateComponent::X, &mut curr_ptr_offset);
333-
let y_ptr = component_loc(&layout, CoordinateComponent::Y, &mut curr_ptr_offset);
334-
let z_ptr = component_loc(&layout, CoordinateComponent::Z, &mut curr_ptr_offset);
335-
let m_ptr = component_loc(&layout, CoordinateComponent::M, &mut curr_ptr_offset);
332+
let x_ptr = component_loc(&layout, CoordinateDim::X, &mut curr_ptr_offset);
333+
let y_ptr = component_loc(&layout, CoordinateDim::Y, &mut curr_ptr_offset);
334+
let z_ptr = component_loc(&layout, CoordinateDim::Z, &mut curr_ptr_offset);
335+
let m_ptr = component_loc(&layout, CoordinateDim::M, &mut curr_ptr_offset);
336336

337337
// Should be OK to just use the offset even for unused components...
338338
gdal_sys::OGR_G_SetPointsZM(
@@ -360,32 +360,40 @@ impl Geometry {
360360

361361
/// Writes all points in the geometry to `out_points` according to the specified `layout`.
362362
///
363-
/// For some geometry types, like polygons, that don't consist of points, Err will be returned
364-
/// and `out_points` will only be resized.
365-
pub fn get_points(&self, out_points: &mut Vec<f64>, layout: CoordinateLayout) -> Result<usize> {
363+
/// `out_points` must be at least the same length as [`Geometry::point_count`] *
364+
/// [`CoordinateLayout::num_dims`], or Err will be returned.
365+
///
366+
/// For some geometry types, like polygons, that don't consist of points, Err will be returned.
367+
pub fn get_points(&self, out_points: &mut [f64], layout: CoordinateLayout) -> Result<usize> {
366368
let num_points = unsafe { gdal_sys::OGR_G_GetPointCount(self.c_geometry()) } as usize;
367369

368370
// Number of dims
369-
let coord_size = layout.coordinate_size();
371+
let coord_size = layout.num_dims();
370372

371-
// Total length of the output.
372-
let length = num_points * coord_size;
373-
374-
out_points.resize(length, Default::default());
373+
let out_points_len = out_points.len();
374+
if out_points_len < coord_size * num_points {
375+
return Err (
376+
GdalError::InvalidDataInput {
377+
msg: Some(format!(
378+
"out_points length of {out_points_len} is too small for a point count of {num_points} and a CoordinateLayout with {coord_size} components."
379+
)),
380+
method_name: "get_points"
381+
}
382+
);
383+
}
375384

376385
let component_size = size_of::<f64>();
377386

378-
let (stride, ptr_offset): (c_int, isize) =
379-
match layout.has_component(CoordinateComponent::AoS) {
380-
true => (
381-
(component_size * coord_size) as c_int,
382-
component_size as isize,
383-
),
384-
false => (
385-
component_size as c_int,
386-
(num_points * component_size) as isize,
387-
),
388-
};
387+
let (stride, ptr_offset): (c_int, isize) = match layout.has_dim(CoordinateDim::AoS) {
388+
true => (
389+
(component_size * coord_size) as c_int,
390+
component_size as isize,
391+
),
392+
false => (
393+
component_size as c_int,
394+
(num_points * component_size) as isize,
395+
),
396+
};
389397

390398
unsafe {
391399
gdal_sys::CPLErrorReset();
@@ -396,10 +404,10 @@ impl Geometry {
396404
let mut curr_ptr_offset: isize = 0;
397405

398406
let component_loc = |layout: &CoordinateLayout,
399-
component: CoordinateComponent,
407+
component: CoordinateDim,
400408
curr_ptr_offset: &mut isize|
401409
-> *mut c_void {
402-
match layout.has_component(component) {
410+
match layout.has_dim(component) {
403411
true => {
404412
let ret = data.byte_offset(*curr_ptr_offset);
405413
*curr_ptr_offset += ptr_offset;
@@ -409,10 +417,10 @@ impl Geometry {
409417
}
410418
};
411419

412-
let x_ptr = component_loc(&layout, CoordinateComponent::X, &mut curr_ptr_offset);
413-
let y_ptr = component_loc(&layout, CoordinateComponent::Y, &mut curr_ptr_offset);
414-
let z_ptr = component_loc(&layout, CoordinateComponent::Z, &mut curr_ptr_offset);
415-
let m_ptr = component_loc(&layout, CoordinateComponent::M, &mut curr_ptr_offset);
420+
let x_ptr = component_loc(&layout, CoordinateDim::X, &mut curr_ptr_offset);
421+
let y_ptr = component_loc(&layout, CoordinateDim::Y, &mut curr_ptr_offset);
422+
let z_ptr = component_loc(&layout, CoordinateDim::Z, &mut curr_ptr_offset);
423+
let m_ptr = component_loc(&layout, CoordinateDim::M, &mut curr_ptr_offset);
416424

417425
// Should be OK to just use the offset even for unused components...
418426
//
@@ -715,20 +723,20 @@ mod tests {
715723

716724
#[test]
717725
fn test_coordinate_layout() {
718-
assert!(CoordinateLayout::XyzXyz.has_component(CoordinateComponent::X));
719-
assert!(CoordinateLayout::XyzXyz.has_component(CoordinateComponent::Y));
720-
assert!(CoordinateLayout::XyzXyz.has_component(CoordinateComponent::Z));
721-
assert!(!CoordinateLayout::XyzXyz.has_component(CoordinateComponent::M));
722-
assert!(CoordinateLayout::XyzXyz.has_component(CoordinateComponent::AoS));
726+
assert!(CoordinateLayout::XyzXyz.has_dim(CoordinateDim::X));
727+
assert!(CoordinateLayout::XyzXyz.has_dim(CoordinateDim::Y));
728+
assert!(CoordinateLayout::XyzXyz.has_dim(CoordinateDim::Z));
729+
assert!(!CoordinateLayout::XyzXyz.has_dim(CoordinateDim::M));
730+
assert!(CoordinateLayout::XyzXyz.has_dim(CoordinateDim::AoS));
723731

724-
assert!(!CoordinateLayout::XxYy.has_component(CoordinateComponent::AoS));
732+
assert!(!CoordinateLayout::XxYy.has_dim(CoordinateDim::AoS));
725733

726-
assert!(!CoordinateLayout::XymXym.has_component(CoordinateComponent::Z));
727-
assert!(CoordinateLayout::XymXym.has_component(CoordinateComponent::M));
734+
assert!(!CoordinateLayout::XymXym.has_dim(CoordinateDim::Z));
735+
assert!(CoordinateLayout::XymXym.has_dim(CoordinateDim::M));
728736

729-
assert_eq!(CoordinateLayout::XyXy.coordinate_size(), 2);
730-
assert_eq!(CoordinateLayout::XxYyZz.coordinate_size(), 3);
731-
assert_eq!(CoordinateLayout::XyzmXyzm.coordinate_size(), 4);
737+
assert_eq!(CoordinateLayout::XyXy.num_dims(), 2);
738+
assert_eq!(CoordinateLayout::XxYyZz.num_dims(), 3);
739+
assert_eq!(CoordinateLayout::XyzmXyzm.num_dims(), 4);
732740
}
733741

734742
#[test]
@@ -869,18 +877,17 @@ mod tests {
869877
ring.add_point_2d((1218405.0658121984, 721108.1805541387));
870878
ring.add_point_2d((1179091.1646903288, 712782.8838459781));
871879
assert!(!ring.is_empty());
872-
let mut ring_vec: Vec<f64> = Vec::new();
880+
let mut ring_vec: Vec<f64> = vec![0.0; 6 * 2];
873881
ring.get_points(&mut ring_vec, CoordinateLayout::XyXy)
874882
.unwrap();
875-
assert_eq!(ring_vec.len(), 6 * 2);
876883
let mut poly = Geometry::empty(wkbPolygon).unwrap();
877884
poly.add_geometry(ring.to_owned()).unwrap();
878885
let mut poly_vec: Vec<f64> = Vec::new();
879886
let res = poly.get_points(&mut poly_vec, CoordinateLayout::XyXy);
880887
assert!(res.is_err());
881888
assert_eq!(poly.geometry_count(), 1);
882889
let ring_out = poly.get_geometry(0);
883-
let mut ring_out_vec: Vec<f64> = Vec::new();
890+
let mut ring_out_vec: Vec<f64> = vec![0.0; 6 * 2];
884891
ring_out
885892
.get_points(&mut ring_out_vec, CoordinateLayout::XyXy)
886893
.unwrap();
@@ -899,7 +906,7 @@ mod tests {
899906
assert_eq!(geom.geometry_type(), OGRwkbGeometryType::wkbPolygon);
900907
assert!(geom.json().unwrap().contains("Polygon"));
901908
let inner = geom.get_geometry(0);
902-
let mut points: Vec<f64> = Vec::new();
909+
let mut points: Vec<f64> = vec![0.0; inner.point_count() * 2];
903910
inner
904911
.get_points(&mut points, CoordinateLayout::XyXy)
905912
.unwrap();
@@ -968,53 +975,48 @@ mod tests {
968975
line.add_point_zm((0.0, 0.0, 0.0, 0.0));
969976
line.add_point_zm((1.0, 0.0, 0.25, 0.5));
970977
line.add_point_zm((1.0, 2.0, 0.5, 1.0));
971-
let mut line_points: Vec<f64> = Vec::new();
978+
let mut line_points: Vec<f64> = vec![0.0; 3 * 2];
972979

973980
line.get_points(&mut line_points, CoordinateLayout::XyXy)
974981
.unwrap();
975-
assert_eq!(line_points.len(), 3 * 2);
976982
assert_eq!(line_points, vec![0.0, 0.0, 1.0, 0.0, 1.0, 2.0]);
977983

978984
line.get_points(&mut line_points, CoordinateLayout::XxYy)
979985
.unwrap();
980-
assert_eq!(line_points.len(), 3 * 2);
981986
assert_eq!(line_points, vec![0.0, 1.0, 1.0, 0.0, 0.0, 2.0]);
982987

988+
line_points.resize(3 * 3, 0.0);
983989
line.get_points(&mut line_points, CoordinateLayout::XyzXyz)
984990
.unwrap();
985-
assert_eq!(line_points.len(), 3 * 3);
986991
assert_eq!(
987992
line_points,
988993
vec![0.0, 0.0, 0.0, 1.0, 0.0, 0.25, 1.0, 2.0, 0.5]
989994
);
990995

991996
line.get_points(&mut line_points, CoordinateLayout::XxYyZz)
992997
.unwrap();
993-
assert_eq!(line_points.len(), 3 * 3);
994998
assert_eq!(
995999
line_points,
9961000
vec![0.0, 1.0, 1.0, 0.0, 0.0, 2.0, 0.0, 0.25, 0.5]
9971001
);
9981002

9991003
line.get_points(&mut line_points, CoordinateLayout::XymXym)
10001004
.unwrap();
1001-
assert_eq!(line_points.len(), 3 * 3);
10021005
assert_eq!(
10031006
line_points,
10041007
vec![0.0, 0.0, 0.0, 1.0, 0.0, 0.5, 1.0, 2.0, 1.0]
10051008
);
10061009

1010+
line_points.resize(3 * 4, 0.0);
10071011
line.get_points(&mut line_points, CoordinateLayout::XyzmXyzm)
10081012
.unwrap();
1009-
assert_eq!(line_points.len(), 3 * 4);
10101013
assert_eq!(
10111014
line_points,
10121015
vec![0.0, 0.0, 0.0, 0.0, 1.0, 0.0, 0.25, 0.5, 1.0, 2.0, 0.5, 1.0]
10131016
);
10141017

10151018
line.get_points(&mut line_points, CoordinateLayout::XxYyZzMm)
10161019
.unwrap();
1017-
assert_eq!(line_points.len(), 3 * 4);
10181020
assert_eq!(
10191021
line_points,
10201022
vec![0.0, 1.0, 1.0, 0.0, 0.0, 2.0, 0.0, 0.25, 0.5, 0.0, 0.5, 1.0]

src/vector/layer.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1271,7 +1271,7 @@ mod tests {
12711271
with_feature("roads.geojson", 236194095, |feature| {
12721272
let geom = feature.geometry().unwrap();
12731273
assert_eq!(geom.geometry_type(), OGRwkbGeometryType::wkbLineString);
1274-
let mut coords: Vec<f64> = Vec::new();
1274+
let mut coords: Vec<f64> = vec![0.0; geom.point_count() * 2];
12751275
geom.get_points(&mut coords, CoordinateLayout::XyXy)
12761276
.unwrap();
12771277
assert_eq!(

src/vector/ops/conversions/gdal_to_geo.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ impl TryFrom<&Geometry> for geo_types::Geometry<f64> {
4545
)))
4646
}
4747
OGRwkbGeometryType::wkbLineString => {
48-
let mut gdal_coords: Vec<f64> = Vec::with_capacity(geo.point_count() * 2);
48+
let mut gdal_coords: Vec<f64> = vec![0.0; geo.point_count() * 2];
4949
geo.get_points(&mut gdal_coords, CoordinateLayout::XyXy)?;
5050
let coords = gdal_coords
5151
.chunks(2)

0 commit comments

Comments
 (0)