[Mesa-dev] [PATCH] gallium/util: don't use blocksize for minify for assertions

[Brian Paul]

On 06/14/2016 03:39 AM, Jose Fonseca wrote:
> On 14/06/16 02:35, sroland at vmware.com wrote:
>> From: Roland Scheidegger <sroland at vmware.com>
>>
>> The previous assertions required for texture sizes smaller than
>> block_size
>> that src_box.x + src_box.width still be block size.
>> (e.g. for a texture with width 3, and src_box.x = 0, src_box.width would
>> have to be 4 to not assert.)
>> This caused some assertions with some other state tracker.
>> It looks though like callers aren't expected to round up widths to
>> block sizes
>> (for sizes larger than block size the assertion would still have
>> verified it
>> wouldn't have been rounded up) so we simply shouldn't use a minify which
>> rounds up to block size.
>> (No piglit change with llvmpipe.)
>> ---
>>   src/gallium/auxiliary/util/u_surface.c | 28
>> ++++++++--------------------
>>   1 file changed, 8 insertions(+), 20 deletions(-)
>>
>> diff --git a/src/gallium/auxiliary/util/u_surface.c
>> b/src/gallium/auxiliary/util/u_surface.c
>> index b9d2da0..8408aa8 100644
>> --- a/src/gallium/auxiliary/util/u_surface.c
>> +++ b/src/gallium/auxiliary/util/u_surface.c
>> @@ -238,14 +238,6 @@ util_fill_box(ubyte * dst,
>>   }
>>
>>
>> -/** Mipmap level size computation, with minimum block size */
>> -static inline unsigned
>> -minify(unsigned value, unsigned levels, unsigned blocksize)
>> -{
>> -   return MAX2(blocksize, value >> levels);
>> -}
>> -
>> -
>>   /**
>>    * Fallback function for pipe->resource_copy_region().
>>    * We support copying between different formats (including compressed/
>> @@ -333,25 +325,21 @@ util_resource_copy_region(struct pipe_context
>> *pipe,
>>      assert(src_box.x % src_bw == 0);
>>      assert(src_box.y % src_bh == 0);
>>      assert(src_box.width % src_bw == 0 ||
>> -          src_box.x + src_box.width == minify(src->width0, src_level,
>> src_bw));
>> +          src_box.x + src_box.width == u_minify(src->width0,
>> src_level));
>>      assert(src_box.height % src_bh == 0 ||
>> -          src_box.y + src_box.height == minify(src->height0,
>> src_level, src_bh));
>> +          src_box.y + src_box.height == u_minify(src->height0,
>> src_level));
>>      assert(dst_box.x % dst_bw == 0);
>>      assert(dst_box.y % dst_bh == 0);
>>      assert(dst_box.width % dst_bw == 0 ||
>> -          dst_box.x + dst_box.width == minify(dst->width0, dst_level,
>> dst_bw));
>> +          dst_box.x + dst_box.width == u_minify(dst->width0,
>> dst_level));
>>      assert(dst_box.height % dst_bh == 0 ||
>> -          dst_box.y + dst_box.height == minify(dst->height0,
>> dst_level, dst_bh));
>> +          dst_box.y + dst_box.height == u_minify(dst->height0,
>> dst_level));
>>
>>      /* check that region boxes are not out of bounds */
>> -   assert(src_box.x + src_box.width <=
>> -          minify(src->width0, src_level, src_bw));
>> -   assert(src_box.y + src_box.height <=
>> -          minify(src->height0, src_level, src_bh));
>> -   assert(dst_box.x + dst_box.width <=
>> -          minify(dst->width0, dst_level, dst_bw));
>> -   assert(dst_box.y + dst_box.height <=
>> -          minify(dst->height0, dst_level, dst_bh));
>> +   assert(src_box.x + src_box.width <= u_minify(src->width0,
>> src_level));
>> +   assert(src_box.y + src_box.height <= u_minify(src->height0,
>> src_level));
>> +   assert(dst_box.x + dst_box.width <= u_minify(dst->width0,
>> dst_level));
>> +   assert(dst_box.y + dst_box.height <= u_minify(dst->height0,
>> dst_level));
>>
>>      /* check that total number of src, dest bytes match */
>>      assert((src_box.width / src_bw) * (src_box.height / src_bh) *
>> src_bs ==
>>
>
> Thanks.
>
> Reviewed-by: Jose Fonseca <jfonseca at vmware.com>

LGTM too.  Thanks, Roland.

Reviewed-by: Brian Paul <brianp at vmware.com>