Use generic math for bindable numbers #6248

huoyaoyuan · 2024-04-16T07:56:56Z

The generic math interfaces were experimental in .NET 6, then received breaking change of namespace in .NET 7. It's considered mature now.

Please DO test for Mono-based runtimes (especially iOS AOT) for such type-system related feature.

bdach · 2024-04-16T07:59:03Z

Context: This was opened on my request to explore it as a possible fix to ppy/osu#27847.

huoyaoyuan · 2024-04-16T08:00:09Z

osu.Framework/Bindables/BindableNumber.cs

 using JetBrains.Annotations;
 using osu.Framework.Utils;

 namespace osu.Framework.Bindables
 {
    public class BindableNumber<T> : RangeConstrainedBindable<T>, IBindableNumber<T>
-        where T : struct, IComparable<T>, IConvertible, IEquatable<T>
+        where T : struct, INumber<T>, IMinMaxValue<T>


Technically this can be combination of operator interfaces rather than the meta one, but you shouldn't be happy about that.

huoyaoyuan · 2024-04-16T08:01:03Z

osu.Framework/Bindables/BindableNumber.cs

                if (typeof(T) == typeof(float))
                    return (T)(object)float.Epsilon;
+                if (typeof(T) == typeof(double))
+                    return (T)(object)double.Epsilon;

-                return (T)(object)double.Epsilon;
+                return T.One;


dotnet/csharplang#6308
Just testing floating-point types here to make things simpler.

huoyaoyuan · 2024-04-16T08:02:48Z

osu.Framework/Bindables/BindableNumber.cs


-                base.Value = convertFromDouble(doubleValue);
+                base.Value = T.CreateTruncating(doubleValue);


Note: CreateTruncating does C-style cast.

I would have hoped for staying within the confines of the T here rather than still always going through doubles. At least for IFloatingPoint<T> implementors.

It's basically rounding to a multiplication of Precision. For integers it can be done by comparisons.
Replicating the ties-to-even behavior is more complex. I'm not sure whether it's expected, or ties-to-zero is fine.

bdach · 2024-04-16T12:47:31Z

Game-side diff:

diff --git a/osu.Game/Graphics/UserInterface/ExpandableSlider.cs b/osu.Game/Graphics/UserInterface/ExpandableSlider.cs
index 121a1eef49..a7a8561b94 100644
--- a/osu.Game/Graphics/UserInterface/ExpandableSlider.cs
+++ b/osu.Game/Graphics/UserInterface/ExpandableSlider.cs
@@ -1,7 +1,7 @@
 // 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.Numerics;
 using osu.Framework.Allocation;
 using osu.Framework.Bindables;
 using osu.Framework.Graphics;
@@ -10,7 +10,7 @@
 using osu.Framework.Localisation;
 using osu.Game.Graphics.Containers;
 using osu.Game.Graphics.Sprites;
-using osuTK;
+using Vector2 = osuTK.Vector2;
 
 namespace osu.Game.Graphics.UserInterface
 {
@@ -18,7 +18,7 @@ namespace osu.Game.Graphics.UserInterface
     /// An <see cref="IExpandable"/> implementation for the UI slider bar control.
     /// </summary>
     public partial class ExpandableSlider<T, TSlider> : CompositeDrawable, IExpandable, IHasCurrentValue<T>
-        where T : struct, IEquatable<T>, IComparable<T>, IConvertible
+        where T : struct, INumber<T>, IMinMaxValue<T>
         where TSlider : RoundedSliderBar<T>, new()
     {
         private readonly OsuSpriteText label;
@@ -129,7 +129,7 @@ protected override void LoadComplete()
     /// An <see cref="IExpandable"/> implementation for the UI slider bar control.
     /// </summary>
     public partial class ExpandableSlider<T> : ExpandableSlider<T, RoundedSliderBar<T>>
-        where T : struct, IEquatable<T>, IComparable<T>, IConvertible
+        where T : struct, INumber<T>, IMinMaxValue<T>
     {
     }
 }
diff --git a/osu.Game/Graphics/UserInterface/OsuSliderBar.cs b/osu.Game/Graphics/UserInterface/OsuSliderBar.cs
index 191a7ca246..9cb6356cab 100644
--- a/osu.Game/Graphics/UserInterface/OsuSliderBar.cs
+++ b/osu.Game/Graphics/UserInterface/OsuSliderBar.cs
@@ -2,6 +2,7 @@
 // See the LICENCE file in the repository root for full licence text.
 
 using System;
+using System.Numerics;
 using System.Globalization;
 using osu.Framework.Allocation;
 using osu.Framework.Audio;
@@ -15,7 +16,7 @@
 namespace osu.Game.Graphics.UserInterface
 {
     public abstract partial class OsuSliderBar<T> : SliderBar<T>, IHasTooltip
-        where T : struct, IEquatable<T>, IComparable<T>, IConvertible
+        where T : struct, INumber<T>, IMinMaxValue<T>
     {
         public bool PlaySamplesOnAdjust { get; set; } = true;
 
@@ -85,11 +86,11 @@ private void playSample(T value)
         private LocalisableString getTooltipText(T value)
         {
             if (CurrentNumber.IsInteger)
-                return value.ToInt32(NumberFormatInfo.InvariantInfo).ToString("N0");
+                return int.CreateTruncating(value).ToString("N0");
 
-            double floatValue = value.ToDouble(NumberFormatInfo.InvariantInfo);
+            double floatValue = double.CreateTruncating(value);
 
-            decimal decimalPrecision = normalise(CurrentNumber.Precision.ToDecimal(NumberFormatInfo.InvariantInfo), max_decimal_digits);
+            decimal decimalPrecision = normalise(decimal.CreateTruncating(CurrentNumber.Precision), max_decimal_digits);
 
             // Find the number of significant digits (we could have less than 5 after normalize())
             int significantDigits = FormatUtils.FindPrecision(decimalPrecision);
diff --git a/osu.Game/Graphics/UserInterface/RoundedSliderBar.cs b/osu.Game/Graphics/UserInterface/RoundedSliderBar.cs
index 0981881ead..56047173bb 100644
--- a/osu.Game/Graphics/UserInterface/RoundedSliderBar.cs
+++ b/osu.Game/Graphics/UserInterface/RoundedSliderBar.cs
@@ -2,7 +2,7 @@
 // See the LICENCE file in the repository root for full licence text.
 
 using System;
-using osuTK;
+using System.Numerics;
 using osuTK.Graphics;
 using osu.Framework.Allocation;
 using osu.Framework.Extensions.Color4Extensions;
@@ -11,11 +11,12 @@
 using osu.Framework.Graphics.Shapes;
 using osu.Framework.Input.Events;
 using osu.Game.Overlays;
+using Vector2 = osuTK.Vector2;
 
 namespace osu.Game.Graphics.UserInterface
 {
     public partial class RoundedSliderBar<T> : OsuSliderBar<T>
-        where T : struct, IEquatable<T>, IComparable<T>, IConvertible
+        where T : struct, INumber<T>, IMinMaxValue<T>
     {
         protected readonly Nub Nub;
         protected readonly Box LeftBox;
diff --git a/osu.Game/Graphics/UserInterface/ShearedSliderBar.cs b/osu.Game/Graphics/UserInterface/ShearedSliderBar.cs
index 60a6670492..0df1c1d204 100644
--- a/osu.Game/Graphics/UserInterface/ShearedSliderBar.cs
+++ b/osu.Game/Graphics/UserInterface/ShearedSliderBar.cs
@@ -2,7 +2,7 @@
 // See the LICENCE file in the repository root for full licence text.
 
 using System;
-using osuTK;
+using System.Numerics;
 using osuTK.Graphics;
 using osu.Framework.Allocation;
 using osu.Framework.Extensions.Color4Extensions;
@@ -12,11 +12,12 @@
 using osu.Framework.Input.Events;
 using osu.Game.Overlays;
 using static osu.Game.Graphics.UserInterface.ShearedNub;
+using Vector2 = osuTK.Vector2;
 
 namespace osu.Game.Graphics.UserInterface
 {
     public partial class ShearedSliderBar<T> : OsuSliderBar<T>
-        where T : struct, IEquatable<T>, IComparable<T>, IConvertible
+        where T : struct, INumber<T>, IMinMaxValue<T>
     {
         protected readonly ShearedNub Nub;
         protected readonly Box LeftBox;
diff --git a/osu.Game/Graphics/UserInterfaceV2/LabelledSliderBar.cs b/osu.Game/Graphics/UserInterfaceV2/LabelledSliderBar.cs
index 4585d3a4c9..4912a21fab 100644
--- a/osu.Game/Graphics/UserInterfaceV2/LabelledSliderBar.cs
+++ b/osu.Game/Graphics/UserInterfaceV2/LabelledSliderBar.cs
@@ -1,14 +1,14 @@
 // 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.Numerics;
 using osu.Framework.Graphics;
 using osu.Game.Overlays.Settings;
 
 namespace osu.Game.Graphics.UserInterfaceV2
 {
     public partial class LabelledSliderBar<TNumber> : LabelledComponent<SettingsSlider<TNumber>, TNumber>
-        where TNumber : struct, IEquatable<TNumber>, IComparable<TNumber>, IConvertible
+        where TNumber : struct, INumber<TNumber>, IMinMaxValue<TNumber>
     {
         public LabelledSliderBar()
             : base(true)
diff --git a/osu.Game/Graphics/UserInterfaceV2/SliderWithTextBoxInput.cs b/osu.Game/Graphics/UserInterfaceV2/SliderWithTextBoxInput.cs
index e5ba7f61bf..abd828e98f 100644
--- a/osu.Game/Graphics/UserInterfaceV2/SliderWithTextBoxInput.cs
+++ b/osu.Game/Graphics/UserInterfaceV2/SliderWithTextBoxInput.cs
@@ -1,7 +1,7 @@
 // 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.Numerics;
 using System.Globalization;
 using osu.Framework.Bindables;
 using osu.Framework.Graphics;
@@ -10,12 +10,12 @@
 using osu.Framework.Localisation;
 using osu.Game.Overlays.Settings;
 using osu.Game.Utils;
-using osuTK;
+using Vector2 = osuTK.Vector2;
 
 namespace osu.Game.Graphics.UserInterfaceV2
 {
     public partial class SliderWithTextBoxInput<T> : CompositeDrawable, IHasCurrentValue<T>
-        where T : struct, IEquatable<T>, IComparable<T>, IConvertible
+        where T : struct, INumber<T>, IMinMaxValue<T>
     {
         /// <summary>
         /// A custom step value for each key press which actuates a change on this control.
@@ -138,7 +138,7 @@ private void updateTextBoxFromSlider(ValueChangedEvent<T> _)
         {
             if (updatingFromTextBox) return;
 
-            decimal decimalValue = slider.Current.Value.ToDecimal(NumberFormatInfo.InvariantInfo);
+            decimal decimalValue = decimal.CreateTruncating(slider.Current.Value);
             textBox.Text = decimalValue.ToString($@"N{FormatUtils.FindPrecision(decimalValue)}");
         }
     }
diff --git a/osu.Game/Overlays/Settings/Sections/SizeSlider.cs b/osu.Game/Overlays/Settings/Sections/SizeSlider.cs
index c73831d8d1..14ef58ff88 100644
--- a/osu.Game/Overlays/Settings/Sections/SizeSlider.cs
+++ b/osu.Game/Overlays/Settings/Sections/SizeSlider.cs
@@ -2,6 +2,7 @@
 // See the LICENCE file in the repository root for full licence text.
 
 using System;
+using System.Numerics;
 using System.Globalization;
 using osu.Framework.Localisation;
 using osu.Game.Graphics.UserInterface;
@@ -12,7 +13,7 @@ namespace osu.Game.Overlays.Settings.Sections
     /// A slider intended to show a "size" multiplier number, where 1x is 1.0.
     /// </summary>
     public partial class SizeSlider<T> : RoundedSliderBar<T>
-        where T : struct, IEquatable<T>, IComparable<T>, IConvertible, IFormattable
+        where T : struct, INumber<T>, IMinMaxValue<T>, IFormattable
     {
         public override LocalisableString TooltipText => Current.Value.ToString(@"0.##x", NumberFormatInfo.CurrentInfo);
     }
diff --git a/osu.Game/Overlays/Settings/SettingsPercentageSlider.cs b/osu.Game/Overlays/Settings/SettingsPercentageSlider.cs
index fa59d18de1..d7a09d3392 100644
--- a/osu.Game/Overlays/Settings/SettingsPercentageSlider.cs
+++ b/osu.Game/Overlays/Settings/SettingsPercentageSlider.cs
@@ -1,7 +1,7 @@
 // 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.Numerics;
 using osu.Framework.Graphics;
 using osu.Game.Configuration;
 using osu.Game.Graphics.UserInterface;
@@ -13,7 +13,7 @@ namespace osu.Game.Overlays.Settings
     /// Mostly provided for convenience of use with <see cref="SettingSourceAttribute"/>.
     /// </summary>
     public partial class SettingsPercentageSlider<TValue> : SettingsSlider<TValue>
-        where TValue : struct, IEquatable<TValue>, IComparable<TValue>, IConvertible
+        where TValue : struct, INumber<TValue>, IMinMaxValue<TValue>
     {
         protected override Drawable CreateControl() => ((RoundedSliderBar<TValue>)base.CreateControl()).With(sliderBar => sliderBar.DisplayAsPercentage = true);
     }
diff --git a/osu.Game/Overlays/Settings/SettingsSlider.cs b/osu.Game/Overlays/Settings/SettingsSlider.cs
index 6c81fece13..2460d78099 100644
--- a/osu.Game/Overlays/Settings/SettingsSlider.cs
+++ b/osu.Game/Overlays/Settings/SettingsSlider.cs
@@ -1,7 +1,7 @@
 // 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.Numerics;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.UserInterface;
 using osu.Game.Graphics.UserInterface;
@@ -9,12 +9,12 @@
 namespace osu.Game.Overlays.Settings
 {
     public partial class SettingsSlider<T> : SettingsSlider<T, RoundedSliderBar<T>>
-        where T : struct, IEquatable<T>, IComparable<T>, IConvertible
+        where T : struct, INumber<T>, IMinMaxValue<T>
     {
     }
 
     public partial class SettingsSlider<TValue, TSlider> : SettingsItem<TValue>
-        where TValue : struct, IEquatable<TValue>, IComparable<TValue>, IConvertible
+        where TValue : struct, INumber<TValue>, IMinMaxValue<TValue>
         where TSlider : RoundedSliderBar<TValue>, new()
     {
         protected override Drawable CreateControl() => new TSlider
diff --git a/osu.Game/Screens/Edit/Timing/IndeterminateSliderWithTextBoxInput.cs b/osu.Game/Screens/Edit/Timing/IndeterminateSliderWithTextBoxInput.cs
index 151d469415..26f374ba85 100644
--- a/osu.Game/Screens/Edit/Timing/IndeterminateSliderWithTextBoxInput.cs
+++ b/osu.Game/Screens/Edit/Timing/IndeterminateSliderWithTextBoxInput.cs
@@ -1,7 +1,7 @@
 // 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.Numerics;
 using System.Globalization;
 using osu.Framework.Bindables;
 using osu.Framework.Graphics;
@@ -12,7 +12,7 @@
 using osu.Game.Graphics.UserInterfaceV2;
 using osu.Game.Overlays.Settings;
 using osu.Game.Utils;
-using osuTK;
+using Vector2 = osuTK.Vector2;
 
 namespace osu.Game.Screens.Edit.Timing
 {
@@ -22,7 +22,7 @@ namespace osu.Game.Screens.Edit.Timing
     /// by providing an "indeterminate state".
     /// </summary>
     public partial class IndeterminateSliderWithTextBoxInput<T> : CompositeDrawable, IHasCurrentValue<T?>
-        where T : struct, IEquatable<T>, IComparable<T>, IConvertible
+        where T : struct, INumber<T>, IMinMaxValue<T>
     {
         /// <summary>
         /// A custom step value for each key press which actuates a change on this control.
@@ -136,7 +136,7 @@ private void updateState()
                 slider.Current.Value = nonNullValue;
 
                 // use the value from the slider to ensure that any precision/min/max set on it via the initial indeterminate value have been applied correctly.
-                decimal decimalValue = slider.Current.Value.ToDecimal(NumberFormatInfo.InvariantInfo);
+                decimal decimalValue = decimal.CreateTruncating(slider.Current.Value);
                 textBox.Text = decimalValue.ToString($@"N{FormatUtils.FindPrecision(decimalValue)}");
                 textBox.PlaceholderText = string.Empty;
             }
diff --git a/osu.Game/Screens/Play/PlayerSettings/PlayerSliderBar.cs b/osu.Game/Screens/Play/PlayerSettings/PlayerSliderBar.cs
index 88b778fafb..1fc1155c0b 100644
--- a/osu.Game/Screens/Play/PlayerSettings/PlayerSliderBar.cs
+++ b/osu.Game/Screens/Play/PlayerSettings/PlayerSliderBar.cs
@@ -1,7 +1,7 @@
 // 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.Numerics;
 using osu.Framework.Allocation;
 using osu.Framework.Graphics;
 using osu.Game.Graphics;
@@ -11,7 +11,7 @@
 namespace osu.Game.Screens.Play.PlayerSettings
 {
     public partial class PlayerSliderBar<T> : SettingsSlider<T>
-        where T : struct, IEquatable<T>, IComparable<T>, IConvertible
+        where T : struct, INumber<T>, IMinMaxValue<T>
     {
         public RoundedSliderBar<T> Bar => (RoundedSliderBar<T>)Control;

Unfortunately this doesn't help with the issue I mentioned so I'm not sure we're going to be prioritising this now, but I wouldn't necessarily merging this at some point.

Caused framework visual tests to crash (toolbar record section had a slider bar of `MinValue == MaxValue == 0`).

bdach · 2024-04-19T11:50:13Z

This seems generally okay to me (after applying some fixes) but at this point it's kind of like a code quality change rather than anything immediately useful so I'm not exactly super wanting to rush this forward. I did test android, it looks like it handled itself okay. iOS check is probably best to do though since that's the only real AOT platform.

I'll want to run game-side tests against this pull too before approving. Maybe later today.

smoogipoo

Tested debug + release on iOS device. Looks to work. Reasonably confident this is fine.

huoyaoyuan added 3 commits April 16, 2024 15:53

Rewrite BindableNumber in stable INumber<T>

a85230f

Update dependents constraints to generic math

f22376f

Use T.One for integers

84df9e2

pull-request-size bot added the size/L label Apr 16, 2024

huoyaoyuan mentioned this pull request Apr 16, 2024

Certain float settings are saved weirdly in the editor ppy/osu#27847

Closed

huoyaoyuan commented Apr 16, 2024

View reviewed changes

huoyaoyuan added 2 commits April 16, 2024 16:10

Generic constraint for test scene

d699d51

Minor fixes

aa81aa7

bdach self-requested a review April 16, 2024 12:16

Fix RangeConstrainedBindable throwing on binding

bc3cfda

bdach added 2 commits April 19, 2024 13:26

Merge branch 'master' into net7.0/generic-math

8142201

Undo semantics change in bound check

339f778

Caused framework visual tests to crash (toolbar record section had a slider bar of `MinValue == MaxValue == 0`).

smoogipoo approved these changes Apr 22, 2024

View reviewed changes

Merge branch 'master' into net7.0/generic-math

1a30d36

smoogipoo enabled auto-merge April 22, 2024 07:00

smoogipoo merged commit 69a26ce into ppy:master Apr 22, 2024
20 of 21 checks passed

huoyaoyuan deleted the net7.0/generic-math branch April 22, 2024 07:26

bdach mentioned this pull request Apr 22, 2024

Apply generic math-related changes ppy/osu#27965

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use generic math for bindable numbers #6248

Use generic math for bindable numbers #6248

huoyaoyuan commented Apr 16, 2024 •

edited

Loading

bdach commented Apr 16, 2024

huoyaoyuan Apr 16, 2024

huoyaoyuan Apr 16, 2024

huoyaoyuan Apr 16, 2024

bdach Apr 16, 2024

huoyaoyuan Apr 16, 2024

bdach commented Apr 16, 2024 •

edited

Loading

bdach commented Apr 19, 2024 •

edited

Loading

smoogipoo left a comment


		base.Value = convertFromDouble(doubleValue);
		base.Value = T.CreateTruncating(doubleValue);

Use generic math for bindable numbers #6248

Use generic math for bindable numbers #6248

Conversation

huoyaoyuan commented Apr 16, 2024 • edited Loading

bdach commented Apr 16, 2024

huoyaoyuan Apr 16, 2024

Choose a reason for hiding this comment

huoyaoyuan Apr 16, 2024

Choose a reason for hiding this comment

huoyaoyuan Apr 16, 2024

Choose a reason for hiding this comment

bdach Apr 16, 2024

Choose a reason for hiding this comment

huoyaoyuan Apr 16, 2024

Choose a reason for hiding this comment

bdach commented Apr 16, 2024 • edited Loading

bdach commented Apr 19, 2024 • edited Loading

smoogipoo left a comment

Choose a reason for hiding this comment

huoyaoyuan commented Apr 16, 2024 •

edited

Loading

bdach commented Apr 16, 2024 •

edited

Loading

bdach commented Apr 19, 2024 •

edited

Loading