First cut at least-squares offset curves by raphlinus · Pull Request #427 · linebender/kurbo

raphlinus · 2025-03-19T01:51:12Z

Replace the quartic solver with the new least-squares one.

The robustness story is not complete. For now, it relies on the existing "regularize" hack, but it would be better to be robust by construction.

The stroke options are not respected. It tries to do a better job with subdivision but does not attempt an absolutely minimal number of segments as before. That could be re-added.

The old offset methods are still exported. They should probably be deprecated.

Progress toward #317

Replace the quartic solver with the new least-squares one. The robustness story is not complete. For now, it relies on the existing "regularize" hack, but it would be better to be robust by construction. The stroke options are not respected. It tries to do a better job with subdivision but does not attempt an absolutely minimal number of segments as before. That could be re-added. The old offset methods are still exported. They should probably be deprecated. Progress toward #317

tomcur

Some initial observations inline based on a first dive into this.

tomcur · 2025-04-03T14:36:14Z

+    tolerance: f64,
+}
+
+// We never let cusp values haven an absolute value smaller than


Suggested change

// We never let cusp values haven an absolute value smaller than

// We never let cusp values have an absolute value smaller than

tomcur · 2025-04-06T10:27:33Z

+            let a_n = 3.0 * mt * t * mt * rec.utan0.dot(n);
+            let a_t = 3.0 * mt * t * mt * rec.utan0.cross(n);
+            let b_n = 3.0 * mt * t * t * rec.utan1.dot(n);
+            let b_t = 3.0 * mt * t * t * rec.utan1.cross(n);


On x86 it may be very slightly faster to group the squarings here: https://godbolt.org/z/b1xa96Kff

let a_n = 3.0 * (mt * mt) * t * rec.utan0.dot(n);

tomcur · 2025-04-08T08:44:08Z

 pub fn stroke(
    path: impl IntoIterator<Item = PathEl>,
    style: &Stroke,
-    opts: &StrokeOpts,
+    _opts: &StrokeOpts,
    tolerance: f64,
 ) -> BezPath {


As an API change (not for this PR), we could consider taking &mut BezPath here, allowing reusing the allocation.

tomcur · 2025-04-09T13:40:55Z

+    }
+
+    fn apply(&self, rec: &OffsetRec, a: f64, b: f64) -> CubicBez {
+        // wondering if p0 and p3 should be in rec


That would also prevent the doubling of those calculations for the midpoint when subdividing.

> [!NOTE] > The measurements in this PR were done very rough via a quick Chrome profile. Because the numbers are small, until we run a rigorous benchmark it'll be hard to validate the impact. Especially across browser engines, and with JS engines optimising WASM and fusing operators. ## Overview This PR adds initial WASM SIMD support to `fearless_simd`, implementing enough operations to enable WASM SIMD in linebender/vello#1053. Rather than implementing all operations in one large PR, this focuses on the essential subset needed for Vello and breaks up an otherwise huge change. There's also some tricky operations to implement using the small amount of WASM instructions 😅 . ## Performance Impact Tested with Ghost Tiger rendering in Vello: ### Without fast kurbo (baseline): - **Without SIMD**: ~42.31ms per frame - **With SIMD**: ~39.91ms per frame - **Improvement**: ~5% faster ### With fast kurbo (linebender/kurbo#427): - **Without SIMD**: ~6ms per frame - **With SIMD**: ~4ms per frame - **Improvement**: ~30% faster (Take this with a huge grain of salt) *Test methodology: I linked `vello` locally to `fearless_simd` via `path` reference, and modified `vello_hybrid` to use the WASM SIMD level.* ## Changes - **New architecture**: Added WASM SIMD128 support - **Operations implemented**: Core subset including: - Binary ops: `add`, `sub`, `mul`, `min`, `max` - Comparison ops: `simd_eq`, `simd_ne`, `simd_lt`, etc. - Math ops: `sqrt`, `madd` - **Testing**: Added parity tests ensuring Fallback and WASM SIMD produce identical results - **Bug fix**: Fixed incorrect mask generation in Fallback comparison operations (was returning `0/1` instead of `0/-1`) ## Test Plan Added `test_wasm_simd_parity!` macro that verifies operations produce identical results across Fallback and WASM SIMD implementations. I only tested a small subset. Maybe in the future we code-gen the tests as well? ## Next Steps Future PRs will add more operations to achieve full WASM SIMD coverage.

Instead of linearized least error, which can generate incorrect approximations (especially in near-cusp situations), use arc drawing. Also make error evaluation more robust, specifically to reject non-finite solutions. Sample points are equally spaced on the approximation rather than on the source curve. Thus, the Newton step refines t values on the source curve. The error report is expanded so that intermediate results can be reused in least square error refinement, as an optimization.

Subdivide by integral of absolute curvature, which should improve near-cusp behavior. I'm trying to make the core algorithm so robust it generates correct results even without the regularization step. However, that approach is not looking especially promising. A major sticking point is that it's difficult to ensure that both sides make consistent decisions. Also adds a stroke example adapted from Vello "tricky strokes", which in turn is adapted from Skia.

Use existing methods rather than doing rotation by hand. Fix typo.

LaurenzV · 2025-06-26T12:17:00Z

See my Zulip message for two suggestions for performance improvements: https://xi.zulipchat.com/#narrow/channel/260979-kurbo/topic/New.20stroker.20for.20sparse.20strips/near/525882812

Make it so regularization doesn't try to fix endpoint tangents, only internal cusps. WIP, this causes a regression.

Three changes intended to improve robustness. First, subdivide at t = 0.5 by default, and only do sophisticated subdivision (subdividing by L1 norm of curvature) when the default subdivision fails to reduce angle delta substantially. This is a better choice for inputs with zero derivative at an endpoint. In addition, this change should be a performance improvement in many cases. Second, make criterion for accepting convex-optimized subdivision point more stringent. Third, protect cusp evaluation at endpoints from divide by zero. With these changes, the logic correctly handles the failure case that Laurenz detected (from the Blend2D suite), as well as the tricky strokes test. Also bumps MSRV, as per discussion in office hours.

Note: there's probably some cleanup due to actually fix clippy warnings.

I've attempted to add a docstring to each major function and struct explaining what is computed, and also provide some rationale for robustness decisions.

jneem

Given that I've previously read your summaries on zulip, the doc comments make it pretty clear what's going on. It would be sweet to have a write-up with pictures and stuff, though!

jneem · 2025-07-10T00:26:45Z

+    /// to handle zero derivatives at endpoints. In fact, it is more robust when
+    /// the curve is not perturbed, as the curve matches the robustly computed
+    /// unit tangents.
+    pub(crate) fn regularize(&self, dimension: f64, do_endpoints: bool) -> CubicBez {


Maybe it's worth having separate regularize_endpoints and regularize_cusp methods?

jneem · 2025-07-10T00:39:43Z

+    /// the offset curve, and appends it to `result`.
+    fn offset_rec(&self, rec: &OffsetRec, result: &mut BezPath) {
+        // First, determine whether the offset curve contains a cusp. If the sign
+        // of the cusp value (curvature times offset plus 1) is different at the


Ok, I was wondering what "cusp value" meant. Maybe it's worth putting this in the docstring of endpoint_cusp, where I think it would be easier to find?

(ChatGPT was typically unhelpful here. It confidently asserted that "cusp value" refers to the parameter value t where the curve has a cusp)

jneem · 2025-07-10T01:04:26Z

+        self.offset_rec(&rec1, result);
+    }
+
+    /// Convert from (a, b) parameter space to the approximate cubic Bézier.


Is there a description of the (a, b) parameter space? If so, I missed it...

tomcur

The additional docstrings are great.

I've added a few suggestions to clarify curvature/cusps.

Several minor optimizations and improvements to comments. Thanks Joe and Tom. Co-authored-by: jneem <joeneeman@gmail.com> Co-authored-by: Tom Churchman <thomas@churchman.nl>

In response to review feedback, explain the (a, b) parameter space, add more docs around the cusp value. Also split the regularize function into its two components rather than taking a boolean. Lastly, fix clippy deprecation warnings.

DJMcNab

I've reviewed here wit han eye towards 0.11.3/0.12.0. This PR has clearly done what it can to be non-breaking, which is good. But it leaves a lot of things in a weird intermediate state as a result.

These can all be addressed post-merge if needed.

DJMcNab · 2025-07-17T08:57:19Z

+/// This struct was formerly used by the stroke expansion logic, but has since been
+/// replaced with a higher performance implementation not based on generic curve
+/// fitting. It should be considered deprecated, and may be removed in a future
+/// version.


We should document where users can find this alternative here

DJMcNab · 2025-07-17T09:03:07Z

We should probably also add a deprecation warning to the module-level docs (probably not using the deprecated attribute, but I'm not sure either way)

It's also not clear if those module level docs are now out of date; is the algorithm used still the one documented in that comment?

DJMcNab · 2025-07-17T09:05:16Z

    path: impl IntoIterator<Item = PathEl>,
    style: &Stroke,
-    opts: &StrokeOpts,
+    _opts: &StrokeOpts,


Should StrokeOpts (or at leasts StokeOptLevel) now be deprecated?

I guess if we deprecated StrokeOpts itself, it would make this API a little bit weird to use.

DJMcNab · 2025-07-17T09:05:27Z

@@ -199,14 +200,14 @@ struct StrokeCtx {
 pub fn stroke(


Are these high level docs of this method still accurate?

This was deprecated in linebender#427

As far as I can tell, benches at the top level of a virtual workspace don't actually get built or run, so this PR moves them down into `kurbo/`. I've also taken the liberty to port them to criterion, so that they can be run with stable rust. This requires bumping MSRV to 1.66 (for `std::hint::black_box`), but I went all the way up to 1.82 so that there won't be merge conflicts with #427 This PR should have no user-visible impact. I don' think there's any rush to get it in a patch release.

This was deprecated in linebender#427

This was deprecated in #427 This PR is easily the most dubious of the three (#488, #487), because the functionality: - is expected to be publicly exposed ("as we probably want to iterate on the exact interface") - Doesn't have a neat replacement. This will likely need Raph's sign-off --------- Co-authored-by: Joe Neeman <joeneeman@gmail.com>

This was deprecated in linebender/kurbo#427 This PR is easily the most dubious of the three (#488, #487), because the functionality: - is expected to be publicly exposed ("as we probably want to iterate on the exact interface") - Doesn't have a neat replacement. This will likely need Raph's sign-off --------- Co-authored-by: Joe Neeman <joeneeman@gmail.com>

raphlinus marked this pull request as ready for review March 20, 2025 14:13

tomcur reviewed Apr 8, 2025

View reviewed changes

tomcur reviewed Apr 9, 2025

View reviewed changes

LaurenzV mentioned this pull request May 4, 2025

Low framerate with vello_hybrid_winit example linebender/vello#955

Open

ajakubowicz-canva mentioned this pull request Jun 18, 2025

Add initial WASM SIMD128 support linebender/fearless_simd#8

Merged

raphlinus added 4 commits June 18, 2025 18:39

Merge branch 'main' into fast_stroke

b1d804f

Minor cleanup

4ecf72d

Use existing methods rather than doing rotation by hand. Fix typo.

LaurenzV and others added 6 commits June 27, 2025 17:43

Use temporary path for result

08d784c

Make regularization less aggressive

4f2a2a0

Make it so regularization doesn't try to fix endpoint tangents, only internal cusps. WIP, this causes a regression.

Merge branch 'main' into fast_stroke

63cb776

Some tweaks to silence clippy

ad1a64e

Note: there's probably some cleanup due to actually fix clippy warnings.

More docstring

e341a51

I've attempted to add a docstring to each major function and struct explaining what is computed, and also provide some rationale for robustness decisions.

jneem approved these changes Jul 10, 2025

View reviewed changes

tomcur approved these changes Jul 10, 2025

View reviewed changes

Comment thread kurbo/src/offset.rs Outdated

Comment thread kurbo/src/offset.rs Outdated

Comment thread kurbo/src/offset.rs Outdated

tomcur approved these changes Jul 10, 2025

View reviewed changes

Comment thread kurbo/src/offset.rs Outdated

raphlinus and others added 3 commits July 10, 2025 16:25

Apply suggestions from code review

d7d1043

Several minor optimizations and improvements to comments. Thanks Joe and Tom. Co-authored-by: jneem <joeneeman@gmail.com> Co-authored-by: Tom Churchman <thomas@churchman.nl>

Fix clippy warnings, more explanation

970239e

In response to review feedback, explain the (a, b) parameter space, add more docs around the cusp value. Also split the regularize function into its two components rather than taking a boolean. Lastly, fix clippy deprecation warnings.

Add changelog entry

7d40438

sagudev mentioned this pull request Jul 17, 2025

Release 0.11.3 #467

Merged

DJMcNab reviewed Jul 17, 2025

View reviewed changes

jneem mentioned this pull request Jul 17, 2025

Move benchmarks into the kurbo crate, and port to criterion #468

Merged

Comment updates to more accurately describe current state

5bd7f1a

raphlinus added this pull request to the merge queue Jul 22, 2025

Merged via the queue into main with commit 77cb963 Jul 22, 2025
15 checks passed

raphlinus deleted the fast_stroke branch July 22, 2025 16:17

DJMcNab added a commit to DJMcNab/kurbo that referenced this pull request Aug 21, 2025

Remove CubicOffset

191880a

This was deprecated in linebender#427

DJMcNab mentioned this pull request Aug 21, 2025

Remove CubicOffset #489

Merged

jneem pushed a commit to DJMcNab/kurbo that referenced this pull request Aug 28, 2025

Remove CubicOffset

8c497b8

This was deprecated in linebender#427

	// We never let cusp values haven an absolute value smaller than
	// We never let cusp values have an absolute value smaller than

Uh oh!

Conversation

raphlinus commented Mar 19, 2025

Uh oh!

tomcur left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LaurenzV commented Jun 26, 2025

Uh oh!

jneem left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tomcur left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

DJMcNab left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants