Re: [Mesa-dev] [PATCH] st/mesa: fix mipmap generation for immutable textures with incomplete pyramids

[Marek Olšák]

On Fri, Oct 23, 2015 at 1:28 AM, Nicolai Hähnle  wrote:
> (This is an alternative to my previous patch, "mesa: clamp MaxLevel for
> immutable textures at initialization"; this patch has no opinion about
> how the spec should be interpreted.)
>
> Without the clamping by NumLevels, the state tracker would reallocate the
> texture storage (incorrect) and even fail to copy the base level image
> after reallocation, leading to the graphical glitch of
> https://bugs.freedesktop.org/show_bug.cgi?id=91993 .
>
> A piglit test has been submitted for review as well (subtest of
> arb_texture_storage-texture-storage).
> ---
>  src/mesa/state_tracker/st_gen_mipmap.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/src/mesa/state_tracker/st_gen_mipmap.c 
> b/src/mesa/state_tracker/st_gen_mipmap.c
> index 26e1c21..3125b2a 100644
> --- a/src/mesa/state_tracker/st_gen_mipmap.c
> +++ b/src/mesa/state_tracker/st_gen_mipmap.c
> @@ -61,6 +61,8 @@ compute_num_levels(struct gl_context *ctx,
>
> numLevels = texObj->BaseLevel + baseImage->MaxNumLevels;
> numLevels = MIN2(numLevels, (GLuint) texObj->MaxLevel + 1);
> +   if (texObj->Immutable)
> +  numLevels = MIN2(numLevels, texObj->NumLevels);
> assert(numLevels >= 1);
>
> return numLevels;

Thanks for looking into this.

Cc: mesa-sta...@lists.freedesktop.org
Reviewed-by: Marek Olšák 

While this may fix the bug, it's not immediately obvious that it won't
reallocate the texture. I think st_generate_mipmap should simply check
the immutable flag and skip all the reallocation code.

Marek
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] st/mesa: fix mipmap generation for immutable textures with incomplete pyramids

[Nicolai Hähnle]

(This is an alternative to my previous patch, "mesa: clamp MaxLevel for
immutable textures at initialization"; this patch has no opinion about
how the spec should be interpreted.)

Without the clamping by NumLevels, the state tracker would reallocate the
texture storage (incorrect) and even fail to copy the base level image
after reallocation, leading to the graphical glitch of
https://bugs.freedesktop.org/show_bug.cgi?id=91993 .

A piglit test has been submitted for review as well (subtest of
arb_texture_storage-texture-storage).
---
 src/mesa/state_tracker/st_gen_mipmap.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/mesa/state_tracker/st_gen_mipmap.c 
b/src/mesa/state_tracker/st_gen_mipmap.c
index 26e1c21..3125b2a 100644
--- a/src/mesa/state_tracker/st_gen_mipmap.c
+++ b/src/mesa/state_tracker/st_gen_mipmap.c
@@ -61,6 +61,8 @@ compute_num_levels(struct gl_context *ctx,
 
numLevels = texObj->BaseLevel + baseImage->MaxNumLevels;
numLevels = MIN2(numLevels, (GLuint) texObj->MaxLevel + 1);
+   if (texObj->Immutable)
+  numLevels = MIN2(numLevels, texObj->NumLevels);
assert(numLevels >= 1);
 
return numLevels;
-- 
2.1.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev