Mir

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

Revision history for this message
Chris Halse Rogers (raof) wrote :

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

This might be our disconnect. Everything does not (and *cannot*) rally around MirPresentationChain. Specifically, the nvidia driver cannot support hardware-backed MirPresentationChain, and requires an EGLStream-backed MirRenderSurface.

I think MirBufferStream will be sufficiently useful to (software) clients for it to stick around, but even if we change the API to make its helper-code-on-top-of-MirPresentationChain nature explicit we still need to have PresentationChain-backed RenderSurfaces and EGLStream-backed RenderSurfaces.

Since MirPresentationChain cannot be the generic content abstraction, I think it makes sense to have a distinguished MirRenderSurface as the generic content abstraction.

> A system of binding would also work, but might be too late to change too much of the naming
> given release?

I don't think any of the current API has actually made it into a release yet, and *if* we think the bind-to-sink API is significantly better it'd be sensible to make the change before we do have a release with the API public.

I don't think that a binding API *is* significantly better, but others may have different opinions.

« Back to merge proposal