Ilia Mirkin <imir...@alum.mit.edu> writes: >> - end = _mesa_image_offset(dimensions, pack, width, height, >> - format, type, depth-1, height-1, width); >> + if (depth == 0 || height == 0) > > Why not width == 0 as well? You could probably just do > > return GL_TRUE; > > in that case as well, and then call _mesa_image_offset unconditionally > for both end/start. IMHO simpler and more efficient.
Yes, that makes sense. I was thinking we would want to preserve the error in the case where the start address is out of range even if the size is zero because that would be an especially weird thing for the application to do and maybe an error would be helpful. However the spec doesn't imply that this should happen and it only says: “An INVALID_OPERATION error is generated if a pixel unpack buffer object is bound and unpacking the pixel data according to the process described below would access memory beyond the size of the pixel unpack buffer’s memory size.” I'll make a V2. >> + end = start; >> + else >> + end = _mesa_image_offset(dimensions, pack, width, height, > > I've always found a single space is enough for me... apparently not > for the original author of this code (which you then just indented). Yes, I noticed this too but I didn't change it because then I was in a dilemma about whether I should put spaces around the - operator too I decided it was easier to just leave the whole line as it is :) Thanks for the review. Regards, - Neil _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
