Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use double in ScrollContainer for scroll tracking #6467

Merged
merged 7 commits into from
Jan 14, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -525,7 +525,7 @@ private void scrollTo(float position, float scrollContentHeight, float extension
AddStep($"scroll to {position}", () =>
{
scrollContainer.ScrollTo(position, false);
immediateScrollPosition = scrollContainer.Current;
immediateScrollPosition = (float)scrollContainer.Current;
});

AddAssert($"immediately scrolled to {clampedTarget}", () => Precision.AlmostEquals(clampedTarget, immediateScrollPosition, 1));
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
// Copyright (c) ppy Pty Ltd <[email protected]>. Licensed under the MIT Licence.
// See the LICENCE file in the repository root for full licence text.

using System;
using System.Diagnostics;
using System.Linq;
using NUnit.Framework;
using osu.Framework.Graphics;
using osu.Framework.Graphics.Containers;
using osu.Framework.Graphics.Shapes;
using osu.Framework.Testing;
using osu.Framework.Utils;
using osuTK;
using osuTK.Graphics;

namespace osu.Framework.Tests.Visual.Containers
{
public partial class TestSceneScrollContainerDoublePrecision : ManualInputManagerTestScene
{
private const float item_height = 5000;
private const int item_count = 8000;

private ScrollContainer<Drawable> scrollContainer = null!;

[SetUp]
public void Setup() => Schedule(Clear);

[Test]
public void TestStandard()
{
AddStep("Create scroll container", () =>
{
Add(scrollContainer = new BasicScrollContainer
{
Anchor = Anchor.Centre,
Origin = Anchor.Centre,
ScrollbarVisible = true,
RelativeSizeAxes = Axes.Both,
Size = new Vector2(0.7f, 0.9f),
});

for (int i = 0; i < item_count; i++)
{
scrollContainer.Add(new BoxWithDouble
{
Colour = new Color4(RNG.NextSingle(1), RNG.NextSingle(1), RNG.NextSingle(1), 1),
RelativeSizeAxes = Axes.X,
Height = item_height,
Y = i * item_height,
});
}
});

scrollIntoView(item_count - 2);
scrollIntoView(item_count - 1);
}

[Test]
public void TestDoublePrecision()
{
AddStep("Create scroll container", () =>
{
Add(scrollContainer = new DoubleScrollContainer
{
Anchor = Anchor.Centre,
Origin = Anchor.Centre,
ScrollbarVisible = true,
RelativeSizeAxes = Axes.Both,
Size = new Vector2(0.7f, 0.9f),
});

for (int i = 0; i < item_count; i++)
{
scrollContainer.Add(new BoxWithDouble
{
Colour = new Color4(RNG.NextSingle(1), RNG.NextSingle(1), RNG.NextSingle(1), 1),
RelativeSizeAxes = Axes.X,
Height = item_height,
DoubleLocation = i * item_height,
});
}
});

scrollIntoView(item_count - 2);
scrollIntoView(item_count - 1);
}

private void scrollIntoView(int index)
{
AddStep($"scroll {index} into view", () => scrollContainer.ScrollIntoView(scrollContainer.ChildrenOfType<BoxWithDouble>().Skip(index).First()));
AddUntilStep($"{index} is visible", () => !scrollContainer.ChildrenOfType<BoxWithDouble>().Skip(index).First().IsMaskedAway);
}

public partial class DoubleScrollContainer : BasicScrollContainer
{
private readonly Container<BoxWithDouble> layoutContent;

public override void Add(Drawable drawable)
{
if (drawable is not BoxWithDouble boxWithDouble)
throw new InvalidOperationException();

Add(boxWithDouble);
}

public void Add(BoxWithDouble drawable)
{
if (drawable is not BoxWithDouble boxWithDouble)
throw new InvalidOperationException();

layoutContent.Height = (float)Math.Max(layoutContent.Height, boxWithDouble.DoubleLocation + boxWithDouble.DrawHeight);
layoutContent.Add(drawable);
}

public DoubleScrollContainer()
{
// Managing our own custom layout within ScrollContent causes feedback with internal ScrollContainer calculations,
// so we must maintain one level of separation from ScrollContent.
base.Add(layoutContent = new Container<BoxWithDouble>
{
RelativeSizeAxes = Axes.X,
});
}

public override double GetChildPosInContent(Drawable d, Vector2 offset)
{
if (d is not BoxWithDouble boxWithDouble)
return base.GetChildPosInContent(d, offset);

return boxWithDouble.DoubleLocation + offset.X;
}

protected override void ApplyCurrentToContent()
{
Debug.Assert(ScrollDirection == Direction.Vertical);

double scrollableExtent = -Current + ScrollableExtent * ScrollContent.RelativeAnchorPosition.Y;

foreach (var d in layoutContent)
d.Y = (float)(d.DoubleLocation + scrollableExtent);
}
}

public partial class BoxWithDouble : Box
{
public double DoubleLocation { get; set; }
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -467,7 +467,7 @@ private BasicRearrangeableListItem<int>.Button getDragger(int index)

private partial class TestRearrangeableList : BasicRearrangeableListContainer<int>
{
public float ScrollPosition => ScrollContainer.Current;
public float ScrollPosition => (float)ScrollContainer.Current;

public new IReadOnlyDictionary<int, RearrangeableListItem<int>> ItemMap => base.ItemMap;

Expand Down
72 changes: 42 additions & 30 deletions osu.Framework/Graphics/Containers/ScrollContainer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -118,35 +118,35 @@ public bool ScrollbarOverlapsContent
/// <summary>
/// The current scroll position.
/// </summary>
public float Current { get; private set; }
public double Current { get; private set; }

/// <summary>
/// The target scroll position which is exponentially approached by current via a rate of distance decay.
/// </summary>
/// <remarks>
/// When not animating scroll position, this will always be equal to <see cref="Current"/>.
/// </remarks>
public float Target { get; private set; }
public double Target { get; private set; }

/// <summary>
/// The maximum distance that can be scrolled in the scroll direction.
/// </summary>
public float ScrollableExtent => Math.Max(AvailableContent - DisplayableContent, 0);
public double ScrollableExtent => Math.Max(AvailableContent - DisplayableContent, 0);

/// <summary>
/// The maximum distance that the scrollbar can move in the scroll direction.
/// </summary>
/// <remarks>
/// May not be accurate to actual display of scrollbar if <see cref="ToScrollbarPosition"/> or <see cref="FromScrollbarPosition"/> are overridden.
/// </remarks>
protected float ScrollbarMovementExtent => Math.Max(DisplayableContent - Scrollbar.DrawSize[ScrollDim], 0);
protected double ScrollbarMovementExtent => Math.Max(DisplayableContent - Scrollbar.DrawSize[ScrollDim], 0);

/// <summary>
/// Clamp a value to the available scroll range.
/// </summary>
/// <param name="position">The value to clamp.</param>
/// <param name="extension">An extension value beyond the normal extent.</param>
protected float Clamp(float position, float extension = 0) => Math.Max(Math.Min(position, ScrollableExtent + extension), -extension);
protected double Clamp(double position, double extension = 0) => Math.Max(Math.Min(position, ScrollableExtent + extension), -extension);

protected override Container<T> Content => ScrollContent;

Expand Down Expand Up @@ -345,8 +345,8 @@ protected override void OnDrag(DragEvent e)

Vector2 childDelta = ToLocalSpace(e.ScreenSpaceMousePosition) - ToLocalSpace(e.ScreenSpaceLastMousePosition);

float scrollOffset = -childDelta[ScrollDim];
float clampedScrollOffset = Clamp(Target + scrollOffset) - Clamp(Target);
double scrollOffset = -childDelta[ScrollDim];
double clampedScrollOffset = Clamp(Target + scrollOffset) - Clamp(Target);

// If we are dragging past the extent of the scrollable area, half the offset
// such that the user can feel it.
Expand Down Expand Up @@ -424,7 +424,7 @@ public void OffsetScrollPosition(float offset)
Current += offset;
}

private void scrollByOffset(float value, bool animated, double distanceDecay = float.PositiveInfinity) =>
private void scrollByOffset(double value, bool animated, double distanceDecay = float.PositiveInfinity) =>
OnUserScroll(Target + value, animated, distanceDecay);

/// <summary>
Expand Down Expand Up @@ -454,15 +454,15 @@ public void ScrollToEnd(bool animated = true, bool allowDuringDrag = false)
/// </summary>
/// <param name="offset">The amount by which we should scroll.</param>
/// <param name="animated">Whether to animate the movement.</param>
public void ScrollBy(float offset, bool animated = true) => scrollTo(Target + offset, animated);
public void ScrollBy(double offset, bool animated = true) => scrollTo(Target + offset, animated);

/// <summary>
/// Handle a scroll to an absolute position from a user input.
/// </summary>
/// <param name="value">The position to scroll to.</param>
/// <param name="animated">Whether to animate the movement.</param>
/// <param name="distanceDecay">Controls the rate with which the target position is approached after jumping to a specific location. Default is <see cref="DistanceDecayJump"/>.</param>
protected virtual void OnUserScroll(float value, bool animated = true, double? distanceDecay = null) =>
protected virtual void OnUserScroll(double value, bool animated = true, double? distanceDecay = null) =>
ScrollTo(value, animated, distanceDecay);

/// <summary>
Expand All @@ -471,9 +471,9 @@ protected virtual void OnUserScroll(float value, bool animated = true, double? d
/// <param name="value">The position to scroll to.</param>
/// <param name="animated">Whether to animate the movement.</param>
/// <param name="distanceDecay">Controls the rate with which the target position is approached after jumping to a specific location. Default is <see cref="DistanceDecayJump"/>.</param>
public void ScrollTo(float value, bool animated = true, double? distanceDecay = null) => scrollTo(value, animated, distanceDecay ?? DistanceDecayJump);
public void ScrollTo(double value, bool animated = true, double? distanceDecay = null) => scrollTo(value, animated, distanceDecay ?? DistanceDecayJump);

private void scrollTo(float value, bool animated, double distanceDecay = float.PositiveInfinity)
private void scrollTo(double value, bool animated, double distanceDecay = double.PositiveInfinity)
{
Target = Clamp(value, ClampExtension);

Expand All @@ -497,11 +497,11 @@ private void scrollTo(float value, bool animated, double distanceDecay = float.P
/// <param name="animated">Whether to animate the movement.</param>
public void ScrollIntoView(Drawable d, bool animated = true)
{
float childPos0 = GetChildPosInContent(d);
float childPos1 = GetChildPosInContent(d, d.DrawSize);
double childPos0 = GetChildPosInContent(d);
double childPos1 = GetChildPosInContent(d, d.DrawSize);

float minPos = Math.Min(childPos0, childPos1);
float maxPos = Math.Max(childPos0, childPos1);
double minPos = Math.Min(childPos0, childPos1);
double maxPos = Math.Max(childPos0, childPos1);

if (minPos < Current || (minPos > Current && d.DrawSize[ScrollDim] > DisplayableContent))
ScrollTo(minPos, animated);
Expand All @@ -515,14 +515,14 @@ public void ScrollIntoView(Drawable d, bool animated = true)
/// <param name="d">The child to get the position from.</param>
/// <param name="offset">Positional offset in the child's space.</param>
/// <returns>The position of the child.</returns>
public float GetChildPosInContent(Drawable d, Vector2 offset) => d.ToSpaceOfOtherDrawable(offset, ScrollContent)[ScrollDim];
public virtual double GetChildPosInContent(Drawable d, Vector2 offset) => d.ToSpaceOfOtherDrawable(offset, ScrollContent)[ScrollDim];
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just to confirm my understanding: this is virtual because positions everywhere else in framework are floats and without overriding this there is no way to recover precision lost on coordinates, correct?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it's basically allowing the few places which require double to be double. This one is important for accurate scrolling.


/// <summary>
/// Determines the position of a child in the content.
/// </summary>
/// <param name="d">The child to get the position from.</param>
/// <returns>The position of the child.</returns>
public float GetChildPosInContent(Drawable d) => GetChildPosInContent(d, Vector2.Zero);
public double GetChildPosInContent(Drawable d) => GetChildPosInContent(d, Vector2.Zero);

private void updatePosition()
{
Expand All @@ -544,15 +544,15 @@ private void updatePosition()
localDistanceDecay = distance_decay_clamping * 2;

// Lastly, we gradually nudge the target towards valid bounds.
Target = (float)Interpolation.Lerp(Clamp(Target), Target, Math.Exp(-distance_decay_clamping * Time.Elapsed));
Target = Interpolation.Lerp(Clamp(Target), Target, Math.Exp(-distance_decay_clamping * Time.Elapsed));

float clampedTarget = Clamp(Target);
double clampedTarget = Clamp(Target);
if (Precision.AlmostEquals(clampedTarget, Target))
Target = clampedTarget;
}

// Exponential interpolation between the target and our current scroll position.
Current = (float)Interpolation.Lerp(Target, Current, Math.Exp(-localDistanceDecay * Time.Elapsed));
Current = Interpolation.Lerp(Target, Current, Math.Exp(-localDistanceDecay * Time.Elapsed));

// This prevents us from entering the de-normalized range of floating point numbers when approaching target closely.
if (Precision.AlmostEquals(Current, Target))
Expand All @@ -578,28 +578,40 @@ protected override void UpdateAfterChildren()
}

if (ScrollDirection == Direction.Horizontal)
{
Scrollbar.X = ToScrollbarPosition(Current);
ScrollContent.X = -Current + ScrollableExtent * ScrollContent.RelativeAnchorPosition.X;
}
else
{
Scrollbar.Y = ToScrollbarPosition(Current);
ScrollContent.Y = -Current + ScrollableExtent * ScrollContent.RelativeAnchorPosition.Y;
}

ApplyCurrentToContent();
}

/// <summary>
/// This is the final internal step of updating the scroll container, which takes
/// <see cref="Current"/> and applies it to <see cref="ScrollContent"/> in order to
/// correctly offset children.
///
/// Overriding this method can be used to inhibit this default behaviour, to for instance
/// redirect the positioning to another container or change the way it is applied.
/// </summary>
protected virtual void ApplyCurrentToContent()
bdach marked this conversation as resolved.
Show resolved Hide resolved
{
if (ScrollDirection == Direction.Horizontal)
ScrollContent.X = (float)(-Current + (ScrollableExtent * ScrollContent.RelativeAnchorPosition.X));
else
ScrollContent.Y = (float)(-Current + (ScrollableExtent * ScrollContent.RelativeAnchorPosition.Y));
}

/// <summary>
/// Converts a scroll position to a scrollbar position.
/// </summary>
/// <param name="scrollPosition">The absolute scroll position (e.g. <see cref="Current"/>).</param>
/// <returns>The scrollbar position.</returns>
protected virtual float ToScrollbarPosition(float scrollPosition)
protected virtual float ToScrollbarPosition(double scrollPosition)
{
if (Precision.AlmostEquals(0, ScrollableExtent))
return 0;

return ScrollbarMovementExtent * (scrollPosition / ScrollableExtent);
return (float)(ScrollbarMovementExtent * (scrollPosition / ScrollableExtent));
}

/// <summary>
Expand All @@ -612,7 +624,7 @@ protected virtual float FromScrollbarPosition(float scrollbarPosition)
if (Precision.AlmostEquals(0, ScrollbarMovementExtent))
return 0;

return ScrollableExtent * (scrollbarPosition / ScrollbarMovementExtent);
return (float)(ScrollableExtent * (scrollbarPosition / ScrollbarMovementExtent));
}

/// <summary>
Expand Down
Loading