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

Setting sampler uniforms calls glUniform1iv with wrong count

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Closed
    • Priority: P2: Important
    • Resolution: Done
    • Affects Version/s: 5.11
    • Fix Version/s: 5.11.1
    • Component/s: Qt3D
    • Labels:
      None
    • Commits:
      0c587292a3b5dd62a194a48f099a3ceb7a90a1d9

      Description

      Attempting to set a sampler uniform will lead to writing garbage to uniform locations other than the location of the sampler when we have a sampler uniform array. (for non-arrays the behavior is undefined as the count is mandated to be 1 by the spec)

      UniformValue(Qt3DCore::QNodeId) calls the default UniformValue() implementation that does:

          UniformValue()
              : m_data(4)
      

      thus making value.byteSize() 16.

      "Promoting" the UniformValue's internal data to ints suitable for samplers happens in RenderView, and its uniformArraySize calculation expects that value.byteSize() == elementCount * sizeof(QNodeId). This is not true with m_data(4) above.

      void RenderView::setUniformValue(ShaderParameterPack &uniformPack, int nameId, const UniformValue &value) const
      {
          if (value.valueType() == UniformValue::NodeId) {
              const Qt3DCore::QNodeId *nodeIds = value.constData<Qt3DCore::QNodeId>();
      
              const int uniformArraySize = value.byteSize() / sizeof(Qt3DCore::QNodeId);
              for (int i = 0; i < uniformArraySize; ++i) { ... }
      
              UniformValue textureValue(uniformArraySize * sizeof(int), UniformValue::TextureValue);
      

      For example, attempting to set a single sampler uniform will lead to uniformArraySize = 16 / 8 = 2 which is incorrect as it should be 1.

      The correct approach is to avoid all the trouble caused by calling UniformValue():

      diff --git a/src/render/backend/uniform_p.h b/src/render/backend/uniform_p.h
      index e31aaa609..7f273566e 100644
      --- a/src/render/backend/uniform_p.h
      +++ b/src/render/backend/uniform_p.h
      @@ -181,7 +181,7 @@ public:
      
           // For nodes which will later be replaced by a Texture or Buffer
           UniformValue(Qt3DCore::QNodeId id)
      -        : UniformValue()
      +        : m_data(sizeof(Qt3DCore::QNodeId) / sizeof(float))
           {
               m_valueType = NodeId;
               memcpy(m_data.data(), &id, sizeof(Qt3DCore::QNodeId));
      

      thus making sure m_data.count() matches the actual data.

        Attachments

          Issue Links

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

            Activity

              People

              Assignee:
              lagocs Laszlo Agocs
              Reporter:
              lagocs Laszlo Agocs
              Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Gerrit Reviews

                  There are no open Gerrit changes