Re: [Mesa-dev] [PATCH] mesa/main/readpix: Correct handling of packed floating point values

[Ian Romanick]

On 04/27/2018 09:04 AM, Gert Wollny wrote:
> From: Gert Wollny 
> 
> Make sure that clamping in the pixel transfer operations is enabled/disabled
> for packed floating point values just like it is done for single normal and
> half precision floating point values.
> 
> This fixes a series of CTS tests with virgl that use r11f_g11f_b10f
> buffers as target, and where virglrenderer reads these surfaces back 
> using the format GL_UNSIGNED_INT_10F_11F_11F_REV.
> 
> Signed-off-by: Gert Wollny 
> ---
> PS: I don't have write permissions to mesa git
> 
>  src/mesa/main/readpix.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/src/mesa/main/readpix.c b/src/mesa/main/readpix.c
> index a4eea3043d..c5fc66988b 100644
> --- a/src/mesa/main/readpix.c
> +++ b/src/mesa/main/readpix.c
> @@ -106,7 +106,8 @@ _mesa_get_readpixels_transfer_ops(const struct gl_context 
> *ctx,
>/* For blit-based ReadPixels packing, the clamping is done 
> automatically
> * unless the type is float. */
>if (_mesa_get_clamp_read_color(ctx, ctx->ReadBuffer) &&
> -  (type == GL_FLOAT || type == GL_HALF_FLOAT)) {
> +  (type == GL_FLOAT || type == GL_HALF_FLOAT ||
> +   type == GL_UNSIGNED_INT_10F_11F_11F_REV)) {

I was thinking that maybe we needed to handle another type or two here,
but I verified that those type (e.g., the 999e5 type) aren't possible.
I'm going to run this through Intel's CI, then push it.

Reviewed-by: Ian Romanick 

>   transferOps |= IMAGE_CLAMP_BIT;
>}
> }
> @@ -114,7 +115,8 @@ _mesa_get_readpixels_transfer_ops(const struct gl_context 
> *ctx,
>/* For CPU-based ReadPixels packing, the clamping must always be done
> * for non-float types, */
>if (_mesa_get_clamp_read_color(ctx, ctx->ReadBuffer) ||
> -  (type != GL_FLOAT && type != GL_HALF_FLOAT)) {
> +  (type != GL_FLOAT && type != GL_HALF_FLOAT &&
> +   type != GL_UNSIGNED_INT_10F_11F_11F_REV)) {
>   transferOps |= IMAGE_CLAMP_BIT;
>}
> }
> 

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] mesa/main/readpix: Correct handling of packed floating point values

[Gurchetan Singh]

For those wondering why the dEQP CTS tests this fixes (for example
dEQP-GLES3.functional.fragment_out.basic.float.r11f_g11f_b10f_lowp_float)
pass outside the VM, it seems it's because the test cases read pixels
with type = GL_FLOAT.  virglrenderer always sets type =
GL_UNSIGNED_INT_10F_11F_11F_REV on the host when reading back
GL_R11F_G11F_B10F and probably converts to GL_FLOAT on the guest.  I
guess nobody, even conformance suites, wanted to deal with
GL_UNSIGNED_INT_10F_11F_11F_REV ...

Reviewed-by: Gurchetan Singh 
(don't have commit access so hopefully someone merges this)

On Fri, Apr 27, 2018 at 9:04 AM, Gert Wollny  wrote:
> From: Gert Wollny 
>
> Make sure that clamping in the pixel transfer operations is enabled/disabled
> for packed floating point values just like it is done for single normal and
> half precision floating point values.
>
> This fixes a series of CTS tests with virgl that use r11f_g11f_b10f
> buffers as target, and where virglrenderer reads these surfaces back
> using the format GL_UNSIGNED_INT_10F_11F_11F_REV.
>
> Signed-off-by: Gert Wollny 
> ---
> PS: I don't have write permissions to mesa git
>
>  src/mesa/main/readpix.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/src/mesa/main/readpix.c b/src/mesa/main/readpix.c
> index a4eea3043d..c5fc66988b 100644
> --- a/src/mesa/main/readpix.c
> +++ b/src/mesa/main/readpix.c
> @@ -106,7 +106,8 @@ _mesa_get_readpixels_transfer_ops(const struct gl_context 
> *ctx,
>/* For blit-based ReadPixels packing, the clamping is done 
> automatically
> * unless the type is float. */
>if (_mesa_get_clamp_read_color(ctx, ctx->ReadBuffer) &&
> -  (type == GL_FLOAT || type == GL_HALF_FLOAT)) {
> +  (type == GL_FLOAT || type == GL_HALF_FLOAT ||
> +   type == GL_UNSIGNED_INT_10F_11F_11F_REV)) {
>   transferOps |= IMAGE_CLAMP_BIT;
>}
> }
> @@ -114,7 +115,8 @@ _mesa_get_readpixels_transfer_ops(const struct gl_context 
> *ctx,
>/* For CPU-based ReadPixels packing, the clamping must always be done
> * for non-float types, */
>if (_mesa_get_clamp_read_color(ctx, ctx->ReadBuffer) ||
> -  (type != GL_FLOAT && type != GL_HALF_FLOAT)) {
> +  (type != GL_FLOAT && type != GL_HALF_FLOAT &&
> +   type != GL_UNSIGNED_INT_10F_11F_11F_REV)) {
>   transferOps |= IMAGE_CLAMP_BIT;
>}
> }
> --
> 2.17.0
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] mesa/main/readpix: Correct handling of packed floating point values

[Gert Wollny]

From: Gert Wollny 

Make sure that clamping in the pixel transfer operations is enabled/disabled
for packed floating point values just like it is done for single normal and
half precision floating point values.

This fixes a series of CTS tests with virgl that use r11f_g11f_b10f
buffers as target, and where virglrenderer reads these surfaces back 
using the format GL_UNSIGNED_INT_10F_11F_11F_REV.

Signed-off-by: Gert Wollny 
---
PS: I don't have write permissions to mesa git

 src/mesa/main/readpix.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/mesa/main/readpix.c b/src/mesa/main/readpix.c
index a4eea3043d..c5fc66988b 100644
--- a/src/mesa/main/readpix.c
+++ b/src/mesa/main/readpix.c
@@ -106,7 +106,8 @@ _mesa_get_readpixels_transfer_ops(const struct gl_context 
*ctx,
   /* For blit-based ReadPixels packing, the clamping is done automatically
* unless the type is float. */
   if (_mesa_get_clamp_read_color(ctx, ctx->ReadBuffer) &&
-  (type == GL_FLOAT || type == GL_HALF_FLOAT)) {
+  (type == GL_FLOAT || type == GL_HALF_FLOAT ||
+   type == GL_UNSIGNED_INT_10F_11F_11F_REV)) {
  transferOps |= IMAGE_CLAMP_BIT;
   }
}
@@ -114,7 +115,8 @@ _mesa_get_readpixels_transfer_ops(const struct gl_context 
*ctx,
   /* For CPU-based ReadPixels packing, the clamping must always be done
* for non-float types, */
   if (_mesa_get_clamp_read_color(ctx, ctx->ReadBuffer) ||
-  (type != GL_FLOAT && type != GL_HALF_FLOAT)) {
+  (type != GL_FLOAT && type != GL_HALF_FLOAT &&
+   type != GL_UNSIGNED_INT_10F_11F_11F_REV)) {
  transferOps |= IMAGE_CLAMP_BIT;
   }
}
-- 
2.17.0

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev