mir_render_surface_get_buffer_stream

[Christopher James Halse Rogers]

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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.ubuntu.com/archives/mir-devel/attachments/20161201/9a3f977e/attachment.html>