Merge pull request #12122 from Naxesss/circular-arc-freeze

Fix freezes due to large circular arc radius

osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderControlPointPiece.cs

@@ -0,0 +1,175 @@
+// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
+// See the LICENCE file in the repository root for full licence text.
+
+using NUnit.Framework;
+using osu.Framework.Utils;
+using osu.Game.Beatmaps;
+using osu.Game.Beatmaps.ControlPoints;
+using osu.Game.Rulesets.Objects;
+using osu.Game.Rulesets.Objects.Types;
+using osu.Game.Rulesets.Osu.Edit.Blueprints.HitCircles.Components;
+using osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders;
+using osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components;
+using osu.Game.Rulesets.Osu.Objects;
+using osu.Game.Rulesets.Osu.Objects.Drawables;
+using osu.Game.Tests.Visual;
+using osuTK;
+using osuTK.Input;
+
+namespace osu.Game.Rulesets.Osu.Tests.Editor
+{
+    public class TestSceneSliderControlPointPiece : SelectionBlueprintTestScene
+    {
+        private Slider slider;
+        private DrawableSlider drawableObject;
+
+        [SetUp]
+        public void Setup() => Schedule(() =>
+        {
+            Clear();
+
+            slider = new Slider
+            {
+                Position = new Vector2(256, 192),
+                Path = new SliderPath(new[]
+                {
+                    new PathControlPoint(Vector2.Zero, PathType.PerfectCurve),
+                    new PathControlPoint(new Vector2(150, 150)),
+                    new PathControlPoint(new Vector2(300, 0), PathType.PerfectCurve),
+                    new PathControlPoint(new Vector2(400, 0)),
+                    new PathControlPoint(new Vector2(400, 150))
+                })
+            };
+
+            slider.ApplyDefaults(new ControlPointInfo(), new BeatmapDifficulty { CircleSize = 2 });
+
+            Add(drawableObject = new DrawableSlider(slider));
+            AddBlueprint(new TestSliderBlueprint(drawableObject));
+        });
+
+        [Test]
+        public void TestDragControlPoint()
+        {
+            moveMouseToControlPoint(1);
+            AddStep("hold", () => InputManager.PressButton(MouseButton.Left));
+
+            addMovementStep(new Vector2(150, 50));
+            AddStep("release", () => InputManager.ReleaseButton(MouseButton.Left));
+
+            assertControlPointPosition(1, new Vector2(150, 50));
+            assertControlPointType(0, PathType.PerfectCurve);
+        }
+
+        [Test]
+        public void TestDragControlPointAlmostLinearlyExterior()
+        {
+            moveMouseToControlPoint(1);
+            AddStep("hold", () => InputManager.PressButton(MouseButton.Left));
+
+            addMovementStep(new Vector2(400, 0.01f));
+            AddStep("release", () => InputManager.ReleaseButton(MouseButton.Left));
+
+            assertControlPointPosition(1, new Vector2(400, 0.01f));
+            assertControlPointType(0, PathType.Bezier);
+        }
+
+        [Test]
+        public void TestDragControlPointPathRecovery()
+        {
+            moveMouseToControlPoint(1);
+            AddStep("hold", () => InputManager.PressButton(MouseButton.Left));
+
+            addMovementStep(new Vector2(400, 0.01f));
+            assertControlPointType(0, PathType.Bezier);
+
+            addMovementStep(new Vector2(150, 50));
+            AddStep("release", () => InputManager.ReleaseButton(MouseButton.Left));
+
+            assertControlPointPosition(1, new Vector2(150, 50));
+            assertControlPointType(0, PathType.PerfectCurve);
+        }
+
+        [Test]
+        public void TestDragControlPointPathRecoveryOtherSegment()
+        {
+            moveMouseToControlPoint(4);
+            AddStep("hold", () => InputManager.PressButton(MouseButton.Left));
+
+            addMovementStep(new Vector2(350, 0.01f));
+            assertControlPointType(2, PathType.Bezier);
+
+            addMovementStep(new Vector2(150, 150));
+            AddStep("release", () => InputManager.ReleaseButton(MouseButton.Left));
+
+            assertControlPointPosition(4, new Vector2(150, 150));
+            assertControlPointType(2, PathType.PerfectCurve);
+        }
+
+        [Test]
+        public void TestDragControlPointPathAfterChangingType()
+        {
+            AddStep("change type to bezier", () => slider.Path.ControlPoints[2].Type.Value = PathType.Bezier);
+            AddStep("add point", () => slider.Path.ControlPoints.Add(new PathControlPoint(new Vector2(500, 10))));
+            AddStep("change type to perfect", () => slider.Path.ControlPoints[3].Type.Value = PathType.PerfectCurve);
+
+            moveMouseToControlPoint(4);
+            AddStep("hold", () => InputManager.PressButton(MouseButton.Left));
+
+            assertControlPointType(3, PathType.PerfectCurve);
+
+            addMovementStep(new Vector2(350, 0.01f));
+            AddStep("release", () => InputManager.ReleaseButton(MouseButton.Left));
+
+            assertControlPointPosition(4, new Vector2(350, 0.01f));
+            assertControlPointType(3, PathType.Bezier);
+        }
+
+        private void addMovementStep(Vector2 relativePosition)
+        {
+            AddStep($"move mouse to {relativePosition}", () =>
+            {
+                Vector2 position = slider.Position + relativePosition;
+                InputManager.MoveMouseTo(drawableObject.Parent.ToScreenSpace(position));
+            });
+        }
+
+        private void moveMouseToControlPoint(int index)
+        {
+            AddStep($"move mouse to control point {index}", () =>
+            {
+                Vector2 position = slider.Position + slider.Path.ControlPoints[index].Position.Value;
+                InputManager.MoveMouseTo(drawableObject.Parent.ToScreenSpace(position));
+            });
+        }
+
+        private void assertControlPointType(int index, PathType type) => AddAssert($"control point {index} is {type}", () => slider.Path.ControlPoints[index].Type.Value == type);
+
+        private void assertControlPointPosition(int index, Vector2 position) =>
+            AddAssert($"control point {index} at {position}", () => Precision.AlmostEquals(position, slider.Path.ControlPoints[index].Position.Value, 1));
+
+        private class TestSliderBlueprint : SliderSelectionBlueprint
+        {
+            public new SliderBodyPiece BodyPiece => base.BodyPiece;
+            public new TestSliderCircleBlueprint HeadBlueprint => (TestSliderCircleBlueprint)base.HeadBlueprint;
+            public new TestSliderCircleBlueprint TailBlueprint => (TestSliderCircleBlueprint)base.TailBlueprint;
+            public new PathControlPointVisualiser ControlPointVisualiser => base.ControlPointVisualiser;
+
+            public TestSliderBlueprint(DrawableSlider slider)
+                : base(slider)
+            {
+            }
+
+            protected override SliderCircleSelectionBlueprint CreateCircleSelectionBlueprint(DrawableSlider slider, SliderPosition position) => new TestSliderCircleBlueprint(slider, position);
+        }
+
+        private class TestSliderCircleBlueprint : SliderCircleSelectionBlueprint
+        {
+            public new HitCirclePiece CirclePiece => base.CirclePiece;
+
+            public TestSliderCircleBlueprint(DrawableSlider slider, SliderPosition position)
+                : base(slider, position)
+            {
+            }
+        }
+    }
+}

osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderPlacementBlueprint.cs

@@ -276,6 +276,104 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor
             assertControlPointType(0, PathType.Linear);
         }
 
+        [Test]
+        public void TestPlacePerfectCurveSegmentAlmostLinearlyExterior()
+        {
+            Vector2 startPosition = new Vector2(200);
+
+            addMovementStep(startPosition);
+            addClickStep(MouseButton.Left);
+
+            addMovementStep(startPosition + new Vector2(300, 0));
+            addClickStep(MouseButton.Left);
+
+            addMovementStep(startPosition + new Vector2(150, 0.1f));
+            addClickStep(MouseButton.Right);
+
+            assertPlaced(true);
+            assertControlPointCount(3);
+            assertControlPointType(0, PathType.Bezier);
+        }
+
+        [Test]
+        public void TestPlacePerfectCurveSegmentRecovery()
+        {
+            Vector2 startPosition = new Vector2(200);
+
+            addMovementStep(startPosition);
+            addClickStep(MouseButton.Left);
+
+            addMovementStep(startPosition + new Vector2(300, 0));
+            addClickStep(MouseButton.Left);
+
+            addMovementStep(startPosition + new Vector2(150, 0.1f)); // Should convert to bezier
+            addMovementStep(startPosition + new Vector2(400.0f, 50.0f)); // Should convert back to perfect
+            addClickStep(MouseButton.Right);
+
+            assertPlaced(true);
+            assertControlPointCount(3);
+            assertControlPointType(0, PathType.PerfectCurve);
+        }
+
+        [Test]
+        public void TestPlacePerfectCurveSegmentLarge()
+        {
+            Vector2 startPosition = new Vector2(400);
+
+            addMovementStep(startPosition);
+            addClickStep(MouseButton.Left);
+
+            addMovementStep(startPosition + new Vector2(220, 220));
+            addClickStep(MouseButton.Left);
+
+            // Playfield dimensions are 640 x 480.
+            // So a 440 x 440 bounding box should be ok.
+            addMovementStep(startPosition + new Vector2(-220, 220));
+            addClickStep(MouseButton.Right);
+
+            assertPlaced(true);
+            assertControlPointCount(3);
+            assertControlPointType(0, PathType.PerfectCurve);
+        }
+
+        [Test]
+        public void TestPlacePerfectCurveSegmentTooLarge()
+        {
+            Vector2 startPosition = new Vector2(480, 200);
+
+            addMovementStep(startPosition);
+            addClickStep(MouseButton.Left);
+
+            addMovementStep(startPosition + new Vector2(400, 400));
+            addClickStep(MouseButton.Left);
+
+            // Playfield dimensions are 640 x 480.
+            // So an 800 * 800 bounding box area should not be ok.
+            addMovementStep(startPosition + new Vector2(-400, 400));
+            addClickStep(MouseButton.Right);
+
+            assertPlaced(true);
+            assertControlPointCount(3);
+            assertControlPointType(0, PathType.Bezier);
+        }
+
+        [Test]
+        public void TestPlacePerfectCurveSegmentCompleteArc()
+        {
+            addMovementStep(new Vector2(400));
+            addClickStep(MouseButton.Left);
+
+            addMovementStep(new Vector2(600, 400));
+            addClickStep(MouseButton.Left);
+
+            addMovementStep(new Vector2(400, 410));
+            addClickStep(MouseButton.Right);
+
+            assertPlaced(true);
+            assertControlPointCount(3);
+            assertControlPointType(0, PathType.PerfectCurve);
+        }
+
         private void addMovementStep(Vector2 position) => AddStep($"move mouse to {position}", () => InputManager.MoveMouseTo(InputManager.ToScreenSpace(position)));
 
         private void addClickStep(MouseButton button)

osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointPiece.cs

@@ -2,14 +2,18 @@
 // See the LICENCE file in the repository root for full licence text.
 
 using System;
+using System.Collections.Generic;
+using System.Linq;
 using osu.Framework.Allocation;
 using osu.Framework.Bindables;
 using osu.Framework.Extensions.Color4Extensions;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.Containers;
 using osu.Framework.Graphics.Cursor;
+using osu.Framework.Graphics.Primitives;
 using osu.Framework.Graphics.Shapes;
 using osu.Framework.Input.Events;
+using osu.Framework.Utils;
 using osu.Game.Graphics;
 using osu.Game.Rulesets.Edit;
 using osu.Game.Rulesets.Objects;
@@ -28,6 +32,7 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components
     public class PathControlPointPiece : BlueprintPiece<Slider>, IHasTooltip
     {
         public Action<PathControlPointPiece, MouseButtonEvent> RequestSelection;
+        public List<PathControlPoint> PointsInSegment;
 
         public readonly BindableBool IsSelected = new BindableBool();
         public readonly PathControlPoint ControlPoint;
@@ -54,6 +59,12 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components
             this.slider = slider;
             ControlPoint = controlPoint;
 
+            slider.Path.Version.BindValueChanged(_ =>
+            {
+                PointsInSegment = slider.Path.PointsInSegment(ControlPoint);
+                updatePathType();
+            }, runOnceImmediately: true);
+
             controlPoint.Type.BindValueChanged(_ => updateMarkerDisplay());
 
             Origin = Anchor.Centre;
@@ -150,6 +161,7 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components
         protected override bool OnClick(ClickEvent e) => RequestSelection != null;
 
         private Vector2 dragStartPosition;
+        private PathType? dragPathType;
 
         protected override bool OnDragStart(DragStartEvent e)
         {
@@ -159,6 +171,8 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components
             if (e.Button == MouseButton.Left)
             {
                 dragStartPosition = ControlPoint.Position.Value;
+                dragPathType = PointsInSegment[0].Type.Value;
+
                 changeHandler?.BeginChange();
                 return true;
             }
@@ -184,10 +198,30 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components
             }
             else
                 ControlPoint.Position.Value = dragStartPosition + (e.MousePosition - e.MouseDownPosition);
+
+            // Maintain the path type in case it got defaulted to bezier at some point during the drag.
+            PointsInSegment[0].Type.Value = dragPathType;
         }
 
         protected override void OnDragEnd(DragEndEvent e) => changeHandler?.EndChange();
 
+        /// <summary>
+        /// Handles correction of invalid path types.
+        /// </summary>
+        private void updatePathType()
+        {
+            if (ControlPoint.Type.Value != PathType.PerfectCurve)
+                return;
+
+            ReadOnlySpan<Vector2> points = PointsInSegment.Select(p => p.Position.Value).ToArray();
+            if (points.Length != 3)
+                return;
+
+            RectangleF boundingBox = PathApproximator.CircularArcBoundingBox(points);
+            if (boundingBox.Width >= 640 || boundingBox.Height >= 480)
+                ControlPoint.Type.Value = PathType.Bezier;
+        }
+
         /// <summary>
         /// Updates the state of the circular control point marker.
         /// </summary>

osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderPlacementBlueprint.cs

@@ -142,6 +142,9 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders
         {
             base.Update();
             updateSlider();
+
+            // Maintain the path type in case it got defaulted to bezier at some point during the drag.
+            updatePathType();
         }
 
         private void updatePathType()

osu.Game.Rulesets.Osu/Edit/OsuSelectionHandler.cs

@@ -33,6 +33,7 @@ namespace osu.Game.Rulesets.Osu.Edit
         {
             base.OnOperationEnded();
             referenceOrigin = null;
+            referencePathTypes = null;
         }
 
         public override bool HandleMovement(MoveSelectionEvent moveEvent)
@@ -53,6 +54,12 @@ namespace osu.Game.Rulesets.Osu.Edit
         /// </summary>
         private Vector2? referenceOrigin;
 
+        /// <summary>
+        /// During a transform, the initial path types of a single selected slider are stored so they
+        /// can be maintained throughout the operation.
+        /// </summary>
+        private List<PathType?> referencePathTypes;
+
         public override bool HandleReverse()
         {
             var hitObjects = EditorBeatmap.SelectedHitObjects;
@@ -194,6 +201,8 @@ namespace osu.Game.Rulesets.Osu.Edit
 
         private void scaleSlider(Slider slider, Vector2 scale)
         {
+            referencePathTypes ??= slider.Path.ControlPoints.Select(p => p.Type.Value).ToList();
+
             Quad sliderQuad = getSurroundingQuad(slider.Path.ControlPoints.Select(p => p.Position.Value));
 
             // Limit minimum distance between control points  after scaling to almost 0. Less than 0 causes the slider to flip, exactly 0 causes a crash through division by 0.
@@ -209,6 +218,10 @@ namespace osu.Game.Rulesets.Osu.Edit
                 point.Position.Value *= pathRelativeDeltaScale;
             }
 
+            // Maintain the path types in case they were defaulted to bezier at some point during scaling
+            for (int i = 0; i < slider.Path.ControlPoints.Count; ++i)
+                slider.Path.ControlPoints[i].Type.Value = referencePathTypes[i];
+
             //if sliderhead or sliderend end up outside playfield, revert scaling.
             Quad scaledQuad = getSurroundingQuad(new OsuHitObject[] { slider });
             (bool xInBounds, bool yInBounds) = isQuadInBounds(scaledQuad);

osu.Game/Rulesets/Objects/SliderPath.cs

@@ -156,6 +156,39 @@ namespace osu.Game.Rulesets.Objects
             return interpolateVertices(indexOfDistance(d), d);
         }
 
+        /// <summary>
+        /// Returns the control points belonging to the same segment as the one given.
+        /// The first point has a PathType which all other points inherit.
+        /// </summary>
+        /// <param name="controlPoint">One of the control points in the segment.</param>
+        /// <returns></returns>
+        public List<PathControlPoint> PointsInSegment(PathControlPoint controlPoint)
+        {
+            bool found = false;
+            List<PathControlPoint> pointsInCurrentSegment = new List<PathControlPoint>();
+
+            foreach (PathControlPoint point in ControlPoints)
+            {
+                if (point.Type.Value != null)
+                {
+                    if (!found)
+                        pointsInCurrentSegment.Clear();
+                    else
+                    {
+                        pointsInCurrentSegment.Add(point);
+                        break;
+                    }
+                }
+
+                pointsInCurrentSegment.Add(point);
+
+                if (point == controlPoint)
+                    found = true;
+            }
+
+            return pointsInCurrentSegment;
+        }
+
         private void invalidate()
         {
             pathCache.Invalidate();