Re: [Mesa-dev] [PATCH] st/mesa: ensure that images don't try to reference non-existent levels
The problem is that st_finalize_texture isn't doing its job in some circumstances. Specifically, if I have a 2-level 2d array with level0 = (2,2,7) and level1 = (1,1,7) with a non-mipmap filter (e.g. GL_NEAREST), then at no point are two resources combined into one. Clamping the level will avoid the assert, but it's not the right thing either since it'll still be passing in the wrong resource. On Mon, Mar 5, 2018 at 2:05 PM, Marek Olšákwrote: > Maybe st/mesa should clamp the level instead of assert. > > Marek > > On Thu, Mar 1, 2018 at 7:23 AM, Ilia Mirkin wrote: >> Yes, as I mentioned this makes some tests assert. >> >> They were passing before, but it was through luck since the actual images >> were never accessed. >> >> On Mar 1, 2018 04:04, "Timothy Arceri" wrote: >>> >>> This causes the CTS tests to assert on radeonsi where they previously >>> passed. If that expected? >>> >>> On 27/02/18 16:19, Ilia Mirkin wrote: Ideally the st_finalize_texture call would take care of that, but it doesn't seem to with KHR-GL45.shader_image_size.advanced-nonMS-*. This assertions makes sure that no such values are passed to the driver. Signed-off-by: Ilia Mirkin --- This will trigger asserts in CTS, but I think that's better than feeding broken values to driver backends. src/mesa/state_tracker/st_atom_image.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/mesa/state_tracker/st_atom_image.c b/src/mesa/state_tracker/st_atom_image.c index 1c4980173f4..421c926cf04 100644 --- a/src/mesa/state_tracker/st_atom_image.c +++ b/src/mesa/state_tracker/st_atom_image.c @@ -97,6 +97,7 @@ st_convert_image(const struct st_context *st, const struct gl_image_unit *u, img->resource = stObj->pt; img->u.tex.level = u->Level + stObj->base.MinLevel; + assert(img->u.tex.level <= img->resource->last_level); if (stObj->pt->target == PIPE_TEXTURE_3D) { if (u->Layered) { img->u.tex.first_layer = 0; >> >> ___ >> 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] st/mesa: ensure that images don't try to reference non-existent levels
Maybe st/mesa should clamp the level instead of assert. Marek On Thu, Mar 1, 2018 at 7:23 AM, Ilia Mirkinwrote: > Yes, as I mentioned this makes some tests assert. > > They were passing before, but it was through luck since the actual images > were never accessed. > > On Mar 1, 2018 04:04, "Timothy Arceri" wrote: >> >> This causes the CTS tests to assert on radeonsi where they previously >> passed. If that expected? >> >> On 27/02/18 16:19, Ilia Mirkin wrote: >>> >>> Ideally the st_finalize_texture call would take care of that, but it >>> doesn't seem to with KHR-GL45.shader_image_size.advanced-nonMS-*. This >>> assertions makes sure that no such values are passed to the driver. >>> >>> Signed-off-by: Ilia Mirkin >>> --- >>> >>> This will trigger asserts in CTS, but I think that's better than feeding >>> broken values to driver backends. >>> >>> src/mesa/state_tracker/st_atom_image.c | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git a/src/mesa/state_tracker/st_atom_image.c >>> b/src/mesa/state_tracker/st_atom_image.c >>> index 1c4980173f4..421c926cf04 100644 >>> --- a/src/mesa/state_tracker/st_atom_image.c >>> +++ b/src/mesa/state_tracker/st_atom_image.c >>> @@ -97,6 +97,7 @@ st_convert_image(const struct st_context *st, const >>> struct gl_image_unit *u, >>> img->resource = stObj->pt; >>> img->u.tex.level = u->Level + stObj->base.MinLevel; >>> + assert(img->u.tex.level <= img->resource->last_level); >>> if (stObj->pt->target == PIPE_TEXTURE_3D) { >>>if (u->Layered) { >>> img->u.tex.first_layer = 0; >>> > > ___ > 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] st/mesa: ensure that images don't try to reference non-existent levels
Yes, as I mentioned this makes some tests assert. They were passing before, but it was through luck since the actual images were never accessed. On Mar 1, 2018 04:04, "Timothy Arceri"wrote: > This causes the CTS tests to assert on radeonsi where they previously > passed. If that expected? > > On 27/02/18 16:19, Ilia Mirkin wrote: > >> Ideally the st_finalize_texture call would take care of that, but it >> doesn't seem to with KHR-GL45.shader_image_size.advanced-nonMS-*. This >> assertions makes sure that no such values are passed to the driver. >> >> Signed-off-by: Ilia Mirkin >> --- >> >> This will trigger asserts in CTS, but I think that's better than feeding >> broken values to driver backends. >> >> src/mesa/state_tracker/st_atom_image.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/src/mesa/state_tracker/st_atom_image.c >> b/src/mesa/state_tracker/st_atom_image.c >> index 1c4980173f4..421c926cf04 100644 >> --- a/src/mesa/state_tracker/st_atom_image.c >> +++ b/src/mesa/state_tracker/st_atom_image.c >> @@ -97,6 +97,7 @@ st_convert_image(const struct st_context *st, const >> struct gl_image_unit *u, >> img->resource = stObj->pt; >> img->u.tex.level = u->Level + stObj->base.MinLevel; >> + assert(img->u.tex.level <= img->resource->last_level); >> if (stObj->pt->target == PIPE_TEXTURE_3D) { >>if (u->Layered) { >> img->u.tex.first_layer = 0; >> >> ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] st/mesa: ensure that images don't try to reference non-existent levels
This causes the CTS tests to assert on radeonsi where they previously passed. If that expected? On 27/02/18 16:19, Ilia Mirkin wrote: Ideally the st_finalize_texture call would take care of that, but it doesn't seem to with KHR-GL45.shader_image_size.advanced-nonMS-*. This assertions makes sure that no such values are passed to the driver. Signed-off-by: Ilia Mirkin--- This will trigger asserts in CTS, but I think that's better than feeding broken values to driver backends. src/mesa/state_tracker/st_atom_image.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/mesa/state_tracker/st_atom_image.c b/src/mesa/state_tracker/st_atom_image.c index 1c4980173f4..421c926cf04 100644 --- a/src/mesa/state_tracker/st_atom_image.c +++ b/src/mesa/state_tracker/st_atom_image.c @@ -97,6 +97,7 @@ st_convert_image(const struct st_context *st, const struct gl_image_unit *u, img->resource = stObj->pt; img->u.tex.level = u->Level + stObj->base.MinLevel; + assert(img->u.tex.level <= img->resource->last_level); if (stObj->pt->target == PIPE_TEXTURE_3D) { if (u->Layered) { img->u.tex.first_layer = 0; ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] st/mesa: ensure that images don't try to reference non-existent levels
Reviewed-by: Marek OlšákMarek On Tue, Feb 27, 2018 at 6:19 AM, Ilia Mirkin wrote: > Ideally the st_finalize_texture call would take care of that, but it > doesn't seem to with KHR-GL45.shader_image_size.advanced-nonMS-*. This > assertions makes sure that no such values are passed to the driver. > > Signed-off-by: Ilia Mirkin > --- > > This will trigger asserts in CTS, but I think that's better than feeding > broken values to driver backends. > > src/mesa/state_tracker/st_atom_image.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/src/mesa/state_tracker/st_atom_image.c > b/src/mesa/state_tracker/st_atom_image.c > index 1c4980173f4..421c926cf04 100644 > --- a/src/mesa/state_tracker/st_atom_image.c > +++ b/src/mesa/state_tracker/st_atom_image.c > @@ -97,6 +97,7 @@ st_convert_image(const struct st_context *st, const struct > gl_image_unit *u, > >img->resource = stObj->pt; >img->u.tex.level = u->Level + stObj->base.MinLevel; > + assert(img->u.tex.level <= img->resource->last_level); >if (stObj->pt->target == PIPE_TEXTURE_3D) { > if (u->Layered) { > img->u.tex.first_layer = 0; > -- > 2.16.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] st/mesa: ensure that images don't try to reference non-existent levels
Ideally the st_finalize_texture call would take care of that, but it doesn't seem to with KHR-GL45.shader_image_size.advanced-nonMS-*. This assertions makes sure that no such values are passed to the driver. Signed-off-by: Ilia Mirkin--- This will trigger asserts in CTS, but I think that's better than feeding broken values to driver backends. src/mesa/state_tracker/st_atom_image.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/mesa/state_tracker/st_atom_image.c b/src/mesa/state_tracker/st_atom_image.c index 1c4980173f4..421c926cf04 100644 --- a/src/mesa/state_tracker/st_atom_image.c +++ b/src/mesa/state_tracker/st_atom_image.c @@ -97,6 +97,7 @@ st_convert_image(const struct st_context *st, const struct gl_image_unit *u, img->resource = stObj->pt; img->u.tex.level = u->Level + stObj->base.MinLevel; + assert(img->u.tex.level <= img->resource->last_level); if (stObj->pt->target == PIPE_TEXTURE_3D) { if (u->Layered) { img->u.tex.first_layer = 0; -- 2.16.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev