Use double in ScrollContainer for scroll tracking
#6467
Conversation
This is the first step towards supporting scrolling over very large amounts of content. With this along, the animation already becomes somewhat smoother (no longer "locking" into place too early) but scroll content will still look jagged when scrolled too far down. Fixing actual content requires a small amount of extra implementation on top of this. To keep things simple this would be up to the consumer to implement and maintain. Intended to help fix osu! song select from becoming weird when hundreds of thousands of beatmaps are loaded.
… to content This allows a point of intercept to distribute `double` values to children and bypass floating point precision issues.
| @@ -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.
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.
Yeah, it's basically allowing the few places which require double to be double. This one is important for accurate scrolling.
| ApplyCurrentToContent(); | ||
| } | ||
|
|
||
| protected virtual void ApplyCurrentToContent() |
There was a problem hiding this comment.
method needs xmldoc explaining when it's called and what work is expected of consumers when overriding it, it's not exactly clear from the name alone
| } | ||
| } | ||
|
|
||
| public partial class DoubleScrollContainer : BasicScrollContainer |
There was a problem hiding this comment.
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.
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.
This is the first step towards supporting scrolling over very large amounts of content. With this along, the animation already becomes somewhat smoother (no longer "locking" into place too early) but scroll content will still look jagged when scrolled too far down.
Fixing actual content requires a small amount of extra implementation on top of this. To keep things simple this would be up to the consumer to implement and maintain.
Intended to help fix osu! song select from becoming weird when hundreds of thousands of beatmaps are loaded.
A sample implementation is provided in
TestSceneScrollContainerDoublePrecision.master with a
BasicScrollContainer:osu.Framework.Tests.2024-12-20.at.09.12.41.mp4
This PR with a
BasicScrollContainer:osu.Framework.Tests.2024-12-20.at.09.09.57.mp4
This PR with the custom double implementation:
osu.Framework.Tests.2024-12-20.at.09.11.43.mp4