Hi Akihiko On Sun, Feb 13, 2022 at 6:44 AM Akihiko Odaki <akihiko.od...@gmail.com> wrote:
> ui/dbus required to have multiple DisplayChangeListeners (possibly with > OpenGL) > for a console but that caused several problems: > - It broke egl-headless, an unusual display which implements OpenGL > rendering > for non-OpenGL displays. The code to support multiple > DisplayChangeListeners > does not consider the case where non-OpenGL displays listens OpenGL > consoles. > Can you provide instructions on what broke? Even better write a test, please. "make check-avocado AVOCADO_TESTS=tests/avocado/virtio-gpu.py", which covers egl-headless usage, works. > - Multiple OpenGL DisplayChangeListeners of dbus shares one DisplaySurface > and > modifies its texture field, causing OpenGL texture leak and > use-after-free. > Again, please provide instructions. I have regularly run -display dbus with multiple clients and qemu compiled with sanitizers. I don't see any leak or use after free. > - Introduced extra code to check the compatibility of OpenGL context > providers > and OpenGL texture renderers where those are often inherently tightly > coupled > since they must share the same hardware. > So code checks are meant to prevent misusage. They might be too limited or broken in some ways, but reverting is likely going to introduce other regressions I was trying to fix. - Needed extra work to broadcast the same change to multiple dbus listeners. > > Compared to what? > This series solve them by implementing the change broadcast in ui/dbus, > which > knows exactly what is needed. Changes for the common code to support > multiple > OpenGL DisplayChangeListeners were reverted to fix egl-headless and reduce > the code size. > Thanks a lot for your work, I am going to take a look at your approach. But please help us understand better what the problem actually is, by giving examples & tests to avoid future regressions and document the expected behaviour. > Akihiko Odaki (6): > ui/dbus: Share one listener for a console > Revert "console: save current scanout details" > Revert "ui: split the GL context in a different object" > Revert "ui: dispatch GL events to all listeners" > Revert "ui: associate GL context outside of display listener > registration" > Revert "ui: factor out qemu_console_set_display_gl_ctx()" > > include/ui/console.h | 60 +----- > include/ui/egl-context.h | 6 +- > include/ui/gtk.h | 11 +- > include/ui/sdl2.h | 7 +- > include/ui/spice-display.h | 1 - > ui/console.c | 258 +++++++---------------- > ui/dbus-console.c | 109 ++-------- > ui/dbus-listener.c | 417 +++++++++++++++++++++++++++---------- > ui/dbus.c | 22 -- > ui/dbus.h | 36 +++- > ui/egl-context.c | 6 +- > ui/egl-headless.c | 20 +- > ui/gtk-egl.c | 10 +- > ui/gtk-gl-area.c | 8 +- > ui/gtk.c | 25 +-- > ui/sdl2-gl.c | 10 +- > ui/sdl2.c | 14 +- > ui/spice-display.c | 19 +- > 18 files changed, 498 insertions(+), 541 deletions(-) > > -- > 2.32.0 (Apple Git-132) > > > -- Marc-André Lureau