Hi
On Thu, Feb 17, 2022 at 5:09 PM Akihiko Odaki <akihiko.od...@gmail.com>
wrote:
> On Thu, Feb 17, 2022 at 8:58 PM <marcandre.lur...@redhat.com> wrote:
> >
> > From: Marc-André Lureau <marcandre.lur...@redhat.com>
> >
> > Hi,
> >
> > In the thread "[PATCH 0/6] ui/dbus: Share one listener for a console",
> Akihiko
> > Odaki reported a number of issues with the GL and D-Bus display. His
> series
> > propose a different design, and reverting some of my previous generic
> console
> > changes to fix those issues.
> >
> > However, as I work through the issue so far, they can be solved by
> reasonable
> > simple fixes while keeping the console changes generic (not specific to
> the
> > D-Bus display backend). I belive a shared infrastructure is more
> beneficial long
> > term than having GL-specific code in the DBus code (in particular, the
> > egl-headless & VNC combination could potentially use it)
> >
> > Thanks a lot Akihiko for reporting the issues proposing a different
> approach!
> > Please test this alternative series and let me know of any further
> issues. My
> > understanding is that you are mainly concerned with the Cocoa backend,
> and I
> > don't have a way to test it, so please check it. If necessary, we may
> well have
> > to revert my earlier changes and go your way, eventually.
> >
> > Marc-André Lureau (12):
> > ui/console: fix crash when using gl context with non-gl listeners
> > ui/console: fix texture leak when calling surface_gl_create_texture()
> > ui: do not create a surface when resizing a GL scanout
> > ui/console: move check for compatible GL context
> > ui/console: move dcl compatiblity check to a callback
> > ui/console: egl-headless is compatible with non-gl listeners
> > ui/dbus: associate the DBusDisplayConsole listener with the given
> > console
> > ui/console: move console compatibility check to dcl_display_console()
> > ui/shader: fix potential leak of shader on error
> > ui/shader: free associated programs
> > ui/console: add a dpy_gfx_switch callback helper
> > ui/dbus: fix texture sharing
> >
> > include/ui/console.h | 19 ++++---
> > ui/dbus.h | 3 ++
> > ui/console-gl.c | 4 ++
> > ui/console.c | 119 ++++++++++++++++++++++++++-----------------
> > ui/dbus-console.c | 27 +++++-----
> > ui/dbus-listener.c | 11 ----
> > ui/dbus.c | 33 +++++++++++-
> > ui/egl-headless.c | 17 ++++++-
> > ui/gtk.c | 18 ++++++-
> > ui/sdl2.c | 9 +++-
> > ui/shader.c | 9 +++-
> > ui/spice-display.c | 9 +++-
> > 12 files changed, 192 insertions(+), 86 deletions(-)
> >
> > --
> > 2.34.1.428.gdcc0cd074f0c
> >
> >
>
> You missed only one thing:
> >- that console_select and register_displaychangelistener may not call
> > dpy_gfx_switch and call dpy_gl_scanout_texture instead. It is
> > incompatible with non-OpenGL displays
>
> displaychangelistener_display_console always has to call
> dpy_gfx_switch for non-OpenGL displays, but it still doesn't.
>
Ok, would that be what you have in mind?
--- a/ui/console.c
+++ b/ui/console.c
@@ -1122,6 +1122,10 @@ static void
displaychangelistener_display_console(DisplayChangeListener *dcl,
} else if (con->scanout.kind == SCANOUT_SURFACE) {
dpy_gfx_create_texture(con, con->surface);
displaychangelistener_gfx_switch(dcl, con->surface);
+ } else {
+ /* use the fallback surface, egl-headless keeps it updated */
+ assert(con->surface);
+ displaychangelistener_gfx_switch(dcl, con->surface);
}
I wish such egl-headless specific code would be there, but we would need
more refactoring.
I think I would rather have a backend split for GL context, like "-object
egl-context". egl-headless-specific copy code would be handled by
common/util code for anything that wants a pixman surface (VNC, screen
capture, non-GL display etc).
This split would allow sharing the context code, and introduce other system
specific GL initialization, such as WGL etc. Right now, I doubt the EGL
code works on anything but Linux.
> Anything else should be addressed with this patch series. (And it also
> has nice fixes for shader leaks.)
>
thanks
>
> cocoa doesn't have OpenGL output and egl-headless, the cause of many
> pains addressed here, does not work on macOS so you need little
> attention. I have an out-of-tree OpenGL support for cocoa but it is
> out-of-tree anyway and I can fix it anytime.
>
Great!
btw, I suppose you checked your DBus changes against the WIP "qemu-display"
project. What was your experience? I don't think many people have tried it
yet. Do you think this could be made to work on macOS? at least the
non-dmabuf support should work, as long as Gtk4 has good macOS support. I
don't know if dmabuf or similar exist there, any idea?
--
Marc-André Lureau
