Re: [Qemu-devel] Patch: Precautionary glBindTexture in surface_gl_update_texture

[Marcel Apfelbaum]

Hi Qiming,
Thanks for submitting the patch!

On 5/6/19 10:50 AM, Hou Qiming wrote:
> From 48d1f092a7960d711fb2c77ab8d3f9d0a0ca0d5c Mon Sep 17 00:00:00 2001
> From: HQM <address@hidden <mailto:address@hidden>>
> Date: Mon, 6 May 2019 15:37:59 +0800
> Subject: [PATCH] Precautionary glBindTexture and surface->texture 
> validation
>  in surface_gl_update_texture

The lines above should not go into the patch comment, while the mail 
subject should
start with [PATCH].

I suggest preparing the patch with git format-patch and sending it with 
git send-email.
You can also prepare it "manually" as long the format is correct.

> In a GVT-g setup with dmabuf and GTK GUI, the current 2D texture at 
> surface_gl_update_texture is not necessarily
> surface->texture. Adding a glBindTexture fixes related crashes and 
> artifacts, and is generally more secure.
>
> Signed-off-by: HQM <address@hidden <mailto:address@hidden>>
> ---
>  ui/console-gl.c | 18 +++++++++++-------
>  1 file changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/ui/console-gl.c b/ui/console-gl.c
> index a56e1cd..c1cb3bd 100644
> --- a/ui/console-gl.c
> +++ b/ui/console-gl.c
> @@ -92,13 +92,17 @@ void surface_gl_update_texture(QemuGLShader *gls,
>
>      assert(gls);
>
> -    glPixelStorei(GL_UNPACK_ROW_LENGTH_EXT,
> -                  surface_stride(surface) / 
> surface_bytes_per_pixel(surface));
> -    glTexSubImage2D(GL_TEXTURE_2D, 0,
> -                    x, y, w, h,
> -                    surface->glformat, surface->gltype,
> -                    data + surface_stride(surface) * y
> -                    + surface_bytes_per_pixel(surface) * x);
> +    if (surface->texture) {

I confirm it fixes a boot QEMU crash when the Windows guest i915 driver 
loads.

> +        glBindTexture(GL_TEXTURE_2D, surface->texture);

I confirm it fixes strange artifacts seen on screen (some huge mouse 
icon on the upper left side)
when guest monitor "turns off" or the GTK window gets resized and the 
guest desktop resolution changes.

> +        glPixelStorei(GL_UNPACK_ROW_LENGTH_EXT,
> +                      surface_stride(surface)
> +                      / surface_bytes_per_pixel(surface));
> +        glTexSubImage2D(GL_TEXTURE_2D, 0,
> +                        x, y, w, h,
> +                        surface->glformat, surface->gltype,
> +                        data + surface_stride(surface) * y
> +                        + surface_bytes_per_pixel(surface) * x);
> +    }
>  }
>
>  void surface_gl_render_texture(QemuGLShader *gls,
> --
> 2.17.1

I have no OpenGL background to understand the consequences, but the patch
does solve 2 gvt issues, so:

Tested-by: Marcel Apfelbaum<address@hidden>


Thanks,
Marcel