From ed3b13437279fe5223f3b9742979ffdda492b20f Mon Sep 17 00:00:00 2001 From: Florian Albrechtskirchinger Date: Fri, 7 Jul 2023 18:57:26 +0200 Subject: [PATCH 1/8] Add helper for floating-point to integer rounding Add iround() helper function for branchless rounding of floating-point values to integer types.. --- libs/common/include/helpers/mathFuncs.h | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/libs/common/include/helpers/mathFuncs.h b/libs/common/include/helpers/mathFuncs.h index ddd697bcc..89b926d31 100644 --- a/libs/common/include/helpers/mathFuncs.h +++ b/libs/common/include/helpers/mathFuncs.h @@ -4,6 +4,8 @@ #pragma once +#include "RTTR_Assert.h" +#include #include namespace helpers { @@ -79,4 +81,16 @@ constexpr T inverseLerp(const T startVal, const T endVal, const T value) noexcep { return (value - startVal) / (endVal - startVal); } + +// TODO can be constexpr in C++20 +/// Arithmetically round floating point values to integers +template::value && std::is_floating_point::value, int> = 0> +inline IntType iround(const FloatType val) noexcept +{ + if(std::is_unsigned::value) + RTTR_Assert_Msg(!(val < 0), "Floating-point value must not be negative when casting to unsigned integer type."); + + return static_cast(std::lround(val)); +} } // namespace helpers From a1543baddd06ba0ae66ce52d291a4b2822e095b4 Mon Sep 17 00:00:00 2001 From: Florian Albrechtskirchinger Date: Fri, 7 Jul 2023 20:09:54 +0200 Subject: [PATCH 2/8] Replace uses of std::lround() with iround() --- libs/s25main/FrameCounter.cpp | 4 ++-- libs/s25main/controls/ctrlProgress.cpp | 6 +++--- libs/s25main/controls/ctrlTable.cpp | 4 ++-- libs/s25main/ingameWindows/iwConnecting.cpp | 4 ++-- libs/s25main/network/GameServer.cpp | 4 ++-- libs/s25main/world/GameWorld.cpp | 6 +++--- 6 files changed, 14 insertions(+), 14 deletions(-) diff --git a/libs/s25main/FrameCounter.cpp b/libs/s25main/FrameCounter.cpp index c93ff7aad..24324585e 100644 --- a/libs/s25main/FrameCounter.cpp +++ b/libs/s25main/FrameCounter.cpp @@ -3,9 +3,9 @@ // SPDX-License-Identifier: GPL-2.0-or-later #include "FrameCounter.h" +#include "helpers/mathFuncs.h" #include "helpers/win32_nanosleep.h" // IWYU pragma: keep #include -#include //-V:clock::time_point:813 @@ -36,7 +36,7 @@ unsigned FrameCounter::getCurFrameRate() const if(timeDiff == clock::duration::zero()) return 0; using dSeconds = std::chrono::duration; - return std::lround(curNumFrames_ / std::chrono::duration_cast(timeDiff).count()); + return helpers::iround(curNumFrames_ / std::chrono::duration_cast(timeDiff).count()); } FrameTimer::FrameTimer(int targetFramerate, unsigned maxLagFrames, clock::time_point curTime) diff --git a/libs/s25main/controls/ctrlProgress.cpp b/libs/s25main/controls/ctrlProgress.cpp index 2a4c9b470..de8289592 100644 --- a/libs/s25main/controls/ctrlProgress.cpp +++ b/libs/s25main/controls/ctrlProgress.cpp @@ -6,9 +6,9 @@ #include "CollisionDetection.h" #include "Loader.h" #include "WindowManager.h" +#include "helpers/mathFuncs.h" #include "ogl/FontStyle.h" #include "ogl/glFont.h" -#include ctrlProgress::ctrlProgress(Window* parent, const unsigned id, const DrawPoint& pos, const Extent& size, const TextureColor tc, unsigned short button_minus, unsigned short button_plus, @@ -140,8 +140,8 @@ bool ctrlProgress::Msg_LeftDown(const MouseCoords& mc) Extent progressSize = GetSize() - Extent((GetSize().y + 1) * 2, 8) - padding_ * 2u; if(IsPointInRect(mc.GetPos(), Rect(progressOrigin, progressSize))) { - position = static_cast( - std::lround(static_cast((mc.pos.x - progressOrigin.x) * maximum) / progressSize.x)); + position = + helpers::iround(static_cast((mc.pos.x - progressOrigin.x) * maximum) / progressSize.x); if(GetParent()) GetParent()->Msg_ProgressChange(GetID(), position); diff --git a/libs/s25main/controls/ctrlTable.cpp b/libs/s25main/controls/ctrlTable.cpp index fd2284aa0..6b9628bd3 100644 --- a/libs/s25main/controls/ctrlTable.cpp +++ b/libs/s25main/controls/ctrlTable.cpp @@ -8,11 +8,11 @@ #include "ctrlScrollBar.h" #include "driver/KeyEvent.h" #include "driver/MouseCoords.h" +#include "helpers/mathFuncs.h" #include "ogl/glFont.h" #include "s25util/StringConversion.h" #include "s25util/strAlgos.h" #include -#include #include #include @@ -465,7 +465,7 @@ void ctrlTable::ResetButtonWidths() for(unsigned i = 0; i < columns_.size(); ++i) { auto* button = GetCtrl(i + 1); - button->SetWidth(std::lround(columns_[i].width * sizeFactor)); + button->SetWidth(helpers::iround(columns_[i].width * sizeFactor)); button->SetPos(btPos); btPos.x += button->GetSize().x; } diff --git a/libs/s25main/ingameWindows/iwConnecting.cpp b/libs/s25main/ingameWindows/iwConnecting.cpp index 983602596..a6de212b7 100644 --- a/libs/s25main/ingameWindows/iwConnecting.cpp +++ b/libs/s25main/ingameWindows/iwConnecting.cpp @@ -8,11 +8,11 @@ #include "controls/ctrlPercent.h" #include "controls/ctrlText.h" #include "desktops/dskGameLobby.h" +#include "helpers/mathFuncs.h" #include "iwMsgbox.h" #include "network/GameClient.h" #include "gameData/const_gui_ids.h" #include "s25util/colors.h" -#include namespace { enum @@ -92,7 +92,7 @@ void iwConnecting::CI_NextConnectState(const ConnectState cs) void iwConnecting::CI_MapPartReceived(uint32_t bytesReceived, uint32_t bytesTotal) { - downloadProgress_ = static_cast(std::lround(bytesReceived * 100. / bytesTotal)); + downloadProgress_ = helpers::iround(bytesReceived * 100. / bytesTotal); } void iwConnecting::setStatus(const std::string& status) diff --git a/libs/s25main/network/GameServer.cpp b/libs/s25main/network/GameServer.cpp index 42d898e78..8bfa1a46e 100644 --- a/libs/s25main/network/GameServer.cpp +++ b/libs/s25main/network/GameServer.cpp @@ -16,6 +16,7 @@ #include "commonDefines.h" #include "files.h" #include "helpers/containerUtils.h" +#include "helpers/mathFuncs.h" #include "helpers/random.h" #include "network/CreateServerInfo.h" #include "network/GameMessages.h" @@ -36,7 +37,6 @@ #include #include #include -#include #include #include #include @@ -763,7 +763,7 @@ void GameServer::ExecuteNWF() using MsDouble = duration; double newNWFLen = framesinfo.nwf_length * framesinfo.gf_length / duration_cast(framesinfo.gfLengthReq); - newInfo.nextNWF = lastNWF + std::max(1l, std::lround(newNWFLen)); + newInfo.nextNWF = lastNWF + std::max(1u, helpers::iround(newNWFLen)); } SendNWFDone(newInfo); } diff --git a/libs/s25main/world/GameWorld.cpp b/libs/s25main/world/GameWorld.cpp index b3791ef8f..2f08ad510 100644 --- a/libs/s25main/world/GameWorld.cpp +++ b/libs/s25main/world/GameWorld.cpp @@ -17,6 +17,7 @@ #include "figures/nofPassiveSoldier.h" #include "figures/nofScout_Free.h" #include "helpers/containerUtils.h" +#include "helpers/mathFuncs.h" #include "helpers/reverse.h" #include "lua/LuaInterfaceGame.h" #include "notifications/BuildingNote.h" @@ -37,7 +38,6 @@ #include "gameData/MilitaryConsts.h" #include "gameData/TerrainDesc.h" #include -#include #include #include #include @@ -1357,8 +1357,8 @@ void GameWorld::PlaceAndFixWater() } } if(minHumidity) - curNodeResource = Resource( - ResourceType::Water, waterEverywhere ? 7 : static_cast(std::lround(minHumidity * 7. / 100.))); + curNodeResource = + Resource(ResourceType::Water, waterEverywhere ? 7 : helpers::iround(minHumidity * 7. / 100.)); else curNodeResource = Resource(ResourceType::Nothing, 0); From 4967a6728e61a8e67e09aa76518e6195a553abce Mon Sep 17 00:00:00 2001 From: Florian Albrechtskirchinger Date: Sat, 8 Jul 2023 09:57:58 +0200 Subject: [PATCH 3/8] Add rounding Point conversion constructor Overload the Point conversion constructor for floating-point to integer element types to use rounding instead of plain casting. --- libs/common/include/Point.h | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/libs/common/include/Point.h b/libs/common/include/Point.h index c08144988..d7371e14e 100644 --- a/libs/common/include/Point.h +++ b/libs/common/include/Point.h @@ -4,6 +4,7 @@ #pragma once +#include "helpers/mathFuncs.h" #include #include #include @@ -24,9 +25,12 @@ struct Point //-V690 T x, y; constexpr Point() noexcept : x(getInvalidValue()), y(getInvalidValue()) {} constexpr Point(const T x, const T y) noexcept : x(x), y(y) {} - template + template::value && std::is_floating_point::value), int> = 0> constexpr explicit Point(const Point& pt) noexcept : x(static_cast(pt.x)), y(static_cast(pt.y)) {} + template::value && std::is_floating_point::value, int> = 0> + constexpr explicit Point(const Point& pt) noexcept : x(helpers::iround(pt.x)), y(helpers::iround(pt.y)) + {} constexpr Point(const Point&) = default; constexpr Point& operator=(const Point&) = default; From ea2a55df5361d461486cf72c6b5ef427f7950918 Mon Sep 17 00:00:00 2001 From: Florian Albrechtskirchinger Date: Sat, 8 Jul 2023 10:06:29 +0200 Subject: [PATCH 4/8] Add a truncating Point conversion constructor Add Truncate_t tag type to select a truncating version of the Point floating-point to integer conversion constructor. --- libs/common/include/Point.h | 12 ++++++++++++ libs/s25main/TerrainRenderer.cpp | 4 ++-- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/libs/common/include/Point.h b/libs/common/include/Point.h index d7371e14e..7bec58f6f 100644 --- a/libs/common/include/Point.h +++ b/libs/common/include/Point.h @@ -22,12 +22,24 @@ struct Point //-V690 using ElementType = T; static_assert(std::is_arithmetic::value, "Requires an arithmetic type"); + struct Truncate_t + { + constexpr explicit Truncate_t() = default; + }; + + static constexpr Truncate_t Truncate{}; + T x, y; constexpr Point() noexcept : x(getInvalidValue()), y(getInvalidValue()) {} constexpr Point(const T x, const T y) noexcept : x(x), y(y) {} template::value && std::is_floating_point::value), int> = 0> constexpr explicit Point(const Point& pt) noexcept : x(static_cast(pt.x)), y(static_cast(pt.y)) {} + /// Convert floating-point to integer by truncating + template::value && std::is_floating_point::value, int> = 0> + constexpr explicit Point(Truncate_t, const Point& pt) noexcept : x(static_cast(pt.x)), y(static_cast(pt.y)) + {} + /// Convert floating-point to integer with rounding (default behavior) template::value && std::is_floating_point::value, int> = 0> constexpr explicit Point(const Point& pt) noexcept : x(helpers::iround(pt.x)), y(helpers::iround(pt.y)) {} diff --git a/libs/s25main/TerrainRenderer.cpp b/libs/s25main/TerrainRenderer.cpp index c9245a86c..8173ed533 100644 --- a/libs/s25main/TerrainRenderer.cpp +++ b/libs/s25main/TerrainRenderer.cpp @@ -886,7 +886,7 @@ void TerrainRenderer::PrepareWaysPoint(PreparedRoads& sorted_roads, const GameWo const Position& offset) const { const WorldDescription& desc = gwViewer.GetWorld().GetDescription(); - Position startPos = Position(GetVertexPos(pt)) + offset; + Position startPos = Position(Position::Truncate, GetVertexPos(pt)) + offset; Visibility visibility = gwViewer.GetVisibility(pt); @@ -902,7 +902,7 @@ void TerrainRenderer::PrepareWaysPoint(PreparedRoads& sorted_roads, const GameWo const Direction targetDir = toDirection(dir); MapPoint ta = gwViewer.GetNeighbour(pt, targetDir); - Position endPos = Position(GetVertexPos(ta)) + offset; + Position endPos = Position(Position::Truncate, GetVertexPos(ta)) + offset; Position diff = startPos - endPos; // Gehen wir über einen Kartenrand (horizontale Richung?) From 87ac1ea8251fd96048260a5fc8c0867bd80e6078 Mon Sep 17 00:00:00 2001 From: Florian Albrechtskirchinger Date: Tue, 11 Jul 2023 12:21:59 +0200 Subject: [PATCH 5/8] Add tests for Point conversion constructors --- tests/common/testPoint.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/common/testPoint.cpp b/tests/common/testPoint.cpp index 5796ced5c..f30802b25 100644 --- a/tests/common/testPoint.cpp +++ b/tests/common/testPoint.cpp @@ -283,3 +283,12 @@ BOOST_AUTO_TEST_CASE(ProdOfComponents) Point ptF(256.5, 256.5); BOOST_TEST(prodOfComponents(ptF) == 256.5f * 256.5f); } + +BOOST_AUTO_TEST_CASE(ConvertFloatToIntPoints) +{ + BOOST_TEST(Point(Point(1.5f, 3.4f)) == Point(2, 3)); + BOOST_TEST(Point(Point::Truncate, Point(1.5f, 3.4f)) == Point(1, 3)); + + BOOST_TEST(Point(Point(-1.5f, -3.4f)) == Point(-2, -3)); + BOOST_TEST(Point(Point::Truncate, Point(-1.5f, -3.4f)) == Point(-1, -3)); +} From 6b657246fbb3947af203fa14caea70e361de38c6 Mon Sep 17 00:00:00 2001 From: Florian Albrechtskirchinger Date: Tue, 18 Jul 2023 19:06:48 +0200 Subject: [PATCH 6/8] Add additional error handling to iround() Check: * is NaN? * is infinity? --- libs/common/include/helpers/mathFuncs.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libs/common/include/helpers/mathFuncs.h b/libs/common/include/helpers/mathFuncs.h index 89b926d31..f2068c529 100644 --- a/libs/common/include/helpers/mathFuncs.h +++ b/libs/common/include/helpers/mathFuncs.h @@ -88,6 +88,9 @@ template::value && std::is_floating_point::value, int> = 0> inline IntType iround(const FloatType val) noexcept { + RTTR_Assert(!std::isnan(val)); + RTTR_Assert(!std::isinf(val)); + if(std::is_unsigned::value) RTTR_Assert_Msg(!(val < 0), "Floating-point value must not be negative when casting to unsigned integer type."); From f872adcb0522c0346a4875373c5a38a39af0d02e Mon Sep 17 00:00:00 2001 From: Florian Albrechtskirchinger Date: Wed, 19 Jul 2023 14:06:23 +0200 Subject: [PATCH 7/8] Simplify error checks in iround() Co-authored-by: Alexander Grund --- libs/common/include/helpers/mathFuncs.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/libs/common/include/helpers/mathFuncs.h b/libs/common/include/helpers/mathFuncs.h index f2068c529..33c715257 100644 --- a/libs/common/include/helpers/mathFuncs.h +++ b/libs/common/include/helpers/mathFuncs.h @@ -88,8 +88,7 @@ template::value && std::is_floating_point::value, int> = 0> inline IntType iround(const FloatType val) noexcept { - RTTR_Assert(!std::isnan(val)); - RTTR_Assert(!std::isinf(val)); + RTTR_Assert(std::isfinite(val)); if(std::is_unsigned::value) RTTR_Assert_Msg(!(val < 0), "Floating-point value must not be negative when casting to unsigned integer type."); From d712cf588707cf1326ea44389bc59de8432b04c2 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Wed, 19 Jul 2023 14:12:58 +0200 Subject: [PATCH 8/8] Remove redundant `inline` --- libs/common/include/helpers/mathFuncs.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/common/include/helpers/mathFuncs.h b/libs/common/include/helpers/mathFuncs.h index 33c715257..1aa6d351a 100644 --- a/libs/common/include/helpers/mathFuncs.h +++ b/libs/common/include/helpers/mathFuncs.h @@ -86,7 +86,7 @@ constexpr T inverseLerp(const T startVal, const T endVal, const T value) noexcep /// Arithmetically round floating point values to integers template::value && std::is_floating_point::value, int> = 0> -inline IntType iround(const FloatType val) noexcept +IntType iround(const FloatType val) noexcept { RTTR_Assert(std::isfinite(val));