Skip to content

Commit ffef6ae

Browse files
committed
Fix possible crash when scaling objects in editor
The specific fail case here is when `s.{X,Y}` is 0, and `s{Lower,Upper}Bound` is `Infinity`. Because IEEE math is IEEE math, `0 * Infinity` is `NaN`. `MathHelper.Clamp()` is written the following way: https://github.com/ppy/osuTK/blob/af742f1afd01828efc7bc9fe77536b54aab8b419/src/osuTK/Math/MathHelper.cs#L284-L306 `Math.{Min,Max}` are both documented as reporting `NaN` when any of their operands are `NaN`: https://learn.microsoft.com/en-us/dotnet/api/system.math.min?view=net-8.0#system-math-min(system-single-system-single) https://learn.microsoft.com/en-us/dotnet/api/system.math.max?view=net-8.0#system-math-max(system-single-system-single) which means that if a `NaN` happens to sneak into the bounds, it will start spreading outwards in an uncontrolled manner, and likely crash things. In contrast, the standard library provided `Math.Clamp()` is written like so: https://github.com/dotnet/runtime/blob/577c36cee56480dec4d4610b35605b5d5836888b/src/libraries/System.Private.CoreLib/src/System/Math.cs#L711-L729 With this implementation, if either bound is `NaN`, it will essentially not be checked (because any and all comparisons involving `NaN` return false). This prevents the spread of `NaN`s, all the way to positions of hitobjects, and thus fixes the crash.
1 parent 79b737b commit ffef6ae

File tree

1 file changed

+6
-6
lines changed

1 file changed

+6
-6
lines changed

osu.Game.Rulesets.Osu/Edit/OsuSelectionScaleHandler.cs

+6-6
Original file line numberDiff line numberDiff line change
@@ -263,24 +263,24 @@ Vector2 clampToBounds(Vector2 s, Vector2 p, Vector2 lowerBounds, Vector2 upperBo
263263
{
264264
case Axes.X:
265265
(sLowerBound, sUpperBound) = computeBounds(lowerBounds - b, upperBounds - b, a);
266-
s.X = MathHelper.Clamp(s.X, sLowerBound, sUpperBound);
266+
s.X = Math.Clamp(s.X, sLowerBound, sUpperBound);
267267
break;
268268

269269
case Axes.Y:
270270
(sLowerBound, sUpperBound) = computeBounds(lowerBounds - a, upperBounds - a, b);
271-
s.Y = MathHelper.Clamp(s.Y, sLowerBound, sUpperBound);
271+
s.Y = Math.Clamp(s.Y, sLowerBound, sUpperBound);
272272
break;
273273

274274
case Axes.Both:
275275
// Here we compute the bounds for the magnitude multiplier of the scale vector
276276
// Therefore the ratio s.X / s.Y will be maintained
277277
(sLowerBound, sUpperBound) = computeBounds(lowerBounds, upperBounds, a * s.X + b * s.Y);
278278
s.X = s.X < 0
279-
? MathHelper.Clamp(s.X, s.X * sUpperBound, s.X * sLowerBound)
280-
: MathHelper.Clamp(s.X, s.X * sLowerBound, s.X * sUpperBound);
279+
? Math.Clamp(s.X, s.X * sUpperBound, s.X * sLowerBound)
280+
: Math.Clamp(s.X, s.X * sLowerBound, s.X * sUpperBound);
281281
s.Y = s.Y < 0
282-
? MathHelper.Clamp(s.Y, s.Y * sUpperBound, s.Y * sLowerBound)
283-
: MathHelper.Clamp(s.Y, s.Y * sLowerBound, s.Y * sUpperBound);
282+
? Math.Clamp(s.Y, s.Y * sUpperBound, s.Y * sLowerBound)
283+
: Math.Clamp(s.Y, s.Y * sLowerBound, s.Y * sUpperBound);
284284
break;
285285
}
286286

0 commit comments

Comments
 (0)