Uploaded image for project: 'Qt'
  1. Qt
  2. QTBUG-20546

Fix column vs row-major confusion in QMatrix4x4 API

    XMLWordPrintable

Details

    • Bug
    • Resolution: Duplicate
    • P2: Important
    • Qt3D 1.0
    • 4.7.3
    • Qt3D
    • None

    Description

      QMatrix4x4 documentation and API says or implies row-major order but the data() and constData() functions return the data in column major order, and also the internal storage is in column major order. The doco for data() and constData() are silent on the format the data is returned in, so users assume it is row-major.

      This is not a bug in functionality as such - we don't count on the result from the data() or constData() functions as being in row-major order anywhere.

      The reason is that those functions are used for passing the data to OpenGL which expects column-major order.

      The sorts of confusion users might have is that:

      qreal *mat_data = { .... };   // some data
      
      QMatrix4x4 m1(mat_data);
      QMatrix4x4 m2(m1.data);
      
      m1 != m2;  
      

      This is where constructors fall miserably short.

      QMatrix4x4::fromColumnOrderData(...)
      QMatrix4x4::fromRowOrderData(...) 
      

      Would be so much less ambigous.

      The constructor is row-order because it is the natural way to write the array data.

      data = {
         m11, m12, m13, m14,
         m21, m22, m23, m24,
         ...
      }
      

      With column order that you would basically have to specify the transposed matrix, which I tend to find rather confusing. We could work on the naming to make things more consistent though...

      If we called the data accessors columnOrderData() and columnOrderConstData(), or a better/shorter name if you have it, and deprecate the data()/constData() functions, it would probably be more self-explanatory. Then deprecate the data constructor for the benefit of a factory function, and the confusion would be gone from code.

      Also needs an example showing how to correctly use these functions, eg with a simple translation matrix. Obviously also unit tests.

      Attachments

        Issue Links

          No reviews matched the request. Check your Options in the drop-down menu of this sections header.

          Activity

            People

              sarasmit Sarah Smith (closed Nokia identity) (Inactive)
              sarasmit Sarah Smith (closed Nokia identity) (Inactive)
              Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Gerrit Reviews

                  There are no open Gerrit changes