osu-framework icon indicating copy to clipboard operation
osu-framework copied to clipboard

Implement performant Start/End progress for `Path` using BBH

Open EVAST9919 opened this issue 5 months ago • 4 comments

Depends on:

  • [x] https://github.com/ppy/osu-framework/pull/6603 (not really, I just started to build on top of that, actual diff is half the size)
  • [ ] https://github.com/ppy/osu-framework/pull/6630

This pr is basically plug'n'play (all use cases are preserved), so even without proper osu-side integration of new start/end progress methods everything will work as expected (though with the additional cost of tree creation).

osu-side diff to enable new performant start/end progress
--- a/osu.Game.Rulesets.Osu/Skinning/SnakingSliderBody.cs
+++ b/osu.Game.Rulesets.Osu/Skinning/SnakingSliderBody.cs
@@ -150,8 +150,9 @@ private void setRange(double p0, double p1)
-          drawableSlider.HitObject.Path.GetPathToProgress(CurrentCurve, p0, p1);
-
-            SetVertices(CurrentCurve);
+           Path.StartProgress = (float)p0;
+           Path.EndProgress = (float)p1;

Start/End progress

To create a sub-path starting at progress we will do the following:

  1. Undo previous changes to the tree (if any were made)
  2. Find a first node at which path distance will be less than progress*total path distance
  3. Modify its start point and save it's index (to be able to undo changes in step 1 on a new progress value set)
  4. Go up the tree computing the new bounds without nodes from the left

For end progress we are doing the same but from the right side. This approach has additional benefit - we can copy all the segments of the path to the DrawNode once (after tree creation), and only change the range of segments to be drawn (replacing first and last segment to modified ones).

Path.SetStartProgress benchmark

pr:

Method Mean Error StdDev Allocated
SetStartProgressBBH100 161.0 ns 0.43 ns 0.40 ns -
SetStartProgressBBH1000 307.0 ns 0.98 ns 0.91 ns -
SetStartProgressBBH10000 549.4 ns 0.57 ns 0.48 ns -
SetStartProgressBBH100000 774.0 ns 1.40 ns 1.31 ns -
SetStartProgressBBH1000000 1,303.3 ns 2.06 ns 1.93 ns -

EVAST9919 avatar Jul 15 '25 06:07 EVAST9919

@EVAST9919 did you happen to benchmark against existing code under similar scenarios?

peppy avatar Jul 18 '25 08:07 peppy

@EVAST9919 did you happen to benchmark against existing code under similar scenarios?

Good idea to compare overall time of arbitrary path creation, will do that

EVAST9919 avatar Jul 18 '25 09:07 EVAST9919

@EVAST9919 did you happen to benchmark against existing code under similar scenarios?

I've updated the pr description to include that.

EVAST9919 avatar Jul 18 '25 21:07 EVAST9919

With more and more changes in https://github.com/ppy/osu-framework/pull/6630 it would be easier to make a new pr instead of fixing all the conflicts, so I'll mark this one as a draft to use as a reference for an updated pr after https://github.com/ppy/osu-framework/pull/6630 merge.

EVAST9919 avatar Sep 17 '25 19:09 EVAST9919