search

linebender / kurbo

A Rust library for manipulating curves
Apache License 2.0
716 stars 69 forks source link

Various missing `Mul<Affine>` implementations for types of shape #324

Closed ratmice closed 11 months ago

ratmice commented 11 months ago

Most types that implement Shape, also implement Mul<Affine>. A few outliers are missing. Because of to_path and friends it is still pretty easy to mul all of these. But it seemed worth considering adding the rest for completeness?

the outliers at the moment are Circle, CircleSegment, CubicBez, Rect, RoundedRect.

raphlinus commented 11 months ago

First, Mul<Affine> isn't the correct type, it's Mul<S> for Affine. Circle and CubicBez already have these. The other three mentioned don't, but there's a reason for that - there's no static type that represents the closure under affine. We could invent types (Rect is perhaps the one that makes the most sense, as it's a parallelogram; this could be represented as the affine transformation from the unit square). But for the more complex shapes, it seems to me you're better off manipulating the Bézier path; extra types feels like bloat and YAGNI.

ratmice commented 11 months ago

Sorry about the thinko was/am a bit tired, regarding the last 3, I was thinking you could use type Output=BezPath without introducing a new type (not sure I exactly see why that wouldn't be the case, but nevermind). But indeed it isn't that important, though I tend to think uniform trait coverage is nice, but definitely not worth adding extra types for.

Edit: Ahh, I think what I hadn't considered is that implementing Mul like that would hard code a tolerance value.

ratmice commented 11 months ago

Regarding the YAGNI (I agree with your assesment there), but still felt it might be prudent to show what motivated this bug report, So I posted a repository with the parts of my code base that I can. There is really no need to look at it but it is there if it helps...

https://github.com/ratmice/selvage/blob/main/src/shape_transform.rs