Re: [Mesa-dev] [PATCH] gallium/util: Fix off-by-one in box intersection
On Thu, Feb 28, 2019 at 8:15 PM Roland Scheidegger wrote:
>
> Am 01.03.19 um 00:28 schrieb Gurchetan Singh:
> > On Thu, Feb 28, 2019 at 12:39 AM Boris Brezillon
> > wrote:
> >>
> >> Hello Gurchetan,
> >>
> >> On Wed, 27 Feb 2019 10:34:26 -0800
> >> Gurchetan Singh wrote:
> >>
> >>> On Mon, Feb 25, 2019 at 12:35 AM Boris Brezillon
> >>> wrote:
>
> From: Daniel Stone
>
> 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.
> >
> > Thanks for the information. You can just modify this function to be
> > something like:
> >
> > u_box_test_intersection_2d(const struct pipe_box *a, const struct
> > pipe_box *b, boolean adjacent_allowed)
> >
> > [or add another function --- whatever you prefer]
> >
> > That way we can keep behavior for virgl/nine unchanged.
>
> I can't see why you'd want to know if the regions are adjacent?
> If they are adjacent you can still do blits etc. without having to worry
> about overwriting src regions etc.
> Now for 1d regions (buffers) I could see adjacent being useful - could
> use that to combine multiple ranges into one for instance. But I don't
> think you'd want to use a 2d intersect test for that...
virgl piggybacks off u_box_test_intersection_2d to do 1d adjacency +
intersection tests. If the preferred approach is to split off an
u_box_test_intersection_1d function, that sounds fine.
>
> Roland
>
>
>
> >
> >>
> >> Regards,
> >>
> >> Boris
> >>
> >> [1]https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.collabora.com%2Fbbrezillon%2Fmesa%2Fcommits%2Fetna-texture-atlas-18.2.4data=02%7C01%7Csroland%40vmware.com%7Ce72daea7c212452556f208d69dd47aa1%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C636869933286320567sdata=lHnhl1gM19Gt%2FU3KVv%2FlpBgPXFoSl4BqwZ93yHgbbRQ%3Dreserved=0
> >>
> >>>
>
> Signed-off-by: Daniel Stone
> Signed-off-by: Boris Brezillon
> ---
> 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@lists.freedesktop.org
> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fmesa-devdata=02%7C01%7Csroland%40vmware.com%7Ce72daea7c212452556f208d69dd47aa1%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C636869933286320567sdata=%2FSECNIFewcH6gECXxq94DXvX6QfN8PEKpDQd3h%2Boxz8%3Dreserved=0
> >>
> >
Re: [Mesa-dev] [PATCH] gallium/util: Fix off-by-one in box intersection
Boris Brezillon writes:
> From: Daniel Stone
>
> 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
> Signed-off-by: Boris Brezillon
> ---
> 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.
signature.asc
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] gallium/util: Fix off-by-one in box intersection
Am 01.03.19 um 00:28 schrieb Gurchetan Singh:
> On Thu, Feb 28, 2019 at 12:39 AM Boris Brezillon
> wrote:
>>
>> Hello Gurchetan,
>>
>> On Wed, 27 Feb 2019 10:34:26 -0800
>> Gurchetan Singh wrote:
>>
>>> On Mon, Feb 25, 2019 at 12:35 AM Boris Brezillon
>>> wrote:
From: Daniel Stone
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.
>
> Thanks for the information. You can just modify this function to be
> something like:
>
> u_box_test_intersection_2d(const struct pipe_box *a, const struct
> pipe_box *b, boolean adjacent_allowed)
>
> [or add another function --- whatever you prefer]
>
> That way we can keep behavior for virgl/nine unchanged.
I can't see why you'd want to know if the regions are adjacent?
If they are adjacent you can still do blits etc. without having to worry
about overwriting src regions etc.
Now for 1d regions (buffers) I could see adjacent being useful - could
use that to combine multiple ranges into one for instance. But I don't
think you'd want to use a 2d intersect test for that...
Roland
>
>>
>> Regards,
>>
>> Boris
>>
>> [1]https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.collabora.com%2Fbbrezillon%2Fmesa%2Fcommits%2Fetna-texture-atlas-18.2.4data=02%7C01%7Csroland%40vmware.com%7Ce72daea7c212452556f208d69dd47aa1%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C636869933286320567sdata=lHnhl1gM19Gt%2FU3KVv%2FlpBgPXFoSl4BqwZ93yHgbbRQ%3Dreserved=0
>>
>>>
Signed-off-by: Daniel Stone
Signed-off-by: Boris Brezillon
---
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@lists.freedesktop.org
https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fmesa-devdata=02%7C01%7Csroland%40vmware.com%7Ce72daea7c212452556f208d69dd47aa1%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C636869933286320567sdata=%2FSECNIFewcH6gECXxq94DXvX6QfN8PEKpDQd3h%2Boxz8%3Dreserved=0
>>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fmesa-devdata=02%7C01%7Csroland%40vmware.com%7Ce72daea7c212452556f208d69dd47aa1%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C636869933286320567sdata=%2FSECNIFewcH6gECXxq94DXvX6QfN8PEKpDQd3h%2Boxz8%3Dreserved=0
>
___
mesa-dev mailing list
Re: [Mesa-dev] [PATCH] gallium/util: Fix off-by-one in box intersection
On Thu, Feb 28, 2019 at 12:39 AM Boris Brezillon
wrote:
>
> Hello Gurchetan,
>
> On Wed, 27 Feb 2019 10:34:26 -0800
> Gurchetan Singh wrote:
>
> > On Mon, Feb 25, 2019 at 12:35 AM Boris Brezillon
> > wrote:
> > >
> > > From: Daniel Stone
> > >
> > > 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.
Thanks for the information. You can just modify this function to be
something like:
u_box_test_intersection_2d(const struct pipe_box *a, const struct
pipe_box *b, boolean adjacent_allowed)
[or add another function --- whatever you prefer]
That way we can keep behavior for virgl/nine unchanged.
>
> Regards,
>
> Boris
>
> [1]https://gitlab.collabora.com/bbrezillon/mesa/commits/etna-texture-atlas-18.2.4
>
> >
> > >
> > > Signed-off-by: Daniel Stone
> > > Signed-off-by: Boris Brezillon
> > > ---
> > > 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@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] gallium/util: Fix off-by-one in box intersection
Hello Gurchetan,
On Wed, 27 Feb 2019 10:34:26 -0800
Gurchetan Singh wrote:
> On Mon, Feb 25, 2019 at 12:35 AM Boris Brezillon
> wrote:
> >
> > From: Daniel Stone
> >
> > 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
> > Signed-off-by: Boris Brezillon
> > ---
> > 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@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] gallium/util: Fix off-by-one in box intersection
On Mon, Feb 25, 2019 at 12:35 AM Boris Brezillon
wrote:
>
> From: Daniel Stone
>
> 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).
>
> Signed-off-by: Daniel Stone
> Signed-off-by: Boris Brezillon
> ---
> 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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] gallium/util: Fix off-by-one in box intersection
From: Daniel Stone
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
Signed-off-by: Boris Brezillon
---
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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
