I mistakenly dropped CC (again). My apologies.
---------- Forwarded message ---------
From: Akihiko Odaki <akihiko.od...@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.lur...@redhat.com>
On Thu, Feb 17, 2022 at 9:02 PM <marcandre.lur...@redhat.com> wrote:
>
> From: Marc-André Lureau <marcandre.lur...@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.od...@gmail.com>
> Signed-off-by: Marc-André Lureau <marcandre.lur...@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
>
