From ba7f22997b515911663d7cbb214bf69e0352f68b Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 20 Sep 2023 16:35:35 +0900 Subject: [PATCH 01/43] Make `InterpolatingFramedClock`'s sources non-null --- .../Timing/InterpolatingFramedClock.cs | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/osu.Framework/Timing/InterpolatingFramedClock.cs b/osu.Framework/Timing/InterpolatingFramedClock.cs index 898e586889..a47f6d458b 100644 --- a/osu.Framework/Timing/InterpolatingFramedClock.cs +++ b/osu.Framework/Timing/InterpolatingFramedClock.cs @@ -2,6 +2,7 @@ // See the LICENCE file in the repository root for full licence text. using System; +using System.Diagnostics; namespace osu.Framework.Timing { @@ -27,23 +28,19 @@ public class InterpolatingFramedClock : IFrameBasedClock, ISourceChangeableClock /// /// The drift in milliseconds between the source and interpolation at the last processed frame. /// - public double Drift => CurrentTime - (FramedSourceClock?.CurrentTime ?? 0); + public double Drift => CurrentTime - FramedSourceClock.CurrentTime; - public virtual double Rate - { - get => FramedSourceClock?.Rate ?? 1; - set => throw new NotSupportedException(); - } + public virtual double Rate => FramedSourceClock.Rate; public virtual bool IsRunning => sourceIsRunning; public virtual double ElapsedFrameTime => currentInterpolatedTime - lastInterpolatedTime; - public IClock? Source { get; private set; } + public IClock Source { get; private set; } - public virtual double CurrentTime => currentTime; + protected IFrameBasedClock FramedSourceClock; - protected IFrameBasedClock? FramedSourceClock; + public virtual double CurrentTime => currentTime; private readonly FramedClock realtimeClock = new FramedClock(new StopwatchClock(true)); @@ -58,6 +55,8 @@ public virtual double Rate public InterpolatingFramedClock(IClock? source = null) { ChangeSource(source); + Debug.Assert(Source != null); + Debug.Assert(FramedSourceClock != null); } public virtual void ChangeSource(IClock? source) @@ -73,8 +72,6 @@ public virtual void ChangeSource(IClock? source) public virtual void ProcessFrame() { - if (FramedSourceClock == null) return; - realtimeClock.ProcessFrame(); FramedSourceClock.ProcessFrame(); From 39497f3f2acb4e9bdf091870a9f11bc694ee9432 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 20 Sep 2023 16:39:08 +0900 Subject: [PATCH 02/43] Update `TestSceneClock` to mark game-wide clock with a name --- osu.Framework.Tests/Visual/Clocks/TestSceneClock.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/osu.Framework.Tests/Visual/Clocks/TestSceneClock.cs b/osu.Framework.Tests/Visual/Clocks/TestSceneClock.cs index b82764f3ac..9d1ee7f9aa 100644 --- a/osu.Framework.Tests/Visual/Clocks/TestSceneClock.cs +++ b/osu.Framework.Tests/Visual/Clocks/TestSceneClock.cs @@ -30,18 +30,18 @@ protected TestSceneClock() { fill.Clear(); lastClock = null; - AddClock(Clock); + AddClock(Clock, "game"); }); } private IClock? lastClock; - protected IClock AddClock(IClock clock) + protected IClock AddClock(IClock clock, string? name = null) { if (lastClock != null && clock is ISourceChangeableClock framed) framed.ChangeSource(lastClock); - fill.Add(new VisualClock(lastClock = clock)); + fill.Add(new VisualClock(lastClock = clock, name)); return clock; } @@ -59,7 +59,7 @@ public partial class VisualClock : CompositeDrawable private readonly Box bg; private readonly Box hand; - public VisualClock(IClock clock) + public VisualClock(IClock clock, string? name = null) { this.clock = clock; @@ -79,7 +79,7 @@ public VisualClock(IClock clock) }, new SpriteText { - Text = clock.GetType().Name, + Text = clock.GetType().Name + (!string.IsNullOrEmpty(name) ? $" ({name})" : string.Empty), Anchor = Anchor.Centre, Origin = Anchor.Centre, Y = -25, From 8044cbb4f855bc88693dfee66bdfdf857548f3b9 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 20 Sep 2023 16:38:26 +0900 Subject: [PATCH 03/43] Add basic implementation of `DecouplingFramedClock` --- osu.Framework/Timing/DecouplingFramedClock.cs | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) create mode 100644 osu.Framework/Timing/DecouplingFramedClock.cs diff --git a/osu.Framework/Timing/DecouplingFramedClock.cs b/osu.Framework/Timing/DecouplingFramedClock.cs new file mode 100644 index 0000000000..185d966f63 --- /dev/null +++ b/osu.Framework/Timing/DecouplingFramedClock.cs @@ -0,0 +1,60 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System.Diagnostics; + +namespace osu.Framework.Timing +{ + public class DecouplingFramedClock : IFrameBasedClock, ISourceChangeableClock + { + public virtual double Rate => FramedSourceClock.Rate; + + public virtual bool IsRunning => sourceIsRunning; + + public virtual double ElapsedFrameTime => FramedSourceClock.ElapsedFrameTime; + + public IClock Source { get; private set; } + + protected IFrameBasedClock FramedSourceClock; + + public virtual double CurrentTime => currentTime; + + private readonly FramedClock realtimeClock = new FramedClock(new StopwatchClock(true)); + + private bool sourceIsRunning; + + private double currentTime; + + public DecouplingFramedClock(IClock? source = null) + { + ChangeSource(source); + Debug.Assert(Source != null); + Debug.Assert(FramedSourceClock != null); + } + + public void ChangeSource(IClock? source) + { + Source = source ?? new StopwatchClock(true); + + // We need a frame-based source to correctly process interpolation. + // If the provided source is not already a framed clock, encapsulate it in one. + FramedSourceClock = Source as IFrameBasedClock ?? new FramedClock(source); + } + + public virtual void ProcessFrame() + { + realtimeClock.ProcessFrame(); + FramedSourceClock.ProcessFrame(); + + sourceIsRunning = FramedSourceClock.IsRunning; + + if (FramedSourceClock.IsRunning) + { + } + + currentTime = FramedSourceClock.CurrentTime; + } + + double IFrameBasedClock.FramesPerSecond => 0; + } +} From f9b3a22fe3f324b8c6311cf634457520b78601b7 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 20 Sep 2023 17:13:29 +0900 Subject: [PATCH 04/43] Complete implementation of `DecouplingFramedClock` for pass-through mode --- osu.Framework/Timing/DecouplingFramedClock.cs | 93 +++++++++++++++---- osu.Framework/Timing/ManualFramedClock.cs | 21 +++++ 2 files changed, 94 insertions(+), 20 deletions(-) create mode 100644 osu.Framework/Timing/ManualFramedClock.cs diff --git a/osu.Framework/Timing/DecouplingFramedClock.cs b/osu.Framework/Timing/DecouplingFramedClock.cs index 185d966f63..0c57e4b908 100644 --- a/osu.Framework/Timing/DecouplingFramedClock.cs +++ b/osu.Framework/Timing/DecouplingFramedClock.cs @@ -1,60 +1,113 @@ // 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.Diagnostics; namespace osu.Framework.Timing { - public class DecouplingFramedClock : IFrameBasedClock, ISourceChangeableClock + public class DecouplingFramedClock : IFrameBasedClock, ISourceChangeableClock, IAdjustableClock { - public virtual double Rate => FramedSourceClock.Rate; - - public virtual bool IsRunning => sourceIsRunning; - - public virtual double ElapsedFrameTime => FramedSourceClock.ElapsedFrameTime; + public bool AllowDecoupling = true; public IClock Source { get; private set; } - protected IFrameBasedClock FramedSourceClock; - - public virtual double CurrentTime => currentTime; - - private readonly FramedClock realtimeClock = new FramedClock(new StopwatchClock(true)); + /// + /// This clock is used when we are decoupling from the source, to figure out how much elapsed time has passed between frames. + /// + private readonly FramedClock realtimeReferenceClock = new FramedClock(new StopwatchClock(true)); - private bool sourceIsRunning; + /// + /// This clock is used to set final outputs on. + /// + private readonly ManualFramedClock outputClock = new ManualFramedClock(); - private double currentTime; + private IFrameBasedClock framedSourceClock; + private IAdjustableClock adjustableSourceClock; public DecouplingFramedClock(IClock? source = null) { ChangeSource(source); Debug.Assert(Source != null); - Debug.Assert(FramedSourceClock != null); + Debug.Assert(framedSourceClock != null); + Debug.Assert(adjustableSourceClock != null); } public void ChangeSource(IClock? source) { Source = source ?? new StopwatchClock(true); + if (Source is not IAdjustableClock adjustableSource) + throw new ArgumentException($"Clock must be of type {nameof(IAdjustableClock)}"); + + adjustableSourceClock = adjustableSource; + // We need a frame-based source to correctly process interpolation. // If the provided source is not already a framed clock, encapsulate it in one. - FramedSourceClock = Source as IFrameBasedClock ?? new FramedClock(source); + framedSourceClock = Source as IFrameBasedClock ?? new FramedClock(source); } public virtual void ProcessFrame() { - realtimeClock.ProcessFrame(); - FramedSourceClock.ProcessFrame(); + updateRealtimeReference(); - sourceIsRunning = FramedSourceClock.IsRunning; + framedSourceClock.ProcessFrame(); - if (FramedSourceClock.IsRunning) + if (AllowDecoupling) { } - currentTime = FramedSourceClock.CurrentTime; + outputClock.Rate = framedSourceClock.Rate; + outputClock.CurrentTime = framedSourceClock.CurrentTime; + outputClock.IsRunning = framedSourceClock.IsRunning; + outputClock.ElapsedFrameTime = framedSourceClock.ElapsedFrameTime; } + private void updateRealtimeReference() + { + ((StopwatchClock)realtimeReferenceClock.Source).Rate = framedSourceClock.Rate; + realtimeReferenceClock.ProcessFrame(); + } + + #region IAdjustableClock implementation + + public void Reset() + { + adjustableSourceClock.Reset(); + } + + public void Start() + { + adjustableSourceClock.Start(); + } + + public void Stop() + { + adjustableSourceClock.Stop(); + } + + public bool Seek(double position) + { + return adjustableSourceClock.Seek(position); + } + + public void ResetSpeedAdjustments() => adjustableSourceClock.ResetSpeedAdjustments(); + + public virtual double Rate + { + get => adjustableSourceClock.Rate; + set => adjustableSourceClock.Rate = value; + } + + #endregion + + # region IFrameBasedClock delegation + + public double ElapsedFrameTime => outputClock.ElapsedFrameTime; + public bool IsRunning => outputClock.IsRunning; + public virtual double CurrentTime => outputClock.CurrentTime; double IFrameBasedClock.FramesPerSecond => 0; + + #endregion } } diff --git a/osu.Framework/Timing/ManualFramedClock.cs b/osu.Framework/Timing/ManualFramedClock.cs new file mode 100644 index 0000000000..e4f4c11545 --- /dev/null +++ b/osu.Framework/Timing/ManualFramedClock.cs @@ -0,0 +1,21 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +namespace osu.Framework.Timing +{ + /// + /// A completely manual framed clock implementation. Everything is settable. + /// + public class ManualFramedClock : IFrameBasedClock + { + public double CurrentTime { get; set; } + public double Rate { get; set; } + public bool IsRunning { get; set; } + public double ElapsedFrameTime { get; set; } + public double FramesPerSecond { get; set; } + + public void ProcessFrame() + { + } + } +} From 37cd19b2cbbf4411cd1acb6ed2ebf5b60b93d1f5 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 20 Sep 2023 18:01:06 +0900 Subject: [PATCH 05/43] Fix rate/running states not being updated immediately --- osu.Framework/Timing/DecouplingFramedClock.cs | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/osu.Framework/Timing/DecouplingFramedClock.cs b/osu.Framework/Timing/DecouplingFramedClock.cs index 0c57e4b908..c089d8b6d3 100644 --- a/osu.Framework/Timing/DecouplingFramedClock.cs +++ b/osu.Framework/Timing/DecouplingFramedClock.cs @@ -17,14 +17,11 @@ public class DecouplingFramedClock : IFrameBasedClock, ISourceChangeableClock, I /// private readonly FramedClock realtimeReferenceClock = new FramedClock(new StopwatchClock(true)); - /// - /// This clock is used to set final outputs on. - /// - private readonly ManualFramedClock outputClock = new ManualFramedClock(); - private IFrameBasedClock framedSourceClock; private IAdjustableClock adjustableSourceClock; + private bool isRunningDecoupled; + public DecouplingFramedClock(IClock? source = null) { ChangeSource(source); @@ -57,10 +54,8 @@ public virtual void ProcessFrame() { } - outputClock.Rate = framedSourceClock.Rate; - outputClock.CurrentTime = framedSourceClock.CurrentTime; - outputClock.IsRunning = framedSourceClock.IsRunning; - outputClock.ElapsedFrameTime = framedSourceClock.ElapsedFrameTime; + CurrentTime = framedSourceClock.CurrentTime; + ElapsedFrameTime = framedSourceClock.ElapsedFrameTime; } private void updateRealtimeReference() @@ -103,9 +98,9 @@ public virtual double Rate # region IFrameBasedClock delegation - public double ElapsedFrameTime => outputClock.ElapsedFrameTime; - public bool IsRunning => outputClock.IsRunning; - public virtual double CurrentTime => outputClock.CurrentTime; + public double ElapsedFrameTime { get; private set; } + public bool IsRunning => framedSourceClock.IsRunning ? true : isRunningDecoupled; + public virtual double CurrentTime { get; private set; } double IFrameBasedClock.FramesPerSecond => 0; #endregion From 6fbd71a26340c06121c4641d32f8a6e76b1c6c88 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 20 Sep 2023 17:18:48 +0900 Subject: [PATCH 06/43] Update tests to test new class --- .../Clocks/DecouplingClockTest.cs | 457 ++++++++++++++++++ 1 file changed, 457 insertions(+) create mode 100644 osu.Framework.Tests/Clocks/DecouplingClockTest.cs diff --git a/osu.Framework.Tests/Clocks/DecouplingClockTest.cs b/osu.Framework.Tests/Clocks/DecouplingClockTest.cs new file mode 100644 index 0000000000..1eef6c27e6 --- /dev/null +++ b/osu.Framework.Tests/Clocks/DecouplingClockTest.cs @@ -0,0 +1,457 @@ +// 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 NUnit.Framework; +using osu.Framework.Timing; + +namespace osu.Framework.Tests.Clocks +{ + [TestFixture] + public class DecouplingClockTest + { + private TestClockWithRange source = null!; + private DecouplingFramedClock decoupleable = null!; + + [SetUp] + public void SetUp() + { + source = new TestClockWithRange(); + + decoupleable = new DecouplingFramedClock(); + decoupleable.ChangeSource(source); + } + + #region Start/stop by decoupleable + + /// + /// Tests that the source clock starts when the coupled clock starts. + /// + [Test] + public void TestSourceStartedByCoupled() + { + decoupleable.Start(); + + Assert.IsTrue(source.IsRunning, "Source should be running."); + } + + /// + /// Tests that the source clock stops when the coupled clock stops. + /// + [Test] + public void TestSourceStoppedByCoupled() + { + decoupleable.Start(); + decoupleable.Stop(); + + Assert.IsFalse(source.IsRunning, "Source should not be running."); + } + + /// + /// Tests that the source clock starts when the decoupled clock starts. + /// + [Test] + public void TestSourceStartedByDecoupled() + { + decoupleable.AllowDecoupling = true; + decoupleable.Start(); + + Assert.IsTrue(source.IsRunning, "Source should be running."); + } + + /// + /// Tests that the source clock stops when the decoupled clock stops. + /// + [Test] + public void TestSourceStoppedByDecoupled() + { + decoupleable.Start(); + + decoupleable.AllowDecoupling = true; + decoupleable.Stop(); + + Assert.IsFalse(source.IsRunning, "Source should not be running."); + } + + #endregion + + #region Start/stop by source + + /// + /// Tests that the coupled clock starts when the source clock starts. + /// + [Test] + public void TestCoupledStartedBySourceClock() + { + source.Start(); + decoupleable.ProcessFrame(); + + Assert.IsTrue(decoupleable.IsRunning, "Coupled should be running."); + } + + /// + /// Tests that the coupled clock stops when the source clock stops. + /// + [Test] + public void TestCoupledStoppedBySourceClock() + { + decoupleable.Start(); + + source.Stop(); + decoupleable.ProcessFrame(); + + Assert.IsFalse(decoupleable.IsRunning, "Coupled should not be running."); + Assert.That(decoupleable.CurrentTime, Is.EqualTo(source.CurrentTime)); + } + + /// + /// Tests that the decoupled clock doesn't start when the source clock starts. + /// + [Test] + public void TestDecoupledNotStartedBySourceClock() + { + decoupleable.AllowDecoupling = true; + + source.Start(); + decoupleable.ProcessFrame(); + + Assert.IsFalse(decoupleable.IsRunning, "Decoupled should not be running."); + } + + /// + /// Tests that the decoupled clock doesn't stop when the source clock stops. + /// + [Test] + public void TestDecoupledNotStoppedBySourceClock() + { + decoupleable.Start(); + decoupleable.AllowDecoupling = true; + + source.Stop(); + decoupleable.ProcessFrame(); + + Assert.IsTrue(decoupleable.IsRunning, "Decoupled should be running."); + } + + #endregion + + #region Source changes + + [Test] + public void SourceChangeTransfersValueAdjustable() + { + // For decoupled clocks, value transfer is preferred in the direction of the track if possible. + // In other words, we want to keep our current time even if the source changes, as long as the source supports it. + // + // This tests the case where it is supported. + + const double first_source_time = 256000; + const double second_source_time = 128000; + + source.Seek(first_source_time); + source.Start(); + + var secondSource = new TestClock + { + // importantly, test a value lower than the original source. + // this is to both test value transfer *and* the case where time is going backwards, as + // some clocks have special provisions for this. + CurrentTime = second_source_time + }; + + decoupleable.ProcessFrame(); + Assert.That(decoupleable.CurrentTime, Is.EqualTo(first_source_time)); + + decoupleable.ChangeSource(secondSource); + decoupleable.ProcessFrame(); + + Assert.That(secondSource.CurrentTime, Is.EqualTo(first_source_time)); + Assert.That(decoupleable.CurrentTime, Is.EqualTo(first_source_time)); + } + + [Test] + public void SourceChangeTransfersValueNonAdjustable() + { + // For decoupled clocks, value transfer is preferred in the direction of the track if possible. + // In other words, we want to keep our current time even if the source changes, as long as the source supports it. + // + // This tests the case where it is NOT supported. + + const double first_source_time = 256000; + const double second_source_time = 128000; + + source.Seek(first_source_time); + source.Start(); + + var secondSource = new TestNonAdjustableClock + { + // importantly, test a value lower than the original source. + // this is to both test value transfer *and* the case where time is going backwards, as + // some clocks have special provisions for this. + CurrentTime = second_source_time + }; + + decoupleable.ProcessFrame(); + Assert.That(decoupleable.CurrentTime, Is.EqualTo(first_source_time)); + + decoupleable.ChangeSource(secondSource); + decoupleable.ProcessFrame(); + + Assert.That(secondSource.CurrentTime, Is.EqualTo(second_source_time)); + Assert.That(decoupleable.CurrentTime, Is.EqualTo(second_source_time)); + } + + #endregion + + #region Offset start + + /// + /// Tests that the coupled clock seeks to the correct position when the source clock starts. + /// + [Test] + public void TestCoupledStartBySourceWithSourceOffset() + { + source.Seek(1000); + + source.Start(); + decoupleable.ProcessFrame(); + + Assert.AreEqual(source.CurrentTime, decoupleable.CurrentTime, "Coupled time should match source time."); + } + + /// + /// Tests that the coupled clock seeks the source clock to its time when it starts. + /// + [Test] + public void TestCoupledStartWithSouceOffset() + { + source.Seek(1000); + decoupleable.Start(); + + Assert.AreEqual(0, source.CurrentTime); + Assert.AreEqual(source.CurrentTime, decoupleable.CurrentTime, "Coupled time should match source time."); + } + + [Test] + public void TestFromNegativeCoupledMode() + { + decoupleable.AllowDecoupling = false; + decoupleable.Seek(-1000); + + decoupleable.ProcessFrame(); + + Assert.AreEqual(0, source.CurrentTime); + Assert.AreEqual(0, decoupleable.CurrentTime); + } + + /// + /// Tests that the decoupled clocks starts the source as a result of being able to handle the current time. + /// + [Test] + public void TestDecoupledStartsSourceIfAllowable() + { + decoupleable.AllowDecoupling = true; + // decoupleable.CustomAllowableErrorMilliseconds = 1000; + decoupleable.Seek(-50); + decoupleable.ProcessFrame(); + decoupleable.Start(); + + // Delay a bit to make sure the clock crosses the 0 boundary + Thread.Sleep(100); + decoupleable.ProcessFrame(); + + Assert.That(source.IsRunning, Is.True); + } + + /// + /// Tests that during forward playback the decoupled clock always moves in the forwards direction after starting the source clock. + /// For this test, the source clock is started when the decoupled time crosses the 0ms-boundary. + /// + [Test] + public void TestForwardPlaybackDecoupledTimeDoesNotRewindAfterSourceStarts() + { + decoupleable.AllowDecoupling = true; + // decoupleable.CustomAllowableErrorMilliseconds = 1000; + decoupleable.Seek(-50); + decoupleable.ProcessFrame(); + decoupleable.Start(); + + // Delay a bit to make sure the clock crosses the 0ms boundary + Thread.Sleep(100); + decoupleable.ProcessFrame(); + + // Make sure that time doesn't rewind. Note that the source clock does not move by itself, + double last = decoupleable.CurrentTime; + decoupleable.ProcessFrame(); + Assert.That(decoupleable.CurrentTime, Is.GreaterThanOrEqualTo(last)); + } + + /// + /// Tests that during backwards playback the decoupled clock always moves in the backwards direction after starting the source clock. + /// For this test, the source clock is started when the decoupled time crosses the 1000ms-boundary. + /// + [Test] + public void TestBackwardPlaybackDecoupledTimeDoesNotRewindAfterSourceStarts() + { + source.MaxTime = 1000; + source.Rate = -1; + decoupleable.AllowDecoupling = true; + // decoupleable.CustomAllowableErrorMilliseconds = 1000; + + // Bring the source clock into a good state by seeking to a valid time + decoupleable.Seek(1000); + decoupleable.Start(); + decoupleable.ProcessFrame(); + decoupleable.Stop(); + + decoupleable.Seek(1050); + decoupleable.ProcessFrame(); + decoupleable.Start(); + + // Delay a bit to make sure the clock crosses the 1000ms boundary + Thread.Sleep(100); + decoupleable.ProcessFrame(); + + // Make sure that time doesn't rewind + double last = decoupleable.CurrentTime; + decoupleable.ProcessFrame(); + Assert.That(decoupleable.CurrentTime, Is.LessThanOrEqualTo(last)); + } + + /// + /// Tests that the decoupled clock seeks the source clock to its time when it starts. + /// + [Test] + public void TestDecoupledStartWithSourceOffset() + { + decoupleable.AllowDecoupling = true; + + source.Seek(1000); + decoupleable.Start(); + + Assert.AreEqual(0, source.CurrentTime); + Assert.AreEqual(source.CurrentTime, decoupleable.CurrentTime, "Deoupled time should match source time."); + } + + #endregion + + #region Seeking + + /// + /// Tests that the source clock is seeked when the coupled clock is seeked. + /// + [Test] + public void TestSourceSeekedByCoupledSeek() + { + decoupleable.Seek(1000); + + Assert.AreEqual(source.CurrentTime, decoupleable.CurrentTime, "Source time should match coupled time."); + } + + /// + /// Tests that the coupled clock is seeked when the source clock is seeked. + /// + [Test] + public void TestCoupledSeekedBySourceSeek() + { + decoupleable.Start(); + + source.Seek(1000); + decoupleable.ProcessFrame(); + + Assert.AreEqual(source.CurrentTime, decoupleable.CurrentTime, "Coupled time should match source time."); + } + + /// + /// Tests that the source clock is seeked when the decoupled clock is seeked. + /// + [Test] + public void TestSourceSeekedByDecoupledSeek() + { + decoupleable.AllowDecoupling = true; + decoupleable.Seek(1000); + + Assert.AreEqual(decoupleable.CurrentTime, 1000, "Decoupled time should match seek target."); + // Seek on the source is not performed as the clock is stopped. + Assert.AreNotEqual(source.CurrentTime, decoupleable.CurrentTime, "Source time should not match coupled time."); + } + + /// + /// Tests that the coupled clock is not seeked while stopped and the source clock is seeked. + /// + [Test] + public void TestDecoupledNotSeekedBySourceSeekWhenStopped() + { + decoupleable.AllowDecoupling = true; + + source.Seek(1000); + decoupleable.ProcessFrame(); + + Assert.AreEqual(0, decoupleable.CurrentTime); + Assert.AreNotEqual(source.CurrentTime, decoupleable.CurrentTime, "Coupled time should not match source time."); + } + + /// + /// Tests that seeking a decoupled clock negatively does not cause it to seek to the positive source time. + /// + [Test] + public void TestDecoupledNotSeekedPositivelyByFailedNegativeSeek() + { + decoupleable.AllowDecoupling = true; + decoupleable.Start(); + + decoupleable.Seek(-5000); + + Assert.That(source.IsRunning, Is.False); + Assert.That(decoupleable.IsRunning, Is.True); + Assert.That(decoupleable.CurrentTime, Is.LessThan(0)); + } + + #endregion + + /// + /// Tests that the state of the decouplable clock is preserved when it is stopped after processing a frame. + /// + [Test] + public void TestStoppingAfterProcessingFramePreservesState() + { + decoupleable.Start(); + source.CurrentTime = 1000; + + decoupleable.ProcessFrame(); + decoupleable.Stop(); + + Assert.AreEqual(source.CurrentTime, decoupleable.CurrentTime, "Decoupled should match source time."); + } + + /// + /// Tests that the state of the decouplable clock is preserved when it is stopped after having being started by the source clock. + /// + [Test] + public void TestStoppingAfterStartingBySourcePreservesState() + { + source.Start(); + source.CurrentTime = 1000; + + decoupleable.ProcessFrame(); + decoupleable.Stop(); + + Assert.AreEqual(source.CurrentTime, decoupleable.CurrentTime, "Decoupled should match source time."); + } + + private class TestClockWithRange : TestClock + { + public double MinTime => 0; + public double MaxTime { get; set; } = double.PositiveInfinity; + + public override bool Seek(double position) + { + if (Math.Clamp(position, MinTime, MaxTime) != position) + return false; + + return base.Seek(position); + } + } + } +} From 2a592dc761f0e315da60469dd3a8e3a20cdde031 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 20 Sep 2023 17:20:33 +0900 Subject: [PATCH 07/43] And now that the old tests are updated, let's rewrite them all --- .../Clocks/DecouplingClockTest.cs | 457 ------------------ .../Clocks/DecouplingFramedClockTest.cs | 162 +++++++ 2 files changed, 162 insertions(+), 457 deletions(-) delete mode 100644 osu.Framework.Tests/Clocks/DecouplingClockTest.cs create mode 100644 osu.Framework.Tests/Clocks/DecouplingFramedClockTest.cs diff --git a/osu.Framework.Tests/Clocks/DecouplingClockTest.cs b/osu.Framework.Tests/Clocks/DecouplingClockTest.cs deleted file mode 100644 index 1eef6c27e6..0000000000 --- a/osu.Framework.Tests/Clocks/DecouplingClockTest.cs +++ /dev/null @@ -1,457 +0,0 @@ -// 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 NUnit.Framework; -using osu.Framework.Timing; - -namespace osu.Framework.Tests.Clocks -{ - [TestFixture] - public class DecouplingClockTest - { - private TestClockWithRange source = null!; - private DecouplingFramedClock decoupleable = null!; - - [SetUp] - public void SetUp() - { - source = new TestClockWithRange(); - - decoupleable = new DecouplingFramedClock(); - decoupleable.ChangeSource(source); - } - - #region Start/stop by decoupleable - - /// - /// Tests that the source clock starts when the coupled clock starts. - /// - [Test] - public void TestSourceStartedByCoupled() - { - decoupleable.Start(); - - Assert.IsTrue(source.IsRunning, "Source should be running."); - } - - /// - /// Tests that the source clock stops when the coupled clock stops. - /// - [Test] - public void TestSourceStoppedByCoupled() - { - decoupleable.Start(); - decoupleable.Stop(); - - Assert.IsFalse(source.IsRunning, "Source should not be running."); - } - - /// - /// Tests that the source clock starts when the decoupled clock starts. - /// - [Test] - public void TestSourceStartedByDecoupled() - { - decoupleable.AllowDecoupling = true; - decoupleable.Start(); - - Assert.IsTrue(source.IsRunning, "Source should be running."); - } - - /// - /// Tests that the source clock stops when the decoupled clock stops. - /// - [Test] - public void TestSourceStoppedByDecoupled() - { - decoupleable.Start(); - - decoupleable.AllowDecoupling = true; - decoupleable.Stop(); - - Assert.IsFalse(source.IsRunning, "Source should not be running."); - } - - #endregion - - #region Start/stop by source - - /// - /// Tests that the coupled clock starts when the source clock starts. - /// - [Test] - public void TestCoupledStartedBySourceClock() - { - source.Start(); - decoupleable.ProcessFrame(); - - Assert.IsTrue(decoupleable.IsRunning, "Coupled should be running."); - } - - /// - /// Tests that the coupled clock stops when the source clock stops. - /// - [Test] - public void TestCoupledStoppedBySourceClock() - { - decoupleable.Start(); - - source.Stop(); - decoupleable.ProcessFrame(); - - Assert.IsFalse(decoupleable.IsRunning, "Coupled should not be running."); - Assert.That(decoupleable.CurrentTime, Is.EqualTo(source.CurrentTime)); - } - - /// - /// Tests that the decoupled clock doesn't start when the source clock starts. - /// - [Test] - public void TestDecoupledNotStartedBySourceClock() - { - decoupleable.AllowDecoupling = true; - - source.Start(); - decoupleable.ProcessFrame(); - - Assert.IsFalse(decoupleable.IsRunning, "Decoupled should not be running."); - } - - /// - /// Tests that the decoupled clock doesn't stop when the source clock stops. - /// - [Test] - public void TestDecoupledNotStoppedBySourceClock() - { - decoupleable.Start(); - decoupleable.AllowDecoupling = true; - - source.Stop(); - decoupleable.ProcessFrame(); - - Assert.IsTrue(decoupleable.IsRunning, "Decoupled should be running."); - } - - #endregion - - #region Source changes - - [Test] - public void SourceChangeTransfersValueAdjustable() - { - // For decoupled clocks, value transfer is preferred in the direction of the track if possible. - // In other words, we want to keep our current time even if the source changes, as long as the source supports it. - // - // This tests the case where it is supported. - - const double first_source_time = 256000; - const double second_source_time = 128000; - - source.Seek(first_source_time); - source.Start(); - - var secondSource = new TestClock - { - // importantly, test a value lower than the original source. - // this is to both test value transfer *and* the case where time is going backwards, as - // some clocks have special provisions for this. - CurrentTime = second_source_time - }; - - decoupleable.ProcessFrame(); - Assert.That(decoupleable.CurrentTime, Is.EqualTo(first_source_time)); - - decoupleable.ChangeSource(secondSource); - decoupleable.ProcessFrame(); - - Assert.That(secondSource.CurrentTime, Is.EqualTo(first_source_time)); - Assert.That(decoupleable.CurrentTime, Is.EqualTo(first_source_time)); - } - - [Test] - public void SourceChangeTransfersValueNonAdjustable() - { - // For decoupled clocks, value transfer is preferred in the direction of the track if possible. - // In other words, we want to keep our current time even if the source changes, as long as the source supports it. - // - // This tests the case where it is NOT supported. - - const double first_source_time = 256000; - const double second_source_time = 128000; - - source.Seek(first_source_time); - source.Start(); - - var secondSource = new TestNonAdjustableClock - { - // importantly, test a value lower than the original source. - // this is to both test value transfer *and* the case where time is going backwards, as - // some clocks have special provisions for this. - CurrentTime = second_source_time - }; - - decoupleable.ProcessFrame(); - Assert.That(decoupleable.CurrentTime, Is.EqualTo(first_source_time)); - - decoupleable.ChangeSource(secondSource); - decoupleable.ProcessFrame(); - - Assert.That(secondSource.CurrentTime, Is.EqualTo(second_source_time)); - Assert.That(decoupleable.CurrentTime, Is.EqualTo(second_source_time)); - } - - #endregion - - #region Offset start - - /// - /// Tests that the coupled clock seeks to the correct position when the source clock starts. - /// - [Test] - public void TestCoupledStartBySourceWithSourceOffset() - { - source.Seek(1000); - - source.Start(); - decoupleable.ProcessFrame(); - - Assert.AreEqual(source.CurrentTime, decoupleable.CurrentTime, "Coupled time should match source time."); - } - - /// - /// Tests that the coupled clock seeks the source clock to its time when it starts. - /// - [Test] - public void TestCoupledStartWithSouceOffset() - { - source.Seek(1000); - decoupleable.Start(); - - Assert.AreEqual(0, source.CurrentTime); - Assert.AreEqual(source.CurrentTime, decoupleable.CurrentTime, "Coupled time should match source time."); - } - - [Test] - public void TestFromNegativeCoupledMode() - { - decoupleable.AllowDecoupling = false; - decoupleable.Seek(-1000); - - decoupleable.ProcessFrame(); - - Assert.AreEqual(0, source.CurrentTime); - Assert.AreEqual(0, decoupleable.CurrentTime); - } - - /// - /// Tests that the decoupled clocks starts the source as a result of being able to handle the current time. - /// - [Test] - public void TestDecoupledStartsSourceIfAllowable() - { - decoupleable.AllowDecoupling = true; - // decoupleable.CustomAllowableErrorMilliseconds = 1000; - decoupleable.Seek(-50); - decoupleable.ProcessFrame(); - decoupleable.Start(); - - // Delay a bit to make sure the clock crosses the 0 boundary - Thread.Sleep(100); - decoupleable.ProcessFrame(); - - Assert.That(source.IsRunning, Is.True); - } - - /// - /// Tests that during forward playback the decoupled clock always moves in the forwards direction after starting the source clock. - /// For this test, the source clock is started when the decoupled time crosses the 0ms-boundary. - /// - [Test] - public void TestForwardPlaybackDecoupledTimeDoesNotRewindAfterSourceStarts() - { - decoupleable.AllowDecoupling = true; - // decoupleable.CustomAllowableErrorMilliseconds = 1000; - decoupleable.Seek(-50); - decoupleable.ProcessFrame(); - decoupleable.Start(); - - // Delay a bit to make sure the clock crosses the 0ms boundary - Thread.Sleep(100); - decoupleable.ProcessFrame(); - - // Make sure that time doesn't rewind. Note that the source clock does not move by itself, - double last = decoupleable.CurrentTime; - decoupleable.ProcessFrame(); - Assert.That(decoupleable.CurrentTime, Is.GreaterThanOrEqualTo(last)); - } - - /// - /// Tests that during backwards playback the decoupled clock always moves in the backwards direction after starting the source clock. - /// For this test, the source clock is started when the decoupled time crosses the 1000ms-boundary. - /// - [Test] - public void TestBackwardPlaybackDecoupledTimeDoesNotRewindAfterSourceStarts() - { - source.MaxTime = 1000; - source.Rate = -1; - decoupleable.AllowDecoupling = true; - // decoupleable.CustomAllowableErrorMilliseconds = 1000; - - // Bring the source clock into a good state by seeking to a valid time - decoupleable.Seek(1000); - decoupleable.Start(); - decoupleable.ProcessFrame(); - decoupleable.Stop(); - - decoupleable.Seek(1050); - decoupleable.ProcessFrame(); - decoupleable.Start(); - - // Delay a bit to make sure the clock crosses the 1000ms boundary - Thread.Sleep(100); - decoupleable.ProcessFrame(); - - // Make sure that time doesn't rewind - double last = decoupleable.CurrentTime; - decoupleable.ProcessFrame(); - Assert.That(decoupleable.CurrentTime, Is.LessThanOrEqualTo(last)); - } - - /// - /// Tests that the decoupled clock seeks the source clock to its time when it starts. - /// - [Test] - public void TestDecoupledStartWithSourceOffset() - { - decoupleable.AllowDecoupling = true; - - source.Seek(1000); - decoupleable.Start(); - - Assert.AreEqual(0, source.CurrentTime); - Assert.AreEqual(source.CurrentTime, decoupleable.CurrentTime, "Deoupled time should match source time."); - } - - #endregion - - #region Seeking - - /// - /// Tests that the source clock is seeked when the coupled clock is seeked. - /// - [Test] - public void TestSourceSeekedByCoupledSeek() - { - decoupleable.Seek(1000); - - Assert.AreEqual(source.CurrentTime, decoupleable.CurrentTime, "Source time should match coupled time."); - } - - /// - /// Tests that the coupled clock is seeked when the source clock is seeked. - /// - [Test] - public void TestCoupledSeekedBySourceSeek() - { - decoupleable.Start(); - - source.Seek(1000); - decoupleable.ProcessFrame(); - - Assert.AreEqual(source.CurrentTime, decoupleable.CurrentTime, "Coupled time should match source time."); - } - - /// - /// Tests that the source clock is seeked when the decoupled clock is seeked. - /// - [Test] - public void TestSourceSeekedByDecoupledSeek() - { - decoupleable.AllowDecoupling = true; - decoupleable.Seek(1000); - - Assert.AreEqual(decoupleable.CurrentTime, 1000, "Decoupled time should match seek target."); - // Seek on the source is not performed as the clock is stopped. - Assert.AreNotEqual(source.CurrentTime, decoupleable.CurrentTime, "Source time should not match coupled time."); - } - - /// - /// Tests that the coupled clock is not seeked while stopped and the source clock is seeked. - /// - [Test] - public void TestDecoupledNotSeekedBySourceSeekWhenStopped() - { - decoupleable.AllowDecoupling = true; - - source.Seek(1000); - decoupleable.ProcessFrame(); - - Assert.AreEqual(0, decoupleable.CurrentTime); - Assert.AreNotEqual(source.CurrentTime, decoupleable.CurrentTime, "Coupled time should not match source time."); - } - - /// - /// Tests that seeking a decoupled clock negatively does not cause it to seek to the positive source time. - /// - [Test] - public void TestDecoupledNotSeekedPositivelyByFailedNegativeSeek() - { - decoupleable.AllowDecoupling = true; - decoupleable.Start(); - - decoupleable.Seek(-5000); - - Assert.That(source.IsRunning, Is.False); - Assert.That(decoupleable.IsRunning, Is.True); - Assert.That(decoupleable.CurrentTime, Is.LessThan(0)); - } - - #endregion - - /// - /// Tests that the state of the decouplable clock is preserved when it is stopped after processing a frame. - /// - [Test] - public void TestStoppingAfterProcessingFramePreservesState() - { - decoupleable.Start(); - source.CurrentTime = 1000; - - decoupleable.ProcessFrame(); - decoupleable.Stop(); - - Assert.AreEqual(source.CurrentTime, decoupleable.CurrentTime, "Decoupled should match source time."); - } - - /// - /// Tests that the state of the decouplable clock is preserved when it is stopped after having being started by the source clock. - /// - [Test] - public void TestStoppingAfterStartingBySourcePreservesState() - { - source.Start(); - source.CurrentTime = 1000; - - decoupleable.ProcessFrame(); - decoupleable.Stop(); - - Assert.AreEqual(source.CurrentTime, decoupleable.CurrentTime, "Decoupled should match source time."); - } - - private class TestClockWithRange : TestClock - { - public double MinTime => 0; - public double MaxTime { get; set; } = double.PositiveInfinity; - - public override bool Seek(double position) - { - if (Math.Clamp(position, MinTime, MaxTime) != position) - return false; - - return base.Seek(position); - } - } - } -} diff --git a/osu.Framework.Tests/Clocks/DecouplingFramedClockTest.cs b/osu.Framework.Tests/Clocks/DecouplingFramedClockTest.cs new file mode 100644 index 0000000000..2f88a33233 --- /dev/null +++ b/osu.Framework.Tests/Clocks/DecouplingFramedClockTest.cs @@ -0,0 +1,162 @@ +// 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 NUnit.Framework; +using osu.Framework.Timing; + +namespace osu.Framework.Tests.Clocks +{ + [TestFixture] + public class DecouplingFramedClockTest + { + private TestClockWithRange source = null!; + private DecouplingFramedClock decouplingClock = null!; + + [SetUp] + public void SetUp() + { + source = new TestClockWithRange(); + + decouplingClock = new DecouplingFramedClock(); + decouplingClock.ChangeSource(source); + } + + #region Basic assumptions + + [Test] + public void TestStartFromDecoupling() + { + Assert.That(source.IsRunning, Is.False); + Assert.That(decouplingClock.IsRunning, Is.False); + + decouplingClock.Start(); + + Assert.That(source.IsRunning, Is.True); + Assert.That(decouplingClock.IsRunning, Is.True); + } + + [Test] + public void TestStartFromSource() + { + Assert.That(source.IsRunning, Is.False); + Assert.That(decouplingClock.IsRunning, Is.False); + + source.Start(); + + Assert.That(source.IsRunning, Is.True); + Assert.That(decouplingClock.IsRunning, Is.True); + } + + [Test] + public void TestSeekFromDecoupling() + { + Assert.That(source.CurrentTime, Is.EqualTo(0)); + Assert.That(decouplingClock.CurrentTime, Is.EqualTo(0)); + + decouplingClock.Seek(1000); + + Assert.That(source.CurrentTime, Is.EqualTo(1000)); + Assert.That(decouplingClock.CurrentTime, Is.EqualTo(0)); + + decouplingClock.ProcessFrame(); + + Assert.That(source.CurrentTime, Is.EqualTo(1000)); + Assert.That(decouplingClock.CurrentTime, Is.EqualTo(1000)); + } + + [Test] + public void TestSeekFromSource() + { + Assert.That(source.CurrentTime, Is.EqualTo(0)); + Assert.That(decouplingClock.CurrentTime, Is.EqualTo(0)); + + source.Seek(1000); + + Assert.That(source.CurrentTime, Is.EqualTo(1000)); + Assert.That(decouplingClock.CurrentTime, Is.EqualTo(0)); + + decouplingClock.ProcessFrame(); + + Assert.That(source.CurrentTime, Is.EqualTo(1000)); + Assert.That(decouplingClock.CurrentTime, Is.EqualTo(1000)); + } + + [Test] + public void ChangeSourceUpdatesToNewSourceTime() + { + const double first_source_time = 256000; + const double second_source_time = 128000; + + source.Seek(first_source_time); + source.Start(); + + var secondSource = new TestClock { CurrentTime = second_source_time }; + + decouplingClock.ProcessFrame(); + Assert.That(decouplingClock.CurrentTime, Is.EqualTo(first_source_time)); + + decouplingClock.ChangeSource(secondSource); + + Assert.That(decouplingClock.CurrentTime, Is.EqualTo(first_source_time)); + decouplingClock.ProcessFrame(); + Assert.That(secondSource.CurrentTime, Is.EqualTo(second_source_time)); + } + + #endregion + + #region Operation in non-decoupling mode + + [Test] + public void TestSourceStoppedWhileNotDecoupling() + { + decouplingClock.AllowDecoupling = false; + decouplingClock.Start(); + + Assert.That(source.IsRunning, Is.True); + Assert.That(decouplingClock.IsRunning, Is.True); + + source.Stop(); + decouplingClock.ProcessFrame(); + + Assert.That(source.IsRunning, Is.False); + Assert.That(decouplingClock.IsRunning, Is.False); + } + + [Test] + public void TestSeekNegativeWhileNotDecoupling() + { + decouplingClock.AllowDecoupling = false; + decouplingClock.Seek(-1000); + + decouplingClock.ProcessFrame(); + + Assert.AreEqual(0, source.CurrentTime); + Assert.AreEqual(0, decouplingClock.CurrentTime); + } + + #endregion + + #region Operation in decoupling mode + + // TODO: test playback is always forward over the 0ms boundary. + + // TODO: test backwards playback. (over the boundary?) + + #endregion + + private class TestClockWithRange : TestClock + { + public double MinTime => 0; + public double MaxTime { get; set; } = double.PositiveInfinity; + + public override bool Seek(double position) + { + if (Math.Clamp(position, MinTime, MaxTime) != position) + return false; + + return base.Seek(position); + } + } + } +} From 888e5067ef0e670b89e814635baa8ba755baa3ec Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 20 Sep 2023 18:39:14 +0900 Subject: [PATCH 08/43] Add basic setup for decoupling --- .../Clocks/DecouplingFramedClockTest.cs | 46 +++++++++++++++++-- osu.Framework/Timing/DecouplingFramedClock.cs | 39 +++++++++++++--- 2 files changed, 75 insertions(+), 10 deletions(-) diff --git a/osu.Framework.Tests/Clocks/DecouplingFramedClockTest.cs b/osu.Framework.Tests/Clocks/DecouplingFramedClockTest.cs index 2f88a33233..5188576839 100644 --- a/osu.Framework.Tests/Clocks/DecouplingFramedClockTest.cs +++ b/osu.Framework.Tests/Clocks/DecouplingFramedClockTest.cs @@ -27,6 +27,8 @@ public void SetUp() [Test] public void TestStartFromDecoupling() { + decouplingClock.AllowDecoupling = false; + Assert.That(source.IsRunning, Is.False); Assert.That(decouplingClock.IsRunning, Is.False); @@ -39,6 +41,8 @@ public void TestStartFromDecoupling() [Test] public void TestStartFromSource() { + decouplingClock.AllowDecoupling = false; + Assert.That(source.IsRunning, Is.False); Assert.That(decouplingClock.IsRunning, Is.False); @@ -51,6 +55,8 @@ public void TestStartFromSource() [Test] public void TestSeekFromDecoupling() { + decouplingClock.AllowDecoupling = false; + Assert.That(source.CurrentTime, Is.EqualTo(0)); Assert.That(decouplingClock.CurrentTime, Is.EqualTo(0)); @@ -68,6 +74,8 @@ public void TestSeekFromDecoupling() [Test] public void TestSeekFromSource() { + decouplingClock.AllowDecoupling = false; + Assert.That(source.CurrentTime, Is.EqualTo(0)); Assert.That(decouplingClock.CurrentTime, Is.EqualTo(0)); @@ -85,6 +93,8 @@ public void TestSeekFromSource() [Test] public void ChangeSourceUpdatesToNewSourceTime() { + decouplingClock.AllowDecoupling = false; + const double first_source_time = 256000; const double second_source_time = 128000; @@ -127,18 +137,48 @@ public void TestSourceStoppedWhileNotDecoupling() public void TestSeekNegativeWhileNotDecoupling() { decouplingClock.AllowDecoupling = false; - decouplingClock.Seek(-1000); + Assert.That(decouplingClock.Seek(-1000), Is.False); decouplingClock.ProcessFrame(); - Assert.AreEqual(0, source.CurrentTime); - Assert.AreEqual(0, decouplingClock.CurrentTime); + Assert.That(source.CurrentTime, Is.EqualTo(0)); + Assert.That(decouplingClock.CurrentTime, Is.EqualTo(0)); } #endregion #region Operation in decoupling mode + [Test] + public void TestSourceStoppedWhileDecoupling() + { + decouplingClock.AllowDecoupling = true; + decouplingClock.Start(); + + Assert.That(source.IsRunning, Is.True); + Assert.That(decouplingClock.IsRunning, Is.True); + + source.Stop(); + decouplingClock.ProcessFrame(); + + Assert.That(source.IsRunning, Is.False); + // We're decoupling, so should still be running. + Assert.That(decouplingClock.IsRunning, Is.True); + } + + [Test] + public void TestSeekNegativeWhileDecoupling() + { + decouplingClock.AllowDecoupling = true; + Assert.That(decouplingClock.Seek(-1000), Is.True); + + decouplingClock.ProcessFrame(); + + Assert.That(source.CurrentTime, Is.EqualTo(0)); + // We're decoupling, so should be able to go beyond zero. + Assert.That(decouplingClock.CurrentTime, Is.EqualTo(-1000)); + } + // TODO: test playback is always forward over the 0ms boundary. // TODO: test backwards playback. (over the boundary?) diff --git a/osu.Framework/Timing/DecouplingFramedClock.cs b/osu.Framework/Timing/DecouplingFramedClock.cs index c089d8b6d3..c9779ec076 100644 --- a/osu.Framework/Timing/DecouplingFramedClock.cs +++ b/osu.Framework/Timing/DecouplingFramedClock.cs @@ -20,7 +20,7 @@ public class DecouplingFramedClock : IFrameBasedClock, ISourceChangeableClock, I private IFrameBasedClock framedSourceClock; private IAdjustableClock adjustableSourceClock; - private bool isRunningDecoupled; + private bool isRunning; public DecouplingFramedClock(IClock? source = null) { @@ -50,12 +50,16 @@ public virtual void ProcessFrame() framedSourceClock.ProcessFrame(); - if (AllowDecoupling) + if (Source.IsRunning || !AllowDecoupling) { + CurrentTime = framedSourceClock.CurrentTime; + ElapsedFrameTime = framedSourceClock.ElapsedFrameTime; + } + else + { + if (isRunning) + CurrentTime += realtimeReferenceClock.ElapsedFrameTime * Rate; } - - CurrentTime = framedSourceClock.CurrentTime; - ElapsedFrameTime = framedSourceClock.ElapsedFrameTime; } private void updateRealtimeReference() @@ -83,7 +87,16 @@ public void Stop() public bool Seek(double position) { - return adjustableSourceClock.Seek(position); + if (adjustableSourceClock.Seek(position)) + { + return true; + } + + if (!AllowDecoupling) + return false; + + CurrentTime = position; + return true; } public void ResetSpeedAdjustments() => adjustableSourceClock.ResetSpeedAdjustments(); @@ -99,7 +112,19 @@ public virtual double Rate # region IFrameBasedClock delegation public double ElapsedFrameTime { get; private set; } - public bool IsRunning => framedSourceClock.IsRunning ? true : isRunningDecoupled; + + public bool IsRunning + { + get + { + // Always immediately use the source clock's running state if it's running. + if (framedSourceClock.IsRunning || !AllowDecoupling) + return isRunning = framedSourceClock.IsRunning; + + return isRunning; + } + } + public virtual double CurrentTime { get; private set; } double IFrameBasedClock.FramesPerSecond => 0; From be3b8598d782df4adbb65403236b9dd9906958f0 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 20 Sep 2023 18:40:29 +0900 Subject: [PATCH 09/43] Don't require a framed source --- osu.Framework/Timing/DecouplingFramedClock.cs | 20 ++++++------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/osu.Framework/Timing/DecouplingFramedClock.cs b/osu.Framework/Timing/DecouplingFramedClock.cs index c9779ec076..b4f732af71 100644 --- a/osu.Framework/Timing/DecouplingFramedClock.cs +++ b/osu.Framework/Timing/DecouplingFramedClock.cs @@ -17,7 +17,6 @@ public class DecouplingFramedClock : IFrameBasedClock, ISourceChangeableClock, I /// private readonly FramedClock realtimeReferenceClock = new FramedClock(new StopwatchClock(true)); - private IFrameBasedClock framedSourceClock; private IAdjustableClock adjustableSourceClock; private bool isRunning; @@ -26,7 +25,6 @@ public DecouplingFramedClock(IClock? source = null) { ChangeSource(source); Debug.Assert(Source != null); - Debug.Assert(framedSourceClock != null); Debug.Assert(adjustableSourceClock != null); } @@ -38,22 +36,18 @@ public void ChangeSource(IClock? source) throw new ArgumentException($"Clock must be of type {nameof(IAdjustableClock)}"); adjustableSourceClock = adjustableSource; - - // We need a frame-based source to correctly process interpolation. - // If the provided source is not already a framed clock, encapsulate it in one. - framedSourceClock = Source as IFrameBasedClock ?? new FramedClock(source); } public virtual void ProcessFrame() { updateRealtimeReference(); - framedSourceClock.ProcessFrame(); + double lastTime = CurrentTime; if (Source.IsRunning || !AllowDecoupling) { - CurrentTime = framedSourceClock.CurrentTime; - ElapsedFrameTime = framedSourceClock.ElapsedFrameTime; + ElapsedFrameTime = Source.CurrentTime - lastTime; + CurrentTime = Source.CurrentTime; } else { @@ -64,7 +58,7 @@ public virtual void ProcessFrame() private void updateRealtimeReference() { - ((StopwatchClock)realtimeReferenceClock.Source).Rate = framedSourceClock.Rate; + ((StopwatchClock)realtimeReferenceClock.Source).Rate = Source.Rate; realtimeReferenceClock.ProcessFrame(); } @@ -88,9 +82,7 @@ public void Stop() public bool Seek(double position) { if (adjustableSourceClock.Seek(position)) - { return true; - } if (!AllowDecoupling) return false; @@ -118,8 +110,8 @@ public bool IsRunning get { // Always immediately use the source clock's running state if it's running. - if (framedSourceClock.IsRunning || !AllowDecoupling) - return isRunning = framedSourceClock.IsRunning; + if (Source.IsRunning || !AllowDecoupling) + return isRunning = Source.IsRunning; return isRunning; } From 984923f72b944635e34862112fd6c56f3cdee7c8 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 20 Sep 2023 18:51:04 +0900 Subject: [PATCH 10/43] Make `DecouplingFramedClock`... not a framed clock --- ...medClockTest.cs => DecouplingClockTest.cs} | 28 +----- ...plingFramedClock.cs => DecouplingClock.cs} | 95 +++++++++---------- 2 files changed, 49 insertions(+), 74 deletions(-) rename osu.Framework.Tests/Clocks/{DecouplingFramedClockTest.cs => DecouplingClockTest.cs} (85%) rename osu.Framework/Timing/{DecouplingFramedClock.cs => DecouplingClock.cs} (61%) diff --git a/osu.Framework.Tests/Clocks/DecouplingFramedClockTest.cs b/osu.Framework.Tests/Clocks/DecouplingClockTest.cs similarity index 85% rename from osu.Framework.Tests/Clocks/DecouplingFramedClockTest.cs rename to osu.Framework.Tests/Clocks/DecouplingClockTest.cs index 5188576839..39b55ee8e0 100644 --- a/osu.Framework.Tests/Clocks/DecouplingFramedClockTest.cs +++ b/osu.Framework.Tests/Clocks/DecouplingClockTest.cs @@ -8,17 +8,17 @@ namespace osu.Framework.Tests.Clocks { [TestFixture] - public class DecouplingFramedClockTest + public class DecouplingClockTest { private TestClockWithRange source = null!; - private DecouplingFramedClock decouplingClock = null!; + private DecouplingClock decouplingClock = null!; [SetUp] public void SetUp() { source = new TestClockWithRange(); - decouplingClock = new DecouplingFramedClock(); + decouplingClock = new DecouplingClock(); decouplingClock.ChangeSource(source); } @@ -62,11 +62,6 @@ public void TestSeekFromDecoupling() decouplingClock.Seek(1000); - Assert.That(source.CurrentTime, Is.EqualTo(1000)); - Assert.That(decouplingClock.CurrentTime, Is.EqualTo(0)); - - decouplingClock.ProcessFrame(); - Assert.That(source.CurrentTime, Is.EqualTo(1000)); Assert.That(decouplingClock.CurrentTime, Is.EqualTo(1000)); } @@ -81,11 +76,6 @@ public void TestSeekFromSource() source.Seek(1000); - Assert.That(source.CurrentTime, Is.EqualTo(1000)); - Assert.That(decouplingClock.CurrentTime, Is.EqualTo(0)); - - decouplingClock.ProcessFrame(); - Assert.That(source.CurrentTime, Is.EqualTo(1000)); Assert.That(decouplingClock.CurrentTime, Is.EqualTo(1000)); } @@ -103,13 +93,8 @@ public void ChangeSourceUpdatesToNewSourceTime() var secondSource = new TestClock { CurrentTime = second_source_time }; - decouplingClock.ProcessFrame(); Assert.That(decouplingClock.CurrentTime, Is.EqualTo(first_source_time)); - decouplingClock.ChangeSource(secondSource); - - Assert.That(decouplingClock.CurrentTime, Is.EqualTo(first_source_time)); - decouplingClock.ProcessFrame(); Assert.That(secondSource.CurrentTime, Is.EqualTo(second_source_time)); } @@ -127,7 +112,6 @@ public void TestSourceStoppedWhileNotDecoupling() Assert.That(decouplingClock.IsRunning, Is.True); source.Stop(); - decouplingClock.ProcessFrame(); Assert.That(source.IsRunning, Is.False); Assert.That(decouplingClock.IsRunning, Is.False); @@ -137,9 +121,8 @@ public void TestSourceStoppedWhileNotDecoupling() public void TestSeekNegativeWhileNotDecoupling() { decouplingClock.AllowDecoupling = false; - Assert.That(decouplingClock.Seek(-1000), Is.False); - decouplingClock.ProcessFrame(); + Assert.That(decouplingClock.Seek(-1000), Is.False); Assert.That(source.CurrentTime, Is.EqualTo(0)); Assert.That(decouplingClock.CurrentTime, Is.EqualTo(0)); @@ -159,7 +142,6 @@ public void TestSourceStoppedWhileDecoupling() Assert.That(decouplingClock.IsRunning, Is.True); source.Stop(); - decouplingClock.ProcessFrame(); Assert.That(source.IsRunning, Is.False); // We're decoupling, so should still be running. @@ -172,8 +154,6 @@ public void TestSeekNegativeWhileDecoupling() decouplingClock.AllowDecoupling = true; Assert.That(decouplingClock.Seek(-1000), Is.True); - decouplingClock.ProcessFrame(); - Assert.That(source.CurrentTime, Is.EqualTo(0)); // We're decoupling, so should be able to go beyond zero. Assert.That(decouplingClock.CurrentTime, Is.EqualTo(-1000)); diff --git a/osu.Framework/Timing/DecouplingFramedClock.cs b/osu.Framework/Timing/DecouplingClock.cs similarity index 61% rename from osu.Framework/Timing/DecouplingFramedClock.cs rename to osu.Framework/Timing/DecouplingClock.cs index b4f732af71..a532f4a49e 100644 --- a/osu.Framework/Timing/DecouplingFramedClock.cs +++ b/osu.Framework/Timing/DecouplingClock.cs @@ -6,22 +6,59 @@ namespace osu.Framework.Timing { - public class DecouplingFramedClock : IFrameBasedClock, ISourceChangeableClock, IAdjustableClock + public class DecouplingClock : ISourceChangeableClock, IAdjustableClock { public bool AllowDecoupling = true; public IClock Source { get; private set; } /// - /// This clock is used when we are decoupling from the source, to figure out how much elapsed time has passed between frames. + /// This clock is used when we are decoupling from the source. /// - private readonly FramedClock realtimeReferenceClock = new FramedClock(new StopwatchClock(true)); + private readonly StopwatchClock realtimeReferenceClock = new StopwatchClock(true); private IAdjustableClock adjustableSourceClock; private bool isRunning; + private double? lastDecoupledTimeConsumed; - public DecouplingFramedClock(IClock? source = null) + public bool IsRunning + { + get + { + // Always use the source clock's running state if it's running. + if (Source.IsRunning || !AllowDecoupling) + return isRunning = Source.IsRunning; + + return isRunning; + } + } + + private double currentTime; + + public virtual double CurrentTime + { + get + { + if (Source.IsRunning || !AllowDecoupling) + return currentTime = Source.CurrentTime; + + if (!isRunning) + return currentTime; + + if (lastDecoupledTimeConsumed != null) + currentTime += (realtimeReferenceClock.CurrentTime - lastDecoupledTimeConsumed.Value) * Rate; + + lastDecoupledTimeConsumed = realtimeReferenceClock.CurrentTime; + + if (lastDecoupledTimeConsumed < 0 && currentTime >= 0) + adjustableSourceClock.Start(); + + return currentTime; + } + } + + public DecouplingClock(IClock? source = null) { ChangeSource(source); Debug.Assert(Source != null); @@ -38,45 +75,24 @@ public void ChangeSource(IClock? source) adjustableSourceClock = adjustableSource; } - public virtual void ProcessFrame() - { - updateRealtimeReference(); - - double lastTime = CurrentTime; - - if (Source.IsRunning || !AllowDecoupling) - { - ElapsedFrameTime = Source.CurrentTime - lastTime; - CurrentTime = Source.CurrentTime; - } - else - { - if (isRunning) - CurrentTime += realtimeReferenceClock.ElapsedFrameTime * Rate; - } - } - - private void updateRealtimeReference() - { - ((StopwatchClock)realtimeReferenceClock.Source).Rate = Source.Rate; - realtimeReferenceClock.ProcessFrame(); - } - #region IAdjustableClock implementation public void Reset() { adjustableSourceClock.Reset(); + isRunning = false; } public void Start() { adjustableSourceClock.Start(); + isRunning = adjustableSourceClock.IsRunning || AllowDecoupling; } public void Stop() { adjustableSourceClock.Stop(); + isRunning = false; } public bool Seek(double position) @@ -87,7 +103,7 @@ public bool Seek(double position) if (!AllowDecoupling) return false; - CurrentTime = position; + currentTime = position; return true; } @@ -100,26 +116,5 @@ public virtual double Rate } #endregion - - # region IFrameBasedClock delegation - - public double ElapsedFrameTime { get; private set; } - - public bool IsRunning - { - get - { - // Always immediately use the source clock's running state if it's running. - if (Source.IsRunning || !AllowDecoupling) - return isRunning = Source.IsRunning; - - return isRunning; - } - } - - public virtual double CurrentTime { get; private set; } - double IFrameBasedClock.FramesPerSecond => 0; - - #endregion } } From 80518286774e2859c26ee7e524eaf75ac253da3b Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 20 Sep 2023 19:13:52 +0900 Subject: [PATCH 11/43] Add more testability to visual clocks test scene --- .../Visual/Clocks/TestSceneClock.cs | 31 ++++++------- .../Visual/Clocks/TestSceneClocks.cs | 45 ++++++++++++++++++- osu.Framework/Timing/StopwatchClock.cs | 2 +- 3 files changed, 61 insertions(+), 17 deletions(-) diff --git a/osu.Framework.Tests/Visual/Clocks/TestSceneClock.cs b/osu.Framework.Tests/Visual/Clocks/TestSceneClock.cs index 9d1ee7f9aa..b91445240a 100644 --- a/osu.Framework.Tests/Visual/Clocks/TestSceneClock.cs +++ b/osu.Framework.Tests/Visual/Clocks/TestSceneClock.cs @@ -48,7 +48,8 @@ protected IClock AddClock(IClock clock, string? name = null) public partial class VisualClock : CompositeDrawable { - private readonly IClock clock; + public IClock TrackingClock { get; } + private readonly SpriteText time; private readonly SpriteText rate; @@ -59,9 +60,9 @@ public partial class VisualClock : CompositeDrawable private readonly Box bg; private readonly Box hand; - public VisualClock(IClock clock, string? name = null) + public VisualClock(IClock trackingClock, string? name = null) { - this.clock = clock; + TrackingClock = trackingClock; Size = new Vector2(width); CornerRadius = width / 2; @@ -74,12 +75,12 @@ public VisualClock(IClock clock, string? name = null) { bg = new Box { - Colour = clock is IAdjustableClock ? Color4.Tomato : Color4.Navy, + Colour = trackingClock is IAdjustableClock ? Color4.Tomato : Color4.Navy, RelativeSizeAxes = Axes.Both, }, new SpriteText { - Text = clock.GetType().Name + (!string.IsNullOrEmpty(name) ? $" ({name})" : string.Empty), + Text = trackingClock.GetType().Name + (!string.IsNullOrEmpty(name) ? $" ({name})" : string.Empty), Anchor = Anchor.Centre, Origin = Anchor.Centre, Y = -25, @@ -109,7 +110,7 @@ public VisualClock(IClock clock, string? name = null) protected override bool OnClick(ClickEvent e) { - if (clock is IAdjustableClock adjustable) + if (TrackingClock is IAdjustableClock adjustable) { if (adjustable.IsRunning) adjustable.Stop(); @@ -122,7 +123,7 @@ protected override bool OnClick(ClickEvent e) protected override bool OnScroll(ScrollEvent e) { - if (clock is IAdjustableClock adjustable) + if (TrackingClock is IAdjustableClock adjustable) adjustable.Rate += e.ScrollDelta.Y / 1000; return base.OnScroll(e); @@ -132,20 +133,20 @@ protected override void Update() { base.Update(); - double lastTime = clock.CurrentTime; + double lastTime = TrackingClock.CurrentTime; - (clock as IFrameBasedClock)?.ProcessFrame(); + (TrackingClock as IFrameBasedClock)?.ProcessFrame(); - var timespan = TimeSpan.FromMilliseconds(clock.CurrentTime); + var timespan = TimeSpan.FromMilliseconds(TrackingClock.CurrentTime); time.Text = $"{timespan.Minutes:00}:{timespan.Seconds:00}:{timespan.Milliseconds:00}"; - rate.Text = $"{clock.Rate:N2}x"; + rate.Text = $"{TrackingClock.Rate:N2}x"; - if (clock.CurrentTime != lastTime) - BorderColour = clock.CurrentTime >= lastTime ? Color4.White : Color4.Red; + if (TrackingClock.CurrentTime != lastTime) + BorderColour = TrackingClock.CurrentTime >= lastTime ? Color4.White : Color4.Red; - Colour = clock.IsRunning ? Color4.White : Color4.Gray; + Colour = TrackingClock.IsRunning ? Color4.White : Color4.Gray; - hand.Rotation = (float)(clock.CurrentTime / 1000) * 360 % 360; + hand.Rotation = (float)(TrackingClock.CurrentTime / 1000) * 360 % 360; if (hand.Rotation < 180) { diff --git a/osu.Framework.Tests/Visual/Clocks/TestSceneClocks.cs b/osu.Framework.Tests/Visual/Clocks/TestSceneClocks.cs index e5272f09b5..a25b6a907f 100644 --- a/osu.Framework.Tests/Visual/Clocks/TestSceneClocks.cs +++ b/osu.Framework.Tests/Visual/Clocks/TestSceneClocks.cs @@ -1,6 +1,8 @@ // 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 osu.Framework.Testing; using osu.Framework.Timing; namespace osu.Framework.Tests.Visual.Clocks @@ -12,10 +14,51 @@ protected override void LoadComplete() base.LoadComplete(); AddStep("add stopwatch", () => AddClock(new StopwatchClock(true))); + AddStep("add non-negative stopwatch", () => AddClock(new TestClockWithRangeLimit())); AddStep("add framed", () => AddClock(new FramedClock())); AddStep("add lag", () => AddClock(new LaggyFramedClock())); AddStep("add interpolating", () => AddClock(new InterpolatingFramedClock())); - AddStep("add decoupled", () => AddClock(new DecoupleableInterpolatingFramedClock())); + AddStep("add decoupling(true)", () => AddClock(new DecouplingClock { AllowDecoupling = true })); + AddStep("add decoupling(false)", () => AddClock(new DecouplingClock { AllowDecoupling = false })); + + AddStep("seek decoupling to -10000", () => + { + foreach (var c in this.ChildrenOfType()) + (c.TrackingClock as DecouplingClock)?.Seek(-10000); + }); + + AddStep("seek decoupling to 10000", () => + { + foreach (var c in this.ChildrenOfType()) + (c.TrackingClock as DecouplingClock)?.Seek(10000); + }); + } + + internal class TestClockWithRangeLimit : StopwatchClock + { + public double MinTime => 0; + public double MaxTime { get; set; } = double.PositiveInfinity; + + public TestClockWithRangeLimit() + : base(true) + { + } + + public override bool Seek(double position) + { + double clamped = Math.Clamp(position, MinTime, MaxTime); + + if (clamped != position) + { + // Emulate what bass will probably do in this case. + // TODO: confirm. + Stop(); + Seek(clamped); + return false; + } + + return base.Seek(position); + } } } } diff --git a/osu.Framework/Timing/StopwatchClock.cs b/osu.Framework/Timing/StopwatchClock.cs index ea304afedb..be4f70afc6 100644 --- a/osu.Framework/Timing/StopwatchClock.cs +++ b/osu.Framework/Timing/StopwatchClock.cs @@ -66,7 +66,7 @@ public double Rate public void ResetSpeedAdjustments() => Rate = 1; - public bool Seek(double position) + public virtual bool Seek(double position) { // Determine the offset that when added to stopwatchCurrentTime; results in the requested time value seekOffset = position - stopwatchCurrentTime; From 17511bcd50006b9d6ad1863be17c94f64e90ff1b Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 20 Sep 2023 19:14:12 +0900 Subject: [PATCH 12/43] Fix decoupling edge cases --- osu.Framework/Timing/DecouplingClock.cs | 48 ++++++++++++++++--------- 1 file changed, 32 insertions(+), 16 deletions(-) diff --git a/osu.Framework/Timing/DecouplingClock.cs b/osu.Framework/Timing/DecouplingClock.cs index a532f4a49e..12fab3d8ac 100644 --- a/osu.Framework/Timing/DecouplingClock.cs +++ b/osu.Framework/Timing/DecouplingClock.cs @@ -20,7 +20,7 @@ public class DecouplingClock : ISourceChangeableClock, IAdjustableClock private IAdjustableClock adjustableSourceClock; private bool isRunning; - private double? lastDecoupledTimeConsumed; + private double? lastReferenceTimeConsumed; public bool IsRunning { @@ -40,21 +40,33 @@ public virtual double CurrentTime { get { - if (Source.IsRunning || !AllowDecoupling) - return currentTime = Source.CurrentTime; - - if (!isRunning) - return currentTime; - - if (lastDecoupledTimeConsumed != null) - currentTime += (realtimeReferenceClock.CurrentTime - lastDecoupledTimeConsumed.Value) * Rate; - - lastDecoupledTimeConsumed = realtimeReferenceClock.CurrentTime; - - if (lastDecoupledTimeConsumed < 0 && currentTime >= 0) - adjustableSourceClock.Start(); - - return currentTime; + try + { + if (Source.IsRunning || !AllowDecoupling) + return currentTime = Source.CurrentTime; + + if (!isRunning) + return currentTime; + + if (lastReferenceTimeConsumed == null) + return currentTime; + + double elapsedSinceLastCall = (realtimeReferenceClock.CurrentTime - lastReferenceTimeConsumed.Value) * Rate; + + // Crossing the zero time boundary, we can attempt to start and use the source clock. + if (currentTime < 0 && currentTime + elapsedSinceLastCall >= 0) + { + adjustableSourceClock.Start(); + if (Source.IsRunning) + return currentTime = Source.CurrentTime; + } + + return currentTime += elapsedSinceLastCall; + } + finally + { + lastReferenceTimeConsumed = realtimeReferenceClock.CurrentTime; + } } } @@ -98,7 +110,11 @@ public void Stop() public bool Seek(double position) { if (adjustableSourceClock.Seek(position)) + { + if (isRunning && !Source.IsRunning) + adjustableSourceClock.Start(); return true; + } if (!AllowDecoupling) return false; From fceec3b626edd45907c7d557eb487c7cba616370 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 21 Sep 2023 02:39:57 +0900 Subject: [PATCH 13/43] Add xmldoc to class --- osu.Framework/Timing/DecouplingClock.cs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/osu.Framework/Timing/DecouplingClock.cs b/osu.Framework/Timing/DecouplingClock.cs index 12fab3d8ac..d422d71721 100644 --- a/osu.Framework/Timing/DecouplingClock.cs +++ b/osu.Framework/Timing/DecouplingClock.cs @@ -6,6 +6,15 @@ namespace osu.Framework.Timing { + /// + /// A decoupling clock allows taking an existing clock which has seek limitations and provides + /// a controllable encapsulated view of that clock with the ability to seek and track time + /// outside of the normally seekable bounds. + /// + /// Put simply, it will take a Track which can only track time from 0..trackLength and allow + /// both negative seeks and seeks beyond trackLength. It will also allow time to continue counting + /// beyond the end of the track even when not explicitly seeked. + /// public class DecouplingClock : ISourceChangeableClock, IAdjustableClock { public bool AllowDecoupling = true; From f6837fc56ccfa4a6679a02d222be586abe5f41db Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 21 Sep 2023 15:11:47 +0900 Subject: [PATCH 14/43] Mark `DecoupleableInterpolatingFramedClock` as obsolete --- ...st.cs => DecoupleableInterpolatingFramedClockTest.cs} | 4 +++- .../Timing/DecoupleableInterpolatingFramedClock.cs | 9 +++++---- osu.Framework/Timing/InterpolatingFramedClock.cs | 2 +- 3 files changed, 9 insertions(+), 6 deletions(-) rename osu.Framework.Tests/Clocks/{DecoupleableClockTest.cs => DecoupleableInterpolatingFramedClockTest.cs} (99%) diff --git a/osu.Framework.Tests/Clocks/DecoupleableClockTest.cs b/osu.Framework.Tests/Clocks/DecoupleableInterpolatingFramedClockTest.cs similarity index 99% rename from osu.Framework.Tests/Clocks/DecoupleableClockTest.cs rename to osu.Framework.Tests/Clocks/DecoupleableInterpolatingFramedClockTest.cs index d213e8cfe0..77649f407b 100644 --- a/osu.Framework.Tests/Clocks/DecoupleableClockTest.cs +++ b/osu.Framework.Tests/Clocks/DecoupleableInterpolatingFramedClockTest.cs @@ -9,7 +9,7 @@ namespace osu.Framework.Tests.Clocks { [TestFixture] - public class DecoupleableClockTest + public class DecoupleableInterpolatingFramedClockTest { private TestClockWithRange source = null!; private TestDecoupleableClock decoupleable = null!; @@ -440,7 +440,9 @@ public void TestStoppingAfterStartingBySourcePreservesState() Assert.AreEqual(source.CurrentTime, decoupleable.CurrentTime, decoupleable.AllowableErrorMilliseconds, "Decoupled should match source time."); } +#pragma warning disable CS0618 private class TestDecoupleableClock : DecoupleableInterpolatingFramedClock +#pragma warning restore CS0618 { public double? CustomAllowableErrorMilliseconds { get; set; } diff --git a/osu.Framework/Timing/DecoupleableInterpolatingFramedClock.cs b/osu.Framework/Timing/DecoupleableInterpolatingFramedClock.cs index 72ad443d8a..064f936620 100644 --- a/osu.Framework/Timing/DecoupleableInterpolatingFramedClock.cs +++ b/osu.Framework/Timing/DecoupleableInterpolatingFramedClock.cs @@ -16,6 +16,7 @@ namespace osu.Framework.Timing /// If a is set, it is presumed that we have exclusive control over operations on it. /// This is used to our advantage to allow correct state tracking in the event of cross-thread communication delays (with an audio thread, for instance). /// + [Obsolete("This clock implementation is too complex and no longer. Use DecouplingClock instead.")] // can be removed 20240321. public class DecoupleableInterpolatingFramedClock : InterpolatingFramedClock, IAdjustableClock { /// @@ -26,7 +27,7 @@ public class DecoupleableInterpolatingFramedClock : InterpolatingFramedClock, IA /// /// In some cases we should always use the interpolated source. /// - private bool useInterpolatedSourceTime => IsRunning && FramedSourceClock?.IsRunning == true; + private bool useInterpolatedSourceTime => IsRunning && FramedSourceClock.IsRunning; private readonly FramedClock decoupledClock; private readonly StopwatchClock decoupledStopwatch; @@ -50,9 +51,9 @@ public class DecoupleableInterpolatingFramedClock : InterpolatingFramedClock, IA public override double ElapsedFrameTime => elapsedFrameTime; - public override double Rate + public new double Rate { - get => Source?.Rate ?? 1; + get => Source.Rate; set { if (adjustableSource == null) @@ -73,7 +74,7 @@ public override void ProcessFrame() { base.ProcessFrame(); - bool sourceRunning = Source?.IsRunning ?? false; + bool sourceRunning = Source.IsRunning; decoupledStopwatch.Rate = adjustableSource?.Rate ?? 1; diff --git a/osu.Framework/Timing/InterpolatingFramedClock.cs b/osu.Framework/Timing/InterpolatingFramedClock.cs index a47f6d458b..4b702587a4 100644 --- a/osu.Framework/Timing/InterpolatingFramedClock.cs +++ b/osu.Framework/Timing/InterpolatingFramedClock.cs @@ -9,7 +9,7 @@ namespace osu.Framework.Timing /// /// A clock which uses an internal stopwatch to interpolate (smooth out) a source. /// - public class InterpolatingFramedClock : IFrameBasedClock, ISourceChangeableClock + public class InterpolatingFramedClock : IFrameBasedClock, ISourceChangeableClock // TODO: seal when DecoupleableInterpolatingFramedClock is gone. { /// /// The amount of error that is allowed between the source and interpolated time before the interpolated time is ignored and the source time is used. From 6330038a9d271ecbc7c0993c2afbc98f2d43029d Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 21 Sep 2023 15:12:12 +0900 Subject: [PATCH 15/43] Seal new decoupling implementation to avoid any future funny-business --- osu.Framework/Timing/DecouplingClock.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/osu.Framework/Timing/DecouplingClock.cs b/osu.Framework/Timing/DecouplingClock.cs index d422d71721..4129b5d136 100644 --- a/osu.Framework/Timing/DecouplingClock.cs +++ b/osu.Framework/Timing/DecouplingClock.cs @@ -15,7 +15,7 @@ namespace osu.Framework.Timing /// both negative seeks and seeks beyond trackLength. It will also allow time to continue counting /// beyond the end of the track even when not explicitly seeked. /// - public class DecouplingClock : ISourceChangeableClock, IAdjustableClock + public sealed class DecouplingClock : ISourceChangeableClock, IAdjustableClock { public bool AllowDecoupling = true; @@ -45,7 +45,7 @@ public bool IsRunning private double currentTime; - public virtual double CurrentTime + public double CurrentTime { get { @@ -134,7 +134,7 @@ public bool Seek(double position) public void ResetSpeedAdjustments() => adjustableSourceClock.ResetSpeedAdjustments(); - public virtual double Rate + public double Rate { get => adjustableSourceClock.Rate; set => adjustableSourceClock.Rate = value; From 5b4775ff9ea100a4e22e92c6f23553277191df6a Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 21 Sep 2023 15:27:22 +0900 Subject: [PATCH 16/43] Ensure `DecoupleableInterpolatingFramedClock` can still work correctly with new assertions --- osu.Framework/Timing/DecoupleableInterpolatingFramedClock.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Framework/Timing/DecoupleableInterpolatingFramedClock.cs b/osu.Framework/Timing/DecoupleableInterpolatingFramedClock.cs index 064f936620..f50f203e6b 100644 --- a/osu.Framework/Timing/DecoupleableInterpolatingFramedClock.cs +++ b/osu.Framework/Timing/DecoupleableInterpolatingFramedClock.cs @@ -134,7 +134,7 @@ public override void ProcessFrame() public override void ChangeSource(IClock? source) { - if (source == null) return; + source ??= new StopwatchClock(true); // transfer our value to the source clock. (source as IAdjustableClock)?.Seek(CurrentTime); From 4dea285871539c46d69c2b6c03c4de4d329ae7b7 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 21 Sep 2023 17:16:14 +0900 Subject: [PATCH 17/43] Fix `Seek` target not transferring correctly when seek succeeds on source while stopped --- .../Clocks/DecouplingClockTest.cs | 20 +++++++++++++++++++ osu.Framework/Timing/DecouplingClock.cs | 15 +++++++++----- 2 files changed, 30 insertions(+), 5 deletions(-) diff --git a/osu.Framework.Tests/Clocks/DecouplingClockTest.cs b/osu.Framework.Tests/Clocks/DecouplingClockTest.cs index 39b55ee8e0..85e0096115 100644 --- a/osu.Framework.Tests/Clocks/DecouplingClockTest.cs +++ b/osu.Framework.Tests/Clocks/DecouplingClockTest.cs @@ -128,6 +128,16 @@ public void TestSeekNegativeWhileNotDecoupling() Assert.That(decouplingClock.CurrentTime, Is.EqualTo(0)); } + [Test] + public void TestSeekPositiveWhileNotDecoupling() + { + decouplingClock.AllowDecoupling = false; + Assert.That(decouplingClock.Seek(1000), Is.True); + + Assert.That(source.CurrentTime, Is.EqualTo(1000)); + Assert.That(decouplingClock.CurrentTime, Is.EqualTo(1000)); + } + #endregion #region Operation in decoupling mode @@ -159,6 +169,16 @@ public void TestSeekNegativeWhileDecoupling() Assert.That(decouplingClock.CurrentTime, Is.EqualTo(-1000)); } + [Test] + public void TestSeekPositiveWhileDecoupling() + { + decouplingClock.AllowDecoupling = true; + Assert.That(decouplingClock.Seek(1000), Is.True); + + Assert.That(source.CurrentTime, Is.EqualTo(1000)); + Assert.That(decouplingClock.CurrentTime, Is.EqualTo(1000)); + } + // TODO: test playback is always forward over the 0ms boundary. // TODO: test backwards playback. (over the boundary?) diff --git a/osu.Framework/Timing/DecouplingClock.cs b/osu.Framework/Timing/DecouplingClock.cs index 4129b5d136..97f244fb7e 100644 --- a/osu.Framework/Timing/DecouplingClock.cs +++ b/osu.Framework/Timing/DecouplingClock.cs @@ -118,15 +118,20 @@ public void Stop() public bool Seek(double position) { - if (adjustableSourceClock.Seek(position)) + bool sourceCouldSeek = adjustableSourceClock.Seek(position); + + if (sourceCouldSeek) { + // Transfer attempt to transfer decoupled running state to source + // in the case we succeeded. if (isRunning && !Source.IsRunning) adjustableSourceClock.Start(); - return true; } - - if (!AllowDecoupling) - return false; + else + { + if (!AllowDecoupling) + return false; + } currentTime = position; return true; From eedd1bd9a34f0d177e66f59f15d054f3b654cf6e Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 22 Sep 2023 13:45:10 +0900 Subject: [PATCH 18/43] Fix `DecouplingClock.Start` incorrect resetting negative time to zero --- osu.Framework/Timing/DecouplingClock.cs | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/osu.Framework/Timing/DecouplingClock.cs b/osu.Framework/Timing/DecouplingClock.cs index 97f244fb7e..065e44b65c 100644 --- a/osu.Framework/Timing/DecouplingClock.cs +++ b/osu.Framework/Timing/DecouplingClock.cs @@ -31,6 +31,12 @@ public sealed class DecouplingClock : ISourceChangeableClock, IAdjustableClock private bool isRunning; private double? lastReferenceTimeConsumed; + /// + /// Whether the last operation failed. + /// This denotes that we need to in decoupled mode (if possible). + /// + private bool lastSeekFailed; + public bool IsRunning { get @@ -63,6 +69,7 @@ public double CurrentTime double elapsedSinceLastCall = (realtimeReferenceClock.CurrentTime - lastReferenceTimeConsumed.Value) * Rate; // Crossing the zero time boundary, we can attempt to start and use the source clock. + // TODO: consider if this is too specific. may need to be mentioned in class xmldoc if we keep it. if (currentTime < 0 && currentTime + elapsedSinceLastCall >= 0) { adjustableSourceClock.Start(); @@ -102,10 +109,23 @@ public void Reset() { adjustableSourceClock.Reset(); isRunning = false; + lastSeekFailed = false; } public void Start() { + if (isRunning) + return; + + // If the previous seek failed, avoid calling `Start` on the source clock. + // Doing so would potentially cause it to start from an incorrect location (ie. 0 in the case where we are tracking negative time). + // TODO: add test coverage + if (lastSeekFailed && AllowDecoupling) + { + isRunning = true; + return; + } + adjustableSourceClock.Start(); isRunning = adjustableSourceClock.IsRunning || AllowDecoupling; } @@ -118,9 +138,9 @@ public void Stop() public bool Seek(double position) { - bool sourceCouldSeek = adjustableSourceClock.Seek(position); + lastSeekFailed = !adjustableSourceClock.Seek(position); - if (sourceCouldSeek) + if (!lastSeekFailed) { // Transfer attempt to transfer decoupled running state to source // in the case we succeeded. From 38bf12562d7d2ea427d5b854ce3bb5418a197c45 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 22 Sep 2023 13:47:54 +0900 Subject: [PATCH 19/43] Fix potentially incorrect `isRunning` state after source change via `ChangeSource` --- .../Clocks/DecouplingClockTest.cs | 18 ++++++++++++++++++ osu.Framework/Timing/DecouplingClock.cs | 1 + 2 files changed, 19 insertions(+) diff --git a/osu.Framework.Tests/Clocks/DecouplingClockTest.cs b/osu.Framework.Tests/Clocks/DecouplingClockTest.cs index 85e0096115..4e049ee6ac 100644 --- a/osu.Framework.Tests/Clocks/DecouplingClockTest.cs +++ b/osu.Framework.Tests/Clocks/DecouplingClockTest.cs @@ -98,6 +98,24 @@ public void ChangeSourceUpdatesToNewSourceTime() Assert.That(secondSource.CurrentTime, Is.EqualTo(second_source_time)); } + [TestCase(true)] + [TestCase(false)] + public void ChangeSourceUpdatesToCorrectSourceState(bool allowDecoupling) + { + decouplingClock.AllowDecoupling = allowDecoupling; + + source.Start(); + Assert.That(decouplingClock.IsRunning, Is.True); + + var secondSource = new TestClock(); + + decouplingClock.ChangeSource(secondSource); + Assert.That(decouplingClock.IsRunning, Is.False); + + decouplingClock.ChangeSource(source); + Assert.That(decouplingClock.IsRunning, Is.True); + } + #endregion #region Operation in non-decoupling mode diff --git a/osu.Framework/Timing/DecouplingClock.cs b/osu.Framework/Timing/DecouplingClock.cs index 065e44b65c..a8c1417110 100644 --- a/osu.Framework/Timing/DecouplingClock.cs +++ b/osu.Framework/Timing/DecouplingClock.cs @@ -101,6 +101,7 @@ public void ChangeSource(IClock? source) throw new ArgumentException($"Clock must be of type {nameof(IAdjustableClock)}"); adjustableSourceClock = adjustableSource; + isRunning = adjustableSource.IsRunning; } #region IAdjustableClock implementation From aebf28ebba841bcc2d7463e8a5b5c562a4d56dfb Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 22 Sep 2023 13:48:45 +0900 Subject: [PATCH 20/43] Fix underlying clock not being `Stop`ped when entering a decoupled state --- osu.Framework/Timing/DecouplingClock.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/osu.Framework/Timing/DecouplingClock.cs b/osu.Framework/Timing/DecouplingClock.cs index a8c1417110..18287142d6 100644 --- a/osu.Framework/Timing/DecouplingClock.cs +++ b/osu.Framework/Timing/DecouplingClock.cs @@ -152,6 +152,10 @@ public bool Seek(double position) { if (!AllowDecoupling) return false; + + // Ensure the underlying clock is stopped as we enter decoupled mode. + // TODO: test coverage + adjustableSourceClock.Stop(); } currentTime = position; From 1db323dd48a7e2b3cc40ff3211fcf64626f48b9c Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 22 Sep 2023 13:49:51 +0900 Subject: [PATCH 21/43] Refactor `IsRunning` to hopefully be easier to follow --- osu.Framework/Timing/DecouplingClock.cs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/osu.Framework/Timing/DecouplingClock.cs b/osu.Framework/Timing/DecouplingClock.cs index 18287142d6..d23629b020 100644 --- a/osu.Framework/Timing/DecouplingClock.cs +++ b/osu.Framework/Timing/DecouplingClock.cs @@ -42,10 +42,16 @@ public bool IsRunning get { // Always use the source clock's running state if it's running. - if (Source.IsRunning || !AllowDecoupling) - return isRunning = Source.IsRunning; + if (Source.IsRunning) + return isRunning = true; - return isRunning; + // If we're allowed to be decoupled, maintain our internal running state. + if (AllowDecoupling) + return isRunning; + + // Otherwise we're definitely not running. + Debug.Assert(!Source.IsRunning); + return isRunning = false; } } From 7d4597744755007925b6f3d3e4164a3233de218f Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 22 Sep 2023 14:03:41 +0900 Subject: [PATCH 22/43] Add note about not being a framed clock --- osu.Framework/Timing/DecouplingClock.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/osu.Framework/Timing/DecouplingClock.cs b/osu.Framework/Timing/DecouplingClock.cs index d23629b020..17ff699215 100644 --- a/osu.Framework/Timing/DecouplingClock.cs +++ b/osu.Framework/Timing/DecouplingClock.cs @@ -15,6 +15,11 @@ namespace osu.Framework.Timing /// both negative seeks and seeks beyond trackLength. It will also allow time to continue counting /// beyond the end of the track even when not explicitly seeked. /// + /// + /// This clock is intentionally *not* an to keep things simple. + /// It is important therefore that if the source clock is set to a framed clock, you will need to call + /// externally. + /// public sealed class DecouplingClock : ISourceChangeableClock, IAdjustableClock { public bool AllowDecoupling = true; From 28a5ca982696869bd4b2e254da1209b3b306434f Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 22 Sep 2023 17:10:06 +0900 Subject: [PATCH 23/43] Update all basic assumption tests to test in both coupled modes --- .../Clocks/DecouplingClockTest.cs | 41 +++++++++++-------- 1 file changed, 25 insertions(+), 16 deletions(-) diff --git a/osu.Framework.Tests/Clocks/DecouplingClockTest.cs b/osu.Framework.Tests/Clocks/DecouplingClockTest.cs index 4e049ee6ac..5b43fc8ad9 100644 --- a/osu.Framework.Tests/Clocks/DecouplingClockTest.cs +++ b/osu.Framework.Tests/Clocks/DecouplingClockTest.cs @@ -22,12 +22,13 @@ public void SetUp() decouplingClock.ChangeSource(source); } - #region Basic assumptions + #region Basic assumptions (which hold for both decoupled and not) - [Test] - public void TestStartFromDecoupling() + [TestCase(true)] + [TestCase(false)] + public void TestStartFromDecoupling(bool allowDecoupling) { - decouplingClock.AllowDecoupling = false; + decouplingClock.AllowDecoupling = allowDecoupling; Assert.That(source.IsRunning, Is.False); Assert.That(decouplingClock.IsRunning, Is.False); @@ -38,10 +39,11 @@ public void TestStartFromDecoupling() Assert.That(decouplingClock.IsRunning, Is.True); } - [Test] - public void TestStartFromSource() + [TestCase(true)] + [TestCase(false)] + public void TestStartFromSource(bool allowDecoupling) { - decouplingClock.AllowDecoupling = false; + decouplingClock.AllowDecoupling = allowDecoupling; Assert.That(source.IsRunning, Is.False); Assert.That(decouplingClock.IsRunning, Is.False); @@ -52,10 +54,11 @@ public void TestStartFromSource() Assert.That(decouplingClock.IsRunning, Is.True); } - [Test] - public void TestSeekFromDecoupling() + [TestCase(true)] + [TestCase(false)] + public void TestSeekFromDecoupling(bool allowDecoupling) { - decouplingClock.AllowDecoupling = false; + decouplingClock.AllowDecoupling = allowDecoupling; Assert.That(source.CurrentTime, Is.EqualTo(0)); Assert.That(decouplingClock.CurrentTime, Is.EqualTo(0)); @@ -66,10 +69,15 @@ public void TestSeekFromDecoupling() Assert.That(decouplingClock.CurrentTime, Is.EqualTo(1000)); } - [Test] - public void TestSeekFromSource() + [TestCase(true)] + [TestCase(false)] + public void TestSeekFromSource(bool allowDecoupling) { - decouplingClock.AllowDecoupling = false; + decouplingClock.AllowDecoupling = allowDecoupling; + + // Seeking the source when in decoupled mode isn't really supported. + // But it will work if the source is running. + source.Start(); Assert.That(source.CurrentTime, Is.EqualTo(0)); Assert.That(decouplingClock.CurrentTime, Is.EqualTo(0)); @@ -80,10 +88,11 @@ public void TestSeekFromSource() Assert.That(decouplingClock.CurrentTime, Is.EqualTo(1000)); } - [Test] - public void ChangeSourceUpdatesToNewSourceTime() + [TestCase(true)] + [TestCase(false)] + public void ChangeSourceUpdatesToNewSourceTime(bool allowDecoupling) { - decouplingClock.AllowDecoupling = false; + decouplingClock.AllowDecoupling = allowDecoupling; const double first_source_time = 256000; const double second_source_time = 128000; From 51eccf4942af0507b2def321b2abffab0ceb13b9 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 22 Sep 2023 17:20:24 +0900 Subject: [PATCH 24/43] Add test to show seeking source clock when decoupling can go awry --- .../Clocks/DecouplingClockTest.cs | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/osu.Framework.Tests/Clocks/DecouplingClockTest.cs b/osu.Framework.Tests/Clocks/DecouplingClockTest.cs index 5b43fc8ad9..967b679273 100644 --- a/osu.Framework.Tests/Clocks/DecouplingClockTest.cs +++ b/osu.Framework.Tests/Clocks/DecouplingClockTest.cs @@ -206,6 +206,31 @@ public void TestSeekPositiveWhileDecoupling() Assert.That(decouplingClock.CurrentTime, Is.EqualTo(1000)); } + /// + /// In decoupled operation, seeking the source while it's not playing is undefined + /// behaviour. + /// + [Test] + public void TestSeekFromSourceWhileDecoupling() + { + decouplingClock.AllowDecoupling = true; + + Assert.That(source.CurrentTime, Is.EqualTo(0)); + Assert.That(decouplingClock.CurrentTime, Is.EqualTo(0)); + + source.Seek(1000); + + Assert.That(source.CurrentTime, Is.EqualTo(1000)); + // One might expect this to match the source, but with the current implementation, it doesn't. + Assert.That(decouplingClock.CurrentTime, Is.Not.EqualTo(1000)); + + // One should seek the decoupling clock directly. + decouplingClock.Seek(1000); + + Assert.That(source.CurrentTime, Is.EqualTo(1000)); + Assert.That(decouplingClock.CurrentTime, Is.EqualTo(1000)); + } + // TODO: test playback is always forward over the 0ms boundary. // TODO: test backwards playback. (over the boundary?) From 142d2c98a90c3a1ba4908ebfef0c14b3c2711934 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 22 Sep 2023 17:23:08 +0900 Subject: [PATCH 25/43] Change `InterpolatingFramedClock`'s ctor to require a framed source --- osu.Framework/Timing/InterpolatingFramedClock.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Framework/Timing/InterpolatingFramedClock.cs b/osu.Framework/Timing/InterpolatingFramedClock.cs index 4b702587a4..4cc5432daa 100644 --- a/osu.Framework/Timing/InterpolatingFramedClock.cs +++ b/osu.Framework/Timing/InterpolatingFramedClock.cs @@ -52,7 +52,7 @@ public class InterpolatingFramedClock : IFrameBasedClock, ISourceChangeableClock private double currentTime; - public InterpolatingFramedClock(IClock? source = null) + public InterpolatingFramedClock(IFrameBasedClock? source = null) { ChangeSource(source); Debug.Assert(Source != null); From d8b3ebd1f1770f9706cbc1b011516801cf349c4e Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 22 Sep 2023 17:47:48 +0900 Subject: [PATCH 26/43] Fix `TestSceneClocks` failing and not making much sense --- .../Visual/Clocks/LaggyFramedClock.cs | 16 +++++++--------- .../Visual/Clocks/TestSceneClock.cs | 6 +++++- .../Visual/Clocks/TestSceneClocks.cs | 19 +++++++++++-------- 3 files changed, 23 insertions(+), 18 deletions(-) diff --git a/osu.Framework.Tests/Visual/Clocks/LaggyFramedClock.cs b/osu.Framework.Tests/Visual/Clocks/LaggyFramedClock.cs index 87d866d62b..4f47ab6075 100644 --- a/osu.Framework.Tests/Visual/Clocks/LaggyFramedClock.cs +++ b/osu.Framework.Tests/Visual/Clocks/LaggyFramedClock.cs @@ -7,22 +7,20 @@ namespace osu.Framework.Tests.Visual.Clocks { public class LaggyFramedClock : FramedClock { - public LaggyFramedClock(IClock? source = null) + private readonly int frameSkip; + + private int currentFrame; + + public LaggyFramedClock(int frameSkip, IClock? source = null) : base(source) { + this.frameSkip = frameSkip; } - private int lastSecond; - public override void ProcessFrame() { - int second = (int)Source.CurrentTime / 500; - - if (second != lastSecond) - { - lastSecond = second; + if (currentFrame++ % frameSkip == 0) base.ProcessFrame(); - } } } } diff --git a/osu.Framework.Tests/Visual/Clocks/TestSceneClock.cs b/osu.Framework.Tests/Visual/Clocks/TestSceneClock.cs index b91445240a..72562669fc 100644 --- a/osu.Framework.Tests/Visual/Clocks/TestSceneClock.cs +++ b/osu.Framework.Tests/Visual/Clocks/TestSceneClock.cs @@ -7,6 +7,7 @@ using osu.Framework.Graphics.Shapes; using osu.Framework.Graphics.Sprites; using osu.Framework.Input.Events; +using osu.Framework.Testing; using osu.Framework.Timing; using osuTK; using osuTK.Graphics; @@ -25,12 +26,15 @@ protected TestSceneClock() Direction = FillDirection.Full, RelativeSizeAxes = Axes.Both, }; + } + [SetUpSteps] + public void SetUpSteps() + { AddStep("clear all", () => { fill.Clear(); lastClock = null; - AddClock(Clock, "game"); }); } diff --git a/osu.Framework.Tests/Visual/Clocks/TestSceneClocks.cs b/osu.Framework.Tests/Visual/Clocks/TestSceneClocks.cs index a25b6a907f..872c5de661 100644 --- a/osu.Framework.Tests/Visual/Clocks/TestSceneClocks.cs +++ b/osu.Framework.Tests/Visual/Clocks/TestSceneClocks.cs @@ -2,6 +2,7 @@ // See the LICENCE file in the repository root for full licence text. using System; +using NUnit.Framework; using osu.Framework.Testing; using osu.Framework.Timing; @@ -9,17 +10,19 @@ namespace osu.Framework.Tests.Visual.Clocks { public partial class TestSceneClocks : TestSceneClock { - protected override void LoadComplete() + [Test] + public void TestInterpolationWithLaggyClock() { - base.LoadComplete(); - AddStep("add stopwatch", () => AddClock(new StopwatchClock(true))); - AddStep("add non-negative stopwatch", () => AddClock(new TestClockWithRangeLimit())); - AddStep("add framed", () => AddClock(new FramedClock())); - AddStep("add lag", () => AddClock(new LaggyFramedClock())); + AddStep("add lag", () => AddClock(new LaggyFramedClock(50))); AddStep("add interpolating", () => AddClock(new InterpolatingFramedClock())); - AddStep("add decoupling(true)", () => AddClock(new DecouplingClock { AllowDecoupling = true })); - AddStep("add decoupling(false)", () => AddClock(new DecouplingClock { AllowDecoupling = false })); + } + + [Test] + public void TestDecouplingWithRangeLimited() + { + AddStep("add non-negative stopwatch", () => AddClock(new TestClockWithRangeLimit())); + AddStep("add decoupling", () => AddClock(new DecouplingClock { AllowDecoupling = true })); AddStep("seek decoupling to -10000", () => { From c7d68bd7f8bc0983fb6c2d8440ce4f7c8a5ac815 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 22 Sep 2023 17:51:57 +0900 Subject: [PATCH 27/43] Add elapsed millisecond display to clocks test scene --- .../Visual/Clocks/TestSceneClock.cs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/osu.Framework.Tests/Visual/Clocks/TestSceneClock.cs b/osu.Framework.Tests/Visual/Clocks/TestSceneClock.cs index 72562669fc..7ca2e894a1 100644 --- a/osu.Framework.Tests/Visual/Clocks/TestSceneClock.cs +++ b/osu.Framework.Tests/Visual/Clocks/TestSceneClock.cs @@ -55,6 +55,7 @@ public partial class VisualClock : CompositeDrawable public IClock TrackingClock { get; } private readonly SpriteText time; + private readonly SpriteText elapsed; private readonly SpriteText rate; private bool zeroed = true; @@ -102,6 +103,13 @@ public VisualClock(IClock trackingClock, string? name = null) Font = new FontUsage(size: 14), Y = 40, }, + elapsed = new SpriteText + { + Anchor = Anchor.Centre, + Origin = Anchor.Centre, + Font = new FontUsage(size: 14), + Y = 60, + }, hand = new Box { Colour = Color4.White, @@ -139,7 +147,11 @@ protected override void Update() double lastTime = TrackingClock.CurrentTime; - (TrackingClock as IFrameBasedClock)?.ProcessFrame(); + if (TrackingClock is IFrameBasedClock framedClock) + { + framedClock.ProcessFrame(); + elapsed.Text = $"{framedClock.ElapsedFrameTime:+0.00;-0.00} ms"; + } var timespan = TimeSpan.FromMilliseconds(TrackingClock.CurrentTime); time.Text = $"{timespan.Minutes:00}:{timespan.Seconds:00}:{timespan.Milliseconds:00}"; From 0fe7147f26ca8066410599dfefcc57ae1e715b85 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 22 Sep 2023 17:57:16 +0900 Subject: [PATCH 28/43] Reset `currentTime` on `Reset` for good measure (and add test coverage) --- .../Clocks/DecouplingClockTest.cs | 24 +++++++++++++++++-- osu.Framework.Tests/Clocks/TestClock.cs | 10 ++++++-- osu.Framework/Timing/DecouplingClock.cs | 1 + 3 files changed, 31 insertions(+), 4 deletions(-) diff --git a/osu.Framework.Tests/Clocks/DecouplingClockTest.cs b/osu.Framework.Tests/Clocks/DecouplingClockTest.cs index 967b679273..b183a2523a 100644 --- a/osu.Framework.Tests/Clocks/DecouplingClockTest.cs +++ b/osu.Framework.Tests/Clocks/DecouplingClockTest.cs @@ -90,7 +90,7 @@ public void TestSeekFromSource(bool allowDecoupling) [TestCase(true)] [TestCase(false)] - public void ChangeSourceUpdatesToNewSourceTime(bool allowDecoupling) + public void TestChangeSourceUpdatesToNewSourceTime(bool allowDecoupling) { decouplingClock.AllowDecoupling = allowDecoupling; @@ -109,7 +109,7 @@ public void ChangeSourceUpdatesToNewSourceTime(bool allowDecoupling) [TestCase(true)] [TestCase(false)] - public void ChangeSourceUpdatesToCorrectSourceState(bool allowDecoupling) + public void TestChangeSourceUpdatesToCorrectSourceState(bool allowDecoupling) { decouplingClock.AllowDecoupling = allowDecoupling; @@ -125,6 +125,26 @@ public void ChangeSourceUpdatesToCorrectSourceState(bool allowDecoupling) Assert.That(decouplingClock.IsRunning, Is.True); } + [TestCase(true)] + [TestCase(false)] + public void TestReset(bool allowDecoupling) + { + decouplingClock.AllowDecoupling = allowDecoupling; + + source.Seek(2000); + source.Start(); + + Assert.That(decouplingClock.IsRunning, Is.True); + Assert.That(decouplingClock.CurrentTime, Is.EqualTo(2000)); + + decouplingClock.Reset(); + + Assert.That(decouplingClock.IsRunning, Is.False); + Assert.That(source.IsRunning, Is.False); + Assert.That(decouplingClock.CurrentTime, Is.EqualTo(0)); + Assert.That(source.CurrentTime, Is.EqualTo(0)); + } + #endregion #region Operation in non-decoupling mode diff --git a/osu.Framework.Tests/Clocks/TestClock.cs b/osu.Framework.Tests/Clocks/TestClock.cs index a91b4de8cd..4b56c04375 100644 --- a/osu.Framework.Tests/Clocks/TestClock.cs +++ b/osu.Framework.Tests/Clocks/TestClock.cs @@ -1,6 +1,7 @@ // 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 osu.Framework.Timing; namespace osu.Framework.Tests.Clocks @@ -12,7 +13,12 @@ internal class TestClock : IAdjustableClock public bool IsRunning { get; private set; } - public void Reset() => throw new System.NotImplementedException(); + public void Reset() + { + CurrentTime = 0; + IsRunning = false; + } + public void Start() => IsRunning = true; public void Stop() => IsRunning = false; @@ -22,6 +28,6 @@ public virtual bool Seek(double position) return true; } - public void ResetSpeedAdjustments() => throw new System.NotImplementedException(); + public void ResetSpeedAdjustments() => throw new NotImplementedException(); } } diff --git a/osu.Framework/Timing/DecouplingClock.cs b/osu.Framework/Timing/DecouplingClock.cs index 17ff699215..fb449dc36b 100644 --- a/osu.Framework/Timing/DecouplingClock.cs +++ b/osu.Framework/Timing/DecouplingClock.cs @@ -122,6 +122,7 @@ public void Reset() adjustableSourceClock.Reset(); isRunning = false; lastSeekFailed = false; + currentTime = 0; } public void Start() From 44581107e2daf67a2f1b508bfcc1de5181bda99c Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 22 Sep 2023 19:40:59 +0900 Subject: [PATCH 29/43] Convert `DecouplingClock` to be a framed clock implementation --- ...ckTest.cs => DecouplingFramedClockTest.cs} | 25 ++++++- .../Visual/Clocks/TestSceneClocks.cs | 6 +- ...plingClock.cs => DecouplingFramedClock.cs} | 70 +++++++++++-------- 3 files changed, 65 insertions(+), 36 deletions(-) rename osu.Framework.Tests/Clocks/{DecouplingClockTest.cs => DecouplingFramedClockTest.cs} (92%) rename osu.Framework/Timing/{DecouplingClock.cs => DecouplingFramedClock.cs} (72%) diff --git a/osu.Framework.Tests/Clocks/DecouplingClockTest.cs b/osu.Framework.Tests/Clocks/DecouplingFramedClockTest.cs similarity index 92% rename from osu.Framework.Tests/Clocks/DecouplingClockTest.cs rename to osu.Framework.Tests/Clocks/DecouplingFramedClockTest.cs index b183a2523a..9ccfad14c6 100644 --- a/osu.Framework.Tests/Clocks/DecouplingClockTest.cs +++ b/osu.Framework.Tests/Clocks/DecouplingFramedClockTest.cs @@ -8,17 +8,17 @@ namespace osu.Framework.Tests.Clocks { [TestFixture] - public class DecouplingClockTest + public class DecouplingFramedClockTest { private TestClockWithRange source = null!; - private DecouplingClock decouplingClock = null!; + private DecouplingFramedClock decouplingClock = null!; [SetUp] public void SetUp() { source = new TestClockWithRange(); - decouplingClock = new DecouplingClock(); + decouplingClock = new DecouplingFramedClock(); decouplingClock.ChangeSource(source); } @@ -65,6 +65,11 @@ public void TestSeekFromDecoupling(bool allowDecoupling) decouplingClock.Seek(1000); + Assert.That(source.CurrentTime, Is.EqualTo(1000)); + Assert.That(decouplingClock.CurrentTime, Is.EqualTo(0)); + + decouplingClock.ProcessFrame(); + Assert.That(source.CurrentTime, Is.EqualTo(1000)); Assert.That(decouplingClock.CurrentTime, Is.EqualTo(1000)); } @@ -83,6 +88,7 @@ public void TestSeekFromSource(bool allowDecoupling) Assert.That(decouplingClock.CurrentTime, Is.EqualTo(0)); source.Seek(1000); + decouplingClock.ProcessFrame(); Assert.That(source.CurrentTime, Is.EqualTo(1000)); Assert.That(decouplingClock.CurrentTime, Is.EqualTo(1000)); @@ -100,10 +106,15 @@ public void TestChangeSourceUpdatesToNewSourceTime(bool allowDecoupling) source.Seek(first_source_time); source.Start(); + decouplingClock.ProcessFrame(); + var secondSource = new TestClock { CurrentTime = second_source_time }; Assert.That(decouplingClock.CurrentTime, Is.EqualTo(first_source_time)); + decouplingClock.ChangeSource(secondSource); + decouplingClock.ProcessFrame(); + Assert.That(secondSource.CurrentTime, Is.EqualTo(second_source_time)); } @@ -134,10 +145,13 @@ public void TestReset(bool allowDecoupling) source.Seek(2000); source.Start(); + decouplingClock.ProcessFrame(); + Assert.That(decouplingClock.IsRunning, Is.True); Assert.That(decouplingClock.CurrentTime, Is.EqualTo(2000)); decouplingClock.Reset(); + decouplingClock.ProcessFrame(); Assert.That(decouplingClock.IsRunning, Is.False); Assert.That(source.IsRunning, Is.False); @@ -180,6 +194,7 @@ public void TestSeekPositiveWhileNotDecoupling() { decouplingClock.AllowDecoupling = false; Assert.That(decouplingClock.Seek(1000), Is.True); + decouplingClock.ProcessFrame(); Assert.That(source.CurrentTime, Is.EqualTo(1000)); Assert.That(decouplingClock.CurrentTime, Is.EqualTo(1000)); @@ -211,6 +226,8 @@ public void TestSeekNegativeWhileDecoupling() decouplingClock.AllowDecoupling = true; Assert.That(decouplingClock.Seek(-1000), Is.True); + decouplingClock.ProcessFrame(); + Assert.That(source.CurrentTime, Is.EqualTo(0)); // We're decoupling, so should be able to go beyond zero. Assert.That(decouplingClock.CurrentTime, Is.EqualTo(-1000)); @@ -221,6 +238,7 @@ public void TestSeekPositiveWhileDecoupling() { decouplingClock.AllowDecoupling = true; Assert.That(decouplingClock.Seek(1000), Is.True); + decouplingClock.ProcessFrame(); Assert.That(source.CurrentTime, Is.EqualTo(1000)); Assert.That(decouplingClock.CurrentTime, Is.EqualTo(1000)); @@ -246,6 +264,7 @@ public void TestSeekFromSourceWhileDecoupling() // One should seek the decoupling clock directly. decouplingClock.Seek(1000); + decouplingClock.ProcessFrame(); Assert.That(source.CurrentTime, Is.EqualTo(1000)); Assert.That(decouplingClock.CurrentTime, Is.EqualTo(1000)); diff --git a/osu.Framework.Tests/Visual/Clocks/TestSceneClocks.cs b/osu.Framework.Tests/Visual/Clocks/TestSceneClocks.cs index 872c5de661..3a359ecfca 100644 --- a/osu.Framework.Tests/Visual/Clocks/TestSceneClocks.cs +++ b/osu.Framework.Tests/Visual/Clocks/TestSceneClocks.cs @@ -22,18 +22,18 @@ public void TestInterpolationWithLaggyClock() public void TestDecouplingWithRangeLimited() { AddStep("add non-negative stopwatch", () => AddClock(new TestClockWithRangeLimit())); - AddStep("add decoupling", () => AddClock(new DecouplingClock { AllowDecoupling = true })); + AddStep("add decoupling", () => AddClock(new DecouplingFramedClock { AllowDecoupling = true })); AddStep("seek decoupling to -10000", () => { foreach (var c in this.ChildrenOfType()) - (c.TrackingClock as DecouplingClock)?.Seek(-10000); + (c.TrackingClock as DecouplingFramedClock)?.Seek(-10000); }); AddStep("seek decoupling to 10000", () => { foreach (var c in this.ChildrenOfType()) - (c.TrackingClock as DecouplingClock)?.Seek(10000); + (c.TrackingClock as DecouplingFramedClock)?.Seek(10000); }); } diff --git a/osu.Framework/Timing/DecouplingClock.cs b/osu.Framework/Timing/DecouplingFramedClock.cs similarity index 72% rename from osu.Framework/Timing/DecouplingClock.cs rename to osu.Framework/Timing/DecouplingFramedClock.cs index fb449dc36b..feddb7edb9 100644 --- a/osu.Framework/Timing/DecouplingClock.cs +++ b/osu.Framework/Timing/DecouplingFramedClock.cs @@ -15,12 +15,7 @@ namespace osu.Framework.Timing /// both negative seeks and seeks beyond trackLength. It will also allow time to continue counting /// beyond the end of the track even when not explicitly seeked. /// - /// - /// This clock is intentionally *not* an to keep things simple. - /// It is important therefore that if the source clock is set to a framed clock, you will need to call - /// externally. - /// - public sealed class DecouplingClock : ISourceChangeableClock, IAdjustableClock + public sealed class DecouplingFramedClock : ISourceChangeableClock, IAdjustableClock, IFrameBasedClock { public bool AllowDecoupling = true; @@ -60,44 +55,59 @@ public bool IsRunning } } + public double CurrentTime { get; private set; } + + public double ElapsedFrameTime { get; private set; } + public double FramesPerSecond => 0; + + /// + /// We need to track our internal time separately from the exposed to make sure + /// the exposed value is only ever updated on . + /// private double currentTime; - public double CurrentTime + public void ProcessFrame() { - get + double lastTime = CurrentTime; + + (Source as IFrameBasedClock)?.ProcessFrame(); + + try { - try + if (Source.IsRunning || !AllowDecoupling) { - if (Source.IsRunning || !AllowDecoupling) - return currentTime = Source.CurrentTime; - - if (!isRunning) - return currentTime; + currentTime = Source.CurrentTime; + return; + } - if (lastReferenceTimeConsumed == null) - return currentTime; + if (!isRunning) + return; - double elapsedSinceLastCall = (realtimeReferenceClock.CurrentTime - lastReferenceTimeConsumed.Value) * Rate; + if (lastReferenceTimeConsumed == null) + return; - // Crossing the zero time boundary, we can attempt to start and use the source clock. - // TODO: consider if this is too specific. may need to be mentioned in class xmldoc if we keep it. - if (currentTime < 0 && currentTime + elapsedSinceLastCall >= 0) - { - adjustableSourceClock.Start(); - if (Source.IsRunning) - return currentTime = Source.CurrentTime; - } + double elapsedSinceLastCall = (realtimeReferenceClock.CurrentTime - lastReferenceTimeConsumed.Value) * Rate; - return currentTime += elapsedSinceLastCall; - } - finally + // Crossing the zero time boundary, we can attempt to start and use the source clock. + // TODO: consider if this is too specific. may need to be mentioned in class xmldoc if we keep it. + if (CurrentTime < 0 && CurrentTime + elapsedSinceLastCall >= 0) { - lastReferenceTimeConsumed = realtimeReferenceClock.CurrentTime; + adjustableSourceClock.Start(); + if (Source.IsRunning) + currentTime = Source.CurrentTime; } + + currentTime += elapsedSinceLastCall; + } + finally + { + lastReferenceTimeConsumed = realtimeReferenceClock.CurrentTime; + CurrentTime = currentTime; + ElapsedFrameTime = CurrentTime - lastTime; } } - public DecouplingClock(IClock? source = null) + public DecouplingFramedClock(IClock? source = null) { ChangeSource(source); Debug.Assert(Source != null); From eb7eb6376271d076f1934853b007fe03160e3ee0 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 22 Sep 2023 19:42:08 +0900 Subject: [PATCH 30/43] Add more documentation, tidy up `ProcessFrame` and reorder members --- osu.Framework/Timing/DecouplingFramedClock.cs | 101 ++++++++++++------ 1 file changed, 66 insertions(+), 35 deletions(-) diff --git a/osu.Framework/Timing/DecouplingFramedClock.cs b/osu.Framework/Timing/DecouplingFramedClock.cs index feddb7edb9..0539dc6b9e 100644 --- a/osu.Framework/Timing/DecouplingFramedClock.cs +++ b/osu.Framework/Timing/DecouplingFramedClock.cs @@ -15,27 +15,20 @@ namespace osu.Framework.Timing /// both negative seeks and seeks beyond trackLength. It will also allow time to continue counting /// beyond the end of the track even when not explicitly seeked. /// + /// + /// This clock is intentionally *not* an to keep things simple. + /// It is important therefore that if the source clock is set to a framed clock, you will need to call + /// externally. + /// public sealed class DecouplingFramedClock : ISourceChangeableClock, IAdjustableClock, IFrameBasedClock { - public bool AllowDecoupling = true; - - public IClock Source { get; private set; } - - /// - /// This clock is used when we are decoupling from the source. - /// - private readonly StopwatchClock realtimeReferenceClock = new StopwatchClock(true); - - private IAdjustableClock adjustableSourceClock; - - private bool isRunning; - private double? lastReferenceTimeConsumed; - /// - /// Whether the last operation failed. - /// This denotes that we need to in decoupled mode (if possible). + /// Whether to allow operation in decoupled mode. Defaults to true. /// - private bool lastSeekFailed; + /// + /// When set to false, this clock will operate in a transparent pass-through mode. + /// + public bool AllowDecoupling { get; set; } = true; public bool IsRunning { @@ -58,14 +51,46 @@ public bool IsRunning public double CurrentTime { get; private set; } public double ElapsedFrameTime { get; private set; } + public double FramesPerSecond => 0; + /// + /// We maintain an internal running state so that when we notice the source clock has stopped, + /// we can continue to run in a decoupled mode (and know if we should be running or not). + /// + private bool isRunning; + /// /// We need to track our internal time separately from the exposed to make sure /// the exposed value is only ever updated on . /// private double currentTime; + /// + /// Tracks the current time of one ago. + /// + private double? lastReferenceTime; + + /// + /// Whether the last operation failed. + /// This denotes that we need to in decoupled mode (if possible). + /// + private bool lastSeekFailed; + + /// + /// This clock is used when we are decoupling from the source. + /// + private readonly StopwatchClock realtimeReferenceClock = new StopwatchClock(true); + + private IAdjustableClock adjustableSourceClock; + + public DecouplingFramedClock(IClock? source = null) + { + ChangeSource(source); + Debug.Assert(Source != null); + Debug.Assert(adjustableSourceClock != null); + } + public void ProcessFrame() { double lastTime = CurrentTime; @@ -74,45 +99,49 @@ public void ProcessFrame() try { - if (Source.IsRunning || !AllowDecoupling) + // If the source is running, there is never a need for any decoupling logic. + if (Source.IsRunning) { currentTime = Source.CurrentTime; return; } - if (!isRunning) + // If we're not allowed to decouple, we should also just pass-through the source time. + if (!AllowDecoupling) + { + currentTime = Source.CurrentTime; return; + } - if (lastReferenceTimeConsumed == null) + // We then want to check whether our internal running state permits time to elapse in decoupled mode. + if (!isRunning) return; - double elapsedSinceLastCall = (realtimeReferenceClock.CurrentTime - lastReferenceTimeConsumed.Value) * Rate; + double elapsedReferenceTime = (realtimeReferenceClock.CurrentTime - (lastReferenceTime ?? 0)) * Rate; - // Crossing the zero time boundary, we can attempt to start and use the source clock. - // TODO: consider if this is too specific. may need to be mentioned in class xmldoc if we keep it. - if (CurrentTime < 0 && CurrentTime + elapsedSinceLastCall >= 0) + currentTime += elapsedReferenceTime; + + // When crossing the zero time boundary, we should start and use the source clock. + if (lastTime < 0 && currentTime >= 0) { + adjustableSourceClock.Seek(currentTime); adjustableSourceClock.Start(); - if (Source.IsRunning) - currentTime = Source.CurrentTime; - } - currentTime += elapsedSinceLastCall; + // Don't use the source's time until next frame, as our decoupled time is likely more accurate + // (starting a clock, especially a TrackBass may have slight discrepancies). + } } finally { - lastReferenceTimeConsumed = realtimeReferenceClock.CurrentTime; + lastReferenceTime = realtimeReferenceClock.CurrentTime; CurrentTime = currentTime; ElapsedFrameTime = CurrentTime - lastTime; } } - public DecouplingFramedClock(IClock? source = null) - { - ChangeSource(source); - Debug.Assert(Source != null); - Debug.Assert(adjustableSourceClock != null); - } + #region ISourceChangeableClock implementation + + public IClock Source { get; private set; } public void ChangeSource(IClock? source) { @@ -125,6 +154,8 @@ public void ChangeSource(IClock? source) isRunning = adjustableSource.IsRunning; } + #endregion + #region IAdjustableClock implementation public void Reset() From 6651bae15ba29ac97b636b08d0673dcd3e227f6b Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sat, 23 Sep 2023 00:23:58 +0900 Subject: [PATCH 31/43] Fix incorrect reference time handling Regressed due to an innocent looking refactor in eb7eb63. --- .../Clocks/DecouplingFramedClockTest.cs | 32 +++++++++++++++++++ osu.Framework/Timing/DecouplingFramedClock.cs | 6 +++- 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/osu.Framework.Tests/Clocks/DecouplingFramedClockTest.cs b/osu.Framework.Tests/Clocks/DecouplingFramedClockTest.cs index 9ccfad14c6..01d780b2ff 100644 --- a/osu.Framework.Tests/Clocks/DecouplingFramedClockTest.cs +++ b/osu.Framework.Tests/Clocks/DecouplingFramedClockTest.cs @@ -2,6 +2,7 @@ // See the LICENCE file in the repository root for full licence text. using System; +using System.Threading; using NUnit.Framework; using osu.Framework.Timing; @@ -270,6 +271,37 @@ public void TestSeekFromSourceWhileDecoupling() Assert.That(decouplingClock.CurrentTime, Is.EqualTo(1000)); } + [Test] + public void TestStartFromNegativeTimeIncrementsCorrectly() + { + // Intentionally wait some time to allow the reference clock to + // build up some elapsed difference. + // + // We want to make sure that this isn't all applied at once causing a large jump. + Thread.Sleep(500); + + decouplingClock.AllowDecoupling = true; + + decouplingClock.Seek(-300); + decouplingClock.Start(); + + decouplingClock.ProcessFrame(); + + Assert.That(source.IsRunning, Is.False); + Assert.That(source.CurrentTime, Is.EqualTo(0)); + + double time = decouplingClock.CurrentTime; + + Assert.That(decouplingClock.IsRunning, Is.True); + Assert.That(decouplingClock.CurrentTime, Is.LessThan(0)); + + Thread.Sleep(100); + + decouplingClock.ProcessFrame(); + Assert.That(decouplingClock.CurrentTime, Is.LessThan(0)); + Assert.That(decouplingClock.CurrentTime, Is.GreaterThan(time)); + } + // TODO: test playback is always forward over the 0ms boundary. // TODO: test backwards playback. (over the boundary?) diff --git a/osu.Framework/Timing/DecouplingFramedClock.cs b/osu.Framework/Timing/DecouplingFramedClock.cs index 0539dc6b9e..60dde20760 100644 --- a/osu.Framework/Timing/DecouplingFramedClock.cs +++ b/osu.Framework/Timing/DecouplingFramedClock.cs @@ -117,7 +117,11 @@ public void ProcessFrame() if (!isRunning) return; - double elapsedReferenceTime = (realtimeReferenceClock.CurrentTime - (lastReferenceTime ?? 0)) * Rate; + // We can only begin tracking time from the second frame, as we need an elapsed real time reference. + if (lastReferenceTime == null) + return; + + double elapsedReferenceTime = (realtimeReferenceClock.CurrentTime - lastReferenceTime.Value) * Rate; currentTime += elapsedReferenceTime; From 4bebf98e57e2830d22cd4d6f3e6ddd4bd1cf1e6a Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sat, 23 Sep 2023 00:53:43 +0900 Subject: [PATCH 32/43] Remove note about test coverage (added in 6651bae1) --- osu.Framework/Timing/DecouplingFramedClock.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/osu.Framework/Timing/DecouplingFramedClock.cs b/osu.Framework/Timing/DecouplingFramedClock.cs index 60dde20760..55888fbeda 100644 --- a/osu.Framework/Timing/DecouplingFramedClock.cs +++ b/osu.Framework/Timing/DecouplingFramedClock.cs @@ -177,7 +177,6 @@ public void Start() // If the previous seek failed, avoid calling `Start` on the source clock. // Doing so would potentially cause it to start from an incorrect location (ie. 0 in the case where we are tracking negative time). - // TODO: add test coverage if (lastSeekFailed && AllowDecoupling) { isRunning = true; From de5683ce3bb4b375cb462ec00c09d421b4991696 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sat, 23 Sep 2023 00:56:00 +0900 Subject: [PATCH 33/43] Remove outdated remark on frameless implementation and mention change source behaviour --- osu.Framework/Timing/DecouplingFramedClock.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/osu.Framework/Timing/DecouplingFramedClock.cs b/osu.Framework/Timing/DecouplingFramedClock.cs index 55888fbeda..05ad9c28b1 100644 --- a/osu.Framework/Timing/DecouplingFramedClock.cs +++ b/osu.Framework/Timing/DecouplingFramedClock.cs @@ -16,9 +16,8 @@ namespace osu.Framework.Timing /// beyond the end of the track even when not explicitly seeked. /// /// - /// This clock is intentionally *not* an to keep things simple. - /// It is important therefore that if the source clock is set to a framed clock, you will need to call - /// externally. + /// Changing the source clock via will always take on the new source's running + /// state and current time, regardless of decoupled state. /// public sealed class DecouplingFramedClock : ISourceChangeableClock, IAdjustableClock, IFrameBasedClock { From 57aa7620a1a48fbe31da07b4e139524e4e9923dd Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sat, 23 Sep 2023 00:59:29 +0900 Subject: [PATCH 34/43] Add xmldoc notes on seek and zero handling --- osu.Framework/Timing/DecouplingFramedClock.cs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/osu.Framework/Timing/DecouplingFramedClock.cs b/osu.Framework/Timing/DecouplingFramedClock.cs index 05ad9c28b1..4197c2ce45 100644 --- a/osu.Framework/Timing/DecouplingFramedClock.cs +++ b/osu.Framework/Timing/DecouplingFramedClock.cs @@ -16,8 +16,13 @@ namespace osu.Framework.Timing /// beyond the end of the track even when not explicitly seeked. /// /// - /// Changing the source clock via will always take on the new source's running - /// state and current time, regardless of decoupled state. + /// There are a few things to note about this implementation: + /// - Changing the source clock via will always take on the new source's running + /// state and current time, regardless of decoupled state. + /// - It is always assumed that after a on the source, it will be able to track time. + /// - It is assumed that a source is generally able to start tracking from zero. Special handling ensures + /// that when arriving at zero from negative time, the source will attempt to be started once so it can + /// take over. /// public sealed class DecouplingFramedClock : ISourceChangeableClock, IAdjustableClock, IFrameBasedClock { From b68dcecda87e305ab71c8b0f56467781dd6020b4 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sat, 23 Sep 2023 01:02:53 +0900 Subject: [PATCH 35/43] Add test coverage of source clock stopping when entering decoupled --- .../Clocks/DecouplingFramedClockTest.cs | 18 ++++++++++++++---- osu.Framework/Timing/DecouplingFramedClock.cs | 1 - 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/osu.Framework.Tests/Clocks/DecouplingFramedClockTest.cs b/osu.Framework.Tests/Clocks/DecouplingFramedClockTest.cs index 01d780b2ff..8c936c342d 100644 --- a/osu.Framework.Tests/Clocks/DecouplingFramedClockTest.cs +++ b/osu.Framework.Tests/Clocks/DecouplingFramedClockTest.cs @@ -230,6 +230,7 @@ public void TestSeekNegativeWhileDecoupling() decouplingClock.ProcessFrame(); Assert.That(source.CurrentTime, Is.EqualTo(0)); + // We're decoupling, so should be able to go beyond zero. Assert.That(decouplingClock.CurrentTime, Is.EqualTo(-1000)); } @@ -271,8 +272,9 @@ public void TestSeekFromSourceWhileDecoupling() Assert.That(decouplingClock.CurrentTime, Is.EqualTo(1000)); } - [Test] - public void TestStartFromNegativeTimeIncrementsCorrectly() + [TestCase(false)] + [TestCase(true)] + public void TestStartFromNegativeTimeIncrementsCorrectly(bool seekBeforeStart) { // Intentionally wait some time to allow the reference clock to // build up some elapsed difference. @@ -282,8 +284,16 @@ public void TestStartFromNegativeTimeIncrementsCorrectly() decouplingClock.AllowDecoupling = true; - decouplingClock.Seek(-300); - decouplingClock.Start(); + if (seekBeforeStart) + { + decouplingClock.Seek(-300); + decouplingClock.Start(); + } + else + { + decouplingClock.Start(); + decouplingClock.Seek(-300); + } decouplingClock.ProcessFrame(); diff --git a/osu.Framework/Timing/DecouplingFramedClock.cs b/osu.Framework/Timing/DecouplingFramedClock.cs index 4197c2ce45..898866642c 100644 --- a/osu.Framework/Timing/DecouplingFramedClock.cs +++ b/osu.Framework/Timing/DecouplingFramedClock.cs @@ -214,7 +214,6 @@ public bool Seek(double position) return false; // Ensure the underlying clock is stopped as we enter decoupled mode. - // TODO: test coverage adjustableSourceClock.Stop(); } From 597c3c728390a6a97075fb8fead8a987f5843482 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sat, 23 Sep 2023 01:32:27 +0900 Subject: [PATCH 36/43] Improve and split out range limited stopwatch clock --- .../TestStopwatchClockWithRangeLimit.cs | 51 +++++++++++++++++++ .../Visual/Clocks/TestSceneClocks.cs | 31 +---------- osu.Framework/Timing/StopwatchClock.cs | 2 +- 3 files changed, 54 insertions(+), 30 deletions(-) create mode 100644 osu.Framework.Tests/Clocks/TestStopwatchClockWithRangeLimit.cs diff --git a/osu.Framework.Tests/Clocks/TestStopwatchClockWithRangeLimit.cs b/osu.Framework.Tests/Clocks/TestStopwatchClockWithRangeLimit.cs new file mode 100644 index 0000000000..917b634cc3 --- /dev/null +++ b/osu.Framework.Tests/Clocks/TestStopwatchClockWithRangeLimit.cs @@ -0,0 +1,51 @@ +// 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 osu.Framework.Timing; + +namespace osu.Framework.Tests.Clocks +{ + public class TestStopwatchClockWithRangeLimit : StopwatchClock + { + public double MinTime => 0; + public double MaxTime { get; set; } = double.PositiveInfinity; + + public TestStopwatchClockWithRangeLimit() + : base(true) + { + } + + public override double CurrentTime + { + get + { + double currentTime = base.CurrentTime; + double clamped = Math.Clamp(currentTime, MinTime, MaxTime); + + if (clamped == currentTime) return clamped; + + if ((Rate > 0 && clamped == MaxTime) || (Rate < 0 && clamped == MinTime)) + Stop(); + + return clamped; + } + } + + public override bool Seek(double position) + { + double clamped = Math.Clamp(position, MinTime, MaxTime); + + if (clamped != position) + { + // Emulate what a bass track will do in this situation. + if (position >= MaxTime) + Stop(); + Seek(clamped); + return false; + } + + return base.Seek(position); + } + } +} diff --git a/osu.Framework.Tests/Visual/Clocks/TestSceneClocks.cs b/osu.Framework.Tests/Visual/Clocks/TestSceneClocks.cs index 3a359ecfca..32735e014d 100644 --- a/osu.Framework.Tests/Visual/Clocks/TestSceneClocks.cs +++ b/osu.Framework.Tests/Visual/Clocks/TestSceneClocks.cs @@ -1,9 +1,9 @@ // 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 NUnit.Framework; using osu.Framework.Testing; +using osu.Framework.Tests.Clocks; using osu.Framework.Timing; namespace osu.Framework.Tests.Visual.Clocks @@ -21,7 +21,7 @@ public void TestInterpolationWithLaggyClock() [Test] public void TestDecouplingWithRangeLimited() { - AddStep("add non-negative stopwatch", () => AddClock(new TestClockWithRangeLimit())); + AddStep("add non-negative stopwatch", () => AddClock(new TestStopwatchClockWithRangeLimit())); AddStep("add decoupling", () => AddClock(new DecouplingFramedClock { AllowDecoupling = true })); AddStep("seek decoupling to -10000", () => @@ -36,32 +36,5 @@ public void TestDecouplingWithRangeLimited() (c.TrackingClock as DecouplingFramedClock)?.Seek(10000); }); } - - internal class TestClockWithRangeLimit : StopwatchClock - { - public double MinTime => 0; - public double MaxTime { get; set; } = double.PositiveInfinity; - - public TestClockWithRangeLimit() - : base(true) - { - } - - public override bool Seek(double position) - { - double clamped = Math.Clamp(position, MinTime, MaxTime); - - if (clamped != position) - { - // Emulate what bass will probably do in this case. - // TODO: confirm. - Stop(); - Seek(clamped); - return false; - } - - return base.Seek(position); - } - } } } diff --git a/osu.Framework/Timing/StopwatchClock.cs b/osu.Framework/Timing/StopwatchClock.cs index be4f70afc6..cb82eb56ed 100644 --- a/osu.Framework/Timing/StopwatchClock.cs +++ b/osu.Framework/Timing/StopwatchClock.cs @@ -27,7 +27,7 @@ public StopwatchClock(bool start = false) Start(); } - public double CurrentTime => stopwatchCurrentTime + seekOffset; + public virtual double CurrentTime => stopwatchCurrentTime + seekOffset; /// /// The current time, represented solely by the accumulated time. From c7a92d2b9d9743441f29e7412992fad970cb7bd0 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sat, 23 Sep 2023 01:19:23 +0900 Subject: [PATCH 37/43] Add basic testing of zero boundary playback --- .../Clocks/DecouplingFramedClockTest.cs | 62 ++++++++++++++++++- 1 file changed, 59 insertions(+), 3 deletions(-) diff --git a/osu.Framework.Tests/Clocks/DecouplingFramedClockTest.cs b/osu.Framework.Tests/Clocks/DecouplingFramedClockTest.cs index 8c936c342d..a77817f0ce 100644 --- a/osu.Framework.Tests/Clocks/DecouplingFramedClockTest.cs +++ b/osu.Framework.Tests/Clocks/DecouplingFramedClockTest.cs @@ -11,7 +11,7 @@ namespace osu.Framework.Tests.Clocks [TestFixture] public class DecouplingFramedClockTest { - private TestClockWithRange source = null!; + private IAdjustableClock source = null!; private DecouplingFramedClock decouplingClock = null!; [SetUp] @@ -312,9 +312,65 @@ public void TestStartFromNegativeTimeIncrementsCorrectly(bool seekBeforeStart) Assert.That(decouplingClock.CurrentTime, Is.GreaterThan(time)); } - // TODO: test playback is always forward over the 0ms boundary. + [Test] + public void TestBackwardPlaybackOverZeroBoundary() + { + source = new TestStopwatchClockWithRangeLimit(); + decouplingClock.ChangeSource(source); + decouplingClock.AllowDecoupling = true; + + decouplingClock.Seek(300); + decouplingClock.Rate = -1; + decouplingClock.Start(); + + decouplingClock.ProcessFrame(); - // TODO: test backwards playback. (over the boundary?) + while (source.IsRunning) + { + decouplingClock.ProcessFrame(); + Assert.That(decouplingClock.CurrentTime, Is.EqualTo(source.CurrentTime).Within(5)); + } + + Assert.That(source.IsRunning, Is.False); + + double time = decouplingClock.CurrentTime; + + while (decouplingClock.CurrentTime > -300) + { + Assert.That(source.IsRunning, Is.False); + Assert.That(decouplingClock.CurrentTime, Is.LessThanOrEqualTo(time)); + time = decouplingClock.CurrentTime; + + decouplingClock.ProcessFrame(); + } + } + + [Test] + public void TestForwardPlaybackOverZeroBoundary() + { + source = new TestStopwatchClockWithRangeLimit(); + decouplingClock.ChangeSource(source); + decouplingClock.AllowDecoupling = true; + + decouplingClock.Seek(-300); + decouplingClock.Start(); + + decouplingClock.ProcessFrame(); + + double time = decouplingClock.CurrentTime; + + while (decouplingClock.CurrentTime < 0) + { + Assert.That(source.IsRunning, Is.False); + Assert.That(decouplingClock.CurrentTime, Is.GreaterThanOrEqualTo(time)); + time = decouplingClock.CurrentTime; + + decouplingClock.ProcessFrame(); + } + + Assert.That(source.CurrentTime, Is.EqualTo(decouplingClock.CurrentTime).Within(5)); + Assert.That(source.IsRunning, Is.True); + } #endregion From 1350e56794fa7e9973f1790fa6ec5f1f3f5af1b7 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 25 Sep 2023 22:29:14 +0900 Subject: [PATCH 38/43] Change `IsRunning` to only update state on `ProcessFrame` call --- .../Clocks/DecouplingFramedClockTest.cs | 8 +++++++ osu.Framework/Timing/DecouplingFramedClock.cs | 21 ++++--------------- 2 files changed, 12 insertions(+), 17 deletions(-) diff --git a/osu.Framework.Tests/Clocks/DecouplingFramedClockTest.cs b/osu.Framework.Tests/Clocks/DecouplingFramedClockTest.cs index a77817f0ce..9025b1cd24 100644 --- a/osu.Framework.Tests/Clocks/DecouplingFramedClockTest.cs +++ b/osu.Framework.Tests/Clocks/DecouplingFramedClockTest.cs @@ -35,6 +35,7 @@ public void TestStartFromDecoupling(bool allowDecoupling) Assert.That(decouplingClock.IsRunning, Is.False); decouplingClock.Start(); + decouplingClock.ProcessFrame(); Assert.That(source.IsRunning, Is.True); Assert.That(decouplingClock.IsRunning, Is.True); @@ -50,6 +51,7 @@ public void TestStartFromSource(bool allowDecoupling) Assert.That(decouplingClock.IsRunning, Is.False); source.Start(); + decouplingClock.ProcessFrame(); Assert.That(source.IsRunning, Is.True); Assert.That(decouplingClock.IsRunning, Is.True); @@ -126,14 +128,17 @@ public void TestChangeSourceUpdatesToCorrectSourceState(bool allowDecoupling) decouplingClock.AllowDecoupling = allowDecoupling; source.Start(); + decouplingClock.ProcessFrame(); Assert.That(decouplingClock.IsRunning, Is.True); var secondSource = new TestClock(); decouplingClock.ChangeSource(secondSource); + decouplingClock.ProcessFrame(); Assert.That(decouplingClock.IsRunning, Is.False); decouplingClock.ChangeSource(source); + decouplingClock.ProcessFrame(); Assert.That(decouplingClock.IsRunning, Is.True); } @@ -169,11 +174,13 @@ public void TestSourceStoppedWhileNotDecoupling() { decouplingClock.AllowDecoupling = false; decouplingClock.Start(); + decouplingClock.ProcessFrame(); Assert.That(source.IsRunning, Is.True); Assert.That(decouplingClock.IsRunning, Is.True); source.Stop(); + decouplingClock.ProcessFrame(); Assert.That(source.IsRunning, Is.False); Assert.That(decouplingClock.IsRunning, Is.False); @@ -210,6 +217,7 @@ public void TestSourceStoppedWhileDecoupling() { decouplingClock.AllowDecoupling = true; decouplingClock.Start(); + decouplingClock.ProcessFrame(); Assert.That(source.IsRunning, Is.True); Assert.That(decouplingClock.IsRunning, Is.True); diff --git a/osu.Framework/Timing/DecouplingFramedClock.cs b/osu.Framework/Timing/DecouplingFramedClock.cs index 898866642c..5fc61a4153 100644 --- a/osu.Framework/Timing/DecouplingFramedClock.cs +++ b/osu.Framework/Timing/DecouplingFramedClock.cs @@ -34,23 +34,7 @@ public sealed class DecouplingFramedClock : ISourceChangeableClock, IAdjustableC /// public bool AllowDecoupling { get; set; } = true; - public bool IsRunning - { - get - { - // Always use the source clock's running state if it's running. - if (Source.IsRunning) - return isRunning = true; - - // If we're allowed to be decoupled, maintain our internal running state. - if (AllowDecoupling) - return isRunning; - - // Otherwise we're definitely not running. - Debug.Assert(!Source.IsRunning); - return isRunning = false; - } - } + public bool IsRunning { get; private set; } public double CurrentTime { get; private set; } @@ -107,6 +91,7 @@ public void ProcessFrame() if (Source.IsRunning) { currentTime = Source.CurrentTime; + isRunning = true; return; } @@ -114,6 +99,7 @@ public void ProcessFrame() if (!AllowDecoupling) { currentTime = Source.CurrentTime; + isRunning = false; return; } @@ -141,6 +127,7 @@ public void ProcessFrame() } finally { + IsRunning = isRunning; lastReferenceTime = realtimeReferenceClock.CurrentTime; CurrentTime = currentTime; ElapsedFrameTime = CurrentTime - lastTime; From 28d159eed38d835c577c85594d08df5875308408 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 25 Sep 2023 22:34:46 +0900 Subject: [PATCH 39/43] Add test coverage of playback over end boundary --- .../Clocks/DecouplingFramedClockTest.cs | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/osu.Framework.Tests/Clocks/DecouplingFramedClockTest.cs b/osu.Framework.Tests/Clocks/DecouplingFramedClockTest.cs index 9025b1cd24..eb262fe92a 100644 --- a/osu.Framework.Tests/Clocks/DecouplingFramedClockTest.cs +++ b/osu.Framework.Tests/Clocks/DecouplingFramedClockTest.cs @@ -380,6 +380,48 @@ public void TestForwardPlaybackOverZeroBoundary() Assert.That(source.IsRunning, Is.True); } + [Test] + public void TestForwardPlaybackOverLengthBoundary() + { + source = new TestStopwatchClockWithRangeLimit + { + MaxTime = 10000 + }; + + decouplingClock.ChangeSource(source); + decouplingClock.AllowDecoupling = true; + + decouplingClock.Seek(9800); + decouplingClock.Start(); + + decouplingClock.ProcessFrame(); + + double time = decouplingClock.CurrentTime; + + while (decouplingClock.CurrentTime < 10000) + { + Assert.That(source.IsRunning, Is.True); + Assert.That(source.CurrentTime, Is.EqualTo(decouplingClock.CurrentTime).Within(5)); + Assert.That(decouplingClock.CurrentTime, Is.GreaterThanOrEqualTo(time)); + time = decouplingClock.CurrentTime; + + decouplingClock.ProcessFrame(); + } + + Assert.That(source.IsRunning, Is.False); + + while (decouplingClock.CurrentTime < 10200) + { + Assert.That(decouplingClock.IsRunning, Is.True); + Assert.That(decouplingClock.CurrentTime, Is.GreaterThanOrEqualTo(time)); + time = decouplingClock.CurrentTime; + + decouplingClock.ProcessFrame(); + } + + Assert.That(source.IsRunning, Is.False); + } + #endregion private class TestClockWithRange : TestClock From 98f832f7a5fe2a7b596dd9a47e0a8b91706440a3 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 25 Sep 2023 22:36:45 +0900 Subject: [PATCH 40/43] Add test coverage of paused seek beyond source length --- .../Clocks/DecouplingFramedClockTest.cs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/osu.Framework.Tests/Clocks/DecouplingFramedClockTest.cs b/osu.Framework.Tests/Clocks/DecouplingFramedClockTest.cs index eb262fe92a..1c992df443 100644 --- a/osu.Framework.Tests/Clocks/DecouplingFramedClockTest.cs +++ b/osu.Framework.Tests/Clocks/DecouplingFramedClockTest.cs @@ -254,6 +254,24 @@ public void TestSeekPositiveWhileDecoupling() Assert.That(decouplingClock.CurrentTime, Is.EqualTo(1000)); } + [Test] + public void TestSeekBeyondLengthWhileDecoupling() + { + source = new TestStopwatchClockWithRangeLimit + { + MaxTime = 500 + }; + + decouplingClock.ChangeSource(source); + decouplingClock.AllowDecoupling = true; + + Assert.That(decouplingClock.Seek(1000), Is.True); + decouplingClock.ProcessFrame(); + + Assert.That(source.CurrentTime, Is.EqualTo(500)); + Assert.That(decouplingClock.CurrentTime, Is.EqualTo(1000)); + } + /// /// In decoupled operation, seeking the source while it's not playing is undefined /// behaviour. From da86faad92d17a5e41b0c35fddaa56035cc6f62d Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 25 Sep 2023 23:52:44 +0900 Subject: [PATCH 41/43] Add more correct test coverage of `ChangeSource` post-time and fix actual failure --- osu.Framework.Tests/Clocks/DecouplingFramedClockTest.cs | 1 + osu.Framework/Timing/DecouplingFramedClock.cs | 1 + 2 files changed, 2 insertions(+) diff --git a/osu.Framework.Tests/Clocks/DecouplingFramedClockTest.cs b/osu.Framework.Tests/Clocks/DecouplingFramedClockTest.cs index 1c992df443..33f24714f8 100644 --- a/osu.Framework.Tests/Clocks/DecouplingFramedClockTest.cs +++ b/osu.Framework.Tests/Clocks/DecouplingFramedClockTest.cs @@ -119,6 +119,7 @@ public void TestChangeSourceUpdatesToNewSourceTime(bool allowDecoupling) decouplingClock.ProcessFrame(); Assert.That(secondSource.CurrentTime, Is.EqualTo(second_source_time)); + Assert.That(decouplingClock.CurrentTime, Is.EqualTo(second_source_time)); } [TestCase(true)] diff --git a/osu.Framework/Timing/DecouplingFramedClock.cs b/osu.Framework/Timing/DecouplingFramedClock.cs index 5fc61a4153..0d179459d4 100644 --- a/osu.Framework/Timing/DecouplingFramedClock.cs +++ b/osu.Framework/Timing/DecouplingFramedClock.cs @@ -146,6 +146,7 @@ public void ChangeSource(IClock? source) throw new ArgumentException($"Clock must be of type {nameof(IAdjustableClock)}"); adjustableSourceClock = adjustableSource; + currentTime = adjustableSource.CurrentTime; isRunning = adjustableSource.IsRunning; } From 60ce8c7a1c5a8bfd42cdb5ce3584d7b363fb06a7 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 26 Sep 2023 13:44:08 +0900 Subject: [PATCH 42/43] Fix intermittent test failure due to `<10000` condition not guaranteeing source stopped --- osu.Framework.Tests/Clocks/DecouplingFramedClockTest.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/osu.Framework.Tests/Clocks/DecouplingFramedClockTest.cs b/osu.Framework.Tests/Clocks/DecouplingFramedClockTest.cs index 33f24714f8..5eb2113776 100644 --- a/osu.Framework.Tests/Clocks/DecouplingFramedClockTest.cs +++ b/osu.Framework.Tests/Clocks/DecouplingFramedClockTest.cs @@ -427,7 +427,11 @@ public void TestForwardPlaybackOverLengthBoundary() decouplingClock.ProcessFrame(); } + while (source.IsRunning) + decouplingClock.ProcessFrame(); + Assert.That(source.IsRunning, Is.False); + Assert.That(decouplingClock.CurrentTime, Is.LessThan(10100)); while (decouplingClock.CurrentTime < 10200) { From 83a505c51892ad241bd0629deed963244c975408 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 26 Sep 2023 17:08:10 +0200 Subject: [PATCH 43/43] Add explanatory comments for not handling right-side boundary cross when decoupled --- osu.Framework/Timing/DecouplingFramedClock.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/osu.Framework/Timing/DecouplingFramedClock.cs b/osu.Framework/Timing/DecouplingFramedClock.cs index 0d179459d4..5ff5555c4c 100644 --- a/osu.Framework/Timing/DecouplingFramedClock.cs +++ b/osu.Framework/Timing/DecouplingFramedClock.cs @@ -23,6 +23,8 @@ namespace osu.Framework.Timing /// - It is assumed that a source is generally able to start tracking from zero. Special handling ensures /// that when arriving at zero from negative time, the source will attempt to be started once so it can /// take over. + /// Note that no such special handling is assured for when the source has a maximum allowable time, + /// since it is not known what that time is. /// public sealed class DecouplingFramedClock : ISourceChangeableClock, IAdjustableClock, IFrameBasedClock { @@ -115,7 +117,10 @@ public void ProcessFrame() currentTime += elapsedReferenceTime; - // When crossing the zero time boundary, we should start and use the source clock. + // When crossing the zero time boundary forwards, we should start and use the source clock. + // Note that this implicitly assumes the source starts at zero, + // and additionally the right-side boundary is not handled as we don't know where the source's max time is. + // This could be potentially handled if need be, if we had a notion of what the source's max allowable time is. if (lastTime < 0 && currentTime >= 0) { adjustableSourceClock.Seek(currentTime);