Mir

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

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

> (3) Even if you have multiple displays running at the same rate, they will be
> out of phase by half a frame on average, sometimes worse. So if you
> synchronize their post() calls then you're dooming all-but-one display to get
> a shorter render time than 16ms, and this will often leave insufficient render
> time to composite a whole frame without missing the next one. Our old
> workaround of triple buffering saves us here, but that's a step backwards in
> performance. So you have to choose between high lag or sometimes skipping
> frames. For this reason, I disapprove of the groups design.

I disapprove of the HWC multimonitor design, but we're stuck with it :).

Given that we really-truly-have-to synchronize and commit from one thread in android, I like DisplayGroups because it keeps the threading model implicit. The alternative is to have an explicit threading model (some sort of "force single threaded composition" flag) or to require the compositor writers to call strange functions that give the android code hints about when it can commit. [1] Both of these are somewhat vague to a compositor writer ("why do I have to call this?, or "what does 'force_single_threading' mean?")

Its also nice that we're splitting DisplayBuffer's concerns. DisplayBuffer is concerned about the content of the buffer, and DisplayGroup is concerned with issuing the commands to post. I chose the execute-around way of giving access to the DisplayBuffers, but down the road, we could probably even give ownership of the DB's.

The other nice thing is that we regain control of the posting, that is, the post() call that drives the display update is now called from MultiThreadedCompositor. The DisplayBufferCompositors are now just what we intended them to be; they are a place where the compositor writers can override to draw different content into the monitors.

[1] specifically, looking at lp:~kdub/mir/hwc-sync-and-post-external, you can see how with the existing model, I can't tell if the external display wants its content updated, so I just make the assumption that we're getting a 1-for-1 buffer update on both displays.

« Back to merge proposal