Rewrite ControlPointTable to use virtualised list

osu.Game/Screens/Edit/EditorTable.cs

@@ -4,15 +4,11 @@
 using System;
 using System.Diagnostics;
 using osu.Framework.Allocation;
-using osu.Framework.Extensions.LocalisationExtensions;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.Containers;
 using osu.Framework.Graphics.Shapes;
 using osu.Framework.Input.Events;
-using osu.Framework.Localisation;
-using osu.Game.Graphics;
 using osu.Game.Graphics.Containers;
-using osu.Game.Graphics.Sprites;
 using osu.Game.Overlays;
 using osuTK.Graphics;
 
@@ -89,16 +85,7 @@ namespace osu.Game.Screens.Edit
             return BackgroundFlow[index].Item;
         }
 
-        protected override Drawable CreateHeader(int index, TableColumn? column) => new HeaderText(column?.Header ?? default);
+        protected override Drawable CreateHeader(int index, TableColumn? column) => new TableHeaderText(column?.Header ?? default);
-
-        private partial class HeaderText : OsuSpriteText
-        {
-            public HeaderText(LocalisableString text)
-            {
-                Text = text.ToUpper();
-                Font = OsuFont.GetFont(size: 12, weight: FontWeight.Bold);
-            }
-        }
 
         public partial class RowBackground : OsuClickableContainer
         {

osu.Game/Screens/Edit/TableHeaderText.cs

@@ -0,0 +1,19 @@
+// 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 osu.Framework.Extensions.LocalisationExtensions;
+using osu.Framework.Localisation;
+using osu.Game.Graphics;
+using osu.Game.Graphics.Sprites;
+
+namespace osu.Game.Screens.Edit
+{
+    public partial class TableHeaderText : OsuSpriteText
+    {
+        public TableHeaderText(LocalisableString text)
+        {
+            Text = text.ToUpper();
+            Font = OsuFont.GetFont(size: 12, weight: FontWeight.Bold);
+        }
+    }
+}

osu.Game/Screens/Edit/Timing/ControlPointList.cs

@@ -7,10 +7,8 @@ using osu.Framework.Allocation;
 using osu.Framework.Bindables;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.Containers;
-using osu.Framework.Graphics.Shapes;
 using osu.Framework.Input.Events;
 using osu.Game.Beatmaps.ControlPoints;
-using osu.Game.Graphics.Containers;
 using osu.Game.Graphics.UserInterface;
 using osu.Game.Graphics.UserInterfaceV2;
 using osu.Game.Overlays;
@@ -21,12 +19,8 @@ namespace osu.Game.Screens.Edit.Timing
     public partial class ControlPointList : CompositeDrawable
     {
         private OsuButton deleteButton = null!;
-        private ControlPointTable table = null!;
-        private OsuScrollContainer scroll = null!;
         private RoundedButton addButton = null!;
 
-        private readonly IBindableList<ControlPointGroup> controlPointGroups = new BindableList<ControlPointGroup>();
-
         [Resolved]
         private EditorClock clock { get; set; } = null!;
 
@@ -36,9 +30,6 @@ namespace osu.Game.Screens.Edit.Timing
         [Resolved]
         private Bindable<ControlPointGroup?> selectedGroup { get; set; } = null!;
 
-        [Resolved]
-        private IEditorChangeHandler? changeHandler { get; set; }
-
         [BackgroundDependencyLoader]
         private void load(OverlayColourProvider colours)
         {
@@ -47,21 +38,10 @@ namespace osu.Game.Screens.Edit.Timing
             const float margins = 10;
             InternalChildren = new Drawable[]
             {
-                new Box
+                new ControlPointTable
-                {
-                    Colour = colours.Background4,
-                    RelativeSizeAxes = Axes.Both,
-                },
-                new Box
-                {
-                    Colour = colours.Background3,
-                    RelativeSizeAxes = Axes.Y,
-                    Width = ControlPointTable.TIMING_COLUMN_WIDTH + margins,
-                },
-                scroll = new OsuScrollContainer
                 {
                     RelativeSizeAxes = Axes.Both,
-                    Child = table = new ControlPointTable(),
+                    Groups = { BindTarget = Beatmap.ControlPointInfo.Groups, },
                 },
                 new FillFlowContainer
                 {
@@ -106,19 +86,7 @@ namespace osu.Game.Screens.Edit.Timing
                     : "+ Add at current time";
             }, true);
 
-            controlPointGroups.BindTo(Beatmap.ControlPointInfo.Groups);
+            //table.OnRowSelected += drawable => scroll.ScrollIntoView(drawable);
-            controlPointGroups.BindCollectionChanged((_, _) =>
-            {
-                // This callback can happen many times in a change operation. It gets expensive.
-                // We really should be handling the `CollectionChanged` event properly.
-                Scheduler.AddOnce(() =>
-                {
-                    table.ControlGroups = controlPointGroups;
-                    changeHandler?.SaveState();
-                });
-            }, true);
-
-            table.OnRowSelected += drawable => scroll.ScrollIntoView(drawable);
         }
 
         protected override bool OnClick(ClickEvent e)

osu.Game/Screens/Edit/Timing/ControlPointTable.cs

@@ -2,149 +2,270 @@
 // 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;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.Containers;
+using osu.Framework.Graphics.Pooling;
+using osu.Framework.Graphics.Shapes;
+using osu.Framework.Graphics.UserInterface;
+using osu.Framework.Input.Events;
+using osu.Framework.Localisation;
 using osu.Game.Beatmaps.ControlPoints;
 using osu.Game.Extensions;
 using osu.Game.Graphics;
+using osu.Game.Graphics.Containers;
 using osu.Game.Graphics.Sprites;
+using osu.Game.Overlays;
 using osu.Game.Screens.Edit.Timing.RowAttributes;
 using osuTK;
 
 namespace osu.Game.Screens.Edit.Timing
 {
-    public partial class ControlPointTable : EditorTable
+    public partial class ControlPointTable : CompositeDrawable
     {
-        [Resolved]
+        public BindableList<ControlPointGroup> Groups { get; } = new BindableList<ControlPointGroup>();
-        private Bindable<ControlPointGroup?> selectedGroup { get; set; } = null!;
 
-        [Resolved]
+        private const float timing_column_width = 300;
-        private EditorClock clock { get; set; } = null!;
+        private const float row_height = 25;
+        private const float row_horizontal_padding = 20;
 
-        public const float TIMING_COLUMN_WIDTH = 300;
+        [BackgroundDependencyLoader]
-
+        private void load(OverlayColourProvider colours)
-        public IEnumerable<ControlPointGroup> ControlGroups
         {
-            set
+            RelativeSizeAxes = Axes.Both;
+
+            InternalChildren = new Drawable[]
             {
-                int selectedIndex = GetIndexForObject(selectedGroup.Value);
+                new Box
-
-                Content = null;
-                BackgroundFlow.Clear();
-
-                if (!value.Any())
-                    return;
-
-                foreach (var group in value)
                 {
-                    BackgroundFlow.Add(new RowBackground(group)
+                    Colour = colours.Background4,
+                    RelativeSizeAxes = Axes.Both,
+                },
+                new Box
+                {
+                    Colour = colours.Background3,
+                    RelativeSizeAxes = Axes.Y,
+                    Width = timing_column_width + 10,
+                },
+                new Container
+                {
+                    RelativeSizeAxes = Axes.X,
+                    Height = row_height,
+                    Padding = new MarginPadding { Horizontal = row_horizontal_padding },
+                    Children = new Drawable[]
                     {
-                        // schedule to give time for any modified focused text box to lose focus and commit changes (e.g. BPM / time signature textboxes) before switching to new point.
+                        new TableHeaderText("Time")
-                        Action = () => Schedule(() =>
                         {
-                            SetSelectedRow(group);
+                            Anchor = Anchor.CentreLeft,
-                            clock.SeekSmoothlyTo(group.Time);
+                            Origin = Anchor.CentreLeft,
-                        })
+                        },
-                    });
+                        new TableHeaderText("Attributes")
-                }
+                        {
+                            Anchor = Anchor.CentreLeft,
+                            Origin = Anchor.CentreLeft,
+                            Margin = new MarginPadding { Left = ControlPointTable.timing_column_width }
+                        },
+                    }
+                },
+                new Container
+                {
+                    RelativeSizeAxes = Axes.Both,
+                    Padding = new MarginPadding { Top = row_height },
+                    Child = new ControlPointRowList
+                    {
+                        RelativeSizeAxes = Axes.Both,
+                        RowData = { BindTarget = Groups, },
+                    },
+                },
+            };
+        }
 
-                Columns = createHeaders();
+        private partial class ControlPointRowList : VirtualisedListContainer<ControlPointGroup, DrawableControlGroup>
-                Content = value.Select(createContent).ToArray().ToRectangular();
+        {
+            [Resolved]
+            private Bindable<ControlPointGroup?> selectedGroup { get; set; } = null!;
 
-                // Attempt to retain selection.
+            public ControlPointRowList()
-                if (SetSelectedRow(selectedGroup.Value))
+                : base(row_height, 50)
-                    return;
+            {
+            }
 
-                // Some operations completely obliterate references, so best-effort reselect based on index.
+            protected override ScrollContainer<Drawable> CreateScrollContainer() => new OsuScrollContainer();
-                if (SetSelectedRow(GetObjectAtIndex(selectedIndex)))
-                    return;
 
-                // Selection could not be retained.
+            protected override void LoadComplete()
-                selectedGroup.Value = null;
+            {
+                base.LoadComplete();
+
+                selectedGroup.BindValueChanged(val =>
+                {
+                    var row = Items.FlowingChildren.SingleOrDefault(item => item.Row.Equals(val.NewValue));
+                    if (row != null)
+                        Scroll.ScrollIntoView(row);
+                });
             }
         }
 
-        protected override void LoadComplete()
+        public partial class DrawableControlGroup : PoolableDrawable, IHasCurrentValue<ControlPointGroup>
         {
-            base.LoadComplete();
+            public Bindable<ControlPointGroup> Current
-
-            // Handle external selections.
-            selectedGroup.BindValueChanged(g => SetSelectedRow(g.NewValue), true);
-        }
-
-        protected override bool SetSelectedRow(object? item)
-        {
-            if (!base.SetSelectedRow(item))
-                return false;
-
-            selectedGroup.Value = item as ControlPointGroup;
-            return true;
-        }
-
-        private TableColumn[] createHeaders()
-        {
-            var columns = new List<TableColumn>
             {
-                new TableColumn("Time", Anchor.CentreLeft, new Dimension(GridSizeMode.Absolute, TIMING_COLUMN_WIDTH)),
+                get => current.Current;
-                new TableColumn("Attributes", Anchor.CentreLeft),
+                set => current.Current = value;
-            };
+            }
 
-            return columns.ToArray();
+            private readonly BindableWithCurrent<ControlPointGroup> current = new BindableWithCurrent<ControlPointGroup>();
-        }
 
-        private Drawable[] createContent(ControlPointGroup group)
+            private Box background = null!;
-        {
+
-            return new Drawable[]
+            [Resolved]
+            private OverlayColourProvider colourProvider { get; set; } = null!;
+
+            [Resolved]
+            private Bindable<ControlPointGroup?> selectedGroup { get; set; } = null!;
+
+            [Resolved]
+            private EditorClock editorClock { get; set; } = null!;
+
+            [BackgroundDependencyLoader]
+            private void load()
             {
-                new ControlGroupTiming(group),
+                RelativeSizeAxes = Axes.Both;
-                new ControlGroupAttributes(group, c => c is not TimingControlPoint)
+
-            };
+                InternalChildren = new Drawable[]
+                {
+                    background = new Box
+                    {
+                        RelativeSizeAxes = Axes.Both,
+                        Colour = colourProvider.Background1,
+                        Alpha = 0,
+                    },
+                    new Container
+                    {
+                        RelativeSizeAxes = Axes.Both,
+                        Padding = new MarginPadding { Horizontal = row_horizontal_padding, },
+                        Children = new Drawable[]
+                        {
+                            new ControlGroupTiming { Group = { BindTarget = current }, },
+                            new ControlGroupAttributes(point => point is not TimingControlPoint)
+                            {
+                                Group = { BindTarget = current },
+                                Margin = new MarginPadding { Left = timing_column_width }
+                            }
+                        }
+                    }
+                };
+            }
+
+            protected override void LoadComplete()
+            {
+                base.LoadComplete();
+
+                selectedGroup.BindValueChanged(_ => updateState(), true);
+                FinishTransforms(true);
+            }
+
+            protected override void PrepareForUse()
+            {
+                base.PrepareForUse();
+
+                updateState();
+            }
+
+            protected override bool OnHover(HoverEvent e)
+            {
+                updateState();
+                return true;
+            }
+
+            protected override void OnHoverLost(HoverLostEvent e)
+            {
+                base.OnHoverLost(e);
+                updateState();
+            }
+
+            protected override bool OnClick(ClickEvent e)
+            {
+                // schedule to give time for any modified focused text box to lose focus and commit changes (e.g. BPM / time signature textboxes) before switching to new point.
+                var currentGroup = Current.Value;
+                Schedule(() =>
+                {
+                    selectedGroup.Value = currentGroup;
+                    editorClock.SeekSmoothlyTo(currentGroup.Time);
+                });
+                return true;
+            }
+
+            private void updateState()
+            {
+                bool isSelected = selectedGroup.Value?.Equals(current.Value) == true;
+
+                if (IsHovered || isSelected)
+                    background.FadeIn(100, Easing.OutQuint);
+                else
+                    background.FadeOut(100, Easing.OutQuint);
+
+                background.Colour = isSelected ? colourProvider.Colour3 : colourProvider.Background1;
+            }
         }
 
         private partial class ControlGroupTiming : FillFlowContainer
         {
-            public ControlGroupTiming(ControlPointGroup group)
+            public Bindable<ControlPointGroup> Group { get; } = new Bindable<ControlPointGroup>();
+
+            private OsuSpriteText timeText = null!;
+
+            [BackgroundDependencyLoader]
+            private void load()
             {
                 Name = @"ControlGroupTiming";
                 RelativeSizeAxes = Axes.Y;
-                Width = TIMING_COLUMN_WIDTH;
+                Width = timing_column_width;
                 Spacing = new Vector2(5);
                 Children = new Drawable[]
                 {
-                    new OsuSpriteText
+                    timeText = new OsuSpriteText
                     {
-                        Text = group.Time.ToEditorFormattedString(),
+                        Font = OsuFont.GetFont(size: 14, weight: FontWeight.Bold),
-                        Font = OsuFont.GetFont(size: TEXT_SIZE, weight: FontWeight.Bold),
                         Width = 70,
                         Anchor = Anchor.CentreLeft,
                         Origin = Anchor.CentreLeft,
                     },
-                    new ControlGroupAttributes(group, c => c is TimingControlPoint)
+                    new ControlGroupAttributes(c => c is TimingControlPoint)
                     {
                         Anchor = Anchor.CentreLeft,
                         Origin = Anchor.CentreLeft,
+                        Group = { BindTarget = Group },
                     }
                 };
             }
+
+            protected override void LoadComplete()
+            {
+                base.LoadComplete();
+
+                Group.BindValueChanged(_ => timeText.Text = Group.Value?.Time.ToEditorFormattedString() ?? default(LocalisableString), true);
+            }
         }
 
         private partial class ControlGroupAttributes : CompositeDrawable
         {
+            public Bindable<ControlPointGroup> Group { get; } = new Bindable<ControlPointGroup>();
+            private BindableList<ControlPoint> controlPoints { get; } = new BindableList<ControlPoint>();
+
             private readonly Func<ControlPoint, bool> matchFunction;
 
-            private readonly IBindableList<ControlPoint> controlPoints = new BindableList<ControlPoint>();
+            private FillFlowContainer fill = null!;
 
-            private readonly FillFlowContainer fill;
+            public ControlGroupAttributes(Func<ControlPoint, bool> matchFunction)
-
-            public ControlGroupAttributes(ControlPointGroup group, Func<ControlPoint, bool> matchFunction)
             {
                 this.matchFunction = matchFunction;
+            }
 
+            [BackgroundDependencyLoader]
+            private void load()
+            {
                 AutoSizeAxes = Axes.X;
                 RelativeSizeAxes = Axes.Y;
                 Name = @"ControlGroupAttributes";
@@ -156,20 +277,21 @@ namespace osu.Game.Screens.Edit.Timing
                     Direction = FillDirection.Horizontal,
                     Spacing = new Vector2(2)
                 };
-
-                controlPoints.BindTo(group.ControlPoints);
-            }
-
-            [BackgroundDependencyLoader]
-            private void load()
-            {
-                createChildren();
             }
 
             protected override void LoadComplete()
             {
                 base.LoadComplete();
-                controlPoints.CollectionChanged += (_, _) => createChildren();
+
+                Group.BindValueChanged(_ =>
+                {
+                    controlPoints.UnbindBindings();
+                    controlPoints.Clear();
+                    if (Group.Value != null)
+                        ((IBindableList<ControlPoint>)controlPoints).BindTo(Group.Value.ControlPoints);
+                }, true);
+
+                controlPoints.BindCollectionChanged((_, _) => createChildren(), true);
             }
 
             private void createChildren()

osu.Game/Screens/Edit/Timing/RowAttributes/AttributeProgressBar.cs

@@ -36,6 +36,7 @@ namespace osu.Game.Screens.Edit.Timing.RowAttributes
 
             BackgroundColour = overlayColours.Background6;
             FillColour = controlPoint.GetRepresentingColour(colours);
+            FinishTransforms(true);
         }
     }
 }