add minimum bounding box func of polygon #959
Conversation
|
Thanks for the excellent contribution @hzw456 . A couple of suggestions: The bounding box in the name is ambiguous as it typically used for axis aligned ones. Shapely calls this functionality as "minimum_rotated_rectangle". Since the approach uses convex hull, would it generalize to all geoms that support conv hull? |
| /// Returns the minimun bounding rect of polygon. | ||
| /// | ||
|
|
||
| pub trait MinimunRotatedRect<'a, T> { |
There was a problem hiding this comment.
It looks like T is unused in the definition. Remove?
There was a problem hiding this comment.
i found ConvexHull function has same usage for T，not sure about its real meaning.
There was a problem hiding this comment.
if i delete T,it will have lifetime error
There was a problem hiding this comment.
Oh lol, let me create a separate issue to track removing it there. Let's leave this as-is for now.
There was a problem hiding this comment.
any process with this?
There was a problem hiding this comment.
I've added a few minor code suggestions, but otherwise lgtm!
To summarize for other reviewers: this PR adds support for the minimum_rotated_rect algorithm for all our geometry types. This is the smallest area rectangle that contains the given geometry. The algorithm involves two steps:
- compute convex hull of geom: since rectangle is convex, any covering rect. should cover the convex hull of the geom.
- for any convex shape, it is known that the min rectangle overlaps with some side of the polygon (citation needed). This implementation iterates over all sides and computes the min. area rectangle. The current implementation constructs rotated polygons, to compute the bounding rect thus giving a quadratic total run-time for this step. This may be optimized in the future via the rotating calipers technique.
@urschrei @frewsxcv @michaelkirk Any one else have time for a quick skim? We could merge it in a week if all looks good.
|
Looks like all reviewer suggestions/comments have been addressed. Will merge it in a day. |
There was a problem hiding this comment.
Please add an entry for this new algorithm in our top level docs: https://docs.rs/geo/latest/geo/#boundary
edit: fixed link🤦
|
@hzw456 One more thing– can you add this to |
|
bors r+ |
|
Build succeeded: |
CHANGES.mdif knowledge of this change could be valuable to users.add minimun bounding box of polygon(mbr)