[Mesa-dev] [PATCH] mesa: clamp MaxLevel for immutable textures at initialization

2015-10-22 Thread Nicolai Hähnle
The same clamping already happens for glTexParameteri. This change
also fixes a bug in mipmap generation, see
https://bugs.freedesktop.org/show_bug.cgi?id=91993

piglit test cases have been submitted for review (as additions to
arb_texture_storage-texture-storage and arb_texture_view-max-level).
---
 src/mesa/main/textureview.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/mesa/main/textureview.c b/src/mesa/main/textureview.c
index 04b7d73..b6eaa77 100644
--- a/src/mesa/main/textureview.c
+++ b/src/mesa/main/textureview.c
@@ -408,6 +408,8 @@ _mesa_set_texture_view_state(struct gl_context *ctx,
   texObj->NumLayers = 6;
   break;
}
+
+   texObj->MaxLevel = MIN2(texObj->MaxLevel, texObj->ImmutableLevels - 1);
 }
 
 /**
@@ -680,6 +682,7 @@ _mesa_TextureView(GLuint texture, GLenum target, GLuint 
origtexture,
texObj->NumLayers = newViewNumLayers;
texObj->Immutable = GL_TRUE;
texObj->ImmutableLevels = origTexObj->ImmutableLevels;
+   texObj->MaxLevel = MIN2(texObj->MaxLevel, texObj->ImmutableLevels - 1);
texObj->Target = target;
texObj->TargetIndex = _mesa_tex_target_to_index(ctx, target);
assert(texObj->TargetIndex < NUM_TEXTURE_TARGETS);
-- 
2.1.4

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


Re: [Mesa-dev] [PATCH] mesa: clamp MaxLevel for immutable textures at initialization

2015-10-22 Thread Fredrik Höglund
On Thursday 22 October 2015, Nicolai Hähnle wrote:
> The same clamping already happens for glTexParameteri. This change
> also fixes a bug in mipmap generation, see
> https://bugs.freedesktop.org/show_bug.cgi?id=91993

I don't think this patch is correct.  The ARB_texture_view specification
doesn't say that MaxLevel should be initialized to the value of
TEXTURE_IMMUTABLE_LEVELS, only that it's interpreted relative to
the view and not relative to the original data store.

Liam Middlebrook also pointed out recently that the clamping done
in glTexParameteri is in fact a bug:

http://lists.freedesktop.org/archives/piglit/2015-June/016342.html

The language in the specification that says that MaxLevel is clamped
when the texture is immutable applies to texture minification,
magnification, and texture completeness; not to gl*Tex*Parameter*.

> piglit test cases have been submitted for review (as additions to
> arb_texture_storage-texture-storage and arb_texture_view-max-level).
>
> ---
>  src/mesa/main/textureview.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/src/mesa/main/textureview.c b/src/mesa/main/textureview.c
> index 04b7d73..b6eaa77 100644
> --- a/src/mesa/main/textureview.c
> +++ b/src/mesa/main/textureview.c
> @@ -408,6 +408,8 @@ _mesa_set_texture_view_state(struct gl_context *ctx,
>texObj->NumLayers = 6;
>break;
> }
> +
> +   texObj->MaxLevel = MIN2(texObj->MaxLevel, texObj->ImmutableLevels - 1);
>  }
>  
>  /**
> @@ -680,6 +682,7 @@ _mesa_TextureView(GLuint texture, GLenum target, GLuint 
> origtexture,
> texObj->NumLayers = newViewNumLayers;
> texObj->Immutable = GL_TRUE;
> texObj->ImmutableLevels = origTexObj->ImmutableLevels;
> +   texObj->MaxLevel = MIN2(texObj->MaxLevel, texObj->ImmutableLevels - 1);
> texObj->Target = target;
> texObj->TargetIndex = _mesa_tex_target_to_index(ctx, target);
> assert(texObj->TargetIndex < NUM_TEXTURE_TARGETS);
> 

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


Re: [Mesa-dev] [PATCH] mesa: clamp MaxLevel for immutable textures at initialization

2015-10-22 Thread Ilia Mirkin
On Thu, Oct 22, 2015 at 12:03 PM, Nicolai Hähnle  wrote:
> On 22.10.2015 15:57, Fredrik Höglund wrote:
>>
>> On Thursday 22 October 2015, Nicolai Hähnle wrote:
>>>
>>> The same clamping already happens for glTexParameteri. This change
>>> also fixes a bug in mipmap generation, see
>>> https://bugs.freedesktop.org/show_bug.cgi?id=91993
>>
>>
>> I don't think this patch is correct.  The ARB_texture_view specification
>> doesn't say that MaxLevel should be initialized to the value of
>> TEXTURE_IMMUTABLE_LEVELS, only that it's interpreted relative to
>> the view and not relative to the original data store.
>>
>> Liam Middlebrook also pointed out recently that the clamping done
>> in glTexParameteri is in fact a bug:
>>
>> http://lists.freedesktop.org/archives/piglit/2015-June/016342.html
>>
>> The language in the specification that says that MaxLevel is clamped
>> when the texture is immutable applies to texture minification,
>> magnification, and texture completeness; not to gl*Tex*Parameter*.
>
>
> Ugh. I was torn between those two interpretations. I suppose nobody was
> confident enough to change gl*Tex*Parameter* either ;)
>
> Thinking more on this, there is also a problematic interaction between
> glTextureView and glGenerate*Mipmap when the view does not extend to the
> highest level in the underlying texture. Clearly, this part of the spec
> could use some cleanups.
>
> Any chance of an "official" clarification? I did not find corresponding
> Issues in the corresponding extensions.

Check the actual GL spec -- a lot of the time such things are
clarified in there.

> What are non-Mesa drivers doing?

Write some piglits and find out? :)

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


Re: [Mesa-dev] [PATCH] mesa: clamp MaxLevel for immutable textures at initialization

2015-10-22 Thread Nicolai Hähnle

On 22.10.2015 15:57, Fredrik Höglund wrote:

On Thursday 22 October 2015, Nicolai Hähnle wrote:

The same clamping already happens for glTexParameteri. This change
also fixes a bug in mipmap generation, see
https://bugs.freedesktop.org/show_bug.cgi?id=91993


I don't think this patch is correct.  The ARB_texture_view specification
doesn't say that MaxLevel should be initialized to the value of
TEXTURE_IMMUTABLE_LEVELS, only that it's interpreted relative to
the view and not relative to the original data store.

Liam Middlebrook also pointed out recently that the clamping done
in glTexParameteri is in fact a bug:

http://lists.freedesktop.org/archives/piglit/2015-June/016342.html

The language in the specification that says that MaxLevel is clamped
when the texture is immutable applies to texture minification,
magnification, and texture completeness; not to gl*Tex*Parameter*.


Ugh. I was torn between those two interpretations. I suppose nobody was 
confident enough to change gl*Tex*Parameter* either ;)


Thinking more on this, there is also a problematic interaction between 
glTextureView and glGenerate*Mipmap when the view does not extend to the 
highest level in the underlying texture. Clearly, this part of the spec 
could use some cleanups.


Any chance of an "official" clarification? I did not find corresponding 
Issues in the corresponding extensions. What are non-Mesa drivers doing?


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