[Mesa-dev] [PATCH] gallium/util: Fix off-by-one in box intersection

[Eric Anholt]

Boris Brezillon <boris.brezillon at collabora.com> writes:

> From: Daniel Stone <daniels at collabora.com>
>
> pipe_boxes are x/y + width/height, rather than x0/y0 -> x1/y1. This
> means that (x+width) is not included in the box.
>
> The box intersection check was seemingly written for inclusive regions,
> and would falsely assert that adjacent boxes would overlap.
>
> Fix the off-by-one by being one pixel less greedy.
>
> Signed-off-by: Daniel Stone <daniels at collabora.com>
> Signed-off-by: Boris Brezillon <boris.brezillon at collabora.com>
> ---
>  src/gallium/auxiliary/util/u_box.h | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/src/gallium/auxiliary/util/u_box.h b/src/gallium/auxiliary/util/u_box.h
> index b3f478e7bfc4..ead7189ecaf8 100644
> --- a/src/gallium/auxiliary/util/u_box.h
> +++ b/src/gallium/auxiliary/util/u_box.h
> @@ -161,15 +161,15 @@ u_box_test_intersection_2d(const struct pipe_box *a,
>     unsigned i;
>     int a_l[2], a_r[2], b_l[2], b_r[2];
>  
> -   a_l[0] = MIN2(a->x, a->x + a->width);
> -   a_r[0] = MAX2(a->x, a->x + a->width);
> -   a_l[1] = MIN2(a->y, a->y + a->height);
> -   a_r[1] = MAX2(a->y, a->y + a->height);
> +   a_l[0] = MIN2(a->x, a->x + a->width - 1);
> +   a_r[0] = MAX2(a->x, a->x + a->width - 1);
> +   a_l[1] = MIN2(a->y, a->y + a->height - 1);
> +   a_r[1] = MAX2(a->y, a->y + a->height - 1);
>  
> -   b_l[0] = MIN2(b->x, b->x + b->width);
> -   b_r[0] = MAX2(b->x, b->x + b->width);
> -   b_l[1] = MIN2(b->y, b->y + b->height);
> -   b_r[1] = MAX2(b->y, b->y + b->height);
> +   b_l[0] = MIN2(b->x, b->x + b->width - 1);
> +   b_r[0] = MAX2(b->x, b->x + b->width - 1);
> +   b_l[1] = MIN2(b->y, b->y + b->height - 1);
> +   b_r[1] = MAX2(b->y, b->y + b->height - 1);
>  
>     for (i = 0; i < 2; ++i) {
>        if (a_l[i] > b_r[i] || a_r[i] < b_l[i])

All this min/maxing is about trying to find the bounds if width/height
is negative (indicating a flip in blits), right?  So, when width/height
are negative, I think you end up expanding the range instead of
shrinking it like you intended.

(I think the negative width/height thing is a serious misfeature and we
should just insist that pipe_boxes don't have it and fix up blits to
pass the flip state separately)

FWIW, I do definitely think this function should be fixed. It's called
u_box_test_intersection, not u_box_test_intersection_or_adjacency.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20190301/078c1faf/attachment.sig>