From 93175eaf6efda8c782557c8624b4cc23f2075c66 Mon Sep 17 00:00:00 2001
From: Salman Ahmed <frenzibyte@gmail.com>
Date: Mon, 25 Jul 2022 11:39:23 +0300
Subject: [PATCH 1/6] Re-enable timeline zoom test and remove flaky attribute
---
.../Visual/Editing/TestSceneTimelineZoom.cs | 11 -----------
1 file changed, 11 deletions(-)
diff --git a/osu.Game.Tests/Visual/Editing/TestSceneTimelineZoom.cs b/osu.Game.Tests/Visual/Editing/TestSceneTimelineZoom.cs
index 09d753ba41..11ac102814 100644
--- a/osu.Game.Tests/Visual/Editing/TestSceneTimelineZoom.cs
+++ b/osu.Game.Tests/Visual/Editing/TestSceneTimelineZoom.cs
@@ -8,22 +8,11 @@ using osu.Framework.Graphics;
namespace osu.Game.Tests.Visual.Editing
{
- [Ignore("Timeline initialisation is kinda broken.")] // Initial work to rectify this was done in https://github.com/ppy/osu/pull/19297, but needs more massaging to work.
public class TestSceneTimelineZoom : TimelineTestScene
{
public override Drawable CreateTestComponent() => Empty();
[Test]
- [FlakyTest]
- /*
- * Fail rate around 0.3%
- *
- * TearDown : osu.Framework.Testing.Drawables.Steps.AssertButton+TracedException : range halved
- * --TearDown
- * at osu.Framework.Threading.ScheduledDelegate.RunTaskInternal()
- * at osu.Framework.Threading.Scheduler.Update()
- * at osu.Framework.Graphics.Drawable.UpdateSubTree()
- */
public void TestVisibleRangeUpdatesOnZoomChange()
{
double initialVisibleRange = 0;
From 123930306bc0146f52273f66e26f5c2c5e48f6e2 Mon Sep 17 00:00:00 2001
From: Salman Ahmed <frenzibyte@gmail.com>
Date: Mon, 25 Jul 2022 11:54:10 +0300
Subject: [PATCH 2/6] Refactor `ZoomableScrollContainer` to allow setting up
zoom range and initial zoom after load
---
.../TestSceneZoomableScrollContainer.cs | 17 +---
.../Timeline/ZoomableScrollContainer.cs | 99 ++++++++++---------
2 files changed, 56 insertions(+), 60 deletions(-)
diff --git a/osu.Game.Tests/Visual/Editing/TestSceneZoomableScrollContainer.cs b/osu.Game.Tests/Visual/Editing/TestSceneZoomableScrollContainer.cs
index 9dc403814b..ce418f33f0 100644
--- a/osu.Game.Tests/Visual/Editing/TestSceneZoomableScrollContainer.cs
+++ b/osu.Game.Tests/Visual/Editing/TestSceneZoomableScrollContainer.cs
@@ -46,7 +46,7 @@ namespace osu.Game.Tests.Visual.Editing
RelativeSizeAxes = Axes.Both,
Colour = OsuColour.Gray(30)
},
- scrollContainer = new ZoomableScrollContainer
+ scrollContainer = new ZoomableScrollContainer(1, 60, 1)
{
Anchor = Anchor.Centre,
Origin = Anchor.Centre,
@@ -80,21 +80,6 @@ namespace osu.Game.Tests.Visual.Editing
AddAssert("Inner container width matches scroll container", () => innerBox.DrawWidth == scrollContainer.DrawWidth);
}
- [Test]
- public void TestZoomRangeUpdate()
- {
- AddStep("set zoom to 2", () => scrollContainer.Zoom = 2);
- AddStep("set min zoom to 5", () => scrollContainer.MinZoom = 5);
- AddAssert("zoom = 5", () => scrollContainer.Zoom == 5);
-
- AddStep("set max zoom to 10", () => scrollContainer.MaxZoom = 10);
- AddAssert("zoom = 5", () => scrollContainer.Zoom == 5);
-
- AddStep("set min zoom to 20", () => scrollContainer.MinZoom = 20);
- AddStep("set max zoom to 40", () => scrollContainer.MaxZoom = 40);
- AddAssert("zoom = 20", () => scrollContainer.Zoom == 20);
- }
-
[Test]
public void TestZoom0()
{
diff --git a/osu.Game/Screens/Edit/Compose/Components/Timeline/ZoomableScrollContainer.cs b/osu.Game/Screens/Edit/Compose/Components/Timeline/ZoomableScrollContainer.cs
index 83fd1dea2b..fb2297e88c 100644
--- a/osu.Game/Screens/Edit/Compose/Components/Timeline/ZoomableScrollContainer.cs
+++ b/osu.Game/Screens/Edit/Compose/Components/Timeline/ZoomableScrollContainer.cs
@@ -32,19 +32,27 @@ namespace osu.Game.Screens.Edit.Compose.Components.Timeline
private readonly Container zoomedContent;
protected override Container<Drawable> Content => zoomedContent;
- private float currentZoom = 1;
/// <summary>
- /// The current zoom level of <see cref="ZoomableScrollContainer" />.
- /// It may differ from <see cref="Zoom" /> during transitions.
+ /// The current zoom level of <see cref="ZoomableScrollContainer"/>.
+ /// It may differ from <see cref="Zoom"/> during transitions.
/// </summary>
- public float CurrentZoom => currentZoom;
+ public float CurrentZoom { get; private set; } = 1;
+
+ private bool isZoomSetUp;
[Resolved(canBeNull: true)]
private IFrameBasedClock editorClock { get; set; }
private readonly LayoutValue zoomedContentWidthCache = new LayoutValue(Invalidation.DrawSize);
+ private float minZoom;
+ private float maxZoom;
+
+ /// <summary>
+ /// Creates a <see cref="ZoomableScrollContainer"/> with no zoom range.
+ /// Functionality will be disabled until zoom is set up via <see cref="SetupZoom"/>.
+ /// </summary>
public ZoomableScrollContainer()
: base(Direction.Horizontal)
{
@@ -53,46 +61,36 @@ namespace osu.Game.Screens.Edit.Compose.Components.Timeline
AddLayout(zoomedContentWidthCache);
}
- private float minZoom = 1;
-
/// <summary>
- /// The minimum zoom level allowed.
+ /// Creates a <see cref="ZoomableScrollContainer"/> with a defined zoom range.
/// </summary>
- public float MinZoom
+ public ZoomableScrollContainer(float minimum, float maximum, float initial)
+ : this()
{
- get => minZoom;
- set
- {
- if (value < 1)
- throw new ArgumentException($"{nameof(MinZoom)} must be >= 1.", nameof(value));
-
- minZoom = value;
-
- // ensure zoom range is in valid state before updating zoom.
- if (MinZoom < MaxZoom)
- updateZoom();
- }
+ SetupZoom(initial, minimum, maximum);
}
- private float maxZoom = 60;
-
/// <summary>
- /// The maximum zoom level allowed.
+ /// Sets up the minimum and maximum range of this zoomable scroll container, along with the initial zoom value.
/// </summary>
- public float MaxZoom
+ /// <param name="initial">The initial zoom value, applied immediately.</param>
+ /// <param name="minimum">The minimum zoom value.</param>
+ /// <param name="maximum">The maximum zoom value.</param>
+ public void SetupZoom(float initial, float minimum, float maximum)
{
- get => maxZoom;
- set
- {
- if (value < 1)
- throw new ArgumentException($"{nameof(MaxZoom)} must be >= 1.", nameof(value));
+ if (minimum < 1)
+ throw new ArgumentException($"{nameof(minimum)} ({minimum}) must be >= 1.", nameof(maximum));
- maxZoom = value;
+ if (maximum < 1)
+ throw new ArgumentException($"{nameof(maximum)} ({maximum}) must be >= 1.", nameof(maximum));
- // ensure zoom range is in valid state before updating zoom.
- if (MaxZoom > MinZoom)
- updateZoom();
- }
+ if (minimum > maximum)
+ throw new ArgumentException($"{nameof(minimum)} ({minimum}) must be less than {nameof(maximum)} ({maximum})");
+
+ minZoom = minimum;
+ maxZoom = maximum;
+ CurrentZoom = zoomTarget = initial;
+ isZoomSetUp = true;
}
/// <summary>
@@ -104,14 +102,17 @@ namespace osu.Game.Screens.Edit.Compose.Components.Timeline
set => updateZoom(value);
}
- private void updateZoom(float? value = null)
+ private void updateZoom(float value)
{
- float newZoom = Math.Clamp(value ?? Zoom, MinZoom, MaxZoom);
+ if (!isZoomSetUp)
+ return;
+
+ float newZoom = Math.Clamp(value, minZoom, maxZoom);
if (IsLoaded)
setZoomTarget(newZoom, ToSpaceOfOtherDrawable(new Vector2(DrawWidth / 2, 0), zoomedContent).X);
else
- currentZoom = zoomTarget = newZoom;
+ CurrentZoom = zoomTarget = newZoom;
}
protected override void Update()
@@ -141,22 +142,32 @@ namespace osu.Game.Screens.Edit.Compose.Components.Timeline
private void updateZoomedContentWidth()
{
- zoomedContent.Width = DrawWidth * currentZoom;
+ zoomedContent.Width = DrawWidth * CurrentZoom;
zoomedContentWidthCache.Validate();
}
public void AdjustZoomRelatively(float change, float? focusPoint = null)
{
+ if (!isZoomSetUp)
+ return;
+
const float zoom_change_sensitivity = 0.02f;
- setZoomTarget(zoomTarget + change * (MaxZoom - minZoom) * zoom_change_sensitivity, focusPoint);
+ setZoomTarget(zoomTarget + change * (maxZoom - minZoom) * zoom_change_sensitivity, focusPoint);
+ }
+
+ protected void SetZoomImmediately(float value, float min, float max)
+ {
+ maxZoom = max;
+ minZoom = min;
+ CurrentZoom = zoomTarget = value;
}
private float zoomTarget = 1;
private void setZoomTarget(float newZoom, float? focusPoint = null)
{
- zoomTarget = Math.Clamp(newZoom, MinZoom, MaxZoom);
+ zoomTarget = Math.Clamp(newZoom, minZoom, maxZoom);
focusPoint ??= zoomedContent.ToLocalSpace(ToScreenSpace(new Vector2(DrawWidth / 2, 0))).X;
transformZoomTo(zoomTarget, focusPoint.Value, ZoomDuration, ZoomEasing);
@@ -192,7 +203,7 @@ namespace osu.Game.Screens.Edit.Compose.Components.Timeline
private readonly float scrollOffset;
/// <summary>
- /// Transforms <see cref="ZoomableScrollContainer.currentZoom"/> to a new value.
+ /// Transforms <see cref="ZoomableScrollContainer.CurrentZoom"/> to a new value.
/// </summary>
/// <param name="focusPoint">The focus point in absolute coordinates local to the content.</param>
/// <param name="contentSize">The size of the content.</param>
@@ -204,7 +215,7 @@ namespace osu.Game.Screens.Edit.Compose.Components.Timeline
this.scrollOffset = scrollOffset;
}
- public override string TargetMember => nameof(currentZoom);
+ public override string TargetMember => nameof(CurrentZoom);
private float valueAt(double time)
{
@@ -222,7 +233,7 @@ namespace osu.Game.Screens.Edit.Compose.Components.Timeline
float expectedWidth = d.DrawWidth * newZoom;
float targetOffset = expectedWidth * (focusPoint / contentSize) - focusOffset;
- d.currentZoom = newZoom;
+ d.CurrentZoom = newZoom;
d.updateZoomedContentWidth();
// Temporarily here to make sure ScrollTo gets the correct DrawSize for scrollable area.
@@ -231,7 +242,7 @@ namespace osu.Game.Screens.Edit.Compose.Components.Timeline
d.ScrollTo(targetOffset, false);
}
- protected override void ReadIntoStartValue(ZoomableScrollContainer d) => StartValue = d.currentZoom;
+ protected override void ReadIntoStartValue(ZoomableScrollContainer d) => StartValue = d.CurrentZoom;
}
}
}
From 07c6b4486491848b358f1a2e8c8de56523cc62e3 Mon Sep 17 00:00:00 2001
From: Salman Ahmed <frenzibyte@gmail.com>
Date: Mon, 25 Jul 2022 11:54:47 +0300
Subject: [PATCH 3/6] Fix `Timeline` attempting to setup zoom with unloaded
track
---
.../Compose/Components/Timeline/Timeline.cs | 22 +++++++++++++------
1 file changed, 15 insertions(+), 7 deletions(-)
diff --git a/osu.Game/Screens/Edit/Compose/Components/Timeline/Timeline.cs b/osu.Game/Screens/Edit/Compose/Components/Timeline/Timeline.cs
index bbe011a2e0..54f2d13707 100644
--- a/osu.Game/Screens/Edit/Compose/Components/Timeline/Timeline.cs
+++ b/osu.Game/Screens/Edit/Compose/Components/Timeline/Timeline.cs
@@ -146,13 +146,7 @@ namespace osu.Game.Screens.Edit.Compose.Components.Timeline
waveform.Waveform = b.NewValue.Waveform;
track = b.NewValue.Track;
- // todo: i don't think this is safe, the track may not be loaded yet.
- if (track.Length > 0)
- {
- MaxZoom = getZoomLevelForVisibleMilliseconds(500);
- MinZoom = getZoomLevelForVisibleMilliseconds(10000);
- defaultTimelineZoom = getZoomLevelForVisibleMilliseconds(6000);
- }
+ setupTimelineZoom();
}, true);
Zoom = (float)(defaultTimelineZoom * editorBeatmap.BeatmapInfo.TimelineZoom);
@@ -205,6 +199,20 @@ namespace osu.Game.Screens.Edit.Compose.Components.Timeline
scrollToTrackTime();
}
+ private void setupTimelineZoom()
+ {
+ if (!track.IsLoaded)
+ {
+ Scheduler.AddOnce(setupTimelineZoom);
+ return;
+ }
+
+ defaultTimelineZoom = getZoomLevelForVisibleMilliseconds(6000);
+
+ float initialZoom = (float)(defaultTimelineZoom * editorBeatmap.BeatmapInfo.TimelineZoom);
+ SetupZoom(initialZoom, getZoomLevelForVisibleMilliseconds(10000), getZoomLevelForVisibleMilliseconds(500));
+ }
+
protected override bool OnScroll(ScrollEvent e)
{
// if this is not a precision scroll event, let the editor handle the seek itself (for snapping support)
From bc2b629ee7c89ec71dfbac7d11bd36adfdc4bb8a Mon Sep 17 00:00:00 2001
From: Salman Ahmed <frenzibyte@gmail.com>
Date: Mon, 25 Jul 2022 11:43:34 +0300
Subject: [PATCH 4/6] Let tests wait until track load before testing zoom
---
osu.Game.Tests/Visual/Editing/TestSceneTimelineZoom.cs | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/osu.Game.Tests/Visual/Editing/TestSceneTimelineZoom.cs b/osu.Game.Tests/Visual/Editing/TestSceneTimelineZoom.cs
index 11ac102814..630d048867 100644
--- a/osu.Game.Tests/Visual/Editing/TestSceneTimelineZoom.cs
+++ b/osu.Game.Tests/Visual/Editing/TestSceneTimelineZoom.cs
@@ -17,6 +17,8 @@ namespace osu.Game.Tests.Visual.Editing
{
double initialVisibleRange = 0;
+ AddUntilStep("wait for load", () => MusicController.TrackLoaded);
+
AddStep("reset zoom", () => TimelineArea.Timeline.Zoom = 100);
AddStep("get initial range", () => initialVisibleRange = TimelineArea.Timeline.VisibleRange);
@@ -34,6 +36,8 @@ namespace osu.Game.Tests.Visual.Editing
{
double initialVisibleRange = 0;
+ AddUntilStep("wait for load", () => MusicController.TrackLoaded);
+
AddStep("reset timeline size", () => TimelineArea.Timeline.Width = 1);
AddStep("get initial range", () => initialVisibleRange = TimelineArea.Timeline.VisibleRange);
From 48bcf57066f1b5f67e0849a3cac9a9c8e4feb8bc Mon Sep 17 00:00:00 2001
From: Salman Ahmed <frenzibyte@gmail.com>
Date: Mon, 25 Jul 2022 12:06:54 +0300
Subject: [PATCH 5/6] Mark `SetupZoom` and parameterless
`ZoomableScrollContainer` ctor as protected
---
.../Compose/Components/Timeline/ZoomableScrollContainer.cs | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/osu.Game/Screens/Edit/Compose/Components/Timeline/ZoomableScrollContainer.cs b/osu.Game/Screens/Edit/Compose/Components/Timeline/ZoomableScrollContainer.cs
index fb2297e88c..725f94e7db 100644
--- a/osu.Game/Screens/Edit/Compose/Components/Timeline/ZoomableScrollContainer.cs
+++ b/osu.Game/Screens/Edit/Compose/Components/Timeline/ZoomableScrollContainer.cs
@@ -53,7 +53,7 @@ namespace osu.Game.Screens.Edit.Compose.Components.Timeline
/// Creates a <see cref="ZoomableScrollContainer"/> with no zoom range.
/// Functionality will be disabled until zoom is set up via <see cref="SetupZoom"/>.
/// </summary>
- public ZoomableScrollContainer()
+ protected ZoomableScrollContainer()
: base(Direction.Horizontal)
{
base.Content.Add(zoomedContent = new Container { RelativeSizeAxes = Axes.Y });
@@ -76,7 +76,7 @@ namespace osu.Game.Screens.Edit.Compose.Components.Timeline
/// <param name="initial">The initial zoom value, applied immediately.</param>
/// <param name="minimum">The minimum zoom value.</param>
/// <param name="maximum">The maximum zoom value.</param>
- public void SetupZoom(float initial, float minimum, float maximum)
+ protected void SetupZoom(float initial, float minimum, float maximum)
{
if (minimum < 1)
throw new ArgumentException($"{nameof(minimum)} ({minimum}) must be >= 1.", nameof(maximum));
From 1b2158ff048da3e37c13f908eac533e4cefe2aca Mon Sep 17 00:00:00 2001
From: Dan Balasescu <smoogipoo@smgi.me>
Date: Tue, 26 Jul 2022 14:15:59 +0900
Subject: [PATCH 6/6] Remove unused method
---
.../Compose/Components/Timeline/ZoomableScrollContainer.cs | 7 -------
1 file changed, 7 deletions(-)
diff --git a/osu.Game/Screens/Edit/Compose/Components/Timeline/ZoomableScrollContainer.cs b/osu.Game/Screens/Edit/Compose/Components/Timeline/ZoomableScrollContainer.cs
index 725f94e7db..7d51284f46 100644
--- a/osu.Game/Screens/Edit/Compose/Components/Timeline/ZoomableScrollContainer.cs
+++ b/osu.Game/Screens/Edit/Compose/Components/Timeline/ZoomableScrollContainer.cs
@@ -156,13 +156,6 @@ namespace osu.Game.Screens.Edit.Compose.Components.Timeline
setZoomTarget(zoomTarget + change * (maxZoom - minZoom) * zoom_change_sensitivity, focusPoint);
}
- protected void SetZoomImmediately(float value, float min, float max)
- {
- maxZoom = max;
- minZoom = min;
- CurrentZoom = zoomTarget = value;
- }
-
private float zoomTarget = 1;
private void setZoomTarget(float newZoom, float? focusPoint = null)