Clipping ops #886
Clipping ops #886
Conversation
geo/src/algorithm/bool_ops/mod.rs
Outdated
| @@ -37,6 +38,7 @@ pub trait BooleanOps: Sized { | |||
| fn difference(&self, other: &Self) -> MultiPolygon<Self::Scalar> { | |||
| self.boolean_op(other, OpType::Difference) | |||
| } | |||
| fn clip(&self, ls: &MultiLineString<Self::Scalar>) -> MultiLineString<Self::Scalar>; | |||
There was a problem hiding this comment.
The difference between intersection and clip is that the input/output of intersection are 2D, while the input/output of clip are 1D. Is that right?
I wonder if there's a way to reorganize this trait to handle that more uniformly - maybe related to #881.
I think that this is a good addition, and am happy to have it included in any form, I'm just "thinking out loud" about future directions that will make this more uniform.
| } | ||
| } | ||
| impl<T: GeoFloat> Spec<T> for BoolOp<T> { | ||
| type Region = Region; |
There was a problem hiding this comment.
Maybe I'm missing something, but it seems like we have two implementations of impl Spec, and both have type Region = Region. Rather than an associated type, could we then more simply refer to the concrete Region type? Or am I missing something?
There was a problem hiding this comment.
I set that up for flexibility but didn't end up using it. Looking ahead: the algo. can actually be generalized to handle "cascade union" / union of a vector of polygons. In those cases, the region would be different.
| events: BinaryHeap<Event<C::Scalar, IMSegment<C>>>, | ||
| active_segments: BTreeSet<Active<IMSegment<C>>>, | ||
| } | ||
|
|
||
| impl<C: Cross + Clone> Sweep<C> { | ||
| pub(crate) fn new<I>(iter: I) -> Self | ||
| pub(crate) fn new<I>(iter: I, is_simple: bool) -> Self |
There was a problem hiding this comment.
Is there a definition of what is_simple means in this context?
Thanks @michaelkirk Co-authored-by: Michael Kirk <michael.code@endoftheworl.de>
|
(reminder that this still needs a changelog entry, but otherwise feel free to merge) |
|
Are you waiting on anything else before merging this @rmanoka? If it's helpful, I can provide the changelog entry if that's the only thing we're waiting on. |
|
No major changes; didn't get to wrap it up due to travel. I should get to this by this weekend, but feel free to merge if pushing for a release before then. |
|
bors r+ |
|
Build succeeded: |
CHANGES.mdif knowledge of this change could be valuable to users.I had a use-case for clipping a 1-d geom. by a 2-d goem. It's conceptually a bool ops, but needed some refactoring of the region construction implementation.