Re: [PATCH v2 00/12] GL & D-Bus display related fixes
Hi On Thu, Feb 17, 2022 at 9:36 PM Marc-André Lureau < marcandre.lur...@gmail.com> wrote: > > On Thu, Feb 17, 2022 at 9:25 PM Akihiko Odaki > wrote: > >> >> > 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? >> >> >> >> I tested it on Fedora. I think it would probably work on macOS but >> >> maybe require some tweaks. IOSurface is a counterpart of DMA-BUF in >> >> macOS but its situation is bad; it must be delivered via macOS's own >> >> IPC mechanisms (Mach port and XPC), but they are for server-client >> >> model and not for P2P. fileport mechanism allows to convert Mach port >> >> to file descriptor, but it is not documented. (In reality, I think all >> >> of the major browsers, Chromium, Firefox and Safari use fileport for >> >> this purpose. Apple should really document it if they use it for their >> >> app.) It is also possible to share IOSurface with a global number, but >> >> it can be brute-forced and is insecure. >> >> >> > >> > Thanks, the Gtk developers might have some clue. They have been working >> on improving macOS support, and it can use opengl now ( >> https://blogs.gnome.org/chergert/2020/12/15/gtk-4-got-a-new-macos-backend-now-with-opengl/ >> ). >> >> They don't need IPC for passing textures so that is a different story. >> > > Yes but they have web-engine and video decoding concerns (beside > qemu/dmabuf gtk display they should be aware of). I'll try to reach > Christian about it. > > fwiw, here is Christian Hergert comment about texture sharing & gtk on macos: "There is, and we are using it in GTK 4 as of 4.6 to render from OpenGL to a surface we can attach to CALayers or OpenGL textures. It has allowed us to do a bunch of tricks to ensure we have opaque surfaces since NSWindow doesn't have anything like set_opaque_region() from Wayland. It's called IOSurface and the browsers use this to pass rendererings between processes." -- Marc-André Lureau
Re: [PATCH v2 00/12] GL & D-Bus display related fixes
On 2022/02/18 1:11, Akihiko Odaki wrote:
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);
}
It should call displaychangelistener_gfx_switch even when e.g.
con->scanout.kind == SCANOUT_TEXTURE. egl-headless renders the content
to the last DisplaySurface it received while con->scanout.kind ==
SCANOUT_TEXTURE.
Hi,
Let me remind that the release date is approaching but the regression
which breaks switching the console for vnc with egl-headless is still
not fixed. (vnc has the feature to switch consoles with Ctrl+Alt+[1-9]
if it is not bound to a particular console.)
Please fix this or, if not possible, revert the changes related to dbus.
My patch series is available for fixing the problem. The design it
adopted is somewhat controversial and it cannot be applied to the
current master branch. Please tell me if it is necessary to rebase this.
https://patchew.org/QEMU/20220213024222.3548-1-akihiko.od...@gmail.com/
Regards,
Akihiko Odaki
Re: [PATCH v2 00/12] GL & D-Bus display related fixes
On Fri, Feb 18, 2022 at 2:36 AM Marc-André Lureau
wrote:
>
> Hi
>
> On Thu, Feb 17, 2022 at 9:25 PM Akihiko Odaki wrote:
>>
>> On Fri, Feb 18, 2022 at 2:07 AM Marc-André Lureau
>> wrote:
>> >
>> > Hi
>> >
>> > On Thu, Feb 17, 2022 at 8:39 PM Akihiko Odaki
>> > wrote:
>> >>
>> >> On Fri, Feb 18, 2022 at 1:12 AM Marc-André Lureau
>> >> wrote:
>> >> >
>> >> > Hi
>> >> >
>> >> > On Thu, Feb 17, 2022 at 5:09 PM Akihiko Odaki
>> >> > wrote:
>> >> >>
>> >> >> On Thu, Feb 17, 2022 at 8:58 PM wrote:
>> >> >> >
>> >> >> > From: Marc-André Lureau
>> >> >> >
>> >> >> > 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);
>> >> > }
>> >>
>> >> It should call displaychangelistener_gfx_switch even when e.g.
>> >> con->scanout.kind == SCANOUT_TEXTURE. egl-headless renders the content
>> >> to the last DisplaySurface it received while con->scanout.kind ==
>> >> SCANOUT_TEXTURE.
>> >
>> >
>> > I see, egl-headless is really not a "listener".
>> >
>> >>
>> >> >
>> >> > I wish such egl-headless specific code would be
Re: [PATCH v2 00/12] GL & D-Bus display related fixes
Hi
On Thu, Feb 17, 2022 at 9:25 PM Akihiko Odaki
wrote:
> On Fri, Feb 18, 2022 at 2:07 AM Marc-André Lureau
> wrote:
> >
> > Hi
> >
> > On Thu, Feb 17, 2022 at 8:39 PM Akihiko Odaki
> wrote:
> >>
> >> On Fri, Feb 18, 2022 at 1:12 AM Marc-André Lureau
> >> wrote:
> >> >
> >> > 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 wrote:
> >> >> >
> >> >> > From: Marc-André Lureau
> >> >> >
> >> >> > 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);
> >> > }
> >>
> >> It should call displaychangelistener_gfx_switch even when e.g.
> >> con->scanout.kind == SCANOUT_TEXTURE. egl-headless renders the content
> >> to the last DisplaySurface it received while con->scanout.kind ==
> >> SCANOUT_TEXTURE.
> >
> >
> > I see, egl-headless is really not a "listener".
> >
> >>
> >> >
> >> > 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
Re: [PATCH v2 00/12] GL & D-Bus display related fixes
On Fri, Feb 18, 2022 at 2:07 AM Marc-André Lureau
wrote:
>
> Hi
>
> On Thu, Feb 17, 2022 at 8:39 PM Akihiko Odaki wrote:
>>
>> On Fri, Feb 18, 2022 at 1:12 AM Marc-André Lureau
>> wrote:
>> >
>> > Hi
>> >
>> > On Thu, Feb 17, 2022 at 5:09 PM Akihiko Odaki
>> > wrote:
>> >>
>> >> On Thu, Feb 17, 2022 at 8:58 PM wrote:
>> >> >
>> >> > From: Marc-André Lureau
>> >> >
>> >> > 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);
>> > }
>>
>> It should call displaychangelistener_gfx_switch even when e.g.
>> con->scanout.kind == SCANOUT_TEXTURE. egl-headless renders the content
>> to the last DisplaySurface it received while con->scanout.kind ==
>> SCANOUT_TEXTURE.
>
>
> I see, egl-headless is really not a "listener".
>
>>
>> >
>> > 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.
>>
>>
Re: [PATCH v2 00/12] GL & D-Bus display related fixes
Hi
On Thu, Feb 17, 2022 at 8:39 PM Akihiko Odaki
wrote:
> On Fri, Feb 18, 2022 at 1:12 AM Marc-André Lureau
> wrote:
> >
> > Hi
> >
> > On Thu, Feb 17, 2022 at 5:09 PM Akihiko Odaki
> wrote:
> >>
> >> On Thu, Feb 17, 2022 at 8:58 PM wrote:
> >> >
> >> > From: Marc-André Lureau
> >> >
> >> > 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);
> > }
>
> It should call displaychangelistener_gfx_switch even when e.g.
> con->scanout.kind == SCANOUT_TEXTURE. egl-headless renders the content
> to the last DisplaySurface it received while con->scanout.kind ==
> SCANOUT_TEXTURE.
>
I see, egl-headless is really not a "listener".
> >
> > 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.
>
> Sharing the context code is unlikely to happen. Usually the toolkit
> (GTK, SDL, or Cocoa in my fork) knows what graphics accelerator should
> be used and how the context should be created for a particular window.
> The context sharing can be achieved only for headless
Re: [PATCH v2 00/12] GL & D-Bus display related fixes
On Fri, Feb 18, 2022 at 1:12 AM Marc-André Lureau
wrote:
>
> Hi
>
> On Thu, Feb 17, 2022 at 5:09 PM Akihiko Odaki wrote:
>>
>> On Thu, Feb 17, 2022 at 8:58 PM wrote:
>> >
>> > From: Marc-André Lureau
>> >
>> > 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);
> }
It should call displaychangelistener_gfx_switch even when e.g.
con->scanout.kind == SCANOUT_TEXTURE. egl-headless renders the content
to the last DisplaySurface it received while con->scanout.kind ==
SCANOUT_TEXTURE.
>
> 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.
Sharing the context code is unlikely to happen. Usually the toolkit
(GTK, SDL, or Cocoa in my fork) knows what graphics accelerator should
be used and how the context should be created for a particular window.
The context sharing can be achieved only for headless displays, namely
dbus, egl-headless and spice. Few people would want to use them in
combination.
>
>>
>> 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,
Re: [PATCH v2 00/12] GL & D-Bus display related fixes
Hi
On Thu, Feb 17, 2022 at 5:09 PM Akihiko Odaki
wrote:
> On Thu, Feb 17, 2022 at 8:58 PM wrote:
> >
> > From: Marc-André Lureau
> >
> > 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
Re: [PATCH v2 00/12] GL & D-Bus display related fixes
On Thu, Feb 17, 2022 at 8:58 PM wrote: > > From: Marc-André Lureau > > 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. Anything else should be addressed with this patch series. (And it also has nice fixes for shader leaks.) 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. Regards, Akihiko Odaki
