From ff8fe2b49864106e39fbd4311d679100261da1c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89milie=20Feral?= Date: Fri, 25 Aug 2017 16:37:13 +0200 Subject: [PATCH] [apps/statistics] Simplify histogram controller and resolve bugs: with very large values, casting in float gives approximate values that trigger bugs Change-Id: I5aac31ad07f267f1b96ee3406b041e527cf247ba --- apps/statistics/histogram_controller.cpp | 18 ++++------- apps/statistics/histogram_controller.h | 1 - apps/statistics/store.cpp | 38 ++++++++++++------------ apps/statistics/store.h | 24 +++++++-------- apps/statistics/store_controller.cpp | 4 +++ 5 files changed, 41 insertions(+), 44 deletions(-) diff --git a/apps/statistics/histogram_controller.cpp b/apps/statistics/histogram_controller.cpp index 0c9ef5c1e..a7895b8fe 100644 --- a/apps/statistics/histogram_controller.cpp +++ b/apps/statistics/histogram_controller.cpp @@ -200,13 +200,13 @@ bool HistogramController::moveSelection(int deltaIndex) { if (deltaIndex > 0) { do { newSelectedBarIndex++; - } while (m_store->heightOfBarAtIndex(newSelectedBarIndex) == 0 && newSelectedBarIndex < displayedNumberOfBars()); + } while (m_store->heightOfBarAtIndex(newSelectedBarIndex) == 0 && newSelectedBarIndex < m_store->numberOfBars()); } else { do { newSelectedBarIndex--; } while (m_store->heightOfBarAtIndex(newSelectedBarIndex) == 0 && newSelectedBarIndex >= 0); } - if (newSelectedBarIndex >= 0 && newSelectedBarIndex < displayedNumberOfBars() && *m_selectedBarIndex != newSelectedBarIndex) { + if (newSelectedBarIndex >= 0 && newSelectedBarIndex < m_store->numberOfBars() && *m_selectedBarIndex != newSelectedBarIndex) { *m_selectedBarIndex = newSelectedBarIndex; m_view.setHighlight(m_store->startOfBarAtIndex(*m_selectedBarIndex), m_store->endOfBarAtIndex(*m_selectedBarIndex)); m_store->scrollToSelectedBarIndex(*m_selectedBarIndex); @@ -232,7 +232,7 @@ void HistogramController::initRangeParameters() { m_store->setXMin(xMin - Store::k_displayLeftMarginRatio*(xMax-xMin)); m_store->setXMax(xMax + Store::k_displayRightMarginRatio*(xMax-xMin)); float yMax = -FLT_MAX; - for (int index = 0; index < displayedNumberOfBars(); index++) { + for (int index = 0; index < m_store->numberOfBars(); index++) { float size = m_store->heightOfBarAtIndex(index); if (size > yMax) { yMax = size; @@ -259,23 +259,17 @@ void HistogramController::initBarParameters() { void HistogramController::initBarSelection() { *m_selectedBarIndex = 0; while ((m_store->heightOfBarAtIndex(*m_selectedBarIndex) == 0 || - m_store->startOfBarAtIndex(*m_selectedBarIndex) < m_store->firstDrawnBarAbscissa()) && *m_selectedBarIndex < displayedNumberOfBars()) { + m_store->startOfBarAtIndex(*m_selectedBarIndex) < m_store->firstDrawnBarAbscissa()) && *m_selectedBarIndex < m_store->numberOfBars()) { *m_selectedBarIndex = *m_selectedBarIndex+1; } - if (*m_selectedBarIndex >= displayedNumberOfBars()) { + if (*m_selectedBarIndex >= m_store->numberOfBars()) { /* No bar is after m_firstDrawnBarAbscissa, so we select the first bar */ *m_selectedBarIndex = 0; - while (m_store->heightOfBarAtIndex(*m_selectedBarIndex) == 0 && *m_selectedBarIndex < displayedNumberOfBars()) { + while (m_store->heightOfBarAtIndex(*m_selectedBarIndex) == 0 && *m_selectedBarIndex < m_store->numberOfBars()) { *m_selectedBarIndex = *m_selectedBarIndex+1; } } m_store->scrollToSelectedBarIndex(*m_selectedBarIndex); } -int HistogramController::displayedNumberOfBars() { - float numberOfBars = m_store->numberOfBars(); - numberOfBars = isnan(numberOfBars) || isinf(numberOfBars) || numberOfBars > Store::k_maxNumberOfBars ? 0 : numberOfBars; - return numberOfBars; -} - } diff --git a/apps/statistics/histogram_controller.h b/apps/statistics/histogram_controller.h index 4826db9d3..84d13a447 100644 --- a/apps/statistics/histogram_controller.h +++ b/apps/statistics/histogram_controller.h @@ -41,7 +41,6 @@ private: void initBarSelection(); // return true if the window has scrolled bool moveSelection(int deltaIndex); - int displayedNumberOfBars(); HistogramBannerView m_bannerView; HistogramView m_view; Button m_settingButton; diff --git a/apps/statistics/store.cpp b/apps/statistics/store.cpp index 2ddd77050..b9c5f2780 100644 --- a/apps/statistics/store.cpp +++ b/apps/statistics/store.cpp @@ -12,57 +12,57 @@ namespace Statistics { Store::Store() : MemoizedCurveViewRange(), FloatPairStore(), - m_barWidth(1.0f), - m_firstDrawnBarAbscissa(0.0f) + m_barWidth(1.0), + m_firstDrawnBarAbscissa(0.0) { } uint32_t Store::barChecksum() { - float data[2] = {m_barWidth, m_firstDrawnBarAbscissa}; - size_t dataLengthInBytes = 2*sizeof(float); + double data[2] = {m_barWidth, m_firstDrawnBarAbscissa}; + size_t dataLengthInBytes = 2*sizeof(double); assert((dataLengthInBytes & 0x3) == 0); // Assert that dataLengthInBytes is a multiple of 4 return Ion::crc32((uint32_t *)data, dataLengthInBytes/sizeof(uint32_t)); } /* Histogram bars */ -float Store::barWidth() { +double Store::barWidth() { return m_barWidth; } -void Store::setBarWidth(float barWidth) { - if (barWidth <= 0.0f) { +void Store::setBarWidth(double barWidth) { + if (barWidth <= 0.0) { return; } m_barWidth = barWidth; } -float Store::firstDrawnBarAbscissa() { +double Store::firstDrawnBarAbscissa() { return m_firstDrawnBarAbscissa; } -void Store::setFirstDrawnBarAbscissa(float firstBarAbscissa) { +void Store::setFirstDrawnBarAbscissa(double firstBarAbscissa) { m_firstDrawnBarAbscissa = firstBarAbscissa; } -float Store::heightOfBarAtIndex(int index) { +double Store::heightOfBarAtIndex(int index) { return sumOfValuesBetween(startOfBarAtIndex(index), endOfBarAtIndex(index)); } -float Store::heightOfBarAtValue(float value) { - float width = barWidth(); +double Store::heightOfBarAtValue(double value) { + double width = barWidth(); int barNumber = std::floor((value - m_firstDrawnBarAbscissa)/width); - float lowerBound = m_firstDrawnBarAbscissa + barNumber*width; - float upperBound = m_firstDrawnBarAbscissa + (barNumber+1)*width; + double lowerBound = m_firstDrawnBarAbscissa + barNumber*width; + double upperBound = m_firstDrawnBarAbscissa + (barNumber+1)*width; return sumOfValuesBetween(lowerBound, upperBound); } -float Store::startOfBarAtIndex(int index) { - float firstBarAbscissa = m_firstDrawnBarAbscissa + m_barWidth*std::floor((minValue()- m_firstDrawnBarAbscissa)/m_barWidth); +double Store::startOfBarAtIndex(int index) { + double firstBarAbscissa = m_firstDrawnBarAbscissa + m_barWidth*std::floor((minValue()- m_firstDrawnBarAbscissa)/m_barWidth); return firstBarAbscissa + index * m_barWidth; } -float Store::endOfBarAtIndex(int index) { +double Store::endOfBarAtIndex(int index) { return startOfBarAtIndex(index+1); } @@ -153,7 +153,7 @@ double Store::median() { if (totalMod2 == 0) { double minusMedian = sortedElementNumber(halfTotal); double maxMedian = sortedElementNumber(halfTotal+1); - return (minusMedian+maxMedian)/2.0f; + return (minusMedian+maxMedian)/2.0; } else { return sortedElementNumber(halfTotal+1); } @@ -184,7 +184,7 @@ double Store::defaultValue(int i) { return 1.0; } -float Store::sumOfValuesBetween(float x1, float x2) { +double Store::sumOfValuesBetween(double x1, double x2) { int result = 0; for (int k = 0; k < m_numberOfPairs; k++) { if (m_data[0][k] < x2 && x1 <= m_data[0][k]) { diff --git a/apps/statistics/store.h b/apps/statistics/store.h index 2894f64ab..aac2f8adf 100644 --- a/apps/statistics/store.h +++ b/apps/statistics/store.h @@ -11,14 +11,14 @@ public: Store(); uint32_t barChecksum(); // Histogram bars - float barWidth(); - void setBarWidth(float barWidth); - float firstDrawnBarAbscissa(); - void setFirstDrawnBarAbscissa(float firstDrawnBarAbscissa); - float heightOfBarAtIndex(int index); - float heightOfBarAtValue(float value); - float startOfBarAtIndex(int index); - float endOfBarAtIndex(int index); + double barWidth(); + void setBarWidth(double barWidth); + double firstDrawnBarAbscissa(); + void setFirstDrawnBarAbscissa(double firstDrawnBarAbscissa); + double heightOfBarAtIndex(int index); + double heightOfBarAtValue(double value); + double startOfBarAtIndex(int index); + double endOfBarAtIndex(int index); double numberOfBars(); // return true if the window has scrolled bool scrollToSelectedBarIndex(int index); @@ -37,19 +37,19 @@ public: double median(); double sum(); double squaredValueSum(); - constexpr static float k_maxNumberOfBars = 10000.0f; + constexpr static double k_maxNumberOfBars = 10000.0; constexpr static float k_displayTopMarginRatio = 0.1f; constexpr static float k_displayRightMarginRatio = 0.04f; constexpr static float k_displayBottomMarginRatio = 0.4f; constexpr static float k_displayLeftMarginRatio = 0.04f; private: double defaultValue(int i) override; - float sumOfValuesBetween(float x1, float x2); + double sumOfValuesBetween(double x1, double x2); double sortedElementNumber(int k); int minIndex(double * bufferValues, int bufferLength); // Histogram bars - float m_barWidth; - float m_firstDrawnBarAbscissa; + double m_barWidth; + double m_firstDrawnBarAbscissa; }; typedef double (Store::*CalculPointer)(); diff --git a/apps/statistics/store_controller.cpp b/apps/statistics/store_controller.cpp index a3fd806cf..0b9a8714a 100644 --- a/apps/statistics/store_controller.cpp +++ b/apps/statistics/store_controller.cpp @@ -3,6 +3,7 @@ #include "../apps_container.h" #include "../constant.h" #include +#include #include using namespace Shared; @@ -34,6 +35,9 @@ HighlightCell * StoreController::titleCells(int index) { } bool StoreController::setDataAtLocation(double floatBody, int columnIndex, int rowIndex) { + if (std::fabs(floatBody) > FLT_MAX) { + return false; + } if (columnIndex == 1) { if (floatBody < 0) { return false;