Re: [Mesa-dev] [PATCH] gallium/util: don't use blocksize for minify for assertions
On 06/14/2016 03:39 AM, Jose Fonseca wrote: On 14/06/16 02:35, srol...@vmware.com wrote: From: Roland ScheideggerThe 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 LGTM too. Thanks, Roland. Reviewed-by: Brian Paul ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] gallium/util: don't use blocksize for minify for assertions
On 14/06/16 02:35, srol...@vmware.com wrote: From: Roland ScheideggerThe 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 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] gallium/util: don't use blocksize for minify for assertions
From: Roland ScheideggerThe 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 == -- 2.1.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev