#48 : bounding_rect method for Quadratic and Cubic Bezier Segments #49
Conversation
bezier/src/lib.rs
Outdated
| let min_y = self.from.y.min(self.ctrl.y).min(self.to.y); | ||
| let max_y = self.from.y.max(self.ctrl.y).max(self.to.y); | ||
|
|
||
| return Rect::new(Point::new(min_x, min_y), Size::new(max_x - min_x, max_y - min_y)); |
There was a problem hiding this comment.
nit: you can use euclid::rect::rect; and then this can be written return rect(min_x, min_y, max_x - min_x, max_y - min_y);
Not a huge deal but I find it a bit easier on the eye.
There was a problem hiding this comment.
https://autumnai.github.io/cuticula/euclid/rect/struct.Rect.html I don't find any rect constructor which would allow me to do this, could you please point me to the right spot ?
There was a problem hiding this comment.
It looks like the euclid doc you are looking at is out of date, the one in docs.rs has what we are looking for.
bezier/src/lib.rs
Outdated
| to: Point::new(2.0, 0.0), | ||
| }; | ||
|
|
||
| let expected_bounding_rect = Rect::new(Point::new(0.0, 0.0), Size::new(2.0, 1.0)); |
There was a problem hiding this comment.
nit: same thing with rect(0.0, 0.0, 2.0, 1.0)
bezier/src/lib.rs
Outdated
| pub type Rect = euclid::Rect<f32>; | ||
|
|
||
| /// Allows to get a Rect structure which contains the Segment | ||
| trait HasBoundingRect { |
There was a problem hiding this comment.
The idea of adding a trait is interesting, but I'd prefer to do this separately. There are other common APIs between the different types of segments, and I'd like to brainstorm what the use cases are and how these traits should look like first (should we group them in some Curve trait? or should we have a trait for each group of common method? etc.).
Also, having a method exposed as trait implementations means you need to import the trait to use it which I find inconvenient, so I prefer the approach of implementing all methods on the type itself and then using them in trait impls.
So, could you remove the trait from this PR, I'll open an issue so that we can brainstorm the API and we'll add traits as a followup.
There was a problem hiding this comment.
Thinking of it, I've seen lots of "common methods" among bezier curves, so I'll remove the trait, because this is a big commitment indeed. :)
|
Thanks to your documentation, I've just pushed a modification using euclid::rect::rect instead. Is there an elegant way to import it ? If not I guess we should be fine :) |
|
There isn't a more elegant way until euclid 0.13 gets published, until then that's fine. |
bezier/src/lib.rs
Outdated
| let min_y = self.from.y.min(self.ctrl.y).min(self.to.y); | ||
| let max_y = self.from.y.max(self.ctrl.y).max(self.to.y); | ||
|
|
||
| return euclid::rect::rect(min_x, min_y, max_x - min_x, max_y - min_y); |
There was a problem hiding this comment.
nit: just use euclid::rect::rect at the top of this file and then call rect(....
… for Quadratic and Cubic Bezier Segments. Used an import to ease instanciation of rect.
|
This looks better, with one squashed commit :) |
Added HasBoundingRect trait, and implemented the bounding_rect method for Quadratic and Cubic Bezier Segments.
This is the first part, I'll implement the other required methods for the issue #48 as soon as I find some time to do so :)