Skip to content

Commit 5f8b842

Browse files
AlanGriffithsAlanGriffiths
authored
[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
2 parents 317060e + bb4bca6 commit 5f8b842

File tree

3 files changed

+137
-76
lines changed

3 files changed

+137
-76
lines changed

‎src/platforms/wayland/displayclient.cpp

+17-23
Original file line numberDiff line numberDiff line change
@@ -66,10 +66,6 @@ class mgw::DisplayClient::Output :
6666
xdg_surface* shell_surface{nullptr};
6767
xdg_toplevel* shell_toplevel{nullptr};
6868

69-
EGLContext eglctx{EGL_NO_CONTEXT};
70-
wl_egl_window* egl_window{nullptr};
71-
EGLSurface eglsurface{EGL_NO_SURFACE};
72-
7369
std::optional<geometry::Size> pending_toplevel_size;
7470
bool has_initialized{false};
7571

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

106102
private:
103+
std::mutex mutex;
107104
std::unique_ptr<WlDisplayAllocator::Framebuffer> next_frame;
108105
std::shared_ptr<WlDisplayAllocator> provider;
109106
};
@@ -159,11 +156,6 @@ mgw::DisplayClient::Output::~Output()
159156
}
160157

161158
wl_surface_destroy(surface);
162-
163-
if (egl_window != nullptr)
164-
{
165-
wl_egl_window_destroy(egl_window);
166-
}
167159
}
168160

169161
void mgw::DisplayClient::Output::geometry(
@@ -305,20 +297,17 @@ void mgw::DisplayClient::Output::surface_configure(uint32_t serial)
305297
dcout.custom_logical_size = pending_toplevel_size.value();
306298
pending_toplevel_size.reset();
307299
output_size = dcout.extents().size;
308-
if (!has_initialized)
300+
if (!has_initialized || size_is_changed)
309301
{
310302
has_initialized = true;
311-
provider = std::make_shared<WlDisplayAllocator>(
312-
owner_->provider->get_egl_display(),
313-
surface,
314-
output_size);
315-
}
316-
else if (size_is_changed)
317-
{
318-
/* TODO: We should, again, handle this by storing the pending size, raising a hardware-changed
319-
* notification, and then letting the `configure()` system tear down everything and bring it back
320-
* up at the new size.
321-
*/
303+
{
304+
std::lock_guard lock{mutex};
305+
provider = std::make_shared<WlDisplayAllocator>(
306+
owner_->provider->get_egl_display(),
307+
surface,
308+
output_size);
309+
}
310+
owner_->on_display_config_changed();
322311
}
323312
}
324313
}
@@ -383,8 +372,10 @@ void mgw::DisplayClient::Output::post()
383372
});
384373

385374
// The Framebuffer ensures that this swap_buffers call doesn't block...
386-
next_frame->swap_buffers();
387-
375+
{
376+
std::lock_guard lock{mutex};
377+
next_frame->swap_buffers();
378+
}
388379
// ...so we need external synchronisation to throttle rendering.
389380
// Wait for the host compositor to tell us to render.
390381
frame_sync->wait_for_done();
@@ -418,6 +409,8 @@ auto mgw::DisplayClient::Output::maybe_create_allocator(DisplayAllocator::Tag co
418409
{
419410
BOOST_THROW_EXCEPTION((std::runtime_error{"Attempted to create allocator before Output is fully initialised"}));
420411
}
412+
413+
std::lock_guard lock{mutex};
421414
return provider.get();
422415
}
423416
return nullptr;
@@ -443,6 +436,7 @@ void mgw::DisplayClient::Output::set_next_image(std::unique_ptr<Framebuffer> con
443436
{
444437
if (auto wl_content = unique_ptr_cast<WlDisplayAllocator::Framebuffer>(std::move(content)))
445438
{
439+
std::lock_guard lock{mutex};
446440
next_frame = std::move(wl_content);
447441
}
448442
else

‎src/platforms/wayland/wl_egl_display_provider.cpp

+115-45
Original file line numberDiff line numberDiff line change
@@ -12,47 +12,133 @@ namespace mg = mir::graphics;
1212
namespace mgw = mir::graphics::wayland;
1313
namespace geom = mir::geometry;
1414

15+
namespace
16+
{
17+
// Utility to restore EGL state on scope exit
18+
class CacheEglState
19+
{
20+
public:
21+
CacheEglState() = default;
22+
23+
~CacheEglState()
24+
{
25+
eglMakeCurrent(dpy, draw_surf, read_surf, ctx);
26+
}
27+
private:
28+
CacheEglState(CacheEglState const&) = delete;
29+
EGLDisplay const dpy = eglGetCurrentDisplay();
30+
EGLContext const ctx = eglGetCurrentContext();
31+
EGLSurface const draw_surf = eglGetCurrentSurface(EGL_DRAW);
32+
EGLSurface const read_surf = eglGetCurrentSurface(EGL_READ);
33+
};
34+
}
35+
36+
class mgw::WlDisplayAllocator::SurfaceState
37+
{
38+
public:
39+
SurfaceState(EGLDisplay dpy, struct ::wl_egl_window* wl_window) :
40+
dpy{dpy}, wl_window{wl_window} {}
41+
42+
~SurfaceState()
43+
{
44+
if (egl_surface != EGL_NO_SURFACE) eglDestroySurface(dpy, egl_surface);
45+
wl_egl_window_destroy(wl_window);
46+
}
47+
48+
auto surface(EGLConfig egl_config) -> EGLSurface
49+
{
50+
std::lock_guard lock{mutex};
51+
if (egl_surface == EGL_NO_SURFACE)
52+
{
53+
egl_surface = eglCreatePlatformWindowSurface(dpy, egl_config, wl_window, nullptr);
54+
}
55+
56+
if (egl_surface == EGL_NO_SURFACE)
57+
{
58+
BOOST_THROW_EXCEPTION(egl_error("Failed to create EGL surface"));
59+
}
60+
61+
return egl_surface;
62+
}
63+
64+
private:
65+
EGLDisplay const dpy;
66+
struct ::wl_egl_window* const wl_window;
67+
68+
std::mutex mutex;
69+
EGLSurface egl_surface{EGL_NO_SURFACE};
70+
};
71+
1572
class mgw::WlDisplayAllocator::Framebuffer::EGLState
1673
{
1774
public:
18-
EGLState(EGLDisplay dpy, EGLContext ctx, EGLSurface surf)
75+
EGLState(EGLDisplay dpy, EGLContext ctx, EGLSurface surf, std::shared_ptr<SurfaceState> ss)
1976
: dpy{dpy},
2077
ctx{ctx},
21-
surf{surf}
78+
surf{surf},
79+
ss{std::move(ss)}
2280
{
81+
CacheEglState stash;
82+
83+
make_current();
84+
// Don't block in eglSwapBuffers; we rely on external synchronisation to throttle rendering
85+
eglSwapInterval(dpy, 0);
86+
release_current();
2387
}
2488

2589
~EGLState()
2690
{
27-
eglMakeCurrent(dpy, EGL_NO_SURFACE, EGL_NO_SURFACE, EGL_NO_CONTEXT);
91+
if (ctx == eglGetCurrentContext())
92+
{
93+
eglMakeCurrent(dpy, EGL_NO_SURFACE, EGL_NO_SURFACE, EGL_NO_CONTEXT);
94+
}
2895
eglDestroyContext(dpy, ctx);
29-
eglDestroySurface(dpy, surf);
3096
}
31-
97+
98+
void make_current() const
99+
{
100+
if (eglMakeCurrent(dpy, surf, surf, ctx) != EGL_TRUE)
101+
{
102+
BOOST_THROW_EXCEPTION((mg::egl_error("eglMakeCurrent failed")));
103+
}
104+
}
105+
106+
void release_current() const
107+
{
108+
if (eglMakeCurrent(dpy, EGL_NO_SURFACE, EGL_NO_SURFACE, EGL_NO_CONTEXT) != EGL_TRUE)
109+
{
110+
BOOST_THROW_EXCEPTION(mg::egl_error("Failed to release EGL context"));
111+
}
112+
}
113+
114+
void swap_buffers() const
115+
{
116+
if (eglSwapBuffers(dpy, surf) != EGL_TRUE)
117+
{
118+
BOOST_THROW_EXCEPTION((mg::egl_error("eglSwapBuffers failed")));
119+
}
120+
}
121+
32122
EGLDisplay const dpy;
33123
EGLContext const ctx;
34124
EGLSurface const surf;
125+
std::shared_ptr<SurfaceState> const ss;
35126
};
36127

37-
mgw::WlDisplayAllocator::Framebuffer::Framebuffer(EGLDisplay dpy, EGLContext ctx, EGLSurface surf, geom::Size size)
38-
: Framebuffer(std::make_shared<EGLState>(dpy, ctx, surf), size)
128+
mgw::WlDisplayAllocator::Framebuffer::Framebuffer(
129+
EGLDisplay dpy, EGLContext ctx, EGLSurface surf, std::shared_ptr<SurfaceState> ss, mir::geometry::Size size) :
130+
Framebuffer{std::make_shared<EGLState>(dpy, ctx, surf, ss), size}
39131
{
40-
auto current_ctx = eglGetCurrentContext();
41-
auto current_draw_surf = eglGetCurrentSurface(EGL_DRAW);
42-
auto current_read_surf = eglGetCurrentSurface(EGL_READ);
43-
44-
make_current();
45-
// Don't block in eglSwapBuffers; we rely on external synchronisation to throttle rendering
46-
eglSwapInterval(dpy, 0);
47-
release_current();
48-
49-
// Paranoia: Restore the previous EGL context state, just in case
50-
eglMakeCurrent(dpy, current_draw_surf, current_read_surf, current_ctx);
51132
}
52133

53134
mgw::WlDisplayAllocator::Framebuffer::Framebuffer(std::shared_ptr<EGLState const> state, geom::Size size)
54-
: state{std::move(state)},
55-
size_{size}
135+
: state{std::move(state)}, size_{size}
136+
{
137+
}
138+
139+
mgw::WlDisplayAllocator::Framebuffer::Framebuffer(Framebuffer const& that)
140+
: state{that.state},
141+
size_{that.size_}
56142
{
57143
}
58144

@@ -63,46 +149,36 @@ auto mgw::WlDisplayAllocator::Framebuffer::size() const -> geom::Size
63149

64150
void mgw::WlDisplayAllocator::Framebuffer::make_current()
65151
{
66-
if (eglMakeCurrent(state->dpy, state->surf, state->surf, state->ctx) != EGL_TRUE)
67-
{
68-
BOOST_THROW_EXCEPTION((mg::egl_error("eglMakeCurrent failed")));
69-
}
152+
state->make_current();
70153
}
71154

72155
void mgw::WlDisplayAllocator::Framebuffer::release_current()
73156
{
74-
if (eglMakeCurrent(state->dpy, EGL_NO_SURFACE, EGL_NO_SURFACE, EGL_NO_CONTEXT) != EGL_TRUE)
75-
{
76-
BOOST_THROW_EXCEPTION(mg::egl_error("Failed to release EGL context"));
77-
}
157+
state->release_current();
78158
}
79159

80160
void mgw::WlDisplayAllocator::Framebuffer::swap_buffers()
81161
{
82-
if (eglSwapBuffers(state->dpy, state->surf) != EGL_TRUE)
83-
{
84-
BOOST_THROW_EXCEPTION((mg::egl_error("eglSwapBuffers failed")));
85-
}
162+
state->swap_buffers();
86163
}
87164

88165
auto mgw::WlDisplayAllocator::Framebuffer::clone_handle() -> std::unique_ptr<mg::GenericEGLDisplayAllocator::EGLFramebuffer>
89166
{
90-
return std::unique_ptr<mg::GenericEGLDisplayAllocator::EGLFramebuffer>{new Framebuffer(state, size_)};
167+
return std::unique_ptr<mg::GenericEGLDisplayAllocator::EGLFramebuffer>{new Framebuffer(*this)};
91168
}
92169

93170
mgw::WlDisplayAllocator::WlDisplayAllocator(
94171
EGLDisplay dpy,
95172
struct wl_surface* surface,
96173
geometry::Size size)
97174
: dpy{dpy},
98-
wl_window{wl_egl_window_create(surface, size.width.as_int(), size.height.as_int())},
175+
surface_state{std::make_shared<SurfaceState>(dpy, wl_egl_window_create(surface, size.width.as_int(), size.height.as_int()))},
99176
size{size}
100177
{
101178
}
102179

103180
mgw::WlDisplayAllocator::~WlDisplayAllocator()
104181
{
105-
wl_egl_window_destroy(wl_window);
106182
}
107183

108184
auto mgw::WlDisplayAllocator::alloc_framebuffer(
@@ -138,19 +214,13 @@ auto mgw::WlDisplayAllocator::alloc_framebuffer(
138214

139215
auto egl_context = eglCreateContext(dpy, egl_config, share_context, context_attr);
140216
if (egl_context == EGL_NO_CONTEXT)
141-
BOOST_THROW_EXCEPTION(egl_error("Failed to create EGL context"));
142-
143-
auto surf = eglCreatePlatformWindowSurface(dpy, egl_config, wl_window, nullptr);
144-
if (surf == EGL_NO_SURFACE)
145217
{
146-
BOOST_THROW_EXCEPTION(egl_error("Failed to create EGL surface"));
218+
BOOST_THROW_EXCEPTION(egl_error("Failed to create EGL context"));
147219
}
148220

149-
return std::make_unique<Framebuffer>(
150-
dpy,
151-
egl_context,
152-
surf,
153-
size);
221+
auto surface = surface_state->surface(egl_config);
222+
223+
return std::make_unique<Framebuffer>(dpy, egl_context, surface, surface_state, size);
154224
}
155225

156226
mgw::WlDisplayProvider::WlDisplayProvider(EGLDisplay dpy)

‎src/platforms/wayland/wl_egl_display_provider.h

+5-8
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,6 @@ class WlDisplayProvider : public GenericEGLDisplayProvider
3131
public:
3232
WlDisplayProvider(EGLDisplay dpy);
3333

34-
WlDisplayProvider(
35-
WlDisplayProvider const& from,
36-
struct wl_surface* surface,
37-
geometry::Size size);
38-
3934
auto get_egl_display() -> EGLDisplay override;
4035
private:
4136
EGLDisplay const dpy;
@@ -50,6 +45,7 @@ class WlDisplayAllocator : public GenericEGLDisplayAllocator
5045
auto alloc_framebuffer(GLConfig const& config, EGLContext share_context)
5146
-> std::unique_ptr<EGLFramebuffer> override;
5247

48+
struct SurfaceState;
5349
class Framebuffer : public GenericEGLDisplayAllocator::EGLFramebuffer
5450
{
5551
public:
@@ -60,7 +56,7 @@ class WlDisplayAllocator : public GenericEGLDisplayAllocator
6056
* final handle generated from this Framebuffer is released,
6157
* the EGL resources \param ctx and \param surff will be freed.
6258
*/
63-
Framebuffer(EGLDisplay dpy, EGLContext ctx, EGLSurface surf, geometry::Size size);
59+
Framebuffer(EGLDisplay dpy, EGLContext ctx, EGLSurface surf, std::shared_ptr<SurfaceState> ss, geometry::Size size);
6460

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

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

7673
std::shared_ptr<EGLState const> const state;
7774
geometry::Size const size_;
7875
};
7976
private:
8077
EGLDisplay const dpy;
81-
struct ::wl_egl_window* const wl_window;
78+
std::shared_ptr<SurfaceState> const surface_state;
8279
geometry::Size const size;
8380
};
8481
}

0 commit comments

Comments
 (0)