Merge pull request #20586 from ekrctb/time-based-selection

Use hit object time for timeline drag selection instead of relying on blueprint

osu.Game.Tests/Visual/Editing/TestSceneTimelineSelection.cs

@@ -29,16 +29,18 @@ namespace osu.Game.Tests.Visual.Editing
         private TimelineBlueprintContainer blueprintContainer
             => Editor.ChildrenOfType<TimelineBlueprintContainer>().First();
 
+        private Vector2 getPosition(HitObject hitObject) =>
+            blueprintContainer.SelectionBlueprints.First(s => s.Item == hitObject).ScreenSpaceDrawQuad.Centre;
+
+        private Vector2 getMiddlePosition(HitObject hitObject1, HitObject hitObject2) =>
+            (getPosition(hitObject1) + getPosition(hitObject2)) / 2;
+
         private void moveMouseToObject(Func<HitObject> targetFunc)
         {
             AddStep("move mouse to object", () =>
             {
-                var pos = blueprintContainer.SelectionBlueprints
+                var hitObject = targetFunc();
-                                            .First(s => s.Item == targetFunc())
+                InputManager.MoveMouseTo(getPosition(hitObject));
-                                            .ChildrenOfType<TimelineHitObjectBlueprint>()
-                                            .First().ScreenSpaceDrawQuad.Centre;
-
-                InputManager.MoveMouseTo(pos);
             });
         }
 
@@ -262,6 +264,56 @@ namespace osu.Game.Tests.Visual.Editing
             AddStep("release shift", () => InputManager.ReleaseKey(Key.ShiftLeft));
         }
 
+        [Test]
+        public void TestBasicDragSelection()
+        {
+            var addedObjects = new[]
+            {
+                new HitCircle { StartTime = 0 },
+                new HitCircle { StartTime = 500, Position = new Vector2(100) },
+                new HitCircle { StartTime = 1000, Position = new Vector2(200) },
+                new HitCircle { StartTime = 1500, Position = new Vector2(300) },
+            };
+            AddStep("add hitobjects", () => EditorBeatmap.AddRange(addedObjects));
+
+            AddStep("move mouse", () => InputManager.MoveMouseTo(getMiddlePosition(addedObjects[0], addedObjects[1])));
+            AddStep("mouse down", () => InputManager.PressButton(MouseButton.Left));
+
+            AddStep("drag to select", () => InputManager.MoveMouseTo(getMiddlePosition(addedObjects[2], addedObjects[3])));
+            assertSelectionIs(new[] { addedObjects[1], addedObjects[2] });
+
+            AddStep("drag to deselect", () => InputManager.MoveMouseTo(getMiddlePosition(addedObjects[1], addedObjects[2])));
+            assertSelectionIs(new[] { addedObjects[1] });
+
+            AddStep("mouse up", () => InputManager.ReleaseButton(MouseButton.Left));
+            assertSelectionIs(new[] { addedObjects[1] });
+        }
+
+        [Test]
+        public void TestFastDragSelection()
+        {
+            var addedObjects = new[]
+            {
+                new HitCircle { StartTime = 0 },
+                new HitCircle { StartTime = 500 },
+                new HitCircle { StartTime = 20000, Position = new Vector2(100) },
+                new HitCircle { StartTime = 31000, Position = new Vector2(200) },
+                new HitCircle { StartTime = 60000, Position = new Vector2(300) },
+            };
+
+            AddStep("add hitobjects", () => EditorBeatmap.AddRange(addedObjects));
+
+            AddStep("move mouse", () => InputManager.MoveMouseTo(getMiddlePosition(addedObjects[0], addedObjects[1])));
+            AddStep("mouse down", () => InputManager.PressButton(MouseButton.Left));
+            AddStep("start drag", () => InputManager.MoveMouseTo(getPosition(addedObjects[1])));
+
+            AddStep("jump editor clock", () => EditorClock.Seek(30000));
+            AddStep("jump editor clock", () => EditorClock.Seek(60000));
+            AddStep("end drag", () => InputManager.ReleaseButton(MouseButton.Left));
+            assertSelectionIs(addedObjects.Skip(1));
+            AddAssert("all blueprints are present", () => blueprintContainer.SelectionBlueprints.Count == EditorBeatmap.SelectedHitObjects.Count);
+        }
+
         private void assertSelectionIs(IEnumerable<HitObject> hitObjects)
             => AddAssert("correct hitobjects selected", () => EditorBeatmap.SelectedHitObjects.OrderBy(h => h.StartTime).SequenceEqual(hitObjects));
     }

osu.Game/Screens/Edit/Compose/Components/BlueprintContainer.cs

@@ -3,7 +3,6 @@
 
 #nullable disable
 
-using System;
 using System.Collections.Generic;
 using System.Collections.Specialized;
 using System.Diagnostics;
@@ -13,11 +12,9 @@ using osu.Framework.Allocation;
 using osu.Framework.Bindables;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.Containers;
-using osu.Framework.Graphics.Primitives;
 using osu.Framework.Input;
 using osu.Framework.Input.Bindings;
 using osu.Framework.Input.Events;
-using osu.Game.Graphics.UserInterface;
 using osu.Game.Rulesets.Edit;
 using osuTK;
 using osuTK.Input;
@@ -61,25 +58,31 @@ namespace osu.Game.Screens.Edit.Compose.Components
                 {
                     case NotifyCollectionChangedAction.Add:
                         foreach (object o in args.NewItems)
-                            SelectionBlueprints.FirstOrDefault(b => b.Item == o)?.Select();
+                        {
+                            if (blueprintMap.TryGetValue((T)o, out var blueprint))
+                                blueprint.Select();
+                        }
 
                         break;
 
                     case NotifyCollectionChangedAction.Remove:
                         foreach (object o in args.OldItems)
-                            SelectionBlueprints.FirstOrDefault(b => b.Item == o)?.Deselect();
+                        {
+                            if (blueprintMap.TryGetValue((T)o, out var blueprint))
+                                blueprint.Deselect();
+                        }
 
                         break;
                 }
             };
 
             SelectionHandler = CreateSelectionHandler();
-            SelectionHandler.DeselectAll = deselectAll;
+            SelectionHandler.DeselectAll = DeselectAll;
             SelectionHandler.SelectedItems.BindTo(SelectedItems);
 
             AddRangeInternal(new[]
             {
-                DragBox = CreateDragBox(selectBlueprintsFromDragRectangle),
+                DragBox = CreateDragBox(),
                 SelectionHandler,
                 SelectionBlueprints = CreateSelectionBlueprintContainer(),
                 SelectionHandler.CreateProxy(),
@@ -101,7 +104,7 @@ namespace osu.Game.Screens.Edit.Compose.Components
         [CanBeNull]
         protected virtual SelectionBlueprint<T> CreateBlueprintFor(T item) => null;
 
-        protected virtual DragBox CreateDragBox(Action<RectangleF> performSelect) => new DragBox(performSelect);
+        protected virtual DragBox CreateDragBox() => new DragBox();
 
         /// <summary>
         /// Whether this component is in a state where items outside a drag selection should be deselected. If false, selection will only be added to.
@@ -142,7 +145,7 @@ namespace osu.Game.Screens.Edit.Compose.Components
             if (endClickSelection(e) || ClickedBlueprint != null)
                 return true;
 
-            deselectAll();
+            DeselectAll();
             return true;
         }
 
@@ -183,13 +186,9 @@ namespace osu.Game.Screens.Edit.Compose.Components
                 return true;
             }
 
-            if (DragBox.HandleDrag(e))
+            DragBox.HandleDrag(e);
-            {
+            DragBox.Show();
-                DragBox.Show();
+            return true;
-                return true;
-            }
-
-            return false;
         }
 
         protected override void OnDrag(DragEvent e)
@@ -198,7 +197,10 @@ namespace osu.Game.Screens.Edit.Compose.Components
                 return;
 
             if (DragBox.State == Visibility.Visible)
+            {
                 DragBox.HandleDrag(e);
+                UpdateSelectionFromDragBox();
+            }
 
             moveCurrentSelection(e);
         }
@@ -214,8 +216,7 @@ namespace osu.Game.Screens.Edit.Compose.Components
                 changeHandler?.EndChange();
             }
 
-            if (DragBox.State == Visibility.Visible)
+            DragBox.Hide();
-                DragBox.Hide();
         }
 
         /// <summary>
@@ -233,7 +234,7 @@ namespace osu.Game.Screens.Edit.Compose.Components
                     if (!SelectionHandler.SelectedBlueprints.Any())
                         return false;
 
-                    deselectAll();
+                    DeselectAll();
                     return true;
             }
 
@@ -380,44 +381,32 @@ namespace osu.Game.Screens.Edit.Compose.Components
         }
 
         /// <summary>
-        /// Select all masks in a given rectangle selection area.
+        /// Select all blueprints in a selection area specified by <see cref="DragBox"/>.
         /// </summary>
-        /// <param name="rect">The rectangle to perform a selection on in screen-space coordinates.</param>
+        protected virtual void UpdateSelectionFromDragBox()
-        private void selectBlueprintsFromDragRectangle(RectangleF rect)
         {
+            var quad = DragBox.Box.ScreenSpaceDrawQuad;
+
             foreach (var blueprint in SelectionBlueprints)
             {
-                // only run when utmost necessary to avoid unnecessary rect computations.
+                if (blueprint.IsSelected && !AllowDeselectionDuringDrag)
-                bool isValidForSelection() => blueprint.IsAlive && blueprint.IsPresent && rect.Contains(blueprint.ScreenSpaceSelectionPoint);
+                    continue;
 
-                switch (blueprint.State)
+                bool shouldBeSelected = blueprint.IsAlive && blueprint.IsPresent && quad.Contains(blueprint.ScreenSpaceSelectionPoint);
-                {
+                if (blueprint.IsSelected != shouldBeSelected)
-                    case SelectionState.NotSelected:
+                    blueprint.ToggleSelection();
-                        if (isValidForSelection())
-                            blueprint.Select();
-                        break;
-
-                    case SelectionState.Selected:
-                        if (AllowDeselectionDuringDrag && !isValidForSelection())
-                            blueprint.Deselect();
-                        break;
-                }
             }
         }
 
         /// <summary>
-        /// Selects all <see cref="SelectionBlueprint{T}"/>s.
+        /// Select all currently-present items.
         /// </summary>
-        protected virtual void SelectAll()
+        protected abstract void SelectAll();
-        {
-            // Scheduled to allow the change in lifetime to take place.
-            Schedule(() => SelectionBlueprints.ToList().ForEach(m => m.Select()));
-        }
 
         /// <summary>
-        /// Deselects all selected <see cref="SelectionBlueprint{T}"/>s.
+        /// Deselect all selected items.
         /// </summary>
-        private void deselectAll() => SelectionHandler.SelectedBlueprints.ToList().ForEach(m => m.Deselect());
+        protected void DeselectAll() => SelectedItems.Clear();
 
         protected virtual void OnBlueprintSelected(SelectionBlueprint<T> blueprint)
         {

osu.Game/Screens/Edit/Compose/Components/ComposeBlueprintContainer.cs

@@ -83,6 +83,8 @@ namespace osu.Game.Screens.Edit.Compose.Components
             }
         }
 
+        protected override bool AllowDeselectionDuringDrag => !EditorClock.IsRunning;
+
         protected override void TransferBlueprintFor(HitObject hitObject, DrawableHitObject drawableObject)
         {
             base.TransferBlueprintFor(hitObject, drawableObject);

osu.Game/Screens/Edit/Compose/Components/DragBox.cs

@@ -8,7 +8,6 @@ using osu.Framework;
 using osu.Framework.Allocation;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.Containers;
-using osu.Framework.Graphics.Primitives;
 using osu.Framework.Graphics.Shapes;
 using osu.Framework.Input.Events;
 using osu.Framework.Layout;
@@ -21,18 +20,13 @@ namespace osu.Game.Screens.Edit.Compose.Components
     /// </summary>
     public class DragBox : CompositeDrawable, IStateful<Visibility>
     {
-        protected readonly Action<RectangleF> PerformSelection;
+        public Drawable Box { get; private set; }
-
-        protected Drawable Box;
 
         /// <summary>
         /// Creates a new <see cref="DragBox"/>.
         /// </summary>
-        /// <param name="performSelection">A delegate that performs drag selection.</param>
+        public DragBox()
-        public DragBox(Action<RectangleF> performSelection)
         {
-            PerformSelection = performSelection;
-
             RelativeSizeAxes = Axes.Both;
             AlwaysPresent = true;
             Alpha = 0;
@@ -46,30 +40,14 @@ namespace osu.Game.Screens.Edit.Compose.Components
 
         protected virtual Drawable CreateBox() => new BoxWithBorders();
 
-        private RectangleF? dragRectangle;
-
         /// <summary>
         /// Handle a forwarded mouse event.
         /// </summary>
         /// <param name="e">The mouse event.</param>
-        /// <returns>Whether the event should be handled and blocking.</returns>
+        public virtual void HandleDrag(MouseButtonEvent e)
-        public virtual bool HandleDrag(MouseButtonEvent e)
         {
-            var dragPosition = e.ScreenSpaceMousePosition;
+            Box.Position = Vector2.ComponentMin(e.MouseDownPosition, e.MousePosition);
-            var dragStartPosition = e.ScreenSpaceMouseDownPosition;
+            Box.Size = Vector2.ComponentMax(e.MouseDownPosition, e.MousePosition) - Box.Position;
-
-            var dragQuad = new Quad(dragStartPosition.X, dragStartPosition.Y, dragPosition.X - dragStartPosition.X, dragPosition.Y - dragStartPosition.Y);
-
-            // We use AABBFloat instead of RectangleF since it handles negative sizes for us
-            var rec = dragQuad.AABBFloat;
-            dragRectangle = rec;
-
-            var topLeft = ToLocalSpace(rec.TopLeft);
-            var bottomRight = ToLocalSpace(rec.BottomRight);
-
-            Box.Position = topLeft;
-            Box.Size = bottomRight - topLeft;
-            return true;
         }
 
         private Visibility state;
@@ -87,19 +65,7 @@ namespace osu.Game.Screens.Edit.Compose.Components
             }
         }
 
-        protected override void Update()
+        public override void Hide() => State = Visibility.Hidden;
-        {
-            base.Update();
-
-            if (dragRectangle != null)
-                PerformSelection?.Invoke(dragRectangle.Value);
-        }
-
-        public override void Hide()
-        {
-            State = Visibility.Hidden;
-            dragRectangle = null;
-        }
 
         public override void Show() => State = Visibility.Visible;
 

osu.Game/Screens/Edit/Compose/Components/EditorBlueprintContainer.cs

@@ -66,8 +66,6 @@ namespace osu.Game.Screens.Edit.Compose.Components
         protected override IEnumerable<SelectionBlueprint<HitObject>> SortForMovement(IReadOnlyList<SelectionBlueprint<HitObject>> blueprints)
             => blueprints.OrderBy(b => b.Item.StartTime);
 
-        protected override bool AllowDeselectionDuringDrag => !EditorClock.IsRunning;
-
         protected override bool ApplySnapResult(SelectionBlueprint<HitObject>[] blueprints, SnapResult result)
         {
             if (!base.ApplySnapResult(blueprints, result))
@@ -133,8 +131,7 @@ namespace osu.Game.Screens.Edit.Compose.Components
         protected override void SelectAll()
         {
             Composer.Playfield.KeepAllAlive();
-
+            SelectedItems.AddRange(Beatmap.HitObjects.Except(SelectedItems).ToArray());
-            base.SelectAll();
         }
 
         protected override void OnBlueprintSelected(SelectionBlueprint<HitObject> blueprint)

osu.Game/Screens/Edit/Compose/Components/SelectionHandler.cs

@@ -305,7 +305,7 @@ namespace osu.Game.Screens.Edit.Compose.Components
 
         protected void DeleteSelected()
         {
-            DeleteItems(selectedBlueprints.Select(b => b.Item));
+            DeleteItems(SelectedItems.ToArray());
         }
 
         #endregion

osu.Game/Screens/Edit/Compose/Components/Timeline/Timeline.cs

@@ -304,10 +304,20 @@ namespace osu.Game.Screens.Edit.Compose.Components.Timeline
         /// </summary>
         public double VisibleRange => editorClock.TrackLength / Zoom;
 
-        public SnapResult FindSnappedPositionAndTime(Vector2 screenSpacePosition, SnapType snapType = SnapType.All) =>
+        public double TimeAtPosition(float x)
-            new SnapResult(screenSpacePosition, beatSnapProvider.SnapTime(getTimeFromPosition(Content.ToLocalSpace(screenSpacePosition))));
+        {
+            return x / Content.DrawWidth * editorClock.TrackLength;
+        }
 
-        private double getTimeFromPosition(Vector2 localPosition) =>
+        public float PositionAtTime(double time)
-            (localPosition.X / Content.DrawWidth) * editorClock.TrackLength;
+        {
+            return (float)(time / editorClock.TrackLength * Content.DrawWidth);
+        }
+
+        public SnapResult FindSnappedPositionAndTime(Vector2 screenSpacePosition, SnapType snapType = SnapType.All)
+        {
+            double time = TimeAtPosition(Content.ToLocalSpace(screenSpacePosition).X);
+            return new SnapResult(screenSpacePosition, beatSnapProvider.SnapTime(time));
+        }
     }
 }

osu.Game/Screens/Edit/Compose/Components/Timeline/TimelineBlueprintContainer.cs

@@ -3,7 +3,6 @@
 
 #nullable disable
 
-using System;
 using System.Collections.Generic;
 using System.Linq;
 using osu.Framework.Allocation;
@@ -13,7 +12,6 @@ using osu.Framework.Extensions.ObjectExtensions;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.Colour;
 using osu.Framework.Graphics.Containers;
-using osu.Framework.Graphics.Primitives;
 using osu.Framework.Graphics.Shapes;
 using osu.Framework.Input.Events;
 using osu.Framework.Utils;
@@ -65,7 +63,6 @@ namespace osu.Game.Screens.Edit.Compose.Components.Timeline
         protected override void LoadComplete()
         {
             base.LoadComplete();
-            DragBox.Alpha = 0;
 
             placement = Beatmap.PlacementObject.GetBoundCopy();
             placement.ValueChanged += placementChanged;
@@ -93,6 +90,14 @@ namespace osu.Game.Screens.Edit.Compose.Components.Timeline
 
         protected override Container<SelectionBlueprint<HitObject>> CreateSelectionBlueprintContainer() => new TimelineSelectionBlueprintContainer { RelativeSizeAxes = Axes.Both };
 
+        protected override bool OnDragStart(DragStartEvent e)
+        {
+            if (!base.ReceivePositionalInputAt(e.ScreenSpaceMouseDownPosition))
+                return false;
+
+            return base.OnDragStart(e);
+        }
+
         protected override void OnDrag(DragEvent e)
         {
             handleScrollViaDrag(e);
@@ -169,7 +174,28 @@ namespace osu.Game.Screens.Edit.Compose.Components.Timeline
             };
         }
 
-        protected override DragBox CreateDragBox(Action<RectangleF> performSelect) => new TimelineDragBox(performSelect);
+        protected sealed override DragBox CreateDragBox() => new TimelineDragBox();
+
+        protected override void UpdateSelectionFromDragBox()
+        {
+            var dragBox = (TimelineDragBox)DragBox;
+            double minTime = dragBox.MinTime;
+            double maxTime = dragBox.MaxTime;
+
+            SelectedItems.RemoveAll(hitObject => !shouldBeSelected(hitObject));
+
+            foreach (var hitObject in Beatmap.HitObjects.Except(SelectedItems).Where(shouldBeSelected))
+            {
+                Composer.Playfield.SetKeepAlive(hitObject, true);
+                SelectedItems.Add(hitObject);
+            }
+
+            bool shouldBeSelected(HitObject hitObject)
+            {
+                double midTime = (hitObject.StartTime + hitObject.GetEndTime()) / 2;
+                return minTime <= midTime && midTime <= maxTime;
+            }
+        }
 
         private void handleScrollViaDrag(DragEvent e)
         {

osu.Game/Screens/Edit/Compose/Components/Timeline/TimelineDragBox.cs

@@ -6,76 +6,44 @@
 using System;
 using osu.Framework.Allocation;
 using osu.Framework.Graphics;
-using osu.Framework.Graphics.Primitives;
 using osu.Framework.Graphics.Shapes;
 using osu.Framework.Input.Events;
-using osu.Framework.Utils;
 
 namespace osu.Game.Screens.Edit.Compose.Components.Timeline
 {
     public class TimelineDragBox : DragBox
     {
-        // the following values hold the start and end X positions of the drag box in the timeline's local space,
+        public double MinTime { get; private set; }
-        // but with zoom unapplied in order to be able to compensate for positional changes
+
-        // while the timeline is being zoomed in/out.
+        public double MaxTime { get; private set; }
-        private float? selectionStart;
+
-        private float selectionEnd;
+        private double? startTime;
 
         [Resolved]
         private Timeline timeline { get; set; }
 
-        public TimelineDragBox(Action<RectangleF> performSelect)
-            : base(performSelect)
-        {
-        }
-
         protected override Drawable CreateBox() => new Box
         {
             RelativeSizeAxes = Axes.Y,
             Alpha = 0.3f
         };
 
-        public override bool HandleDrag(MouseButtonEvent e)
+        public override void HandleDrag(MouseButtonEvent e)
         {
-            // The dragbox should only be active if the mouseDownPosition.Y is within this drawable's bounds.
+            startTime ??= timeline.TimeAtPosition(e.MouseDownPosition.X);
-            float localY = ToLocalSpace(e.ScreenSpaceMouseDownPosition).Y;
+            double endTime = timeline.TimeAtPosition(e.MousePosition.X);
-            if (DrawRectangle.Top > localY || DrawRectangle.Bottom < localY)
-                return false;
 
-            selectionStart ??= e.MouseDownPosition.X / timeline.CurrentZoom;
+            MinTime = Math.Min(startTime.Value, endTime);
+            MaxTime = Math.Max(startTime.Value, endTime);
 
-            // only calculate end when a transition is not in progress to avoid bouncing.
+            Box.X = timeline.PositionAtTime(MinTime);
-            if (Precision.AlmostEquals(timeline.CurrentZoom, timeline.Zoom))
+            Box.Width = timeline.PositionAtTime(MaxTime) - Box.X;
-                selectionEnd = e.MousePosition.X / timeline.CurrentZoom;
-
-            updateDragBoxPosition();
-            return true;
-        }
-
-        private void updateDragBoxPosition()
-        {
-            if (selectionStart == null)
-                return;
-
-            float rescaledStart = selectionStart.Value * timeline.CurrentZoom;
-            float rescaledEnd = selectionEnd * timeline.CurrentZoom;
-
-            Box.X = Math.Min(rescaledStart, rescaledEnd);
-            Box.Width = Math.Abs(rescaledStart - rescaledEnd);
-
-            var boxScreenRect = Box.ScreenSpaceDrawQuad.AABBFloat;
-
-            // we don't care about where the hitobjects are vertically. in cases like stacking display, they may be outside the box without this adjustment.
-            boxScreenRect.Y -= boxScreenRect.Height;
-            boxScreenRect.Height *= 2;
-
-            PerformSelection?.Invoke(boxScreenRect);
         }
 
         public override void Hide()
         {
             base.Hide();
-            selectionStart = null;
+            startTime = null;
         }
     }
 }

osu.Game/Screens/Edit/EditorBeatmap.cs

@@ -352,6 +352,8 @@ namespace osu.Game.Screens.Edit
             var updates = batchPendingUpdates.ToArray();
             batchPendingUpdates.Clear();
 
+            foreach (var h in deletes) SelectedHitObjects.Remove(h);
+
             foreach (var h in deletes) HitObjectRemoved?.Invoke(h);
             foreach (var h in inserts) HitObjectAdded?.Invoke(h);
             foreach (var h in updates) HitObjectUpdated?.Invoke(h);

osu.Game/Skinning/Editor/SkinBlueprintContainer.cs

@@ -117,6 +117,11 @@ namespace osu.Game.Skinning.Editor
             return false;
         }
 
+        protected override void SelectAll()
+        {
+            SelectedItems.AddRange(targetComponents.SelectMany(list => list).Except(SelectedItems).ToArray());
+        }
+
         /// <summary>
         /// Move the current selection spatially by the specified delta, in screen coordinates (ie. the same coordinates as the blueprints).
         /// </summary>