Dave Airlie <airl...@gmail.com> writes:
> On 1 March 2018 at 04:08, Francisco Jerez <curroje...@riseup.net> wrote:
>> Dave Airlie <airl...@gmail.com> writes:
>>
>>> From: Dave Airlie <airl...@redhat.com>
>>>
>>> This drops one from the max images as the fragment shader needs
>>> one output for outputing the results
>>>
>>> Signed-off-by: Dave Airlie <airl...@redhat.com>
>>> ---
>>> tests/spec/arb_shader_image_load_store/image.c | 5 ++++-
>>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/tests/spec/arb_shader_image_load_store/image.c
>>> b/tests/spec/arb_shader_image_load_store/image.c
>>> index e664d3cc4..1bfaebfdb 100644
>>> --- a/tests/spec/arb_shader_image_load_store/image.c
>>> +++ b/tests/spec/arb_shader_image_load_store/image.c
>>> @@ -670,11 +670,14 @@ num_reserved_images(GLbitfield stages)
>>> unsigned
>>> image_stage_max_images(const struct image_stage_info *stage)
>>> {
>>> - int n = 0;
>>> + int n = 0, n2 = 0;
>>>
>>> switch (stage->stage) {
>>> case GL_FRAGMENT_SHADER:
>>> glGetIntegerv(GL_MAX_FRAGMENT_IMAGE_UNIFORMS, &n);
>>> + glGetIntegerv(GL_MAX_COMBINED_SHADER_OUTPUT_RESOURCES,
>>> &n2);
>>> + if (n == n2)
>>> + n--;
>>
>> I don't think this is guaranteed to fix the problem where there is one.
>> GL_MAX_COMBINED_SHADER_OUTPUT_RESOURCES imposes a limit on the number of
>> active image units in the whole pipeline, and you can reach it whether
>> GL_MAX_FRAGMENT_IMAGE_UNIFORMS is equal, lower or greater than
>> GL_MAX_COMBINED_SHADER_OUTPUT_RESOURCES. You probably need to fix the
>> test case instead to require a lower number of image units.
>
> In practice the number of drivers that set
> GL_MAX_COMBINED_SHADER_OUTPUT_RESOURCES
> to a lower value is evergreen gpus, and they don't support image units
> anywhere else in the pipeline.
>
> I'm not sure we'll see any other gpu where it's very different.
>This hack might work for you, but assuming that there is a bug in some of the image load/store tests (I don't think you mentioned what this even fixes) that causes it to use more than GL_MAX_COMBINED_SHADER_OUTPUT_RESOURCES, this cannot possibly be addressing the root of the problem, it's only papering over the symptoms for evergreen, and the test could possibly blow up again for the next driver that enables image load/store. When that happens the next developer that comes fix this is unlikely to be aware of your hack, so it will likely be "fixed" twice and the test will no longer behave as expected on some systems. Same if somebody comes by and writes a new image load/store test not expecting image_stage_max_images() to subtract an arbitrary number from the number of supported uniforms... but only for the fragment shader... and only in an uncertain subset of systems including evergreen. Take that as a NAK. Fixing it properly is likely to involve as much typing. > Dave.
signature.asc
Description: PGP signature
_______________________________________________ Piglit mailing list Piglit@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/piglit
