First cut at stroke expansion #286
Conversation
This is a starting point; no dashes, only butt and miter. Also not tested yet. Will close #285
Fix some todo() items. Reverse forward and backward so area is positive. Implement line caps and joins. Make end_point a public method of `PathEl`, as suggested. Dashes remain, as well as closed paths.
Handle stroking of closed subpaths. This generates two closed subpaths as output, one for the inner contour, one for the outer. Also apply a threshold for joins.
This should handle closed paths and phase, but do need to validate.
Add an additional argument to stroke expansion. It's designed so it can be Default::default() in the common case, and uses the builder pattern to add more options as needed without breaking semver.
| @@ -17,6 +17,9 @@ features = ["mint", "schemars", "serde"] | |||
| default = ["std"] | |||
| std = [] | |||
|
|
|||
| [dependencies] | |||
| smallvec = "1.10" | |||
There was a problem hiding this comment.
I think tinyvec would be better if possible since it has no unsafe code.
| } | ||
|
|
||
| /// Collection of values representing lengths in a dash pattern. | ||
| pub type Dashes = SmallVec<[f64; 4]>; |
There was a problem hiding this comment.
It may be better to do three dashes by default, since on 64 bit platforms there would be no wasted stack space compared to a Vec.
|
Looks like it's skipping some segments? For comparison, here's what
|
|
In addition, I'm getting stack overflows in this function under certain dash patterns. Standby for more details |
|
Are you also patching #289 with the stack overflows? We need to get these landed, then consider more improvements on the remaining failures. That integration is now nontrivial, you have to explicitly opt in to the regularize behavior. I'll also add, I appreciate clear actionable reports of any variance of the kurbo method from the accepted behavior - I've done a bit of manual testing but not a careful validation or test suite. (A test suite is also welcome) |
src/stroke.rs
Outdated
| } | ||
|
|
||
| /// Expand a stroke into a fill. | ||
| pub fn stroke(path: impl IntoIterator<Item = PathEl>, style: &Stroke, tolerance: f64) -> BezPath { |
There was a problem hiding this comment.
Did we want to add an additional options parameter to this? I believe one use was to toggle optimal curve fitting.
src/stroke.rs
Outdated
| } | ||
| } | ||
| PathEl::CurveTo(p1, p2, p3) => { | ||
| if p1 != p0 && p2 != p0 && p3 != p0 { |
There was a problem hiding this comment.
Per f2f this appears to be too strict. For example, there are valid cases where p1 == p0. Same above for the quad case.
| } | ||
| PathEl::CurveTo(p1, p2, p3) => { | ||
| if p1 != p0 && p2 != p0 && p3 != p0 { | ||
| let c = CubicBez::new(p0, p1, p2, p3); |
There was a problem hiding this comment.
friendly note: .regularize() :)
Wire up regularization in parallel curve generation (now merged into branch). Fix zero length segment detection (it was overly eager, also triggering on degenerate tangents). In response to review comments.
src/stroke.rs
Outdated
| // while a value too small may fail to generate smooth curves. This is a somewhat | ||
| // arbitrary value, and should be revisited. | ||
| const DIM_TUNE: f64 = 0.25; | ||
| let co = CubicOffset::new_regularized(c, 0.5 * style.width, tolerance * DIM_TUNE); |
There was a problem hiding this comment.
I’m confused as to why is this done here and only for the backward path? In my testing version, I called regularize on the initial cubic itself in stroke_undashed before computing the tangents for joins. My assumption was that regularize fixes zero length tangent vectors and we would want consistency there.
There was a problem hiding this comment.
Simple brain fart error. I should have validated this more carefully before pushing. Thanks for the catch!
This is a starting point; no dashes, only butt and miter, and closed paths are also not yet implemented.
Also not tested yet.
Will close #285