Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add class for efficiently building B-Splines #6044

Merged
merged 7 commits into from
Nov 13, 2023

Conversation

default0
Copy link
Contributor

@default0 default0 commented Nov 10, 2023

This adds the IncrementalBSplineBuilder class which can be used to build up a B-Spline from a series of linear points efficiently. The class takes special care to keep the number of control points.

This is a preparatory framework side change to implement free-hand drawing of sliders in the osu!lazer editor.

Breaking changes

PathApproximator method signatures changed

The following PathApproximator methods have been renamed:

old new
ApproximateBezier BezierToPiecewiseLinear
ApproximateBSpline BSplineToPiecewiseLinear
ApproximateCatmull CatmullToPiecewiseLinear
ApproximateCircularArc CircularArcToPiecewiseLinear
ApproximateLinear LinearToPiecewiseLinear
ApproximateLagrangePolynomial LagrangePolynomialToPiecewiseLinear

Additionally, ApproximateBSpline's polynomial order argument p has been renamed to degree and lost its default value of 0.

@OliBomby
Copy link
Contributor

OliBomby commented Nov 10, 2023

osu.Framework.Tests_eW6t9KFjIX.mp4

It looks pretty cool, but i dont like how the B-splines dont approximate the drawn path accurately if you increase the tolerance so it doesn't spam too many anchors. It places the anchors on the drawn path so the spline path naturally drifts away from the drawn path. You're relying on spamming many anchors to get the spline to be close to the drawn path. I think you should use an algorithm to find optimal positions for the anchors so you can approximate the drawn path with fewer anchors.

Also would be nice if this didn't use B-splines because I don't think peppy is planning to add B-splines to the editor. ppy/osu#24457 (comment)

@default0
Copy link
Contributor Author

osu.Framework.Tests_eW6t9KFjIX.mp4

It looks pretty cool, but i dont like how the B-splines dont approximate the drawn path accurately if you increase the tolerance so it doesn't spam too many anchors. It places the anchors on the drawn path so the spline path naturally drifts away from the drawn path. You're relying on spamming many anchors to get the spline to be close to the drawn path. I think you should use an algorithm to find optimal positions for the anchors so you can approximate the drawn path with fewer anchors.

Also would be nice if this didn't use B-splines because I don't think peppy is planning to add B-splines to the editor. ppy/osu#24457 (comment)

Such an algorithm was already tried. It is both very complex, very hard to actually make computationally feasible, and is likely to cause bulging of the spline around corners - which is highly undesirable from a users POV.
The current solution always ensures that the path curves inwards from an approximate convex shape drawn by the user - this is usually very handy behavior for sliders because generally you would prefer something that is more compact than an ideal approximation - which this naturally provides.

If you have a concrete algorithm / approach in mind for determining the positions of control points, please share. I am open to ideas that are both runtime efficient enough to execute in real time even for fairly long hand-drawn paths as well as not causing undesirable side effects like bulging or instability of the approximation.

This adds the IncrementalBSplineBuilder class which can be used to
build up a B-Spline from a series of linear points efficiently.
The class takes special care to keep the number of control points
@OliBomby
Copy link
Contributor

Such an algorithm was already tried. It is both very complex, very hard to actually make computationally feasible, and is likely to cause bulging of the spline around corners - which is highly undesirable from a users POV. The current solution always ensures that the path curves inwards from an approximate convex shape drawn by the user - this is usually very handy behavior for sliders because generally you would prefer something that is more compact than an ideal approximation - which this naturally provides.

If you have a concrete algorithm / approach in mind for determining the positions of control points, please share. I am open to ideas that are both runtime efficient enough to execute in real time even for fairly long hand-drawn paths as well as not causing undesirable side effects like bulging or instability of the approximation.

I don't think the path having a bias to curve inwards is preferred behaviour. The property which is preferred is the smoothing. You want a path that smoothes out bumps in the piecewise linear drawn path. The smoothing doesn't necessarily need to curve inwards, i think it's better if it tries to follow the drawn shape as closely as possible.

I have a concrete algorithm which can achieve this. It's an optimization based algorithm and it's implemented in Python. It can do simple shapes in very few iterations, as fast as 6 milliseconds for a full circle shape with 10 anchors. It scales pretty nicely higher orders too, up to around order 1000. It currently only does Beziers, but B-splines could be pretty easily adapted since thats in the same class of interpolations.
https://github.com/OliBomby/Bezier-Approximation/blob/master/shape_approximator.py

@Tom94
Copy link
Collaborator

Tom94 commented Nov 12, 2023

Hi OliBomby -- thanks for the elaborate answer!

I have a concrete algorithm which can achieve this.

That sounds really promising, actually, and I like the loss descent via Adam! How stable is the optimization in your experience? What I mean is: if a user were to interactively draw a path and run your algorithm every frame, could it be made (e.g. with a fixed RNG seed) such that the resulting sliders look like they're incrementally drawn without wobbling around too much?

The smoothing doesn't necessarily need to curve inwards, i think it's better if it tries to follow the drawn shape as closely as possible.

The problem we encountered while implementing such a method was that those methods achieved a better fit by bulging around corners rather than becoming sharp. This was much more visually unpleasant than the inward-curving current solution with a larger curve distance... but at the same time we didn't do exactly the same thing that you did. Ours was more of a genetic algorithm and didn't make use of gradient information. Can your approach deal with this sort of situation? I suspect a careful regularizer / loss term might work and I'm hoping you've already ran into this and figured something out.

It can do simple shapes in very few iterations, as fast as 6 milliseconds for a full circle shape with 10 anchors.

We might need 1-2 orders of magnitude more speed, depending on the context in which you benchmarked -- we need around 5-10ms (to leave headroom for 60 fps) for the most complicated sliders a mapper might draw on a fairly old machine (say a 5-10 years old laptop) to ensure this feature will work for everyone. But I feel like that's probably achievable with careful tuning and if the bulging situation can be kept in check.

It'd be tremendously helpful if you could implement your BSpline-adapted algorithm into the PathApproximator API. Then, we could compare it to the current simple/naive approach in many different situations interactively and, if it works better, easily transition interactive slider drawing to it in the future. (Probably out of scope for this particular PR, though.)

@bdach bdach self-requested a review November 12, 2023 05:15
@bdach
Copy link
Collaborator

bdach commented Nov 12, 2023

I'm pretty okay with this in general as is. I'd say I generally prefer this over @OliBomby's approach as it is just simpler and appears to work well enough.

Personally, to entertain the alternative, I'd have to see that it (a) can perform at comparable speed or faster, and (b) be a measurable improvement in a significant number of reasonable cases. Otherwise I think there's no point complicating this with black-box optimisation.

@OliBomby
Copy link
Contributor

I agree my suggestion is a bit too complex to consider for this PR. I'll attempt implementing it in framework so we can do more comparisons and probably follow up with my own PR.

I'm optimistic that the mentioned concerns can be resolved and I know a few ways to make the algorithm even faster and more viable for real-time.

@bdach bdach enabled auto-merge November 13, 2023 03:44
@bdach bdach merged commit 1962cf6 into ppy:master Nov 13, 2023
11 of 13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants