On Thu, Feb 28, 2019 at 8:15 PM Roland Scheidegger <srol...@vmware.com> wrote: > > Am 01.03.19 um 00:28 schrieb Gurchetan Singh: > > On Thu, Feb 28, 2019 at 12:39 AM Boris Brezillon > > <boris.brezil...@collabora.com> wrote: > >> > >> Hello Gurchetan, > >> > >> On Wed, 27 Feb 2019 10:34:26 -0800 > >> Gurchetan Singh <gurchetansi...@chromium.org> wrote: > >> > >>> On Mon, Feb 25, 2019 at 12:35 AM Boris Brezillon > >>> <boris.brezil...@collabora.com> wrote: > >>>> > >>>> From: Daniel Stone <dani...@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. > > > > 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.4&data=02%7C01%7Csroland%40vmware.com%7Ce72daea7c212452556f208d69dd47aa1%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C636869933286320567&sdata=lHnhl1gM19Gt%2FU3KVv%2FlpBgPXFoSl4BqwZ93yHgbbRQ%3D&reserved=0 > >> > >>> > >>>> > >>>> Signed-off-by: Daniel Stone <dani...@collabora.com> > >>>> Signed-off-by: Boris Brezillon <boris.brezil...@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@lists.freedesktop.org > >>>> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fmesa-dev&data=02%7C01%7Csroland%40vmware.com%7Ce72daea7c212452556f208d69dd47aa1%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C636869933286320567&sdata=%2FSECNIFewcH6gECXxq94DXvX6QfN8PEKpDQd3h%2Boxz8%3D&reserved=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-dev&data=02%7C01%7Csroland%40vmware.com%7Ce72daea7c212452556f208d69dd47aa1%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C636869933286320567&sdata=%2FSECNIFewcH6gECXxq94DXvX6QfN8PEKpDQd3h%2Boxz8%3D&reserved=0 > > > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
