A texture-data alignment bug when tracing

[Paul Berry]

On 14 September 2012 09:03, José Fonseca <jose.r.fonseca at gmail.com> wrote:

> Hi Carl,
>
> On Mon, Sep 10, 2012 at 11:11 PM, Carl Worth <cworth at cworth.org> wrote:
> > I've attached a trace that demonstrates a bug in capturing texture data
> > blobs while tracing. The code being traced does the following:
> >
> >         /* A 2x2 RGB texture requires 16 bytes with the default
> >          * GL_UNPACK_ALIGNMENT value of 4. */
> >         uint8_t data[16];
> >
> >         /* ... Initialize data, gen and bind texture here ... */
> >
> >         glTexImage2D (GL_TEXTURE_2D,
> >                       0, GL_RGB8,
> >                       2, 2, 0,
> >                       GL_RGB, GL_UNSIGNED_BYTE, data);
> >
> > As you can see, the resulting trace has:
> >
> >         850 glTexImage2D(target = GL_TEXTURE_2D, level = 0,
> >         internalformat = GL_RGB8, width = 2, height = 2, border = 0,
> >         format = GL_RGB, type = GL_UNSIGNED_BYTE, pixels = blob(12))
> >
> > That is, the trace has captured only 12 bytes of data. But there is
> > required data in 14 bytes, (and for full alignment 16 bytes are
> > required).
> >
> > It appears that _gl_image_size in glsize.hpp is doing lots of work to
> > compute the correct image size (and checking GL_UNPACK_ALIGNMENT). I
> > believe that the code that should be performing the alignment is the
> > following:
> >
> >     if ((GLint)bits_per_pixel < alignment*8 &&
> >         (bits_per_pixel & 7) == 0 &&
> >         _is_pot(bits_per_pixel)) {
> >         row_stride = _align(row_stride, alignment);
> >     }
> >
> > The second and third sub-conditions of that condition appear to be
> > causing the problem here. In this case bits_per_pixel is 12, so
> > (bits_per_pixel & 7) != 0. Also, bits_per_pixel is not a power of 2.
>
> Actually bits_per_pixel here should be 3*8 == 24. But indeed that's not a
> POT.
>
> > I'm not sure why either of these sub-conditions are present. Why would
> > we not want to always satisfy the alignment requirements?
> >
> > I'll follow up with a patch that just removes these
> > sub-conditions. Please let me know if there's some more appropriate
> > solution here.
>
> glspec21.20061201.pdf , pg 131, says
>
>   "If the number of bits per element is not 1, 2, 4, or 8 times
> the number of bits in a GL ubyte, then k = nl for all values of a."
>
> where a is alignment. That is, when the number of bits per element is
> not 8bits, 16bits, 32bits, or 64bits, then alignment should be ignore
> for row_stride.


> The "(bits_per_pixel & 7) == 0 &&  _is_pot(bits_per_pixel)" is not
> fully correct either. it should be
>
> diff --git a/helpers/glsize.hpp b/helpers/glsize.hpp
> index a589199..cc31414 100644
> --- a/helpers/glsize.hpp
> +++ b/helpers/glsize.hpp
> @@ -683,9 +683,11 @@ _gl_image_size(GLenum format, GLenum type,
> GLsizei width, GLsizei height, GLsize
>
>      size_t row_stride = (row_length*bits_per_pixel + 7)/8;
>
> -    if ((GLint)bits_per_pixel < alignment*8 &&
> -        (bits_per_pixel & 7) == 0 &&
> -        _is_pot(bits_per_pixel)) {
> +    if ((bits_per_pixel == 1*8 ||
> +         bits_per_pixel == 2*8 ||
> +         bits_per_pixel == 4*8 ||
> +         bits_per_pixel == 8*8) &&
> +        (GLint)bits_per_pixel < alignment*8) {
>          row_stride = _align(row_stride, alignment);
>      }
>
>
> But still, AFAICT, unless I'm misinterpreting the meaning of "bits per
> element", apitrace is correct, and the driver/application is wrong.
>

I think you're right, Jose.  And unless I'm much mistaken, Mesa has never
followed this rule correctly.  Carl, would you mind sending me the test
program you were using to investigate this?  I'd like to try it on my
nVidia machine.  Assuming that the nVidia driver follows the spec, I'll
file a Mesa bug report.


>
> Jose
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/apitrace/attachments/20120914/2368a500/attachment.html>