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

Reply via email to