From d47c4cb47946877f7ba00bfe0d497137638f8230 Mon Sep 17 00:00:00 2001 From: Aurelian Date: Fri, 24 May 2024 06:28:19 +0200 Subject: [PATCH 01/10] Test for scaling slider flat --- .../Editor/TestSliderScaling.cs | 113 ++++++++++++++++++ 1 file changed, 113 insertions(+) diff --git a/osu.Game.Rulesets.Osu.Tests/Editor/TestSliderScaling.cs b/osu.Game.Rulesets.Osu.Tests/Editor/TestSliderScaling.cs index 021fdba225..157a08df46 100644 --- a/osu.Game.Rulesets.Osu.Tests/Editor/TestSliderScaling.cs +++ b/osu.Game.Rulesets.Osu.Tests/Editor/TestSliderScaling.cs @@ -68,6 +68,119 @@ public void TestScalingLinearSlider() AddAssert("slider length shrunk", () => slider.Path.Distance < distanceBefore); } + [Test] + [Timeout(4000)] //Catches crashes in other threads, but not ideal. Hopefully there is a improvement to this. + public void TestScalingSliderFlat( + [Values(0, 1, 2, 3)] int type_int + ) + { + Slider slider = null; + + switch (type_int) + { + case 0: + AddStep("Add linear slider", () => + { + slider = new Slider { StartTime = EditorClock.CurrentTime, Position = new Vector2(300) }; + + PathControlPoint[] points = + { + new PathControlPoint(new Vector2(0), PathType.LINEAR), + new PathControlPoint(new Vector2(50, 100)), + }; + + slider.Path = new SliderPath(points); + EditorBeatmap.Add(slider); + }); + break; + case 1: + AddStep("Add perfect curve slider", () => + { + slider = new Slider { StartTime = EditorClock.CurrentTime, Position = new Vector2(300) }; + + PathControlPoint[] points = + { + new PathControlPoint(new Vector2(0), PathType.PERFECT_CURVE), + new PathControlPoint(new Vector2(50, 25)), + new PathControlPoint(new Vector2(25, 100)), + }; + + slider.Path = new SliderPath(points); + EditorBeatmap.Add(slider); + }); + break; + case 2: + AddStep("Add bezier slider", () => + { + slider = new Slider { StartTime = EditorClock.CurrentTime, Position = new Vector2(300) }; + + PathControlPoint[] points = + { + new PathControlPoint(new Vector2(0), PathType.BEZIER), + new PathControlPoint(new Vector2(50, 25)), + new PathControlPoint(new Vector2(25, 80)), + new PathControlPoint(new Vector2(40, 100)), + }; + + slider.Path = new SliderPath(points); + EditorBeatmap.Add(slider); + }); + break; + AddStep("Add perfect curve slider", () => + { + slider = new Slider { StartTime = EditorClock.CurrentTime, Position = new Vector2(300) }; + + PathControlPoint[] points = + { + new PathControlPoint(new Vector2(0), PathType.PERFECT_CURVE), + new PathControlPoint(new Vector2(50, 25)), + new PathControlPoint(new Vector2(25, 100)), + }; + + slider.Path = new SliderPath(points); + EditorBeatmap.Add(slider); + }); + break; + case 3: + AddStep("Add catmull slider", () => + { + slider = new Slider { StartTime = EditorClock.CurrentTime, Position = new Vector2(300) }; + + PathControlPoint[] points = + { + new PathControlPoint(new Vector2(0), PathType.CATMULL), + new PathControlPoint(new Vector2(50, 25)), + new PathControlPoint(new Vector2(25, 80)), + new PathControlPoint(new Vector2(40, 100)), + }; + + slider.Path = new SliderPath(points); + EditorBeatmap.Add(slider); + }); + break; + } + + AddAssert("ensure object placed", () => EditorBeatmap.HitObjects.Count == 1); + + moveMouse(new Vector2(300)); + AddStep("select slider", () => InputManager.Click(MouseButton.Left)); + AddStep("slider is valid", () => slider.Path.GetSegmentEnds()); //To run ensureValid(); + + SelectionBoxDragHandle dragHandle = null!; + AddStep("store drag handle", () => dragHandle = Editor.ChildrenOfType().Skip(1).First()); + AddAssert("is dragHandle not null", () => dragHandle != null); + + AddStep("move mouse to handle", () => InputManager.MoveMouseTo(dragHandle)); + AddStep("begin drag", () => InputManager.PressButton(MouseButton.Left)); + moveMouse(new Vector2(0, 300)); + AddStep("end drag", () => InputManager.ReleaseButton(MouseButton.Left)); + + AddStep("move mouse to handle", () => InputManager.MoveMouseTo(dragHandle)); + AddStep("begin drag", () => InputManager.PressButton(MouseButton.Left)); + moveMouse(new Vector2(0, 300)); //Should crash here if broken, although doesn't count as failed... + AddStep("end drag", () => InputManager.ReleaseButton(MouseButton.Left)); + } + private void moveMouse(Vector2 pos) => AddStep($"move mouse to {pos}", () => InputManager.MoveMouseTo(playfield.ToScreenSpace(pos))); } From d948e0fc5c3af124d56b6d8bb80d2d904f73a704 Mon Sep 17 00:00:00 2001 From: Aurelian Date: Fri, 24 May 2024 08:26:17 +0200 Subject: [PATCH 02/10] Nearly straight sliders are treated as linear --- osu.Game/Rulesets/Objects/SliderPath.cs | 35 +++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/osu.Game/Rulesets/Objects/SliderPath.cs b/osu.Game/Rulesets/Objects/SliderPath.cs index e8e769e3fa..b0a5d02e71 100644 --- a/osu.Game/Rulesets/Objects/SliderPath.cs +++ b/osu.Game/Rulesets/Objects/SliderPath.cs @@ -269,6 +269,37 @@ private void ensureValid() pathCache.Validate(); } + /// + /// Checks if the array of vectors is almost straight. + /// + /// + /// The angle is first obtained based on the farthest vector from the first, + /// then we find the angle of each vector from the first, + /// and calculate the distance between the two angle vectors. + /// We than scale this distance to the distance from the first vector + /// (or by 10 if the distance is smaller), + /// and if it is greater than acceptableDifference, we return false. + /// + private static bool isAlmostStraight(Vector2[] vectors, float acceptableDifference = 0.1f) + { + if (vectors.Length <= 2 || vectors.All(x => x == vectors.First())) return true; + + Vector2 first = vectors.First(); + Vector2 farthest = vectors.MaxBy(x => Vector2.Distance(first, x)); + + Vector2 angle = Vector2.Normalize(farthest - first); + foreach (Vector2 vector in vectors) + { + if (vector == first) + continue; + + if (Math.Max(10.0f, Vector2.Distance(vector, first)) * Vector2.Distance(Vector2.Normalize(vector - first), angle) > acceptableDifference) + return false; + } + + return true; + } + private void calculatePath() { calculatedPath.Clear(); @@ -293,6 +324,10 @@ private void calculatePath() var segmentVertices = vertices.AsSpan().Slice(start, i - start + 1); var segmentType = ControlPoints[start].Type ?? PathType.LINEAR; + //If a segment is almost straight, treat it as linear. + if (segmentType != PathType.LINEAR && isAlmostStraight(segmentVertices.ToArray())) + segmentType = PathType.LINEAR; + // No need to calculate path when there is only 1 vertex if (segmentVertices.Length == 1) calculatedPath.Add(segmentVertices[0]); From 481883801fbdb8df3303d2cbec05fbe52cfdbdc9 Mon Sep 17 00:00:00 2001 From: Aurelian Date: Fri, 24 May 2024 08:55:10 +0200 Subject: [PATCH 03/10] Removed duplicate unreachable code --- .../Editor/TestSliderScaling.cs | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/osu.Game.Rulesets.Osu.Tests/Editor/TestSliderScaling.cs b/osu.Game.Rulesets.Osu.Tests/Editor/TestSliderScaling.cs index 157a08df46..44f85837dc 100644 --- a/osu.Game.Rulesets.Osu.Tests/Editor/TestSliderScaling.cs +++ b/osu.Game.Rulesets.Osu.Tests/Editor/TestSliderScaling.cs @@ -126,21 +126,6 @@ public void TestScalingSliderFlat( EditorBeatmap.Add(slider); }); break; - AddStep("Add perfect curve slider", () => - { - slider = new Slider { StartTime = EditorClock.CurrentTime, Position = new Vector2(300) }; - - PathControlPoint[] points = - { - new PathControlPoint(new Vector2(0), PathType.PERFECT_CURVE), - new PathControlPoint(new Vector2(50, 25)), - new PathControlPoint(new Vector2(25, 100)), - }; - - slider.Path = new SliderPath(points); - EditorBeatmap.Add(slider); - }); - break; case 3: AddStep("Add catmull slider", () => { From fff52be59a9bc076aa30d7c9045dcf3acd1aff58 Mon Sep 17 00:00:00 2001 From: Aurelian Date: Fri, 24 May 2024 09:30:24 +0200 Subject: [PATCH 04/10] Addressed code quality issues --- .../Editor/TestSliderScaling.cs | 21 +++++++++++-------- osu.Game/Rulesets/Objects/SliderPath.cs | 5 +++-- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/osu.Game.Rulesets.Osu.Tests/Editor/TestSliderScaling.cs b/osu.Game.Rulesets.Osu.Tests/Editor/TestSliderScaling.cs index 44f85837dc..99694f82d1 100644 --- a/osu.Game.Rulesets.Osu.Tests/Editor/TestSliderScaling.cs +++ b/osu.Game.Rulesets.Osu.Tests/Editor/TestSliderScaling.cs @@ -71,12 +71,12 @@ public void TestScalingLinearSlider() [Test] [Timeout(4000)] //Catches crashes in other threads, but not ideal. Hopefully there is a improvement to this. public void TestScalingSliderFlat( - [Values(0, 1, 2, 3)] int type_int + [Values(0, 1, 2, 3)] int typeInt ) { - Slider slider = null; + Slider slider = null!; - switch (type_int) + switch (typeInt) { case 0: AddStep("Add linear slider", () => @@ -93,6 +93,7 @@ public void TestScalingSliderFlat( EditorBeatmap.Add(slider); }); break; + case 1: AddStep("Add perfect curve slider", () => { @@ -109,14 +110,15 @@ public void TestScalingSliderFlat( EditorBeatmap.Add(slider); }); break; - case 2: - AddStep("Add bezier slider", () => + + case 3: + AddStep("Add catmull slider", () => { slider = new Slider { StartTime = EditorClock.CurrentTime, Position = new Vector2(300) }; PathControlPoint[] points = { - new PathControlPoint(new Vector2(0), PathType.BEZIER), + new PathControlPoint(new Vector2(0), PathType.CATMULL), new PathControlPoint(new Vector2(50, 25)), new PathControlPoint(new Vector2(25, 80)), new PathControlPoint(new Vector2(40, 100)), @@ -126,14 +128,15 @@ public void TestScalingSliderFlat( EditorBeatmap.Add(slider); }); break; - case 3: - AddStep("Add catmull slider", () => + + default: + AddStep("Add bezier slider", () => { slider = new Slider { StartTime = EditorClock.CurrentTime, Position = new Vector2(300) }; PathControlPoint[] points = { - new PathControlPoint(new Vector2(0), PathType.CATMULL), + new PathControlPoint(new Vector2(0), PathType.BEZIER), new PathControlPoint(new Vector2(50, 25)), new PathControlPoint(new Vector2(25, 80)), new PathControlPoint(new Vector2(40, 100)), diff --git a/osu.Game/Rulesets/Objects/SliderPath.cs b/osu.Game/Rulesets/Objects/SliderPath.cs index b0a5d02e71..ddea23034c 100644 --- a/osu.Game/Rulesets/Objects/SliderPath.cs +++ b/osu.Game/Rulesets/Objects/SliderPath.cs @@ -274,9 +274,9 @@ private void ensureValid() /// /// /// The angle is first obtained based on the farthest vector from the first, - /// then we find the angle of each vector from the first, + /// then we find the angle of each vector from the first, /// and calculate the distance between the two angle vectors. - /// We than scale this distance to the distance from the first vector + /// We than scale this distance to the distance from the first vector /// (or by 10 if the distance is smaller), /// and if it is greater than acceptableDifference, we return false. /// @@ -288,6 +288,7 @@ private static bool isAlmostStraight(Vector2[] vectors, float acceptableDifferen Vector2 farthest = vectors.MaxBy(x => Vector2.Distance(first, x)); Vector2 angle = Vector2.Normalize(farthest - first); + foreach (Vector2 vector in vectors) { if (vector == first) From b2c4e0e951bb9fc5e1ba50ffe4429109f11b34cc Mon Sep 17 00:00:00 2001 From: Aurelian Date: Fri, 24 May 2024 14:05:56 +0200 Subject: [PATCH 05/10] Reworked linear line check, and optimized scaled flat slider test --- .../Editor/TestSliderScaling.cs | 147 ++++++------------ osu.Game/Rulesets/Objects/SliderPath.cs | 43 +---- 2 files changed, 53 insertions(+), 137 deletions(-) diff --git a/osu.Game.Rulesets.Osu.Tests/Editor/TestSliderScaling.cs b/osu.Game.Rulesets.Osu.Tests/Editor/TestSliderScaling.cs index 99694f82d1..ef3824b5b0 100644 --- a/osu.Game.Rulesets.Osu.Tests/Editor/TestSliderScaling.cs +++ b/osu.Game.Rulesets.Osu.Tests/Editor/TestSliderScaling.cs @@ -6,6 +6,7 @@ using System.Linq; using NUnit.Framework; using osu.Framework.Testing; +using osu.Framework.Utils; using osu.Game.Beatmaps; using osu.Game.Rulesets.Objects; using osu.Game.Rulesets.Objects.Types; @@ -68,108 +69,52 @@ public void TestScalingLinearSlider() AddAssert("slider length shrunk", () => slider.Path.Distance < distanceBefore); } - [Test] - [Timeout(4000)] //Catches crashes in other threads, but not ideal. Hopefully there is a improvement to this. - public void TestScalingSliderFlat( - [Values(0, 1, 2, 3)] int typeInt - ) - { - Slider slider = null!; - - switch (typeInt) - { - case 0: - AddStep("Add linear slider", () => - { - slider = new Slider { StartTime = EditorClock.CurrentTime, Position = new Vector2(300) }; - - PathControlPoint[] points = - { - new PathControlPoint(new Vector2(0), PathType.LINEAR), - new PathControlPoint(new Vector2(50, 100)), - }; - - slider.Path = new SliderPath(points); - EditorBeatmap.Add(slider); - }); - break; - - case 1: - AddStep("Add perfect curve slider", () => - { - slider = new Slider { StartTime = EditorClock.CurrentTime, Position = new Vector2(300) }; - - PathControlPoint[] points = - { - new PathControlPoint(new Vector2(0), PathType.PERFECT_CURVE), - new PathControlPoint(new Vector2(50, 25)), - new PathControlPoint(new Vector2(25, 100)), - }; - - slider.Path = new SliderPath(points); - EditorBeatmap.Add(slider); - }); - break; - - case 3: - AddStep("Add catmull slider", () => - { - slider = new Slider { StartTime = EditorClock.CurrentTime, Position = new Vector2(300) }; - - PathControlPoint[] points = - { - new PathControlPoint(new Vector2(0), PathType.CATMULL), - new PathControlPoint(new Vector2(50, 25)), - new PathControlPoint(new Vector2(25, 80)), - new PathControlPoint(new Vector2(40, 100)), - }; - - slider.Path = new SliderPath(points); - EditorBeatmap.Add(slider); - }); - break; - - default: - AddStep("Add bezier slider", () => - { - slider = new Slider { StartTime = EditorClock.CurrentTime, Position = new Vector2(300) }; - - PathControlPoint[] points = - { - new PathControlPoint(new Vector2(0), PathType.BEZIER), - new PathControlPoint(new Vector2(50, 25)), - new PathControlPoint(new Vector2(25, 80)), - new PathControlPoint(new Vector2(40, 100)), - }; - - slider.Path = new SliderPath(points); - EditorBeatmap.Add(slider); - }); - break; - } - - AddAssert("ensure object placed", () => EditorBeatmap.HitObjects.Count == 1); - - moveMouse(new Vector2(300)); - AddStep("select slider", () => InputManager.Click(MouseButton.Left)); - AddStep("slider is valid", () => slider.Path.GetSegmentEnds()); //To run ensureValid(); - - SelectionBoxDragHandle dragHandle = null!; - AddStep("store drag handle", () => dragHandle = Editor.ChildrenOfType().Skip(1).First()); - AddAssert("is dragHandle not null", () => dragHandle != null); - - AddStep("move mouse to handle", () => InputManager.MoveMouseTo(dragHandle)); - AddStep("begin drag", () => InputManager.PressButton(MouseButton.Left)); - moveMouse(new Vector2(0, 300)); - AddStep("end drag", () => InputManager.ReleaseButton(MouseButton.Left)); - - AddStep("move mouse to handle", () => InputManager.MoveMouseTo(dragHandle)); - AddStep("begin drag", () => InputManager.PressButton(MouseButton.Left)); - moveMouse(new Vector2(0, 300)); //Should crash here if broken, although doesn't count as failed... - AddStep("end drag", () => InputManager.ReleaseButton(MouseButton.Left)); - } - private void moveMouse(Vector2 pos) => AddStep($"move mouse to {pos}", () => InputManager.MoveMouseTo(playfield.ToScreenSpace(pos))); } + [TestFixture] + public class TestSliderNearLinearScaling + { + [Test] + public void TestScalingSliderFlat() + { + Slider sliderPerfect = new Slider + { + Position = new Vector2(300), + Path = new SliderPath( + [ + new PathControlPoint(new Vector2(0), PathType.PERFECT_CURVE), + new PathControlPoint(new Vector2(50, 25)), + new PathControlPoint(new Vector2(25, 100)), + ]) + }; + + Slider sliderBezier = new Slider + { + Position = new Vector2(300), + Path = new SliderPath( + [ + new PathControlPoint(new Vector2(0), PathType.BEZIER), + new PathControlPoint(new Vector2(50, 25)), + new PathControlPoint(new Vector2(25, 100)), + ]) + }; + + scaleSlider(sliderPerfect, new Vector2(0.000001f, 1)); + scaleSlider(sliderBezier, new Vector2(0.000001f, 1)); + + for (int i = 0; i < 100; i++) + { + Assert.True(Precision.AlmostEquals(sliderPerfect.Path.PositionAt(i / 100.0f), sliderBezier.Path.PositionAt(i / 100.0f))); + } + } + + private void scaleSlider(Slider slider, Vector2 scale) + { + for (int i = 0; i < slider.Path.ControlPoints.Count; i++) + { + slider.Path.ControlPoints[i].Position *= scale; + } + } + } } diff --git a/osu.Game/Rulesets/Objects/SliderPath.cs b/osu.Game/Rulesets/Objects/SliderPath.cs index ddea23034c..eca14269fe 100644 --- a/osu.Game/Rulesets/Objects/SliderPath.cs +++ b/osu.Game/Rulesets/Objects/SliderPath.cs @@ -269,38 +269,6 @@ private void ensureValid() pathCache.Validate(); } - /// - /// Checks if the array of vectors is almost straight. - /// - /// - /// The angle is first obtained based on the farthest vector from the first, - /// then we find the angle of each vector from the first, - /// and calculate the distance between the two angle vectors. - /// We than scale this distance to the distance from the first vector - /// (or by 10 if the distance is smaller), - /// and if it is greater than acceptableDifference, we return false. - /// - private static bool isAlmostStraight(Vector2[] vectors, float acceptableDifference = 0.1f) - { - if (vectors.Length <= 2 || vectors.All(x => x == vectors.First())) return true; - - Vector2 first = vectors.First(); - Vector2 farthest = vectors.MaxBy(x => Vector2.Distance(first, x)); - - Vector2 angle = Vector2.Normalize(farthest - first); - - foreach (Vector2 vector in vectors) - { - if (vector == first) - continue; - - if (Math.Max(10.0f, Vector2.Distance(vector, first)) * Vector2.Distance(Vector2.Normalize(vector - first), angle) > acceptableDifference) - return false; - } - - return true; - } - private void calculatePath() { calculatedPath.Clear(); @@ -325,10 +293,6 @@ private void calculatePath() var segmentVertices = vertices.AsSpan().Slice(start, i - start + 1); var segmentType = ControlPoints[start].Type ?? PathType.LINEAR; - //If a segment is almost straight, treat it as linear. - if (segmentType != PathType.LINEAR && isAlmostStraight(segmentVertices.ToArray())) - segmentType = PathType.LINEAR; - // No need to calculate path when there is only 1 vertex if (segmentVertices.Length == 1) calculatedPath.Add(segmentVertices[0]); @@ -366,6 +330,13 @@ private List calculateSubPath(ReadOnlySpan subControlPoints, P if (subControlPoints.Length != 3) break; + //If a curve's theta range almost equals zero, the radius needed to have more than a + //floating point error difference is very large and results in a nearly straight path. + //Calculate it via a bezier aproximation instead. + //0.0005 corresponds with a radius of 8000 to have a more than 0.001 shift in the X value + if (Math.Abs(new CircularArcProperties(subControlPoints).ThetaRange) <= 0.0005d) + break; + List 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. From d81be56adf67a46c0302695b273ac7c0f6c8e212 Mon Sep 17 00:00:00 2001 From: Aurelian Date: Mon, 27 May 2024 19:35:33 +0200 Subject: [PATCH 06/10] Test for accuracy of perfect curves --- .../Editor/TestSliderScaling.cs | 125 ++++++++++++++---- 1 file changed, 99 insertions(+), 26 deletions(-) diff --git a/osu.Game.Rulesets.Osu.Tests/Editor/TestSliderScaling.cs b/osu.Game.Rulesets.Osu.Tests/Editor/TestSliderScaling.cs index ef3824b5b0..2ffde0d3a3 100644 --- a/osu.Game.Rulesets.Osu.Tests/Editor/TestSliderScaling.cs +++ b/osu.Game.Rulesets.Osu.Tests/Editor/TestSliderScaling.cs @@ -3,8 +3,10 @@ #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; @@ -72,48 +74,119 @@ 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() { - Slider sliderPerfect = new Slider - { - Position = new Vector2(300), - Path = new SliderPath( - [ - new PathControlPoint(new Vector2(0), PathType.PERFECT_CURVE), - new PathControlPoint(new Vector2(50, 25)), - new PathControlPoint(new Vector2(25, 100)), - ]) - }; + SliderPath sliderPathPerfect = new SliderPath( + [ + new PathControlPoint(new Vector2(0), PathType.PERFECT_CURVE), + new PathControlPoint(new Vector2(50, 25)), + new PathControlPoint(new Vector2(25, 100)), + ]); - Slider sliderBezier = new Slider - { - Position = new Vector2(300), - Path = new SliderPath( - [ - new PathControlPoint(new Vector2(0), PathType.BEZIER), - 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(sliderPerfect, new Vector2(0.000001f, 1)); - scaleSlider(sliderBezier, new Vector2(0.000001f, 1)); + 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(sliderPerfect.Path.PositionAt(i / 100.0f), sliderBezier.Path.PositionAt(i / 100.0f))); + Assert.True(Precision.AlmostEquals(sliderPathPerfect.PositionAt(i / 100.0f), sliderPathBezier.PositionAt(i / 100.0f))); } } - private void scaleSlider(Slider slider, Vector2 scale) + [Test] + public void TestPerfectCurveMatchesTheoretical() { - for (int i = 0; i < slider.Path.ControlPoints.Count; i++) + for (int i = 0; i < 20000; i++) { - slider.Path.ControlPoints[i].Position *= scale; + //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; } } } From 172cfdf88d4fb6ffb1b4dd35c5183b272407629e Mon Sep 17 00:00:00 2001 From: Aurelian Date: Mon, 27 May 2024 19:41:38 +0200 Subject: [PATCH 07/10] Added missing brackets for formulas --- osu.Game.Rulesets.Osu.Tests/Editor/TestSliderScaling.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game.Rulesets.Osu.Tests/Editor/TestSliderScaling.cs b/osu.Game.Rulesets.Osu.Tests/Editor/TestSliderScaling.cs index 2ffde0d3a3..52a170b84e 100644 --- a/osu.Game.Rulesets.Osu.Tests/Editor/TestSliderScaling.cs +++ b/osu.Game.Rulesets.Osu.Tests/Editor/TestSliderScaling.cs @@ -149,7 +149,7 @@ private void assertMatchesPerfectCircle(SliderPath path) 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)))); + 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 @@ -171,7 +171,7 @@ private void assertMatchesPerfectCircle(SliderPath path) if (progress == 0.0f || progress >= (subpoints - 2) / (float)(subpoints - 1)) continue; - double theta = circularArcProperties.ThetaStart + circularArcProperties.Direction * progress * circularArcProperties.ThetaRange; + 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), From 6c4def1c097298abbda31be5338ba791cd12dc1d Mon Sep 17 00:00:00 2001 From: Aurelian Date: Mon, 27 May 2024 20:32:18 +0200 Subject: [PATCH 08/10] Added check for infinite subpoints for PerfectCurve --- osu.Game/Rulesets/Objects/SliderPath.cs | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/osu.Game/Rulesets/Objects/SliderPath.cs b/osu.Game/Rulesets/Objects/SliderPath.cs index eca14269fe..aa2570c336 100644 --- a/osu.Game/Rulesets/Objects/SliderPath.cs +++ b/osu.Game/Rulesets/Objects/SliderPath.cs @@ -330,11 +330,18 @@ private List calculateSubPath(ReadOnlySpan subControlPoints, P if (subControlPoints.Length != 3) break; - //If a curve's theta range almost equals zero, the radius needed to have more than a - //floating point error difference is very large and results in a nearly straight path. - //Calculate it via a bezier aproximation instead. - //0.0005 corresponds with a radius of 8000 to have a more than 0.001 shift in the X value - if (Math.Abs(new CircularArcProperties(subControlPoints).ThetaRange) <= 0.0005d) + CircularArcProperties circularArcProperties = new CircularArcProperties(subControlPoints); + + //if false, we'll end up breaking anyways when calculating subPath + if (!circularArcProperties.IsValid) + break; + + //Coppied from PathApproximator.CircularArcToPiecewiseLinear + 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))))); + + //if the amount of subpoints is int.MaxValue, causes an out of memory issue, so we default to bezier + //this only occurs for very large radii, so the result should be essentially a straight line anyways + if (subPoints == int.MaxValue) break; List subPath = PathApproximator.CircularArcToPiecewiseLinear(subControlPoints); From 542809a748072f8438b9289027bafd018a33c5f3 Mon Sep 17 00:00:00 2001 From: Aurelian Date: Wed, 29 May 2024 09:39:46 +0200 Subject: [PATCH 09/10] Reduced subpoints limit to be a more practical value --- osu.Game/Rulesets/Objects/SliderPath.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/osu.Game/Rulesets/Objects/SliderPath.cs b/osu.Game/Rulesets/Objects/SliderPath.cs index aa2570c336..730a2013b0 100644 --- a/osu.Game/Rulesets/Objects/SliderPath.cs +++ b/osu.Game/Rulesets/Objects/SliderPath.cs @@ -339,9 +339,10 @@ private List calculateSubPath(ReadOnlySpan subControlPoints, P //Coppied from PathApproximator.CircularArcToPiecewiseLinear 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))))); - //if the amount of subpoints is int.MaxValue, causes an out of memory issue, so we default to bezier - //this only occurs for very large radii, so the result should be essentially a straight line anyways - if (subPoints == int.MaxValue) + //theoretically can be int.MaxValue, but lets set this to a lower value anyways + //1000 requires an arc length of over 20 thousand to surpass this limit, which should be safe. + //See here for calculations https://www.desmos.com/calculator/210bwswkbb + if (subPoints >= 1000) break; List subPath = PathApproximator.CircularArcToPiecewiseLinear(subControlPoints); From 9111da81d22c2a9e35533df964cb9e5c0e691807 Mon Sep 17 00:00:00 2001 From: Aurelian Date: Fri, 31 May 2024 01:30:26 +0200 Subject: [PATCH 10/10] Updated comments --- osu.Game/Rulesets/Objects/SliderPath.cs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/osu.Game/Rulesets/Objects/SliderPath.cs b/osu.Game/Rulesets/Objects/SliderPath.cs index 730a2013b0..5550815370 100644 --- a/osu.Game/Rulesets/Objects/SliderPath.cs +++ b/osu.Game/Rulesets/Objects/SliderPath.cs @@ -332,16 +332,15 @@ private List calculateSubPath(ReadOnlySpan subControlPoints, P CircularArcProperties circularArcProperties = new CircularArcProperties(subControlPoints); - //if false, we'll end up breaking anyways when calculating subPath + // `PathApproximator` will already internally revert to B-spline if the arc isn't valid. if (!circularArcProperties.IsValid) break; - //Coppied from PathApproximator.CircularArcToPiecewiseLinear + // 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))))); - //theoretically can be int.MaxValue, but lets set this to a lower value anyways - //1000 requires an arc length of over 20 thousand to surpass this limit, which should be safe. - //See here for calculations https://www.desmos.com/calculator/210bwswkbb + // 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;