Code review comment for lp:~mir-team/mir/chain-simplification

> Weak disapprove.
>
> So, I think this is less typing in the uncommon case in return for higher
> complexity everywhere.
>
> I don't think having more types makes the code more complex. I think having
> more types, each of which is conceptually atomic, makes the code *less*
> complex because the type gives you more information.
>
> This makes the MirRenderSurface type more complex - it's no longer a generic
> handle to some sort of rendered content, it's now *sometimes* a generic handle
> to some sort of rendered content and *sometimes* a specific thing you can
> submit buffers to. You can't tell how it's being used without reading the
> code.

I agree that having more types reduces complexity (in terms of how complicated it is for a user to grasp the concept behind the type). If the types are conceptually different, then they need different types.

I think though that MirPresentationChain and MirRenderSurface are the same thing. They're the generic content. Its the "Surface".

MirBufferStream is pretty useless, if you discount legacy usage. Its built on top of MirBuffer+MirPresentationChain internally, and I see it as a helper class that we provide.

If MirBufferStream is useless/legacy, and everything wants to rally around MirPresentationChain [1], then I don't know if there's any value in segregating the parts the driver wants to use (MirPresentationChain) from the parts that the user uses (MirRenderSurface).

The added benefit for the user is that we don't have to construct a MirPresentationChain from a MirRenderSurface, and we don't have to explain externally why the user has to call a function to switch a MirRenderSurface to a MirPresentationChain.

I can get used to the way it is (so obviously won't be offended if MP is rejected), but still think that reducing the number of types and treating MirBufferStream as legacy makes for cleaner code for the user and driver to use.

[1]
MirPresentationChain needs some functions added to it to support the other 3 Vulkan submission modes, and perhaps multimedia-specific submissions (timestamps), but those can be done post 1.0 without ABI breakage.

>
> If we really want to rearrange how MirRenderSurface works we could call it
> MirContentSinkId or something, re-introduce the
> mir_connection_create_{buffer_stream,presentation_chain,eglstream}, and add
> mir_{buffer_stream,presentation_chain,eglstream}_bind_id($OBJECT,
> MirContentSinkId id).
>
A system of binding would also work, but might be too late to change too much of the naming given release?

> Finally, this is optimising for the uncommon case. The majority of code that
> interacts with MirRenderSurface will just pass it to an EGL implementation¹
> and go to town. The code that *doesn't* just pass it to an EGL implementation
> is going to associate it with a software-backed MirBufferStream and render to
> that.
>
> This reduces the length of (unrepresentative) example code and driver code,
> but does not change expected real-world code.

It does save a type conversion and a concern about how to clean up MirPresentationChain in the driver code.

> ¹: Or other hardware abstraction layer, like GStreamer.