From 8a54d25a4672b69a7d7ebcf7573d850df7177586 Mon Sep 17 00:00:00 2001 From: Ivan Solovev Date: Fri, 19 Apr 2024 17:49:03 +0200 Subject: [PATCH] Fix QMarginsF::operator==() for zero margins The (in)equality operators for QMarginsF are documented to use fuzzy comparison. However, the implementation unconditionally used qFuzzyCompare which is incorrect for the case of 0 margins. Update the implementation to use a combination of qFuzzyIsNull and qFuzzyCompare, like it's done for QPointF. [ChangeLog][QtCore][QMarginsF] Fixed a bug when equality comparison returned incorrect results if one of the margins was zero. Task-number: QTBUG-120308 Change-Id: I6e785fe8b523f6aa5f5317fb75877fdbf4e086c2 Reviewed-by: Tatiana Borisova --- src/corelib/tools/qmargins.h | 15 ++++-- .../corelib/tools/qmargins/CMakeLists.txt | 2 + .../corelib/tools/qmargins/tst_qmargins.cpp | 46 +++++++++++++++++++ 3 files changed, 59 insertions(+), 4 deletions(-) diff --git a/src/corelib/tools/qmargins.h b/src/corelib/tools/qmargins.h index f8d7150dfdc..63c0b2a8e23 100644 --- a/src/corelib/tools/qmargins.h +++ b/src/corelib/tools/qmargins.h @@ -304,18 +304,25 @@ private: qreal m_right; qreal m_bottom; + QT_WARNING_PUSH + QT_WARNING_DISABLE_FLOAT_COMPARE friend constexpr inline bool operator==(const QMarginsF &lhs, const QMarginsF &rhs) noexcept { - return qFuzzyCompare(lhs.left(), rhs.left()) - && qFuzzyCompare(lhs.top(), rhs.top()) - && qFuzzyCompare(lhs.right(), rhs.right()) - && qFuzzyCompare(lhs.bottom(), rhs.bottom()); + return ((!lhs.m_left || !rhs.m_left) ? qFuzzyIsNull(lhs.m_left - rhs.m_left) + : qFuzzyCompare(lhs.m_left, rhs.m_left)) + && ((!lhs.m_top || !rhs.m_top) ? qFuzzyIsNull(lhs.m_top - rhs.m_top) + : qFuzzyCompare(lhs.m_top, rhs.m_top)) + && ((!lhs.m_right || !rhs.m_right) ? qFuzzyIsNull(lhs.m_right - rhs.m_right) + : qFuzzyCompare(lhs.m_right, rhs.m_right)) + && ((!lhs.m_bottom || !rhs.m_bottom) ? qFuzzyIsNull(lhs.m_bottom - rhs.m_bottom) + : qFuzzyCompare(lhs.m_bottom, rhs.m_bottom)); } friend constexpr inline bool operator!=(const QMarginsF &lhs, const QMarginsF &rhs) noexcept { return !(lhs == rhs); } + QT_WARNING_POP template #include +#include #include Q_DECLARE_METATYPE(QMargins) +constexpr static qreal qreal_min = std::numeric_limits::min(); + class tst_QMargins : public QObject { Q_OBJECT private slots: + void comparison_data(); + void comparison(); + void getSetCheck(); #ifndef QT_NO_DATASTREAM void dataStreamCheck(); @@ -65,6 +71,46 @@ private slots: void toMarginsF(); }; +void tst_QMargins::comparison_data() +{ + QTest::addColumn("lhs"); + QTest::addColumn("rhs"); + QTest::addColumn("result"); + QTest::addColumn("floatResult"); + QTest::addColumn("mixedResult"); + + auto row = [](const QMarginsF &lhs, const QMarginsF &rhs, bool res, bool fRes, bool mRes) { + QString str; + QDebug dbg(&str); + dbg.nospace() << "(" << lhs.left() << ", " << lhs.top() << ", " << lhs.right() << ", " + << lhs.bottom() << ") vs (" << rhs.left() << ", " << rhs.top() << ", " + << rhs.right() << ", " << rhs.bottom() << ")"; + QTest::addRow("%s", str.toLatin1().constData()) << lhs << rhs << res << fRes << mRes; + }; + + row(QMarginsF(0.0, 0.0, 0.0, 0.0), QMarginsF(0.0, 0.0, 0.0, 0.0), true, true, true); + row(QMarginsF(qreal_min, -qreal_min, -qreal_min, qreal_min), QMarginsF(0.0, 0.0, 0.0, 0.0), true, true, true); + row(QMarginsF(1.0, 2.0, 3.0, 4.0), QMarginsF(1.1, 2.1, 2.9, 3.9), true, false, true); + row(QMarginsF(1.5, 2.5, 3.0, 4.0), QMarginsF(1.1, 2.1, 2.9, 3.9), false, false, false); +} + +void tst_QMargins::comparison() +{ + QFETCH(const QMarginsF, lhs); + QFETCH(const QMarginsF, rhs); + QFETCH(const bool, result); + QFETCH(const bool, floatResult); + QFETCH(const bool, mixedResult); + + QT_TEST_EQUALITY_OPS(lhs, rhs, floatResult); + + const QMargins lhsInt = lhs.toMargins(); + const QMargins rhsInt = rhs.toMargins(); + QT_TEST_EQUALITY_OPS(lhsInt, rhsInt, result); + + QT_TEST_EQUALITY_OPS(lhs, rhsInt, mixedResult); +} + // Testing get/set functions void tst_QMargins::getSetCheck() {