Tangentially, I'm still hanging out for a rename before mir_render_surface_* becomes a public API.

MirRenderSurface I hope will get a shorter name that closer to a one word noun. Although the ideal of making it MirSurface after the existing MirSurface becomes MirWindow is quite ambitious.


On 01/12/16 10:35, Christopher James Halse Rogers wrote:
So, client-eventloop-driven MirConnection is getting really close to
working properly.

The latest problem I've run into is mir_render_surface_get_buffer_stream
- this purports to be a no-RPC call and so doesn't take a callback/have
a _sync version, but the mcl::BufferStream constructor *does* perform
hidden (poorly¹) RPC.

That's easy enough; just change it to take a callback & add a _sync version.

But looking at it I've got a second concern.
mir_render_surface_get_buffer_stream takes a whole bunch of parameters -
width, height, pixel format, buffer_usage. However, these are only used
in the *first* call to get_buffer_stream; they're silently ignored in
any subsequent call.

This seems like an API with an unnecessary hidden razorblade - the
specifics of the buffer stream returned from this depend on whether or
not the function had previously been called, which seems likely to lead
to difficult-to-debug problems for clients.

I think we should disallow multiple calls to
mir_render_surface_get_buffer_stream². It means that client code needs
to be slightly more entangled - any time code might need to modify both
the RenderSurface and the BufferStream it'll need to be provided with
both rather than just the RenderSurface. This won't result in tighter
coupling of the client code, though, because the code is *currently*
coupled. The API just doesn't make it clear.

Thoughts?

¹: If you use the buffer stream at the wrong time you'll deadlock.
²: For the same RenderSurface, obviously.



--
Mir-devel mailing list
Mir-devel@lists.ubuntu.com
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/mir-devel

Reply via email to