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://lists.freedesktop.org/archives/mesa-dev/2017-November/177977.html
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...

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.
> 
> Reviewed-by: Brian Paul <bri...@vmware.com>
> 
_______________________________________________
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

Reply via email to