Fix: Nans at surfacedata row/column 0 render fail

If the first or last column or row of the data
given to surfacedataproxy contains NaN values,
the surfacedataproxy might fail to find the
limit values for the axis. Changed limiValues
function in surfacedataproxy so that if it
doesn't find min/max values from the first/last
column/row it continues from the next column/row.
Added a test to verify that the limit values are
found when first/last row contains NaN values.

Fixes: QTBUG-95112
Fixes: QTBUG-95109
Change-Id: I74a343e5bc8cae5b8020243859000fdb5ca7f86e
Reviewed-by: Tomi Korpipää <tomi.korpipaa@qt.io>

4 files changed, 358 insertions, 26 deletions

diff --git a/src/datavisualization/data/qsurfacedataproxy.cpp b/src/datavisualization/data/qsurfacedataproxy.cpp
index 7601d021..c9c6425d 100644
--- a/src/datavisualization/data/qsurfacedataproxy.cpp
+++ b/src/datavisualization/data/qsurfacedataproxy.cpp
@@ -552,9 +552,11 @@ void QSurfaceDataProxyPrivate::limitValues(QVector3D &minValues, QVector3D &maxV
                 float itemValue = m_dataArray->at(i)->at(j).y();
                 if (qIsNaN(itemValue) || qIsInf(itemValue))
                     continue;
-                if (min > itemValue && isValidValue(itemValue, axisY))
+                if ((min > itemValue || (qIsNaN(min) || qIsInf(min)))
+                        && isValidValue(itemValue, axisY)) {
                     min = itemValue;
-                if (max < itemValue)
+                }
+                if (max < itemValue  || (qIsNaN(max) || qIsInf(max)))
                     max = itemValue;
             }
         }
@@ -569,33 +571,59 @@ void QSurfaceDataProxyPrivate::limitValues(QVector3D &minValues, QVector3D &maxV
         float xHigh = m_dataArray->at(0)->last().x();
         float zLow = m_dataArray->at(0)->at(0).z();
         float zHigh = m_dataArray->last()->at(0).z();
-        for (int i = 0; i < columns; i++) {
-            float zItemValue = m_dataArray->at(0)->at(i).z();
-            if (qIsNaN(zItemValue) || qIsInf(zItemValue))
-                continue;
-            else if (isValidValue(zItemValue, axisZ))
-                zLow = qMin(zLow,zItemValue);
+        for (int i = 0; i < rows; i++) {
+            for (int j = 0; j < columns; j++) {
+                float zItemValue = m_dataArray->at(i)->at(j).z();
+                if (qIsNaN(zItemValue) || qIsInf(zItemValue))
+                    continue;
+                else if (isValidValue(zItemValue, axisZ))
+                    zLow = qMin(zLow,zItemValue);
+            }
+            if (!qIsNaN(zLow) && !qIsInf(zLow))
+                break;
         }
-        for (int i = 0; i < columns; i++) {
-            float zItemValue = m_dataArray->last()->at(i).z();
-            if (qIsNaN(zItemValue) || qIsInf(zItemValue))
-                continue;
-            else if (isValidValue(zItemValue, axisZ))
-                zHigh = qMax(zHigh, zItemValue);
+        for (int i = rows - 1; i >= 0; i--) {
+            for (int j = 0; j < columns; j++) {
+                float zItemValue = m_dataArray->at(i)->at(j).z();
+                if (qIsNaN(zItemValue) || qIsInf(zItemValue))
+                    continue;
+                else if (isValidValue(zItemValue, axisZ))
+                {
+                    if (!qIsNaN(zHigh) && !qIsInf(zHigh))
+                        zHigh = qMax(zHigh, zItemValue);
+                    else
+                        zHigh = zItemValue;
+                }
+            }
+            if (!qIsNaN(zHigh) && !qIsInf(zHigh))
+                break;
         }
-        for (int i = 0; i < rows; i++) {
-            float xItemValue = m_dataArray->at(i)->at(0).x();
-            if (qIsNaN(xItemValue) || qIsInf(xItemValue))
-                continue;
-            else if (isValidValue(xItemValue, axisX))
-                xLow = qMin(xLow, xItemValue);
+        for (int j = 0; j<columns; j++){
+            for (int i = 0; i < rows; i++) {
+                float xItemValue = m_dataArray->at(i)->at(j).x();
+                if (qIsNaN(xItemValue) || qIsInf(xItemValue))
+                    continue;
+                else if (isValidValue(xItemValue, axisX))
+                    xLow = qMin(xLow, xItemValue);
+            }
+            if (!qIsNaN(xLow) && !qIsInf(xLow))
+                break;
         }
-        for (int i = 0; i < rows; i++) {
-            float xItemValue = m_dataArray->at(i)->last().x();
-            if (qIsNaN(xItemValue) || qIsInf(xItemValue))
-                continue;
-            else if (isValidValue(xItemValue, axisX))
-                xHigh = qMax(xHigh, xItemValue);
+        for (int j = columns-1; j >= 0; j--){
+            for (int i = 0; i < rows; i++) {
+                float xItemValue = m_dataArray->at(i)->at(j).x();
+                if (qIsNaN(xItemValue) || qIsInf(xItemValue))
+                    continue;
+                else if (isValidValue(xItemValue, axisX))
+                {
+                    if (!qIsNaN(xHigh) && !qIsInf(xHigh))
+                        xHigh = qMax(xHigh, xItemValue);
+                    else
+                        xHigh = xItemValue;
+                }
+            }
+            if (!qIsNaN(xHigh) && !qIsInf(xHigh))
+                break;
         }
         minValues.setX(xLow);
         minValues.setZ(zLow);
diff --git a/tests/auto/cpptest/CMakeLists.txt b/tests/auto/cpptest/CMakeLists.txt
index 7b6ef00b..bed23651 100644
--- a/tests/auto/cpptest/CMakeLists.txt
+++ b/tests/auto/cpptest/CMakeLists.txt
@@ -9,6 +9,7 @@ add_subdirectory(q3dscatter-series)
 add_subdirectory(q3dsurface)
 add_subdirectory(q3dsurface-proxy)
 add_subdirectory(q3dsurface-modelproxy)
+add_subdirectory(q3dsurface-modelproxy-nan)
 add_subdirectory(q3dsurface-heightproxy)
 add_subdirectory(q3dsurface-series)
 add_subdirectory(q3daxis-category)
diff --git a/tests/auto/cpptest/q3dsurface-modelproxy-nan/CMakeLists.txt b/tests/auto/cpptest/q3dsurface-modelproxy-nan/CMakeLists.txt
new file mode 100644
index 00000000..bab59c3b
--- /dev/null
+++ b/tests/auto/cpptest/q3dsurface-modelproxy-nan/CMakeLists.txt
@@ -0,0 +1,11 @@
+qt_add_test(q3dsurface-modelproxy-nan
+    SOURCES
+        tst_proxy.cpp
+    INCLUDE_DIRECTORIES
+        ../common
+    PUBLIC_LIBRARIES
+        Qt::Gui
+        Qt::GuiPrivate
+        Qt::Widgets
+        Qt::DataVisualization
+)
diff --git a/tests/auto/cpptest/q3dsurface-modelproxy-nan/tst_proxy.cpp b/tests/auto/cpptest/q3dsurface-modelproxy-nan/tst_proxy.cpp
new file mode 100644
index 00000000..53b9ac39
--- /dev/null
+++ b/tests/auto/cpptest/q3dsurface-modelproxy-nan/tst_proxy.cpp
@@ -0,0 +1,292 @@
+/****************************************************************************
+**
+** Copyright (C) 2021 The Qt Company Ltd.
+** Contact: https://www.qt.io/licensing/
+**
+** This file is part of the Qt Data Visualization module of the Qt Toolkit.
+**
+** $QT_BEGIN_LICENSE:GPL$
+** Commercial License Usage
+** Licensees holding valid commercial Qt licenses may use this file in
+** accordance with the commercial license agreement provided with the
+** Software or, alternatively, in accordance with the terms contained in
+** a written agreement between you and The Qt Company. For licensing terms
+** and conditions see https://www.qt.io/terms-conditions. For further
+** information use the contact form at https://www.qt.io/contact-us.
+**
+** GNU General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU
+** General Public License version 3 or (at your option) any later version
+** approved by the KDE Free Qt Foundation. The licenses are as published by
+** the Free Software Foundation and appearing in the file LICENSE.GPL3
+** included in the packaging of this file. Please review the following
+** information to ensure the GNU General Public License requirements will
+** be met: https://www.gnu.org/licenses/gpl-3.0.html.
+**
+** $QT_END_LICENSE$
+**
+****************************************************************************/
+
+#include <QtTest/QtTest>
+
+#include <QtDataVisualization/QItemModelSurfaceDataProxy>
+#include <QtDataVisualization/Q3DSurface>
+
+#include "cpptestutil.h"
+
+class tst_proxy: public QObject
+{
+    Q_OBJECT
+
+private slots:
+    void initTestCase();
+    void cleanupTestCase();
+    void init();
+    void cleanup();
+
+    void dataContainingNaNFirstRow();
+    void dataContainingNaNLastRow();
+    void dataContainingNaNFirstLastRow();
+};
+
+void tst_proxy::initTestCase()
+{
+}
+
+void tst_proxy::cleanupTestCase()
+{
+}
+
+void tst_proxy::init()
+{
+}
+
+void tst_proxy::cleanup()
+{
+}
+
+void tst_proxy::dataContainingNaNFirstRow()
+{
+    if (!CpptestUtil::isOpenGLSupported())
+        QSKIP("OpenGL not supported on this platform");
+
+    const int size = 10;
+    const int missingRow = 0;
+
+    QItemModelSurfaceDataProxy *proxy  = new QItemModelSurfaceDataProxy();
+    QSurface3DSeries *series = new QSurface3DSeries(proxy);
+    Q3DSurface *graph = new Q3DSurface();
+    graph->addSeries(series);
+
+    // X
+    QSurfaceDataArray *array = new QSurfaceDataArray();
+    array->reserve(size);
+    for (int i = 0; i < size; i++) {
+        QSurfaceDataRow *row = new QSurfaceDataRow(size);
+        for (int j = 0; j < size; j++) {
+            (*row)[j].setPosition(QVector3D((i == missingRow) ? std::numeric_limits<float>::quiet_NaN()
+                                                              : static_cast<float>(i),
+                                            qSin(static_cast<float>(i)), static_cast<float>(j)));
+        }
+        *array << row;
+    }
+    proxy->resetArray(array);
+    QVERIFY(!qIsNaN(graph->axisX()->min()));
+    QVERIFY(!qIsNaN(graph->axisX()->max()));
+    QVERIFY(!qIsNaN(graph->axisY()->min()));
+    QVERIFY(!qIsNaN(graph->axisY()->max()));
+    QVERIFY(!qIsNaN(graph->axisZ()->min()));
+    QVERIFY(!qIsNaN(graph->axisZ()->max()));
+
+    // Y
+    for (int i = 0; i < size; i++) {
+        QSurfaceDataRow *row = new QSurfaceDataRow(size);
+        for (int j = 0; j < size; j++) {
+            (*row)[j].setPosition(QVector3D(static_cast<float>(i),
+                                            (i == missingRow) ? std::numeric_limits<float>::quiet_NaN()
+                                                              : qSin(static_cast<float>(i)),
+                                             static_cast<float>(j)));
+        }
+        *array << row;
+    }
+    proxy->resetArray(array);
+    QVERIFY(!qIsNaN(graph->axisX()->min()));
+    QVERIFY(!qIsNaN(graph->axisX()->max()));
+    QVERIFY(!qIsNaN(graph->axisY()->min()));
+    QVERIFY(!qIsNaN(graph->axisY()->max()));
+    QVERIFY(!qIsNaN(graph->axisZ()->min()));
+    QVERIFY(!qIsNaN(graph->axisZ()->max()));
+
+    // Z
+    for (int i = 0; i < size; i++) {
+        QSurfaceDataRow *row = new QSurfaceDataRow(size);
+        for (int j = 0; j < size; j++) {
+            (*row)[j].setPosition(QVector3D(static_cast<float>(i),
+                                            qSin(static_cast<float>(i)),
+                                            (i == missingRow) ? std::numeric_limits<float>::quiet_NaN()
+                                                              : static_cast<float>(j)));
+        }
+        *array << row;
+    }
+    proxy->resetArray(array);
+    QVERIFY(!qIsNaN(graph->axisX()->min()));
+    QVERIFY(!qIsNaN(graph->axisX()->max()));
+    QVERIFY(!qIsNaN(graph->axisY()->min()));
+    QVERIFY(!qIsNaN(graph->axisY()->max()));
+    QVERIFY(!qIsNaN(graph->axisZ()->min()));
+    QVERIFY(!qIsNaN(graph->axisZ()->max()));
+
+    delete graph;
+
+}
+
+void tst_proxy::dataContainingNaNLastRow()
+{
+    if (!CpptestUtil::isOpenGLSupported())
+        QSKIP("OpenGL not supported on this platform");
+
+    const int size = 10;
+    const int missingRow = size -  1;
+    QItemModelSurfaceDataProxy *proxy  = new QItemModelSurfaceDataProxy();
+    QSurface3DSeries *series = new QSurface3DSeries(proxy);
+    Q3DSurface *graph = new Q3DSurface();
+    graph->addSeries(series);
+
+    // X
+    QSurfaceDataArray *array = new QSurfaceDataArray();
+    array->reserve(size);
+    for (int i = 0; i < size; i++) {
+        QSurfaceDataRow *row = new QSurfaceDataRow(size);
+        for (int j = 0; j < size; j++) {
+            (*row)[j].setPosition(QVector3D((i == missingRow) ? std::numeric_limits<float>::quiet_NaN()
+                                                              : static_cast<float>(i),
+                                            qSin(static_cast<float>(i)), static_cast<float>(j)));
+        }
+        *array << row;
+    }
+    proxy->resetArray(array);
+    QVERIFY(!qIsNaN(graph->axisX()->min()));
+    QVERIFY(!qIsNaN(graph->axisX()->max()));
+    QVERIFY(!qIsNaN(graph->axisY()->min()));
+    QVERIFY(!qIsNaN(graph->axisY()->max()));
+    QVERIFY(!qIsNaN(graph->axisZ()->min()));
+    QVERIFY(!qIsNaN(graph->axisZ()->max()));
+
+    // Y
+    for (int i = 0; i < size; i++) {
+        QSurfaceDataRow *row = new QSurfaceDataRow(size);
+        for (int j = 0; j < size; j++) {
+            (*row)[j].setPosition(QVector3D(static_cast<float>(i),
+                                            (i == missingRow) ? std::numeric_limits<float>::quiet_NaN()
+                                                              : qSin(static_cast<float>(i)),
+                                             static_cast<float>(j)));
+        }
+        *array << row;
+    }
+    proxy->resetArray(array);
+    QVERIFY(!qIsNaN(graph->axisX()->min()));
+    QVERIFY(!qIsNaN(graph->axisX()->max()));
+    QVERIFY(!qIsNaN(graph->axisY()->min()));
+    QVERIFY(!qIsNaN(graph->axisY()->max()));
+    QVERIFY(!qIsNaN(graph->axisZ()->min()));
+    QVERIFY(!qIsNaN(graph->axisZ()->max()));
+
+    // Z
+    for (int i = 0; i < size; i++) {
+        QSurfaceDataRow *row = new QSurfaceDataRow(size);
+        for (int j = 0; j < size; j++) {
+            (*row)[j].setPosition(QVector3D(static_cast<float>(i),
+                                            qSin(static_cast<float>(i)),
+                                            (i == missingRow) ? std::numeric_limits<float>::quiet_NaN()
+                                                              : static_cast<float>(j)));
+        }
+        *array << row;
+    }
+    proxy->resetArray(array);
+    QVERIFY(!qIsNaN(graph->axisX()->min()));
+    QVERIFY(!qIsNaN(graph->axisX()->max()));
+    QVERIFY(!qIsNaN(graph->axisY()->min()));
+    QVERIFY(!qIsNaN(graph->axisY()->max()));
+    QVERIFY(!qIsNaN(graph->axisZ()->min()));
+    QVERIFY(!qIsNaN(graph->axisZ()->max()));
+    delete graph;
+}
+
+void tst_proxy::dataContainingNaNFirstLastRow()
+{
+    if (!CpptestUtil::isOpenGLSupported())
+        QSKIP("OpenGL not supported on this platform");
+
+    const int size = 10;
+    const int rowFirst = 0;
+    const int rowLast = size -  1;
+    QItemModelSurfaceDataProxy *proxy = new QItemModelSurfaceDataProxy();
+    QSurface3DSeries *series = new QSurface3DSeries(proxy);
+    Q3DSurface *graph = new Q3DSurface();
+    graph->addSeries(series);
+
+    // X
+    QSurfaceDataArray *array = new QSurfaceDataArray();
+    array->reserve(size);
+    for (int i = 0; i < size; i++) {
+        bool missingRow = (i == rowFirst || i == rowLast);
+        QSurfaceDataRow *row = new QSurfaceDataRow(size);
+        for (int j = 0; j < size; j++) {
+            (*row)[j].setPosition(QVector3D(missingRow ? std::numeric_limits<float>::quiet_NaN()
+                                                       : static_cast<float>(i),
+                                            qSin(static_cast<float>(i)), static_cast<float>(j)));
+        }
+        *array << row;
+    }
+    proxy->resetArray(array);
+    QVERIFY(!qIsNaN(graph->axisX()->min()));
+    QVERIFY(!qIsNaN(graph->axisX()->max()));
+    QVERIFY(!qIsNaN(graph->axisY()->min()));
+    QVERIFY(!qIsNaN(graph->axisY()->max()));
+    QVERIFY(!qIsNaN(graph->axisZ()->min()));
+    QVERIFY(!qIsNaN(graph->axisZ()->max()));
+
+    // Y
+    for (int i = 0; i < size; i++) {
+        bool missingRow = (i == rowFirst || i == rowLast);
+        QSurfaceDataRow *row = new QSurfaceDataRow(size);
+        for (int j = 0; j < size; j++) {
+            (*row)[j].setPosition(QVector3D(static_cast<float>(i),
+                                            missingRow ? std::numeric_limits<float>::quiet_NaN()
+                                                       : qSin(static_cast<float>(i)),
+                                             static_cast<float>(j)));
+        }
+        *array << row;
+    }
+    proxy->resetArray(array);
+    QVERIFY(!qIsNaN(graph->axisX()->min()));
+    QVERIFY(!qIsNaN(graph->axisX()->max()));
+    QVERIFY(!qIsNaN(graph->axisY()->min()));
+    QVERIFY(!qIsNaN(graph->axisY()->max()));
+    QVERIFY(!qIsNaN(graph->axisZ()->min()));
+    QVERIFY(!qIsNaN(graph->axisZ()->max()));
+
+    // Z
+    for (int i = 0; i < size; i++) {
+        bool missingRow = (i == rowFirst || i == rowLast);
+        QSurfaceDataRow *row = new QSurfaceDataRow(size);
+        for (int j = 0; j < size; j++) {
+            (*row)[j].setPosition(QVector3D(static_cast<float>(i),
+                                            qSin(static_cast<float>(i)),
+                                            missingRow ? std::numeric_limits<float>::quiet_NaN()
+                                                       : static_cast<float>(j)));
+        }
+        *array << row;
+    }
+    proxy->resetArray(array);
+    QVERIFY(!qIsNaN(graph->axisX()->min()));
+    QVERIFY(!qIsNaN(graph->axisX()->max()));
+    QVERIFY(!qIsNaN(graph->axisY()->min()));
+    QVERIFY(!qIsNaN(graph->axisY()->max()));
+    QVERIFY(!qIsNaN(graph->axisZ()->min()));
+    QVERIFY(!qIsNaN(graph->axisZ()->max()));
+    delete graph;
+}
+
+QTEST_MAIN(tst_proxy)
+#include "tst_proxy.moc"