[Piglit] [PATCH 31/35] arb_gpu_shader5: Add support for testing spirv with XFB streams

Timothy Arceri tarceri at itsqueeze.com
Mon Aug 20 02:11:38 UTC 2018


On 18/08/18 19:30, Alejandro PiƱeiro wrote:
> On 18/08/18 06:36, Timothy Arceri wrote:
>> On 18/08/18 14:32, Timothy Arceri wrote:
>>> Won't this cause shader runner to needlessly parse the .shader_test
>>> file?
> 
> True, good point. In any case, as the test lacks a [test] section, full
> shader.py runs would just check if the test links (assuming that
> ARB_gl_spirv are available). Having said so ...
> 
>>
>> The file extension is also confusing. Maybe we should name these type
>> of files .shader_source or something similar rather than .shader_test ???
> 
> ... this makes sense. That name fits better, and would avoid the
> previous issue. As that would be a small change, I will make the change
> locally, while I wait for the review of the other patches.

I've skimmed over most of the series and I didn't see anything other 
than this that I had issue with. It's not very common to get a full 
review for a piglit series. The general rule is once its been on the 
list for a few weeks it's usually ok to push. After all it's better to 
have wrong tests that can be fixed later than to have no tests at all.

I understand the external dependency might have stopped you from just 
pushing but since its an optional dependency it's fine IMO.

Anyway for the series feel free to add:

Acked-by: Timothy Arceri <tarceri at itsqueeze.com>


More information about the Piglit mailing list