From 7ffead6878059d35329b0cdeedb9a30f083fab53 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 2 Jun 2025 23:19:22 +0900 Subject: [PATCH 1/4] SongSelectV2: Fix backgrounds taking too long to load due to model backed drawable --- .../Screens/SelectV2/PanelSetBackground.cs | 136 +++++++++--------- 1 file changed, 71 insertions(+), 65 deletions(-) diff --git a/osu.Game/Screens/SelectV2/PanelSetBackground.cs b/osu.Game/Screens/SelectV2/PanelSetBackground.cs index dd07be0410..eeac9c4f89 100644 --- a/osu.Game/Screens/SelectV2/PanelSetBackground.cs +++ b/osu.Game/Screens/SelectV2/PanelSetBackground.cs @@ -9,94 +9,100 @@ using osu.Framework.Graphics.Containers; using osu.Framework.Graphics.Shapes; using osu.Framework.Graphics.Sprites; using osu.Game.Beatmaps; -using osu.Game.Overlays; using osuTK; using osuTK.Graphics; namespace osu.Game.Screens.SelectV2 { - public partial class PanelSetBackground : ModelBackedDrawable + public partial class PanelSetBackground : CompositeDrawable { - protected override double TransformDuration => 400; + private Sprite? sprite; + + private WorkingBeatmap? working; public WorkingBeatmap? Beatmap { - get => Model; - set => Model = value; + get => working; + set + { + working = value; + loadNextBackground(); + } } - protected override Drawable CreateDrawable(WorkingBeatmap? model) => new BackgroundSprite(model); - - private partial class BackgroundSprite : CompositeDrawable + public PanelSetBackground() { - private readonly WorkingBeatmap? working; + RelativeSizeAxes = Axes.Both; + } - public BackgroundSprite(WorkingBeatmap? working) + [BackgroundDependencyLoader] + private void load() + { + InternalChildren = new Drawable[] { - this.working = working; - - RelativeSizeAxes = Axes.Both; - } - - [BackgroundDependencyLoader] - private void load(OverlayColourProvider colourProvider) - { - var texture = working?.GetPanelBackground(); - - if (texture != null) + new FillFlowContainer { - InternalChildren = new Drawable[] + Depth = -1, + RelativeSizeAxes = Axes.Both, + Direction = FillDirection.Horizontal, + // This makes the gradient not be perfectly horizontal, but diagonal at a ~40° angle + Shear = new Vector2(0.8f, 0), + Children = new[] { - new Sprite + // The left half with no gradient applied + new Box { RelativeSizeAxes = Axes.Both, - Anchor = Anchor.Centre, - Origin = Anchor.Centre, - FillMode = FillMode.Fill, - Texture = texture, + Colour = Color4.Black.Opacity(0.5f), + Width = 0.4f, }, - new FillFlowContainer + new Box { - Depth = -1, RelativeSizeAxes = Axes.Both, - Direction = FillDirection.Horizontal, - // This makes the gradient not be perfectly horizontal, but diagonal at a ~40° angle - Shear = new Vector2(0.8f, 0), - Children = new[] - { - // The left half with no gradient applied - new Box - { - RelativeSizeAxes = Axes.Both, - Colour = Color4.Black.Opacity(0.5f), - Width = 0.4f, - }, - new Box - { - RelativeSizeAxes = Axes.Both, - Colour = ColourInfo.GradientHorizontal(Color4.Black.Opacity(0.5f), Color4.Black.Opacity(0.3f)), - Width = 0.2f, - }, - new Box - { - RelativeSizeAxes = Axes.Both, - Colour = ColourInfo.GradientHorizontal(Color4.Black.Opacity(0.3f), Color4.Black.Opacity(0.2f)), - // Slightly more than 1.0 in total to account for shear. - Width = 0.45f, - }, - } + Colour = ColourInfo.GradientHorizontal(Color4.Black.Opacity(0.5f), Color4.Black.Opacity(0.3f)), + Width = 0.2f, }, - }; - } - else - { - InternalChild = new Box - { - RelativeSizeAxes = Axes.Both, - Colour = colourProvider.Background6, - }; - } + new Box + { + RelativeSizeAxes = Axes.Both, + Colour = ColourInfo.GradientHorizontal(Color4.Black.Opacity(0.3f), Color4.Black.Opacity(0.2f)), + // Slightly more than 1.0 in total to account for shear. + Width = 0.45f, + }, + } + }, + }; + } + + private void loadNextBackground() + { + const double transition_duration = 500; + + var texture = working?.GetPanelBackground(); + + if (texture == null) + { + sprite?.FadeOut(transition_duration, Easing.OutQuint); + sprite = null; + return; } + + LoadComponentAsync(new Sprite + { + Depth = float.MaxValue, + RelativeSizeAxes = Axes.Both, + Anchor = Anchor.Centre, + Origin = Anchor.Centre, + FillMode = FillMode.Fill, + Texture = texture, + }, s => + { + sprite?.Delay(transition_duration) + .FadeOut(); + + AddInternal(sprite = s); + sprite.FadeInFromZero(transition_duration, Easing.OutQuint); + }); } } } From 920eec2c589299616274d5e78b8e6dae8b453839 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 3 Jun 2025 00:31:16 +0900 Subject: [PATCH 2/4] Add basic delay before beginning background load to avoid load on large scroll --- .../Screens/SelectV2/PanelSetBackground.cs | 72 +++++++++++++++---- 1 file changed, 59 insertions(+), 13 deletions(-) diff --git a/osu.Game/Screens/SelectV2/PanelSetBackground.cs b/osu.Game/Screens/SelectV2/PanelSetBackground.cs index eeac9c4f89..743d0d489a 100644 --- a/osu.Game/Screens/SelectV2/PanelSetBackground.cs +++ b/osu.Game/Screens/SelectV2/PanelSetBackground.cs @@ -1,11 +1,14 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. +using System; +using System.Threading; using osu.Framework.Allocation; using osu.Framework.Extensions.Color4Extensions; 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.Graphics.Sprites; using osu.Game.Beatmaps; @@ -14,27 +17,52 @@ using osuTK.Graphics; namespace osu.Game.Screens.SelectV2 { - public partial class PanelSetBackground : CompositeDrawable + public partial class PanelSetBackground : BufferedContainer { + [Resolved] + private BeatmapCarousel? beatmapCarousel { get; set; } + private Sprite? sprite; private WorkingBeatmap? working; + private CancellationTokenSource? loadCancellation; + + private double timeSinceUnpool; + public WorkingBeatmap? Beatmap { get => working; set { + if (value == working) + return; + working = value; - loadNextBackground(); + + loadCancellation?.Cancel(); + loadCancellation = null; + + sprite?.Expire(); + sprite = null; + + timeSinceUnpool = 0; } } public PanelSetBackground() + // : base(cachedFrameBuffer: true) { RelativeSizeAxes = Axes.Both; } + protected override void Update() + { + base.Update(); + + loadContentIfRequired(); + } + [BackgroundDependencyLoader] private void load() { @@ -74,18 +102,39 @@ namespace osu.Game.Screens.SelectV2 }; } - private void loadNextBackground() + private void loadContentIfRequired() { - const double transition_duration = 500; + // A load is already in progress if the cancellation token is non-null. + if (loadCancellation != null) + return; + + if (beatmapCarousel != null) + { + Quad containingSsdq = beatmapCarousel.ScreenSpaceDrawQuad; + + // Using DelayedLoadWrappers would only allow us to load content when on screen, but we want to preload while off-screen + // to provide a better user experience. + + // This is tracking time that this drawable is updating since the last pool. + // This is intended to provide a debounce so very fast scrolls (from one end to the other of the carousel) + // don't cause huge overheads. + // + // We increase the delay based on distance from centre, so the beatmaps the user is currently looking at load first. + float timeUpdatingBeforeLoad = 50 + Math.Abs(containingSsdq.Centre.Y - ScreenSpaceDrawQuad.Centre.Y) / containingSsdq.Height * 100; + + timeSinceUnpool += Time.Elapsed; + + // We only trigger a load after this set has been in an updating state for a set amount of time. + if (timeSinceUnpool <= timeUpdatingBeforeLoad) + return; + } + + loadCancellation = new CancellationTokenSource(); var texture = working?.GetPanelBackground(); if (texture == null) - { - sprite?.FadeOut(transition_duration, Easing.OutQuint); - sprite = null; return; - } LoadComponentAsync(new Sprite { @@ -97,12 +146,9 @@ namespace osu.Game.Screens.SelectV2 Texture = texture, }, s => { - sprite?.Delay(transition_duration) - .FadeOut(); - AddInternal(sprite = s); - sprite.FadeInFromZero(transition_duration, Easing.OutQuint); - }); + sprite.FadeInFromZero(200, Easing.OutQuint); + }, loadCancellation.Token); } } } From fa4c72887f07dd18d61959447e1bb91a2d2725a2 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 3 Jun 2025 01:14:18 +0900 Subject: [PATCH 3/4] Bring back missing logic to avoid stutters when scrolling Again, I don't know why the new implementation didn't just draw from the old which was known to work. This mostly matches what was there in v1. --- .../Screens/SelectV2/PanelSetBackground.cs | 36 +++++++++++++------ 1 file changed, 26 insertions(+), 10 deletions(-) diff --git a/osu.Game/Screens/SelectV2/PanelSetBackground.cs b/osu.Game/Screens/SelectV2/PanelSetBackground.cs index 743d0d489a..ae7c7d3138 100644 --- a/osu.Game/Screens/SelectV2/PanelSetBackground.cs +++ b/osu.Game/Screens/SelectV2/PanelSetBackground.cs @@ -5,6 +5,7 @@ using System; using System.Threading; using osu.Framework.Allocation; using osu.Framework.Extensions.Color4Extensions; +using osu.Framework.Extensions.PolygonExtensions; using osu.Framework.Graphics; using osu.Framework.Graphics.Colour; using osu.Framework.Graphics.Containers; @@ -51,7 +52,9 @@ namespace osu.Game.Screens.SelectV2 } public PanelSetBackground() - // : base(cachedFrameBuffer: true) + // TODO: for performance reasons we probably want this to be true + // for it to work we will need to move transforms accordingly. + : base(cachedFrameBuffer: false) { RelativeSizeAxes = Axes.Both; } @@ -105,7 +108,7 @@ namespace osu.Game.Screens.SelectV2 private void loadContentIfRequired() { // A load is already in progress if the cancellation token is non-null. - if (loadCancellation != null) + if (loadCancellation != null || working == null) return; if (beatmapCarousel != null) @@ -131,24 +134,37 @@ namespace osu.Game.Screens.SelectV2 loadCancellation = new CancellationTokenSource(); - var texture = working?.GetPanelBackground(); - - if (texture == null) - return; - - LoadComponentAsync(new Sprite + LoadComponentAsync(new PanelBeatmapBackground(working) { Depth = float.MaxValue, RelativeSizeAxes = Axes.Both, Anchor = Anchor.Centre, Origin = Anchor.Centre, FillMode = FillMode.Fill, - Texture = texture, }, s => { AddInternal(sprite = s); - sprite.FadeInFromZero(200, Easing.OutQuint); + bool spriteOnScreen = beatmapCarousel?.ScreenSpaceDrawQuad.Intersects(sprite.ScreenSpaceDrawQuad) != false; + sprite.FadeInFromZero(spriteOnScreen ? 400 : 0, Easing.OutQuint); }, loadCancellation.Token); } + + public partial class PanelBeatmapBackground : Sprite + { + private readonly IWorkingBeatmap working; + + public PanelBeatmapBackground(IWorkingBeatmap working) + { + ArgumentNullException.ThrowIfNull(working); + + this.working = working; + } + + [BackgroundDependencyLoader] + private void load() + { + Texture = working.GetPanelBackground(); + } + } } } From 429c9d42c1e4d515d15dfd4d2bdb0a4eea8d79f5 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 3 Jun 2025 15:57:54 +0900 Subject: [PATCH 4/4] Update inline comments to add clarity to implementation details --- .../Screens/SelectV2/PanelSetBackground.cs | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/osu.Game/Screens/SelectV2/PanelSetBackground.cs b/osu.Game/Screens/SelectV2/PanelSetBackground.cs index ae7c7d3138..d81a6007d8 100644 --- a/osu.Game/Screens/SelectV2/PanelSetBackground.cs +++ b/osu.Game/Screens/SelectV2/PanelSetBackground.cs @@ -52,8 +52,9 @@ namespace osu.Game.Screens.SelectV2 } public PanelSetBackground() - // TODO: for performance reasons we probably want this to be true - // for it to work we will need to move transforms accordingly. + // TODO: for performance reasons we may want this to be true. + // Setting to true will require that the buffered portion is moved to a child such that `FadeIn`/`FadeOut` transforms + // still work. : base(cachedFrameBuffer: false) { RelativeSizeAxes = Axes.Both; @@ -115,14 +116,14 @@ namespace osu.Game.Screens.SelectV2 { Quad containingSsdq = beatmapCarousel.ScreenSpaceDrawQuad; - // Using DelayedLoadWrappers would only allow us to load content when on screen, but we want to preload while off-screen - // to provide a better user experience. - - // This is tracking time that this drawable is updating since the last pool. - // This is intended to provide a debounce so very fast scrolls (from one end to the other of the carousel) - // don't cause huge overheads. + // One may ask why we are not using `DelayedLoadWrapper` for this delayed load logic. // - // We increase the delay based on distance from centre, so the beatmaps the user is currently looking at load first. + // - Using `DelayedLoadWrapper` would only allow us to load content when on screen, but we want to preload while panels are off-screen. + // This allows a more seamless experience when a user is scrolling at a moderate speed, as we are loading in backgrounds before they + // enter the visible viewport. + // - By using a slightly customised formula to decide when to start the load, we can coerce the loading of backgrounds into an order that + // prioritises panels which are closest to the centre of the screen. Basically, we want to load backgrounds "outwards" from the visual + // centre to give the user the best experience possible. float timeUpdatingBeforeLoad = 50 + Math.Abs(containingSsdq.Centre.Y - ScreenSpaceDrawQuad.Centre.Y) / containingSsdq.Height * 100; timeSinceUnpool += Time.Elapsed;