[Piglit] [PATCH 31/35] arb_gpu_shader5: Add support for testing spirv with XFB streams
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
> 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