From 61701509b01e805bc262176b53982d8302a85c80 Mon Sep 17 00:00:00 2001 From: Berthold Stoeger Date: Tue, 30 Aug 2022 17:55:43 +0200 Subject: [PATCH] core: replace IS_FP_SAME macro by inline function No reason to keep this as a macro - a function is easier to read, type safe and easier to debug. Moreover, give it the more appropriate name "nearly_equal()". After all, it precisely does NOT check floating points for equality. Signed-off-by: Berthold Stoeger --- core/parse-xml.c | 4 ++-- core/settings/qPrefDisplay.cpp | 6 +++--- core/settings/qPrefPrivate.cpp | 2 +- core/subsurface-string.h | 9 +++++--- desktop-widgets/downloadfromdivecomputer.cpp | 2 +- mobile-widgets/themeinterface.cpp | 2 +- profile-widget/divecartesianaxis.cpp | 2 +- profile-widget/qmlprofile.cpp | 4 ++-- tests/testunitconversion.cpp | 22 ++++++++++---------- 9 files changed, 28 insertions(+), 25 deletions(-) diff --git a/core/parse-xml.c b/core/parse-xml.c index 91578f8d3..2600d10b7 100644 --- a/core/parse-xml.c +++ b/core/parse-xml.c @@ -161,7 +161,7 @@ static enum number_type parse_float(const char *buffer, double *res, const char if (errno || *endp == buffer) return NEITHER; if (**endp == ',') { - if (IS_FP_SAME(val, rint(val))) { + if (nearly_equal(val, rint(val))) { /* we really want to send an error if this is a Subsurface native file * as this is likely indication of a bug - but right now we don't have * that information available */ @@ -604,7 +604,7 @@ static void fahrenheit(char *buffer, temperature_t *temperature) switch (integer_or_float(buffer, &val)) { case FLOATVAL: - if (IS_FP_SAME(val.fp, 32.0)) + if (nearly_equal(val.fp, 32.0)) break; if (val.fp < 32.0) temperature->mkelvin = C_to_mkelvin(val.fp); diff --git a/core/settings/qPrefDisplay.cpp b/core/settings/qPrefDisplay.cpp index 3ac23b97f..d49826d23 100644 --- a/core/settings/qPrefDisplay.cpp +++ b/core/settings/qPrefDisplay.cpp @@ -96,7 +96,7 @@ void qPrefDisplay::disk_divelist_font(bool doSync) void qPrefDisplay::set_font_size(double value) { - if (!IS_FP_SAME(value, prefs.font_size)) { + if (!nearly_equal(value, prefs.font_size)) { prefs.font_size = value; disk_font_size(true); @@ -121,7 +121,7 @@ void qPrefDisplay::disk_font_size(bool doSync) void qPrefDisplay::set_mobile_scale(double value) { - if (!IS_FP_SAME(value, prefs.mobile_scale)) { + if (!nearly_equal(value, prefs.mobile_scale)) { prefs.mobile_scale = value; disk_mobile_scale(true); @@ -159,7 +159,7 @@ void qPrefDisplay::setCorrectFont() // get the font from the settings or our defaults // respect the system default font size if none is explicitly set QFont defaultFont = qPrefPrivate::propValue(keyFromGroupAndName(group, "divelist_font"), prefs.divelist_font).value(); - if (IS_FP_SAME(system_divelist_default_font_size, -1.0)) { + if (nearly_equal(system_divelist_default_font_size, -1.0)) { prefs.font_size = qApp->font().pointSizeF(); system_divelist_default_font_size = prefs.font_size; // this way we don't save it on exit } diff --git a/core/settings/qPrefPrivate.cpp b/core/settings/qPrefPrivate.cpp index f021027e7..0d07bf17d 100644 --- a/core/settings/qPrefPrivate.cpp +++ b/core/settings/qPrefPrivate.cpp @@ -25,7 +25,7 @@ void qPrefPrivate::propSetValue(const QString &key, const QVariant &value, const #else if (value.isValid() && value.type() == QVariant::Double) #endif - isDefault = IS_FP_SAME(value.toDouble(), defaultValue.toDouble()); + isDefault = nearly_equal(value.toDouble(), defaultValue.toDouble()); else isDefault = (value == defaultValue); diff --git a/core/subsurface-string.h b/core/subsurface-string.h index 15dcbb728..e02cdd337 100644 --- a/core/subsurface-string.h +++ b/core/subsurface-string.h @@ -5,6 +5,7 @@ #include #include #include +#include // shared generic definitions and macros // mostly about strings, but a couple of math macros are here as well @@ -22,13 +23,15 @@ (void) (&_max1 == &_max2); \ _max1 > _max2 ? _max1 : _max2; }) -#define IS_FP_SAME(_a, _b) (fabs((_a) - (_b)) <= 0.000001 * MAX(fabs(_a), fabs(_b))) - - #ifdef __cplusplus extern "C" { #endif +static inline bool nearly_equal(double a, double b) +{ + return fabs(a - b) <= 1e-6 * fmax(fabs(a), fabs(b)); +} + static inline bool nearly_0(double fp) { return fabs(fp) <= 1e-6; diff --git a/desktop-widgets/downloadfromdivecomputer.cpp b/desktop-widgets/downloadfromdivecomputer.cpp index 6a859ff4b..7cdc368b6 100644 --- a/desktop-widgets/downloadfromdivecomputer.cpp +++ b/desktop-widgets/downloadfromdivecomputer.cpp @@ -183,7 +183,7 @@ void DownloadFromDCWidget::updateProgressBar() if (empty_string(last_text)) { // if we get the first actual text after the download is finished // (which happens for example on the OSTC), then don't bother - if (!empty_string(progress_bar_text) && IS_FP_SAME(progress_bar_fraction, 1.0)) + if (!empty_string(progress_bar_text) && nearly_equal(progress_bar_fraction, 1.0)) progress_bar_text = ""; } if (!empty_string(progress_bar_text)) { diff --git a/mobile-widgets/themeinterface.cpp b/mobile-widgets/themeinterface.cpp index d3ac51960..af633cb6f 100644 --- a/mobile-widgets/themeinterface.cpp +++ b/mobile-widgets/themeinterface.cpp @@ -81,7 +81,7 @@ double ThemeInterface::currentScale() void ThemeInterface::set_currentScale(double newScale) { - if (!IS_FP_SAME(newScale, qPrefDisplay::mobile_scale())) { + if (!nearly_equal(newScale, qPrefDisplay::mobile_scale())) { qPrefDisplay::set_mobile_scale(newScale); emit currentScaleChanged(); m_needSignals = true; diff --git a/profile-widget/divecartesianaxis.cpp b/profile-widget/divecartesianaxis.cpp index 541d4fbf3..33cf492e5 100644 --- a/profile-widget/divecartesianaxis.cpp +++ b/profile-widget/divecartesianaxis.cpp @@ -442,7 +442,7 @@ double DiveCartesianAxis::posAtValue(double value, double max, double min) const double screenFrom = position == Position::Bottom ? m.x1() : m.y1(); double screenTo = position == Position::Bottom ? m.x2() : m.y2(); - if (IS_FP_SAME(min, max)) + if (nearly_equal(min, max)) return (screenFrom + screenTo) / 2.0; if ((position == Position::Bottom) == inverted) std::swap(screenFrom, screenTo); diff --git a/profile-widget/qmlprofile.cpp b/profile-widget/qmlprofile.cpp index 35c720967..0ff3ce13e 100644 --- a/profile-widget/qmlprofile.cpp +++ b/profile-widget/qmlprofile.cpp @@ -96,7 +96,7 @@ void QMLProfile::setDevicePixelRatio(qreal dpr) // don't update the profile here, have the user update x and y and then manually trigger an update void QMLProfile::setXOffset(qreal value) { - if (IS_FP_SAME(value, m_xOffset)) + if (nearly_equal(value, m_xOffset)) return; m_xOffset = value; emit xOffsetChanged(); @@ -105,7 +105,7 @@ void QMLProfile::setXOffset(qreal value) // don't update the profile here, have the user update x and y and then manually trigger an update void QMLProfile::setYOffset(qreal value) { - if (IS_FP_SAME(value, m_yOffset)) + if (nearly_equal(value, m_yOffset)) return; m_yOffset = value; emit yOffsetChanged(); diff --git a/tests/testunitconversion.cpp b/tests/testunitconversion.cpp index a14240ea3..a0cc547dc 100644 --- a/tests/testunitconversion.cpp +++ b/tests/testunitconversion.cpp @@ -5,23 +5,23 @@ void TestUnitConversion::testUnitConversions() { - QCOMPARE(IS_FP_SAME(grams_to_lbs(1000), 2.204586), true); + QCOMPARE(nearly_equal(grams_to_lbs(1000), 2.204586), true); QCOMPARE(lbs_to_grams(1), 454); - QCOMPARE(IS_FP_SAME(ml_to_cuft(1000), 0.0353147), true); - QCOMPARE(IS_FP_SAME(cuft_to_l(1), 28.316847), true); - QCOMPARE(IS_FP_SAME(mm_to_feet(1000), 3.280840), true); + QCOMPARE(nearly_equal(ml_to_cuft(1000), 0.0353147), true); + QCOMPARE(nearly_equal(cuft_to_l(1), 28.316847), true); + QCOMPARE(nearly_equal(mm_to_feet(1000), 3.280840), true); QCOMPARE(feet_to_mm(1), 305L); QCOMPARE(to_feet((depth_t){1000}), 3); - QCOMPARE(IS_FP_SAME(mkelvin_to_C(647000), 373.85), true); - QCOMPARE(IS_FP_SAME(mkelvin_to_F(647000), 704.93), true); + QCOMPARE(nearly_equal(mkelvin_to_C(647000), 373.85), true); + QCOMPARE(nearly_equal(mkelvin_to_F(647000), 704.93), true); QCOMPARE(F_to_mkelvin(704.93), 647000UL); QCOMPARE(C_to_mkelvin(373.85), 647000UL); - QCOMPARE(IS_FP_SAME(psi_to_bar(14.6959488), 1.01325), true); + QCOMPARE(nearly_equal(psi_to_bar(14.6959488), 1.01325), true); QCOMPARE(psi_to_mbar(14.6959488), 1013L); - QCOMPARE(IS_FP_SAME(to_PSI((pressure_t){1013}), 14.6923228594), true); - QCOMPARE(IS_FP_SAME(bar_to_atm(1.013), 1.0), true); - QCOMPARE(IS_FP_SAME(mbar_to_atm(1013), 1.0), true); - QCOMPARE(IS_FP_SAME(mbar_to_PSI(1013), 14.6923228594), true); + QCOMPARE(nearly_equal(to_PSI((pressure_t){1013}), 14.6923228594), true); + QCOMPARE(nearly_equal(bar_to_atm(1.013), 1.0), true); + QCOMPARE(nearly_equal(mbar_to_atm(1013), 1.0), true); + QCOMPARE(nearly_equal(mbar_to_PSI(1013), 14.6923228594), true); } QTEST_GUILESS_MAIN(TestUnitConversion)