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

[Boris Brezillon]

Hello Gurchetan,

On Wed, 27 Feb 2019 10:34:26 -0800
Gurchetan Singh <gurchetansingh at chromium.org> wrote:

> On Mon, Feb 25, 2019 at 12:35 AM Boris Brezillon
> <boris.brezillon at collabora.com> wrote:
> >
> > 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.  
> 
> Is there a reason for this change?  I only see this used in a warning
> in the nine state tracker and virgl (where reporting adjacent
> intersections is preferred).

This patch was part of a series Daniel worked on to optimize texture
atlas updates on Vivante GPUs [1]. In the end, this work has been put
on hold because the perf optimization was not as high as expected, but
it might be resurrected at some point.
Anyway, back to the point. In this patchset, the pipe_region_overlaps()
helper needs to check when regions overlap and not when they're
adjacent. If other users need u_box_test_intersection_2d() to also
detect when boxes are adjacent, then we should definitely keep the code
unchanged, but maybe it's worth a comment in the code to clarify the
behavior.

Regards,

Boris

[1]https://gitlab.collabora.com/bbrezillon/mesa/commits/etna-texture-atlas-18.2.4

> 
> >
> > 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])
> > --
> > 2.20.1
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev