From 2abcc454b0ccce28301337d7f34e4fe1f36dafe1 Mon Sep 17 00:00:00 2001 From: Brad Hefta-Gaub Date: Thu, 21 Apr 2016 15:48:18 -0700 Subject: [PATCH] CR feedback --- libraries/networking/src/Node.cpp | 2 +- libraries/shared/src/MovingPercentile.cpp | 2 +- libraries/shared/src/MovingPercentile.h | 10 +- libraries/shared/src/SharedUtil.cpp | 112 ++++++++++++---------- libraries/shared/src/SharedUtil.h | 8 +- 5 files changed, 69 insertions(+), 65 deletions(-) diff --git a/libraries/networking/src/Node.cpp b/libraries/networking/src/Node.cpp index 66f0a028ad..e4fe292223 100644 --- a/libraries/networking/src/Node.cpp +++ b/libraries/networking/src/Node.cpp @@ -74,7 +74,7 @@ void Node::setType(char type) { } void Node::updateClockSkewUsec(qint64 clockSkewSample) { - _clockSkewMovingPercentile.updatePercentile((double)clockSkewSample); + _clockSkewMovingPercentile.updatePercentile(clockSkewSample); _clockSkewUsec = (quint64)_clockSkewMovingPercentile.getValueAtPercentile(); } diff --git a/libraries/shared/src/MovingPercentile.cpp b/libraries/shared/src/MovingPercentile.cpp index 90f2bbc290..5bcdbb5e80 100644 --- a/libraries/shared/src/MovingPercentile.cpp +++ b/libraries/shared/src/MovingPercentile.cpp @@ -21,7 +21,7 @@ MovingPercentile::MovingPercentile(int numSamples, float percentile) { } -void MovingPercentile::updatePercentile(quint64 sample) { +void MovingPercentile::updatePercentile(qint64 sample) { // insert the new sample into _samplesSorted int newSampleIndex; diff --git a/libraries/shared/src/MovingPercentile.h b/libraries/shared/src/MovingPercentile.h index 72de9b125c..bc52c4b4e7 100644 --- a/libraries/shared/src/MovingPercentile.h +++ b/libraries/shared/src/MovingPercentile.h @@ -18,21 +18,19 @@ class MovingPercentile { public: MovingPercentile(int numSamples, float percentile = 0.5f); - // FIXME - this class is only currently used in calculating the clockSkew, which is a signed 64bit int, not a double - // I am somewhat tempted to make this a type sensitive template and/or swith to using qint64's internally - void updatePercentile(quint64 sample); - quint64 getValueAtPercentile() const { return _valueAtPercentile; } + void updatePercentile(qint64 sample); + qint64 getValueAtPercentile() const { return _valueAtPercentile; } private: const int _numSamples; const float _percentile; - QList _samplesSorted; + QList _samplesSorted; QList _sampleIds; // incrementally assigned, is cyclic int _newSampleId; int _indexOfPercentile; - quint64 _valueAtPercentile; + qint64 _valueAtPercentile; }; #endif diff --git a/libraries/shared/src/SharedUtil.cpp b/libraries/shared/src/SharedUtil.cpp index 580f23c0d9..7925c8ad4e 100644 --- a/libraries/shared/src/SharedUtil.cpp +++ b/libraries/shared/src/SharedUtil.cpp @@ -19,6 +19,9 @@ #include #include +#include + + #ifdef _WIN32 #include #endif @@ -621,72 +624,75 @@ void debug::checkDeadBeef(void* memoryVoid, int size) { assert(memcmp((unsigned char*)memoryVoid, DEADBEEF, std::min(size, DEADBEEF_SIZE)) != 0); } -QString formatUsecTime(quint64 usecs, int prec) { - static const quint64 USECS_PER_MINUTE = USECS_PER_SECOND * 60; - static const quint64 USECS_PER_HOUR = USECS_PER_MINUTE * 60; - static const quint64 TWO_HOURS = USECS_PER_HOUR * 2; + +// glm::abs() works for signed or unsigned types +template +QString formatUsecTime(T usecs) { + static const int PRECISION = 3; + static const int FRACTION_MASK = pow(10, PRECISION); + + static const T USECS_PER_MSEC = 1000; + static const T USECS_PER_SECOND = 1000 * USECS_PER_MSEC; + static const T USECS_PER_MINUTE = USECS_PER_SECOND * 60; + static const T USECS_PER_HOUR = USECS_PER_MINUTE * 60; QString result; - if (usecs > TWO_HOURS) { - result = QString::number(usecs / USECS_PER_HOUR) + "hrs"; - } else if (usecs > USECS_PER_MINUTE) { - result = QString::number(usecs / USECS_PER_MINUTE) + "min"; - } else if (usecs > USECS_PER_SECOND) { - result = QString::number(usecs / USECS_PER_SECOND) + 's'; - } else if (usecs > USECS_PER_MSEC) { - result = QString::number(usecs / USECS_PER_MSEC) + "ms"; + if (glm::abs(usecs) > USECS_PER_HOUR) { + if (std::is_integral::value) { + result = QString::number(usecs / USECS_PER_HOUR); + result += "." + QString::number(((int)(usecs * FRACTION_MASK / USECS_PER_HOUR)) % FRACTION_MASK); + } else { + result = QString::number(usecs / USECS_PER_HOUR, 'f', PRECISION); + } + result += " hrs"; + } else if (glm::abs(usecs) > USECS_PER_MINUTE) { + if (std::is_integral::value) { + result = QString::number(usecs / USECS_PER_MINUTE); + result += "." + QString::number(((int)(usecs * FRACTION_MASK / USECS_PER_MINUTE)) % FRACTION_MASK); + } else { + result = QString::number(usecs / USECS_PER_MINUTE, 'f', PRECISION); + } + result += " mins"; + } else if (glm::abs(usecs) > USECS_PER_SECOND) { + if (std::is_integral::value) { + result = QString::number(usecs / USECS_PER_SECOND); + result += "." + QString::number(((int)(usecs * FRACTION_MASK / USECS_PER_SECOND)) % FRACTION_MASK); + } else { + result = QString::number(usecs / USECS_PER_SECOND, 'f', PRECISION); + } + result += " secs"; + } else if (glm::abs(usecs) > USECS_PER_MSEC) { + if (std::is_integral::value) { + result = QString::number(usecs / USECS_PER_MSEC); + result += "." + QString::number(((int)(usecs * FRACTION_MASK / USECS_PER_MSEC)) % FRACTION_MASK); + } else { + result = QString::number(usecs / USECS_PER_MSEC, 'f', PRECISION); + } + result += " msecs"; } else { - result = QString::number(usecs) + "us"; + result = QString::number(usecs) + " usecs"; } return result; } -QString formatUsecTime(qint64 usecs, int prec) { - static const qint64 USECS_PER_MSEC = 1000; - static const qint64 USECS_PER_SECOND = 1000 * USECS_PER_MSEC; - static const qint64 USECS_PER_MINUTE = USECS_PER_SECOND * 60; - static const qint64 USECS_PER_HOUR = USECS_PER_MINUTE * 60; - static const qint64 TWO_HOURS = USECS_PER_HOUR * 2; - QString result; - if (usecs > TWO_HOURS || usecs < -TWO_HOURS) { - result = QString::number(usecs / USECS_PER_HOUR) + "hrs"; - } else if (usecs > USECS_PER_MINUTE || usecs < -USECS_PER_MINUTE) { - result = QString::number(usecs / USECS_PER_MINUTE) + "min"; - } else if (usecs > USECS_PER_SECOND || usecs < -USECS_PER_SECOND) { - result = QString::number(usecs / USECS_PER_SECOND) + 's'; - } else if (usecs > USECS_PER_MSEC || usecs < -USECS_PER_MSEC) { - result = QString::number(usecs / USECS_PER_MSEC) + "ms"; - } else { - result = QString::number(usecs) + "us"; - } - return result; + +QString formatUsecTime(quint64 usecs) { + return formatUsecTime(usecs); } -QString formatUsecTime(float usecs, int prec) { - return formatUsecTime((double)usecs, prec); +QString formatUsecTime(qint64 usecs) { + return formatUsecTime(usecs); } -QString formatUsecTime(double usecs, int prec) { - static const double USECS_PER_MSEC = 1000.0; - static const double USECS_PER_SECOND = 1000.0 * USECS_PER_MSEC; - static const double USECS_PER_MINUTE = USECS_PER_SECOND * 60.0; - static const double USECS_PER_HOUR = USECS_PER_MINUTE * 60.0; - static const double TWO_HOURS = USECS_PER_HOUR * 2; - QString result; - if (usecs > TWO_HOURS || usecs < -TWO_HOURS) { - result = QString::number(usecs / USECS_PER_HOUR, 'f', prec) + "hrs"; - } else if (usecs > USECS_PER_MINUTE || usecs < -USECS_PER_MINUTE) { - result = QString::number(usecs / USECS_PER_MINUTE, 'f', prec) + "min"; - } else if (usecs > USECS_PER_SECOND || usecs < -USECS_PER_SECOND) { - result = QString::number(usecs / USECS_PER_SECOND, 'f', prec) + 's'; - } else if (usecs > USECS_PER_MSEC || usecs < -USECS_PER_MSEC) { - result = QString::number(usecs / USECS_PER_MSEC, 'f', prec) + "ms"; - } else { - result = QString::number(usecs, 'f', prec) + "us"; - } - return result; +QString formatUsecTime(float usecs) { + return formatUsecTime(usecs); } +QString formatUsecTime(double usecs) { + return formatUsecTime(usecs); +} + + QString formatSecondsElapsed(float seconds) { QString result; diff --git a/libraries/shared/src/SharedUtil.h b/libraries/shared/src/SharedUtil.h index 62173dae43..042396f474 100644 --- a/libraries/shared/src/SharedUtil.h +++ b/libraries/shared/src/SharedUtil.h @@ -186,10 +186,10 @@ inline bool isBetween(int64_t value, int64_t max, int64_t min) { return ((value /// \return bool is the float NaN inline bool isNaN(float value) { return value != value; } -QString formatUsecTime(float usecs, int prec = 3); -QString formatUsecTime(double usecs, int prec = 3); -QString formatUsecTime(quint64 usecs, int prec = 3); -QString formatUsecTime(qint64 usecs, int prec = 3); +QString formatUsecTime(float usecs); +QString formatUsecTime(double usecs); +QString formatUsecTime(quint64 usecs); +QString formatUsecTime(qint64 usecs); QString formatSecondsElapsed(float seconds); bool similarStrings(const QString& stringA, const QString& stringB);