Clamp scale with lower and upper bounds #30080
Conversation
|
I'm kinda confused by this PR in general.
Is this used anywhere?
It is? Even in light of the previous quoted statement?
In what circumstances is this possible? |
| if (Precision.AlmostEquals(p.X, 0)) | ||
| (sLowerBounds.X, sUpperBounds.X) = (float.NegativeInfinity, float.PositiveInfinity); | ||
| if (Precision.AlmostEquals(p.Y, 0)) | ||
| (sLowerBounds.Y, sUpperBounds.Y) = (float.NegativeInfinity, float.PositiveInfinity); |
Currently not yet, but Marvin considered adding negative scaling to the selection box which is WIP
Maybe not entirely accurate statement. It started as a slight feature enhancement for the lower bound stuff, but in the end the code quality became a lot more self-explaining, so I think its a code quality upgrade too.
It's the circumstances shown in the screenshot. It has a rotated grid and scaling on one axis, so a decrease in scaling moves the circle straight towards the grid's X axis which increases the Y coordinate out of bounds. |
Spotted in passing when reviewing ppy#30080. The popover would very arbitrarily revert to scaling by Y axis if both checkboxes were checked off. Not sure how this passed review.
This is mostly a code quality upgrade.
Reworked
ClampScaleToPlayfieldBoundsto clamp the scale value to both a upper bound and an upper bound. Previously only did an upper bound. With this it supports negative scale values. I also added comments to make it more clear what is going on there.There is this rare situation where the lower bound equals 1, previously it would allow you to scale this by 0.5 which would direct the circle off-screen.
