[virglrenderer-devel] [PATCH] vrend: Warn on different type and format when reading back FBOs

[Tomeu Vizoso]

On 05/15/2018 12:35 PM, Jakob Bornecrantz wrote:
> Warn if the expected native format/type differs from the allowed
> type and format that are accepted by the OpenGL ES spec.

I think besides the what, we should also mention always the why. For example:

"""
GLES clients in Android expect to be always able to read pixels in the 
native format, so to be compliant with both the spec and dEQP, the GLES 
implementation in the host needs to return the native formats in its 
GL_IMPLEMENTATION_COLOR_READ_* implementations.

To ease troubleshooting when the host stack doesn't meet that 
requirement, warn when this condition is encountered.
"""

That could save a few WTFs later as we get this running on top of more blobs.

> Signed-off-by: Jakob Bornecrantz <jakob at collabora.com>
> ---
>   src/vrend_renderer.c | 19 +++++++++++++++++++
>   1 file changed, 19 insertions(+)
> 
> diff --git a/src/vrend_renderer.c b/src/vrend_renderer.c
> index 8beaae1..bc9929f 100644
> --- a/src/vrend_renderer.c
> +++ b/src/vrend_renderer.c
> @@ -5396,6 +5396,25 @@ static int vrend_transfer_send_readpixels(struct vrend_context *ctx,
>            glPixelTransferf(GL_DEPTH_SCALE, depth_scale);
>         }
>      }
> +
> +   // Warn if the driver doesn't agree about the native format.
> +   if (vrend_state.use_gles) {
> +      GLint imp;
> +      if (type != GL_UNSIGNED_BYTE && format != GL_UNSIGNED_INT &&
> +          type != GL_INT && type != GL_FLOAT) {
> +         glGetIntegerv(GL_IMPLEMENTATION_COLOR_READ_TYPE, &imp);
> +         if (imp != type) {
> +            fprintf(stderr, "GL_IMPLEMENTATION_COLOR_READ_TYPE 0x%x != type 0x%x\n", imp, type);

Maybe make it a more verbose?

Thanks,

Tomeu

> +         }
> +      }
> +      if (format != GL_RGBA && format != GL_RGBA_INTEGER) {
> +         glGetIntegerv(GL_IMPLEMENTATION_COLOR_READ_FORMAT, &imp);
> +         if (imp != format) {
> +            fprintf(stderr, "GL_IMPLEMENTATION_COLOR_READ_FORMAT 0x%x != format 0x%x\n", imp, format);
> +         }
> +      }
> +   }
> +
>      if (vrend_state.have_arb_robustness)
>         glReadnPixelsARB(info->box->x, y1, info->box->width, info->box->height, format, type, send_size, data);
>      else if (vrend_state.have_gles_khr_robustness)
>