diff --git a/osu.Game.Rulesets.Mania/UI/Column.cs b/osu.Game.Rulesets.Mania/UI/Column.cs
index 9aabcc6699..d2a9b69b60 100644
--- a/osu.Game.Rulesets.Mania/UI/Column.cs
+++ b/osu.Game.Rulesets.Mania/UI/Column.cs
@@ -97,7 +97,7 @@ namespace osu.Game.Rulesets.Mania.UI
DrawableManiaHitObject maniaObject = (DrawableManiaHitObject)hitObject;
maniaObject.CheckHittable = hitPolicy.IsHittable;
- HitObjectContainer.Add(hitObject);
+ base.Add(hitObject);
}
public override bool Remove(DrawableHitObject h)
diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneDrawableScrollingRuleset.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneDrawableScrollingRuleset.cs
index 1a1babb4a8..9931ee4a45 100644
--- a/osu.Game.Tests/Visual/Gameplay/TestSceneDrawableScrollingRuleset.cs
+++ b/osu.Game.Tests/Visual/Gameplay/TestSceneDrawableScrollingRuleset.cs
@@ -21,13 +21,13 @@ using osu.Game.Rulesets.Difficulty;
using osu.Game.Rulesets.Mods;
using osu.Game.Rulesets.Objects;
using osu.Game.Rulesets.Objects.Drawables;
-using osu.Game.Rulesets.Objects.Legacy;
using osu.Game.Rulesets.Objects.Types;
using osu.Game.Rulesets.Osu;
using osu.Game.Rulesets.UI;
using osu.Game.Rulesets.UI.Scrolling;
using osuTK;
using osuTK.Graphics;
+using JetBrains.Annotations;
namespace osu.Game.Tests.Visual.Gameplay
{
@@ -46,6 +46,50 @@ namespace osu.Game.Tests.Visual.Gameplay
[SetUp]
public void Setup() => Schedule(() => testClock.CurrentTime = 0);
+ [TestCase("pooled")]
+ [TestCase("non-pooled")]
+ public void TestHitObjectLifetime(string pooled)
+ {
+ var beatmap = createBeatmap(_ => pooled == "pooled" ? new TestPooledHitObject() : new TestHitObject());
+ beatmap.ControlPointInfo.Add(0, new TimingControlPoint { BeatLength = time_range });
+ createTest(beatmap);
+
+ assertPosition(0, 0f);
+ assertDead(3);
+
+ setTime(3 * time_range);
+ assertPosition(3, 0f);
+ assertDead(0);
+
+ setTime(0 * time_range);
+ assertPosition(0, 0f);
+ assertDead(3);
+ }
+
+ [TestCase("pooled")]
+ [TestCase("non-pooled")]
+ public void TestNestedHitObject(string pooled)
+ {
+ var beatmap = createBeatmap(i =>
+ {
+ var h = pooled == "pooled" ? new TestPooledParentHitObject() : new TestParentHitObject();
+ h.Duration = 300;
+ h.ChildTimeOffset = i % 3 * 100;
+ return h;
+ });
+ beatmap.ControlPointInfo.Add(0, new TimingControlPoint { BeatLength = time_range });
+ createTest(beatmap);
+
+ assertPosition(0, 0f);
+ assertHeight(0);
+ assertChildPosition(0);
+
+ setTime(5 * time_range);
+ assertPosition(5, 0f);
+ assertHeight(5);
+ assertChildPosition(5);
+ }
+
[Test]
public void TestRelativeBeatLengthScaleSingleTimingPoint()
{
@@ -147,8 +191,37 @@ namespace osu.Game.Tests.Visual.Gameplay
assertPosition(1, 1);
}
+ ///
+ /// Get a corresponding to the 'th .
+ /// When the hit object is not alive, `null` is returned.
+ ///
+ [CanBeNull]
+ private DrawableTestHitObject getDrawableHitObject(int index)
+ {
+ var hitObject = drawableRuleset.Beatmap.HitObjects.ElementAt(index);
+ return (DrawableTestHitObject)drawableRuleset.Playfield.HitObjectContainer.AliveObjects.FirstOrDefault(obj => obj.HitObject == hitObject);
+ }
+
+ private float yScale => drawableRuleset.Playfield.HitObjectContainer.DrawHeight;
+
+ private void assertDead(int index) => AddAssert($"hitobject {index} is dead", () => getDrawableHitObject(index) == null);
+
+ private void assertHeight(int index) => AddAssert($"hitobject {index} height", () =>
+ {
+ var d = getDrawableHitObject(index);
+ return d != null && Precision.AlmostEquals(d.DrawHeight, yScale * (float)(d.HitObject.Duration / time_range), 0.1f);
+ });
+
+ private void assertChildPosition(int index) => AddAssert($"hitobject {index} child position", () =>
+ {
+ var d = getDrawableHitObject(index);
+ return d is DrawableTestParentHitObject && Precision.AlmostEquals(
+ d.NestedHitObjects.First().DrawPosition.Y,
+ yScale * (float)((TestParentHitObject)d.HitObject).ChildTimeOffset / time_range, 0.1f);
+ });
+
private void assertPosition(int index, float relativeY) => AddAssert($"hitobject {index} at {relativeY}",
- () => Precision.AlmostEquals(drawableRuleset.Playfield.AllHitObjects.ElementAt(index).DrawPosition.Y, drawableRuleset.Playfield.HitObjectContainer.DrawHeight * relativeY));
+ () => Precision.AlmostEquals(getDrawableHitObject(index)?.DrawPosition.Y ?? -1, yScale * relativeY));
private void setTime(double time)
{
@@ -160,12 +233,16 @@ namespace osu.Game.Tests.Visual.Gameplay
/// The hitobjects are spaced milliseconds apart.
///
/// The .
- private IBeatmap createBeatmap()
+ private IBeatmap createBeatmap(Func createAction = null)
{
- var beatmap = new Beatmap { BeatmapInfo = { Ruleset = new OsuRuleset().RulesetInfo } };
+ var beatmap = new Beatmap { BeatmapInfo = { Ruleset = new OsuRuleset().RulesetInfo } };
for (int i = 0; i < 10; i++)
- beatmap.HitObjects.Add(new HitObject { StartTime = i * time_range });
+ {
+ var h = createAction?.Invoke(i) ?? new TestHitObject();
+ h.StartTime = i * time_range;
+ beatmap.HitObjects.Add(h);
+ }
return beatmap;
}
@@ -225,7 +302,21 @@ namespace osu.Game.Tests.Visual.Gameplay
TimeRange.Value = time_range;
}
- public override DrawableHitObject CreateDrawableRepresentation(TestHitObject h) => new DrawableTestHitObject(h);
+ public override DrawableHitObject CreateDrawableRepresentation(TestHitObject h)
+ {
+ switch (h)
+ {
+ case TestPooledHitObject _:
+ case TestPooledParentHitObject _:
+ return null;
+
+ case TestParentHitObject p:
+ return new DrawableTestParentHitObject(p);
+
+ default:
+ return new DrawableTestHitObject(h);
+ }
+ }
protected override PassThroughInputManager CreateInputManager() => new PassThroughInputManager();
@@ -265,6 +356,9 @@ namespace osu.Game.Tests.Visual.Gameplay
}
}
});
+
+ RegisterPool(1);
+ RegisterPool(1);
}
}
@@ -277,30 +371,46 @@ namespace osu.Game.Tests.Visual.Gameplay
public override bool CanConvert() => true;
- protected override IEnumerable ConvertHitObject(HitObject original, IBeatmap beatmap, CancellationToken cancellationToken)
- {
- yield return new TestHitObject
- {
- StartTime = original.StartTime,
- Duration = (original as IHasDuration)?.Duration ?? 100
- };
- }
+ protected override IEnumerable ConvertHitObject(HitObject original, IBeatmap beatmap, CancellationToken cancellationToken) =>
+ throw new NotImplementedException();
}
#endregion
#region HitObject
- private class TestHitObject : ConvertHitObject, IHasDuration
+ private class TestHitObject : HitObject, IHasDuration
{
public double EndTime => StartTime + Duration;
- public double Duration { get; set; }
+ public double Duration { get; set; } = 100;
+ }
+
+ private class TestPooledHitObject : TestHitObject
+ {
+ }
+
+ private class TestParentHitObject : TestHitObject
+ {
+ public double ChildTimeOffset;
+
+ protected override void CreateNestedHitObjects(CancellationToken cancellationToken)
+ {
+ AddNested(new TestHitObject { StartTime = StartTime + ChildTimeOffset });
+ }
+ }
+
+ private class TestPooledParentHitObject : TestParentHitObject
+ {
+ protected override void CreateNestedHitObjects(CancellationToken cancellationToken)
+ {
+ AddNested(new TestPooledHitObject { StartTime = StartTime + ChildTimeOffset });
+ }
}
private class DrawableTestHitObject : DrawableHitObject
{
- public DrawableTestHitObject(TestHitObject hitObject)
+ public DrawableTestHitObject([CanBeNull] TestHitObject hitObject)
: base(hitObject)
{
Anchor = Anchor.TopCentre;
@@ -324,6 +434,52 @@ namespace osu.Game.Tests.Visual.Gameplay
}
});
}
+
+ protected override void Update() => LifetimeEnd = HitObject.EndTime;
+ }
+
+ private class DrawableTestPooledHitObject : DrawableTestHitObject
+ {
+ public DrawableTestPooledHitObject()
+ : base(null)
+ {
+ InternalChildren[0].Colour = Color4.LightSkyBlue;
+ InternalChildren[1].Colour = Color4.Blue;
+ }
+ }
+
+ private class DrawableTestParentHitObject : DrawableTestHitObject
+ {
+ private readonly Container container;
+
+ public DrawableTestParentHitObject([CanBeNull] TestHitObject hitObject)
+ : base(hitObject)
+ {
+ InternalChildren[0].Colour = Color4.LightYellow;
+ InternalChildren[1].Colour = Color4.Yellow;
+
+ AddInternal(container = new Container
+ {
+ RelativeSizeAxes = Axes.Both,
+ });
+ }
+
+ protected override DrawableHitObject CreateNestedHitObject(HitObject hitObject) =>
+ new DrawableTestHitObject((TestHitObject)hitObject);
+
+ protected override void AddNestedHitObject(DrawableHitObject hitObject) => container.Add(hitObject);
+
+ protected override void ClearNestedHitObjects() => container.Clear(false);
+ }
+
+ private class DrawableTestPooledParentHitObject : DrawableTestParentHitObject
+ {
+ public DrawableTestPooledParentHitObject()
+ : base(null)
+ {
+ InternalChildren[0].Colour = Color4.LightSeaGreen;
+ InternalChildren[1].Colour = Color4.Green;
+ }
}
#endregion
diff --git a/osu.Game/Rulesets/UI/HitObjectContainer.cs b/osu.Game/Rulesets/UI/HitObjectContainer.cs
index 5fbda305c8..ac5d281ddc 100644
--- a/osu.Game/Rulesets/UI/HitObjectContainer.cs
+++ b/osu.Game/Rulesets/UI/HitObjectContainer.cs
@@ -114,6 +114,7 @@ namespace osu.Game.Rulesets.UI
bindStartTime(drawable);
AddInternal(drawableMap[entry] = drawable, false);
+ OnAdd(drawable);
HitObjectUsageBegan?.Invoke(entry.HitObject);
}
@@ -129,6 +130,7 @@ namespace osu.Game.Rulesets.UI
drawableMap.Remove(entry);
+ OnRemove(drawable);
unbindStartTime(drawable);
RemoveInternal(drawable);
@@ -147,10 +149,12 @@ namespace osu.Game.Rulesets.UI
hitObject.OnRevertResult += onRevertResult;
AddInternal(hitObject);
+ OnAdd(hitObject);
}
public virtual bool Remove(DrawableHitObject hitObject)
{
+ OnRemove(hitObject);
if (!RemoveInternal(hitObject))
return false;
@@ -178,6 +182,26 @@ namespace osu.Game.Rulesets.UI
#endregion
+ ///
+ /// Invoked when a is added to this container.
+ ///
+ ///
+ /// This method is not invoked for nested s.
+ ///
+ protected virtual void OnAdd(DrawableHitObject drawableHitObject)
+ {
+ }
+
+ ///
+ /// Invoked when a is removed from this container.
+ ///
+ ///
+ /// This method is not invoked for nested s.
+ ///
+ protected virtual void OnRemove(DrawableHitObject drawableHitObject)
+ {
+ }
+
public virtual void Clear(bool disposeChildren = true)
{
lifetimeManager.ClearEntries();
diff --git a/osu.Game/Rulesets/UI/Playfield.cs b/osu.Game/Rulesets/UI/Playfield.cs
index 2f589f4ce9..a2ac234471 100644
--- a/osu.Game/Rulesets/UI/Playfield.cs
+++ b/osu.Game/Rulesets/UI/Playfield.cs
@@ -135,10 +135,8 @@ namespace osu.Game.Rulesets.UI
/// The DrawableHitObject to add.
public virtual void Add(DrawableHitObject h)
{
- if (h.IsInitialized)
- throw new InvalidOperationException($"{nameof(Add)} doesn't support {nameof(DrawableHitObject)} reuse. Use pooling instead.");
-
- onNewDrawableHitObject(h);
+ if (!h.IsInitialized)
+ onNewDrawableHitObject(h);
HitObjectContainer.Add(h);
OnHitObjectAdded(h.HitObject);
diff --git a/osu.Game/Rulesets/UI/Scrolling/ScrollingHitObjectContainer.cs b/osu.Game/Rulesets/UI/Scrolling/ScrollingHitObjectContainer.cs
index bf64175468..3a5e3c098f 100644
--- a/osu.Game/Rulesets/UI/Scrolling/ScrollingHitObjectContainer.cs
+++ b/osu.Game/Rulesets/UI/Scrolling/ScrollingHitObjectContainer.cs
@@ -2,13 +2,10 @@
// See the LICENCE file in the repository root for full licence text.
using System.Collections.Generic;
-using JetBrains.Annotations;
using osu.Framework.Allocation;
using osu.Framework.Bindables;
-using osu.Framework.Caching;
using osu.Framework.Graphics;
using osu.Framework.Layout;
-using osu.Framework.Threading;
using osu.Game.Rulesets.Objects.Drawables;
using osu.Game.Rulesets.Objects.Types;
using osuTK;
@@ -19,7 +16,16 @@ namespace osu.Game.Rulesets.UI.Scrolling
{
private readonly IBindable timeRange = new BindableDouble();
private readonly IBindable direction = new Bindable();
- private readonly Dictionary hitObjectInitialStateCache = new Dictionary();
+
+ ///
+ /// Hit objects which require lifetime computation in the next update call.
+ ///
+ private readonly HashSet toComputeLifetime = new HashSet();
+
+ ///
+ /// A set containing all which have an up-to-date layout.
+ ///
+ private readonly HashSet layoutComputed = new HashSet();
[Resolved]
private IScrollingInfo scrollingInfo { get; set; }
@@ -27,10 +33,6 @@ namespace osu.Game.Rulesets.UI.Scrolling
// Responds to changes in the layout. When the layout changes, all hit object states must be recomputed.
private readonly LayoutValue layoutCache = new LayoutValue(Invalidation.RequiredParentSizeToFit | Invalidation.DrawInfo);
- // A combined cache across all hit object states to reduce per-update iterations.
- // When invalidated, one or more (but not necessarily all) hitobject states must be re-validated.
- private readonly Cached combinedObjCache = new Cached();
-
public ScrollingHitObjectContainer()
{
RelativeSizeAxes = Axes.Both;
@@ -48,37 +50,12 @@ namespace osu.Game.Rulesets.UI.Scrolling
timeRange.ValueChanged += _ => layoutCache.Invalidate();
}
- public override void Add(DrawableHitObject hitObject)
- {
- combinedObjCache.Invalidate();
- hitObject.DefaultsApplied += onDefaultsApplied;
- base.Add(hitObject);
- }
-
- public override bool Remove(DrawableHitObject hitObject)
- {
- var result = base.Remove(hitObject);
-
- if (result)
- {
- combinedObjCache.Invalidate();
- hitObjectInitialStateCache.Remove(hitObject);
-
- hitObject.DefaultsApplied -= onDefaultsApplied;
- }
-
- return result;
- }
-
public override void Clear(bool disposeChildren = true)
{
- foreach (var h in Objects)
- h.DefaultsApplied -= onDefaultsApplied;
-
base.Clear(disposeChildren);
- combinedObjCache.Invalidate();
- hitObjectInitialStateCache.Clear();
+ toComputeLifetime.Clear();
+ layoutComputed.Clear();
}
///
@@ -173,15 +150,40 @@ namespace osu.Game.Rulesets.UI.Scrolling
}
}
- private void onDefaultsApplied(DrawableHitObject drawableObject)
+ protected override void OnAdd(DrawableHitObject drawableHitObject) => onAddRecursive(drawableHitObject);
+
+ protected override void OnRemove(DrawableHitObject drawableHitObject) => onRemoveRecursive(drawableHitObject);
+
+ private void onAddRecursive(DrawableHitObject hitObject)
{
- // The cache may not exist if the hitobject state hasn't been computed yet (e.g. if the hitobject was added + defaults applied in the same frame).
- // In such a case, combinedObjCache will take care of updating the hitobject.
- if (hitObjectInitialStateCache.TryGetValue(drawableObject, out var state))
- {
- combinedObjCache.Invalidate();
- state.Cache.Invalidate();
- }
+ invalidateHitObject(hitObject);
+
+ hitObject.DefaultsApplied += invalidateHitObject;
+
+ foreach (var nested in hitObject.NestedHitObjects)
+ onAddRecursive(nested);
+ }
+
+ private void onRemoveRecursive(DrawableHitObject hitObject)
+ {
+ toComputeLifetime.Remove(hitObject);
+ layoutComputed.Remove(hitObject);
+
+ hitObject.DefaultsApplied -= invalidateHitObject;
+
+ foreach (var nested in hitObject.NestedHitObjects)
+ onRemoveRecursive(nested);
+ }
+
+ ///
+ /// Make this lifetime and layout computed in next update.
+ ///
+ private void invalidateHitObject(DrawableHitObject hitObject)
+ {
+ // Lifetime computation is delayed until next update because
+ // when the hit object is not pooled this container is not loaded here and `scrollLength` cannot be computed.
+ toComputeLifetime.Add(hitObject);
+ layoutComputed.Remove(hitObject);
}
private float scrollLength;
@@ -192,17 +194,18 @@ namespace osu.Game.Rulesets.UI.Scrolling
if (!layoutCache.IsValid)
{
- foreach (var state in hitObjectInitialStateCache.Values)
- state.Cache.Invalidate();
- combinedObjCache.Invalidate();
+ toComputeLifetime.Clear();
+
+ foreach (var hitObject in Objects)
+ {
+ if (hitObject.HitObject != null)
+ toComputeLifetime.Add(hitObject);
+ }
+
+ layoutComputed.Clear();
scrollingInfo.Algorithm.Reset();
- layoutCache.Validate();
- }
-
- if (!combinedObjCache.IsValid)
- {
switch (direction.Value)
{
case ScrollingDirection.Up:
@@ -215,32 +218,24 @@ namespace osu.Game.Rulesets.UI.Scrolling
break;
}
- foreach (var obj in Objects)
- {
- if (!hitObjectInitialStateCache.TryGetValue(obj, out var state))
- state = hitObjectInitialStateCache[obj] = new InitialState(new Cached());
-
- if (state.Cache.IsValid)
- continue;
-
- state.ScheduledComputation?.Cancel();
- state.ScheduledComputation = computeInitialStateRecursive(obj);
-
- computeLifetimeStartRecursive(obj);
-
- state.Cache.Validate();
- }
-
- combinedObjCache.Validate();
+ layoutCache.Validate();
}
- }
- private void computeLifetimeStartRecursive(DrawableHitObject hitObject)
- {
- hitObject.LifetimeStart = computeOriginAdjustedLifetimeStart(hitObject);
+ foreach (var hitObject in toComputeLifetime)
+ hitObject.LifetimeStart = computeOriginAdjustedLifetimeStart(hitObject);
- foreach (var obj in hitObject.NestedHitObjects)
- computeLifetimeStartRecursive(obj);
+ toComputeLifetime.Clear();
+
+ // only AliveObjects need to be considered for layout (reduces overhead in the case of scroll speed changes).
+ foreach (var obj in AliveObjects)
+ {
+ if (layoutComputed.Contains(obj))
+ continue;
+
+ updateLayoutRecursive(obj);
+
+ layoutComputed.Add(obj);
+ }
}
private double computeOriginAdjustedLifetimeStart(DrawableHitObject hitObject)
@@ -271,7 +266,7 @@ namespace osu.Game.Rulesets.UI.Scrolling
return scrollingInfo.Algorithm.GetDisplayStartTime(hitObject.HitObject.StartTime, originAdjustment, timeRange.Value, scrollLength);
}
- private ScheduledDelegate computeInitialStateRecursive(DrawableHitObject hitObject) => hitObject.Schedule(() =>
+ private void updateLayoutRecursive(DrawableHitObject hitObject)
{
if (hitObject.HitObject is IHasDuration e)
{
@@ -291,12 +286,12 @@ namespace osu.Game.Rulesets.UI.Scrolling
foreach (var obj in hitObject.NestedHitObjects)
{
- computeInitialStateRecursive(obj);
+ updateLayoutRecursive(obj);
// Nested hitobjects don't need to scroll, but they do need accurate positions
updatePosition(obj, hitObject.HitObject.StartTime);
}
- });
+ }
protected override void UpdateAfterChildrenLife()
{
@@ -328,19 +323,5 @@ namespace osu.Game.Rulesets.UI.Scrolling
break;
}
}
-
- private class InitialState
- {
- [NotNull]
- public readonly Cached Cache;
-
- [CanBeNull]
- public ScheduledDelegate ScheduledComputation;
-
- public InitialState(Cached cache)
- {
- Cache = cache;
- }
- }
}
}
diff --git a/osu.Game/Rulesets/UI/Scrolling/ScrollingPlayfield.cs b/osu.Game/Rulesets/UI/Scrolling/ScrollingPlayfield.cs
index 9dac3f4de1..2b75f93f9e 100644
--- a/osu.Game/Rulesets/UI/Scrolling/ScrollingPlayfield.cs
+++ b/osu.Game/Rulesets/UI/Scrolling/ScrollingPlayfield.cs
@@ -15,6 +15,8 @@ namespace osu.Game.Rulesets.UI.Scrolling
{
protected readonly IBindable Direction = new Bindable();
+ public new ScrollingHitObjectContainer HitObjectContainer => (ScrollingHitObjectContainer)base.HitObjectContainer;
+
[Resolved]
protected IScrollingInfo ScrollingInfo { get; private set; }
@@ -27,14 +29,12 @@ namespace osu.Game.Rulesets.UI.Scrolling
///
/// Given a position in screen space, return the time within this column.
///
- public virtual double TimeAtScreenSpacePosition(Vector2 screenSpacePosition) =>
- ((ScrollingHitObjectContainer)HitObjectContainer).TimeAtScreenSpacePosition(screenSpacePosition);
+ public virtual double TimeAtScreenSpacePosition(Vector2 screenSpacePosition) => HitObjectContainer.TimeAtScreenSpacePosition(screenSpacePosition);
///
/// Given a time, return the screen space position within this column.
///
- public virtual Vector2 ScreenSpacePositionAtTime(double time)
- => ((ScrollingHitObjectContainer)HitObjectContainer).ScreenSpacePositionAtTime(time);
+ public virtual Vector2 ScreenSpacePositionAtTime(double time) => HitObjectContainer.ScreenSpacePositionAtTime(time);
protected sealed override HitObjectContainer CreateHitObjectContainer() => new ScrollingHitObjectContainer();
}