[Piglit] [PATCH] arb_shader_image_load_store: Create complete textures.

[Dorian Apanel]

Hi Nicolai,

Thanks for checking it.
Yes I can add some negative tests for it - as another patch, maybe in this
week.

As I understand the spec, log2(size) is not relevant here.
Example: You create texture, default filter uses mipmaps, and default max
level is 1000, base 0. You supply 5 mips (16,8,4,2,1). At this point, even
if log2(16)+1 == 5, texture is not complete. OGL expects 995 more levels of
size 1.

>From Spec: "required width for mipmap level i is max(1, floor(w_b / 2^i))"
See also this SO answer http://stackoverflow.com/a/3906855/1688267 and
"Creating a complete texture" paragraph here
https://www.khronos.org/opengl/wiki/Common_Mistakes

Thanks,
Dorian

2017-04-05 15:58 GMT+02:00 Nicolai Hähnle <nhaehnle at gmail.com>:

> On 05.04.2017 09:55, Dorian Apanel wrote:
>
>> From: Dorian Apanel <dorian.apanel+piglit at gmail.com>
>>
>> Textures created by image load/store tests are not complete (max level
>> defaults to 1000).
>> Load/Store on incomplete textures should return zeros/change nothing.
>> This fix sets proper base and max level of textures.
>>
>> Signed-off-by: Dorian Apanel <dorian.apanel at gmail.com>
>>
>
> Interesting. It seems you're right, but could you please also add a
> corresponding negative test and check it against whatever drivers you have?
> I.e., a test that creates a texture with mipmaps enabled, defines only the
> first level image, and then tries to use it from a shader to make sure that
> (a) reads return 0 and (b) writes are ignored?
>
> Also, the commit message doesn't quite capture the heart of it as far as I
> understand it. The textures are incomplete because num_levels levels are
> created, which may be less than log2(size). So GL_TEXTURE_MAX_LEVEL is used
> to restrict the required number of levels.
>
> Thanks,
> Nicolai
>
>
>
> ---
>>  tests/spec/arb_shader_image_load_store/common.c | 9 +++++++++
>>  1 file changed, 9 insertions(+)
>>
>> diff --git a/tests/spec/arb_shader_image_load_store/common.c
>> b/tests/spec/arb_shader_image_load_store/common.c
>> index cbeaac7..fdc2ef3 100644
>> --- a/tests/spec/arb_shader_image_load_store/common.c
>> +++ b/tests/spec/arb_shader_image_load_store/common.c
>> @@ -141,6 +141,11 @@ upload_image_levels(const struct image_info img,
>> unsigned num_levels,
>>          glGenTextures(1, &textures[unit]);
>>          glBindTexture(img.target->target, textures[unit]);
>>
>> +        if (img.target->target != GL_TEXTURE_BUFFER) {
>> +            glTexParameteri(img.target->target, GL_TEXTURE_BASE_LEVEL,
>> 0);
>> +            glTexParameteri(img.target->target, GL_TEXTURE_MAX_LEVEL,
>> num_levels - 1);
>> +        }
>> +
>>          switch (img.target->target) {
>>          case GL_TEXTURE_1D:
>>                  for (l = 0; l < num_levels; ++l) {
>> @@ -301,6 +306,8 @@ upload_image_levels(const struct image_info img,
>> unsigned num_levels,
>>
>>                  glGenTextures(1, &tmp_tex);
>>                  glBindTexture(GL_TEXTURE_2D, tmp_tex);
>> +                glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_BASE_LEVEL, 0);
>> +                glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAX_LEVEL, 0);
>>
>>                  if (img.target->target == GL_TEXTURE_2D_MULTISAMPLE_ARRAY)
>> {
>>                          glTexImage3DMultisample(GL_TE
>> XTURE_2D_MULTISAMPLE_ARRAY,
>> @@ -462,6 +469,8 @@ download_image_levels(const struct image_info img,
>> unsigned num_levels,
>>
>>                  glGenTextures(1, &tmp_tex);
>>                  glBindTexture(GL_TEXTURE_2D, tmp_tex);
>> +                glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_BASE_LEVEL, 0);
>> +                glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAX_LEVEL, 0);
>>
>>                  glTexImage2D(GL_TEXTURE_2D, 0, img.format->format,
>>                               grid.size.x, grid.size.y, 0,
>>
>>
>
> --
> Lerne, wie die Welt wirklich ist,
> Aber vergiss niemals, wie sie sein sollte.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/piglit/attachments/20170410/fe72972c/attachment.html>