[PATCH v2 00/18] ui: dbus & misc fixes

[PATCH v2 00/18] ui: dbus & misc fixes

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Hi,

This is a respin of a series I sent end of January that didn't get reviews, here
is your chance! :) A collection of fixes for UI related-stuff, and paving the
way for accelerated/egl dbus display support on win32 (for 8.1 hopefully).

I think they are worth to include in 8.0, as little fixes and improvements. It
would be nice to have included as early as rc0.

v2:
- add "ui/dbus: initialize cursor_fb"
- add "ui/shader: fix #version directive must occur on first line"
- add "ui/egl: print EGL error, helping debugging"
- add "ui/sdl: add optional logging when _SDL_DEBUG is set"
- add "ui/sdl: try to instantiate the matching opengl renderer"
- add "ui: introduce egl_init()"
- add "ui/dbus: do not require opengl & gbm"
- add "ui/dbus: restrict opengl to gbm-enabled config"
- drop "ui/dbus: update the display when switching surface", as I
  am not yet confident enough about it
- rebased

Marc-André Lureau (18):
  ui/dbus: initialize cursor_fb
  ui/dbus: unregister clipboard on connection close
  audio/dbus: there are no sender for p2p mode
  ui/dbus: set mouse is-absolute during console creation
  meson: ensure dbus-display generated code is built before other units
  ui: rename cursor_{put->unref}
  ui: rename cursor_{get->ref}, return it
  ui: keep current cursor with QemuConsole
  ui: set cursor upon listener registration
  ui: set cursor position upon listener registration
  ui/sdl: get the GL context from the window
  ui/shader: fix #version directive must occur on first line
  ui/egl: print EGL error, helping debugging
  ui/sdl: add optional logging when _SDL_DEBUG is set
  ui/sdl: try to instantiate the matching opengl renderer
  ui: introduce egl_init()
  ui/dbus: do not require opengl & gbm
  ui/dbus: restrict opengl to gbm-enabled config

 meson.build                      |  2 -
 audio/audio_int.h                |  2 +-
 include/ui/console.h             |  5 +-
 include/ui/egl-helpers.h         |  4 ++
 include/ui/sdl2.h                |  2 +
 ui/vnc.h                         |  1 -
 audio/dbusaudio.c                |  6 ++-
 hw/display/qxl-render.c          |  4 +-
 hw/display/qxl.c                 |  2 +-
 hw/display/vmware_vga.c          |  4 +-
 ui/console.c                     | 18 ++++++++
 ui/cursor.c                      |  5 +-
 ui/dbus-clipboard.c              | 18 +++-----
 ui/dbus-console.c                | 13 ++++--
 ui/dbus-listener.c               | 22 ++++++---
 ui/dbus.c                        | 29 ++++++++----
 ui/egl-headless.c                | 16 +++----
 ui/egl-helpers.c                 | 79 +++++++++++++++++++++++++++++---
 ui/sdl2.c                        | 17 ++++++-
 ui/spice-core.c                  |  7 +--
 ui/spice-display.c               |  8 ++--
 ui/vnc.c                         |  8 +---
 tests/qtest/meson.build          |  2 +-
 ui/meson.build                   |  6 ++-
 ui/shader/texture-blit-flip.vert |  1 -
 ui/shader/texture-blit.frag      |  1 -
 ui/shader/texture-blit.vert      |  1 -
 27 files changed, 203 insertions(+), 80 deletions(-)

-- 
2.39.2

[PATCH v2 01/18] ui/dbus: initialize cursor_fb

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Or else, we may randomly destroy some textures..

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 include/ui/egl-helpers.h | 2 ++
 ui/dbus-listener.c       | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/ui/egl-helpers.h b/include/ui/egl-helpers.h
index 2fb6e0dd6b..c92dd90e33 100644
--- a/include/ui/egl-helpers.h
+++ b/include/ui/egl-helpers.h
@@ -22,6 +22,8 @@ typedef struct egl_fb {
     QemuDmaBuf *dmabuf;
 } egl_fb;
 
+#define EGL_FB_INIT { 0, }
+
 void egl_fb_destroy(egl_fb *fb);
 void egl_fb_setup_default(egl_fb *fb, int width, int height);
 void egl_fb_setup_for_tex(egl_fb *fb, int width, int height,
diff --git a/ui/dbus-listener.c b/ui/dbus-listener.c
index 57d4e401db..2dafd6569c 100644
--- a/ui/dbus-listener.c
+++ b/ui/dbus-listener.c
@@ -149,7 +149,7 @@ static void dbus_cursor_dmabuf(DisplayChangeListener *dcl,
     DBusDisplayListener *ddl = container_of(dcl, DBusDisplayListener, dcl);
     DisplaySurface *ds;
     GVariant *v_data = NULL;
-    egl_fb cursor_fb;
+    egl_fb cursor_fb = EGL_FB_INIT;
 
     if (!dmabuf) {
         qemu_dbus_display1_listener_call_mouse_set(
-- 
2.39.2

Re: [PATCH v2 01/18] ui/dbus: initialize cursor_fb

[Philippe Mathieu-Daudé]

On 7/3/23 12:56, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> Or else, we may randomly destroy some textures..
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>   include/ui/egl-helpers.h | 2 ++
>   ui/dbus-listener.c       | 2 +-
>   2 files changed, 3 insertions(+), 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

[PATCH v2 02/18] ui/dbus: unregister clipboard on connection close

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Fixes unregistration with p2p connections, since they don't have an
associated name owner.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 ui/dbus-clipboard.c | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/ui/dbus-clipboard.c b/ui/dbus-clipboard.c
index df9a754a8d..fe7fcdecb6 100644
--- a/ui/dbus-clipboard.c
+++ b/ui/dbus-clipboard.c
@@ -204,15 +204,6 @@ dbus_clipboard_unregister_proxy(DBusDisplay *dpy)
     g_clear_object(&dpy->clipboard_proxy);
 }
 
-static void
-dbus_on_clipboard_proxy_name_owner_changed(
-    DBusDisplay *dpy,
-    GObject *object,
-    GParamSpec *pspec)
-{
-    dbus_clipboard_unregister_proxy(dpy);
-}
-
 static gboolean
 dbus_clipboard_register(
     DBusDisplay *dpy,
@@ -220,6 +211,7 @@ dbus_clipboard_register(
 {
     g_autoptr(GError) err = NULL;
     const char *name = NULL;
+    GDBusConnection *connection = g_dbus_method_invocation_get_connection(invocation);
 
     if (dpy->clipboard_proxy) {
         g_dbus_method_invocation_return_error(
@@ -232,7 +224,7 @@ dbus_clipboard_register(
 
     dpy->clipboard_proxy =
         qemu_dbus_display1_clipboard_proxy_new_sync(
-            g_dbus_method_invocation_get_connection(invocation),
+            connection,
             G_DBUS_PROXY_FLAGS_DO_NOT_AUTO_START,
             g_dbus_method_invocation_get_sender(invocation),
             "/org/qemu/Display1/Clipboard",
@@ -252,7 +244,11 @@ dbus_clipboard_register(
 
     g_object_connect(dpy->clipboard_proxy,
                      "swapped-signal::notify::g-name-owner",
-                     dbus_on_clipboard_proxy_name_owner_changed, dpy,
+                     dbus_clipboard_unregister_proxy, dpy,
+                     NULL);
+    g_object_connect(connection,
+                     "swapped-signal::closed",
+                     dbus_clipboard_unregister_proxy, dpy,
                      NULL);
     qemu_clipboard_reset_serial();
 
-- 
2.39.2

Re: [PATCH v2 02/18] ui/dbus: unregister clipboard on connection close

[Daniel P. Berrangé]

On Tue, Mar 07, 2023 at 03:56:21PM +0400, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> Fixes unregistration with p2p connections, since they don't have an
> associated name owner.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  ui/dbus-clipboard.c | 18 +++++++-----------
>  1 file changed, 7 insertions(+), 11 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

[PATCH v2 03/18] audio/dbus: there are no sender for p2p mode

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 audio/audio_int.h | 2 +-
 audio/dbusaudio.c | 6 ++++--
 ui/dbus.c         | 2 +-
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/audio/audio_int.h b/audio/audio_int.h
index d51d63f08d..e57ff50155 100644
--- a/audio/audio_int.h
+++ b/audio/audio_int.h
@@ -143,7 +143,7 @@ struct audio_driver {
     void *(*init) (Audiodev *);
     void (*fini) (void *);
 #ifdef CONFIG_GIO
-    void (*set_dbus_server) (AudioState *s, GDBusObjectManagerServer *manager);
+    void (*set_dbus_server) (AudioState *s, GDBusObjectManagerServer *manager, bool p2p);
 #endif
     struct audio_pcm_ops *pcm_ops;
     int can_be_default;
diff --git a/audio/dbusaudio.c b/audio/dbusaudio.c
index 722df0355e..9032dda49c 100644
--- a/audio/dbusaudio.c
+++ b/audio/dbusaudio.c
@@ -43,6 +43,7 @@
 
 typedef struct DBusAudio {
     GDBusObjectManagerServer *server;
+    bool p2p;
     GDBusObjectSkeleton *audio;
     QemuDBusDisplay1Audio *iface;
     GHashTable *out_listeners;
@@ -448,7 +449,7 @@ dbus_audio_register_listener(AudioState *s,
                              bool out)
 {
     DBusAudio *da = s->drv_opaque;
-    const char *sender = g_dbus_method_invocation_get_sender(invocation);
+    const char *sender = da->p2p ? "p2p" : g_dbus_method_invocation_get_sender(invocation);
     g_autoptr(GDBusConnection) listener_conn = NULL;
     g_autoptr(GError) err = NULL;
     g_autoptr(GSocket) socket = NULL;
@@ -591,7 +592,7 @@ dbus_audio_register_in_listener(AudioState *s,
 }
 
 static void
-dbus_audio_set_server(AudioState *s, GDBusObjectManagerServer *server)
+dbus_audio_set_server(AudioState *s, GDBusObjectManagerServer *server, bool p2p)
 {
     DBusAudio *da = s->drv_opaque;
 
@@ -599,6 +600,7 @@ dbus_audio_set_server(AudioState *s, GDBusObjectManagerServer *server)
     g_assert(!da->server);
 
     da->server = g_object_ref(server);
+    da->p2p = p2p;
 
     da->audio = g_dbus_object_skeleton_new(DBUS_DISPLAY1_AUDIO_PATH);
     da->iface = qemu_dbus_display1_audio_skeleton_new();
diff --git a/ui/dbus.c b/ui/dbus.c
index f2dcba03d0..f529928f0b 100644
--- a/ui/dbus.c
+++ b/ui/dbus.c
@@ -220,7 +220,7 @@ dbus_display_complete(UserCreatable *uc, Error **errp)
                        dd->audiodev);
             return;
         }
-        audio_state->drv->set_dbus_server(audio_state, dd->server);
+        audio_state->drv->set_dbus_server(audio_state, dd->server, dd->p2p);
     }
 
     consoles = g_array_new(FALSE, FALSE, sizeof(guint32));
-- 
2.39.2

Re: [PATCH v2 03/18] audio/dbus: there are no sender for p2p mode

[Daniel P. Berrangé]

On Tue, Mar 07, 2023 at 03:56:22PM +0400, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  audio/audio_int.h | 2 +-
>  audio/dbusaudio.c | 6 ++++--
>  ui/dbus.c         | 2 +-
>  3 files changed, 6 insertions(+), 4 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

[PATCH v2 04/18] ui/dbus: set mouse is-absolute during console creation

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 ui/dbus-console.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/ui/dbus-console.c b/ui/dbus-console.c
index 0bfaa2298d..f77bc49d2e 100644
--- a/ui/dbus-console.c
+++ b/ui/dbus-console.c
@@ -411,15 +411,21 @@ dbus_mouse_release(DBusDisplayConsole *ddc,
     return DBUS_METHOD_INVOCATION_HANDLED;
 }
 
+static void
+dbus_mouse_update_is_absolute(DBusDisplayConsole *ddc)
+{
+    g_object_set(ddc->iface_mouse,
+                 "is-absolute", qemu_input_is_absolute(),
+                 NULL);
+}
+
 static void
 dbus_mouse_mode_change(Notifier *notify, void *data)
 {
     DBusDisplayConsole *ddc =
         container_of(notify, DBusDisplayConsole, mouse_mode_notifier);
 
-    g_object_set(ddc->iface_mouse,
-                 "is-absolute", qemu_input_is_absolute(),
-                 NULL);
+    dbus_mouse_update_is_absolute(ddc);
 }
 
 int dbus_display_console_get_index(DBusDisplayConsole *ddc)
@@ -492,6 +498,7 @@ dbus_display_console_new(DBusDisplay *display, QemuConsole *con)
     register_displaychangelistener(&ddc->dcl);
     ddc->mouse_mode_notifier.notify = dbus_mouse_mode_change;
     qemu_add_mouse_mode_change_notifier(&ddc->mouse_mode_notifier);
+    dbus_mouse_update_is_absolute(ddc);
 
     return ddc;
 }
-- 
2.39.2

Re: [PATCH v2 04/18] ui/dbus: set mouse is-absolute during console creation

[Philippe Mathieu-Daudé]

On 7/3/23 12:56, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>   ui/dbus-console.c | 13 ++++++++++---
>   1 file changed, 10 insertions(+), 3 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

[PATCH v2 05/18] meson: ensure dbus-display generated code is built before other units

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

It's simply by luck that dbus-display header is built first before the
other units using it.

With sourceset, I can't find an easier way out than declaring an extra
dependency for dbus-display1 generate code.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 ui/meson.build | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/ui/meson.build b/ui/meson.build
index 612ea2325b..0b2d0d21d1 100644
--- a/ui/meson.build
+++ b/ui/meson.build
@@ -83,7 +83,9 @@ if dbus_display
                                           '--interface-prefix', 'org.qemu.',
                                           '--c-namespace', 'QemuDBus',
                                           '--generate-c-code', '@BASENAME@'])
-  dbus_ss.add(when: [gio, pixman, opengl, gbm],
+  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, opengl, gbm, dbus_display1_dep],
               if_true: [files(
                 'dbus-chardev.c',
                 'dbus-clipboard.c',
@@ -91,7 +93,7 @@ if dbus_display
                 'dbus-error.c',
                 'dbus-listener.c',
                 'dbus.c',
-              ), dbus_display1])
+              )])
   ui_modules += {'dbus' : dbus_ss}
 endif
 
-- 
2.39.2

Re: [PATCH v2 05/18] meson: ensure dbus-display generated code is built before other units

[Daniel P. Berrangé]

On Tue, Mar 07, 2023 at 03:56:24PM +0400, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> It's simply by luck that dbus-display header is built first before the
> other units using it.
> 
> With sourceset, I can't find an easier way out than declaring an extra
> dependency for dbus-display1 generate code.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  ui/meson.build | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

[PATCH v2 06/18] ui: rename cursor_{put->unref}

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

The naming is more conventional in QEMU.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 include/ui/console.h    | 2 +-
 hw/display/qxl-render.c | 4 ++--
 hw/display/qxl.c        | 2 +-
 hw/display/vmware_vga.c | 4 ++--
 ui/cursor.c             | 2 +-
 ui/dbus-listener.c      | 2 +-
 ui/spice-display.c      | 4 ++--
 ui/vnc.c                | 2 +-
 8 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/include/ui/console.h b/include/ui/console.h
index 1cb53acc33..3efd1f6b82 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -152,7 +152,7 @@ typedef struct QEMUCursor {
 
 QEMUCursor *cursor_alloc(int width, int height);
 void cursor_get(QEMUCursor *c);
-void cursor_put(QEMUCursor *c);
+void cursor_unref(QEMUCursor *c);
 QEMUCursor *cursor_builtin_hidden(void);
 QEMUCursor *cursor_builtin_left_ptr(void);
 void cursor_print_ascii_art(QEMUCursor *c, const char *prefix);
diff --git a/hw/display/qxl-render.c b/hw/display/qxl-render.c
index fcfd40c3ac..ec99ec887a 100644
--- a/hw/display/qxl-render.c
+++ b/hw/display/qxl-render.c
@@ -290,7 +290,7 @@ static QEMUCursor *qxl_cursor(PCIQXLDevice *qxl, QXLCursor *cursor,
     return c;
 
 fail:
-    cursor_put(c);
+    cursor_unref(c);
     return NULL;
 }
 
@@ -336,7 +336,7 @@ int qxl_render_cursor(PCIQXLDevice *qxl, QXLCommandExt *ext)
         }
         qemu_mutex_lock(&qxl->ssd.lock);
         if (qxl->ssd.cursor) {
-            cursor_put(qxl->ssd.cursor);
+            cursor_unref(qxl->ssd.cursor);
         }
         qxl->ssd.cursor = c;
         qxl->ssd.mouse_x = cmd->u.set.position.x;
diff --git a/hw/display/qxl.c b/hw/display/qxl.c
index ec712d3ca2..80ce1e9a93 100644
--- a/hw/display/qxl.c
+++ b/hw/display/qxl.c
@@ -299,7 +299,7 @@ void qxl_spice_reset_cursor(PCIQXLDevice *qxl)
     qxl->guest_cursor = 0;
     qemu_mutex_unlock(&qxl->track_lock);
     if (qxl->ssd.cursor) {
-        cursor_put(qxl->ssd.cursor);
+        cursor_unref(qxl->ssd.cursor);
     }
     qxl->ssd.cursor = cursor_builtin_hidden();
 }
diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c
index 59ae7f74b8..09591fbd39 100644
--- a/hw/display/vmware_vga.c
+++ b/hw/display/vmware_vga.c
@@ -550,12 +550,12 @@ static inline void vmsvga_cursor_define(struct vmsvga_state_s *s,
     default:
         fprintf(stderr, "%s: unhandled bpp %d, using fallback cursor\n",
                 __func__, c->bpp);
-        cursor_put(qc);
+        cursor_unref(qc);
         qc = cursor_builtin_left_ptr();
     }
 
     dpy_cursor_define(s->vga.con, qc);
-    cursor_put(qc);
+    cursor_unref(qc);
 }
 #endif
 
diff --git a/ui/cursor.c b/ui/cursor.c
index 835f0802f9..31b09bf058 100644
--- a/ui/cursor.c
+++ b/ui/cursor.c
@@ -111,7 +111,7 @@ void cursor_get(QEMUCursor *c)
     c->refcount++;
 }
 
-void cursor_put(QEMUCursor *c)
+void cursor_unref(QEMUCursor *c)
 {
     if (c == NULL)
         return;
diff --git a/ui/dbus-listener.c b/ui/dbus-listener.c
index 2dafd6569c..906ab296ca 100644
--- a/ui/dbus-listener.c
+++ b/ui/dbus-listener.c
@@ -345,7 +345,7 @@ static void dbus_cursor_define(DisplayChangeListener *dcl,
         c->data,
         c->width * c->height * 4,
         TRUE,
-        (GDestroyNotify)cursor_put,
+        (GDestroyNotify)cursor_unref,
         c);
 
     qemu_dbus_display1_listener_call_cursor_define(
diff --git a/ui/spice-display.c b/ui/spice-display.c
index 16802f99cb..243a794638 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -464,7 +464,7 @@ void qemu_spice_cursor_refresh_bh(void *opaque)
         qemu_mutex_unlock(&ssd->lock);
         dpy_cursor_define(ssd->dcl.con, c);
         qemu_mutex_lock(&ssd->lock);
-        cursor_put(c);
+        cursor_unref(c);
     }
 
     if (ssd->mouse_x != -1 && ssd->mouse_y != -1) {
@@ -766,7 +766,7 @@ static void display_mouse_define(DisplayChangeListener *dcl,
 
     qemu_mutex_lock(&ssd->lock);
     cursor_get(c);
-    cursor_put(ssd->cursor);
+    cursor_unref(ssd->cursor);
     ssd->cursor = c;
     ssd->hot_x = c->hot_x;
     ssd->hot_y = c->hot_y;
diff --git a/ui/vnc.c b/ui/vnc.c
index d9eacad759..0bdcc3dfce 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -1029,7 +1029,7 @@ static void vnc_dpy_cursor_define(DisplayChangeListener *dcl,
     VncDisplay *vd = container_of(dcl, VncDisplay, dcl);
     VncState *vs;
 
-    cursor_put(vd->cursor);
+    cursor_unref(vd->cursor);
     g_free(vd->cursor_mask);
 
     vd->cursor = c;
-- 
2.39.2

Re: [PATCH v2 06/18] ui: rename cursor_{put->unref}

[Daniel P. Berrangé]

On Tue, Mar 07, 2023 at 03:56:25PM +0400, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> The naming is more conventional in QEMU.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  include/ui/console.h    | 2 +-
>  hw/display/qxl-render.c | 4 ++--
>  hw/display/qxl.c        | 2 +-
>  hw/display/vmware_vga.c | 4 ++--
>  ui/cursor.c             | 2 +-
>  ui/dbus-listener.c      | 2 +-
>  ui/spice-display.c      | 4 ++--
>  ui/vnc.c                | 2 +-
>  8 files changed, 11 insertions(+), 11 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

[PATCH v2 07/18] ui: rename cursor_{get->ref}, return it

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

The naming is more conventional in QEMU code, and allows to simplify
some code.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 include/ui/console.h | 2 +-
 ui/cursor.c          | 3 ++-
 ui/dbus-listener.c   | 3 +--
 ui/spice-display.c   | 4 ++--
 ui/vnc.c             | 3 +--
 5 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/include/ui/console.h b/include/ui/console.h
index 3efd1f6b82..0b01df947e 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -151,7 +151,7 @@ typedef struct QEMUCursor {
 } QEMUCursor;
 
 QEMUCursor *cursor_alloc(int width, int height);
-void cursor_get(QEMUCursor *c);
+QEMUCursor *cursor_ref(QEMUCursor *c);
 void cursor_unref(QEMUCursor *c);
 QEMUCursor *cursor_builtin_hidden(void);
 QEMUCursor *cursor_builtin_left_ptr(void);
diff --git a/ui/cursor.c b/ui/cursor.c
index 31b09bf058..6fe67990e2 100644
--- a/ui/cursor.c
+++ b/ui/cursor.c
@@ -106,9 +106,10 @@ QEMUCursor *cursor_alloc(int width, int height)
     return c;
 }
 
-void cursor_get(QEMUCursor *c)
+QEMUCursor *cursor_ref(QEMUCursor *c)
 {
     c->refcount++;
+    return c;
 }
 
 void cursor_unref(QEMUCursor *c)
diff --git a/ui/dbus-listener.c b/ui/dbus-listener.c
index 906ab296ca..2e87ed7e9a 100644
--- a/ui/dbus-listener.c
+++ b/ui/dbus-listener.c
@@ -339,14 +339,13 @@ static void dbus_cursor_define(DisplayChangeListener *dcl,
     DBusDisplayListener *ddl = container_of(dcl, DBusDisplayListener, dcl);
     GVariant *v_data = NULL;
 
-    cursor_get(c);
     v_data = g_variant_new_from_data(
         G_VARIANT_TYPE("ay"),
         c->data,
         c->width * c->height * 4,
         TRUE,
         (GDestroyNotify)cursor_unref,
-        c);
+        cursor_ref(c));
 
     qemu_dbus_display1_listener_call_cursor_define(
         ddl->proxy,
diff --git a/ui/spice-display.c b/ui/spice-display.c
index 243a794638..5bee19a7f9 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -460,7 +460,7 @@ void qemu_spice_cursor_refresh_bh(void *opaque)
     if (ssd->cursor) {
         QEMUCursor *c = ssd->cursor;
         assert(ssd->dcl.con);
-        cursor_get(c);
+        cursor_ref(c);
         qemu_mutex_unlock(&ssd->lock);
         dpy_cursor_define(ssd->dcl.con, c);
         qemu_mutex_lock(&ssd->lock);
@@ -765,7 +765,7 @@ static void display_mouse_define(DisplayChangeListener *dcl,
     SimpleSpiceDisplay *ssd = container_of(dcl, SimpleSpiceDisplay, dcl);
 
     qemu_mutex_lock(&ssd->lock);
-    cursor_get(c);
+    cursor_ref(c);
     cursor_unref(ssd->cursor);
     ssd->cursor = c;
     ssd->hot_x = c->hot_x;
diff --git a/ui/vnc.c b/ui/vnc.c
index 0bdcc3dfce..8aec5d751e 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -1032,8 +1032,7 @@ static void vnc_dpy_cursor_define(DisplayChangeListener *dcl,
     cursor_unref(vd->cursor);
     g_free(vd->cursor_mask);
 
-    vd->cursor = c;
-    cursor_get(vd->cursor);
+    vd->cursor = cursor_ref(vd->cursor);
     vd->cursor_msize = cursor_get_mono_bpl(c) * c->height;
     vd->cursor_mask = g_malloc0(vd->cursor_msize);
     cursor_get_mono_mask(c, 0, vd->cursor_mask);
-- 
2.39.2

Re: [PATCH v2 07/18] ui: rename cursor_{get->ref}, return it

[Daniel P. Berrangé]

On Tue, Mar 07, 2023 at 03:56:26PM +0400, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> The naming is more conventional in QEMU code, and allows to simplify
> some code.

Nit-pick, this commit also changes the API design, so it
returns the input parameter, instead of void. This makes
sense as it simplifies usage, but please mentioned it in
the commit as this is more than just a rename commit.

> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  include/ui/console.h | 2 +-
>  ui/cursor.c          | 3 ++-
>  ui/dbus-listener.c   | 3 +--
>  ui/spice-display.c   | 4 ++--
>  ui/vnc.c             | 3 +--
>  5 files changed, 7 insertions(+), 8 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


> 
> diff --git a/include/ui/console.h b/include/ui/console.h
> index 3efd1f6b82..0b01df947e 100644
> --- a/include/ui/console.h
> +++ b/include/ui/console.h
> @@ -151,7 +151,7 @@ typedef struct QEMUCursor {
>  } QEMUCursor;
>  
>  QEMUCursor *cursor_alloc(int width, int height);
> -void cursor_get(QEMUCursor *c);
> +QEMUCursor *cursor_ref(QEMUCursor *c);
>  void cursor_unref(QEMUCursor *c);
>  QEMUCursor *cursor_builtin_hidden(void);
>  QEMUCursor *cursor_builtin_left_ptr(void);
> diff --git a/ui/cursor.c b/ui/cursor.c
> index 31b09bf058..6fe67990e2 100644
> --- a/ui/cursor.c
> +++ b/ui/cursor.c
> @@ -106,9 +106,10 @@ QEMUCursor *cursor_alloc(int width, int height)
>      return c;
>  }
>  
> -void cursor_get(QEMUCursor *c)
> +QEMUCursor *cursor_ref(QEMUCursor *c)
>  {
>      c->refcount++;
> +    return c;
>  }
>  
>  void cursor_unref(QEMUCursor *c)
> diff --git a/ui/dbus-listener.c b/ui/dbus-listener.c
> index 906ab296ca..2e87ed7e9a 100644
> --- a/ui/dbus-listener.c
> +++ b/ui/dbus-listener.c
> @@ -339,14 +339,13 @@ static void dbus_cursor_define(DisplayChangeListener *dcl,
>      DBusDisplayListener *ddl = container_of(dcl, DBusDisplayListener, dcl);
>      GVariant *v_data = NULL;
>  
> -    cursor_get(c);
>      v_data = g_variant_new_from_data(
>          G_VARIANT_TYPE("ay"),
>          c->data,
>          c->width * c->height * 4,
>          TRUE,
>          (GDestroyNotify)cursor_unref,
> -        c);
> +        cursor_ref(c));
>  
>      qemu_dbus_display1_listener_call_cursor_define(
>          ddl->proxy,
> diff --git a/ui/spice-display.c b/ui/spice-display.c
> index 243a794638..5bee19a7f9 100644
> --- a/ui/spice-display.c
> +++ b/ui/spice-display.c
> @@ -460,7 +460,7 @@ void qemu_spice_cursor_refresh_bh(void *opaque)
>      if (ssd->cursor) {
>          QEMUCursor *c = ssd->cursor;
>          assert(ssd->dcl.con);
> -        cursor_get(c);
> +        cursor_ref(c);
>          qemu_mutex_unlock(&ssd->lock);
>          dpy_cursor_define(ssd->dcl.con, c);
>          qemu_mutex_lock(&ssd->lock);
> @@ -765,7 +765,7 @@ static void display_mouse_define(DisplayChangeListener *dcl,
>      SimpleSpiceDisplay *ssd = container_of(dcl, SimpleSpiceDisplay, dcl);
>  
>      qemu_mutex_lock(&ssd->lock);
> -    cursor_get(c);
> +    cursor_ref(c);
>      cursor_unref(ssd->cursor);
>      ssd->cursor = c;
>      ssd->hot_x = c->hot_x;
> diff --git a/ui/vnc.c b/ui/vnc.c
> index 0bdcc3dfce..8aec5d751e 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -1032,8 +1032,7 @@ static void vnc_dpy_cursor_define(DisplayChangeListener *dcl,
>      cursor_unref(vd->cursor);
>      g_free(vd->cursor_mask);
>  
> -    vd->cursor = c;
> -    cursor_get(vd->cursor);
> +    vd->cursor = cursor_ref(vd->cursor);
>      vd->cursor_msize = cursor_get_mono_bpl(c) * c->height;
>      vd->cursor_mask = g_malloc0(vd->cursor_msize);
>      cursor_get_mono_mask(c, 0, vd->cursor_mask);
> -- 
> 2.39.2
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

[PATCH v2 08/18] ui: keep current cursor with QemuConsole

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Keeping the current cursor around is useful, not only for VNC, but for
other displays. Let's move it down, see the following patches for other
usages.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 include/ui/console.h | 1 +
 ui/vnc.h             | 1 -
 ui/console.c         | 8 ++++++++
 ui/vnc.c             | 7 ++-----
 4 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/include/ui/console.h b/include/ui/console.h
index 0b01df947e..2a8fab091f 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -459,6 +459,7 @@ QemuConsole *qemu_console_lookup_by_device(DeviceState *dev, uint32_t head);
 QemuConsole *qemu_console_lookup_by_device_name(const char *device_id,
                                                 uint32_t head, Error **errp);
 QemuConsole *qemu_console_lookup_unused(void);
+QEMUCursor *qemu_console_get_cursor(QemuConsole *con);
 bool qemu_console_is_visible(QemuConsole *con);
 bool qemu_console_is_graphic(QemuConsole *con);
 bool qemu_console_is_fixedsize(QemuConsole *con);
diff --git a/ui/vnc.h b/ui/vnc.h
index a60fb13115..757fa83044 100644
--- a/ui/vnc.h
+++ b/ui/vnc.h
@@ -159,7 +159,6 @@ struct VncDisplay
     QKbdState *kbd;
     QemuMutex mutex;
 
-    QEMUCursor *cursor;
     int cursor_msize;
     uint8_t *cursor_mask;
 
diff --git a/ui/console.c b/ui/console.c
index 98b701f5a3..0dccbdd4be 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -94,6 +94,7 @@ struct QemuConsole {
     uint32_t head;
     QemuUIInfo ui_info;
     QEMUTimer *ui_timer;
+    QEMUCursor *cursor;
     const GraphicHwOps *hw_ops;
     void *hw;
 
@@ -1923,6 +1924,8 @@ void dpy_cursor_define(QemuConsole *con, QEMUCursor *cursor)
     DisplayState *s = con->ds;
     DisplayChangeListener *dcl;
 
+    cursor_unref(con->cursor);
+    con->cursor = cursor_ref(cursor);
     if (!qemu_console_is_visible(con)) {
         return;
     }
@@ -2288,6 +2291,11 @@ QemuConsole *qemu_console_lookup_unused(void)
     return NULL;
 }
 
+QEMUCursor *qemu_console_get_cursor(QemuConsole *con)
+{
+    return con->cursor;
+}
+
 bool qemu_console_is_visible(QemuConsole *con)
 {
     return (con == active_console) || (con->dcls > 0);
diff --git a/ui/vnc.c b/ui/vnc.c
index 8aec5d751e..bbd8b6baae 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -988,10 +988,10 @@ static void vnc_mouse_set(DisplayChangeListener *dcl,
 
 static int vnc_cursor_define(VncState *vs)
 {
-    QEMUCursor *c = vs->vd->cursor;
+    QEMUCursor *c = qemu_console_get_cursor(vs->vd->dcl.con);
     int isize;
 
-    if (!vs->vd->cursor) {
+    if (!c) {
         return -1;
     }
 
@@ -1029,10 +1029,7 @@ static void vnc_dpy_cursor_define(DisplayChangeListener *dcl,
     VncDisplay *vd = container_of(dcl, VncDisplay, dcl);
     VncState *vs;
 
-    cursor_unref(vd->cursor);
     g_free(vd->cursor_mask);
-
-    vd->cursor = cursor_ref(vd->cursor);
     vd->cursor_msize = cursor_get_mono_bpl(c) * c->height;
     vd->cursor_mask = g_malloc0(vd->cursor_msize);
     cursor_get_mono_mask(c, 0, vd->cursor_mask);
-- 
2.39.2

Re: [PATCH v2 08/18] ui: keep current cursor with QemuConsole

[Daniel P. Berrangé]

On Tue, Mar 07, 2023 at 03:56:27PM +0400, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> Keeping the current cursor around is useful, not only for VNC, but for
> other displays. Let's move it down, see the following patches for other
> usages.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  include/ui/console.h | 1 +
>  ui/vnc.h             | 1 -
>  ui/console.c         | 8 ++++++++
>  ui/vnc.c             | 7 ++-----
>  4 files changed, 11 insertions(+), 6 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

[PATCH v2 09/18] ui: set cursor upon listener registration

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 ui/console.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/ui/console.c b/ui/console.c
index 0dccbdd4be..7b808f080e 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -1662,6 +1662,9 @@ void register_displaychangelistener(DisplayChangeListener *dcl)
         con = active_console;
     }
     displaychangelistener_display_console(dcl, con, dcl->con ? &error_fatal : NULL);
+    if (con->cursor && dcl->ops->dpy_cursor_define) {
+        dcl->ops->dpy_cursor_define(dcl, con->cursor);
+    }
     text_console_update_cursor(NULL);
 }
 
-- 
2.39.2

Re: [PATCH v2 09/18] ui: set cursor upon listener registration

[Daniel P. Berrangé]

On Tue, Mar 07, 2023 at 03:56:28PM +0400, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  ui/console.c | 3 +++
>  1 file changed, 3 insertions(+)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

[PATCH v2 10/18] ui: set cursor position upon listener registration

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 ui/console.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/ui/console.c b/ui/console.c
index 7b808f080e..8fc18c44bf 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -95,6 +95,7 @@ struct QemuConsole {
     QemuUIInfo ui_info;
     QEMUTimer *ui_timer;
     QEMUCursor *cursor;
+    int cursor_x, cursor_y, cursor_on;
     const GraphicHwOps *hw_ops;
     void *hw;
 
@@ -1665,6 +1666,9 @@ void register_displaychangelistener(DisplayChangeListener *dcl)
     if (con->cursor && dcl->ops->dpy_cursor_define) {
         dcl->ops->dpy_cursor_define(dcl, con->cursor);
     }
+    if (dcl->ops->dpy_mouse_set) {
+        dcl->ops->dpy_mouse_set(dcl, con->cursor_x, con->cursor_y, con->cursor_on);
+    }
     text_console_update_cursor(NULL);
 }
 
@@ -1909,6 +1913,9 @@ void dpy_mouse_set(QemuConsole *con, int x, int y, int on)
     DisplayState *s = con->ds;
     DisplayChangeListener *dcl;
 
+    con->cursor_x = x;
+    con->cursor_y = y;
+    con->cursor_on = on;
     if (!qemu_console_is_visible(con)) {
         return;
     }
-- 
2.39.2

Re: [PATCH v2 10/18] ui: set cursor position upon listener registration

[Daniel P. Berrangé]

On Tue, Mar 07, 2023 at 03:56:29PM +0400, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  ui/console.c | 7 +++++++
>  1 file changed, 7 insertions(+)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

[PATCH v2 11/18] ui/sdl: get the GL context from the window

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

There is no guarantee to have a current GL context here. The current
code seems to rely on the renderer using a GL backend, and to set a
current GL context. But this is not always the case, for example if the
renderer backend is DirectX.

This change is enough to fix using virgl with sdl2 on win32, on my setup.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 ui/sdl2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ui/sdl2.c b/ui/sdl2.c
index 8cb77416af..f259e4c4d1 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -101,7 +101,7 @@ void sdl2_window_create(struct sdl2_console *scon)
                                          flags);
     scon->real_renderer = SDL_CreateRenderer(scon->real_window, -1, 0);
     if (scon->opengl) {
-        scon->winctx = SDL_GL_GetCurrentContext();
+        scon->winctx = SDL_GL_CreateContext(scon->real_window);
     }
     sdl_update_caption(scon);
 }
-- 
2.39.2

Re: [PATCH v2 11/18] ui/sdl: get the GL context from the window

[Daniel P. Berrangé]

On Tue, Mar 07, 2023 at 03:56:30PM +0400, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> There is no guarantee to have a current GL context here. The current
> code seems to rely on the renderer using a GL backend, and to set a
> current GL context. But this is not always the case, for example if the
> renderer backend is DirectX.
> 
> This change is enough to fix using virgl with sdl2 on win32, on my setup.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  ui/sdl2.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/ui/sdl2.c b/ui/sdl2.c
> index 8cb77416af..f259e4c4d1 100644
> --- a/ui/sdl2.c
> +++ b/ui/sdl2.c
> @@ -101,7 +101,7 @@ void sdl2_window_create(struct sdl2_console *scon)
>                                           flags);
>      scon->real_renderer = SDL_CreateRenderer(scon->real_window, -1, 0);
>      if (scon->opengl) {
> -        scon->winctx = SDL_GL_GetCurrentContext();
> +        scon->winctx = SDL_GL_CreateContext(scon->real_window);

IIUC, previously this borrowed a pointer to an existing context
and so didn't need to free it. Now this is creating a new
dedicated context, something must free this in sdl2_window_destroy.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

[PATCH v2 12/18] ui/shader: fix #version directive must occur on first line

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

ANGLE fails to compile shaders otherwise.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 ui/shader/texture-blit-flip.vert | 1 -
 ui/shader/texture-blit.frag      | 1 -
 ui/shader/texture-blit.vert      | 1 -
 3 files changed, 3 deletions(-)

diff --git a/ui/shader/texture-blit-flip.vert b/ui/shader/texture-blit-flip.vert
index ba081fa5a6..f7a448d229 100644
--- a/ui/shader/texture-blit-flip.vert
+++ b/ui/shader/texture-blit-flip.vert
@@ -1,4 +1,3 @@
-
 #version 300 es
 
 in vec2  in_position;
diff --git a/ui/shader/texture-blit.frag b/ui/shader/texture-blit.frag
index bfa202c22b..8ed95a46b6 100644
--- a/ui/shader/texture-blit.frag
+++ b/ui/shader/texture-blit.frag
@@ -1,4 +1,3 @@
-
 #version 300 es
 
 uniform sampler2D image;
diff --git a/ui/shader/texture-blit.vert b/ui/shader/texture-blit.vert
index 6fe2744d68..fb48d70665 100644
--- a/ui/shader/texture-blit.vert
+++ b/ui/shader/texture-blit.vert
@@ -1,4 +1,3 @@
-
 #version 300 es
 
 in vec2  in_position;
-- 
2.39.2

Re: [PATCH v2 12/18] ui/shader: fix #version directive must occur on first line

[Daniel P. Berrangé]

On Tue, Mar 07, 2023 at 03:56:31PM +0400, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> ANGLE fails to compile shaders otherwise.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  ui/shader/texture-blit-flip.vert | 1 -
>  ui/shader/texture-blit.frag      | 1 -
>  ui/shader/texture-blit.vert      | 1 -
>  3 files changed, 3 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

[PATCH v2 13/18] ui/egl: print EGL error, helping debugging

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 ui/egl-helpers.c | 54 ++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 48 insertions(+), 6 deletions(-)

diff --git a/ui/egl-helpers.c b/ui/egl-helpers.c
index 3a88245b67..10772b6471 100644
--- a/ui/egl-helpers.c
+++ b/ui/egl-helpers.c
@@ -26,6 +26,48 @@ DisplayGLMode qemu_egl_mode;
 
 /* ------------------------------------------------------------------ */
 
+#if defined(CONFIG_X11) || defined(CONFIG_GBM)
+static const char *egl_get_error_string(void)
+{
+    EGLint error = eglGetError();
+
+    switch (error) {
+        case EGL_SUCCESS:
+            return "EGL_SUCCESS";
+        case EGL_NOT_INITIALIZED:
+            return "EGL_NOT_INITIALIZED";
+        case EGL_BAD_ACCESS:
+            return "EGL_BAD_ACCESS";
+        case EGL_BAD_ALLOC:
+            return "EGL_BAD_ALLOC";
+        case EGL_BAD_ATTRIBUTE:
+            return "EGL_BAD_ATTRIBUTE";
+        case EGL_BAD_CONTEXT:
+            return "EGL_BAD_CONTEXT";
+        case EGL_BAD_CONFIG:
+            return "EGL_BAD_CONFIG";
+        case EGL_BAD_CURRENT_SURFACE:
+            return "EGL_BAD_CURRENT_SURFACE";
+        case EGL_BAD_DISPLAY:
+            return "EGL_BAD_DISPLAY";
+        case EGL_BAD_SURFACE:
+            return "EGL_BAD_SURFACE";
+        case EGL_BAD_MATCH:
+            return "EGL_BAD_MATCH";
+        case EGL_BAD_PARAMETER:
+            return "EGL_BAD_PARAMETER";
+        case EGL_BAD_NATIVE_PIXMAP:
+            return "EGL_BAD_NATIVE_PIXMAP";
+        case EGL_BAD_NATIVE_WINDOW:
+            return "EGL_BAD_NATIVE_WINDOW";
+        case EGL_CONTEXT_LOST:
+            return "EGL_CONTEXT_LOST";
+        default:
+            return "Unknown EGL error";
+    }
+}
+#endif
+
 static void egl_fb_delete_texture(egl_fb *fb)
 {
     if (!fb->delete_texture) {
@@ -438,20 +480,20 @@ static int qemu_egl_init_dpy(EGLNativeDisplayType dpy,
 
     qemu_egl_display = qemu_egl_get_display(dpy, platform);
     if (qemu_egl_display == EGL_NO_DISPLAY) {
-        error_report("egl: eglGetDisplay failed");
+        error_report("egl: eglGetDisplay failed: %s", egl_get_error_string());
         return -1;
     }
 
     b = eglInitialize(qemu_egl_display, &major, &minor);
     if (b == EGL_FALSE) {
-        error_report("egl: eglInitialize failed");
+        error_report("egl: eglInitialize failed: %s", egl_get_error_string());
         return -1;
     }
 
     b = eglBindAPI(gles ?  EGL_OPENGL_ES_API : EGL_OPENGL_API);
     if (b == EGL_FALSE) {
-        error_report("egl: eglBindAPI failed (%s mode)",
-                     gles ? "gles" : "core");
+        error_report("egl: eglBindAPI failed (%s mode): %s",
+                     gles ? "gles" : "core", egl_get_error_string());
         return -1;
     }
 
@@ -459,8 +501,8 @@ static int qemu_egl_init_dpy(EGLNativeDisplayType dpy,
                         gles ? conf_att_gles : conf_att_core,
                         &qemu_egl_config, 1, &n);
     if (b == EGL_FALSE || n != 1) {
-        error_report("egl: eglChooseConfig failed (%s mode)",
-                     gles ? "gles" : "core");
+        error_report("egl: eglChooseConfig failed (%s mode): %s",
+                     gles ? "gles" : "core", egl_get_error_string());
         return -1;
     }
 
-- 
2.39.2

Re: [PATCH v2 13/18] ui/egl: print EGL error, helping debugging

[Philippe Mathieu-Daudé]

On 7/3/23 12:56, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>   ui/egl-helpers.c | 54 ++++++++++++++++++++++++++++++++++++++++++------
>   1 file changed, 48 insertions(+), 6 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

[PATCH v2 14/18] ui/sdl: add optional logging when _SDL_DEBUG is set

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Apparently, there is no environment variable you can set for libsdl to
enable logging.

(similar to _VNC_DEBUG)

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 include/ui/sdl2.h | 2 ++
 ui/sdl2.c         | 4 ++++
 2 files changed, 6 insertions(+)

diff --git a/include/ui/sdl2.h b/include/ui/sdl2.h
index 8fb7e08262..6fea36db82 100644
--- a/include/ui/sdl2.h
+++ b/include/ui/sdl2.h
@@ -6,6 +6,8 @@
 
 #include <SDL.h>
 
+/* #define _SDL_DEBUG 1 */
+
 /* with Alpine / muslc SDL headers pull in directfb headers
  * which in turn trigger warning about redundant decls for
  * direct_waitqueue_deinit.
diff --git a/ui/sdl2.c b/ui/sdl2.c
index f259e4c4d1..592eca3e1c 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -841,6 +841,10 @@ static void sdl2_display_init(DisplayState *ds, DisplayOptions *o)
     }
 #endif
 
+#ifdef _SDL_DEBUG
+    SDL_LogSetAllPriority(SDL_LOG_PRIORITY_VERBOSE);
+#endif
+
     if (SDL_Init(SDL_INIT_VIDEO)) {
         fprintf(stderr, "Could not initialize SDL(%s) - exiting\n",
                 SDL_GetError());
-- 
2.39.2

Re: [PATCH v2 14/18] ui/sdl: add optional logging when _SDL_DEBUG is set

[Philippe Mathieu-Daudé]

On 7/3/23 12:56, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> Apparently, there is no environment variable you can set for libsdl to
> enable logging.

Why not use getenv() in QEMU then?

> (similar to _VNC_DEBUG)
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>   include/ui/sdl2.h | 2 ++
>   ui/sdl2.c         | 4 ++++
>   2 files changed, 6 insertions(+)
> 
> diff --git a/include/ui/sdl2.h b/include/ui/sdl2.h
> index 8fb7e08262..6fea36db82 100644
> --- a/include/ui/sdl2.h
> +++ b/include/ui/sdl2.h
> @@ -6,6 +6,8 @@
>   
>   #include <SDL.h>
>   
> +/* #define _SDL_DEBUG 1 */
> +
>   /* with Alpine / muslc SDL headers pull in directfb headers
>    * which in turn trigger warning about redundant decls for
>    * direct_waitqueue_deinit.
> diff --git a/ui/sdl2.c b/ui/sdl2.c
> index f259e4c4d1..592eca3e1c 100644
> --- a/ui/sdl2.c
> +++ b/ui/sdl2.c
> @@ -841,6 +841,10 @@ static void sdl2_display_init(DisplayState *ds, DisplayOptions *o)
>       }
>   #endif
>   
> +#ifdef _SDL_DEBUG
> +    SDL_LogSetAllPriority(SDL_LOG_PRIORITY_VERBOSE);
> +#endif
> +
>       if (SDL_Init(SDL_INIT_VIDEO)) {
>           fprintf(stderr, "Could not initialize SDL(%s) - exiting\n",
>                   SDL_GetError());

Re: [PATCH v2 14/18] ui/sdl: add optional logging when _SDL_DEBUG is set

[Marc-André Lureau]

[-- Attachment #1: Type: text/plain, Size: 1917 bytes --]

Hi

(adding Sam Lantinga, SDL maintainer, in CC. I noticed he worked on QEMU
SDL backend yesterday! ;)

On Thu, Mar 9, 2023 at 8:40 PM Philippe Mathieu-Daudé <philmd@linaro.org>
wrote:

> On 7/3/23 12:56, marcandre.lureau@redhat.com wrote:
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > Apparently, there is no environment variable you can set for libsdl to
> > enable logging.
>
> Why not use getenv() in QEMU then?
>

QEMU has few environment variables. We generally prefer qmp/cmd arguments.
And that change does not fit with QEMU tracing etc.



> > (similar to _VNC_DEBUG)
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >   include/ui/sdl2.h | 2 ++
> >   ui/sdl2.c         | 4 ++++
> >   2 files changed, 6 insertions(+)
> >
> > diff --git a/include/ui/sdl2.h b/include/ui/sdl2.h
> > index 8fb7e08262..6fea36db82 100644
> > --- a/include/ui/sdl2.h
> > +++ b/include/ui/sdl2.h
> > @@ -6,6 +6,8 @@
> >
> >   #include <SDL.h>
> >
> > +/* #define _SDL_DEBUG 1 */
> > +
> >   /* with Alpine / muslc SDL headers pull in directfb headers
> >    * which in turn trigger warning about redundant decls for
> >    * direct_waitqueue_deinit.
> > diff --git a/ui/sdl2.c b/ui/sdl2.c
> > index f259e4c4d1..592eca3e1c 100644
> > --- a/ui/sdl2.c
> > +++ b/ui/sdl2.c
> > @@ -841,6 +841,10 @@ static void sdl2_display_init(DisplayState *ds,
> DisplayOptions *o)
> >       }
> >   #endif
> >
> > +#ifdef _SDL_DEBUG
> > +    SDL_LogSetAllPriority(SDL_LOG_PRIORITY_VERBOSE);
> > +#endif
>

Sam, do you suggest a better way to enable SDL debugging when starting
QEMU? Is there a way to enable it with existing SDL environment variables?

thanks

> +
> >       if (SDL_Init(SDL_INIT_VIDEO)) {
> >           fprintf(stderr, "Could not initialize SDL(%s) - exiting\n",
> >                   SDL_GetError());
>
>

[-- Attachment #2: Type: text/html, Size: 3111 bytes --]

Re: [PATCH v2 14/18] ui/sdl: add optional logging when _SDL_DEBUG is set

[Sam Lantinga]

[-- Attachment #1: Type: text/plain, Size: 2508 bytes --]

If you wanted, you could use the SDL hint system, e.g.

if (SDL_GetHintBoolean("QEMU_ENABLE_SDL_LOGGING", SDL_FALSE)) {
    SDL_LogSetAllPriority(SDL_LOG_PRIORITY_VERBOSE);
}

Then someone can set that environment variable and it would be using the
SDL mechanisms rather than intruding on QEMU... or, if you want tighter
integration, you can set an SDL log handler that feeds into the QEMU
tracing system.

On Thu, Mar 9, 2023 at 9:18 PM Marc-André Lureau <
marcandre.lureau@redhat.com> wrote:

> Hi
>
> (adding Sam Lantinga, SDL maintainer, in CC. I noticed he worked on QEMU
> SDL backend yesterday! ;)
>
> On Thu, Mar 9, 2023 at 8:40 PM Philippe Mathieu-Daudé <philmd@linaro.org>
> wrote:
>
>> On 7/3/23 12:56, marcandre.lureau@redhat.com wrote:
>> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
>> >
>> > Apparently, there is no environment variable you can set for libsdl to
>> > enable logging.
>>
>> Why not use getenv() in QEMU then?
>>
>
> QEMU has few environment variables. We generally prefer qmp/cmd arguments.
> And that change does not fit with QEMU tracing etc.
>
>
>
>> > (similar to _VNC_DEBUG)
>> >
>> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> > ---
>> >   include/ui/sdl2.h | 2 ++
>> >   ui/sdl2.c         | 4 ++++
>> >   2 files changed, 6 insertions(+)
>> >
>> > diff --git a/include/ui/sdl2.h b/include/ui/sdl2.h
>> > index 8fb7e08262..6fea36db82 100644
>> > --- a/include/ui/sdl2.h
>> > +++ b/include/ui/sdl2.h
>> > @@ -6,6 +6,8 @@
>> >
>> >   #include <SDL.h>
>> >
>> > +/* #define _SDL_DEBUG 1 */
>> > +
>> >   /* with Alpine / muslc SDL headers pull in directfb headers
>> >    * which in turn trigger warning about redundant decls for
>> >    * direct_waitqueue_deinit.
>> > diff --git a/ui/sdl2.c b/ui/sdl2.c
>> > index f259e4c4d1..592eca3e1c 100644
>> > --- a/ui/sdl2.c
>> > +++ b/ui/sdl2.c
>> > @@ -841,6 +841,10 @@ static void sdl2_display_init(DisplayState *ds,
>> DisplayOptions *o)
>> >       }
>> >   #endif
>> >
>> > +#ifdef _SDL_DEBUG
>> > +    SDL_LogSetAllPriority(SDL_LOG_PRIORITY_VERBOSE);
>> > +#endif
>>
>
> Sam, do you suggest a better way to enable SDL debugging when starting
> QEMU? Is there a way to enable it with existing SDL environment variables?
>
> thanks
>
> > +
>> >       if (SDL_Init(SDL_INIT_VIDEO)) {
>> >           fprintf(stderr, "Could not initialize SDL(%s) - exiting\n",
>> >                   SDL_GetError());
>>
>>

[-- Attachment #2: Type: text/html, Size: 3991 bytes --]

[PATCH v2 15/18] ui/sdl: try to instantiate the matching opengl renderer

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

User can still bypass the QEMU choice with SDL_RENDER_DRIVER environment
variable. (for some reason, specifying a driver disables batching and
breaks rendering, so enable it explicitly)

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 ui/sdl2.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/ui/sdl2.c b/ui/sdl2.c
index 592eca3e1c..e83ea53628 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -99,7 +99,18 @@ void sdl2_window_create(struct sdl2_console *scon)
                                          surface_width(scon->surface),
                                          surface_height(scon->surface),
                                          flags);
+    if (scon->opengl) {
+        const char *driver = "opengl";
+
+        if (scon->opts->gl == DISPLAYGL_MODE_ES) {
+            driver = "opengles2";
+        }
+
+        SDL_SetHint(SDL_HINT_RENDER_DRIVER, driver);
+        SDL_SetHint(SDL_HINT_RENDER_BATCHING, "1");
+    }
     scon->real_renderer = SDL_CreateRenderer(scon->real_window, -1, 0);
+
     if (scon->opengl) {
         scon->winctx = SDL_GL_CreateContext(scon->real_window);
     }
-- 
2.39.2

Re: [PATCH v2 15/18] ui/sdl: try to instantiate the matching opengl renderer

[Daniel P. Berrangé]

On Tue, Mar 07, 2023 at 03:56:34PM +0400, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> User can still bypass the QEMU choice with SDL_RENDER_DRIVER environment
> variable. (for some reason, specifying a driver disables batching and
> breaks rendering, so enable it explicitly)

What problem is this change actually fixing ?  Can you explain here.

> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  ui/sdl2.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/ui/sdl2.c b/ui/sdl2.c
> index 592eca3e1c..e83ea53628 100644
> --- a/ui/sdl2.c
> +++ b/ui/sdl2.c
> @@ -99,7 +99,18 @@ void sdl2_window_create(struct sdl2_console *scon)
>                                           surface_width(scon->surface),
>                                           surface_height(scon->surface),
>                                           flags);
> +    if (scon->opengl) {
> +        const char *driver = "opengl";
> +
> +        if (scon->opts->gl == DISPLAYGL_MODE_ES) {
> +            driver = "opengles2";
> +        }
> +
> +        SDL_SetHint(SDL_HINT_RENDER_DRIVER, driver);
> +        SDL_SetHint(SDL_HINT_RENDER_BATCHING, "1");
> +    }
>      scon->real_renderer = SDL_CreateRenderer(scon->real_window, -1, 0);
> +
>      if (scon->opengl) {
>          scon->winctx = SDL_GL_CreateContext(scon->real_window);
>      }
> -- 
> 2.39.2
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH v2 15/18] ui/sdl: try to instantiate the matching opengl renderer

[Marc-André Lureau]

Hi

On Fri, Mar 10, 2023 at 2:03 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Tue, Mar 07, 2023 at 03:56:34PM +0400, marcandre.lureau@redhat.com wrote:
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > User can still bypass the QEMU choice with SDL_RENDER_DRIVER environment
> > variable. (for some reason, specifying a driver disables batching and
> > breaks rendering, so enable it explicitly)
>
> What problem is this change actually fixing ?  Can you explain here.

Simply that -display sdl,gl=es didn't actually use OpenGL ES without this.

Using OpenGL ES allows to use ANGLE, which works generally better than
Windows/OEM OpenGL.

>
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >  ui/sdl2.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> >
> > diff --git a/ui/sdl2.c b/ui/sdl2.c
> > index 592eca3e1c..e83ea53628 100644
> > --- a/ui/sdl2.c
> > +++ b/ui/sdl2.c
> > @@ -99,7 +99,18 @@ void sdl2_window_create(struct sdl2_console *scon)
> >                                           surface_width(scon->surface),
> >                                           surface_height(scon->surface),
> >                                           flags);
> > +    if (scon->opengl) {
> > +        const char *driver = "opengl";
> > +
> > +        if (scon->opts->gl == DISPLAYGL_MODE_ES) {
> > +            driver = "opengles2";
> > +        }
> > +
> > +        SDL_SetHint(SDL_HINT_RENDER_DRIVER, driver);
> > +        SDL_SetHint(SDL_HINT_RENDER_BATCHING, "1");
> > +    }
> >      scon->real_renderer = SDL_CreateRenderer(scon->real_window, -1, 0);
> > +
> >      if (scon->opengl) {
> >          scon->winctx = SDL_GL_CreateContext(scon->real_window);
> >      }
> > --
> > 2.39.2
> >
>
> With regards,
> Daniel
> --
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
>
>


-- 
Marc-André Lureau

[PATCH v2 16/18] ui: introduce egl_init()

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Future patches will introduce EGL support on win32 (too late for 8.0
though). Having a common place for EGL initialization and error handling
will make it simpler.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 include/ui/egl-helpers.h |  2 ++
 ui/dbus.c                |  7 +------
 ui/egl-headless.c        | 16 ++++++++--------
 ui/egl-helpers.c         | 25 +++++++++++++++++++++++++
 ui/spice-core.c          |  7 +------
 5 files changed, 37 insertions(+), 20 deletions(-)

diff --git a/include/ui/egl-helpers.h b/include/ui/egl-helpers.h
index c92dd90e33..53d953ddf4 100644
--- a/include/ui/egl-helpers.h
+++ b/include/ui/egl-helpers.h
@@ -65,4 +65,6 @@ int qemu_egl_init_dpy_mesa(EGLNativeDisplayType dpy, DisplayGLMode mode);
 EGLContext qemu_egl_init_ctx(void);
 bool qemu_egl_has_dmabuf(void);
 
+bool egl_init(const char *rendernode, DisplayGLMode mode, Error **errp);
+
 #endif /* EGL_HELPERS_H */
diff --git a/ui/dbus.c b/ui/dbus.c
index f529928f0b..ebf03bd84d 100644
--- a/ui/dbus.c
+++ b/ui/dbus.c
@@ -451,12 +451,7 @@ early_dbus_init(DisplayOptions *opts)
     DisplayGLMode mode = opts->has_gl ? opts->gl : DISPLAYGL_MODE_OFF;
 
     if (mode != DISPLAYGL_MODE_OFF) {
-        if (egl_rendernode_init(opts->u.dbus.rendernode, mode) < 0) {
-            error_report("dbus: render node init failed");
-            exit(1);
-        }
-
-        display_opengl = 1;
+        egl_init(opts->u.dbus.rendernode, mode, &error_fatal);
     }
 
     type_register(&dbus_vc_type_info);
diff --git a/ui/egl-headless.c b/ui/egl-headless.c
index ae07e91302..ef70e6a18e 100644
--- a/ui/egl-headless.c
+++ b/ui/egl-headless.c
@@ -1,7 +1,7 @@
 #include "qemu/osdep.h"
 #include "qemu/error-report.h"
 #include "qemu/module.h"
-#include "sysemu/sysemu.h"
+#include "qapi/error.h"
 #include "ui/console.h"
 #include "ui/egl-helpers.h"
 #include "ui/egl-context.h"
@@ -191,21 +191,21 @@ static const DisplayGLCtxOps eglctx_ops = {
 
 static void early_egl_headless_init(DisplayOptions *opts)
 {
-    display_opengl = 1;
+    DisplayGLMode mode = DISPLAYGL_MODE_ON;
+
+    if (opts->has_gl) {
+        mode = opts->gl;
+    }
+
+    egl_init(opts->u.egl_headless.rendernode, mode, &error_fatal);
 }
 
 static void egl_headless_init(DisplayState *ds, DisplayOptions *opts)
 {
-    DisplayGLMode mode = opts->has_gl ? opts->gl : DISPLAYGL_MODE_ON;
     QemuConsole *con;
     egl_dpy *edpy;
     int idx;
 
-    if (egl_rendernode_init(opts->u.egl_headless.rendernode, mode) < 0) {
-        error_report("egl: render node init failed");
-        exit(1);
-    }
-
     for (idx = 0;; idx++) {
         DisplayGLCtx *ctx;
 
diff --git a/ui/egl-helpers.c b/ui/egl-helpers.c
index 10772b6471..36b4fc51d9 100644
--- a/ui/egl-helpers.c
+++ b/ui/egl-helpers.c
@@ -19,6 +19,8 @@
 #include "qemu/error-report.h"
 #include "ui/console.h"
 #include "ui/egl-helpers.h"
+#include "sysemu/sysemu.h"
+#include "qapi/error.h"
 
 EGLDisplay *qemu_egl_display;
 EGLConfig qemu_egl_config;
@@ -569,3 +571,26 @@ EGLContext qemu_egl_init_ctx(void)
 
     return ectx;
 }
+
+bool egl_init(const char *rendernode, DisplayGLMode mode, Error **errp)
+{
+    ERRP_GUARD();
+
+    if (mode == DISPLAYGL_MODE_OFF) {
+        error_setg(errp, "egl: turning off GL doesn't make sense");
+        return false;
+    }
+
+#ifdef CONFIG_GBM
+    if (egl_rendernode_init(rendernode, mode) < 0) {
+        error_setg(errp, "egl: render node init failed");
+        return false;
+    }
+#else
+    error_setg(errp, "egl: not available on this platform");
+    return false;
+#endif
+
+    display_opengl = 1;
+    return true;
+}
diff --git a/ui/spice-core.c b/ui/spice-core.c
index 76f7c2bc3d..b05c830086 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -820,12 +820,7 @@ static void qemu_spice_init(void)
                          "incompatible with -spice port/tls-port");
             exit(1);
         }
-        if (egl_rendernode_init(qemu_opt_get(opts, "rendernode"),
-                                DISPLAYGL_MODE_ON) != 0) {
-            error_report("Failed to initialize EGL render node for SPICE GL");
-            exit(1);
-        }
-        display_opengl = 1;
+        egl_init(qemu_opt_get(opts, "rendernode"), DISPLAYGL_MODE_ON, &error_fatal);
         spice_opengl = 1;
     }
 #endif
-- 
2.39.2

Re: [PATCH v2 16/18] ui: introduce egl_init()

[Daniel P. Berrangé]

On Tue, Mar 07, 2023 at 03:56:35PM +0400, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> Future patches will introduce EGL support on win32 (too late for 8.0
> though). Having a common place for EGL initialization and error handling
> will make it simpler.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  include/ui/egl-helpers.h |  2 ++
>  ui/dbus.c                |  7 +------
>  ui/egl-headless.c        | 16 ++++++++--------
>  ui/egl-helpers.c         | 25 +++++++++++++++++++++++++
>  ui/spice-core.c          |  7 +------
>  5 files changed, 37 insertions(+), 20 deletions(-)
> 
> diff --git a/include/ui/egl-helpers.h b/include/ui/egl-helpers.h
> index c92dd90e33..53d953ddf4 100644
> --- a/include/ui/egl-helpers.h
> +++ b/include/ui/egl-helpers.h
> @@ -65,4 +65,6 @@ int qemu_egl_init_dpy_mesa(EGLNativeDisplayType dpy, DisplayGLMode mode);
>  EGLContext qemu_egl_init_ctx(void);
>  bool qemu_egl_has_dmabuf(void);
>  
> +bool egl_init(const char *rendernode, DisplayGLMode mode, Error **errp);
> +
>  #endif /* EGL_HELPERS_H */
> diff --git a/ui/dbus.c b/ui/dbus.c
> index f529928f0b..ebf03bd84d 100644
> --- a/ui/dbus.c
> +++ b/ui/dbus.c
> @@ -451,12 +451,7 @@ early_dbus_init(DisplayOptions *opts)
>      DisplayGLMode mode = opts->has_gl ? opts->gl : DISPLAYGL_MODE_OFF;
>  
>      if (mode != DISPLAYGL_MODE_OFF) {
> -        if (egl_rendernode_init(opts->u.dbus.rendernode, mode) < 0) {
> -            error_report("dbus: render node init failed");
> -            exit(1);
> -        }
> -
> -        display_opengl = 1;
> +        egl_init(opts->u.dbus.rendernode, mode, &error_fatal);
>      }
>  
>      type_register(&dbus_vc_type_info);
> diff --git a/ui/egl-headless.c b/ui/egl-headless.c
> index ae07e91302..ef70e6a18e 100644
> --- a/ui/egl-headless.c
> +++ b/ui/egl-headless.c
> @@ -1,7 +1,7 @@
>  #include "qemu/osdep.h"
>  #include "qemu/error-report.h"
>  #include "qemu/module.h"
> -#include "sysemu/sysemu.h"
> +#include "qapi/error.h"
>  #include "ui/console.h"
>  #include "ui/egl-helpers.h"
>  #include "ui/egl-context.h"
> @@ -191,21 +191,21 @@ static const DisplayGLCtxOps eglctx_ops = {
>  
>  static void early_egl_headless_init(DisplayOptions *opts)
>  {
> -    display_opengl = 1;
> +    DisplayGLMode mode = DISPLAYGL_MODE_ON;
> +
> +    if (opts->has_gl) {
> +        mode = opts->gl;
> +    }
> +
> +    egl_init(opts->u.egl_headless.rendernode, mode, &error_fatal);
>  }
>  
>  static void egl_headless_init(DisplayState *ds, DisplayOptions *opts)
>  {
> -    DisplayGLMode mode = opts->has_gl ? opts->gl : DISPLAYGL_MODE_ON;
>      QemuConsole *con;
>      egl_dpy *edpy;
>      int idx;
>  
> -    if (egl_rendernode_init(opts->u.egl_headless.rendernode, mode) < 0) {
> -        error_report("egl: render node init failed");
> -        exit(1);
> -    }
> -
>      for (idx = 0;; idx++) {
>          DisplayGLCtx *ctx;

Why isn't the egl_init() call being made from this egl_headless_init
method, so egl_rendernode_init() is called at the same logical point
as before this change ?


> diff --git a/ui/egl-helpers.c b/ui/egl-helpers.c
> index 10772b6471..36b4fc51d9 100644
> --- a/ui/egl-helpers.c
> +++ b/ui/egl-helpers.c
> @@ -19,6 +19,8 @@
>  #include "qemu/error-report.h"
>  #include "ui/console.h"
>  #include "ui/egl-helpers.h"
> +#include "sysemu/sysemu.h"
> +#include "qapi/error.h"
>  
>  EGLDisplay *qemu_egl_display;
>  EGLConfig qemu_egl_config;
> @@ -569,3 +571,26 @@ EGLContext qemu_egl_init_ctx(void)
>  
>      return ectx;
>  }
> +
> +bool egl_init(const char *rendernode, DisplayGLMode mode, Error **errp)
> +{
> +    ERRP_GUARD();
> +
> +    if (mode == DISPLAYGL_MODE_OFF) {
> +        error_setg(errp, "egl: turning off GL doesn't make sense");
> +        return false;
> +    }
> +
> +#ifdef CONFIG_GBM
> +    if (egl_rendernode_init(rendernode, mode) < 0) {
> +        error_setg(errp, "egl: render node init failed");
> +        return false;
> +    }
> +#else
> +    error_setg(errp, "egl: not available on this platform");
> +    return false;
> +#endif
> +
> +    display_opengl = 1;
> +    return true;
> +}

Surely this is going to result in compile errors when !CONFIG_GBM
because these two lines are going to be flagged as unreachable
code, due to the 'return false' in the #else branch.

> diff --git a/ui/spice-core.c b/ui/spice-core.c
> index 76f7c2bc3d..b05c830086 100644
> --- a/ui/spice-core.c
> +++ b/ui/spice-core.c
> @@ -820,12 +820,7 @@ static void qemu_spice_init(void)
>                           "incompatible with -spice port/tls-port");
>              exit(1);
>          }
> -        if (egl_rendernode_init(qemu_opt_get(opts, "rendernode"),
> -                                DISPLAYGL_MODE_ON) != 0) {
> -            error_report("Failed to initialize EGL render node for SPICE GL");
> -            exit(1);
> -        }
> -        display_opengl = 1;
> +        egl_init(qemu_opt_get(opts, "rendernode"), DISPLAYGL_MODE_ON, &error_fatal);
>          spice_opengl = 1;
>      }
>  #endif
> -- 
> 2.39.2
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH v2 16/18] ui: introduce egl_init()

[Marc-André Lureau]

Hi

On Fri, Mar 10, 2023 at 2:07 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Tue, Mar 07, 2023 at 03:56:35PM +0400, marcandre.lureau@redhat.com wrote:
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > Future patches will introduce EGL support on win32 (too late for 8.0
> > though). Having a common place for EGL initialization and error handling
> > will make it simpler.
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >  include/ui/egl-helpers.h |  2 ++
> >  ui/dbus.c                |  7 +------
> >  ui/egl-headless.c        | 16 ++++++++--------
> >  ui/egl-helpers.c         | 25 +++++++++++++++++++++++++
> >  ui/spice-core.c          |  7 +------
> >  5 files changed, 37 insertions(+), 20 deletions(-)
> >
> > diff --git a/include/ui/egl-helpers.h b/include/ui/egl-helpers.h
> > index c92dd90e33..53d953ddf4 100644
> > --- a/include/ui/egl-helpers.h
> > +++ b/include/ui/egl-helpers.h
> > @@ -65,4 +65,6 @@ int qemu_egl_init_dpy_mesa(EGLNativeDisplayType dpy, DisplayGLMode mode);
> >  EGLContext qemu_egl_init_ctx(void);
> >  bool qemu_egl_has_dmabuf(void);
> >
> > +bool egl_init(const char *rendernode, DisplayGLMode mode, Error **errp);
> > +
> >  #endif /* EGL_HELPERS_H */
> > diff --git a/ui/dbus.c b/ui/dbus.c
> > index f529928f0b..ebf03bd84d 100644
> > --- a/ui/dbus.c
> > +++ b/ui/dbus.c
> > @@ -451,12 +451,7 @@ early_dbus_init(DisplayOptions *opts)
> >      DisplayGLMode mode = opts->has_gl ? opts->gl : DISPLAYGL_MODE_OFF;
> >
> >      if (mode != DISPLAYGL_MODE_OFF) {
> > -        if (egl_rendernode_init(opts->u.dbus.rendernode, mode) < 0) {
> > -            error_report("dbus: render node init failed");
> > -            exit(1);
> > -        }
> > -
> > -        display_opengl = 1;
> > +        egl_init(opts->u.dbus.rendernode, mode, &error_fatal);
> >      }
> >
> >      type_register(&dbus_vc_type_info);
> > diff --git a/ui/egl-headless.c b/ui/egl-headless.c
> > index ae07e91302..ef70e6a18e 100644
> > --- a/ui/egl-headless.c
> > +++ b/ui/egl-headless.c
> > @@ -1,7 +1,7 @@
> >  #include "qemu/osdep.h"
> >  #include "qemu/error-report.h"
> >  #include "qemu/module.h"
> > -#include "sysemu/sysemu.h"
> > +#include "qapi/error.h"
> >  #include "ui/console.h"
> >  #include "ui/egl-helpers.h"
> >  #include "ui/egl-context.h"
> > @@ -191,21 +191,21 @@ static const DisplayGLCtxOps eglctx_ops = {
> >
> >  static void early_egl_headless_init(DisplayOptions *opts)
> >  {
> > -    display_opengl = 1;
> > +    DisplayGLMode mode = DISPLAYGL_MODE_ON;
> > +
> > +    if (opts->has_gl) {
> > +        mode = opts->gl;
> > +    }
> > +
> > +    egl_init(opts->u.egl_headless.rendernode, mode, &error_fatal);
> >  }
> >
> >  static void egl_headless_init(DisplayState *ds, DisplayOptions *opts)
> >  {
> > -    DisplayGLMode mode = opts->has_gl ? opts->gl : DISPLAYGL_MODE_ON;
> >      QemuConsole *con;
> >      egl_dpy *edpy;
> >      int idx;
> >
> > -    if (egl_rendernode_init(opts->u.egl_headless.rendernode, mode) < 0) {
> > -        error_report("egl: render node init failed");
> > -        exit(1);
> > -    }
> > -
> >      for (idx = 0;; idx++) {
> >          DisplayGLCtx *ctx;
>
> Why isn't the egl_init() call being made from this egl_headless_init
> method, so egl_rendernode_init() is called at the same logical point
> as before this change ?

For consistency with the other egl_init() callers, called during
qemu_create_early_backends(). That way display_opengl is set early too
(as before).
>
>
> > diff --git a/ui/egl-helpers.c b/ui/egl-helpers.c
> > index 10772b6471..36b4fc51d9 100644
> > --- a/ui/egl-helpers.c
> > +++ b/ui/egl-helpers.c
> > @@ -19,6 +19,8 @@
> >  #include "qemu/error-report.h"
> >  #include "ui/console.h"
> >  #include "ui/egl-helpers.h"
> > +#include "sysemu/sysemu.h"
> > +#include "qapi/error.h"
> >
> >  EGLDisplay *qemu_egl_display;
> >  EGLConfig qemu_egl_config;
> > @@ -569,3 +571,26 @@ EGLContext qemu_egl_init_ctx(void)
> >
> >      return ectx;
> >  }
> > +
> > +bool egl_init(const char *rendernode, DisplayGLMode mode, Error **errp)
> > +{
> > +    ERRP_GUARD();
> > +
> > +    if (mode == DISPLAYGL_MODE_OFF) {
> > +        error_setg(errp, "egl: turning off GL doesn't make sense");
> > +        return false;
> > +    }
> > +
> > +#ifdef CONFIG_GBM
> > +    if (egl_rendernode_init(rendernode, mode) < 0) {
> > +        error_setg(errp, "egl: render node init failed");
> > +        return false;
> > +    }
> > +#else
> > +    error_setg(errp, "egl: not available on this platform");
> > +    return false;
> > +#endif
> > +
> > +    display_opengl = 1;
> > +    return true;
> > +}
>
> Surely this is going to result in compile errors when !CONFIG_GBM
> because these two lines are going to be flagged as unreachable
> code, due to the 'return false' in the #else branch.

Interestingly, I don't get a warning (-g -O2).. I'll change the code
nonetheless.

>
> > diff --git a/ui/spice-core.c b/ui/spice-core.c
> > index 76f7c2bc3d..b05c830086 100644
> > --- a/ui/spice-core.c
> > +++ b/ui/spice-core.c
> > @@ -820,12 +820,7 @@ static void qemu_spice_init(void)
> >                           "incompatible with -spice port/tls-port");
> >              exit(1);
> >          }
> > -        if (egl_rendernode_init(qemu_opt_get(opts, "rendernode"),
> > -                                DISPLAYGL_MODE_ON) != 0) {
> > -            error_report("Failed to initialize EGL render node for SPICE GL");
> > -            exit(1);
> > -        }
> > -        display_opengl = 1;
> > +        egl_init(qemu_opt_get(opts, "rendernode"), DISPLAYGL_MODE_ON, &error_fatal);
> >          spice_opengl = 1;
> >      }
> >  #endif
> > --
> > 2.39.2
> >
>
> With regards,
> Daniel
> --
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
>
>


-- 
Marc-André Lureau

[PATCH v2 17/18] ui/dbus: do not require opengl & gbm

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Allow to build & use the DBus display without 3d/GPU acceleration support.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 meson.build             |  2 --
 ui/dbus-listener.c      | 15 +++++++++++++--
 ui/dbus.c               |  8 ++++++++
 tests/qtest/meson.build |  2 +-
 ui/meson.build          |  4 ++--
 5 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/meson.build b/meson.build
index 6bcab8bf0d..a73d050946 100644
--- a/meson.build
+++ b/meson.build
@@ -1746,8 +1746,6 @@ dbus_display = get_option('dbus_display') \
            error_message: '-display dbus requires glib>=2.64') \
   .require(gdbus_codegen.found(),
            error_message: gdbus_codegen_error.format('-display dbus')) \
-  .require(opengl.found() and gbm.found(),
-           error_message: '-display dbus requires epoxy/egl and gbm') \
   .allowed()
 
 have_virtfs = get_option('virtfs') \
diff --git a/ui/dbus-listener.c b/ui/dbus-listener.c
index 2e87ed7e9a..85692f1b27 100644
--- a/ui/dbus-listener.c
+++ b/ui/dbus-listener.c
@@ -27,9 +27,11 @@
 #include "dbus.h"
 #include <gio/gunixfdlist.h>
 
+#ifdef CONFIG_OPENGL
 #include "ui/shader.h"
 #include "ui/egl-helpers.h"
 #include "ui/egl-context.h"
+#endif
 #include "trace.h"
 
 struct _DBusDisplayListener {
@@ -48,6 +50,7 @@ struct _DBusDisplayListener {
 
 G_DEFINE_TYPE(DBusDisplayListener, dbus_display_listener, G_TYPE_OBJECT)
 
+#ifdef CONFIG_OPENGL
 static void dbus_update_gl_cb(GObject *source_object,
                            GAsyncResult *res,
                            gpointer user_data)
@@ -229,12 +232,14 @@ static void dbus_gl_refresh(DisplayChangeListener *dcl)
         ddl->gl_updates = 0;
     }
 }
+#endif
 
 static void dbus_refresh(DisplayChangeListener *dcl)
 {
     graphic_hw_update(dcl->con);
 }
 
+#ifdef CONFIG_OPENGL
 static void dbus_gl_gfx_update(DisplayChangeListener *dcl,
                                int x, int y, int w, int h)
 {
@@ -242,6 +247,7 @@ static void dbus_gl_gfx_update(DisplayChangeListener *dcl,
 
     ddl->gl_updates++;
 }
+#endif
 
 static void dbus_gfx_update(DisplayChangeListener *dcl,
                             int x, int y, int w, int h)
@@ -296,6 +302,7 @@ static void dbus_gfx_update(DisplayChangeListener *dcl,
         DBUS_DEFAULT_TIMEOUT, NULL, NULL, NULL);
 }
 
+#ifdef CONFIG_OPENGL
 static void dbus_gl_gfx_switch(DisplayChangeListener *dcl,
                                struct DisplaySurface *new_surface)
 {
@@ -311,6 +318,7 @@ static void dbus_gl_gfx_switch(DisplayChangeListener *dcl,
                              width, height, 0, 0, width, height);
     }
 }
+#endif
 
 static void dbus_gfx_switch(DisplayChangeListener *dcl,
                             struct DisplaySurface *new_surface)
@@ -361,6 +369,7 @@ static void dbus_cursor_define(DisplayChangeListener *dcl,
         NULL);
 }
 
+#ifdef CONFIG_OPENGL
 const DisplayChangeListenerOps dbus_gl_dcl_ops = {
     .dpy_name                = "dbus-gl",
     .dpy_gfx_update          = dbus_gl_gfx_update,
@@ -378,6 +387,7 @@ const DisplayChangeListenerOps dbus_gl_dcl_ops = {
     .dpy_gl_release_dmabuf   = dbus_release_dmabuf,
     .dpy_gl_update           = dbus_scanout_update,
 };
+#endif
 
 const DisplayChangeListenerOps dbus_dcl_ops = {
     .dpy_name                = "dbus",
@@ -406,11 +416,12 @@ dbus_display_listener_constructed(GObject *object)
 {
     DBusDisplayListener *ddl = DBUS_DISPLAY_LISTENER(object);
 
+    ddl->dcl.ops = &dbus_dcl_ops;
+#ifdef CONFIG_OPENGL
     if (display_opengl) {
         ddl->dcl.ops = &dbus_gl_dcl_ops;
-    } else {
-        ddl->dcl.ops = &dbus_dcl_ops;
     }
+#endif
 
     G_OBJECT_CLASS(dbus_display_listener_parent_class)->constructed(object);
 }
diff --git a/ui/dbus.c b/ui/dbus.c
index ebf03bd84d..904f5a0a6d 100644
--- a/ui/dbus.c
+++ b/ui/dbus.c
@@ -30,8 +30,10 @@
 #include "qom/object_interfaces.h"
 #include "sysemu/sysemu.h"
 #include "ui/dbus-module.h"
+#ifdef CONFIG_OPENGL
 #include "ui/egl-helpers.h"
 #include "ui/egl-context.h"
+#endif
 #include "audio/audio.h"
 #include "audio/audio_int.h"
 #include "qapi/error.h"
@@ -41,6 +43,7 @@
 
 static DBusDisplay *dbus_display;
 
+#ifdef CONFIG_OPENGL
 static QEMUGLContext dbus_create_context(DisplayGLCtx *dgc,
                                          QEMUGLParams *params)
 {
@@ -84,6 +87,7 @@ static const DisplayGLCtxOps dbus_gl_ops = {
     .dpy_gl_ctx_destroy_texture = dbus_destroy_texture,
     .dpy_gl_ctx_update_texture = dbus_update_texture,
 };
+#endif
 
 static NotifierList dbus_display_notifiers =
     NOTIFIER_LIST_INITIALIZER(dbus_display_notifiers);
@@ -112,10 +116,12 @@ dbus_display_init(Object *o)
     DBusDisplay *dd = DBUS_DISPLAY(o);
     g_autoptr(GDBusObjectSkeleton) vm = NULL;
 
+#ifdef CONFIG_OPENGL
     dd->glctx.ops = &dbus_gl_ops;
     if (display_opengl) {
         dd->glctx.gls = qemu_gl_init_shader();
     }
+#endif
     dd->iface = qemu_dbus_display1_vm_skeleton_new();
     dd->consoles = g_ptr_array_new_with_free_func(g_object_unref);
 
@@ -152,7 +158,9 @@ dbus_display_finalize(Object *o)
     g_clear_object(&dd->iface);
     g_free(dd->dbus_addr);
     g_free(dd->audiodev);
+#ifdef CONFIG_OPENGL
     g_clear_pointer(&dd->glctx.gls, qemu_gl_fini_shader);
+#endif
     dbus_display = NULL;
 }
 
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 29a4efb4c2..f23a4aaa24 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -101,7 +101,7 @@ qtests_i386 = \
    'numa-test'
   ]
 
-if dbus_display
+if dbus_display and targetos != 'windows'
   qtests_i386 += ['dbus-display-test']
 endif
 
diff --git a/ui/meson.build b/ui/meson.build
index 0b2d0d21d1..330369707d 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, opengl, gbm, dbus_display1_dep],
+  dbus_ss.add(when: [gio, pixman, dbus_display1_dep],
               if_true: [files(
                 'dbus-chardev.c',
                 'dbus-clipboard.c',
@@ -93,7 +93,7 @@ if dbus_display
                 'dbus-error.c',
                 'dbus-listener.c',
                 'dbus.c',
-              )])
+              ), opengl, gbm])
   ui_modules += {'dbus' : dbus_ss}
 endif
 
-- 
2.39.2

Re: [PATCH v2 17/18] ui/dbus: do not require opengl & gbm

[Marc-André Lureau]

Hi

On Tue, Mar 7, 2023 at 3:59 PM <marcandre.lureau@redhat.com> wrote:
>
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Allow to build & use the DBus display without 3d/GPU acceleration support.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  meson.build             |  2 --
>  ui/dbus-listener.c      | 15 +++++++++++++--
>  ui/dbus.c               |  8 ++++++++
>  tests/qtest/meson.build |  2 +-
>  ui/meson.build          |  4 ++--
>  5 files changed, 24 insertions(+), 7 deletions(-)
>
> diff --git a/meson.build b/meson.build
> index 6bcab8bf0d..a73d050946 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1746,8 +1746,6 @@ dbus_display = get_option('dbus_display') \
>             error_message: '-display dbus requires glib>=2.64') \
>    .require(gdbus_codegen.found(),
>             error_message: gdbus_codegen_error.format('-display dbus')) \
> -  .require(opengl.found() and gbm.found(),
> -           error_message: '-display dbus requires epoxy/egl and gbm') \
>    .allowed()

This change enables compilation of dbus on win32, although it requires
additional fixes. I'll update it with:
+  .require(targetos != 'windows',
+           error_message: '-display dbus is not available on Windows') \

>
>  have_virtfs = get_option('virtfs') \
> diff --git a/ui/dbus-listener.c b/ui/dbus-listener.c
> index 2e87ed7e9a..85692f1b27 100644
> --- a/ui/dbus-listener.c
> +++ b/ui/dbus-listener.c
> @@ -27,9 +27,11 @@
>  #include "dbus.h"
>  #include <gio/gunixfdlist.h>
>
> +#ifdef CONFIG_OPENGL
>  #include "ui/shader.h"
>  #include "ui/egl-helpers.h"
>  #include "ui/egl-context.h"
> +#endif
>  #include "trace.h"
>
>  struct _DBusDisplayListener {
> @@ -48,6 +50,7 @@ struct _DBusDisplayListener {
>
>  G_DEFINE_TYPE(DBusDisplayListener, dbus_display_listener, G_TYPE_OBJECT)
>
> +#ifdef CONFIG_OPENGL
>  static void dbus_update_gl_cb(GObject *source_object,
>                             GAsyncResult *res,
>                             gpointer user_data)
> @@ -229,12 +232,14 @@ static void dbus_gl_refresh(DisplayChangeListener *dcl)
>          ddl->gl_updates = 0;
>      }
>  }
> +#endif
>
>  static void dbus_refresh(DisplayChangeListener *dcl)
>  {
>      graphic_hw_update(dcl->con);
>  }
>
> +#ifdef CONFIG_OPENGL
>  static void dbus_gl_gfx_update(DisplayChangeListener *dcl,
>                                 int x, int y, int w, int h)
>  {
> @@ -242,6 +247,7 @@ static void dbus_gl_gfx_update(DisplayChangeListener *dcl,
>
>      ddl->gl_updates++;
>  }
> +#endif
>
>  static void dbus_gfx_update(DisplayChangeListener *dcl,
>                              int x, int y, int w, int h)
> @@ -296,6 +302,7 @@ static void dbus_gfx_update(DisplayChangeListener *dcl,
>          DBUS_DEFAULT_TIMEOUT, NULL, NULL, NULL);
>  }
>
> +#ifdef CONFIG_OPENGL
>  static void dbus_gl_gfx_switch(DisplayChangeListener *dcl,
>                                 struct DisplaySurface *new_surface)
>  {
> @@ -311,6 +318,7 @@ static void dbus_gl_gfx_switch(DisplayChangeListener *dcl,
>                               width, height, 0, 0, width, height);
>      }
>  }
> +#endif
>
>  static void dbus_gfx_switch(DisplayChangeListener *dcl,
>                              struct DisplaySurface *new_surface)
> @@ -361,6 +369,7 @@ static void dbus_cursor_define(DisplayChangeListener *dcl,
>          NULL);
>  }
>
> +#ifdef CONFIG_OPENGL
>  const DisplayChangeListenerOps dbus_gl_dcl_ops = {
>      .dpy_name                = "dbus-gl",
>      .dpy_gfx_update          = dbus_gl_gfx_update,
> @@ -378,6 +387,7 @@ const DisplayChangeListenerOps dbus_gl_dcl_ops = {
>      .dpy_gl_release_dmabuf   = dbus_release_dmabuf,
>      .dpy_gl_update           = dbus_scanout_update,
>  };
> +#endif
>
>  const DisplayChangeListenerOps dbus_dcl_ops = {
>      .dpy_name                = "dbus",
> @@ -406,11 +416,12 @@ dbus_display_listener_constructed(GObject *object)
>  {
>      DBusDisplayListener *ddl = DBUS_DISPLAY_LISTENER(object);
>
> +    ddl->dcl.ops = &dbus_dcl_ops;
> +#ifdef CONFIG_OPENGL
>      if (display_opengl) {
>          ddl->dcl.ops = &dbus_gl_dcl_ops;
> -    } else {
> -        ddl->dcl.ops = &dbus_dcl_ops;
>      }
> +#endif
>
>      G_OBJECT_CLASS(dbus_display_listener_parent_class)->constructed(object);
>  }
> diff --git a/ui/dbus.c b/ui/dbus.c
> index ebf03bd84d..904f5a0a6d 100644
> --- a/ui/dbus.c
> +++ b/ui/dbus.c
> @@ -30,8 +30,10 @@
>  #include "qom/object_interfaces.h"
>  #include "sysemu/sysemu.h"
>  #include "ui/dbus-module.h"
> +#ifdef CONFIG_OPENGL
>  #include "ui/egl-helpers.h"
>  #include "ui/egl-context.h"
> +#endif
>  #include "audio/audio.h"
>  #include "audio/audio_int.h"
>  #include "qapi/error.h"
> @@ -41,6 +43,7 @@
>
>  static DBusDisplay *dbus_display;
>
> +#ifdef CONFIG_OPENGL
>  static QEMUGLContext dbus_create_context(DisplayGLCtx *dgc,
>                                           QEMUGLParams *params)
>  {
> @@ -84,6 +87,7 @@ static const DisplayGLCtxOps dbus_gl_ops = {
>      .dpy_gl_ctx_destroy_texture = dbus_destroy_texture,
>      .dpy_gl_ctx_update_texture = dbus_update_texture,
>  };
> +#endif
>
>  static NotifierList dbus_display_notifiers =
>      NOTIFIER_LIST_INITIALIZER(dbus_display_notifiers);
> @@ -112,10 +116,12 @@ dbus_display_init(Object *o)
>      DBusDisplay *dd = DBUS_DISPLAY(o);
>      g_autoptr(GDBusObjectSkeleton) vm = NULL;
>
> +#ifdef CONFIG_OPENGL
>      dd->glctx.ops = &dbus_gl_ops;
>      if (display_opengl) {
>          dd->glctx.gls = qemu_gl_init_shader();
>      }
> +#endif
>      dd->iface = qemu_dbus_display1_vm_skeleton_new();
>      dd->consoles = g_ptr_array_new_with_free_func(g_object_unref);
>
> @@ -152,7 +158,9 @@ dbus_display_finalize(Object *o)
>      g_clear_object(&dd->iface);
>      g_free(dd->dbus_addr);
>      g_free(dd->audiodev);
> +#ifdef CONFIG_OPENGL
>      g_clear_pointer(&dd->glctx.gls, qemu_gl_fini_shader);
> +#endif
>      dbus_display = NULL;
>  }
>
> diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
> index 29a4efb4c2..f23a4aaa24 100644
> --- a/tests/qtest/meson.build
> +++ b/tests/qtest/meson.build
> @@ -101,7 +101,7 @@ qtests_i386 = \
>     'numa-test'
>    ]
>
> -if dbus_display
> +if dbus_display and targetos != 'windows'
>    qtests_i386 += ['dbus-display-test']
>  endif
>
> diff --git a/ui/meson.build b/ui/meson.build
> index 0b2d0d21d1..330369707d 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, opengl, gbm, dbus_display1_dep],
> +  dbus_ss.add(when: [gio, pixman, dbus_display1_dep],
>                if_true: [files(
>                  'dbus-chardev.c',
>                  'dbus-clipboard.c',
> @@ -93,7 +93,7 @@ if dbus_display
>                  'dbus-error.c',
>                  'dbus-listener.c',
>                  'dbus.c',
> -              )])
> +              ), opengl, gbm])
>    ui_modules += {'dbus' : dbus_ss}
>  endif
>
> --
> 2.39.2
>
>


-- 
Marc-André Lureau

[PATCH v2 18/18] ui/dbus: restrict opengl to gbm-enabled config

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

We can enable EGL later for non-GBM hosts.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 ui/dbus-listener.c | 10 +++++-----
 ui/dbus.c          | 12 +++++++++++-
 2 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/ui/dbus-listener.c b/ui/dbus-listener.c
index 85692f1b27..911acdc529 100644
--- a/ui/dbus-listener.c
+++ b/ui/dbus-listener.c
@@ -50,7 +50,7 @@ struct _DBusDisplayListener {
 
 G_DEFINE_TYPE(DBusDisplayListener, dbus_display_listener, G_TYPE_OBJECT)
 
-#ifdef CONFIG_OPENGL
+#ifdef CONFIG_GBM
 static void dbus_update_gl_cb(GObject *source_object,
                            GAsyncResult *res,
                            gpointer user_data)
@@ -239,7 +239,7 @@ static void dbus_refresh(DisplayChangeListener *dcl)
     graphic_hw_update(dcl->con);
 }
 
-#ifdef CONFIG_OPENGL
+#ifdef CONFIG_GBM
 static void dbus_gl_gfx_update(DisplayChangeListener *dcl,
                                int x, int y, int w, int h)
 {
@@ -302,7 +302,7 @@ static void dbus_gfx_update(DisplayChangeListener *dcl,
         DBUS_DEFAULT_TIMEOUT, NULL, NULL, NULL);
 }
 
-#ifdef CONFIG_OPENGL
+#ifdef CONFIG_GBM
 static void dbus_gl_gfx_switch(DisplayChangeListener *dcl,
                                struct DisplaySurface *new_surface)
 {
@@ -369,7 +369,7 @@ static void dbus_cursor_define(DisplayChangeListener *dcl,
         NULL);
 }
 
-#ifdef CONFIG_OPENGL
+#ifdef CONFIG_GBM
 const DisplayChangeListenerOps dbus_gl_dcl_ops = {
     .dpy_name                = "dbus-gl",
     .dpy_gfx_update          = dbus_gl_gfx_update,
@@ -417,7 +417,7 @@ dbus_display_listener_constructed(GObject *object)
     DBusDisplayListener *ddl = DBUS_DISPLAY_LISTENER(object);
 
     ddl->dcl.ops = &dbus_dcl_ops;
-#ifdef CONFIG_OPENGL
+#ifdef CONFIG_GBM
     if (display_opengl) {
         ddl->dcl.ops = &dbus_gl_dcl_ops;
     }
diff --git a/ui/dbus.c b/ui/dbus.c
index 904f5a0a6d..0513de9918 100644
--- a/ui/dbus.c
+++ b/ui/dbus.c
@@ -47,8 +47,10 @@ static DBusDisplay *dbus_display;
 static QEMUGLContext dbus_create_context(DisplayGLCtx *dgc,
                                          QEMUGLParams *params)
 {
+#ifdef CONFIG_GBM
     eglMakeCurrent(qemu_egl_display, EGL_NO_SURFACE, EGL_NO_SURFACE,
                    qemu_egl_rn_ctx);
+#endif
     return qemu_egl_create_context(dgc, params);
 }
 
@@ -56,7 +58,11 @@ static bool
 dbus_is_compatible_dcl(DisplayGLCtx *dgc,
                        DisplayChangeListener *dcl)
 {
-    return dcl->ops == &dbus_gl_dcl_ops || dcl->ops == &dbus_console_dcl_ops;
+    return
+#ifdef CONFIG_GBM
+        dcl->ops == &dbus_gl_dcl_ops ||
+#endif
+        dcl->ops == &dbus_console_dcl_ops;
 }
 
 static void
@@ -459,7 +465,11 @@ early_dbus_init(DisplayOptions *opts)
     DisplayGLMode mode = opts->has_gl ? opts->gl : DISPLAYGL_MODE_OFF;
 
     if (mode != DISPLAYGL_MODE_OFF) {
+#ifdef CONFIG_OPENGL
         egl_init(opts->u.dbus.rendernode, mode, &error_fatal);
+#else
+        error_report("dbus: GL rendering is not supported");
+#endif
     }
 
     type_register(&dbus_vc_type_info);
-- 
2.39.2

Re: [PATCH v2 00/18] ui: dbus & misc fixes

[Marc-André Lureau]

Hi

On Tue, Mar 7, 2023 at 3:57 PM <marcandre.lureau@redhat.com> wrote:
>
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Hi,
>
> This is a respin of a series I sent end of January that didn't get reviews, here
> is your chance! :) A collection of fixes for UI related-stuff, and paving the
> way for accelerated/egl dbus display support on win32 (for 8.1 hopefully).
>
> I think they are worth including in 8.0, as little fixes and improvements. It
> would be nice to have included as early as rc0.

If there is nobody available/interested enough to review, I wonder if
I should use my ui/ maintainer hat and send a PR as is.

thanks

>
> v2:
> - add "ui/dbus: initialize cursor_fb"
> - add "ui/shader: fix #version directive must occur on first line"
> - add "ui/egl: print EGL error, helping debugging"
> - add "ui/sdl: add optional logging when _SDL_DEBUG is set"
> - add "ui/sdl: try to instantiate the matching opengl renderer"
> - add "ui: introduce egl_init()"
> - add "ui/dbus: do not require opengl & gbm"
> - add "ui/dbus: restrict opengl to gbm-enabled config"
> - drop "ui/dbus: update the display when switching surface", as I
>   am not yet confident enough about it
> - rebased
>
> Marc-André Lureau (18):
>   ui/dbus: initialize cursor_fb
>   ui/dbus: unregister clipboard on connection close
>   audio/dbus: there are no sender for p2p mode
>   ui/dbus: set mouse is-absolute during console creation
>   meson: ensure dbus-display generated code is built before other units
>   ui: rename cursor_{put->unref}
>   ui: rename cursor_{get->ref}, return it
>   ui: keep current cursor with QemuConsole
>   ui: set cursor upon listener registration
>   ui: set cursor position upon listener registration
>   ui/sdl: get the GL context from the window
>   ui/shader: fix #version directive must occur on first line
>   ui/egl: print EGL error, helping debugging
>   ui/sdl: add optional logging when _SDL_DEBUG is set
>   ui/sdl: try to instantiate the matching opengl renderer
>   ui: introduce egl_init()
>   ui/dbus: do not require opengl & gbm
>   ui/dbus: restrict opengl to gbm-enabled config
>
>  meson.build                      |  2 -
>  audio/audio_int.h                |  2 +-
>  include/ui/console.h             |  5 +-
>  include/ui/egl-helpers.h         |  4 ++
>  include/ui/sdl2.h                |  2 +
>  ui/vnc.h                         |  1 -
>  audio/dbusaudio.c                |  6 ++-
>  hw/display/qxl-render.c          |  4 +-
>  hw/display/qxl.c                 |  2 +-
>  hw/display/vmware_vga.c          |  4 +-
>  ui/console.c                     | 18 ++++++++
>  ui/cursor.c                      |  5 +-
>  ui/dbus-clipboard.c              | 18 +++-----
>  ui/dbus-console.c                | 13 ++++--
>  ui/dbus-listener.c               | 22 ++++++---
>  ui/dbus.c                        | 29 ++++++++----
>  ui/egl-headless.c                | 16 +++----
>  ui/egl-helpers.c                 | 79 +++++++++++++++++++++++++++++---
>  ui/sdl2.c                        | 17 ++++++-
>  ui/spice-core.c                  |  7 +--
>  ui/spice-display.c               |  8 ++--
>  ui/vnc.c                         |  8 +---
>  tests/qtest/meson.build          |  2 +-
>  ui/meson.build                   |  6 ++-
>  ui/shader/texture-blit-flip.vert |  1 -
>  ui/shader/texture-blit.frag      |  1 -
>  ui/shader/texture-blit.vert      |  1 -
>  27 files changed, 203 insertions(+), 80 deletions(-)
>
> --
> 2.39.2
>
>


-- 
Marc-André Lureau