Fwd: [PATCH v2 12/12] ui/dbus: fix texture sharing

[Akihiko Odaki <akihiko.odaki@gmail.com>]

I mistakenly dropped CC (again). My apologies.

---------- Forwarded message ---------
From: Akihiko Odaki <akihiko.odaki@gmail.com>
Date: Thu, Feb 17, 2022 at 9:38 PM
Subject: Re: [PATCH v2 12/12] ui/dbus: fix texture sharing
To: Marc-André Lureau <marcandre.lureau@redhat.com>


On Thu, Feb 17, 2022 at 9:02 PM <marcandre.lureau@redhat.com> wrote:
>
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> The DBus listener naively create, update and destroy textures without
> taking into account other listeners. The texture were shared, but
> texture update was unnecessarily duplicated.
>
> Teach DisplayGLCtx to do optionally shared texture handling. This is
> only implemented for DBus display at this point, however the same
> infrastructure could potentially be used for egl-headless & VNC
> listeners for example, or other future combinations.

egl-headless only needs one DisplayChangeListener per console since
its output is propagated by ui/console. The vnc protocol is for remote
access and therefore cannot have OpenGL textures. The functionality is
unique to dbus.

>
> Reported-by: Akihiko Odaki <akihiko.odaki@gmail.com>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  include/ui/console.h | 10 ++++++++++
>  ui/console.c         | 26 ++++++++++++++++++++++++++
>  ui/dbus-listener.c   | 11 -----------
>  ui/dbus.c            | 24 ++++++++++++++++++++++++
>  4 files changed, 60 insertions(+), 11 deletions(-)
>
> diff --git a/include/ui/console.h b/include/ui/console.h
> index 18a10c0b7db0..0f84861933e1 100644
> --- a/include/ui/console.h
> +++ b/include/ui/console.h
> @@ -290,10 +290,20 @@ typedef struct DisplayGLCtxOps {
>                                 QEMUGLContext ctx);
>      int (*dpy_gl_ctx_make_current)(DisplayGLCtx *dgc,
>                                     QEMUGLContext ctx);
> +    void (*dpy_gl_ctx_create_texture)(DisplayGLCtx *dgc,
> +                                      DisplaySurface *surface);
> +    void (*dpy_gl_ctx_destroy_texture)(DisplayGLCtx *dgc,
> +                                      DisplaySurface *surface);
> +    void (*dpy_gl_ctx_update_texture)(DisplayGLCtx *dgc,
> +                                      DisplaySurface *surface,
> +                                      int x, int y, int w, int h);
>  } DisplayGLCtxOps;
>
>  struct DisplayGLCtx {
>      const DisplayGLCtxOps *ops;
> +#ifdef CONFIG_OPENGL
> +    QemuGLShader *gls; /* optional shared shader */
> +#endif
>  };
>
>  DisplayState *init_displaystate(void);
> diff --git a/ui/console.c b/ui/console.c
> index 102fcf0a5068..b9188559fb12 100644
> --- a/ui/console.c
> +++ b/ui/console.c
> @@ -1066,6 +1066,27 @@ static void displaychangelistener_gfx_switch(DisplayChangeListener *dcl,
>      }
>  }
>
> +static void dpy_gfx_create_texture(QemuConsole *con, DisplaySurface *surface)
> +{
> +    if (con->gl && con->gl->ops->dpy_gl_ctx_create_texture) {
> +        con->gl->ops->dpy_gl_ctx_create_texture(con->gl, surface);
> +    }
> +}
> +
> +static void dpy_gfx_destroy_texture(QemuConsole *con, DisplaySurface *surface)
> +{
> +    if (con->gl && con->gl->ops->dpy_gl_ctx_destroy_texture) {
> +        con->gl->ops->dpy_gl_ctx_destroy_texture(con->gl, surface);
> +    }
> +}
> +
> +static void dpy_gfx_update_texture(QemuConsole *con, DisplaySurface *surface,
> +                                   int x, int y, int w, int h)
> +{
> +    if (con->gl && con->gl->ops->dpy_gl_ctx_update_texture) {
> +        con->gl->ops->dpy_gl_ctx_update_texture(con->gl, surface, x, y, w, h);
> +    }
> +}
>
>  static void displaychangelistener_display_console(DisplayChangeListener *dcl,
>                                                    QemuConsole *con,
> @@ -1078,6 +1099,7 @@ static void displaychangelistener_display_console(DisplayChangeListener *dcl,
>      if (!con || !console_compatible_with(con, dcl, errp)) {
>          if (!dummy) {
>              dummy = qemu_create_placeholder_surface(640, 480, nodev);
> +            dpy_gfx_create_texture(con, dummy);
>          }
>          displaychangelistener_gfx_switch(dcl, dummy);
>          return;
> @@ -1098,6 +1120,7 @@ static void displaychangelistener_display_console(DisplayChangeListener *dcl,
>                                           con->scanout.texture.width,
>                                           con->scanout.texture.height);
>      } else if (con->scanout.kind == SCANOUT_SURFACE) {
> +        dpy_gfx_create_texture(con, con->surface);
>          displaychangelistener_gfx_switch(dcl, con->surface);
>      }
>
> @@ -1634,6 +1657,7 @@ void dpy_gfx_update(QemuConsole *con, int x, int y, int w, int h)
>      if (!qemu_console_is_visible(con)) {
>          return;
>      }
> +    dpy_gfx_update_texture(con, con->surface, x, y, w, h);
>      QLIST_FOREACH(dcl, &s->listeners, next) {
>          if (con != (dcl->con ? dcl->con : active_console)) {
>              continue;
> @@ -1678,12 +1702,14 @@ void dpy_gfx_replace_surface(QemuConsole *con,
>
>      con->scanout.kind = SCANOUT_SURFACE;
>      con->surface = surface;
> +    dpy_gfx_create_texture(con, surface);
>      QLIST_FOREACH(dcl, &s->listeners, next) {
>          if (con != (dcl->con ? dcl->con : active_console)) {
>              continue;
>          }
>          displaychangelistener_gfx_switch(dcl, surface);
>      }
> +    dpy_gfx_destroy_texture(con, old_surface);
>      qemu_free_displaysurface(old_surface);
>  }
>
> diff --git a/ui/dbus-listener.c b/ui/dbus-listener.c
> index 81c119b13a2c..a287edd2fc15 100644
> --- a/ui/dbus-listener.c
> +++ b/ui/dbus-listener.c
> @@ -42,7 +42,6 @@ struct _DBusDisplayListener {
>
>      DisplayChangeListener dcl;
>      DisplaySurface *ds;
> -    QemuGLShader *gls;
>      int gl_updates;
>  };
>
> @@ -240,10 +239,6 @@ static void dbus_gl_gfx_update(DisplayChangeListener *dcl,
>  {
>      DBusDisplayListener *ddl = container_of(dcl, DBusDisplayListener, dcl);
>
> -    if (ddl->ds) {
> -        surface_gl_update_texture(ddl->gls, ddl->ds, x, y, w, h);
> -    }
> -
>      ddl->gl_updates++;
>  }
>
> @@ -285,15 +280,11 @@ static void dbus_gl_gfx_switch(DisplayChangeListener *dcl,
>  {
>      DBusDisplayListener *ddl = container_of(dcl, DBusDisplayListener, dcl);
>
> -    if (ddl->ds) {
> -        surface_gl_destroy_texture(ddl->gls, ddl->ds);
> -    }
>      ddl->ds = new_surface;
>      if (ddl->ds) {
>          int width = surface_width(ddl->ds);
>          int height = surface_height(ddl->ds);
>
> -        surface_gl_create_texture(ddl->gls, ddl->ds);
>          /* TODO: lazy send dmabuf (there are unnecessary sent otherwise) */
>          dbus_scanout_texture(&ddl->dcl, ddl->ds->texture, false,
>                               width, height, 0, 0, width, height);
> @@ -403,7 +394,6 @@ dbus_display_listener_dispose(GObject *object)
>      g_clear_object(&ddl->conn);
>      g_clear_pointer(&ddl->bus_name, g_free);
>      g_clear_object(&ddl->proxy);
> -    g_clear_pointer(&ddl->gls, qemu_gl_fini_shader);
>
>      G_OBJECT_CLASS(dbus_display_listener_parent_class)->dispose(object);
>  }
> @@ -414,7 +404,6 @@ dbus_display_listener_constructed(GObject *object)
>      DBusDisplayListener *ddl = DBUS_DISPLAY_LISTENER(object);
>
>      if (display_opengl) {
> -        ddl->gls = qemu_gl_init_shader();
>          ddl->dcl.ops = &dbus_gl_dcl_ops;
>      } else {
>          ddl->dcl.ops = &dbus_dcl_ops;
> diff --git a/ui/dbus.c b/ui/dbus.c
> index 22c82d2f323a..8e2e4c9cad28 100644
> --- a/ui/dbus.c
> +++ b/ui/dbus.c
> @@ -55,11 +55,33 @@ dbus_is_compatible_dcl(DisplayGLCtx *dgc,
>      return dcl->ops == &dbus_gl_dcl_ops || dcl->ops == &dbus_console_dcl_ops;
>  }
>
> +static void
> +dbus_create_texture(DisplayGLCtx *ctx, DisplaySurface *surface)
> +{
> +    surface_gl_create_texture(ctx->gls, surface);
> +}
> +
> +static void
> +dbus_destroy_texture(DisplayGLCtx *ctx, DisplaySurface *surface)
> +{
> +    surface_gl_destroy_texture(ctx->gls, surface);
> +}
> +
> +static void
> +dbus_update_texture(DisplayGLCtx *ctx, DisplaySurface *surface,
> +                    int x, int y, int w, int h)
> +{
> +    surface_gl_update_texture(ctx->gls, surface, x, y, w, h);
> +}
> +
>  static const DisplayGLCtxOps dbus_gl_ops = {
>      .dpy_gl_ctx_is_compatible_dcl = dbus_is_compatible_dcl,
>      .dpy_gl_ctx_create       = dbus_create_context,
>      .dpy_gl_ctx_destroy      = qemu_egl_destroy_context,
>      .dpy_gl_ctx_make_current = qemu_egl_make_context_current,
> +    .dpy_gl_ctx_create_texture = dbus_create_texture,
> +    .dpy_gl_ctx_destroy_texture = dbus_destroy_texture,
> +    .dpy_gl_ctx_update_texture = dbus_update_texture,
>  };
>
>  static NotifierList dbus_display_notifiers =
> @@ -90,6 +112,7 @@ dbus_display_init(Object *o)
>      g_autoptr(GDBusObjectSkeleton) vm = NULL;
>
>      dd->glctx.ops = &dbus_gl_ops;
> +    dd->glctx.gls = qemu_gl_init_shader();
>      dd->iface = qemu_dbus_display1_vm_skeleton_new();
>      dd->consoles = g_ptr_array_new_with_free_func(g_object_unref);
>
> @@ -126,6 +149,7 @@ dbus_display_finalize(Object *o)
>      g_clear_object(&dd->iface);
>      g_free(dd->dbus_addr);
>      g_free(dd->audiodev);
> +    g_clear_pointer(&dd->glctx.gls, qemu_gl_fini_shader);
>      dbus_display = NULL;
>  }
>
> --
> 2.34.1.428.gdcc0cd074f0c
>