[Piglit] Fwd: [PATCH] Test implicit type conversion of out parameters.

[Paul Berry]

Whoops, forgot to copy the list on this email.


---------- Forwarded message ----------
From: Paul Berry <stereotype441 at gmail.com>
Date: 3 August 2011 07:48
Subject: Re: [Piglit] [PATCH] Test implicit type conversion of out parameters.
To: Ian Romanick <idr at freedesktop.org>
Cc: Chad Versace <chad at chad-versace.us>


On 2 August 2011 18:42, Ian Romanick <idr at freedesktop.org> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 08/02/2011 05:34 PM, Paul Berry wrote:
>> This patch adds four tests of the proper behavior of function out
>> parameters when used in a context that requires implicit type
>> conversion.
>
> Do all of these pass on other implementations?

Hmm, on my nVidia system they all fail with error messages like "error
C1113: actual parameter #1 must be same type as formal out parameter
("x")".

The nVidia behavior seems nonconformant by my reading of the standard.
 GLSL 1.30 says (in section 6.1: function definitions):

"If no exact match is found, then the implicit conversions in Section
4.1.10 “Implicit Conversions” will be applied to find a match.
Mismatched types on input parameters (in or inout or default) must
have a conversion from the calling argument type to the formal
parameter type. Mismatched types on output parameters (out or inout)
must have a conversion from the formal parameter type to the calling
argument type."

This message is from out-03.vert, so the formal parameter type is
"int" and the calling argument type is "float".  Implicit conversion
from int to float should be allowed.

GLSL 1.20 is less specific:

"Otherwise, if no exact match is found, then the implicit conversions
in Section 4.1.10 “Implicit Conversions” will be applied to the
calling arguments if this can make their types match a signature."

It seems to me that the difference between GLSL 1.20 and GLSL 1.30 is
due to clarification, not to a deliberate attempt to specify different
behavior.  So I wrote the tests using a "#version" directove of 120.

Incidentally, the tests still fail on nVidia if I change the
"#version" directive to 130.

Mesa passes these tests (if you apply the patch series I submitted
yesterday), but that's not saying much, since both the tests and the
code that passes them come from the same potentially biased brain
(mine).

Chad, do you have a comment on this?  My patches are standing on the
shoulders of some of yours (e.g.
8b3627fd7b52723102f070957d87f98073e92d7c "glsl: Fix implicit
conversions in non-constructor function calls").

I'd like to know what happens on other platforms.  I have been meaning
to try to get piglit running on my MacOS system at home.  This seems
like a good excuse to do that.  I'll let you know what I discover.

>
>> ---
>>  .../spec/glsl-1.20/compiler/qualifiers/out-03.vert |   17 ++++++++++
>>  .../execution/qualifiers/vs-out-01.shader_test     |   27 ++++++++++++++++
>>  .../execution/qualifiers/vs-out-02.shader_test     |   28 +++++++++++++++++
>>  .../execution/qualifiers/vs-out-03.shader_test     |   32 ++++++++++++++++++++
>
> I think it would be better for these to have long, descriptive names.
> How about:
>
>    vs-out-int-to-float
>    vs-out-int-to-float-return
>    vs-out-int-int-float-to-float-float-float-return
>
> We don't have any out-parameter tests, and I feel like we should have a
> ton.  Giving them descriptive names helps ensure that similar tests get
> grouped together.  With numbers, we might add vs-out-57 that should be
> grouped next to vs-out-05.
>
> Even though I've created a lot of them, I find the foo-01 tests to be
> really unsatisfactory in piglit logs, especially when one of them fails.
>  With the non-constant array tests (which I still need to push), it was
> really helpful, for example, to see that all of names with some certain
> pattern (e.g., vs-*-col-wr) failed.  It helped focus debugging efforts.

That's a good point.  I'll rename the tests.