[Mesa-dev] [PATCH 1/2] mesa: remove buggy assertions in unpack Z24

[Brian Paul]

On 11/20/2011 07:08 AM, Marek Olšák wrote:
> unpack_float_z_Z24_X8 fails with -O2 in:
> - fbo-blit-d24s8
> - fbo-depth-sample-compare
> - fbo-readpixels-depth-formats
> - glean/depthStencil
>
> With -O0, it works fine.
>
> I am removing all the assertions. There's not much point in having them,
> is there?
> ---
>   src/mesa/main/format_unpack.c |    8 --------
>   1 files changed, 0 insertions(+), 8 deletions(-)
>
> diff --git a/src/mesa/main/format_unpack.c b/src/mesa/main/format_unpack.c
> index 6e2ce7a..fbc37ea 100644
> --- a/src/mesa/main/format_unpack.c
> +++ b/src/mesa/main/format_unpack.c
> @@ -567,8 +567,6 @@ unpack_Z24_S8(const void *src, GLfloat dst[][4], GLuint n)
>         dst[i][1] =
>         dst[i][2] = (s[i]>>  8) * scale;
>         dst[i][3] = 1.0F;
> -      ASSERT(dst[i][0]>= 0.0F);
> -      ASSERT(dst[i][0]<= 1.0F);
>      }
>   }
>
> @@ -584,8 +582,6 @@ unpack_S8_Z24(const void *src, GLfloat dst[][4], GLuint n)
>         dst[i][1] =
>         dst[i][2] = (s[i]&  0x00ffffff) * scale;
>         dst[i][3] = 1.0F;
> -      ASSERT(dst[i][0]>= 0.0F);
> -      ASSERT(dst[i][0]<= 1.0F);
>      }
>   }
>
> @@ -1711,8 +1707,6 @@ unpack_float_z_Z24_X8(GLuint n, const void *src, GLfloat *dst)
>      GLuint i;
>      for (i = 0; i<  n; i++) {
>         dst[i] = (s[i]>>  8) * scale;
> -      ASSERT(dst[i]>= 0.0F);
> -      ASSERT(dst[i]<= 1.0F);
>      }
>   }
>
> @@ -1725,8 +1719,6 @@ unpack_float_z_X8_Z24(GLuint n, const void *src, GLfloat *dst)
>      GLuint i;
>      for (i = 0; i<  n; i++) {
>         dst[i] = (s[i]&  0x00ffffff) * scale;
> -      ASSERT(dst[i]>= 0.0F);
> -      ASSERT(dst[i]<= 1.0F);
>      }
>   }
>

I'd like to know more about why this is happening.  It's a little 
worrisome that -O0 vs. -O2 produce different results.

It could be bad if an app reads the depth buffer and gets values < 0 
or > 1.  Can you check if glReadPixels/glGetTexImage() would do the 
clamping?

-Brian