QGenericMatrix class seems to have mistakes in its constructor and method copyDataTo()

Andruxin

Hello guys! I was trying to use QGenericMatrix class constructor: QGenericMatrix(const T *values).
Here's an example:

int values[] = { 1, 2, 3, //here's a matrix as i want it to see and as it must be passed into QGenericMatrix object
                 4, 5, 6, //as documentation(http://doc.qt.io/qt-5/qgenericmatrix.html#QGenericMatrix-2) says
                 7, 8, 9,
                 10,11,12 } 
QGenericMatrix<3, 4, int> matrix(values); //creates matrix object with 3 columns and 4 rows and inits it with values

After that code matrix object won't look as we expected it to. I think there is a simple mistake which you can find in QGenericMatrix.h source code:

template <int N, int M, typename T>
Q_OUTOFLINE_TEMPLATE QGenericMatrix<N, M, T>::QGenericMatrix(const T *values)
{
    for (int col = 0; col < N; ++col)
        for (int row = 0; row < M; ++row)
            m[col][row] = values[row * N + col];
}

I think instead there must be something like:

template <int N, int M, typename T>
Q_OUTOFLINE_TEMPLATE QGenericMatrix<N, M, T>::QGenericMatrix(const T *values)
{   
    for (int row = 0; row < M; ++row)
        for (int col = 0; col < N; ++col)
            m[col][row] = values[row * N + col];
}

Same mistake seems to be in copyDataTo() method and may be in some other methods (i haven't investigated source code that much). I'm just asking because it's very huge and important functionality bug and it's hard to imagine that it could live so long without someone to see it.

mrjj

Hi

After that code matrix object won't look as we expected it to.

Hi the docs says
"The contents of the array values is assumed to be in row-major order."

https://stackoverflow.com/questions/33862730/row-major-vs-column-major-confusion

So you are saying that it reads a row-major array wrongly?

Andruxin

@mrjj I'm sorry I wasn't right. I was looking on the Debug section of QtCreator trying to understand what is happening to matrix. It wasn't wise decision 'cause matrix's storing and displaying are two different things. Thanks for an answer.

mrjj

@Andruxin
Indeed it is and easy to mix row/col but good it wasn't a bug :)O