I've noticed the newly added Rectangle::toNearestInt() function and I have some concerns regarding its implementation:
Imagine I have a Rectangle <float> having
x = 15.4
width = 4.4
This means that the right side of this rectangle lies at 19.8 ( = 15.4 + 4.4)
When rounding this rectangle to int, I'd expect the left side to snap at 15 (from 15.4) and the right side to snap at 20 (from 19.8).
With the current implementation, the left side would be snapped at 15 but the width would be snapped at 4, thus effectively bringing the right side to snap at 19 rather than 20.
What do you think about this?
There are 3 ways it could be done:
- snapping left and right, and allowing the width to lose accuracy
- snapping left and width, and allowing the right to lose accuracy
- snapping right and width, allowing the left to lose accuracy
None of these can ever be perfect for every use-case, so we chose to snap x and width, because those are the actual values stored in the rectangle.
I'm perfectly aware that one of those dimension is destined to lose some accuracy.
As a personal taste, I would have gone the way of "snap left and right, and leave width be what it needs to be", it seems more true to how I'd expect a float rectangle to behave when forced to be drawn in integer pixels, for example.
Secondly, if you wish to keep the current behaviour, I think you should improve the function documentation: the current "rounding the bounds" formulation means to me (and I suspect to others) that it's the "left" and "right" boundaries that are being rounded, not the "x" position and the "width" dimension as it is currently the case.
Thanks. I updated the documentation to reflect a bit better what is going on.
FWIW, as a personal taste, snapping left, top, width and height makes the most sense to me...
Fair enough, thanks for clarifying the doc