* Re: [PATCH RFC v3 1/1] ui/dbus: add org.qemu.Display1.UIInfo interface
2026-08-18 13:45 ` [PATCH RFC v3 1/1] ui/dbus: add org.qemu.Display1.UIInfo interface Chengyang Zhu
@ 2026-08-18 15:01 ` Marc-André Lureau
2026-08-19 14:39 ` Christian Hergert
2026-08-24 12:49 ` Marc-André Lureau
1 sibling, 1 reply; 7+ messages in thread
From: Marc-André Lureau @ 2026-08-18 15:01 UTC (permalink / raw)
To: Chengyang Zhu, christian; +Cc: qemu-devel
Hi
On Tue, Aug 18, 2026 at 5:48 PM Chengyang Zhu <colazcyg@gmail.com> wrote:
>
> Currently, the SetUIInfo method cannot set a refresh rate.
> Simply adding a refresh_rate argument would break the method signature.
>
> This patch adds the UIInfo interface containing
> * properties as staged display settings.
> * property `Supported` indicating whether console UI info is supported.
> * the method `Apply` applying staged settings.
> * the method `Reload` reloading settings from the console.
>
> Signed-off-by: Chengyang Zhu <colazcyg@gmail.com>
That looks good to me. I am aware of concerns about refresh_rate unit
(https://gitlab.gnome.org/GNOME/libmks/-/merge_requests/53#note_2834466),
but I guess we will address this in due course
Christian, does this new interface look right to you?
thanks
> ---
> ui/dbus-console.c | 71 ++++++++++++++++++++++++++++++++++++++
> ui/dbus-display1.xml | 81 ++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 152 insertions(+)
>
> diff --git a/ui/dbus-console.c b/ui/dbus-console.c
> index e1ac06814b..2c71272a1c 100644
> --- a/ui/dbus-console.c
> +++ b/ui/dbus-console.c
> @@ -55,6 +55,8 @@ struct _DBusDisplayConsole {
> guint last_x;
> guint last_y;
> Notifier mouse_mode_notifier;
> +
> + QemuDBusDisplay1UIInfo *iface_ui_info;
> };
>
> G_DEFINE_TYPE(DBusDisplayConsole,
> @@ -155,6 +157,7 @@ dbus_display_console_dispose(GObject *object)
> qemu_input_led_notifier_remove(&ddc->led_notifier);
> qemu_console_unregister_listener(&ddc->dcl);
> qemu_remove_mouse_mode_change_notifier(&ddc->mouse_mode_notifier);
> + g_clear_object(&ddc->iface_ui_info);
> g_clear_object(&ddc->iface_touch);
> g_clear_object(&ddc->iface_mouse);
> g_clear_object(&ddc->iface_kbd);
> @@ -528,6 +531,63 @@ dbus_mouse_mode_change(Notifier *notify, void *data)
> dbus_mouse_update_is_absolute(ddc);
> }
>
> +static gboolean
> +dbus_ui_info_reload(DBusDisplayConsole *ddc,
> + GDBusMethodInvocation *invocation)
> +{
> + if (!qemu_console_ui_info_supported(ddc->dcl.con)) {
> + g_dbus_method_invocation_return_error(invocation,
> + DBUS_DISPLAY_ERROR,
> + DBUS_DISPLAY_ERROR_UNSUPPORTED,
> + "UIInfo is not supported");
> + return DBUS_METHOD_INVOCATION_HANDLED;
> + }
> +
> + QemuUIInfo ui_info = *qemu_console_get_ui_info(ddc->dcl.con);
> +
> + qemu_dbus_display1_uiinfo_set_width_mm(ddc->iface_ui_info,
> + ui_info.width_mm);
> + qemu_dbus_display1_uiinfo_set_height_mm(ddc->iface_ui_info,
> + ui_info.height_mm);
> + qemu_dbus_display1_uiinfo_set_xoff(ddc->iface_ui_info, ui_info.xoff);
> + qemu_dbus_display1_uiinfo_set_yoff(ddc->iface_ui_info, ui_info.yoff);
> + qemu_dbus_display1_uiinfo_set_width(ddc->iface_ui_info, ui_info.width);
> + qemu_dbus_display1_uiinfo_set_height(ddc->iface_ui_info, ui_info.height);
> + qemu_dbus_display1_uiinfo_set_refresh_rate(ddc->iface_ui_info,
> + ui_info.refresh_rate);
> +
> + qemu_dbus_display1_uiinfo_complete_reload(ddc->iface_ui_info, invocation);
> + return DBUS_METHOD_INVOCATION_HANDLED;
> +}
> +
> +static gboolean
> +dbus_ui_info_apply(DBusDisplayConsole *ddc,
> + GDBusMethodInvocation *invocation)
> +{
> + if (!qemu_console_ui_info_supported(ddc->dcl.con)) {
> + g_dbus_method_invocation_return_error(invocation,
> + DBUS_DISPLAY_ERROR,
> + DBUS_DISPLAY_ERROR_UNSUPPORTED,
> + "UIInfo is not supported");
> + return DBUS_METHOD_INVOCATION_HANDLED;
> + }
> +
> + QemuUIInfo ui_info = {
> + .width_mm = qemu_dbus_display1_uiinfo_get_width_mm(ddc->iface_ui_info),
> + .height_mm = qemu_dbus_display1_uiinfo_get_height_mm(ddc->iface_ui_info),
> + .xoff = qemu_dbus_display1_uiinfo_get_xoff(ddc->iface_ui_info),
> + .yoff = qemu_dbus_display1_uiinfo_get_yoff(ddc->iface_ui_info),
> + .width = qemu_dbus_display1_uiinfo_get_width(ddc->iface_ui_info),
> + .height = qemu_dbus_display1_uiinfo_get_height(ddc->iface_ui_info),
> + .refresh_rate = qemu_dbus_display1_uiinfo_get_refresh_rate(ddc->iface_ui_info)
> + };
> +
> + qemu_console_set_ui_info(ddc->dcl.con, &ui_info, false);
> + qemu_dbus_display1_uiinfo_complete_apply(ddc->iface_ui_info, invocation);
> +
> + return DBUS_METHOD_INVOCATION_HANDLED;
> +}
> +
> int dbus_display_console_get_index(DBusDisplayConsole *ddc)
> {
> return qemu_console_get_index(ddc->dcl.con);
> @@ -550,6 +610,7 @@ dbus_display_console_new(DBusDisplay *display, QemuConsole *con)
> "org.qemu.Display1.Keyboard",
> "org.qemu.Display1.Mouse",
> "org.qemu.Display1.MultiTouch",
> + "org.qemu.Display1.UIInfo",
> NULL
> };
>
> @@ -626,5 +687,15 @@ dbus_display_console_new(DBusDisplay *display, QemuConsole *con)
> qemu_add_mouse_mode_change_notifier(&ddc->mouse_mode_notifier);
> dbus_mouse_update_is_absolute(ddc);
>
> + ddc->iface_ui_info = qemu_dbus_display1_uiinfo_skeleton_new();
> + qemu_dbus_display1_uiinfo_set_supported(ddc->iface_ui_info,
> + qemu_console_ui_info_supported(ddc->dcl.con));
> + g_object_connect(ddc->iface_ui_info,
> + "swapped-signal::handle-reload", dbus_ui_info_reload, ddc,
> + "swapped-signal::handle-apply", dbus_ui_info_apply, ddc,
> + NULL);
> + g_dbus_object_skeleton_add_interface(G_DBUS_OBJECT_SKELETON(ddc),
> + G_DBUS_INTERFACE_SKELETON(ddc->iface_ui_info));
> +
> return ddc;
> }
> diff --git a/ui/dbus-display1.xml b/ui/dbus-display1.xml
> index d96bae2ed6..2a8a8563ee 100644
> --- a/ui/dbus-display1.xml
> +++ b/ui/dbus-display1.xml
> @@ -1159,4 +1159,85 @@
> -->
> <property name="Encoding" type="s" access="read"/>
> </interface>
> +
> + <!--
> + org.qemu.Display1.UIInfo:
> +
> + This interface is implemented on
> + ``/org/qemu/Display1/Console_$id`` (see
> + :dbus:iface:`~org.qemu.Display1.Console`).
> + -->
> + <interface name="org.qemu.Display1.UIInfo">
> + <!--
> + Supported:
> +
> + Whether console UI info is supported.
> + -->
> + <property name="Supported" type="b" access="read"/>
> +
> + <!--
> + WidthMm:
> +
> + The physical display width in millimeters.
> + -->
> + <property name="WidthMm" type="q" access="readwrite"/>
> +
> + <!--
> + HeightMm:
> +
> + The physical display height in millimeters.
> + -->
> + <property name="HeightMm" type="q" access="readwrite"/>
> +
> + <!--
> + XOff:
> +
> + The horizontal offset in pixels.
> + -->
> + <property name="XOff" type="i" access="readwrite"/>
> +
> + <!--
> + YOff:
> +
> + The vertical offset in pixels.
> + -->
> + <property name="YOff" type="i" access="readwrite"/>
> +
> + <!--
> + Width:
> +
> + The console width in pixels.
> + -->
> + <property name="Width" type="u" access="readwrite"/>
> +
> + <!--
> + Height:
> +
> + The console height in pixels.
> + -->
> + <property name="Height" type="u" access="readwrite"/>
> +
> + <!--
> + RefreshRate:
> +
> + The display refresh rate in millihertz.
> + -->
> + <property name="RefreshRate" type="u" access="readwrite"/>
> +
> + <!--
> + Reload:
> +
> + Set the staged properties to the state of the console display.
> + -->
> + <method name="Reload">
> + </method>
> +
> + <!--
> + Apply:
> +
> + Apply the staged properties to the console display.
> + -->
> + <method name="Apply">
> + </method>
> + </interface>
> </node>
> --
> 2.55.0
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH RFC v3 1/1] ui/dbus: add org.qemu.Display1.UIInfo interface
2026-08-18 13:45 ` [PATCH RFC v3 1/1] ui/dbus: add org.qemu.Display1.UIInfo interface Chengyang Zhu
2026-08-18 15:01 ` Marc-André Lureau
@ 2026-08-24 12:49 ` Marc-André Lureau
1 sibling, 0 replies; 7+ messages in thread
From: Marc-André Lureau @ 2026-08-24 12:49 UTC (permalink / raw)
To: Chengyang Zhu; +Cc: qemu-devel
Hi Chengyang Zhu
On Tue, Aug 18, 2026 at 5:48 PM Chengyang Zhu <colazcyg@gmail.com> wrote:
>
> Currently, the SetUIInfo method cannot set a refresh rate.
> Simply adding a refresh_rate argument would break the method signature.
>
> This patch adds the UIInfo interface containing
> * properties as staged display settings.
> * property `Supported` indicating whether console UI info is supported.
> * the method `Apply` applying staged settings.
> * the method `Reload` reloading settings from the console.
Sorry for the back and forth, based on discussion with Christian, we
should go back to a dictionary approach.
Let's keep the extra interface, keep "supported" property and add
Get() & Apply() with extensible dictionaries. Sounds good?
Apply() should merge with existing values, I suppose.
Later on, we may want to also have a Updated/Changed signal with the
new values..
thanks
>
> Signed-off-by: Chengyang Zhu <colazcyg@gmail.com>
> ---
> ui/dbus-console.c | 71 ++++++++++++++++++++++++++++++++++++++
> ui/dbus-display1.xml | 81 ++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 152 insertions(+)
>
> diff --git a/ui/dbus-console.c b/ui/dbus-console.c
> index e1ac06814b..2c71272a1c 100644
> --- a/ui/dbus-console.c
> +++ b/ui/dbus-console.c
> @@ -55,6 +55,8 @@ struct _DBusDisplayConsole {
> guint last_x;
> guint last_y;
> Notifier mouse_mode_notifier;
> +
> + QemuDBusDisplay1UIInfo *iface_ui_info;
> };
>
> G_DEFINE_TYPE(DBusDisplayConsole,
> @@ -155,6 +157,7 @@ dbus_display_console_dispose(GObject *object)
> qemu_input_led_notifier_remove(&ddc->led_notifier);
> qemu_console_unregister_listener(&ddc->dcl);
> qemu_remove_mouse_mode_change_notifier(&ddc->mouse_mode_notifier);
> + g_clear_object(&ddc->iface_ui_info);
> g_clear_object(&ddc->iface_touch);
> g_clear_object(&ddc->iface_mouse);
> g_clear_object(&ddc->iface_kbd);
> @@ -528,6 +531,63 @@ dbus_mouse_mode_change(Notifier *notify, void *data)
> dbus_mouse_update_is_absolute(ddc);
> }
>
> +static gboolean
> +dbus_ui_info_reload(DBusDisplayConsole *ddc,
> + GDBusMethodInvocation *invocation)
> +{
> + if (!qemu_console_ui_info_supported(ddc->dcl.con)) {
> + g_dbus_method_invocation_return_error(invocation,
> + DBUS_DISPLAY_ERROR,
> + DBUS_DISPLAY_ERROR_UNSUPPORTED,
> + "UIInfo is not supported");
> + return DBUS_METHOD_INVOCATION_HANDLED;
> + }
> +
> + QemuUIInfo ui_info = *qemu_console_get_ui_info(ddc->dcl.con);
> +
> + qemu_dbus_display1_uiinfo_set_width_mm(ddc->iface_ui_info,
> + ui_info.width_mm);
> + qemu_dbus_display1_uiinfo_set_height_mm(ddc->iface_ui_info,
> + ui_info.height_mm);
> + qemu_dbus_display1_uiinfo_set_xoff(ddc->iface_ui_info, ui_info.xoff);
> + qemu_dbus_display1_uiinfo_set_yoff(ddc->iface_ui_info, ui_info.yoff);
> + qemu_dbus_display1_uiinfo_set_width(ddc->iface_ui_info, ui_info.width);
> + qemu_dbus_display1_uiinfo_set_height(ddc->iface_ui_info, ui_info.height);
> + qemu_dbus_display1_uiinfo_set_refresh_rate(ddc->iface_ui_info,
> + ui_info.refresh_rate);
> +
> + qemu_dbus_display1_uiinfo_complete_reload(ddc->iface_ui_info, invocation);
> + return DBUS_METHOD_INVOCATION_HANDLED;
> +}
> +
> +static gboolean
> +dbus_ui_info_apply(DBusDisplayConsole *ddc,
> + GDBusMethodInvocation *invocation)
> +{
> + if (!qemu_console_ui_info_supported(ddc->dcl.con)) {
> + g_dbus_method_invocation_return_error(invocation,
> + DBUS_DISPLAY_ERROR,
> + DBUS_DISPLAY_ERROR_UNSUPPORTED,
> + "UIInfo is not supported");
> + return DBUS_METHOD_INVOCATION_HANDLED;
> + }
> +
> + QemuUIInfo ui_info = {
> + .width_mm = qemu_dbus_display1_uiinfo_get_width_mm(ddc->iface_ui_info),
> + .height_mm = qemu_dbus_display1_uiinfo_get_height_mm(ddc->iface_ui_info),
> + .xoff = qemu_dbus_display1_uiinfo_get_xoff(ddc->iface_ui_info),
> + .yoff = qemu_dbus_display1_uiinfo_get_yoff(ddc->iface_ui_info),
> + .width = qemu_dbus_display1_uiinfo_get_width(ddc->iface_ui_info),
> + .height = qemu_dbus_display1_uiinfo_get_height(ddc->iface_ui_info),
> + .refresh_rate = qemu_dbus_display1_uiinfo_get_refresh_rate(ddc->iface_ui_info)
> + };
> +
> + qemu_console_set_ui_info(ddc->dcl.con, &ui_info, false);
> + qemu_dbus_display1_uiinfo_complete_apply(ddc->iface_ui_info, invocation);
> +
> + return DBUS_METHOD_INVOCATION_HANDLED;
> +}
> +
> int dbus_display_console_get_index(DBusDisplayConsole *ddc)
> {
> return qemu_console_get_index(ddc->dcl.con);
> @@ -550,6 +610,7 @@ dbus_display_console_new(DBusDisplay *display, QemuConsole *con)
> "org.qemu.Display1.Keyboard",
> "org.qemu.Display1.Mouse",
> "org.qemu.Display1.MultiTouch",
> + "org.qemu.Display1.UIInfo",
> NULL
> };
>
> @@ -626,5 +687,15 @@ dbus_display_console_new(DBusDisplay *display, QemuConsole *con)
> qemu_add_mouse_mode_change_notifier(&ddc->mouse_mode_notifier);
> dbus_mouse_update_is_absolute(ddc);
>
> + ddc->iface_ui_info = qemu_dbus_display1_uiinfo_skeleton_new();
> + qemu_dbus_display1_uiinfo_set_supported(ddc->iface_ui_info,
> + qemu_console_ui_info_supported(ddc->dcl.con));
> + g_object_connect(ddc->iface_ui_info,
> + "swapped-signal::handle-reload", dbus_ui_info_reload, ddc,
> + "swapped-signal::handle-apply", dbus_ui_info_apply, ddc,
> + NULL);
> + g_dbus_object_skeleton_add_interface(G_DBUS_OBJECT_SKELETON(ddc),
> + G_DBUS_INTERFACE_SKELETON(ddc->iface_ui_info));
> +
> return ddc;
> }
> diff --git a/ui/dbus-display1.xml b/ui/dbus-display1.xml
> index d96bae2ed6..2a8a8563ee 100644
> --- a/ui/dbus-display1.xml
> +++ b/ui/dbus-display1.xml
> @@ -1159,4 +1159,85 @@
> -->
> <property name="Encoding" type="s" access="read"/>
> </interface>
> +
> + <!--
> + org.qemu.Display1.UIInfo:
> +
> + This interface is implemented on
> + ``/org/qemu/Display1/Console_$id`` (see
> + :dbus:iface:`~org.qemu.Display1.Console`).
> + -->
> + <interface name="org.qemu.Display1.UIInfo">
> + <!--
> + Supported:
> +
> + Whether console UI info is supported.
> + -->
> + <property name="Supported" type="b" access="read"/>
> +
> + <!--
> + WidthMm:
> +
> + The physical display width in millimeters.
> + -->
> + <property name="WidthMm" type="q" access="readwrite"/>
> +
> + <!--
> + HeightMm:
> +
> + The physical display height in millimeters.
> + -->
> + <property name="HeightMm" type="q" access="readwrite"/>
> +
> + <!--
> + XOff:
> +
> + The horizontal offset in pixels.
> + -->
> + <property name="XOff" type="i" access="readwrite"/>
> +
> + <!--
> + YOff:
> +
> + The vertical offset in pixels.
> + -->
> + <property name="YOff" type="i" access="readwrite"/>
> +
> + <!--
> + Width:
> +
> + The console width in pixels.
> + -->
> + <property name="Width" type="u" access="readwrite"/>
> +
> + <!--
> + Height:
> +
> + The console height in pixels.
> + -->
> + <property name="Height" type="u" access="readwrite"/>
> +
> + <!--
> + RefreshRate:
> +
> + The display refresh rate in millihertz.
> + -->
> + <property name="RefreshRate" type="u" access="readwrite"/>
> +
> + <!--
> + Reload:
> +
> + Set the staged properties to the state of the console display.
> + -->
> + <method name="Reload">
> + </method>
> +
> + <!--
> + Apply:
> +
> + Apply the staged properties to the console display.
> + -->
> + <method name="Apply">
> + </method>
> + </interface>
> </node>
> --
> 2.55.0
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread