[Piglit] [PATCH v2] arb_shader_image_load_store: Test format incompatible texture buffer

[Danylo Piliaiev]

On 20.07.18 02:26, Francisco Jerez wrote:
> Danylo Piliaiev <danylo.piliaiev at gmail.com> writes:
>
>> Test for the regression which happened when GL_TEXTURE_BUFFER was
>> allowed to have incompatible format.
>>
>> v2: Removed unnecessary code duplication - use upload_image instead
>>       of init_level. (Francisco Jerez)
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106465
>>
>> Signed-off-by: Danylo Piliaiev <danylo.piliaiev at globallogic.com>
>> ---
>>   .../arb_shader_image_load_store/invalid.c     | 23 +++++++++++++++----
>>   1 file changed, 19 insertions(+), 4 deletions(-)
>>
>> diff --git a/tests/spec/arb_shader_image_load_store/invalid.c b/tests/spec/arb_shader_image_load_store/invalid.c
>> index ed4b6c064..99f6703a4 100644
>> --- a/tests/spec/arb_shader_image_load_store/invalid.c
>> +++ b/tests/spec/arb_shader_image_load_store/invalid.c
>> @@ -266,13 +266,17 @@ static bool
>>   invalidate_incompatible_format(const struct image_info img, GLuint prog)
>>   {
>>           GLenum base_format = image_base_internal_format(img.format);
>> +        uint32_t pixels[4 * N];
>> +        init_pixels(img, pixels, 1, 1, 1, 1);
>> +
>> +        /* upload_image instead of init_level to support GL_TEXTURE_BUFFER */
>> +        bool ret = upload_image(img, 0, pixels);
>> +
> I don't think you actually need to call upload_image() here, it should
> have been called already by init_image().
I don't know where I'm looking... You are right. Only glBindImageTexture 
will be left
and it's enough for the test.
>>           /* Pick an incompatible texture format with a compatible base
>>            * type. */
>> -        bool ret = init_level(img, 0, (base_format == GL_RGBA32F ?
>> -                                       GL_RGBA8 : GL_RG32UI), W, H);
>> -
>>           glBindImageTexture(0, get_texture(0), 0, GL_TRUE, 0,
>> -                           GL_READ_WRITE, img.format->format);
>> +                           GL_READ_WRITE, (base_format == GL_RGBA32F ?
>> +                                           GL_RGBA8 : GL_RG32UI));
>>   
>>           return ret && piglit_check_gl_error(GL_NO_ERROR);
>>   }
>> @@ -346,6 +350,8 @@ piglit_init(int argc, char **argv)
>>           for (op = image_ops; op->name; ++op) {
>>                   const struct image_info def_img = image_info(
>>                           GL_TEXTURE_2D, op->formats[0].format, W, H);
>> +                const struct image_info def_img_buffer = image_info(
>> +                        GL_TEXTURE_BUFFER, op->formats[0].format, W, H);
>>   
>>                   /*
>>                    * According to the spec, an access is considered
>> @@ -399,6 +405,15 @@ piglit_init(int argc, char **argv)
>>                                    invalidate_incompatible_format, false),
>>                           "%s/incompatible format test", op->name);
>>   
>> +                /* Test for the regression which happened when
>> +                 * GL_TEXTURE_BUFFER was allowed to have incompatible format.
>> +                 */
> FTR, did you confirm whether this test-case causes a crash after
> re-applying the mesa patch that led to the regression?
>
> Thanks.
This test-case doesn't crash with that patch. I picked the first format 
from the array
(same as the other tests) which appears to be GL_RGBA32F, but 
unfortunately it's a
format with which we cannot go out of bounds. Picking the format which 
will result in crash
would be better (already tested and crash occurred). I'll do this in my 
hopefully final version of the patch.

Thank you for reviewing these embarrassing patches.
>> +                subtest(&status, true,
>> +                        run_test(op, def_img_buffer, def_img_buffer,
>> +                                 invalidate_incompatible_format, false),
>> +                        "%s/incompatible format test/image%s",
>> +                        op->name, def_img_buffer.target->name);
>> +
>>                   /*
>>                    * " * the texture bound to the image unit has layers,
>>                    *     and the selected layer or cube map face doesn't
>> -- 
>> 2.17.1