mir_render_surface_get_buffer_stream

[Daniel van Vugt]

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.
>
>