Hi
On Mon, Aug 14, 2023 at 4:10 PM Bilal Elmoussaoui <belmo...@redhat.com> wrote:
>
> Currently, when using `-display dbus,gl=on` all updates to the client
> become "full scanout" updates, meaning there is no way for the client to
> limit damage regions to the display server.
>
> Instead of using an "update count", this patch tracks the damage region
> and propagates it to the client.
>
> This was less of an issue when clients were using GtkGLArea for
> rendering,
> as you'd be doing full-surface redraw. To be efficient, the client needs
> both a DMA-BUF and the damage region to be updated.
>
> Co-authored-by: Christian Hergert <cherg...@redhat.com>
> Signed-off-by: Bilal Elmoussaoui <belmo...@redhat.com>
> ---
> ui/dbus-listener.c | 32 ++++++++++++++++++++++++--------
> 1 file changed, 24 insertions(+), 8 deletions(-)
>
> diff --git a/ui/dbus-listener.c b/ui/dbus-listener.c
> index 30917271ab..d015e8d759 100644
> --- a/ui/dbus-listener.c
> +++ b/ui/dbus-listener.c
> @@ -26,6 +26,9 @@
> #include "qapi/error.h"
> #include "sysemu/sysemu.h"
> #include "dbus.h"
> +#ifdef CONFIG_OPENGL
> +#include <pixman.h>
> +#endif
> #ifdef G_OS_UNIX
> #include <gio/gunixfdlist.h>
> #endif
> @@ -59,12 +62,15 @@ struct _DBusDisplayListener {
>
> QemuDBusDisplay1Listener *proxy;
>
> +#ifdef CONFIG_OPENGL
> + /* Keep track of the damage region */
> + pixman_region32_t gl_damage;
> +#endif
I think it should call pixman_region32_init() in
dbus_display_listener_new(), & _fini() in _dispose()
> +
> DisplayChangeListener dcl;
> DisplaySurface *ds;
> enum share_kind ds_share;
>
> - int gl_updates;
> -
> bool ds_mapped;
> bool can_share_map;
>
> @@ -539,11 +545,16 @@ static void dbus_gl_refresh(DisplayChangeListener *dcl)
> return;
> }
>
> - if (ddl->gl_updates) {
> - dbus_call_update_gl(dcl, 0, 0,
> - surface_width(ddl->ds), surface_height(ddl->ds));
> - ddl->gl_updates = 0;
> + int n_rects = pixman_region32_n_rects(&ddl->gl_damage);
> +
> + for (int i = 0; i < n_rects; i++) {
> + pixman_box32_t *box;
> + box = pixman_region32_rectangles(&ddl->gl_damage, NULL) + i;
> +
> + dbus_call_update_gl(dcl, box->x1, box->y1,
> + box->x2 - box->x1, box->y2 - box->y1);
May be worth to add a "TODO: add Update*List methods" ?
> }
> + pixman_region32_clear(&ddl->gl_damage);
> }
> #endif /* OPENGL */
>
> @@ -558,7 +569,10 @@ static void dbus_gl_gfx_update(DisplayChangeListener
> *dcl,
> {
> DBusDisplayListener *ddl = container_of(dcl, DBusDisplayListener, dcl);
>
> - ddl->gl_updates++;
> + pixman_region32_t rect_region;
> + pixman_region32_init_rect(&rect_region, x, y, w, h);
> + pixman_region32_union(&ddl->gl_damage, &ddl->gl_damage, &rect_region);
> + pixman_region32_fini(&rect_region);
> }
> #endif
>
> @@ -933,7 +947,9 @@ dbus_display_listener_new(const char *bus_name,
> g_object_unref(ddl);
> return NULL;
> }
> -
> +#ifdef CONFIG_OPENGL
> + pixman_region32_init(&ddl->gl_damage);
> +#endif
> ddl->bus_name = g_strdup(bus_name);
> ddl->conn = conn;
> ddl->console = console;
> --
> 2.41.0
>
>
otherwise, lgtm
--
Marc-André Lureau
