<html>
    <head>
      <base href="https://bugs.freedesktop.org/">
    </head>
    <body>
      <p>
        <div>
            <b><a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - mat4: m[i][j] incorrect result with row_major UBO"
   href="https://bugs.freedesktop.org/show_bug.cgi?id=104553#c10">Comment # 10</a>
              on <a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - mat4: m[i][j] incorrect result with row_major UBO"
   href="https://bugs.freedesktop.org/show_bug.cgi?id=104553">bug 104553</a>
              from <span class="vcard"><a class="email" href="mailto:t_arceri@yahoo.com.au" title="Timothy Arceri <t_arceri@yahoo.com.au>"> <span class="fn">Timothy Arceri</span></a>
</span></b>
        <pre>(In reply to Alejandro PiƱeiro (freenode IRC: apinheiro) from <a href="show_bug.cgi?id=104553#c9">comment #9</a>)
<span class="quote">> Found this bug when I was about to report the same problem. Some comments
> below:

> (In reply to florian.will from <a href="show_bug.cgi?id=104553#c0">comment #0</a>)
> > Created <span class="bz_obsolete"><a href="attachment.cgi?id=136630" name="attach_136630" title="Failing piglit test (when test_variant = 2) for this bug.">attachment 136630</a> <a href="attachment.cgi?id=136630&action=edit" title="Failing piglit test (when test_variant = 2) for this bug.">[details]</a></span>
> > Failing piglit test (when test_variant = 2) for this bug.
> > 
> > I hit another bug while trying to get Banshee 3D
> > <<a href="https://github.com/BearishSun/BansheeEngine">https://github.com/BearishSun/BansheeEngine</a>> to work correctly on Mesa
> > radeonsi (HD 7870) / amdgpu kernel module. I use git commit 2719467eb6 right
> > now, which is a few days old.
> > 
> > Accessing a "mat4 m" component, e.g. m[1][2], returns unexpected results if
> > m is declared inside a UBO block and uses row_major matrix format. col_major
> > works as expected. m[1].z also works. Some experiments indicate that for
> > m[i][j], the UBO buffer is accessed at offset (i+j)*4 instead of (i+j*4)*4.
> > 
> > The invalid offsets for loading floats from the UBO are visible in the Mesa
> > IR when linker.cpp is done (probably introduced by lower_ubo_reference()).

> As you imply below, this problem also affects ssbo. So perhaps it would be a
> good idea to update the bug description?

> > While exploring this issue, I prepared a piglit test case that tests for
> > this. It fails on my setup (when test_variant = 2, the other tests succeed).
> > I will attach it to this bug report and send it to the piglit list for
> > review.
> > 
> > 
> > 
> > Possible Fix:
> > 
> > I doubt this is enough/correct, because I haven't fully grasped the IR
> > processing in the glsl compiler and which fields/data types can/can't be
> > row_major and the implications, but this simple change in
> > lower_buffer_access.cpp 

> FWIW, while I was doing a skimming, I also got to lower_buffer_access and
> lower_ubo_reference (this one touches both ssbo and ubo), so I agree that
> the issue is likely at that code. Not sure if your fix is correct or in the
> good path sorry.

> (In reply to florian.will from <a href="show_bug.cgi?id=104553#c7">comment #7</a>)
> > Created <span class=""><a href="attachment.cgi?id=136878" name="attach_136878" title="Changes to piglit UBO test generator">attachment 136878</a> <a href="attachment.cgi?id=136878&action=edit" title="Changes to piglit UBO test generator">[details]</a></span> <a href='page.cgi?id=splinter.html&bug=104553&attachment=136878'>[review]</a> [review] [review]
> > Changes to piglit UBO test generator
> > 
> > I have now extended the random UBO piglit test generator python script (in a
> > hackish way) to generate SSBO tests as well, and added std430 packing rules
> > to generate std430 SSBO tests. My changes are in the attached patch file,
> > but I'd say it's not suitable for piglit git (too ugly).
> > 
> > It was helpful to validate the mesa patch I've attached to this bug report
> > earlier. Using mesa git master, 391 out of the 540 generated UBO&SSBO tests
> > fail. After applying my patch, only a few tests (3-7) fail. The failing
> > tests are always very huge test files (some have more than 10k lines and
> > sometimes up to 5MB shader_test files). Apparently they hit something like
> > an internal size limit for vertex shaders, because the tests pass when
> > commenting out one half of the test conditions in the vertex shader, and
> > they still pass when commenting out the other half of the vertex shader.

> Somewhat off-topic: Timothy mentioned that in the past it was not included
> due all the amount of tests added. So perhaps a compromise would be added
> some (~10?) barebone tests, to at least cover the most basic cases.
> Something like this test I wrote while debugging this:
> <a href="https://github.com/Igalia/piglit/blob/apinheiro/matrix-row-major-failure/">https://github.com/Igalia/piglit/blob/apinheiro/matrix-row-major-failure/</a>
> tests/spec/arb_fake/execution/ubo/matrix-column-vs-row.shader_test
> </span >

Yes for now I'd be happy with someone committing a piglit test that trips up
the  bug. Please go ahead and push your test as its the simplest of the two
tests. Thanks.</pre>
        </div>
      </p>


      <hr>
      <span>You are receiving this mail because:</span>

      <ul>
          <li>You are the assignee for the bug.</li>
      </ul>
    </body>
</html>