Skip to content

Commit

Permalink
Merge pull request #28297 from Hecatia-Lapislazuli/near-straight-slid…
Browse files Browse the repository at this point in the history
…er-crash-fix

Fix for nearly straight sliders causing a crash
  • Loading branch information
peppy authored May 31, 2024
2 parents b76ec96 + 9111da8 commit 4162d0b
Show file tree
Hide file tree
Showing 2 changed files with 133 additions and 0 deletions.
119 changes: 119 additions & 0 deletions osu.Game.Rulesets.Osu.Tests/Editor/TestSliderScaling.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,12 @@

#nullable disable

using System;
using System.Linq;
using NUnit.Framework;
using osu.Framework.Graphics.Primitives;
using osu.Framework.Testing;
using osu.Framework.Utils;
using osu.Game.Beatmaps;
using osu.Game.Rulesets.Objects;
using osu.Game.Rulesets.Objects.Types;
Expand Down Expand Up @@ -71,4 +74,120 @@ public void TestScalingLinearSlider()
private void moveMouse(Vector2 pos) =>
AddStep($"move mouse to {pos}", () => InputManager.MoveMouseTo(playfield.ToScreenSpace(pos)));
}

[TestFixture]
public class TestSliderNearLinearScaling
{
private readonly Random rng = new Random(1337);

[Test]
public void TestScalingSliderFlat()
{
SliderPath sliderPathPerfect = new SliderPath(
[
new PathControlPoint(new Vector2(0), PathType.PERFECT_CURVE),
new PathControlPoint(new Vector2(50, 25)),
new PathControlPoint(new Vector2(25, 100)),
]);

SliderPath sliderPathBezier = new SliderPath(
[
new PathControlPoint(new Vector2(0), PathType.BEZIER),
new PathControlPoint(new Vector2(50, 25)),
new PathControlPoint(new Vector2(25, 100)),
]);

scaleSlider(sliderPathPerfect, new Vector2(0.000001f, 1));
scaleSlider(sliderPathBezier, new Vector2(0.000001f, 1));

for (int i = 0; i < 100; i++)
{
Assert.True(Precision.AlmostEquals(sliderPathPerfect.PositionAt(i / 100.0f), sliderPathBezier.PositionAt(i / 100.0f)));
}
}

[Test]
public void TestPerfectCurveMatchesTheoretical()
{
for (int i = 0; i < 20000; i++)
{
//Only test points that are in the screen's bounds
float p1X = 640.0f * (float)rng.NextDouble();
float p2X = 640.0f * (float)rng.NextDouble();

float p1Y = 480.0f * (float)rng.NextDouble();
float p2Y = 480.0f * (float)rng.NextDouble();
SliderPath sliderPathPerfect = new SliderPath(
[
new PathControlPoint(new Vector2(0, 0), PathType.PERFECT_CURVE),
new PathControlPoint(new Vector2(p1X, p1Y)),
new PathControlPoint(new Vector2(p2X, p2Y)),
]);

assertMatchesPerfectCircle(sliderPathPerfect);

scaleSlider(sliderPathPerfect, new Vector2(0.00001f, 1));

assertMatchesPerfectCircle(sliderPathPerfect);
}
}

private void assertMatchesPerfectCircle(SliderPath path)
{
if (path.ControlPoints.Count != 3)
return;

//Replication of PathApproximator.CircularArcToPiecewiseLinear
CircularArcProperties circularArcProperties = new CircularArcProperties(path.ControlPoints.Select(x => x.Position).ToArray());

if (!circularArcProperties.IsValid)
return;

//Addresses cases where circularArcProperties.ThetaRange>0.5
//Occurs in code in PathControlPointVisualiser.ensureValidPathType
RectangleF boundingBox = PathApproximator.CircularArcBoundingBox(path.ControlPoints.Select(x => x.Position).ToArray());
if (boundingBox.Width >= 640 || boundingBox.Height >= 480)
return;

int subpoints = (2f * circularArcProperties.Radius <= 0.1f) ? 2 : Math.Max(2, (int)Math.Ceiling(circularArcProperties.ThetaRange / (2.0 * Math.Acos(1f - (0.1f / circularArcProperties.Radius)))));

//ignore cases where subpoints is int.MaxValue, result will be garbage
//as well, having this many subpoints will cause an out of memory error, so can't happen during normal useage
if (subpoints == int.MaxValue)
return;

for (int i = 0; i < Math.Min(subpoints, 100); i++)
{
float progress = (float)rng.NextDouble();

//To avoid errors from interpolating points, ensure we check only positions that would be subpoints.
progress = (float)Math.Ceiling(progress * (subpoints - 1)) / (subpoints - 1);

//Special case - if few subpoints, ensure checking every single one rather than randomly
if (subpoints < 100)
progress = i / (float)(subpoints - 1);

//edge points cause issue with interpolation, so ignore the last two points and first
if (progress == 0.0f || progress >= (subpoints - 2) / (float)(subpoints - 1))
continue;

double theta = circularArcProperties.ThetaStart + (circularArcProperties.Direction * progress * circularArcProperties.ThetaRange);
Vector2 vector = new Vector2((float)Math.Cos(theta), (float)Math.Sin(theta)) * circularArcProperties.Radius;

Assert.True(Precision.AlmostEquals(circularArcProperties.Centre + vector, path.PositionAt(progress), 0.01f),
"A perfect circle with points " + string.Join(", ", path.ControlPoints.Select(x => x.Position)) + " and radius" + circularArcProperties.Radius + "from SliderPath does not almost equal a theoretical perfect circle with " + subpoints + " subpoints"
+ ": " + (circularArcProperties.Centre + vector) + " - " + path.PositionAt(progress)
+ " = " + (circularArcProperties.Centre + vector - path.PositionAt(progress))
);
}
}

private void scaleSlider(SliderPath path, Vector2 scale)
{
for (int i = 0; i < path.ControlPoints.Count; i++)
{
path.ControlPoints[i].Position *= scale;
}
}
}
}
14 changes: 14 additions & 0 deletions osu.Game/Rulesets/Objects/SliderPath.cs
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,20 @@ private List<Vector2> calculateSubPath(ReadOnlySpan<Vector2> subControlPoints, P
if (subControlPoints.Length != 3)
break;

CircularArcProperties circularArcProperties = new CircularArcProperties(subControlPoints);

// `PathApproximator` will already internally revert to B-spline if the arc isn't valid.
if (!circularArcProperties.IsValid)
break;

// taken from https://github.com/ppy/osu-framework/blob/1201e641699a1d50d2f6f9295192dad6263d5820/osu.Framework/Utils/PathApproximator.cs#L181-L186
int subPoints = (2f * circularArcProperties.Radius <= 0.1f) ? 2 : Math.Max(2, (int)Math.Ceiling(circularArcProperties.ThetaRange / (2.0 * Math.Acos(1f - (0.1f / circularArcProperties.Radius)))));

// 1000 subpoints requires an arc length of at least ~120 thousand to occur
// See here for calculations https://www.desmos.com/calculator/umj6jvmcz7
if (subPoints >= 1000)
break;

List<Vector2> subPath = PathApproximator.CircularArcToPiecewiseLinear(subControlPoints);

// If for some reason a circular arc could not be fit to the 3 given points, fall back to a numerically stable bezier approximation.
Expand Down

0 comments on commit 4162d0b

Please sign in to comment.