[Mesa-dev] [PATCH] mesa: generate error if pbo offset is not aligned with the size of specified type

[Yuanhan Liu]

On Thu, Oct 13, 2011 at 08:01:32PM -0700, Ian Romanick wrote:
> On 10/13/2011 06:46 PM, Yuanhan Liu wrote:
> >Signed-off-by: Yuanhan Liu<yuanhan.liu at linux.intel.com>
> >---
> >  src/mesa/main/pbo.c |    5 +++++
> >  1 files changed, 5 insertions(+), 0 deletions(-)
> >
> >diff --git a/src/mesa/main/pbo.c b/src/mesa/main/pbo.c
> >index 4e7e6f9..4611c5d 100644
> >--- a/src/mesa/main/pbo.c
> >+++ b/src/mesa/main/pbo.c
> >@@ -82,6 +82,11 @@ _mesa_validate_pbo_access(GLuint dimensions,
> >     } else {
> >        offset = ptr;
> >        sizeAddr = ((const GLubyte *) 0) + pack->BufferObj->Size;
> 
> In cases where an error is generated for non-obvious reasons, it is
> helpful to quote a bit of spec language.  In this case, the language
> in the GL spec is spread over a number of sections, but the language
> in the ARB_pixel_buffer_object spec is all in one place.
> 
>       /* The ARB_pixel_buffer_object spec says:
>        *
>        *    "INVALID_OPERATION is generated by ColorTable, ColorSubTable,
>        *    ConvolutionFilter2D, ConvolutionFilter1D, SeparableFilter2D,
>        *    TexImage1D, TexImage2D, TexImage3D, TexSubImage1D,
>        *    TexSubImage2D, TexSubImage3D, and DrawPixels if the current
>        *    PIXEL_UNPACK_BUFFER_BINDING_ARB value is non-zero and the data
>        *    parameter is not evenly divisible into the number of
> basic machine
>        *    units needed to store in memory a datum indicated by the type
>        *    parameter."
>        */

Thanks for the info.

> 
> >+      if (_mesa_sizeof_type(type)&&
> >+          ((GLuint)offset % _mesa_sizeof_type(type))) {
> 
> Two things:
> 
> 1. Rather than call _mesa_sizeof_type twice, call it once and save
> the result.
> 
> 2. It's not obvious why _mesa_sizeof_type needs to be tested against
> 0.  Is this just for the type == GL_BITMAP case?  If so, the test
> should be:
> 
>       if (type != GL_BITMAP &&
>           ((GLuint)offset % _mesa_sizeof_type(type) != 0) {
> 
> That just makes things more explicit for the next person that reads
> this code.

Got it. Thanks. Will send a update patch later.

Thanks,
Yuanhan Liu
> 
> >+         /* offset not aligned with type size */
> >+         return GL_FALSE;
> >+      }
> >     }
> >
> >     if (sizeAddr == 0)