-
Notifications
You must be signed in to change notification settings - Fork 436
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
Changes from 4 commits
17ef9dc
26c808f
7ed84be
cc595be
870cecd
862a68d
0e558e2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,149 @@ | ||
// 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; | ||
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 |
---|---|---|
|
@@ -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; | ||
|
||
|
@@ -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. | ||
|
@@ -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> | ||
|
@@ -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> | ||
|
@@ -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); | ||
|
||
|
@@ -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); | ||
|
@@ -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]; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, it's basically allowing the few places which require |
||
|
||
/// <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() | ||
{ | ||
|
@@ -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)) | ||
|
@@ -578,28 +578,32 @@ 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(); | ||
} | ||
|
||
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> | ||
|
@@ -612,7 +616,7 @@ protected virtual float FromScrollbarPosition(float scrollbarPosition) | |
if (Precision.AlmostEquals(0, ScrollbarMovementExtent)) | ||
return 0; | ||
|
||
return ScrollableExtent * (scrollbarPosition / ScrollbarMovementExtent); | ||
return (float)(ScrollableExtent * (scrollbarPosition / ScrollbarMovementExtent)); | ||
} | ||
|
||
/// <summary> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the fact that this has to exist is unfortunate but i don't see any better way either. at least not if i correctly understand why this has to exist (see preceding comments about how coords everywhere else are floats)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW this will only be implemented by usages in osu! for instance. I moved this class inside the test scene to make it very clear.