Merge pull request #11259 from LittleEndu/scroll-to-20

Change vertical alignment of scroll target when clicking settings category buttons

osu.Game.Tests/Visual/UserInterface/TestSceneSectionsContainer.cs

@@ -0,0 +1,122 @@
+// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
+// See the LICENCE file in the repository root for full licence text.
+
+using System.Linq;
+using NUnit.Framework;
+using osu.Framework.Graphics;
+using osu.Framework.Graphics.Colour;
+using osu.Framework.Graphics.Shapes;
+using osu.Game.Graphics.Containers;
+using osuTK.Graphics;
+
+namespace osu.Game.Tests.Visual.UserInterface
+{
+    public class TestSceneSectionsContainer : OsuManualInputManagerTestScene
+    {
+        private readonly SectionsContainer<TestSection> container;
+        private float custom;
+        private const float header_height = 100;
+
+        public TestSceneSectionsContainer()
+        {
+            container = new SectionsContainer<TestSection>
+            {
+                RelativeSizeAxes = Axes.Y,
+                Width = 300,
+                Origin = Anchor.Centre,
+                Anchor = Anchor.Centre,
+                FixedHeader = new Box
+                {
+                    Alpha = 0.5f,
+                    Width = 300,
+                    Height = header_height,
+                    Colour = Color4.Red
+                }
+            };
+            container.SelectedSection.ValueChanged += section =>
+            {
+                if (section.OldValue != null)
+                    section.OldValue.Selected = false;
+                if (section.NewValue != null)
+                    section.NewValue.Selected = true;
+            };
+            Add(container);
+        }
+
+        [Test]
+        public void TestSelection()
+        {
+            AddStep("clear", () => container.Clear());
+            AddStep("add 1/8th", () => append(1 / 8.0f));
+            AddStep("add third", () => append(1 / 3.0f));
+            AddStep("add half", () => append(1 / 2.0f));
+            AddStep("add full", () => append(1));
+            AddSliderStep("set custom", 0.1f, 1.1f, 0.5f, i => custom = i);
+            AddStep("add custom", () => append(custom));
+            AddStep("scroll to previous", () => container.ScrollTo(
+                container.Children.Reverse().SkipWhile(s => s != container.SelectedSection.Value).Skip(1).FirstOrDefault() ?? container.Children.First()
+            ));
+            AddStep("scroll to next", () => container.ScrollTo(
+                container.Children.SkipWhile(s => s != container.SelectedSection.Value).Skip(1).FirstOrDefault() ?? container.Children.Last()
+            ));
+            AddStep("scroll up", () => triggerUserScroll(1));
+            AddStep("scroll down", () => triggerUserScroll(-1));
+        }
+
+        [Test]
+        public void TestCorrectSectionSelected()
+        {
+            const int sections_count = 11;
+            float[] alternating = { 0.07f, 0.33f, 0.16f, 0.33f };
+            AddStep("clear", () => container.Clear());
+            AddStep("fill with sections", () =>
+            {
+                for (int i = 0; i < sections_count; i++)
+                    append(alternating[i % alternating.Length]);
+            });
+
+            void step(int scrollIndex)
+            {
+                AddStep($"scroll to section {scrollIndex + 1}", () => container.ScrollTo(container.Children[scrollIndex]));
+                AddUntilStep("correct section selected", () => container.SelectedSection.Value == container.Children[scrollIndex]);
+            }
+
+            for (int i = 1; i < sections_count; i++)
+                step(i);
+            for (int i = sections_count - 2; i >= 0; i--)
+                step(i);
+
+            AddStep("scroll almost to end", () => container.ScrollTo(container.Children[sections_count - 2]));
+            AddUntilStep("correct section selected", () => container.SelectedSection.Value == container.Children[sections_count - 2]);
+            AddStep("scroll down", () => triggerUserScroll(-1));
+            AddUntilStep("correct section selected", () => container.SelectedSection.Value == container.Children[sections_count - 1]);
+        }
+
+        private static readonly ColourInfo selected_colour = ColourInfo.GradientVertical(Color4.Yellow, Color4.Gold);
+        private static readonly ColourInfo default_colour = ColourInfo.GradientVertical(Color4.White, Color4.DarkGray);
+
+        private void append(float multiplier)
+        {
+            container.Add(new TestSection
+            {
+                Width = 300,
+                Height = (container.ChildSize.Y - header_height) * multiplier,
+                Colour = default_colour
+            });
+        }
+
+        private void triggerUserScroll(float direction)
+        {
+            InputManager.MoveMouseTo(container);
+            InputManager.ScrollVerticalBy(direction);
+        }
+
+        private class TestSection : Box
+        {
+            public bool Selected
+            {
+                set => Colour = value ? selected_colour : default_colour;
+            }
+        }
+    }
+}

osu.Game/Graphics/Containers/SectionsContainer.cs

@@ -2,6 +2,7 @@
 // See the LICENCE file in the repository root for full licence text.
 
 using System;
+using System.Diagnostics;
 using System.Linq;
 using JetBrains.Annotations;
 using osu.Framework.Allocation;
@@ -9,6 +10,7 @@ using osu.Framework.Bindables;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.Containers;
 using osu.Framework.Layout;
+using osu.Framework.Utils;
 
 namespace osu.Game.Graphics.Containers
 {
@@ -20,6 +22,7 @@ namespace osu.Game.Graphics.Containers
         where T : Drawable
     {
         public Bindable<T> SelectedSection { get; } = new Bindable<T>();
+        private Drawable lastClickedSection;
 
         public Drawable ExpandableHeader
         {
@@ -36,7 +39,7 @@ namespace osu.Game.Graphics.Containers
                 if (value == null) return;
 
                 AddInternal(expandableHeader);
-                lastKnownScroll = float.NaN;
+                lastKnownScroll = null;
             }
         }
 
@@ -52,7 +55,7 @@ namespace osu.Game.Graphics.Containers
                 if (value == null) return;
 
                 AddInternal(fixedHeader);
-                lastKnownScroll = float.NaN;
+                lastKnownScroll = null;
             }
         }
 
@@ -71,7 +74,7 @@ namespace osu.Game.Graphics.Containers
                 footer.Anchor |= Anchor.y2;
                 footer.Origin |= Anchor.y2;
                 scrollContainer.Add(footer);
-                lastKnownScroll = float.NaN;
+                lastKnownScroll = null;
             }
         }
 
@@ -89,21 +92,26 @@ namespace osu.Game.Graphics.Containers
 
                 headerBackgroundContainer.Add(headerBackground);
 
-                lastKnownScroll = float.NaN;
+                lastKnownScroll = null;
             }
         }
 
         protected override Container<T> Content => scrollContentContainer;
 
-        private readonly OsuScrollContainer scrollContainer;
+        private readonly UserTrackingScrollContainer scrollContainer;
         private readonly Container headerBackgroundContainer;
         private readonly MarginPadding originalSectionsMargin;
         private Drawable expandableHeader, fixedHeader, footer, headerBackground;
         private FlowContainer<T> scrollContentContainer;
 
-        private float headerHeight, footerHeight;
+        private float? headerHeight, footerHeight;
 
-        private float lastKnownScroll;
+        private float? lastKnownScroll;
+
+        /// <summary>
+        /// The percentage of the container to consider the centre-point for deciding the active section (and scrolling to a requested section).
+        /// </summary>
+        private const float scroll_y_centre = 0.1f;
 
         public SectionsContainer()
         {
@@ -128,18 +136,24 @@ namespace osu.Game.Graphics.Containers
         public override void Add(T drawable)
         {
             base.Add(drawable);
-            lastKnownScroll = float.NaN;
-            headerHeight = float.NaN;
-            footerHeight = float.NaN;
+
+            Debug.Assert(drawable != null);
+
+            lastKnownScroll = null;
+            headerHeight = null;
+            footerHeight = null;
         }
 
-        public void ScrollTo(Drawable section) =>
-            scrollContainer.ScrollTo(scrollContainer.GetChildPosInContent(section) - (FixedHeader?.BoundingBox.Height ?? 0));
+        public void ScrollTo(Drawable section)
+        {
+            lastClickedSection = section;
+            scrollContainer.ScrollTo(scrollContainer.GetChildPosInContent(section) - scrollContainer.DisplayableContent * scroll_y_centre - (FixedHeader?.BoundingBox.Height ?? 0));
+        }
 
         public void ScrollToTop() => scrollContainer.ScrollTo(0);
 
         [NotNull]
-        protected virtual OsuScrollContainer CreateScrollContainer() => new OsuScrollContainer();
+        protected virtual UserTrackingScrollContainer CreateScrollContainer() => new UserTrackingScrollContainer();
 
         [NotNull]
         protected virtual FlowContainer<T> CreateScrollContentContainer() =>
@@ -156,7 +170,7 @@ namespace osu.Game.Graphics.Containers
 
             if (source == InvalidationSource.Child && (invalidation & Invalidation.DrawSize) != 0)
             {
-                lastKnownScroll = -1;
+                lastKnownScroll = null;
                 result = true;
             }
 
@@ -167,7 +181,10 @@ namespace osu.Game.Graphics.Containers
         {
             base.UpdateAfterChildren();
 
-            float headerH = (ExpandableHeader?.LayoutSize.Y ?? 0) + (FixedHeader?.LayoutSize.Y ?? 0);
+            float fixedHeaderSize = FixedHeader?.LayoutSize.Y ?? 0;
+            float expandableHeaderSize = ExpandableHeader?.LayoutSize.Y ?? 0;
+
+            float headerH = expandableHeaderSize + fixedHeaderSize;
             float footerH = Footer?.LayoutSize.Y ?? 0;
 
             if (headerH != headerHeight || footerH != footerHeight)
@@ -183,28 +200,39 @@ namespace osu.Game.Graphics.Containers
             {
                 lastKnownScroll = currentScroll;
 
+                // reset last clicked section because user started scrolling themselves
+                if (scrollContainer.UserScrolling)
+                    lastClickedSection = null;
+
                 if (ExpandableHeader != null && FixedHeader != null)
                 {
-                    float offset = Math.Min(ExpandableHeader.LayoutSize.Y, currentScroll);
+                    float offset = Math.Min(expandableHeaderSize, currentScroll);
 
                     ExpandableHeader.Y = -offset;
-                    FixedHeader.Y = -offset + ExpandableHeader.LayoutSize.Y;
+                    FixedHeader.Y = -offset + expandableHeaderSize;
                 }
 
-                headerBackgroundContainer.Height = (ExpandableHeader?.LayoutSize.Y ?? 0) + (FixedHeader?.LayoutSize.Y ?? 0);
+                headerBackgroundContainer.Height = expandableHeaderSize + fixedHeaderSize;
                 headerBackgroundContainer.Y = ExpandableHeader?.Y ?? 0;
 
-                float scrollOffset = FixedHeader?.LayoutSize.Y ?? 0;
-                Func<T, float> diff = section => scrollContainer.GetChildPosInContent(section) - currentScroll - scrollOffset;
+                var smallestSectionHeight = Children.Count > 0 ? Children.Min(d => d.Height) : 0;
 
-                if (scrollContainer.IsScrolledToEnd())
-                {
-                    SelectedSection.Value = Children.LastOrDefault();
-                }
+                // scroll offset is our fixed header height if we have it plus 10% of content height
+                // plus 5% to fix floating point errors and to not have a section instantly unselect when scrolling upwards
+                // but the 5% can't be bigger than our smallest section height, otherwise it won't get selected correctly
+                float selectionLenienceAboveSection = Math.Min(smallestSectionHeight / 2.0f, scrollContainer.DisplayableContent * 0.05f);
+
+                float scrollCentre = fixedHeaderSize + scrollContainer.DisplayableContent * scroll_y_centre + selectionLenienceAboveSection;
+
+                if (Precision.AlmostBigger(0, scrollContainer.Current))
+                    SelectedSection.Value = lastClickedSection as T ?? Children.FirstOrDefault();
+                else if (Precision.AlmostBigger(scrollContainer.Current, scrollContainer.ScrollableExtent))
+                    SelectedSection.Value = lastClickedSection as T ?? Children.LastOrDefault();
                 else
                 {
-                    SelectedSection.Value = Children.TakeWhile(section => diff(section) <= 0).LastOrDefault()
-                                            ?? Children.FirstOrDefault();
+                    SelectedSection.Value = Children
+                                            .TakeWhile(section => scrollContainer.GetChildPosInContent(section) - currentScroll - scrollCentre <= 0)
+                                            .LastOrDefault() ?? Children.FirstOrDefault();
                 }
             }
         }
@@ -214,8 +242,9 @@ namespace osu.Game.Graphics.Containers
             if (!Children.Any()) return;
 
             var newMargin = originalSectionsMargin;
-            newMargin.Top += headerHeight;
-            newMargin.Bottom += footerHeight;
+
+            newMargin.Top += (headerHeight ?? 0);
+            newMargin.Bottom += (footerHeight ?? 0);
 
             scrollContentContainer.Margin = newMargin;
         }

osu.Game/Graphics/Containers/UserTrackingScrollContainer.cs

@@ -0,0 +1,49 @@
+// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
+// See the LICENCE file in the repository root for full licence text.
+
+using osu.Framework.Graphics;
+
+namespace osu.Game.Graphics.Containers
+{
+    public class UserTrackingScrollContainer : UserTrackingScrollContainer<Drawable>
+    {
+        public UserTrackingScrollContainer()
+        {
+        }
+
+        public UserTrackingScrollContainer(Direction direction)
+            : base(direction)
+        {
+        }
+    }
+
+    public class UserTrackingScrollContainer<T> : OsuScrollContainer<T>
+        where T : Drawable
+    {
+        /// <summary>
+        /// Whether the last scroll event was user triggered, directly on the scroll container.
+        /// </summary>
+        public bool UserScrolling { get; private set; }
+
+        public UserTrackingScrollContainer()
+        {
+        }
+
+        public UserTrackingScrollContainer(Direction direction)
+            : base(direction)
+        {
+        }
+
+        protected override void OnUserScroll(float value, bool animated = true, double? distanceDecay = default)
+        {
+            UserScrolling = true;
+            base.OnUserScroll(value, animated, distanceDecay);
+        }
+
+        public new void ScrollTo(float value, bool animated = true, double? distanceDecay = null)
+        {
+            UserScrolling = false;
+            base.ScrollTo(value, animated, distanceDecay);
+        }
+    }
+}

osu.Game/Overlays/OverlayScrollContainer.cs

@@ -17,9 +17,9 @@ using osuTK.Graphics;
 namespace osu.Game.Overlays
 {
     /// <summary>
-    /// <see cref="OsuScrollContainer"/> which provides <see cref="ScrollToTopButton"/>. Mostly used in <see cref="FullscreenOverlay{T}"/>.
+    /// <see cref="UserTrackingScrollContainer"/> which provides <see cref="ScrollToTopButton"/>. Mostly used in <see cref="FullscreenOverlay{T}"/>.
     /// </summary>
-    public class OverlayScrollContainer : OsuScrollContainer
+    public class OverlayScrollContainer : UserTrackingScrollContainer
     {
         /// <summary>
         /// Scroll position at which the <see cref="ScrollToTopButton"/> will be shown.

osu.Game/Overlays/UserProfileOverlay.cs

@@ -202,7 +202,7 @@ namespace osu.Game.Overlays
                 RelativeSizeAxes = Axes.Both;
             }
 
-            protected override OsuScrollContainer CreateScrollContainer() => new OverlayScrollContainer();
+            protected override UserTrackingScrollContainer CreateScrollContainer() => new OverlayScrollContainer();
 
             protected override FlowContainer<ProfileSection> CreateScrollContentContainer() => new FillFlowContainer<ProfileSection>
             {

osu.Game/Screens/Select/BeatmapCarousel.cs

@@ -918,15 +918,10 @@ namespace osu.Game.Screens.Select
             }
         }
 
-        protected class CarouselScrollContainer : OsuScrollContainer<DrawableCarouselItem>
+        protected class CarouselScrollContainer : UserTrackingScrollContainer<DrawableCarouselItem>
         {
             private bool rightMouseScrollBlocked;
 
-            /// <summary>
-            /// Whether the last scroll event was user triggered, directly on the scroll container.
-            /// </summary>
-            public bool UserScrolling { get; private set; }
-
             public CarouselScrollContainer()
             {
                 // size is determined by the carousel itself, due to not all content necessarily being loaded.
@@ -936,18 +931,6 @@ namespace osu.Game.Screens.Select
                 Masking = false;
             }
 
-            protected override void OnUserScroll(float value, bool animated = true, double? distanceDecay = default)
-            {
-                UserScrolling = true;
-                base.OnUserScroll(value, animated, distanceDecay);
-            }
-
-            public new void ScrollTo(float value, bool animated = true, double? distanceDecay = null)
-            {
-                UserScrolling = false;
-                base.ScrollTo(value, animated, distanceDecay);
-            }
-
             protected override bool OnMouseDown(MouseDownEvent e)
             {
                 if (e.Button == MouseButton.Right)