Add `iround()` and round PointF->PointInt by default

[falbrechtskirchinger]

Required by #1594 and split out from it.

Changes:

• Add a floating-point to integer rounding function iround<IntType>().
• Replace uses of std::lround() with the cheaper iround().
• Add a function for rounding integers to multiples of N.
• Modify the Point conversion constructor to round when converting floating-point element types to integers.
  • Add a non-rounding overload for the Point conversion constructor.

Additionally, the following change was made to discover the affected code paths by rounding in the Point conversion constructor:

• The floating-point to integer conversion operator was replaced with two variants – one using rounding, one using plain casting – disambiguated via tag dispatch.

• 6 uses (excluding test code) were identified this way, and only 2 of those don't require rounding because the Point<float> was created from integer values.

~~If desired, a tag dispatch variant to disable rounding could be kept, but the benefits seem negligible. Unless someone identifies a use case where rounding has to be disabled, I recommend deleting both tag dispatch variants and rounding all floating-point to integer conversions indiscriminately.~~

I did consider adding a new header, but mathFuncs.h already contains related functions (e.g., roundedDiv()).

To-do:

• [x] Remove CI commit from this PR once merged. (#1596)
• [x] Add unit tests.
• [x] Decide whether to keep at least one tag dispatch overload around or delete both.

[falbrechtskirchinger]

With only one remaining use, I'm removing the roundToMultiplesOf() function.

As for the Point conversion constructors, after looking at the assembly difference for various archs on Compiler Explorer, I believe it's worthwhile to keep the NoRounding tag dispatch variant but otherwise round by default.

Edit: Even more valid since I fixed the sign error.

[falbrechtskirchinger]

Thanks I think it makes sense to round instead of truncate although it can be a pitfall. What exactly is this required for though?

None of the calculations are so precise that I'd say it's required, but it does feel wrong to me to just truncate in all paces. I guess that's the reason why I've focused on making it fast at least.

Also have you double-checked all files where you added mathFuncs.h and removed std::lround if the cmath include can be removed?

I did check once but would want to double-check if iround() is here to stay.

---

I've already removed one function from this PR and I don't feel strongly about keeping iround() TBH. But Point should round by default for correctness' sake.

[Flamefire]

but it does feel wrong to me to just truncate in all paces.

Initial motivation: Conversion between different Point types is explicit, so Point<int>(floatPoint) acts like static_cast<int>(floatValue) especially as that is what happens between integer Point-types

I guess the idea was that usually a position or size is used. And you don't want that to accidentally exceed a given size due to floating point (in)accuracy, so in those cases 1 px less might be better than 1px more.

I don't feel strongly about keeping iround() TBH.

I think it is good to encapsulate the static_cast(round(value)) thing into a function where we could place asserts. So IMO it is worth keeping.

But Point should round by default for correctness' sake.

See the initial motivation: To me it isn't that clear if it by default should round or act like a cast. But having a ctor that rounds is useful as you identified. I'm fine with that being the default and I like the tag dispatch.
For completeness: An alternative would be to require a rounding mode (e.g. enum: truncate or round) for float->int conversion of points.

Unless @Flow86 feels strongly either way I'd be fine with the currently discussed changes (i.e. use of Truncate and std::lround, round by default)

[falbrechtskirchinger]

Let me know if the additional error handling is too much. I chased a silly bug and that's what I ended up with before I noticed a simple typo.

Edit: Removed the implementation-defined stuff that triggered on MacOS.

[falbrechtskirchinger]

You know … I'm starting to think I wasn't imagining a division-by-zero issue in the FrameCounter …

std::lround() just happens to return 0 in that case in all relevant implementations.

Edit: Hello, my name is Confirmation Bias, and I'd like to talk to you.

[Flamefire]

Uff, sorry I thought this was done now and wanted to spare you the work to apply and rebase such that Github allows me to merge this. Feel free to force-push your version (with the inline-removal applied)

[falbrechtskirchinger]

Don't worry, it's fine. Just running tests locally to double-check, then I'll push.