flo_curves icon indicating copy to clipboard operation
flo_curves copied to clipboard

Possible bug in offset_scaling

Open hellow554 opened this issue 3 years ago • 1 comments

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?

hellow554 avatar Feb 07 '22 12:02 hellow554

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)

Logicalshift avatar Feb 07 '22 20:02 Logicalshift