[virglrenderer-devel] [PATCH v2 2/2] blitter: Make fbo sRGB state always act like on GLES

[Robert Tarasov]

Reviewed-by: Robert Tarasov <t <tarceri at itsqueeze.com>
utankhamen at chromium.org>

On Thu, Jul 26, 2018 at 6:04 AM Gert Wollny <gert.wollny at collabora.com>
wrote:

> On an GL host set the sRGB blit framebuffer state explicitly to make virgl
> behave like on a GLES host.
>
> This does not correct the handing of sRGB completely, because the state
> GL_FRAMEBUFFER_SRGB is not properly transmitted to the host. As a result
> the piglits  "blit texture linear_to_srgb * * *" flip.
> Tests thatset "enable" failed before and pass now, and tests that set
> "disable"
> now fail.
>
> v2: - Move setting the fbo state out of the loop (Robert Tarasov)
>     - Use the blitter context to set and store the state, since it is
> only,
>       and currently we don't pass the state from the guest
>       relevant when the dst texture is SRGB, there is no need to disable it
>     - Just enforce that a source SRGB texture is always decoded when this
> could
>       be disabled
>
> Fixes on GL host:
>   dEQP-GLES3.functional.fbo.blit.conversion.rgb8_to_srgb8_alpha8
>   dEQP-GLES3.functional.fbo.blit.default_framebuffer.srgb8_alpha8
>
> Signed-off-by: Gert Wollny <gert.wollny at collabora.com>
>
> blitter: check srgb feature
> Signed-off-by: Gert Wollny <gert.wollny at collabora.com>
> ---
>  src/vrend_blitter.c  | 13 +++++++++++--
>  src/vrend_renderer.c | 10 +++++++++-
>  src/vrend_renderer.h |  3 ++-
>  3 files changed, 22 insertions(+), 4 deletions(-)
>
> diff --git a/src/vrend_blitter.c b/src/vrend_blitter.c
> index 338577d..dd3c626 100644
> --- a/src/vrend_blitter.c
> +++ b/src/vrend_blitter.c
> @@ -55,6 +55,7 @@ struct vrend_blitter_ctx {
>     virgl_gl_context gl_context;
>     bool initialised;
>     bool use_gles;
> +   int framebuffer_srgb_enabled;
>
>     GLuint vaoid;
>
> @@ -460,6 +461,10 @@ static void vrend_renderer_init_blit_ctx(struct
> vrend_blitter_ctx *blit_ctx)
>        blit_ctx->vertices[i][0][3] = 1; /*v.w*/
>     glBindVertexArray(blit_ctx->vaoid);
>     glBindBuffer(GL_ARRAY_BUFFER, blit_ctx->vbo_id);
> +
> +   if (!blit_ctx->use_gles)
> +      glEnable(GL_FRAMEBUFFER_SRGB);
> +   blit_ctx->framebuffer_srgb_enabled = true;
>  }
>
>  static inline GLenum convert_mag_filter(unsigned int filter)
> @@ -686,7 +691,8 @@ static void calc_dst_deltas_from_src(const struct
> pipe_blit_info *info,
>  void vrend_renderer_blit_gl(UNUSED struct vrend_context *ctx,
>                              struct vrend_resource *src_res,
>                              struct vrend_resource *dst_res,
> -                            const struct pipe_blit_info *info)
> +                            const struct pipe_blit_info *info,
> +                            bool has_texture_srgb_decode)
>  {
>     struct vrend_blitter_ctx *blit_ctx = &vrend_blit_ctx;
>     GLuint buffers;
> @@ -787,6 +793,9 @@ void vrend_renderer_blit_gl(UNUSED struct
> vrend_context *ctx,
>                        to_gl_swizzle(src_entry->swizzle[3]));
>     }
>
> +   /* Just make sure that no stale state disabled decoding */
> +   if (has_texture_srgb_decode &&
> util_format_is_srgb(src_res->base.format))
> +         glTexParameteri(src_res->target, GL_TEXTURE_SRGB_DECODE_EXT,
> GL_DECODE_EXT);
>
>     glTexParameteri(src_res->target, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE);
>     glTexParameteri(src_res->target, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE);
> @@ -833,4 +842,4 @@ void vrend_renderer_blit_gl(UNUSED struct
> vrend_context *ctx,
>                               GL_TEXTURE_2D, 0, 0);
>     glFramebufferTexture2DEXT(GL_FRAMEBUFFER_EXT, GL_COLOR_ATTACHMENT0,
>                               GL_TEXTURE_2D, 0, 0);
> -}
> +}
> \ No newline at end of file
> diff --git a/src/vrend_renderer.c b/src/vrend_renderer.c
> index 8829d97..d77bf82 100644
> --- a/src/vrend_renderer.c
> +++ b/src/vrend_renderer.c
> @@ -6746,7 +6746,8 @@ static void vrend_renderer_blit_int(struct
> vrend_context *ctx,
>        use_gl = true;
>
>     if (use_gl) {
> -      vrend_renderer_blit_gl(ctx, src_res, dst_res, info);
> +      vrend_renderer_blit_gl(ctx, src_res, dst_res, info,
> +                             has_feature(feat_texture_srgb_decode));
>        vrend_clicbs->make_current(0, ctx->sub->gl_context);
>        return;
>     }
> @@ -6865,6 +6866,13 @@ static void vrend_renderer_blit_int(struct
> vrend_context *ctx,
>        vrend_fb_bind_texture(dst_res, 0, info->dst.level, info->dst.box.z
> + i);
>        glBindFramebuffer(GL_DRAW_FRAMEBUFFER, ctx->sub->blit_fb_ids[1]);
>
> +      if (!vrend_state.use_gles) {
> +         if (util_format_is_srgb(dst_res->base.format))
> +            glEnable(GL_FRAMEBUFFER_SRGB);
> +         else
> +            glDisable(GL_FRAMEBUFFER_SRGB);
> +      }
> +
>        glBindFramebuffer(GL_READ_FRAMEBUFFER, intermediate_fbo);
>
>        glBlitFramebuffer(info->src.box.x,
> diff --git a/src/vrend_renderer.h b/src/vrend_renderer.h
> index 8f1192e..9a8f10d 100644
> --- a/src/vrend_renderer.h
> +++ b/src/vrend_renderer.h
> @@ -375,7 +375,8 @@ boolean format_is_copy_compatible(enum pipe_format
> src, enum pipe_format dst);
>  void vrend_renderer_blit_gl(struct vrend_context *ctx,
>                              struct vrend_resource *src_res,
>                              struct vrend_resource *dst_res,
> -                            const struct pipe_blit_info *info);
> +                            const struct pipe_blit_info *info,
> +                            bool has_texture_srgb_decode);
>
>  void vrend_renderer_reset(void);
>  int vrend_renderer_get_poll_fd(void);
> --
> 2.16.4
>
> _______________________________________________
> virglrenderer-devel mailing list
> virglrenderer-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/virglrenderer-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/virglrenderer-devel/attachments/20180731/0c0bd4f7/attachment.html>