search

Logicalshift / flo_curves

Bezier curve library for Rust
Apache License 2.0
84 stars 6 forks source link

Possible bug in offset_scaling #16

Open hellow554 opened 2 years ago

hellow554 commented 2 years ago

https://github.com/Logicalshift/flo_curves/blob/ec1d7820643688e838c092fd6282ba541de47203/src/bezier/offset_scaling.rs#L154

Why is there a || true? This is either a temporary fix and should be commented or it's a bug.

Can you clarify this please?

Logicalshift commented 2 years ago

It does appear to be something left in from when I was testing this, and it appears to mask a bug, as removing it doesn't go well:

Screenshot 2022-02-07 at 19 26 14

Seems to be because find_extremities() returns unordered results, so the subdivisions are all over the place later on. The purpose of this is to reduce the number of subdivisions around sections of the curve that can't be scaled around a point - a downside of the scaling algorithm is it always subdivides the same number of times even if the resulting curve can be constructed without quite so many sections.

I've been wondering if there's an even better approach to subdividing the curve: the scaling algorithm requires the curve to be subdivided into sections such that the normals all converge on a roughly equidistant point: better algorithms subdivide less (but anything that subdivides will eventually find curves good enough to use so it's hard to tell if any given algorithm is 'best' or not).

Anyway, this should now be fixed so it works as it was intended (improving other aspects of the algorithm is at earliest a v0.7 thing I think)