[Piglit] [PATCH 0/23] Port glsl1 Glean test to Piglit

[Brian Paul]

On 11/29/2017 02:26 PM, Fabian Bieler wrote:
> On 2017-11-29 04:53, Brian Paul wrote:
>> On 11/23/2017 01:44 PM, Fabian Bieler wrote:
>>> This series replaces the "glsl1" Glean test with some Piglit tests.
>>>
>>> Most tests were replaced by shader_runner tests.
>>>
>>> The tests for built-in-uniform state were extended.
>>>
>>> Some of the preprocessor tests were modified to make them stricter.
>>>
>>> To port some texture tests shader_runner had to be extended with
>>> "texture rgbw 1D" and "texture rgbw 3D" commands.
>>> These (and the underlying piglit-util-functions) are very basic and
>>> don't allow for variable texture size or pixel format. If desired, I can
>>> remedy that.
>>>
>>> Some tests ("Global vars and initializers", "Global vars and
>>> initializers (2)", "Swizzle", "Writemask") are pretty trivial. I doubt
>>> they would break without some existing, more complex Piglit test
>>> failing, too. However, I opted to port them regardless since I couldn't
>>> find an existing simple Piglit test for the feature in question.
>>>
>>> Attached is a list of all Glean GLSL subtests with the location of the
>>> new or existing Piglit test that replaces it.
>>>
>>> Patch 23 fixes an unrelated test.
>>
>
> Thanks for the review.
>
>> I did a quick read-through and the series looks OK to me.  I presume
>> there's no compiler warnings and all the tests pass/fail as the glean
>> tests did.
>
> I found no compiler warnings with gcc 4, 5, 7 and clang 3.8.
>
> "glsl-1.10-built-in-uniform-state" currently fails on Mesa because the
> values of gl_SpotExponent and gl_SpotCosCutoff are swapped (Glean didn't
> test those uniforms)
>
> "glsl-1.10-built-in-matrix-state" currently fails on Mesa because
> gl_NormalScale is the reciprocal of the desired value (at least for
> drivers that use mesa's modelspace-lighting optimization (Glean didn't
> test gl_NormalScale).
>
> I have posted patches for both issues to mesa-dev.
> https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.freedesktop.org_archives_mesa-2Ddev_2017-2DNovember_177977.html&d=DwIDaQ&c=uilaK90D4TOVoH58JNXRgQ&r=Ie7_encNUsqxbSRbqbNgofw0ITcfE8JKfaUjIQhncGA&m=8oe2QGJRLsBm_JiD7iUuZyin6B6XPbEJkG1lo7JKvu4&s=hr5OoX1SLzO_8DB8fD-yTvJsEfP43HwybACc2i-HHDE&e=
> The first of those is pretty straightforward, though my commit message
> could have been clearer. The second patch could use a Piglit test for
> GL_RESCALE_NORMAL with the fixed-function pipeline to make sure it
> introduces no regressions...

I've reviewed both of those patches now.

>
> Otherwise all tests pass.
>>
>> If there's no other comments in a few days, I can push the series.
> If it hasn't bee revoked due to inactivity I still should have commit
> access to Piglit. I'll try pushing the series at the end of the week and
> report back if that fails.

OK, sounds good.  I didn't know if you had git permission.

-Brian

>>
>> Reviewed-by: Brian Paul <brianp at vmware.com>
>>