From 6f4dab4b6530a91f3d062d8b1a3ae11879a1f0c2 Mon Sep 17 00:00:00 2001 From: Alan Griffiths Date: Tue, 2 Jul 2024 13:25:45 +0000 Subject: [PATCH] [mir:wayland] Fix handling of display unplug/replug (#3433) Fixes: #3427 This is mostly about managing the lifetime of EGL entities correctly, but cleaned up some threading issues along the way --- src/platforms/wayland/displayclient.cpp | 40 ++--- .../wayland/wl_egl_display_provider.cpp | 160 +++++++++++++----- .../wayland/wl_egl_display_provider.h | 13 +- 3 files changed, 137 insertions(+), 76 deletions(-) diff --git a/src/platforms/wayland/displayclient.cpp b/src/platforms/wayland/displayclient.cpp index 9f4f13bce48..c8089af2a4f 100644 --- a/src/platforms/wayland/displayclient.cpp +++ b/src/platforms/wayland/displayclient.cpp @@ -66,10 +66,6 @@ class mgw::DisplayClient::Output : xdg_surface* shell_surface{nullptr}; xdg_toplevel* shell_toplevel{nullptr}; - EGLContext eglctx{EGL_NO_CONTEXT}; - wl_egl_window* egl_window{nullptr}; - EGLSurface eglsurface{EGL_NO_SURFACE}; - std::optional pending_toplevel_size; bool has_initialized{false}; @@ -104,6 +100,7 @@ class mgw::DisplayClient::Output : void set_next_image(std::unique_ptr content) override; private: + std::mutex mutex; std::unique_ptr next_frame; std::shared_ptr provider; }; @@ -159,11 +156,6 @@ mgw::DisplayClient::Output::~Output() } wl_surface_destroy(surface); - - if (egl_window != nullptr) - { - wl_egl_window_destroy(egl_window); - } } void mgw::DisplayClient::Output::geometry( @@ -305,20 +297,17 @@ void mgw::DisplayClient::Output::surface_configure(uint32_t serial) dcout.custom_logical_size = pending_toplevel_size.value(); pending_toplevel_size.reset(); output_size = dcout.extents().size; - if (!has_initialized) + if (!has_initialized || size_is_changed) { has_initialized = true; - provider = std::make_shared( - owner_->provider->get_egl_display(), - surface, - output_size); - } - else if (size_is_changed) - { - /* TODO: We should, again, handle this by storing the pending size, raising a hardware-changed - * notification, and then letting the `configure()` system tear down everything and bring it back - * up at the new size. - */ + { + std::lock_guard lock{mutex}; + provider = std::make_shared( + owner_->provider->get_egl_display(), + surface, + output_size); + } + owner_->on_display_config_changed(); } } } @@ -383,8 +372,10 @@ void mgw::DisplayClient::Output::post() }); // The Framebuffer ensures that this swap_buffers call doesn't block... - next_frame->swap_buffers(); - + { + std::lock_guard lock{mutex}; + next_frame->swap_buffers(); + } // ...so we need external synchronisation to throttle rendering. // Wait for the host compositor to tell us to render. frame_sync->wait_for_done(); @@ -418,6 +409,8 @@ auto mgw::DisplayClient::Output::maybe_create_allocator(DisplayAllocator::Tag co { BOOST_THROW_EXCEPTION((std::runtime_error{"Attempted to create allocator before Output is fully initialised"})); } + + std::lock_guard lock{mutex}; return provider.get(); } return nullptr; @@ -443,6 +436,7 @@ void mgw::DisplayClient::Output::set_next_image(std::unique_ptr con { if (auto wl_content = unique_ptr_cast(std::move(content))) { + std::lock_guard lock{mutex}; next_frame = std::move(wl_content); } else diff --git a/src/platforms/wayland/wl_egl_display_provider.cpp b/src/platforms/wayland/wl_egl_display_provider.cpp index a4803c29e70..8078eca714e 100644 --- a/src/platforms/wayland/wl_egl_display_provider.cpp +++ b/src/platforms/wayland/wl_egl_display_provider.cpp @@ -12,47 +12,133 @@ namespace mg = mir::graphics; namespace mgw = mir::graphics::wayland; namespace geom = mir::geometry; +namespace +{ +// Utility to restore EGL state on scope exit +class CacheEglState +{ +public: + CacheEglState() = default; + + ~CacheEglState() + { + eglMakeCurrent(dpy, draw_surf, read_surf, ctx); + } +private: + CacheEglState(CacheEglState const&) = delete; + EGLDisplay const dpy = eglGetCurrentDisplay(); + EGLContext const ctx = eglGetCurrentContext(); + EGLSurface const draw_surf = eglGetCurrentSurface(EGL_DRAW); + EGLSurface const read_surf = eglGetCurrentSurface(EGL_READ); +}; +} + +class mgw::WlDisplayAllocator::SurfaceState +{ +public: + SurfaceState(EGLDisplay dpy, struct ::wl_egl_window* wl_window) : + dpy{dpy}, wl_window{wl_window} {} + + ~SurfaceState() + { + if (egl_surface != EGL_NO_SURFACE) eglDestroySurface(dpy, egl_surface); + wl_egl_window_destroy(wl_window); + } + + auto surface(EGLConfig egl_config) -> EGLSurface + { + std::lock_guard lock{mutex}; + if (egl_surface == EGL_NO_SURFACE) + { + egl_surface = eglCreatePlatformWindowSurface(dpy, egl_config, wl_window, nullptr); + } + + if (egl_surface == EGL_NO_SURFACE) + { + BOOST_THROW_EXCEPTION(egl_error("Failed to create EGL surface")); + } + + return egl_surface; + } + +private: + EGLDisplay const dpy; + struct ::wl_egl_window* const wl_window; + + std::mutex mutex; + EGLSurface egl_surface{EGL_NO_SURFACE}; +}; + class mgw::WlDisplayAllocator::Framebuffer::EGLState { public: - EGLState(EGLDisplay dpy, EGLContext ctx, EGLSurface surf) + EGLState(EGLDisplay dpy, EGLContext ctx, EGLSurface surf, std::shared_ptr ss) : dpy{dpy}, ctx{ctx}, - surf{surf} + surf{surf}, + ss{std::move(ss)} { + CacheEglState stash; + + make_current(); + // Don't block in eglSwapBuffers; we rely on external synchronisation to throttle rendering + eglSwapInterval(dpy, 0); + release_current(); } ~EGLState() { - eglMakeCurrent(dpy, EGL_NO_SURFACE, EGL_NO_SURFACE, EGL_NO_CONTEXT); + if (ctx == eglGetCurrentContext()) + { + eglMakeCurrent(dpy, EGL_NO_SURFACE, EGL_NO_SURFACE, EGL_NO_CONTEXT); + } eglDestroyContext(dpy, ctx); - eglDestroySurface(dpy, surf); } - + + void make_current() const + { + if (eglMakeCurrent(dpy, surf, surf, ctx) != EGL_TRUE) + { + BOOST_THROW_EXCEPTION((mg::egl_error("eglMakeCurrent failed"))); + } + } + + void release_current() const + { + if (eglMakeCurrent(dpy, EGL_NO_SURFACE, EGL_NO_SURFACE, EGL_NO_CONTEXT) != EGL_TRUE) + { + BOOST_THROW_EXCEPTION(mg::egl_error("Failed to release EGL context")); + } + } + + void swap_buffers() const + { + if (eglSwapBuffers(dpy, surf) != EGL_TRUE) + { + BOOST_THROW_EXCEPTION((mg::egl_error("eglSwapBuffers failed"))); + } + } + EGLDisplay const dpy; EGLContext const ctx; EGLSurface const surf; + std::shared_ptr const ss; }; -mgw::WlDisplayAllocator::Framebuffer::Framebuffer(EGLDisplay dpy, EGLContext ctx, EGLSurface surf, geom::Size size) - : Framebuffer(std::make_shared(dpy, ctx, surf), size) +mgw::WlDisplayAllocator::Framebuffer::Framebuffer( + EGLDisplay dpy, EGLContext ctx, EGLSurface surf, std::shared_ptr ss, mir::geometry::Size size) : + Framebuffer{std::make_shared(dpy, ctx, surf, ss), size} { - auto current_ctx = eglGetCurrentContext(); - auto current_draw_surf = eglGetCurrentSurface(EGL_DRAW); - auto current_read_surf = eglGetCurrentSurface(EGL_READ); - - make_current(); - // Don't block in eglSwapBuffers; we rely on external synchronisation to throttle rendering - eglSwapInterval(dpy, 0); - release_current(); - - // Paranoia: Restore the previous EGL context state, just in case - eglMakeCurrent(dpy, current_draw_surf, current_read_surf, current_ctx); } mgw::WlDisplayAllocator::Framebuffer::Framebuffer(std::shared_ptr state, geom::Size size) - : state{std::move(state)}, - size_{size} + : state{std::move(state)}, size_{size} +{ +} + +mgw::WlDisplayAllocator::Framebuffer::Framebuffer(Framebuffer const& that) + : state{that.state}, + size_{that.size_} { } @@ -63,31 +149,22 @@ auto mgw::WlDisplayAllocator::Framebuffer::size() const -> geom::Size void mgw::WlDisplayAllocator::Framebuffer::make_current() { - if (eglMakeCurrent(state->dpy, state->surf, state->surf, state->ctx) != EGL_TRUE) - { - BOOST_THROW_EXCEPTION((mg::egl_error("eglMakeCurrent failed"))); - } + state->make_current(); } void mgw::WlDisplayAllocator::Framebuffer::release_current() { - if (eglMakeCurrent(state->dpy, EGL_NO_SURFACE, EGL_NO_SURFACE, EGL_NO_CONTEXT) != EGL_TRUE) - { - BOOST_THROW_EXCEPTION(mg::egl_error("Failed to release EGL context")); - } + state->release_current(); } void mgw::WlDisplayAllocator::Framebuffer::swap_buffers() { - if (eglSwapBuffers(state->dpy, state->surf) != EGL_TRUE) - { - BOOST_THROW_EXCEPTION((mg::egl_error("eglSwapBuffers failed"))); - } + state->swap_buffers(); } auto mgw::WlDisplayAllocator::Framebuffer::clone_handle() -> std::unique_ptr { - return std::unique_ptr{new Framebuffer(state, size_)}; + return std::unique_ptr{new Framebuffer(*this)}; } mgw::WlDisplayAllocator::WlDisplayAllocator( @@ -95,14 +172,13 @@ mgw::WlDisplayAllocator::WlDisplayAllocator( struct wl_surface* surface, geometry::Size size) : dpy{dpy}, - wl_window{wl_egl_window_create(surface, size.width.as_int(), size.height.as_int())}, + surface_state{std::make_shared(dpy, wl_egl_window_create(surface, size.width.as_int(), size.height.as_int()))}, size{size} { } mgw::WlDisplayAllocator::~WlDisplayAllocator() { - wl_egl_window_destroy(wl_window); } auto mgw::WlDisplayAllocator::alloc_framebuffer( @@ -138,19 +214,13 @@ auto mgw::WlDisplayAllocator::alloc_framebuffer( auto egl_context = eglCreateContext(dpy, egl_config, share_context, context_attr); if (egl_context == EGL_NO_CONTEXT) - BOOST_THROW_EXCEPTION(egl_error("Failed to create EGL context")); - - auto surf = eglCreatePlatformWindowSurface(dpy, egl_config, wl_window, nullptr); - if (surf == EGL_NO_SURFACE) { - BOOST_THROW_EXCEPTION(egl_error("Failed to create EGL surface")); + BOOST_THROW_EXCEPTION(egl_error("Failed to create EGL context")); } - return std::make_unique( - dpy, - egl_context, - surf, - size); + auto surface = surface_state->surface(egl_config); + + return std::make_unique(dpy, egl_context, surface, surface_state, size); } mgw::WlDisplayProvider::WlDisplayProvider(EGLDisplay dpy) diff --git a/src/platforms/wayland/wl_egl_display_provider.h b/src/platforms/wayland/wl_egl_display_provider.h index a3c92aaddd6..86349b17979 100644 --- a/src/platforms/wayland/wl_egl_display_provider.h +++ b/src/platforms/wayland/wl_egl_display_provider.h @@ -31,11 +31,6 @@ class WlDisplayProvider : public GenericEGLDisplayProvider public: WlDisplayProvider(EGLDisplay dpy); - WlDisplayProvider( - WlDisplayProvider const& from, - struct wl_surface* surface, - geometry::Size size); - auto get_egl_display() -> EGLDisplay override; private: EGLDisplay const dpy; @@ -50,6 +45,7 @@ class WlDisplayAllocator : public GenericEGLDisplayAllocator auto alloc_framebuffer(GLConfig const& config, EGLContext share_context) -> std::unique_ptr override; + struct SurfaceState; class Framebuffer : public GenericEGLDisplayAllocator::EGLFramebuffer { public: @@ -60,7 +56,7 @@ class WlDisplayAllocator : public GenericEGLDisplayAllocator * final handle generated from this Framebuffer is released, * the EGL resources \param ctx and \param surff will be freed. */ - Framebuffer(EGLDisplay dpy, EGLContext ctx, EGLSurface surf, geometry::Size size); + Framebuffer(EGLDisplay dpy, EGLContext ctx, EGLSurface surf, std::shared_ptr ss, geometry::Size size); auto size() const -> geometry::Size override; @@ -71,14 +67,15 @@ class WlDisplayAllocator : public GenericEGLDisplayAllocator void swap_buffers(); private: class EGLState; - Framebuffer(std::shared_ptr surf, geometry::Size size); + Framebuffer(std::shared_ptr state, geometry::Size size); + Framebuffer(Framebuffer const& that); std::shared_ptr const state; geometry::Size const size_; }; private: EGLDisplay const dpy; - struct ::wl_egl_window* const wl_window; + std::shared_ptr const surface_state; geometry::Size const size; }; }