Allow surface rows and cols be in ascending or descending XZ order

This allows adding rows that have smaller Z-value than the
previously added row instead of being forced to insert rows
into the beginning of the data array in these cases.

Task-number: QTRD-2428
Change-Id: I4dc6c5a48a55ca494a2372f917aa7447f61f336e
Reviewed-by: Tomi Korpipää <tomi.korpipaa@digia.com>

5 files changed, 142 insertions, 42 deletions

diff --git a/src/datavisualization/data/qsurfacedataproxy.cpp b/src/datavisualization/data/qsurfacedataproxy.cpp
index 2e66bb5b..f8f2c900 100644
--- a/src/datavisualization/data/qsurfacedataproxy.cpp
+++ b/src/datavisualization/data/qsurfacedataproxy.cpp
@@ -28,8 +28,8 @@ QT_BEGIN_NAMESPACE_DATAVISUALIZATION
  * \brief Base proxy class for Q3DSurface.
  * \since Qt Data Visualization 1.0
  *
- * QSurfaceDataProxy takes care of surface related data handling. The QSurfaceDataProxy handles the data
- * in rows and for this it provides two auxiliary typedefs. QSurfaceDataArray is a QList for
+ * QSurfaceDataProxy takes care of surface related data handling. The QSurfaceDataProxy handles the
+ * data in rows and for this it provides two auxiliary typedefs. QSurfaceDataArray is a QList for
  * controlling the rows. For rows there is a QVector QSurfaceDataRow which contains QSurfaceDataItem
  * objects. See Q3DSurface documentation and basic sample code there how to feed the data for the
  * QSurfaceDataProxy.
@@ -41,17 +41,18 @@ QT_BEGIN_NAMESPACE_DATAVISUALIZATION
  * If you use QSurfaceDataRow pointers to directly modify data after adding the array to the proxy,
  * you must also emit proper signal to make the graph update.
  *
- * To make a sensible surface, the X-value of each successive item in the same row must be greater than the
- * previous item in that row, and the the Z-value of each successive item in a column  must be greater than
- * the previous item in that column.
+ * To make a sensible surface, the X-value of each successive item in all rows must be
+ * either ascending or descending throughout the row.
+ * Similarly, the Z-value of each successive item in all columns must be either ascending or
+ * descending throughout the column.
  *
- * \note In the initial release, only surfaces with straight rows and columns are fully supported. Any row
- * with items that do not have the exact same Z-value or any columns with items that do not have the exact
- * same X-value may get clipped incorrectly if the whole surface doesn't fit to the visible X or Z axis
- * ranges.
+ * \note Currently only surfaces with straight rows and columns are fully supported. Any row
+ * with items that do not have the exact same Z-value or any column with items that do not have
+ * the exact same X-value may get clipped incorrectly if the whole surface doesn't completely fit
+ * in the visible X or Z axis ranges.
  *
  * \note Surfaces with less than two rows or columns are not considered valid surfaces and will
- * not get rendered.
+ * not be rendered.
  *
  * \sa {Qt Data Visualization Data Handling}
  */
diff --git a/src/datavisualization/engine/surface3drenderer.cpp b/src/datavisualization/engine/surface3drenderer.cpp
index 56219104..d3898476 100644
--- a/src/datavisualization/engine/surface3drenderer.cpp
+++ b/src/datavisualization/engine/surface3drenderer.cpp
@@ -581,71 +581,100 @@ QRect Surface3DRenderer::calculateSampleRect(const QSurfaceDataArray &array)
 {
     QRect sampleSpace;
 
-    int rowCount = array.size();
-    int columnCount = array.at(0)->size();
-
-    int i;
-    bool found;
-    float axisMinX = m_axisCacheX.min();
-    float axisMaxX = m_axisCacheX.max();
-    float axisMinZ = m_axisCacheZ.min();
-    float axisMaxZ = m_axisCacheZ.max();
-
+    const int rowCount = array.size();
+    const int columnCount = array.at(0)->size();
+
+    const float axisMinX = m_axisCacheX.min();
+    const float axisMaxX = m_axisCacheX.max();
+    const float axisMinZ = m_axisCacheZ.min();
+    const float axisMaxZ = m_axisCacheZ.max();
+
+    // We assume data is ordered sequentially in rows for X-value and in columns for Z-value.
+    // Determine if data is ascending or descending in each case.
+    const bool ascendingX = array.at(0)->at(0).x() < array.at(0)->at(columnCount - 1).x();
+    const bool ascendingZ = array.at(0)->at(0).z() < array.at(rowCount - 1)->at(0).z();
+
+    const int minCol = ascendingX ? 0 : columnCount - 1;
+    const int maxCol = ascendingX ? columnCount - 1 : 0;
+    const int colStep = ascendingX ? 1 : -1;
+    const int minRow = ascendingZ ? 0 : rowCount - 1;
+    const int maxRow = ascendingZ ? rowCount - 1 : 0;
+    const int rowStep = ascendingZ ? 1 : -1;
+
+    bool found = false;
+    int idx = 0;
+    int i = 0;
     // m_minVisibleColumnValue
-    for (i = 0, found = false; i < columnCount; i++) {
-        if (array.at(0)->at(i).x() >= axisMinX) {
+    for (i = 0, idx = minCol, found = false; i < columnCount; i++) {
+        if (array.at(0)->at(idx).x() >= axisMinX) {
             found = true;
             break;
         }
+        idx += colStep;
     }
     if (found) {
-        m_minVisibleColumnValue = array.at(0)->at(i).x();
-        sampleSpace.setLeft(i);
+        m_minVisibleColumnValue = array.at(0)->at(idx).x();
+        if (ascendingX)
+            sampleSpace.setLeft(idx);
+        else
+            sampleSpace.setRight(idx);
     } else {
         sampleSpace.setWidth(-1); // to indicate nothing needs to be shown
         return sampleSpace;
     }
 
     // m_maxVisibleColumnValue
-    for (i = columnCount - 1, found = false; i >= 0; i--) {
-        if (array.at(0)->at(i).x() <= axisMaxX) {
+    for (i = 0, idx = maxCol, found = false; i < columnCount; i++) {
+        if (array.at(0)->at(idx).x() <= axisMaxX) {
             found = true;
             break;
         }
+        idx -= colStep;
     }
     if (found) {
-        m_maxVisibleColumnValue = array.at(0)->at(i).x();
-        sampleSpace.setRight(i);
+        m_maxVisibleColumnValue = array.at(0)->at(idx).x();
+        if (ascendingX)
+            sampleSpace.setRight(idx);
+        else
+            sampleSpace.setLeft(idx);
     } else {
         sampleSpace.setWidth(-1); // to indicate nothing needs to be shown
         return sampleSpace;
     }
 
     // m_minVisibleRowValue
-    for (i = 0, found = false; i < rowCount; i++) {
-        if (array.at(i)->at(0).z() >= axisMinZ) {
+    for (i = 0, idx = minRow, found = false; i < rowCount; i++) {
+        if (array.at(idx)->at(0).z() >= axisMinZ) {
             found = true;
             break;
         }
+        idx += rowStep;
     }
     if (found) {
-        m_minVisibleRowValue = array.at(i)->at(0).z();
-        sampleSpace.setTop(i);
+        m_minVisibleRowValue = array.at(idx)->at(0).z();
+        if (ascendingZ)
+            sampleSpace.setTop(idx);
+        else
+            sampleSpace.setBottom(idx);
     } else {
         sampleSpace.setWidth(-1); // to indicate nothing needs to be shown
         return sampleSpace;
     }
 
     // m_maxVisibleRowValue
-    for (i = rowCount - 1, found = false; i >= 0; i--) {
-        if (array.at(i)->at(0).z() <= axisMaxZ) {
+    for (i = 0, idx = maxRow, found = false; i < rowCount; i++) {
+        if (array.at(idx)->at(0).z() <= axisMaxZ) {
             found = true;
             break;
         }
+        idx -= rowStep;
     }
     if (found) {
-        m_maxVisibleRowValue = array.at(i)->at(0).z();
-        sampleSpace.setBottom(i);
+        m_maxVisibleRowValue = array.at(idx)->at(0).z();
+        if (ascendingZ)
+            sampleSpace.setBottom(idx);
+        else
+            sampleSpace.setTop(idx);
     } else {
         sampleSpace.setWidth(-1); // to indicate nothing needs to be shown
         return sampleSpace;
diff --git a/tests/surfacetest/graphmodifier.cpp b/tests/surfacetest/graphmodifier.cpp
index 6dd20de7..33cfb8a1 100644
--- a/tests/surfacetest/graphmodifier.cpp
+++ b/tests/surfacetest/graphmodifier.cpp
@@ -782,7 +782,8 @@ QSurfaceDataRow *GraphModifier::createMultiRow(int row, int series, bool change)
 }
 
 void GraphModifier::populateRisingSeries(QSurface3DSeries *series, int rows, int columns,
-                                         float minValue, float maxValue)
+                                         float minValue, float maxValue, bool ascendingX,
+                                         bool ascendingZ)
 {
     QSurfaceDataArray *dataArray = new QSurfaceDataArray;
     dataArray->reserve(rows);
@@ -791,8 +792,10 @@ void GraphModifier::populateRisingSeries(QSurface3DSeries *series, int rows, int
     for (int i = 0; i < rows; i++) {
         QSurfaceDataRow *dataRow = new QSurfaceDataRow(columns);
         for (int j = 0; j < columns; j++) {
+            float xValue = ascendingX ? float(j) : float(columns - j - 1);
             float yValue = minValue + (range * i * j / arraySize);
-            (*dataRow)[j].setPosition(QVector3D(float(j), yValue, float(i)));
+            float zValue = ascendingZ ? float(i) : float(rows - i - 1);
+            (*dataRow)[j].setPosition(QVector3D(xValue, yValue, zValue));
         }
         dataArray->append(dataRow);
     }
@@ -1168,6 +1171,7 @@ void GraphModifier::massiveDataTest()
     const float yRangeMax = 1.0f;
     const float yRangeMargin = 0.05f;
     static QTimer *massiveTestTimer = 0;
+    static QSurface3DSeries *series = new QSurface3DSeries;
 
     // To speed up massive array creation, we generate a smaller cache array
     // and copy rows from that to our main array
@@ -1221,6 +1225,9 @@ void GraphModifier::massiveDataTest()
 
         qDebug() << __FUNCTION__ << testPhase << ": Creating massive array..."
                  << rows << "x" << columns;
+        // Reset to zero first to avoid having memory allocated for two massive arrays at the same
+        // time on the second and subsequent runs.
+        series->dataProxy()->resetArray(0);
         QSurfaceDataArray *massiveArray = new QSurfaceDataArray;
         massiveArray->reserve(rows);
 
@@ -1232,7 +1239,6 @@ void GraphModifier::massiveDataTest()
         }
         qDebug() << __FUNCTION__ << testPhase << ": Massive array creation finished!";
 
-        QSurface3DSeries *series =  new QSurface3DSeries;
         series->dataProxy()->resetArray(massiveArray);
         m_graph->addSeries(series);
         break;
@@ -1316,8 +1322,8 @@ void GraphModifier::testAxisReverse()
         delete series1;
         series0 = new QSurface3DSeries;
         series1 = new QSurface3DSeries;
-        populateRisingSeries(series0, rowCount, colCount, 0.0f, 50.0f);
-        populateRisingSeries(series1, rowCount, colCount, -20.0f, 30.0f);
+        populateRisingSeries(series0, rowCount, colCount, 0.0f, 50.0f, true, true);
+        populateRisingSeries(series1, rowCount, colCount, -20.0f, 30.0f, true, true);
         m_graph->axisX()->setRange(0.0f, 10.0f);
         m_graph->axisY()->setRange(-20.0f, 50.0f);
         m_graph->axisZ()->setRange(5.0f, 15.0f);
@@ -1361,6 +1367,63 @@ void GraphModifier::testAxisReverse()
     counter++;
 }
 
+void GraphModifier::testDataOrdering()
+{
+    static int counter = 0;
+    const int rowCount = 20;
+    const int colCount = 20;
+    static QSurface3DSeries *series0 = 0;
+    static QSurface3DSeries *series1 = 0;
+    const float series0min = 0.0f;
+    const float series0max = 50.0f;
+    const float series1min = -20.0f;
+    const float series1max = 30.0f;
+
+    switch (counter) {
+    case 0: {
+        qDebug() << __FUNCTION__ << counter << "Setup test - both ascending";
+        foreach (QSurface3DSeries *series, m_graph->seriesList())
+            m_graph->removeSeries(series);
+        foreach (QValue3DAxis *axis, m_graph->axes())
+            m_graph->releaseAxis(axis);
+        delete series0;
+        delete series1;
+        series0 = new QSurface3DSeries;
+        series1 = new QSurface3DSeries;
+        populateRisingSeries(series0, rowCount, colCount, series0min, series0max, true, true);
+        populateRisingSeries(series1, rowCount, colCount, series1min, series1max, true, true);
+        m_graph->axisX()->setRange(5.0f, 15.0f);
+        m_graph->axisY()->setRange(-20.0f, 50.0f);
+        m_graph->axisZ()->setRange(5.0f, 15.0f);
+        m_graph->addSeries(series0);
+        m_graph->addSeries(series1);
+    }
+        break;
+    case 1: {
+        qDebug() << __FUNCTION__ << counter << "Ascending X, descending Z";
+        populateRisingSeries(series0, rowCount, colCount, series0min, series0max, true, false);
+        populateRisingSeries(series1, rowCount, colCount, series1min, series1max, true, false);
+    }
+        break;
+    case 2: {
+        qDebug() << __FUNCTION__ << counter << "Descending X, ascending Z";
+        populateRisingSeries(series0, rowCount, colCount, series0min, series0max, false, true);
+        populateRisingSeries(series1, rowCount, colCount, series1min, series1max, false, true);
+    }
+        break;
+    case 3: {
+        qDebug() << __FUNCTION__ << counter << "Both descending";
+        populateRisingSeries(series0, rowCount, colCount, series0min, series0max, false, false);
+        populateRisingSeries(series1, rowCount, colCount, series1min, series1max, false, false);
+    }
+        break;
+    default:
+        qDebug() << __FUNCTION__ << "Resetting test";
+        counter = -1;
+    }
+    counter++;
+}
+
 void GraphModifier::changeMesh()
 {
     static int model = 0;
diff --git a/tests/surfacetest/graphmodifier.h b/tests/surfacetest/graphmodifier.h
index f3e95a1b..b02e1c31 100644
--- a/tests/surfacetest/graphmodifier.h
+++ b/tests/surfacetest/graphmodifier.h
@@ -111,6 +111,7 @@ public:
     void massiveTestScroll();
     void massiveTestAppendAndScroll();
     void testAxisReverse();
+    void testDataOrdering();
 
 public slots:
     void changeShadowQuality(int quality);
@@ -130,7 +131,7 @@ private:
                               float maxX);
     QSurfaceDataRow *createMultiRow(int row, int series, bool change);
     void populateRisingSeries(QSurface3DSeries *series, int rows, int columns, float minValue,
-                              float maxValue);
+                              float maxValue, bool ascendingX, bool ascendingZ);
 
     Q3DSurface *m_graph;
     QSurface3DSeries *m_multiseries[4];
diff --git a/tests/surfacetest/main.cpp b/tests/surfacetest/main.cpp
index 8371e2cf..9d6165e6 100644
--- a/tests/surfacetest/main.cpp
+++ b/tests/surfacetest/main.cpp
@@ -350,6 +350,9 @@ int main(int argc, char *argv[])
     QPushButton *testReverseButton = new QPushButton(widget);
     testReverseButton->setText(QStringLiteral("Test Axis Reversing"));
 
+    QPushButton *testDataOrderingButton = new QPushButton(widget);
+    testDataOrderingButton->setText(QStringLiteral("Test data ordering"));
+
     QFrame* line = new QFrame();
     line->setFrameShape(QFrame::HLine);
     line->setFrameShadow(QFrame::Sunken);
@@ -439,6 +442,7 @@ int main(int argc, char *argv[])
     vLayout2->addWidget(resetArrayEmptyButton);
     vLayout2->addWidget(massiveDataTestButton);
     vLayout2->addWidget(testReverseButton);
+    vLayout2->addWidget(testDataOrderingButton);
 
     widget->show();
 
@@ -598,6 +602,8 @@ int main(int argc, char *argv[])
                      modifier, &GraphModifier::massiveDataTest);
     QObject::connect(testReverseButton, &QPushButton::clicked,
                      modifier, &GraphModifier::testAxisReverse);
+    QObject::connect(testDataOrderingButton, &QPushButton::clicked,
+                     modifier, &GraphModifier::testDataOrdering);
 
 #ifdef MULTI_SERIES
     modifier->setSeries1CB(series1CB);