[virglrenderer-devel] [PATCH] blitter: Set fbo sRGB state according to the destination sourface type

[Gert Wollny]

On an GL host set the sRGB 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.

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>
---

Since this doesn't correct the piglits, only flips some of them, I thought I 
might add a comment about the problems fixing this, I'm actually not sure
whether handling GL_FRAMEBUFFER_SRGB can really be fixed in a reliable way 
considering the approach how framebuffers and blits are implemented: On one 
hand, the state GL_FRAMEBUFFER_SRGB is currently not transmitted to the host,
but even if it would be, one still would have to guess to which framebuffer it
refers, because the guest never sends information about which framebuffers are
currently bound: This is handled within the mesa code, and Gallium isn't
notified about changes in binding.

On the other hand, if GL_FRAMEBUFFER_SRGB is disabled, but an sRGB surface is 
bound to the framebuffer then the framebuffer's state is updated by attaching 
a non-sRGB surface that is also created on the host as a texture view. Then the
changed state is then send to the host via VIRGL_CCMD_SET_FRAMEBUFFER_STATE, 
but the command doesn't have any information about *which* framebuffer's state 
must be set, since Gallium doesn't know this and virglrenderer always sets the
state of the current sub-context's default fb.

Now, when virglrenderer enters vrend_renderer_blit_int it references the 
original textures with there original formats, and when GL_FRAMEBUFFER_SRGB is 
disabled things go wrong. 

The only approach for guessing the right state I could think of is, to change 
Gallium to send the changed GL_FRAMEBUFFER_SRGB state, and when a blit comes up, 
to check whether the target texture is sRGB has a texture view that has the 
corresponding linear format and has otherwise the same parameters. 

Thanks for any comments and for reviewing the patch, 
Gert

 src/vrend_blitter.c  | 19 ++++++++++++++++++-
 src/vrend_renderer.c |  9 ++++++++-
 src/vrend_renderer.h |  3 ++-
 3 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/src/vrend_blitter.c b/src/vrend_blitter.c
index 68b5bab..1439c9b 100644
--- a/src/vrend_blitter.c
+++ b/src/vrend_blitter.c
@@ -686,7 +686,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 use_gles)
 {
    struct vrend_blitter_ctx *blit_ctx = &vrend_blit_ctx;
    GLuint buffers;
@@ -787,6 +788,8 @@ void vrend_renderer_blit_gl(UNUSED struct vrend_context *ctx,
                       to_gl_swizzle(src_entry->swizzle[3]));
    }
 
+   if (!use_gles && 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);
@@ -820,6 +823,13 @@ void vrend_renderer_blit_gl(UNUSED struct vrend_context *ctx,
       glBindFramebuffer(GL_FRAMEBUFFER_EXT, blit_ctx->fb_id);
       vrend_fb_bind_texture(dst_res, 0, info->dst.level, layer);
 
+      if (!use_gles) {
+         if (util_format_is_srgb(dst_res->base.format))
+            glEnable(GL_FRAMEBUFFER_SRGB);
+         else
+            glDisable(GL_FRAMEBUFFER_SRGB);
+      }
+
       buffers = GL_COLOR_ATTACHMENT0_EXT;
       glDrawBuffers(1, &buffers);
       blitter_set_texcoords(blit_ctx, src_res, info->src.level,
@@ -829,4 +839,11 @@ void vrend_renderer_blit_gl(UNUSED struct vrend_context *ctx,
       glBufferData(GL_ARRAY_BUFFER, sizeof(blit_ctx->vertices), blit_ctx->vertices, GL_STATIC_DRAW);
       glDrawArrays(GL_TRIANGLE_FAN, 0, 4);
    }
+   if (!use_gles) {
+      if (util_format_is_srgb(dst_res->base.format))
+         glDisable(GL_FRAMEBUFFER_SRGB);
+
+      if (util_format_is_srgb(src_res->base.format))
+         glTexParameteri(src_res->target, GL_TEXTURE_SRGB_DECODE_EXT, GL_DECODE_EXT);
+   }
 }
diff --git a/src/vrend_renderer.c b/src/vrend_renderer.c
index e357bae..8b3e513 100644
--- a/src/vrend_renderer.c
+++ b/src/vrend_renderer.c
@@ -6571,7 +6571,7 @@ static void vrend_renderer_blit_int(struct vrend_context *ctx,
       use_gl = !vrend_formats_can_blit(tex_conv_table, src_res->base.format, dst_res->base.format);
 
    if (use_gl) {
-      vrend_renderer_blit_gl(ctx, src_res, dst_res, info);
+      vrend_renderer_blit_gl(ctx, src_res, dst_res, info, vrend_state.use_gles);
       vrend_clicbs->make_current(0, ctx->sub->gl_context);
       return;
    }
@@ -6690,6 +6690,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 28e2819..c873c8c 100644
--- a/src/vrend_renderer.h
+++ b/src/vrend_renderer.h
@@ -388,7 +388,8 @@ bool vrend_formats_can_blit(struct vrend_format_table *table, enum virgl_formats
 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 use_gles);
 
 void vrend_renderer_reset(void);
 int vrend_renderer_get_poll_fd(void);
-- 
2.17.1