Mir

Code review comment for lp:~kdub/mir/display-groups

Revision history for this message
Kevin DuBois (kdub) wrote :

> I'd rather see a "MultiDisplayBuffer" or "EverythingDisplayBuffer" that
> composites multiple outputs together with a single "post" for Android. Then
> DisplayBufferCompositor doesn't have to change.
>

I would too, and this is mostly what we get with mesa's OverlappingOutputGroup. Multiple outputs are implemented as one display buffer if they overlap. The reason this is problematic is that android really needs one EGL context per buffer, and cannot set scanout regions as mesa can. For example, with two different display contexts in one DisplayBuffer, mg::DisplayBuffer::make_current() can't know which of the two contexts the user intends to make current. (or swap, etc.)

> The currently proposed design isn't just undesirable for mesa/DRM, but
> undesirable for any raw platform that flips/blits on post. All our future
> platforms will also want to avoid DisplayGroups so that the non-primary
> monitors don't tear or skip frames. So I think it's a better idea to invest in
> making this more clearly Android-specific.

I'd rather see two DisplayBuffers (each modelling content), with a DisplayGroup (which can post to screen) than have a function that mentions that one of our platforms is android, and special steps have to be taken with the threads. Its better to have a universal concept with synchronization implications, than it is to point to one platform and say that its weird in light of another platform. After all, the point of having the platform abstraction is so that the server code doesn't have to be written on a platform-by-platform basis.

> I can only imagine this works at all on Android without tearing or skipping
> (does it?) if Android is abstracting and asynchronously deferring the page
> flipping in the background.

Yes, the kernel helps out to avoid tearing. We're guaranteed that the set() function won't block, so its more that we're scheduling work with the driver and the kernel than controlling the timing of the threads.

« Back to merge proposal