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

[Francisco Jerez]

Danylo Piliaiev <danylo.piliaiev at gmail.com> writes:

> On 18.07.18 00:01, 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.
>>>
>>> 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     | 54 +++++++++++++++++--
>>>   1 file changed, 49 insertions(+), 5 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..adab56ffc 100644
>>> --- a/tests/spec/arb_shader_image_load_store/invalid.c
>>> +++ b/tests/spec/arb_shader_image_load_store/invalid.c
>>> @@ -172,12 +172,45 @@ init_level(const struct image_info img, unsigned level,
>>>              GLenum format, unsigned w, unsigned h)
>>>   {
>>>           uint32_t pixels[4 * N];
>>> -
>>>           init_pixels(img, pixels, 1, 1, 1, 1);
>>> -        glBindTexture(GL_TEXTURE_2D, get_texture(0));
>>> -        glTexImage2D(GL_TEXTURE_2D, level, format,
>>> -                     w, h, 0, img.format->pixel_format,
>>> -                     image_base_type(img.format), pixels);
>>> +
>>> +        glBindTexture(img.target->target, get_texture(0));
>>> +
>>> +        switch (img.target->target) {
>>> +        case GL_TEXTURE_2D: {
>>> +                glTexImage2D(GL_TEXTURE_2D, level, format,
>>> +                        w, h, 0, img.format->pixel_format,
>>> +                        image_base_type(img.format), pixels);
>>> +                break;
>>> +        }
>>> +        case GL_TEXTURE_BUFFER: {
>>> +                const struct image_extent grid = image_optimal_extent(img.size);
>>> +                GLuint packed_tex;
>>> +
>>> +                assert(level == 0);
>>> +
>>> +                glBindBuffer(GL_PIXEL_PACK_BUFFER, get_buffer(0));
>>> +                glBufferData(GL_PIXEL_PACK_BUFFER,
>>> +                             img.size.x * image_pixel_size(img.format) / 8,
>>> +                             NULL, GL_STATIC_DRAW);
>>> +
>>> +                glGenTextures(1, &packed_tex);
>>> +                glBindTexture(GL_TEXTURE_2D, packed_tex);
>>> +
>>> +                glTexImage2D(GL_TEXTURE_2D, 0, format,
>>> +                             grid.x, grid.y, 0, img.format->pixel_format,
>>> +                             image_base_type(img.format), pixels);
>>> +                glGetTexImage(GL_TEXTURE_2D, 0, img.format->pixel_format,
>>> +                              img.format->pixel_type, NULL);
>>> +                glDeleteTextures(1, &packed_tex);
>>> +                glBindBuffer(GL_PIXEL_PACK_BUFFER, 0);
>>> +
>>> +                glTexBuffer(GL_TEXTURE_BUFFER, format, get_buffer(0));
>>> +                break;
>> Can't you call upload_image() instead of reimplementing these hacks here
>> in order to pack the pixels into a PBO?  Texture buffers only have one
>> level anyway so it shouldn't hurt to recreate the whole texture.
>>
> I cannot use upload_image() from common.c as is since there is a subtle 
> difference:
> I need to pass custom internalFormat to glTexImage2D which is the point 
> of this test
> (same reason init_level existed before).
> But yes, it looks like code duplication, an alternative is to extend 
> upload_image_levels()
> which also doesn't look good for me.

I believe that for this purpose you could possibly reuse upload_image()
as-is, but change invalidate_incompatible_format() *not* to call
upload_level() and instead call glBindImageTexture() with the
incompatible format.

>>> +        }
>>> +        default:
>>> +                abort();
>>> +        }
>>>   
>>>           return piglit_check_gl_error(GL_NO_ERROR);
>>>   }
>>> @@ -346,6 +379,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 +434,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.
>>> +                 */
>>> +                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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 227 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/piglit/attachments/20180718/63539c23/attachment.sig>