From 85c42456f25c9a27cd4871fb2cc71e1e3caf1b17 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Tue, 30 Jun 2020 21:38:51 +0900 Subject: [PATCH 1/7] Improve performance of sequential scrolling algorithm --- .../Algorithms/SequentialScrollAlgorithm.cs | 164 +++++++++++------- 1 file changed, 104 insertions(+), 60 deletions(-) diff --git a/osu.Game/Rulesets/UI/Scrolling/Algorithms/SequentialScrollAlgorithm.cs b/osu.Game/Rulesets/UI/Scrolling/Algorithms/SequentialScrollAlgorithm.cs index 0052c877f6..a1f68d7201 100644 --- a/osu.Game/Rulesets/UI/Scrolling/Algorithms/SequentialScrollAlgorithm.cs +++ b/osu.Game/Rulesets/UI/Scrolling/Algorithms/SequentialScrollAlgorithm.cs @@ -3,21 +3,26 @@ using System; using System.Collections.Generic; +using System.Diagnostics; +using JetBrains.Annotations; using osu.Game.Rulesets.Timing; namespace osu.Game.Rulesets.UI.Scrolling.Algorithms { public class SequentialScrollAlgorithm : IScrollAlgorithm { - private readonly Dictionary positionCache; + private static readonly IComparer by_position_comparer = Comparer.Create((c1, c2) => c1.Position.CompareTo(c2.Position)); private readonly IReadOnlyList controlPoints; + /// + /// Stores a mapping of time -> position for each control point. + /// + private readonly List positionMappings = new List(); + public SequentialScrollAlgorithm(IReadOnlyList controlPoints) { this.controlPoints = controlPoints; - - positionCache = new Dictionary(); } public double GetDisplayStartTime(double originTime, float offset, double timeRange, float scrollLength) @@ -27,55 +32,31 @@ namespace osu.Game.Rulesets.UI.Scrolling.Algorithms public float GetLength(double startTime, double endTime, double timeRange, float scrollLength) { - var objectLength = relativePositionAtCached(endTime, timeRange) - relativePositionAtCached(startTime, timeRange); + var objectLength = relativePositionAt(endTime, timeRange) - relativePositionAt(startTime, timeRange); return (float)(objectLength * scrollLength); } public float PositionAt(double time, double currentTime, double timeRange, float scrollLength) { - // Caching is not used here as currentTime is unlikely to have been previously cached - double timelinePosition = relativePositionAt(currentTime, timeRange); - return (float)((relativePositionAtCached(time, timeRange) - timelinePosition) * scrollLength); + double timelineLength = relativePositionAt(time, timeRange) - relativePositionAt(currentTime, timeRange); + return (float)(timelineLength * scrollLength); } public double TimeAt(float position, double currentTime, double timeRange, float scrollLength) { - // Convert the position to a length relative to time = 0 - double length = position / scrollLength + relativePositionAt(currentTime, timeRange); + if (controlPoints.Count == 0) + return position * timeRange; - // We need to consider all timing points until the specified time and not just the currently-active one, - // since each timing point individually affects the positions of _all_ hitobjects after its start time - for (int i = 0; i < controlPoints.Count; i++) - { - var current = controlPoints[i]; - var next = i < controlPoints.Count - 1 ? controlPoints[i + 1] : null; + // Find the position at the current time, and the given length. + double relativePosition = relativePositionAt(currentTime, timeRange) + position / scrollLength; - // Duration of the current control point - var currentDuration = (next?.StartTime ?? double.PositiveInfinity) - current.StartTime; + var positionMapping = findControlPointMapping(timeRange, new PositionMapping(0, null, relativePosition), by_position_comparer); - // Figure out the length of control point - var currentLength = currentDuration / timeRange * current.Multiplier; - - if (currentLength > length) - { - // The point is within this control point - return current.StartTime + length * timeRange / current.Multiplier; - } - - length -= currentLength; - } - - return 0; // Should never occur + // Begin at the control point's time and add the remaining time to reach the given position. + return positionMapping.Time + (relativePosition - positionMapping.Position) * timeRange / positionMapping.ControlPoint.Multiplier; } - private double relativePositionAtCached(double time, double timeRange) - { - if (!positionCache.TryGetValue(time, out double existing)) - positionCache[time] = existing = relativePositionAt(time, timeRange); - return existing; - } - - public void Reset() => positionCache.Clear(); + public void Reset() => positionMappings.Clear(); /// /// Finds the position which corresponds to a point in time. @@ -84,37 +65,100 @@ namespace osu.Game.Rulesets.UI.Scrolling.Algorithms /// The time to find the position at. /// The amount of time visualised by the scrolling area. /// A positive value indicating the position at . - private double relativePositionAt(double time, double timeRange) + private double relativePositionAt(in double time, in double timeRange) { if (controlPoints.Count == 0) return time / timeRange; - double length = 0; + var mapping = findControlPointMapping(timeRange, new PositionMapping(time)); - // We need to consider all timing points until the specified time and not just the currently-active one, - // since each timing point individually affects the positions of _all_ hitobjects after its start time - for (int i = 0; i < controlPoints.Count; i++) + // Begin at the control point's position and add the remaining distance to reach the given time. + return mapping.Position + (time - mapping.Time) / timeRange * mapping.ControlPoint.Multiplier; + } + + /// + /// Finds a 's that is relevant to a given . + /// + /// + /// This is used to find the last occuring prior to a time value, or prior to a position value (if is used). + /// + /// The time range. + /// The to find the closest to. + /// The comparison. If null, the default comparer is used (by time). + /// The 's that is relevant for . + private PositionMapping findControlPointMapping(in double timeRange, in PositionMapping search, IComparer comparer = null) + { + generatePositionMappings(timeRange); + + var mappingIndex = positionMappings.BinarySearch(search, comparer ?? Comparer.Default); + + if (mappingIndex < 0) { - var current = controlPoints[i]; - var next = i < controlPoints.Count - 1 ? controlPoints[i + 1] : null; + // If the search value isn't found, the _next_ control point is returned, but we actually want the _previous_ control point. + // In doing so, we must make sure to not underflow the position mapping list (i.e. always use the 0th control point for time < first_control_point_time). + mappingIndex = Math.Max(0, ~mappingIndex - 1); - // We don't need to consider any control points beyond the current time, since it will not yet - // affect any hitobjects - if (i > 0 && current.StartTime > time) - continue; - - // Duration of the current control point - var currentDuration = (next?.StartTime ?? double.PositiveInfinity) - current.StartTime; - - // We want to consider the minimal amount of time that this control point has affected, - // which may be either its duration, or the amount of time that has passed within it - var durationInCurrent = Math.Min(currentDuration, time - current.StartTime); - - // Figure out how much of the time range the duration represents, and adjust it by the speed multiplier - length += durationInCurrent / timeRange * current.Multiplier; + Debug.Assert(mappingIndex < positionMappings.Count); } - return length; + var mapping = positionMappings[mappingIndex]; + Debug.Assert(mapping.ControlPoint != null); + + return mapping; + } + + /// + /// Generates the mapping of (and their respective start times) to their relative position from 0. + /// + /// The time range. + private void generatePositionMappings(in double timeRange) + { + if (positionMappings.Count > 0) + return; + + if (controlPoints.Count == 0) + return; + + positionMappings.Add(new PositionMapping(controlPoints[0].StartTime, controlPoints[0])); + + for (int i = 0; i < controlPoints.Count - 1; i++) + { + var current = controlPoints[i]; + var next = controlPoints[i + 1]; + + // Figure out how much of the time range the duration represents, and adjust it by the speed multiplier + float length = (float)((next.StartTime - current.StartTime) / timeRange * current.Multiplier); + + positionMappings.Add(new PositionMapping(next.StartTime, next, positionMappings[^1].Position + length)); + } + } + + private readonly struct PositionMapping : IComparable + { + /// + /// The time corresponding to this position. + /// + public readonly double Time; + + /// + /// The at . + /// + [CanBeNull] + public readonly MultiplierControlPoint ControlPoint; + + /// + /// The relative position from 0 of . + /// + public readonly double Position; + + public PositionMapping(double time, MultiplierControlPoint controlPoint = null, double position = default) + { + Time = time; + ControlPoint = controlPoint; + Position = position; + } + + public int CompareTo(PositionMapping other) => Time.CompareTo(other.Time); } } } From ab15b6031d662fb660149f0c9085be99f5c33b59 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Wed, 1 Jul 2020 17:12:07 +0900 Subject: [PATCH 2/7] Update with framework-side storage changes --- osu.Game/IO/OsuStorage.cs | 6 +++--- osu.Game/OsuGameBase.cs | 4 +++- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/osu.Game/IO/OsuStorage.cs b/osu.Game/IO/OsuStorage.cs index 499bcb4063..f5ce1c0105 100644 --- a/osu.Game/IO/OsuStorage.cs +++ b/osu.Game/IO/OsuStorage.cs @@ -24,12 +24,12 @@ namespace osu.Game.IO "storage.ini" }; - public OsuStorage(GameHost host) - : base(host.Storage, string.Empty) + public OsuStorage(GameHost host, Storage defaultStorage) + : base(defaultStorage, string.Empty) { this.host = host; - storageConfig = new StorageConfigManager(host.Storage); + storageConfig = new StorageConfigManager(defaultStorage); var customStoragePath = storageConfig.Get(StorageConfig.FullPath); diff --git a/osu.Game/OsuGameBase.cs b/osu.Game/OsuGameBase.cs index 3e7311092e..c79f710151 100644 --- a/osu.Game/OsuGameBase.cs +++ b/osu.Game/OsuGameBase.cs @@ -312,11 +312,13 @@ namespace osu.Game base.SetHost(host); // may be non-null for certain tests - Storage ??= new OsuStorage(host); + Storage ??= host.Storage; LocalConfig ??= new OsuConfigManager(Storage); } + protected override Storage CreateStorage(GameHost host, Storage defaultStorage) => new OsuStorage(host, defaultStorage); + private readonly List fileImporters = new List(); public async Task Import(params string[] paths) From 6f6376d53c56e5f592a6ae253349b4cc1923f5e8 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 1 Jul 2020 18:52:05 +0900 Subject: [PATCH 3/7] Update framework --- .idea/.idea.osu.Desktop/.idea/modules.xml | 1 - osu.Android.props | 2 +- osu.Game/osu.Game.csproj | 2 +- osu.iOS.props | 4 ++-- 4 files changed, 4 insertions(+), 5 deletions(-) diff --git a/.idea/.idea.osu.Desktop/.idea/modules.xml b/.idea/.idea.osu.Desktop/.idea/modules.xml index 366f172c30..fe63f5faf3 100644 --- a/.idea/.idea.osu.Desktop/.idea/modules.xml +++ b/.idea/.idea.osu.Desktop/.idea/modules.xml @@ -2,7 +2,6 @@ - diff --git a/osu.Android.props b/osu.Android.props index 493b1f5529..a2c97ead2f 100644 --- a/osu.Android.props +++ b/osu.Android.props @@ -52,6 +52,6 @@ - + diff --git a/osu.Game/osu.Game.csproj b/osu.Game/osu.Game.csproj index 5f326a361d..3ef53a2a53 100644 --- a/osu.Game/osu.Game.csproj +++ b/osu.Game/osu.Game.csproj @@ -24,7 +24,7 @@ - + diff --git a/osu.iOS.props b/osu.iOS.props index 72f09ee287..492bf89fab 100644 --- a/osu.iOS.props +++ b/osu.iOS.props @@ -70,7 +70,7 @@ - + @@ -80,7 +80,7 @@ - + From 49aa839872b5291e2df9011c410f8d72edf3823b Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 1 Jul 2020 18:54:11 +0900 Subject: [PATCH 4/7] Update RulesetInputManager to use new method --- osu.Game/Rulesets/UI/RulesetInputManager.cs | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/osu.Game/Rulesets/UI/RulesetInputManager.cs b/osu.Game/Rulesets/UI/RulesetInputManager.cs index ba30fe28d5..f2ac61eaf4 100644 --- a/osu.Game/Rulesets/UI/RulesetInputManager.cs +++ b/osu.Game/Rulesets/UI/RulesetInputManager.cs @@ -18,9 +18,6 @@ using osu.Game.Input.Handlers; using osu.Game.Screens.Play; using osuTK.Input; using static osu.Game.Input.Handlers.ReplayInputHandler; -using JoystickState = osu.Framework.Input.States.JoystickState; -using KeyboardState = osu.Framework.Input.States.KeyboardState; -using MouseState = osu.Framework.Input.States.MouseState; namespace osu.Game.Rulesets.UI { @@ -42,11 +39,7 @@ namespace osu.Game.Rulesets.UI } } - protected override InputState CreateInitialState() - { - var state = base.CreateInitialState(); - return new RulesetInputManagerInputState(state.Mouse, state.Keyboard, state.Joystick); - } + protected override InputState CreateInitialState() => new RulesetInputManagerInputState(base.CreateInitialState()); protected readonly KeyBindingContainer KeyBindingContainer; @@ -203,8 +196,8 @@ namespace osu.Game.Rulesets.UI { public ReplayState LastReplayState; - public RulesetInputManagerInputState(MouseState mouse = null, KeyboardState keyboard = null, JoystickState joystick = null) - : base(mouse, keyboard, joystick) + public RulesetInputManagerInputState(InputState state = null) + : base(state) { } } From 4e839e4f1fb595740caa29f901f7072fc2858f23 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 1 Jul 2020 19:02:05 +0900 Subject: [PATCH 5/7] Fix "welcome" intro test failure due to no wait logic --- .../Visual/Menus/TestSceneIntroWelcome.cs | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/osu.Game.Tests/Visual/Menus/TestSceneIntroWelcome.cs b/osu.Game.Tests/Visual/Menus/TestSceneIntroWelcome.cs index 1347bae2ad..8f20e38494 100644 --- a/osu.Game.Tests/Visual/Menus/TestSceneIntroWelcome.cs +++ b/osu.Game.Tests/Visual/Menus/TestSceneIntroWelcome.cs @@ -2,6 +2,7 @@ // See the LICENCE file in the repository root for full licence text. using NUnit.Framework; +using osu.Framework.Audio.Track; using osu.Framework.Screens; using osu.Game.Screens.Menu; @@ -14,15 +15,11 @@ namespace osu.Game.Tests.Visual.Menus public TestSceneIntroWelcome() { - AddAssert("check if menu music loops", () => - { - var menu = IntroStack?.CurrentScreen as MainMenu; + AddUntilStep("wait for load", () => getTrack() != null); - if (menu == null) - return false; - - return menu.Track.Looping; - }); + AddAssert("check if menu music loops", () => getTrack().Looping); } + + private Track getTrack() => (IntroStack?.CurrentScreen as MainMenu)?.Track; } } From 1edfac4923623a1d78b1379c4f2c7e8e4177a01b Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Wed, 1 Jul 2020 23:21:08 +0900 Subject: [PATCH 6/7] Fix test failing --- osu.Game.Tests/NonVisual/CustomDataDirectoryTest.cs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/osu.Game.Tests/NonVisual/CustomDataDirectoryTest.cs b/osu.Game.Tests/NonVisual/CustomDataDirectoryTest.cs index f3d54d876a..8ea0e34214 100644 --- a/osu.Game.Tests/NonVisual/CustomDataDirectoryTest.cs +++ b/osu.Game.Tests/NonVisual/CustomDataDirectoryTest.cs @@ -127,6 +127,9 @@ namespace osu.Game.Tests.NonVisual var osu = loadOsu(host); var storage = osu.Dependencies.Get(); + // Store the current storage's path. We'll need to refer to this for assertions in the original directory after the migration completes. + string originalDirectory = storage.GetFullPath("."); + // ensure we perform a save host.Dependencies.Get().Save(); @@ -145,25 +148,25 @@ namespace osu.Game.Tests.NonVisual Assert.That(storage.GetFullPath("."), Is.EqualTo(customPath)); // ensure cache was not moved - Assert.That(host.Storage.ExistsDirectory("cache")); + Assert.That(Directory.Exists(Path.Combine(originalDirectory, "cache"))); // ensure nested cache was moved - Assert.That(!host.Storage.ExistsDirectory(Path.Combine("test-nested", "cache"))); + Assert.That(!Directory.Exists(Path.Combine(originalDirectory, "test-nested", "cache"))); Assert.That(storage.ExistsDirectory(Path.Combine("test-nested", "cache"))); foreach (var file in OsuStorage.IGNORE_FILES) { - Assert.That(host.Storage.Exists(file), Is.True); + Assert.That(File.Exists(Path.Combine(originalDirectory, file))); Assert.That(storage.Exists(file), Is.False); } foreach (var dir in OsuStorage.IGNORE_DIRECTORIES) { - Assert.That(host.Storage.ExistsDirectory(dir), Is.True); + Assert.That(Directory.Exists(Path.Combine(originalDirectory, dir))); Assert.That(storage.ExistsDirectory(dir), Is.False); } - Assert.That(new StreamReader(host.Storage.GetStream("storage.ini")).ReadToEnd().Contains($"FullPath = {customPath}")); + Assert.That(new StreamReader(Path.Combine(originalDirectory, "storage.ini")).ReadToEnd().Contains($"FullPath = {customPath}")); } finally { From 5c1f1ab622c8a4e862a7652564b557c76b9514ab Mon Sep 17 00:00:00 2001 From: Joehu Date: Wed, 1 Jul 2020 14:31:06 -0700 Subject: [PATCH 7/7] Fix avatar in score panel being unclickable when statistics panel is visible --- osu.Game/Screens/Ranking/ResultsScreen.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/osu.Game/Screens/Ranking/ResultsScreen.cs b/osu.Game/Screens/Ranking/ResultsScreen.cs index 968b446df9..49ce07b708 100644 --- a/osu.Game/Screens/Ranking/ResultsScreen.cs +++ b/osu.Game/Screens/Ranking/ResultsScreen.cs @@ -79,6 +79,11 @@ namespace osu.Game.Screens.Ranking RelativeSizeAxes = Axes.Both, Children = new Drawable[] { + statisticsPanel = new StatisticsPanel + { + RelativeSizeAxes = Axes.Both, + Score = { BindTarget = SelectedScore } + }, scorePanelList = new ScorePanelList { RelativeSizeAxes = Axes.Both, @@ -89,11 +94,6 @@ namespace osu.Game.Screens.Ranking { RelativeSizeAxes = Axes.Both }, - statisticsPanel = new StatisticsPanel - { - RelativeSizeAxes = Axes.Both, - Score = { BindTarget = SelectedScore } - }, } } },