dsa: avoid stencil8 in gettextureimage-formats

Submitted by Ilia Mirkin on May 23, 2015, 12:37 a.m.

Details

Message ID 1432341430-28890-1-git-send-email-imirkin@alum.mit.edu
State New
Headers show

Not browsing as part of any series.

Commit Message

Ilia Mirkin May 23, 2015, 12:37 a.m.
This causes an assert failure in mesa and it doesn't seem like this test
is designed for depth/stencil formats.

Signed-off-by: Ilia Mirkin <imirkin@alum.mit.edu>
---

Ian, I know you're rewriting half these tests... feel free to fold this
into your series if you like.

 tests/spec/arb_direct_state_access/gettextureimage-formats.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Patch hide | download patch | download mbox

diff --git a/tests/spec/arb_direct_state_access/gettextureimage-formats.c b/tests/spec/arb_direct_state_access/gettextureimage-formats.c
index e2d637c..ce25205 100644
--- a/tests/spec/arb_direct_state_access/gettextureimage-formats.c
+++ b/tests/spec/arb_direct_state_access/gettextureimage-formats.c
@@ -462,7 +462,8 @@  supported_format_set(const struct test_desc *set)
 		 set->format == ext_packed_depth_stencil ||
 		 set->format == arb_texture_rg_int ||
 		 set->format == arb_depth_texture ||
-		 set->format == arb_depth_buffer_float) {
+		 set->format == arb_depth_buffer_float ||
+		 set->format == arb_texture_stencil8) {
 		/*
 		 * texture_integer requires a fragment shader, different
 		 * glTexImage calls.  Depth/stencil formats not implemented.

Comments

[+idr]

Apparently I can't type :( Although git send-email could be a *bit*
more forgiving. Or at least complaining!

On Fri, May 22, 2015 at 8:37 PM, Ilia Mirkin <imirkin@alum.mit.edu> wrote:
> This causes an assert failure in mesa and it doesn't seem like this test
> is designed for depth/stencil formats.
>
> Signed-off-by: Ilia Mirkin <imirkin@alum.mit.edu>
> ---
>
> Ian, I know you're rewriting half these tests... feel free to fold this
> into your series if you like.
>
>  tests/spec/arb_direct_state_access/gettextureimage-formats.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/tests/spec/arb_direct_state_access/gettextureimage-formats.c b/tests/spec/arb_direct_state_access/gettextureimage-formats.c
> index e2d637c..ce25205 100644
> --- a/tests/spec/arb_direct_state_access/gettextureimage-formats.c
> +++ b/tests/spec/arb_direct_state_access/gettextureimage-formats.c
> @@ -462,7 +462,8 @@ supported_format_set(const struct test_desc *set)
>                  set->format == ext_packed_depth_stencil ||
>                  set->format == arb_texture_rg_int ||
>                  set->format == arb_depth_texture ||
> -                set->format == arb_depth_buffer_float) {
> +                set->format == arb_depth_buffer_float ||
> +                set->format == arb_texture_stencil8) {
>                 /*
>                  * texture_integer requires a fragment shader, different
>                  * glTexImage calls.  Depth/stencil formats not implemented.
> --
> 2.3.6
>
On 05/22/2015 06:37 PM, Ilia Mirkin wrote:
> This causes an assert failure in mesa and it doesn't seem like this test
> is designed for depth/stencil formats.

If we've got a piglit test that triggers a Mesa assertion, it means 
there's probably a Mesa bug too, right?

-Brian


>
> Signed-off-by: Ilia Mirkin <imirkin@alum.mit.edu>
> ---
>
> Ian, I know you're rewriting half these tests... feel free to fold this
> into your series if you like.
>
>   tests/spec/arb_direct_state_access/gettextureimage-formats.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/tests/spec/arb_direct_state_access/gettextureimage-formats.c b/tests/spec/arb_direct_state_access/gettextureimage-formats.c
> index e2d637c..ce25205 100644
> --- a/tests/spec/arb_direct_state_access/gettextureimage-formats.c
> +++ b/tests/spec/arb_direct_state_access/gettextureimage-formats.c
> @@ -462,7 +462,8 @@ supported_format_set(const struct test_desc *set)
>   		 set->format == ext_packed_depth_stencil ||
>   		 set->format == arb_texture_rg_int ||
>   		 set->format == arb_depth_texture ||
> -		 set->format == arb_depth_buffer_float) {
> +		 set->format == arb_depth_buffer_float ||
> +		 set->format == arb_texture_stencil8) {
>   		/*
>   		 * texture_integer requires a fragment shader, different
>   		 * glTexImage calls.  Depth/stencil formats not implemented.
>
On Mon, May 25, 2015 at 10:20 AM, Brian Paul <brianp@vmware.com> wrote:
> On 05/22/2015 06:37 PM, Ilia Mirkin wrote:
>>
>> This causes an assert failure in mesa and it doesn't seem like this test
>> is designed for depth/stencil formats.
>
>
> If we've got a piglit test that triggers a Mesa assertion, it means there's
> probably a Mesa bug too, right?

Yep, seems likely. Although the assertion could be overly aggressive,
not sure. Either way, I won't have time to investigate it.
On Fri, May 22, 2015 at 8:37 PM, Ilia Mirkin <imirkin@alum.mit.edu> wrote:
> This causes an assert failure in mesa and it doesn't seem like this test
> is designed for depth/stencil formats.
>
> Signed-off-by: Ilia Mirkin <imirkin@alum.mit.edu>
> ---
>
> Ian, I know you're rewriting half these tests... feel free to fold this
> into your series if you like.
>
>  tests/spec/arb_direct_state_access/gettextureimage-formats.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/tests/spec/arb_direct_state_access/gettextureimage-formats.c b/tests/spec/arb_direct_state_access/gettextureimage-formats.c
> index e2d637c..ce25205 100644
> --- a/tests/spec/arb_direct_state_access/gettextureimage-formats.c
> +++ b/tests/spec/arb_direct_state_access/gettextureimage-formats.c
> @@ -462,7 +462,8 @@ supported_format_set(const struct test_desc *set)
>                  set->format == ext_packed_depth_stencil ||
>                  set->format == arb_texture_rg_int ||
>                  set->format == arb_depth_texture ||
> -                set->format == arb_depth_buffer_float) {
> +                set->format == arb_depth_buffer_float ||
> +                set->format == arb_texture_stencil8) {
>                 /*
>                  * texture_integer requires a fragment shader, different
>                  * glTexImage calls.  Depth/stencil formats not implemented.
> --
> 2.3.6
>

I recently got a conflict while rebasing this which reminded me that I
still hadn't pushed this... any objections?
Ilia Mirkin <imirkin@alum.mit.edu> writes:

> On Fri, May 22, 2015 at 8:37 PM, Ilia Mirkin <imirkin@alum.mit.edu> wrote:
>> This causes an assert failure in mesa and it doesn't seem like this test
>> is designed for depth/stencil formats.
>>
>> Signed-off-by: Ilia Mirkin <imirkin@alum.mit.edu>
>> ---
>>
>> Ian, I know you're rewriting half these tests... feel free to fold this
>> into your series if you like.
>>
>>  tests/spec/arb_direct_state_access/gettextureimage-formats.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/tests/spec/arb_direct_state_access/gettextureimage-formats.c b/tests/spec/arb_direct_state_access/gettextureimage-formats.c
>> index e2d637c..ce25205 100644
>> --- a/tests/spec/arb_direct_state_access/gettextureimage-formats.c
>> +++ b/tests/spec/arb_direct_state_access/gettextureimage-formats.c
>> @@ -462,7 +462,8 @@ supported_format_set(const struct test_desc *set)
>>                  set->format == ext_packed_depth_stencil ||
>>                  set->format == arb_texture_rg_int ||
>>                  set->format == arb_depth_texture ||
>> -                set->format == arb_depth_buffer_float) {
>> +                set->format == arb_depth_buffer_float ||
>> +                set->format == arb_texture_stencil8) {
>>                 /*
>>                  * texture_integer requires a fragment shader, different
>>                  * glTexImage calls.  Depth/stencil formats not implemented.
>> --
>> 2.3.6
>>
>
> I recently got a conflict while rebasing this which reminded me that I
> still hadn't pushed this... any objections?

If we've got a test triggering a Mesa assertion, and this is the only
test doing so, making the test just not trigger the assertion seems bad.
On Tue, Sep 8, 2015 at 4:55 PM, Eric Anholt <eric@anholt.net> wrote:
> Ilia Mirkin <imirkin@alum.mit.edu> writes:
>
>> On Fri, May 22, 2015 at 8:37 PM, Ilia Mirkin <imirkin@alum.mit.edu> wrote:
>>> This causes an assert failure in mesa and it doesn't seem like this test
>>> is designed for depth/stencil formats.
>>>
>>> Signed-off-by: Ilia Mirkin <imirkin@alum.mit.edu>
>>> ---
>>>
>>> Ian, I know you're rewriting half these tests... feel free to fold this
>>> into your series if you like.
>>>
>>>  tests/spec/arb_direct_state_access/gettextureimage-formats.c | 3 ++-
>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/tests/spec/arb_direct_state_access/gettextureimage-formats.c b/tests/spec/arb_direct_state_access/gettextureimage-formats.c
>>> index e2d637c..ce25205 100644
>>> --- a/tests/spec/arb_direct_state_access/gettextureimage-formats.c
>>> +++ b/tests/spec/arb_direct_state_access/gettextureimage-formats.c
>>> @@ -462,7 +462,8 @@ supported_format_set(const struct test_desc *set)
>>>                  set->format == ext_packed_depth_stencil ||
>>>                  set->format == arb_texture_rg_int ||
>>>                  set->format == arb_depth_texture ||
>>> -                set->format == arb_depth_buffer_float) {
>>> +                set->format == arb_depth_buffer_float ||
>>> +                set->format == arb_texture_stencil8) {
>>>                 /*
>>>                  * texture_integer requires a fragment shader, different
>>>                  * glTexImage calls.  Depth/stencil formats not implemented.
>>> --
>>> 2.3.6
>>>
>>
>> I recently got a conflict while rebasing this which reminded me that I
>> still hadn't pushed this... any objections?
>
> If we've got a test triggering a Mesa assertion, and this is the only
> test doing so, making the test just not trigger the assertion seems bad.

But the test isn't supposed to test stencil formats in the first place
(see comment in the context).

  -ilia
Ilia Mirkin <imirkin@alum.mit.edu> writes:

> On Tue, Sep 8, 2015 at 4:55 PM, Eric Anholt <eric@anholt.net> wrote:
>> Ilia Mirkin <imirkin@alum.mit.edu> writes:
>>
>>> On Fri, May 22, 2015 at 8:37 PM, Ilia Mirkin <imirkin@alum.mit.edu> wrote:
>>>> This causes an assert failure in mesa and it doesn't seem like this test
>>>> is designed for depth/stencil formats.
>>>>
>>>> Signed-off-by: Ilia Mirkin <imirkin@alum.mit.edu>
>>>> ---
>>>>
>>>> Ian, I know you're rewriting half these tests... feel free to fold this
>>>> into your series if you like.
>>>>
>>>>  tests/spec/arb_direct_state_access/gettextureimage-formats.c | 3 ++-
>>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/tests/spec/arb_direct_state_access/gettextureimage-formats.c b/tests/spec/arb_direct_state_access/gettextureimage-formats.c
>>>> index e2d637c..ce25205 100644
>>>> --- a/tests/spec/arb_direct_state_access/gettextureimage-formats.c
>>>> +++ b/tests/spec/arb_direct_state_access/gettextureimage-formats.c
>>>> @@ -462,7 +462,8 @@ supported_format_set(const struct test_desc *set)
>>>>                  set->format == ext_packed_depth_stencil ||
>>>>                  set->format == arb_texture_rg_int ||
>>>>                  set->format == arb_depth_texture ||
>>>> -                set->format == arb_depth_buffer_float) {
>>>> +                set->format == arb_depth_buffer_float ||
>>>> +                set->format == arb_texture_stencil8) {
>>>>                 /*
>>>>                  * texture_integer requires a fragment shader, different
>>>>                  * glTexImage calls.  Depth/stencil formats not implemented.
>>>> --
>>>> 2.3.6
>>>>
>>>
>>> I recently got a conflict while rebasing this which reminded me that I
>>> still hadn't pushed this... any objections?
>>
>> If we've got a test triggering a Mesa assertion, and this is the only
>> test doing so, making the test just not trigger the assertion seems bad.
>
> But the test isn't supposed to test stencil formats in the first place
> (see comment in the context).

Sure, but generally we don't avoid Mesa assertions in a piglit test,
even if the test shouldn't be hitting the path, until we make a separate
test that *does* hit that path.
On Wed, Sep 9, 2015 at 2:22 PM, Eric Anholt <eric@anholt.net> wrote:
> Ilia Mirkin <imirkin@alum.mit.edu> writes:
>
>> On Tue, Sep 8, 2015 at 4:55 PM, Eric Anholt <eric@anholt.net> wrote:
>>> Ilia Mirkin <imirkin@alum.mit.edu> writes:
>>>
>>>> On Fri, May 22, 2015 at 8:37 PM, Ilia Mirkin <imirkin@alum.mit.edu> wrote:
>>>>> This causes an assert failure in mesa and it doesn't seem like this test
>>>>> is designed for depth/stencil formats.
>>>>>
>>>>> Signed-off-by: Ilia Mirkin <imirkin@alum.mit.edu>
>>>>> ---
>>>>>
>>>>> Ian, I know you're rewriting half these tests... feel free to fold this
>>>>> into your series if you like.
>>>>>
>>>>>  tests/spec/arb_direct_state_access/gettextureimage-formats.c | 3 ++-
>>>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/tests/spec/arb_direct_state_access/gettextureimage-formats.c b/tests/spec/arb_direct_state_access/gettextureimage-formats.c
>>>>> index e2d637c..ce25205 100644
>>>>> --- a/tests/spec/arb_direct_state_access/gettextureimage-formats.c
>>>>> +++ b/tests/spec/arb_direct_state_access/gettextureimage-formats.c
>>>>> @@ -462,7 +462,8 @@ supported_format_set(const struct test_desc *set)
>>>>>                  set->format == ext_packed_depth_stencil ||
>>>>>                  set->format == arb_texture_rg_int ||
>>>>>                  set->format == arb_depth_texture ||
>>>>> -                set->format == arb_depth_buffer_float) {
>>>>> +                set->format == arb_depth_buffer_float ||
>>>>> +                set->format == arb_texture_stencil8) {
>>>>>                 /*
>>>>>                  * texture_integer requires a fragment shader, different
>>>>>                  * glTexImage calls.  Depth/stencil formats not implemented.
>>>>> --
>>>>> 2.3.6
>>>>>
>>>>
>>>> I recently got a conflict while rebasing this which reminded me that I
>>>> still hadn't pushed this... any objections?
>>>
>>> If we've got a test triggering a Mesa assertion, and this is the only
>>> test doing so, making the test just not trigger the assertion seems bad.
>>
>> But the test isn't supposed to test stencil formats in the first place
>> (see comment in the context).
>
> Sure, but generally we don't avoid Mesa assertions in a piglit test,
> even if the test shouldn't be hitting the path, until we make a separate
> test that *does* hit that path.

OK. TBH I don't even know if there's still a crash... I sent this
patch out 4 months ago. Perhaps in another 4 months someone will
provide such a test and this patch can be pushed. In the meanwhile the
test will fail for drivers that implement ARB_texture_stencil8.

  -ilia