From daacc5c6c2297b7cd8ebda3c9d05003a040d54ca Mon Sep 17 00:00:00 2001 From: Hendiadyoin1 Date: Tue, 25 Jul 2023 17:53:30 +0200 Subject: [PATCH] AK: Rename AK::FixedPoint::round to rint and fix a rounding error `rint` is a more accurate name for the roudning mode as the fixme above stated --- AK/FixedPoint.h | 14 +++++++++----- Tests/AK/TestFixedPoint.cpp | 21 +++++++++++++-------- Tests/LibEDID/TestEDID.cpp | 8 ++++---- 3 files changed, 26 insertions(+), 17 deletions(-) diff --git a/AK/FixedPoint.h b/AK/FixedPoint.h index 428298386f..2dcb78e13f 100644 --- a/AK/FixedPoint.h +++ b/AK/FixedPoint.h @@ -99,17 +99,21 @@ public: return *this; } - // Note/FIXME: This uses round to nearest break-tie to even - // Not break-tie away from 0 as the C99's round does - constexpr This round() const + constexpr This rint() const { + // Note: Round fair, break tie to even Underlying value = m_value >> precision; + + // Note: For negative numbers the ordering are reversed, + // and they were already decremented by the shift, so we need to + // add 1 when we see a fract values behind the `.5`s place set, + // because that means they are smaller than .5 // fract(m_value) >= .5? if (m_value & (1u << (precision - 1))) { // fract(m_value) > .5? if (m_value & (radix_mask >> 2u)) { // yes: round "up"; - value += (m_value > 0 ? 1 : -1); + value += 1; } else { // no: round to even; value += value & 1; @@ -134,7 +138,7 @@ public: : 0)); } - constexpr Underlying lround() const { return round().raw() >> precision; } + constexpr Underlying lrint() const { return rint().raw() >> precision; } constexpr Underlying lfloor() const { return m_value >> precision; } constexpr Underlying lceil() const { diff --git a/Tests/AK/TestFixedPoint.cpp b/Tests/AK/TestFixedPoint.cpp index b11c8d6ee7..5622f56ec4 100644 --- a/Tests/AK/TestFixedPoint.cpp +++ b/Tests/AK/TestFixedPoint.cpp @@ -38,46 +38,51 @@ TEST_CASE(arithmetic) TEST_CASE(rounding) { - EXPECT_EQ(Type(0.5).round(), Type(0)); + EXPECT_EQ(Type(0.5).rint(), Type(0)); EXPECT_EQ(Type(0.5).floor(), Type(0)); EXPECT_EQ(Type(0.5).ceil(), Type(1)); EXPECT_EQ(Type(0.75).trunc(), Type(0)); - EXPECT_EQ(Type(1.5).round(), Type(2)); + EXPECT_EQ(Type(1.5).rint(), Type(2)); EXPECT_EQ(Type(1.5).floor(), Type(1)); EXPECT_EQ(Type(1.5).ceil(), Type(2)); EXPECT_EQ(Type(1.25).trunc(), Type(1)); - EXPECT_EQ(Type(-0.5).round(), Type(0)); + EXPECT_EQ(Type(-0.5).rint(), Type(0)); EXPECT_EQ(Type(-0.5).floor(), Type(-1)); EXPECT_EQ(Type(-0.5).ceil(), Type(0)); EXPECT_EQ(Type(-0.75).trunc(), Type(0)); - EXPECT_EQ(Type(-1.5).round(), Type(-2)); + EXPECT_EQ(Type(-1.5).rint(), Type(-2)); EXPECT_EQ(Type(-1.5).floor(), Type(-2)); EXPECT_EQ(Type(-1.5).ceil(), Type(-1)); EXPECT_EQ(Type(-1.25).trunc(), Type(-1)); - EXPECT_EQ(Type(0.5).lround(), 0); + EXPECT_EQ(Type(0.5).lrint(), 0); EXPECT_EQ(Type(0.5).lfloor(), 0); EXPECT_EQ(Type(0.5).lceil(), 1); EXPECT_EQ(Type(0.5).ltrunc(), 0); - EXPECT_EQ(Type(1.5).lround(), 2); + EXPECT_EQ(Type(1.5).lrint(), 2); EXPECT_EQ(Type(1.5).lfloor(), 1); EXPECT_EQ(Type(1.5).lceil(), 2); EXPECT_EQ(Type(1.5).ltrunc(), 1); - EXPECT_EQ(Type(-0.5).lround(), 0); + EXPECT_EQ(Type(-0.5).lrint(), 0); EXPECT_EQ(Type(-0.5).lfloor(), -1); EXPECT_EQ(Type(-0.5).lceil(), 0); EXPECT_EQ(Type(-0.5).ltrunc(), 0); - EXPECT_EQ(Type(-1.5).lround(), -2); + EXPECT_EQ(Type(-1.5).lrint(), -2); EXPECT_EQ(Type(-1.5).lfloor(), -2); EXPECT_EQ(Type(-1.5).lceil(), -1); EXPECT_EQ(Type(-1.5).ltrunc(), -1); + EXPECT_EQ(Type(-1.6).rint(), -2); + EXPECT_EQ(Type(-1.4).rint(), -1); + EXPECT_EQ(Type(1.6).rint(), 2); + EXPECT_EQ(Type(1.4).rint(), 1); + // Check that sRGB TRC curve parameters match the s15fixed16 values stored in Gimp's built-in profile. // (This only requires that the FixedPoint<> constructor rounds before truncating to the fixed-point value, // as it should anyways.) diff --git a/Tests/LibEDID/TestEDID.cpp b/Tests/LibEDID/TestEDID.cpp index c944201f6e..ed16597b00 100644 --- a/Tests/LibEDID/TestEDID.cpp +++ b/Tests/LibEDID/TestEDID.cpp @@ -131,7 +131,7 @@ TEST_CASE(edid1) EXPECT(block_id == expected_timings.block_id); EXPECT(detailed_timing.horizontal_addressable_pixels() == expected_timings.width); EXPECT(detailed_timing.vertical_addressable_lines() == expected_timings.height); - EXPECT(detailed_timing.refresh_rate().lround() == expected_timings.refresh_rate); + EXPECT(detailed_timing.refresh_rate().lrint() == expected_timings.refresh_rate); detailed_timings_found++; return IterationDecision::Continue; })); @@ -313,7 +313,7 @@ TEST_CASE(edid2) EXPECT(block_id == expected_timings.block_id); EXPECT(detailed_timing.horizontal_addressable_pixels() == expected_timings.width); EXPECT(detailed_timing.vertical_addressable_lines() == expected_timings.height); - EXPECT(detailed_timing.refresh_rate().lround() == expected_timings.refresh_rate); + EXPECT(detailed_timing.refresh_rate().lrint() == expected_timings.refresh_rate); detailed_timings_found++; return IterationDecision::Continue; })); @@ -430,7 +430,7 @@ TEST_CASE(edid_extension_maps) EXPECT(block_id == expected_timings.block_id); EXPECT(detailed_timing.horizontal_addressable_pixels() == expected_timings.width); EXPECT(detailed_timing.vertical_addressable_lines() == expected_timings.height); - EXPECT(detailed_timing.refresh_rate().lround() == expected_timings.refresh_rate); + EXPECT(detailed_timing.refresh_rate().lrint() == expected_timings.refresh_rate); detailed_timings_found++; return IterationDecision::Continue; })); @@ -479,7 +479,7 @@ TEST_CASE(edid_1_0) EXPECT(block_id == expected_timings.block_id); EXPECT(detailed_timing.horizontal_addressable_pixels() == expected_timings.width); EXPECT(detailed_timing.vertical_addressable_lines() == expected_timings.height); - EXPECT(detailed_timing.refresh_rate().lround() == expected_timings.refresh_rate); + EXPECT(detailed_timing.refresh_rate().lrint() == expected_timings.refresh_rate); detailed_timings_found++; return IterationDecision::Continue; }));