From c34b2ffc052e918855478d062bb72bee8efd066c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 22 Oct 2025 11:44:52 +0200 Subject: [PATCH] Fix performance overhead when computing spacing between standalone panels The equality check that was supposed to replace the read of `CurrentSelectionItem` showed up as a hotspot in profiling. The selection updating code looks a little stupid after this, but all in the name of performance... --- osu.Game/Graphics/Carousel/Carousel.cs | 21 ++++++++++++++++---- osu.Game/Screens/SelectV2/BeatmapCarousel.cs | 3 +-- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/osu.Game/Graphics/Carousel/Carousel.cs b/osu.Game/Graphics/Carousel/Carousel.cs index a63fee6914..7b8991a0be 100644 --- a/osu.Game/Graphics/Carousel/Carousel.cs +++ b/osu.Game/Graphics/Carousel/Carousel.cs @@ -761,13 +761,26 @@ namespace osu.Game.Graphics.Carousel { var item = carouselItems[i]; + bool isKeyboardSelection = CheckModelEquality(item.Model, currentKeyboardSelection.Model!); + bool isSelection = CheckModelEquality(item.Model, currentSelection.Model!); + + // while we don't know the Y position of the item yet, as it's about to be updated, + // consumers (specifically `BeatmapCarousel.GetSpacingBetweenPanels()`) benefit from `CurrentSelectionItem` already pointing + // at the correct item to avoid redundant local equality checks. + // the Y positions will be filled in after they're computed. + if (isKeyboardSelection) + currentKeyboardSelection = new Selection(currentKeyboardSelection.Model, item, null, i); + + if (isSelection) + currentSelection = new Selection(currentSelection.Model, item, null, i); + updateItemYPosition(item, ref lastVisible, ref yPos); - if (CheckModelEquality(item.Model, currentKeyboardSelection.Model!)) - currentKeyboardSelection = new Selection(currentKeyboardSelection.Model, item, item.CarouselYPosition + item.DrawHeight / 2, i); + if (isKeyboardSelection) + currentKeyboardSelection = currentKeyboardSelection with { YPosition = item.CarouselYPosition + item.DrawHeight / 2 }; - if (CheckModelEquality(item.Model, currentSelection.Model!)) - currentSelection = new Selection(currentSelection.Model, item, item.CarouselYPosition + item.DrawHeight / 2, i); + if (isSelection) + currentSelection = currentSelection with { YPosition = item.CarouselYPosition + item.DrawHeight / 2 }; } // Update the total height of all items (to make the scroll container scrollable through the full height even though diff --git a/osu.Game/Screens/SelectV2/BeatmapCarousel.cs b/osu.Game/Screens/SelectV2/BeatmapCarousel.cs index 540bd10afc..6b78967b93 100644 --- a/osu.Game/Screens/SelectV2/BeatmapCarousel.cs +++ b/osu.Game/Screens/SelectV2/BeatmapCarousel.cs @@ -86,8 +86,7 @@ namespace osu.Game.Screens.SelectV2 } else { - // `CurrentSelectionItem` cannot be used here because it may not be correctly set yet. - if (CurrentSelection != null && (CheckModelEquality(top.Model, CurrentSelection) || CheckModelEquality(bottom.Model, CurrentSelection))) + if (CurrentSelection != null && (top == CurrentSelectionItem || bottom == CurrentSelectionItem)) return SPACING * 2; }