Code review comment for lp:~raof/mir/waylanding-again

+class WaylandBuffer :
+ void gl_bind_to_texture() override
+ for (auto&& frame : frames)
+ {
+ auto framer = std::move(frame);
+ executor->spawn(
+ [frame = framer.release(), deleter = framer.get_deleter()]()
+ {
+ wl_callback_send_done(frame, 0);
+ wl_client_flush(wl_resource_get_client(frame));
+ deleter(frame);
+ });
+ }
+ frames.clear();

More a question of intent: if a client has no new frame to draw, are we doing wrong to delete the frame on the server? I would expect we only delete a frame when we have got a new one from the client.

Or instead do we tell the client, compositor has released the frame, and client can then specify that buffer be re-used? That seems a bit wasteful. /me trying to interpret the buffer exchange design from wayland.xml

It's the fact you need the Executor just to deal with frame release, that made me bring this up.

+ struct DestructionShim
+ {
+ std::shared_ptr<std::mutex> const mutex = std::make_shared<std::mutex>();
The way you're sharing this mutex between this and the WaylandBuffer is a bit peculiar looking, but I suspect unavoidable, as you need the mutex for the on_buffer_destroyed callback. You *could* use the void*, but what you're doing is indeed cleaner.

+ if (dpy == EGL_NO_DISPLAY)
+ BOOST_THROW_EXCEPTION((std::runtime_error{"Fuck"}));
Mir has been rated PG13 due to its low expletive, violence and sexual content, please don't jeopardise that. The kids are where its at for all that sweet merchandising profit.

+++ src/platforms/mesa/server/kms/platform.cpp
does this file need to change? I only see header files being added.

+++ src/protocol/wrapper_generator.cpp
Missing licence.

to be continued...