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

[Tomeu Vizoso]

Reviewed-by: Tomeu Vizoso <tomeu.vizoso at collabora.com>

Thanks,

Tomeu

On 05/15/2018 01:58 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.
> 
> This is hopefully only a problem for Mesa which we have now fixed.
> Other option is to add two extra copies and format conversions.
> 
> v2: More verbose and better comment.
> 
> Signed-off-by: Jakob Bornecrantz <jakob at collabora.com>
> ---
>   src/vrend_renderer.c | 29 +++++++++++++++++++++++++++++
>   1 file changed, 29 insertions(+)
> 
> diff --git a/src/vrend_renderer.c b/src/vrend_renderer.c
> index 8beaae1..ba89db5 100644
> --- a/src/vrend_renderer.c
> +++ b/src/vrend_renderer.c
> @@ -5396,6 +5396,35 @@ 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 read format and type.
> +      On desktop GL we can use basically any format and type to glReadPixels,
> +      so we picked the format and type that matches the native format.
> +
> +      But on GLES we are limited to a very few set, luckily most GLES
> +      implementations should return type and format that match the native
> +      formats, and can be used for glReadPixels acording to the GLES spec.
> +
> +      But we have found that at least Mesa returned the wrong formats, again
> +      luckily we are able to change Mesa. But just in case there are more bad
> +      drivers out there, or we mess up the format somewhere, we warn here. */
> +   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 is not expected native type 0x%x != imp 0x%x\n", type, imp);
> +         }
> +      }
> +      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 is not expected native format 0x%x != imp 0x%x\n", format, imp);
> +         }
> +      }
> +   }
> +
>      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)
>