Skip to content

Commit

Permalink
[mir:wayland] Fix handling of display unplug/replug (#3433)
Browse files Browse the repository at this point in the history 
Fixes: #3427

This is mostly about managing the lifetime of EGL entities correctly,
but cleaned up some threading issues along the way
  • Loading branch information
@AlanGriffiths @Saviq
AlanGriffiths authored and Saviq committed Aug 8, 2024
1 parent 994e25e commit 6f4dab4
Show file tree
Hide file tree
Showing 3 changed files with 137 additions and 76 deletions.
40 changes: 17 additions & 23 deletions src/platforms/wayland/displayclient.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<geometry::Size> pending_toplevel_size;
bool has_initialized{false};

Expand Down Expand Up @@ -104,6 +100,7 @@ class mgw::DisplayClient::Output :
void set_next_image(std::unique_ptr<Framebuffer> content) override;

private:
std::mutex mutex;
std::unique_ptr<WlDisplayAllocator::Framebuffer> next_frame;
std::shared_ptr<WlDisplayAllocator> provider;
};
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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<WlDisplayAllocator>(
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<WlDisplayAllocator>(
owner_->provider->get_egl_display(),
surface,
output_size);
}
owner_->on_display_config_changed();
}
}
}
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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;
Expand All @@ -443,6 +436,7 @@ void mgw::DisplayClient::Output::set_next_image(std::unique_ptr<Framebuffer> con
{
if (auto wl_content = unique_ptr_cast<WlDisplayAllocator::Framebuffer>(std::move(content)))
{
std::lock_guard lock{mutex};
next_frame = std::move(wl_content);
}
else
Expand Down
160 changes: 115 additions & 45 deletions src/platforms/wayland/wl_egl_display_provider.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<SurfaceState> 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<SurfaceState> const ss;
};

mgw::WlDisplayAllocator::Framebuffer::Framebuffer(EGLDisplay dpy, EGLContext ctx, EGLSurface surf, geom::Size size)
: Framebuffer(std::make_shared<EGLState>(dpy, ctx, surf), size)
mgw::WlDisplayAllocator::Framebuffer::Framebuffer(
EGLDisplay dpy, EGLContext ctx, EGLSurface surf, std::shared_ptr<SurfaceState> ss, mir::geometry::Size size) :
Framebuffer{std::make_shared<EGLState>(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<EGLState const> 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_}
{
}

Expand All @@ -63,46 +149,36 @@ 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<mg::GenericEGLDisplayAllocator::EGLFramebuffer>
{
return std::unique_ptr<mg::GenericEGLDisplayAllocator::EGLFramebuffer>{new Framebuffer(state, size_)};
return std::unique_ptr<mg::GenericEGLDisplayAllocator::EGLFramebuffer>{new Framebuffer(*this)};
}

mgw::WlDisplayAllocator::WlDisplayAllocator(
EGLDisplay dpy,
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<SurfaceState>(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(
Expand Down Expand Up @@ -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<Framebuffer>(
dpy,
egl_context,
surf,
size);
auto surface = surface_state->surface(egl_config);

return std::make_unique<Framebuffer>(dpy, egl_context, surface, surface_state, size);
}

mgw::WlDisplayProvider::WlDisplayProvider(EGLDisplay dpy)
Expand Down
13 changes: 5 additions & 8 deletions src/platforms/wayland/wl_egl_display_provider.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -50,6 +45,7 @@ class WlDisplayAllocator : public GenericEGLDisplayAllocator
auto alloc_framebuffer(GLConfig const& config, EGLContext share_context)
-> std::unique_ptr<EGLFramebuffer> override;

struct SurfaceState;
class Framebuffer : public GenericEGLDisplayAllocator::EGLFramebuffer
{
public:
Expand All @@ -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<SurfaceState> ss, geometry::Size size);

auto size() const -> geometry::Size override;

Expand All @@ -71,14 +67,15 @@ class WlDisplayAllocator : public GenericEGLDisplayAllocator
void swap_buffers();
private:
class EGLState;
Framebuffer(std::shared_ptr<EGLState const> surf, geometry::Size size);
Framebuffer(std::shared_ptr<EGLState const> state, geometry::Size size);
Framebuffer(Framebuffer const& that);

std::shared_ptr<EGLState const> const state;
geometry::Size const size_;
};
private:
EGLDisplay const dpy;
struct ::wl_egl_window* const wl_window;
std::shared_ptr<SurfaceState> const surface_state;
geometry::Size const size;
};
}
Expand Down

0 comments on commit 6f4dab4

Please sign in to comment.