On 10/25/2017 09:54 AM, Roland Scheidegger wrote:
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.

OK, just remove the assertions then.

-Brian


_______________________________________________
mesa-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to