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

[Dorian Apanel]

Thank you very much Nicolai for deep explanation.
You're totally right.

2017-04-11 9:27 GMT+02:00 Nicolai Hähnle <nhaehnle at gmail.com>:

> On 10.04.2017 00:26, Dorian Apanel wrote:
>
>> Hi Nicolai,
>>
>> Thanks for checking it.
>> Yes I can add some negative tests for it - as another patch, maybe in
>> this week.
>>
>
> Thanks.
>
>
> 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
>>
>
> Section 8.17 (Texture Completeness) says:
>
> "Image levels k where k < level_base or k > q are insignificant to the
> definition of completeness."
>
> ... and Section 8.14.3 (Mipmapping) defines q as:
>
> p = floor(log2 (maxsize)) + level_base
> q = min{p, level_max}
>
> Explicitly setting TEXTURE_MAX_LEVEL has *never* been a necessity in
> OpenGL. You only need it if, as in this test, you don't define the smallest
> levels in the mip tree.
>
> Cheers,
> Nicolai
>
>
>
>> Thanks,
>> Dorian
>>
>> 2017-04-05 15:58 GMT+02:00 Nicolai Hähnle <nhaehnle at gmail.com
>> <mailto:nhaehnle at gmail.com>>:
>>
>>     On 05.04.2017 09:55, Dorian Apanel wrote:
>>
>>         From: Dorian Apanel <dorian.apanel+piglit at gmail.com
>>         <mailto:dorian.apanel%2Bpiglit 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
>>         <mailto: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_TEXTURE_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.
>>
>>
>>
>
> --
> 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/20170411/d21c5025/attachment-0001.html>