meson.build | 8 ++++++++ ui/dbus-listener.c | 25 +++++++++++++++++++------ ui/meson.build | 2 +- 3 files changed, 28 insertions(+), 7 deletions(-)
From: Christian Hergert <chergert@redhat.com>
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.
In the future, when additional methods are allowed on the D-Bus
interface,
this should likely be updated to send damage regions as a single RPC to
avoid additional message processing.
Currently, Linux does not propagate damage rectangles when using the
virtio-gpu drm driver. That means compositors such as Mutter which
utilize
drmModePageFlip() will be sending full or near-full surface damages.
https://lists.freedesktop.org/archives/dri-devel/2023-March/395164.html
contains a patch to fix that too.
Signed-off-by: Bilal Elmoussaoui <belmouss@redhat.com>
---
meson.build | 8 ++++++++
ui/dbus-listener.c | 25 +++++++++++++++++++------
ui/meson.build | 2 +-
3 files changed, 28 insertions(+), 7 deletions(-)
diff --git a/meson.build b/meson.build
index 229eb585f7..72678ef78e 100644
--- a/meson.build
+++ b/meson.build
@@ -1761,6 +1761,14 @@ dbus_display = get_option('dbus_display') \
error_message: '-display dbus is not available on Windows') \
.allowed()
+cairo = not_found
+if dbus_display
+ cairo = dependency('cairo',
+ kwargs: static_kwargs,
+ method: 'pkg-config',
+ )
+endif
+
have_virtfs = get_option('virtfs') \
.require(targetos == 'linux' or targetos == 'darwin',
error_message: 'virtio-9p (virtfs) requires Linux or macOS') \
diff --git a/ui/dbus-listener.c b/ui/dbus-listener.c
index 911acdc529..047be5cb3a 100644
--- a/ui/dbus-listener.c
+++ b/ui/dbus-listener.c
@@ -25,6 +25,7 @@
#include "qemu/error-report.h"
#include "sysemu/sysemu.h"
#include "dbus.h"
+#include <cairo.h>
#include <gio/gunixfdlist.h>
#ifdef CONFIG_OPENGL
@@ -43,9 +44,10 @@ struct _DBusDisplayListener {
QemuDBusDisplay1Listener *proxy;
+ cairo_region_t *gl_damage;
+
DisplayChangeListener dcl;
DisplaySurface *ds;
- int gl_updates;
};
G_DEFINE_TYPE(DBusDisplayListener, dbus_display_listener, G_TYPE_OBJECT)
@@ -226,10 +228,17 @@ static void dbus_gl_refresh(DisplayChangeListener *dcl)
return;
}
- if (ddl->gl_updates) {
- dbus_call_update_gl(ddl, 0, 0,
- surface_width(ddl->ds), surface_height(ddl->ds));
- ddl->gl_updates = 0;
+ if (ddl->gl_damage) {
+ int n_rects = cairo_region_num_rectangles(ddl->gl_damage);
+
+ for (int i = 0; i < n_rects; i++) {
+ cairo_rectangle_int_t rect;
+
+ cairo_region_get_rectangle(ddl->gl_damage, i, &rect);
+ dbus_call_update_gl(ddl, rect.x, rect.y, rect.width, rect.height);
+ }
+
+ g_clear_pointer(&ddl->gl_damage, cairo_region_destroy);
}
}
#endif
@@ -245,7 +254,11 @@ static void dbus_gl_gfx_update(DisplayChangeListener *dcl,
{
DBusDisplayListener *ddl = container_of(dcl, DBusDisplayListener, dcl);
- ddl->gl_updates++;
+ if (ddl->gl_damage == NULL) {
+ ddl->gl_damage = cairo_region_create();
+ }
+ cairo_region_union_rectangle(ddl->gl_damage,
+ &(cairo_rectangle_int_t) {x, y, w, h});
}
#endif
diff --git a/ui/meson.build b/ui/meson.build
index 330369707d..8b4004ff01 100644
--- a/ui/meson.build
+++ b/ui/meson.build
@@ -85,7 +85,7 @@ if dbus_display
'--generate-c-code', '@BASENAME@'])
dbus_display1_lib = static_library('dbus-display1', dbus_display1, dependencies: gio)
dbus_display1_dep = declare_dependency(link_with: dbus_display1_lib, include_directories: include_directories('.'))
- dbus_ss.add(when: [gio, pixman, dbus_display1_dep],
+ dbus_ss.add(when: [gio, pixman, dbus_display1_dep, cairo],
if_true: [files(
'dbus-chardev.c',
'dbus-clipboard.c',
--
2.38.4Hi
On Tue, May 9, 2023 at 3:25 PM ~bilelmoussaoui <bilelmoussaoui@git.sr.ht>
wrote:
> From: Christian Hergert <chergert@redhat.com>
>
> 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.
>
> In the future, when additional methods are allowed on the D-Bus
> interface,
> this should likely be updated to send damage regions as a single RPC to
> avoid additional message processing.
>
Yes, I am not sure what is best. Either we introduce extra interfaces
(org.qemu.Display1.Listener.UpdateWithRegions ?), or we add the method as
part of the existing Listener interface. Either approach will need some
introspection mechanism. It may be worth to avoid the need for parsing the
introspect XML (just like
https://dbus.freedesktop.org/doc/dbus-specification.html#message-bus-properties-interfaces
did)
>
> Currently, Linux does not propagate damage rectangles when using the
> virtio-gpu drm driver. That means compositors such as Mutter which
> utilize
> drmModePageFlip() will be sending full or near-full surface damages.
>
> https://lists.freedesktop.org/archives/dri-devel/2023-March/395164.html
> contains a patch to fix that too.
>
It's in the tree now:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=01f05940a9a75e11a2be64993c44ad8dd06e6e26
>
> Signed-off-by: Bilal Elmoussaoui <belmouss@redhat.com>
> ---
> meson.build | 8 ++++++++
> ui/dbus-listener.c | 25 +++++++++++++++++++------
> ui/meson.build | 2 +-
> 3 files changed, 28 insertions(+), 7 deletions(-)
>
> diff --git a/meson.build b/meson.build
> index 229eb585f7..72678ef78e 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1761,6 +1761,14 @@ dbus_display = get_option('dbus_display') \
> error_message: '-display dbus is not available on Windows') \
> .allowed()
>
> +cairo = not_found
> +if dbus_display
> + cairo = dependency('cairo',
> + kwargs: static_kwargs,
> + method: 'pkg-config',
> + )
>
cairo is a pretty large for -display dbus. I wouldn't introduce it for such
a small task of tracking rectangular updated regions. Given that the dbus
code is meant to be used locally, I think it's fine if we simply keep and
send a list of damaged rectangles (without any smart code to try to
optimize it - which I assume cairo would do)
> +endif
> +
> have_virtfs = get_option('virtfs') \
> .require(targetos == 'linux' or targetos == 'darwin',
> error_message: 'virtio-9p (virtfs) requires Linux or macOS')
> \
> diff --git a/ui/dbus-listener.c b/ui/dbus-listener.c
> index 911acdc529..047be5cb3a 100644
> --- a/ui/dbus-listener.c
> +++ b/ui/dbus-listener.c
> @@ -25,6 +25,7 @@
> #include "qemu/error-report.h"
> #include "sysemu/sysemu.h"
> #include "dbus.h"
> +#include <cairo.h>
> #include <gio/gunixfdlist.h>
>
> #ifdef CONFIG_OPENGL
> @@ -43,9 +44,10 @@ struct _DBusDisplayListener {
>
> QemuDBusDisplay1Listener *proxy;
>
> + cairo_region_t *gl_damage;
> +
> DisplayChangeListener dcl;
> DisplaySurface *ds;
> - int gl_updates;
> };
>
> G_DEFINE_TYPE(DBusDisplayListener, dbus_display_listener, G_TYPE_OBJECT)
> @@ -226,10 +228,17 @@ static void dbus_gl_refresh(DisplayChangeListener
> *dcl)
> return;
> }
>
> - if (ddl->gl_updates) {
> - dbus_call_update_gl(ddl, 0, 0,
> - surface_width(ddl->ds),
> surface_height(ddl->ds));
> - ddl->gl_updates = 0;
> + if (ddl->gl_damage) {
> + int n_rects = cairo_region_num_rectangles(ddl->gl_damage);
> +
> + for (int i = 0; i < n_rects; i++) {
> + cairo_rectangle_int_t rect;
> +
> + cairo_region_get_rectangle(ddl->gl_damage, i, &rect);
> + dbus_call_update_gl(ddl, rect.x, rect.y, rect.width,
> rect.height);
> + }
> +
> + g_clear_pointer(&ddl->gl_damage, cairo_region_destroy);
> }
> }
> #endif
> @@ -245,7 +254,11 @@ static void dbus_gl_gfx_update(DisplayChangeListener
> *dcl,
> {
> DBusDisplayListener *ddl = container_of(dcl, DBusDisplayListener,
> dcl);
>
> - ddl->gl_updates++;
> + if (ddl->gl_damage == NULL) {
> + ddl->gl_damage = cairo_region_create();
> + }
> + cairo_region_union_rectangle(ddl->gl_damage,
> + &(cairo_rectangle_int_t) {x, y, w, h});
> }
> #endif
>
> diff --git a/ui/meson.build b/ui/meson.build
> index 330369707d..8b4004ff01 100644
> --- a/ui/meson.build
> +++ b/ui/meson.build
> @@ -85,7 +85,7 @@ if dbus_display
> '--generate-c-code', '@BASENAME@
> '])
> dbus_display1_lib = static_library('dbus-display1', dbus_display1,
> dependencies: gio)
> dbus_display1_dep = declare_dependency(link_with: dbus_display1_lib,
> include_directories: include_directories('.'))
> - dbus_ss.add(when: [gio, pixman, dbus_display1_dep],
> + dbus_ss.add(when: [gio, pixman, dbus_display1_dep, cairo],
> if_true: [files(
> 'dbus-chardev.c',
> 'dbus-clipboard.c',
> --
> 2.38.4
>
>
© 2016 - 2024 Red Hat, Inc.