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

[Nicolai Hähnle]

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.