From 850afcb1c3f9d8dfb35a39ae85a24458e180f1f2 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Wed, 15 Jun 2022 05:43:39 +0300 Subject: [PATCH 1/3] Add failing test case --- .../Editing/TestSceneZoomableScrollContainer.cs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/osu.Game.Tests/Visual/Editing/TestSceneZoomableScrollContainer.cs b/osu.Game.Tests/Visual/Editing/TestSceneZoomableScrollContainer.cs index 2d056bafdd..43e3404d98 100644 --- a/osu.Game.Tests/Visual/Editing/TestSceneZoomableScrollContainer.cs +++ b/osu.Game.Tests/Visual/Editing/TestSceneZoomableScrollContainer.cs @@ -78,6 +78,21 @@ 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() { From 268a7e13343c13001384ac9a700582f9f81464c9 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Wed, 15 Jun 2022 05:19:51 +0300 Subject: [PATCH 2/3] Fix `ZoomableScrollContainer` attempting to update zoom with overlapping range --- .../Components/Timeline/ZoomableScrollContainer.cs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/osu.Game/Screens/Edit/Compose/Components/Timeline/ZoomableScrollContainer.cs b/osu.Game/Screens/Edit/Compose/Components/Timeline/ZoomableScrollContainer.cs index d008368b69..d58cb27274 100644 --- a/osu.Game/Screens/Edit/Compose/Components/Timeline/ZoomableScrollContainer.cs +++ b/osu.Game/Screens/Edit/Compose/Components/Timeline/ZoomableScrollContainer.cs @@ -66,8 +66,9 @@ namespace osu.Game.Screens.Edit.Compose.Components.Timeline minZoom = value; - if (Zoom < value) - Zoom = value; + // ensure zoom range is in valid state before updating zoom. + if (MinZoom < MaxZoom) + Zoom = Zoom; } } @@ -86,8 +87,9 @@ namespace osu.Game.Screens.Edit.Compose.Components.Timeline maxZoom = value; - if (Zoom > value) - Zoom = value; + // ensure zoom range is in valid state before updating zoom. + if (MaxZoom > MinZoom) + Zoom = Zoom; } } From d3feb07bc1b6b861ac9db641226eee5c14344469 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Wed, 15 Jun 2022 07:57:16 +0300 Subject: [PATCH 3/3] Split zoom updating to named method with value as optional --- .../Timeline/ZoomableScrollContainer.cs | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/osu.Game/Screens/Edit/Compose/Components/Timeline/ZoomableScrollContainer.cs b/osu.Game/Screens/Edit/Compose/Components/Timeline/ZoomableScrollContainer.cs index d58cb27274..bc6c66625c 100644 --- a/osu.Game/Screens/Edit/Compose/Components/Timeline/ZoomableScrollContainer.cs +++ b/osu.Game/Screens/Edit/Compose/Components/Timeline/ZoomableScrollContainer.cs @@ -68,7 +68,7 @@ namespace osu.Game.Screens.Edit.Compose.Components.Timeline // ensure zoom range is in valid state before updating zoom. if (MinZoom < MaxZoom) - Zoom = Zoom; + updateZoom(); } } @@ -89,7 +89,7 @@ namespace osu.Game.Screens.Edit.Compose.Components.Timeline // ensure zoom range is in valid state before updating zoom. if (MaxZoom > MinZoom) - Zoom = Zoom; + updateZoom(); } } @@ -99,15 +99,17 @@ namespace osu.Game.Screens.Edit.Compose.Components.Timeline public float Zoom { get => zoomTarget; - set - { - value = Math.Clamp(value, MinZoom, MaxZoom); + set => updateZoom(value); + } - if (IsLoaded) - setZoomTarget(value, ToSpaceOfOtherDrawable(new Vector2(DrawWidth / 2, 0), zoomedContent).X); - else - currentZoom = zoomTarget = value; - } + private void updateZoom(float? value = null) + { + float newZoom = Math.Clamp(value ?? Zoom, MinZoom, MaxZoom); + + if (IsLoaded) + setZoomTarget(newZoom, ToSpaceOfOtherDrawable(new Vector2(DrawWidth / 2, 0), zoomedContent).X); + else + currentZoom = zoomTarget = newZoom; } protected override void Update()