[2/4] arb_texture_view: Constrain accessible mip levels correctly

Submitted by Chris Forbes on Jan. 12, 2014, 12:39 a.m.

Details

Message ID 1389487143-3010-2-git-send-email-chrisf@ijw.co.nz
State New
Headers show

Not browsing as part of any series.

Commit Message

Chris Forbes Jan. 12, 2014, 12:39 a.m.
The test expects us to be sampling from the smallest miplevel accessible
in the view, but the TEXTURE_MAX_LEVEL constraint was out of range,
and TEXTURE_BASE_LEVEL wasn't set at all.

This would have been masked previously because the test fell over due to
use of legacy features in the core profile.

Signed-off-by: Chris Forbes <chrisf@ijw.co.nz>
---
 tests/spec/arb_texture_view/rendering_levels.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Patch hide | download patch | download mbox

diff --git a/tests/spec/arb_texture_view/rendering_levels.c b/tests/spec/arb_texture_view/rendering_levels.c
index 7f02613..ef29f78 100644
--- a/tests/spec/arb_texture_view/rendering_levels.c
+++ b/tests/spec/arb_texture_view/rendering_levels.c
@@ -94,7 +94,8 @@  test_render_levels(void)
 		glTextureView(new_tex, GL_TEXTURE_2D, tex,  GL_RGBA8, l,
 			      numLevels[l], 0, 1);
 		glBindTexture(GL_TEXTURE_2D, new_tex);
-		glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAX_LEVEL, levels-1);
+		glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_BASE_LEVEL, numLevels[l] - 1);
+		glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAX_LEVEL, numLevels[l] - 1);
 
 		glClear(GL_COLOR_BUFFER_BIT);
 

Comments

Hey Chris:

I think the texture level used should be clamped when using immutable 
texture so if
the MAX_LEVEL is out of range it shouldn't matter.  Also an unset 
BASE_LEVEL should default to 0
which is relative to the view.

 From arb_texture_storage:
"

Redefine level_base, level_max to be clamped forms of
         TEXTURE_BASE_LEVEL/TEXTURE_MAX_LEVEL when using immutable
         textures
"

Which appears to be implemented here in Mesa code:
     /** From ARB_texture_storage:
        * However, if TEXTURE_IMMUTABLE_FORMAT is TRUE, then level_base is
        * clamped to the range [0, <levels> - 1] and level_max is then clamped to
        * the range [level_base, <levels> - 1], where <levels> is the parameter
        * passed the call to TexStorage* for the texture object.
        */
       if (texObj->Immutable)
           texObj->MaxLevel = CLAMP(params[0], texObj->BaseLevel,
                                    texObj->ImmutableLevels - 1);
       else
          texObj->MaxLevel = params[0];


Jon
On 01/11/2014 05:39 PM, Chris Forbes wrote:
> The test expects us to be sampling from the smallest miplevel accessible
> in the view, but the TEXTURE_MAX_LEVEL constraint was out of range,
> and TEXTURE_BASE_LEVEL wasn't set at all.
>
> This would have been masked previously because the test fell over due to
> use of legacy features in the core profile.
>
> Signed-off-by: Chris Forbes <chrisf@ijw.co.nz>
> ---
>   tests/spec/arb_texture_view/rendering_levels.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/tests/spec/arb_texture_view/rendering_levels.c b/tests/spec/arb_texture_view/rendering_levels.c
> index 7f02613..ef29f78 100644
> --- a/tests/spec/arb_texture_view/rendering_levels.c
> +++ b/tests/spec/arb_texture_view/rendering_levels.c
> @@ -94,7 +94,8 @@ test_render_levels(void)
>   		glTextureView(new_tex, GL_TEXTURE_2D, tex,  GL_RGBA8, l,
>   			      numLevels[l], 0, 1);
>   		glBindTexture(GL_TEXTURE_2D, new_tex);
> -		glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAX_LEVEL, levels-1);
> +		glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_BASE_LEVEL, numLevels[l] - 1);
> +		glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAX_LEVEL, numLevels[l] - 1);
>   
>   		glClear(GL_COLOR_BUFFER_BIT);
>
You're right, this one is a bit odd, and the commit message didn't
really explain what I was getting at...

Most of what I was trying to do here is eliminate the hardware lod
selection as a complicating factor -- since we're really testing that
levels are view-relative. It's possible that I've misunderstood what
you were trying to do though and destroyed the essence of it. What do
you think?

On Tue, Jan 14, 2014 at 1:58 PM, Jon Ashburn <jon@lunarg.com> wrote:
> Hey Chris:
>
> I think the texture level used should be clamped when using immutable
> texture so if
> the MAX_LEVEL is out of range it shouldn't matter.  Also an unset BASE_LEVEL
> should default to 0
> which is relative to the view.
>
> From arb_texture_storage:
> "
>
> Redefine level_base, level_max to be clamped forms of
>         TEXTURE_BASE_LEVEL/TEXTURE_MAX_LEVEL when using immutable
>         textures
> "
>
> Which appears to be implemented here in Mesa code:
>     /** From ARB_texture_storage:
>        * However, if TEXTURE_IMMUTABLE_FORMAT is TRUE, then level_base is
>        * clamped to the range [0, <levels> - 1] and level_max is then
> clamped to
>        * the range [level_base, <levels> - 1], where <levels> is the
> parameter
>        * passed the call to TexStorage* for the texture object.
>        */
>       if (texObj->Immutable)
>           texObj->MaxLevel = CLAMP(params[0], texObj->BaseLevel,
>                                    texObj->ImmutableLevels - 1);
>       else
>          texObj->MaxLevel = params[0];
>
>
> Jon
>
> On 01/11/2014 05:39 PM, Chris Forbes wrote:
>>
>> The test expects us to be sampling from the smallest miplevel accessible
>> in the view, but the TEXTURE_MAX_LEVEL constraint was out of range,
>> and TEXTURE_BASE_LEVEL wasn't set at all.
>>
>> This would have been masked previously because the test fell over due to
>> use of legacy features in the core profile.
>>
>> Signed-off-by: Chris Forbes <chrisf@ijw.co.nz>
>> ---
>>   tests/spec/arb_texture_view/rendering_levels.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/tests/spec/arb_texture_view/rendering_levels.c
>> b/tests/spec/arb_texture_view/rendering_levels.c
>> index 7f02613..ef29f78 100644
>> --- a/tests/spec/arb_texture_view/rendering_levels.c
>> +++ b/tests/spec/arb_texture_view/rendering_levels.c
>> @@ -94,7 +94,8 @@ test_render_levels(void)
>>                 glTextureView(new_tex, GL_TEXTURE_2D, tex,  GL_RGBA8, l,
>>                               numLevels[l], 0, 1);
>>                 glBindTexture(GL_TEXTURE_2D, new_tex);
>> -               glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAX_LEVEL,
>> levels-1);
>> +               glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_BASE_LEVEL,
>> numLevels[l] - 1);
>> +               glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAX_LEVEL,
>> numLevels[l] - 1);
>>                 glClear(GL_COLOR_BUFFER_BIT);
>>
>
>
> _______________________________________________
> Piglit mailing list
> Piglit@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit
Okay I understand what you were doing. Perhaps if want to simplify and 
focus the test then just remove the setting of BASE_LEVEL  and MAX_LEVEL 
altogether.

I just threw that setting of MAX_LEVEL in since it would potentially 
interact with
the texture_view driver code.

Jon
On 01/13/2014 06:28 PM, Chris Forbes wrote:
> You're right, this one is a bit odd, and the commit message didn't
> really explain what I was getting at...
>
> Most of what I was trying to do here is eliminate the hardware lod
> selection as a complicating factor -- since we're really testing that
> levels are view-relative. It's possible that I've misunderstood what
> you were trying to do though and destroyed the essence of it. What do
> you think?
>
> On Tue, Jan 14, 2014 at 1:58 PM, Jon Ashburn <jon@lunarg.com> wrote:
>> Hey Chris:
>>
>> I think the texture level used should be clamped when using immutable
>> texture so if
>> the MAX_LEVEL is out of range it shouldn't matter.  Also an unset BASE_LEVEL
>> should default to 0
>> which is relative to the view.
>>
>>  From arb_texture_storage:
>> "
>>
>> Redefine level_base, level_max to be clamped forms of
>>          TEXTURE_BASE_LEVEL/TEXTURE_MAX_LEVEL when using immutable
>>          textures
>> "
>>
>> Which appears to be implemented here in Mesa code:
>>      /** From ARB_texture_storage:
>>         * However, if TEXTURE_IMMUTABLE_FORMAT is TRUE, then level_base is
>>         * clamped to the range [0, <levels> - 1] and level_max is then
>> clamped to
>>         * the range [level_base, <levels> - 1], where <levels> is the
>> parameter
>>         * passed the call to TexStorage* for the texture object.
>>         */
>>        if (texObj->Immutable)
>>            texObj->MaxLevel = CLAMP(params[0], texObj->BaseLevel,
>>                                     texObj->ImmutableLevels - 1);
>>        else
>>           texObj->MaxLevel = params[0];
>>
>>
>> Jon
>>
>> On 01/11/2014 05:39 PM, Chris Forbes wrote:
>>> The test expects us to be sampling from the smallest miplevel accessible
>>> in the view, but the TEXTURE_MAX_LEVEL constraint was out of range,
>>> and TEXTURE_BASE_LEVEL wasn't set at all.
>>>
>>> This would have been masked previously because the test fell over due to
>>> use of legacy features in the core profile.
>>>
>>> Signed-off-by: Chris Forbes <chrisf@ijw.co.nz>
>>> ---
>>>    tests/spec/arb_texture_view/rendering_levels.c | 3 ++-
>>>    1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/tests/spec/arb_texture_view/rendering_levels.c
>>> b/tests/spec/arb_texture_view/rendering_levels.c
>>> index 7f02613..ef29f78 100644
>>> --- a/tests/spec/arb_texture_view/rendering_levels.c
>>> +++ b/tests/spec/arb_texture_view/rendering_levels.c
>>> @@ -94,7 +94,8 @@ test_render_levels(void)
>>>                  glTextureView(new_tex, GL_TEXTURE_2D, tex,  GL_RGBA8, l,
>>>                                numLevels[l], 0, 1);
>>>                  glBindTexture(GL_TEXTURE_2D, new_tex);
>>> -               glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAX_LEVEL,
>>> levels-1);
>>> +               glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_BASE_LEVEL,
>>> numLevels[l] - 1);
>>> +               glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAX_LEVEL,
>>> numLevels[l] - 1);
>>>                  glClear(GL_COLOR_BUFFER_BIT);
>>>
>>
>> _______________________________________________
>> Piglit mailing list
>> Piglit@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/piglit