Am 25.10.2017 um 16:57 schrieb Roland Scheidegger: > Am 25.10.2017 um 16:29 schrieb Brian Paul: >> On 10/24/2017 07:06 PM, [email protected] wrote: >>> From: Roland Scheidegger <[email protected]> >>> >>> These assertions were revisited a couple of times in the past, and they >>> still weren't quite right. >>> The problem I was seeing (with some other state tracker) was a copy >>> between >>> two 512x512 s3tc textures, but from mip level 0 to mip level 8. >>> Therefore, >>> the destination has only size 2x2 (not a full block), so the box >>> width/height >>> was only 2, causing the assertion to trigger for src alignment. >>> As far as I can tell, such a copy is completely legal, and because a >>> correct >>> assertion would get ridiculously complicated just get rid of it for good. >>> --- >>> src/gallium/auxiliary/util/u_surface.c | 8 -------- >>> 1 file changed, 8 deletions(-) >>> >>> diff --git a/src/gallium/auxiliary/util/u_surface.c >>> b/src/gallium/auxiliary/util/u_surface.c >>> index 5abf966..0a79a25 100644 >>> --- a/src/gallium/auxiliary/util/u_surface.c >>> +++ b/src/gallium/auxiliary/util/u_surface.c >>> @@ -324,16 +324,8 @@ util_resource_copy_region(struct pipe_context *pipe, >>> /* check that region boxes are block aligned */ >>> 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 == u_minify(src->width0, >>> src_level)); >>> - assert(src_box.height % src_bh == 0 || >>> - 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 == u_minify(dst->width0, >>> dst_level)); >>> - assert(dst_box.height % dst_bh == 0 || >>> - 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 <= u_minify(src->width0, >>> src_level)); >>> >> >> Would one alternative be to put the assertions inside a conditional >> testing that the texture is at least 4x4? That'd keep the checking in >> place for common cases. > > You mean like > if (u_minify(dst->width0, dst_level) >= dst_bw) { > assert(src_box.width % src_bw == 0 || > src_box.x + src_box.width == u_minify(src->width0, src_level) > } > and same for height? > And (maybe) the same for the other direction (I'm not quite sure if the > opposite direction actually should be legal - if you copy a 2x2 s3tc > texture to a larger one, you'll end up with undefined pixels albeit > certainly they will be encoded in the partial source block). > I thought about this but figured ultimately the assertions have more > complexity than the actual code. (And it's not that it is really > critical assertions, since whole blocks should always be copied in any > case, it's just that is is usually illegal to specify non-full blocks, > except there's tons of exceptions...)
Err, I just realized this doesn't even work. If the dst block compressed texture for instance is 5x5 and the src larger, it will wrongly trigger all the same. Roland _______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
