From 421f245c3102bb2ab3719b7ff4717b401a0db11a Mon Sep 17 00:00:00 2001
From: Dean Herbert <pe@ppy.sh>
Date: Wed, 4 Sep 2024 14:31:59 +0900
Subject: [PATCH] Fix per-frame allocations in `BeatmapCarousel`

---
 .../SongSelect/TestSceneBeatmapCarousel.cs     |  4 ++--
 osu.Game/Screens/Select/BeatmapCarousel.cs     | 11 +++++++----
 .../Carousel/DrawableCarouselBeatmapSet.cs     | 18 ++++++++----------
 3 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs b/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs
index ec072a3dd2..218a67e818 100644
--- a/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs
+++ b/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs
@@ -1405,9 +1405,9 @@ namespace osu.Game.Tests.Visual.SongSelect
 
                         yield return item;
 
-                        if (item is DrawableCarouselBeatmapSet set)
+                        if (item is DrawableCarouselBeatmapSet set && set.Beatmaps?.IsLoaded == true)
                         {
-                            foreach (var difficulty in set.DrawableBeatmaps)
+                            foreach (var difficulty in set.Beatmaps)
                                 yield return difficulty;
                         }
                     }
diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs
index 87cea45e87..f7e0eae4a5 100644
--- a/osu.Game/Screens/Select/BeatmapCarousel.cs
+++ b/osu.Game/Screens/Select/BeatmapCarousel.cs
@@ -857,8 +857,9 @@ namespace osu.Game.Screens.Select
                     // Add those items within the previously found index range that should be displayed.
                     foreach (var item in toDisplay)
                     {
-                        var panel = setPool.Get(p => p.Item = item);
+                        var panel = setPool.Get();
 
+                        panel.Item = item;
                         panel.Y = item.CarouselYPosition;
 
                         Scroll.Add(panel);
@@ -898,10 +899,12 @@ namespace osu.Game.Screens.Select
                     Scroll.ChangeChildDepth(item, hasPassedSelection ? -item.Item.CarouselYPosition : item.Item.CarouselYPosition);
                 }
 
-                if (item is DrawableCarouselBeatmapSet set)
+                if (item is DrawableCarouselBeatmapSet set && set.Beatmaps?.IsLoaded == true)
                 {
-                    foreach (var diff in set.DrawableBeatmaps)
+                    foreach (var diff in set.Beatmaps)
+                    {
                         updateItem(diff, item);
+                    }
                 }
             }
         }
@@ -1101,7 +1104,7 @@ namespace osu.Game.Screens.Select
         }
 
         /// <summary>
-        /// Update a item's x position and multiplicative alpha based on its y position and
+        /// Update an item's x position and multiplicative alpha based on its y position and
         /// the current scroll position.
         /// </summary>
         /// <param name="item">The item to be updated.</param>
diff --git a/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmapSet.cs b/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmapSet.cs
index 1cd8b065fc..9a01b46216 100644
--- a/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmapSet.cs
+++ b/osu.Game/Screens/Select/Carousel/DrawableCarouselBeatmapSet.cs
@@ -51,9 +51,7 @@ namespace osu.Game.Screens.Select.Carousel
         [Resolved]
         private IBindable<RulesetInfo> ruleset { get; set; } = null!;
 
-        public IEnumerable<DrawableCarouselItem> DrawableBeatmaps => beatmapContainer?.IsLoaded != true ? Enumerable.Empty<DrawableCarouselItem>() : beatmapContainer.AliveChildren;
-
-        private Container<DrawableCarouselItem>? beatmapContainer;
+        public Container<DrawableCarouselItem>? Beatmaps;
 
         private BeatmapSetInfo beatmapSet = null!;
 
@@ -126,7 +124,7 @@ namespace osu.Game.Screens.Select.Carousel
             Content.Clear();
             Header.Clear();
 
-            beatmapContainer = null;
+            Beatmaps = null;
             beatmapsLoadTask = null;
 
             if (Item == null)
@@ -164,7 +162,7 @@ namespace osu.Game.Screens.Select.Carousel
             // if we are already displaying all the correct beatmaps, only run animation updates.
             // note that the displayed beatmaps may change due to the applied filter.
             // a future optimisation could add/remove only changed difficulties rather than reinitialise.
-            if (beatmapContainer != null && visibleBeatmaps.Length == beatmapContainer.Count && visibleBeatmaps.All(b => beatmapContainer.Any(c => c.Item == b)))
+            if (Beatmaps != null && visibleBeatmaps.Length == Beatmaps.Count && visibleBeatmaps.All(b => Beatmaps.Any(c => c.Item == b)))
             {
                 updateBeatmapYPositions();
             }
@@ -173,17 +171,17 @@ namespace osu.Game.Screens.Select.Carousel
                 // on selection we show our child beatmaps.
                 // for now this is a simple drawable construction each selection.
                 // can be improved in the future.
-                beatmapContainer = new Container<DrawableCarouselItem>
+                Beatmaps = new Container<DrawableCarouselItem>
                 {
                     X = 100,
                     RelativeSizeAxes = Axes.Both,
                     ChildrenEnumerable = visibleBeatmaps.Select(c => c.CreateDrawableRepresentation()!)
                 };
 
-                beatmapsLoadTask = LoadComponentAsync(beatmapContainer, loaded =>
+                beatmapsLoadTask = LoadComponentAsync(Beatmaps, loaded =>
                 {
                     // make sure the pooled target hasn't changed.
-                    if (beatmapContainer != loaded)
+                    if (Beatmaps != loaded)
                         return;
 
                     Content.Child = loaded;
@@ -244,7 +242,7 @@ namespace osu.Game.Screens.Select.Carousel
 
         private void updateBeatmapYPositions()
         {
-            if (beatmapContainer == null)
+            if (Beatmaps == null)
                 return;
 
             if (beatmapsLoadTask == null || !beatmapsLoadTask.IsCompleted)
@@ -254,7 +252,7 @@ namespace osu.Game.Screens.Select.Carousel
 
             bool isSelected = Item?.State.Value == CarouselItemState.Selected;
 
-            foreach (var panel in beatmapContainer)
+            foreach (var panel in Beatmaps)
             {
                 Debug.Assert(panel.Item != null);