Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid atan2 in minimum_rotated_rect #1094

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions geo/CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@
shouldn't break for any common numeric types, but if you are using something
exotic you'll need to manually implement `GeoNum` for your numeric type.
* <https://github.com/georust/geo/pull/1134>
* POSSIBLY BREAKING: `minimum_rotated_rect` is about 33%, but might return
slightly different results.
* <https://github.com/georust/geo/pull/1094>
* POSSIBLY BREAKING: `SimplifyVwPreserve` trait implementation moved from
`geo_types::CoordNum` to `geo::GeoNum` as a consequence of introducing the
`GeoNum::total_cmp`. This shouldn't break anything for common numeric
Expand Down
4 changes: 4 additions & 0 deletions geo/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -114,3 +114,7 @@ harness = false
[[bench]]
name = "triangulate"
harness = false

[[bench]]
name = "minimum_rotated_rect"
harness = false
118 changes: 83 additions & 35 deletions geo/src/algorithm/minimum_rotated_rect.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
use num_traits::Float;
use geo_types::LineString;
use num_traits::Bounded;

use crate::{
algorithm::{centroid::Centroid, rotate::Rotate, BoundingRect, CoordsIter},
Area, ConvexHull, CoordFloat, GeoFloat, GeoNum, LinesIter, Polygon,
};
use crate::{algorithm::CoordsIter, ConvexHull, CoordFloat, GeoFloat, GeoNum, Polygon};
/// Return the minimum bounding rectangle(MBR) of geometry
/// reference: <https://en.wikipedia.org/wiki/Minimum_bounding_box>
/// minimum rotated rect is the rectangle that can enclose all points given
Expand All @@ -15,16 +13,16 @@ use crate::{
/// ```
/// use geo_types::{line_string, polygon, LineString, Polygon};
/// use geo::MinimumRotatedRect;
/// let poly: Polygon<f64> = polygon![(x: 3.3, y: 30.4), (x: 1.7, y: 24.6), (x: 13.4, y: 25.1), (x: 14.4, y: 31.0),(x:3.3,y:30.4)];
/// let poly: Polygon<f64> = polygon![(x: 3.3, y: 30.4), (x: 1.7, y: 24.6), (x: 13.4, y: 25.1), (x: 14.4, y: 31.0), (x:3.3, y:30.4)];
/// let mbr = MinimumRotatedRect::minimum_rotated_rect(&poly).unwrap();
/// assert_eq!(
/// mbr.exterior(),
/// &LineString::from(vec![
/// (1.7000000000000006, 24.6),
/// (1.4501458363715918, 30.446587428904767),
/// (14.4, 31.0),
/// (14.649854163628408, 25.153412571095235),
/// (1.7000000000000006, 24.6),
/// (1.7000000000000004, 24.600000000000005),
/// (14.64985416362841, 25.153412571095235),
/// (14.400000000000002, 31.000000000000004),
/// (1.4501458363715916, 30.446587428904774),
/// (1.7000000000000004, 24.600000000000005),
/// ])
/// );
/// ```
Expand All @@ -41,24 +39,74 @@ where
type Scalar = T;

fn minimum_rotated_rect(&self) -> Option<Polygon<Self::Scalar>> {
let convex_poly = ConvexHull::convex_hull(self);
let mut min_area: T = Float::max_value();
let mut min_angle: T = T::zero();
let mut rect_poly: Option<Polygon<T>> = None;
let rotate_point = convex_poly.centroid();
for line in convex_poly.exterior().lines_iter() {
let (ci, cii) = line.points();
let angle = (cii.y() - ci.y()).atan2(cii.x() - ci.x()).to_degrees();
let rotated_poly = Rotate::rotate_around_point(&convex_poly, -angle, rotate_point?);
let tmp_poly = rotated_poly.bounding_rect()?.to_polygon();
let area = tmp_poly.unsigned_area();
let hull = ConvexHull::convex_hull(self);

// We take unit vectors along and perpendicular to each edge, then use
// them to build a rotation matrix and apply it to the convex hull,
// keeping track of the best AABB found.
//
// See also the discussion at
// https://gis.stackexchange.com/questions/22895/finding-minimum-area-rectangle-for-given-points/22934
let mut min_area = <T as Bounded>::max_value();
let mut best_edge = None;
let (mut best_min_x, mut best_max_x) = (T::zero(), T::zero());
let (mut best_min_y, mut best_max_y) = (T::zero(), T::zero());
for edge in hull.exterior().lines() {
let (dx, dy) = edge.delta().x_y();
let norm = dx.hypot(dy);
if norm.is_zero() {
continue;
}
let edge = (dx / norm, dy / norm);

let (mut min_x, mut max_x) = (<T as Bounded>::max_value(), <T as Bounded>::min_value());
let (mut min_y, mut max_y) = (<T as Bounded>::max_value(), <T as Bounded>::min_value());
for point in hull.exterior().points() {
let x = point.x() * edge.0 + point.y() * edge.1;
let y = point.x() * -edge.1 + point.y() * edge.0;

min_x = min_x.min(x);
max_x = max_x.max(x);
min_y = min_y.min(y);
max_y = max_y.max(y);
}

let area = (max_y - min_y) * (max_x - min_x);
if area < min_area {
min_area = area;
min_angle = angle;
rect_poly = Some(tmp_poly);
best_edge = Some(edge);
best_min_x = min_x;
best_max_x = max_x;
best_min_y = min_y;
best_max_y = max_y;
}
}
Some(rect_poly?.rotate_around_point(min_angle, rotate_point?))

if let Some((dx, dy)) = best_edge {
let p1 = (
best_min_x * dx + -best_min_y * dy,
best_min_x * dy + best_min_y * dx,
);
let p2 = (
best_max_x * dx + -best_min_y * dy,
best_max_x * dy + best_min_y * dx,
);
let p3 = (
best_max_x * dx + -best_max_y * dy,
best_max_x * dy + best_max_y * dx,
);
let p4 = (
best_min_x * dx + -best_max_y * dy,
best_min_x * dy + best_max_y * dx,
);
let rectangle = Polygon::new(
LineString(vec![p1.into(), p2.into(), p3.into(), p4.into(), p1.into()]),
vec![],
);
Some(rectangle)
} else {
None
}
}
}

Expand All @@ -75,11 +123,11 @@ mod test {
assert_eq!(
mbr.exterior(),
&LineString::from(vec![
(1.7000000000000006, 24.6),
(1.4501458363715918, 30.446587428904767),
(14.4, 31.0),
(14.649854163628408, 25.153412571095235),
(1.7000000000000006, 24.6),
(1.7000000000000004, 24.600000000000005),
(14.64985416362841, 25.153412571095235),
(14.400000000000002, 31.000000000000004),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe you noticed this but it looks like the y value for the 2nd and 4th point have been swapped. 😕

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The points have been swapped. I think the vertices are in the reverse order.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh right, I stared at it too long and confused myself. 😄

Is the new ordering correct or is that something that needs to be fixed yet?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the new ordering correct or is that something that needs to be fixed yet?

It's the same rectangle in the opposite winding order (P4 P3 P2 P1 P4 instead of P1 P2 P3 P4 P1). I can probably match the order, but only up to the starting vertex, e.g. P2 P3 P4 P1 P2.

(1.4501458363715916, 30.446587428904774),
(1.7000000000000004, 24.600000000000005),
])
);
}
Expand All @@ -90,11 +138,11 @@ mod test {
assert_eq!(
mbr.exterior(),
&LineString::from(vec![
(1.7000000000000006, 24.6),
(1.4501458363715918, 30.446587428904767),
(14.4, 31.0),
(14.649854163628408, 25.153412571095235),
(1.7000000000000006, 24.6),
(1.7000000000000004, 24.600000000000005),
(14.64985416362841, 25.153412571095235),
(14.400000000000002, 31.000000000000004),
(1.4501458363715916, 30.446587428904774),
(1.7000000000000004, 24.600000000000005),
])
);
}
Expand Down
Loading