From 09dd19df5c4b708960e5aade568eb15d996ef200 Mon Sep 17 00:00:00 2001 From: Mitch Curtis Date: Wed, 1 Aug 2012 16:02:10 +0200 Subject: [PATCH] Do not consider sign in qIsNull. The current implementation of qIsNull only returns true if the value is positive zero. This behaviour is not useful for use cases like QPointF::isNull, where QPointF(-0, -0).isNull() will return false. There doesn't seem to be a reason why the function exhibits this behaviour (-0.0 is not accounted for in the unit tests), and for the case of QSizeF::scale it causes a bug: qIsNull is used to check for division by 0.0 before it proceeds, which fails in the case of -0.0. Task-number: QTBUG-7303 Change-Id: I767e5280bd26614e8e78ae62b274eb9bc4ade385 Reviewed-by: Lars Knoll Reviewed-by: Gunnar Sletta --- src/corelib/global/qglobal.h | 8 +++--- tests/auto/corelib/global/qglobal/tst_qglobal.cpp | 6 +++++ tests/auto/corelib/tools/qpointf/tst_qpointf.cpp | 5 ++++ tests/auto/corelib/tools/qsizef/tst_qsizef.cpp | 31 ++++++++++++++++++++++ .../gui/math3d/qquaternion/tst_qquaternion.cpp | 7 +++++ tests/auto/gui/math3d/qvectornd/tst_qvectornd.cpp | 18 +++++++++++++ 6 files changed, 71 insertions(+), 4 deletions(-) diff --git a/src/corelib/global/qglobal.h b/src/corelib/global/qglobal.h index 546c6cf..2acff12 100644 --- a/src/corelib/global/qglobal.h +++ b/src/corelib/global/qglobal.h @@ -813,7 +813,7 @@ Q_DECL_CONSTEXPR static inline bool qFuzzyIsNull(float f) /* This function tests a double for a null value. It doesn't check whether the actual value is 0 or close to 0, but whether - it is binary 0. + it is binary 0, disregarding sign. */ static inline bool qIsNull(double d) { @@ -823,13 +823,13 @@ static inline bool qIsNull(double d) }; U val; val.d = d; - return val.u == quint64(0); + return (val.u & Q_UINT64_C(0x7fffffffffffffff)) == 0; } /* This function tests a float for a null value. It doesn't check whether the actual value is 0 or close to 0, but whether - it is binary 0. + it is binary 0, disregarding sign. */ static inline bool qIsNull(float f) { @@ -839,7 +839,7 @@ static inline bool qIsNull(float f) }; U val; val.f = f; - return val.u == 0u; + return (val.u & 0x7fffffff) == 0; } /* diff --git a/tests/auto/corelib/global/qglobal/tst_qglobal.cpp b/tests/auto/corelib/global/qglobal/tst_qglobal.cpp index 529bafa..e8dd433 100644 --- a/tests/auto/corelib/global/qglobal/tst_qglobal.cpp +++ b/tests/auto/corelib/global/qglobal/tst_qglobal.cpp @@ -72,6 +72,12 @@ void tst_QGlobal::qIsNull() QVERIFY(!::qIsNull(d)); QVERIFY(!::qIsNull(f)); + + d = -0.0; + f = -0.0f; + + QVERIFY(::qIsNull(d)); + QVERIFY(::qIsNull(f)); } void tst_QGlobal::for_each() diff --git a/tests/auto/corelib/tools/qpointf/tst_qpointf.cpp b/tests/auto/corelib/tools/qpointf/tst_qpointf.cpp index d949de5..df8c7d1 100644 --- a/tests/auto/corelib/tools/qpointf/tst_qpointf.cpp +++ b/tests/auto/corelib/tools/qpointf/tst_qpointf.cpp @@ -111,6 +111,11 @@ void tst_QPointF::isNull() QVERIFY(!point.isNull()); point.rx() -= 2; QVERIFY(!point.isNull()); + + QPointF nullNegativeZero(qreal(-0.0), qreal(-0.0)); + QCOMPARE(nullNegativeZero.x(), (qreal)-0.0f); + QCOMPARE(nullNegativeZero.y(), (qreal)-0.0f); + QVERIFY(nullNegativeZero.isNull()); } void tst_QPointF::manhattanLength_data() diff --git a/tests/auto/corelib/tools/qsizef/tst_qsizef.cpp b/tests/auto/corelib/tools/qsizef/tst_qsizef.cpp index a098abe..67c8c4c 100644 --- a/tests/auto/corelib/tools/qsizef/tst_qsizef.cpp +++ b/tests/auto/corelib/tools/qsizef/tst_qsizef.cpp @@ -48,6 +48,9 @@ class tst_QSizeF : public QObject { Q_OBJECT private slots: + void isNull_data(); + void isNull(); + void scale(); void expandedTo(); @@ -60,6 +63,34 @@ private slots: void transpose(); }; +void tst_QSizeF::isNull_data() +{ + QTest::addColumn("width"); + QTest::addColumn("height"); + QTest::addColumn("isNull"); + + QTest::newRow("0, 0") << qreal(0.0) << qreal(0.0) << true; + QTest::newRow("-0, -0") << qreal(-0.0) << qreal(-0.0) << true; + QTest::newRow("0, -0") << qreal(0) << qreal(-0.0) << true; + QTest::newRow("-0, 0") << qreal(-0.0) << qreal(0) << true; + QTest::newRow("-0.1, 0") << qreal(-0.1) << qreal(0) << false; + QTest::newRow("0, -0.1") << qreal(0) << qreal(-0.1) << false; + QTest::newRow("0.1, 0") << qreal(0.1) << qreal(0) << false; + QTest::newRow("0, 0.1") << qreal(0) << qreal(0.1) << false; +} + +void tst_QSizeF::isNull() +{ + QFETCH(qreal, width); + QFETCH(qreal, height); + QFETCH(bool, isNull); + + QSizeF size(width, height); + QCOMPARE(size.width(), width); + QCOMPARE(size.height(), height); + QCOMPARE(size.isNull(), isNull); +} + void tst_QSizeF::scale() { QSizeF t1(10.4, 12.8); t1.scale(60.6, 60.6, Qt::IgnoreAspectRatio); diff --git a/tests/auto/gui/math3d/qquaternion/tst_qquaternion.cpp b/tests/auto/gui/math3d/qquaternion/tst_qquaternion.cpp index a4e1f94..f42cc30 100644 --- a/tests/auto/gui/math3d/qquaternion/tst_qquaternion.cpp +++ b/tests/auto/gui/math3d/qquaternion/tst_qquaternion.cpp @@ -116,6 +116,13 @@ void tst_QQuaternion::create() QCOMPARE(identity.scalar(), (qreal)1.0f); QVERIFY(identity.isIdentity()); + QQuaternion negativeZeroIdentity(qreal(1.0), qreal(-0.0), qreal(-0.0), qreal(-0.0)); + QCOMPARE(negativeZeroIdentity.x(), qreal(-0.0)); + QCOMPARE(negativeZeroIdentity.y(), qreal(-0.0)); + QCOMPARE(negativeZeroIdentity.z(), qreal(-0.0)); + QCOMPARE(negativeZeroIdentity.scalar(), qreal(1.0)); + QVERIFY(negativeZeroIdentity.isIdentity()); + QQuaternion v1(34.0f, 1.0f, 2.5f, -89.25f); QCOMPARE(v1.x(), (qreal)1.0f); QCOMPARE(v1.y(), (qreal)2.5f); diff --git a/tests/auto/gui/math3d/qvectornd/tst_qvectornd.cpp b/tests/auto/gui/math3d/qvectornd/tst_qvectornd.cpp index 72a4ece..4b6d983 100644 --- a/tests/auto/gui/math3d/qvectornd/tst_qvectornd.cpp +++ b/tests/auto/gui/math3d/qvectornd/tst_qvectornd.cpp @@ -162,6 +162,11 @@ void tst_QVectorND::create2() QCOMPARE(null.y(), (qreal)0.0f); QVERIFY(null.isNull()); + QVector2D nullNegativeZero(qreal(-0.0), qreal(-0.0)); + QCOMPARE(nullNegativeZero.x(), (qreal)-0.0f); + QCOMPARE(nullNegativeZero.y(), (qreal)-0.0f); + QVERIFY(nullNegativeZero.isNull()); + QVector2D v1(1.0f, 2.5f); QCOMPARE(v1.x(), (qreal)1.0f); QCOMPARE(v1.y(), (qreal)2.5f); @@ -252,6 +257,12 @@ void tst_QVectorND::create3() QCOMPARE(null.z(), (qreal)0.0f); QVERIFY(null.isNull()); + QVector3D nullNegativeZero(qreal(-0.0), qreal(-0.0), qreal(-0.0)); + QCOMPARE(nullNegativeZero.x(), (qreal)-0.0f); + QCOMPARE(nullNegativeZero.y(), (qreal)-0.0f); + QCOMPARE(nullNegativeZero.z(), (qreal)-0.0f); + QVERIFY(nullNegativeZero.isNull()); + QVector3D v1(1.0f, 2.5f, -89.25f); QCOMPARE(v1.x(), (qreal)1.0f); QCOMPARE(v1.y(), (qreal)2.5f); @@ -379,6 +390,13 @@ void tst_QVectorND::create4() QCOMPARE(null.w(), (qreal)0.0f); QVERIFY(null.isNull()); + QVector4D nullNegativeZero(qreal(-0.0), qreal(-0.0), qreal(-0.0), qreal(-0.0)); + QCOMPARE(nullNegativeZero.x(), (qreal)-0.0f); + QCOMPARE(nullNegativeZero.y(), (qreal)-0.0f); + QCOMPARE(nullNegativeZero.z(), (qreal)-0.0f); + QCOMPARE(nullNegativeZero.w(), (qreal)-0.0f); + QVERIFY(nullNegativeZero.isNull()); + QVector4D v1(1.0f, 2.5f, -89.25f, 34.0f); QCOMPARE(v1.x(), (qreal)1.0f); QCOMPARE(v1.y(), (qreal)2.5f); -- 2.7.4