Mir

Merge lp:~kdub/mir/display-groups into lp:mir

Proposed by Kevin DuBois
Status: Merged
Approved by: Kevin DuBois
Approved revision: no longer in the source branch.
Merged at revision: 2337
Proposed branch: lp:~kdub/mir/display-groups
Merge into: lp:mir
Diff against target: 2071 lines (+552/-390)
53 files modified
benchmarks/frame-uniformity/vsync_simulating_graphics_platform.cpp (+22/-19)
examples/render_overlays.cpp (+9/-5)
examples/render_surfaces.cpp (+5/-2)
examples/render_to_fb.cpp (+14/-10)
include/platform/mir/graphics/display.h (+28/-2)
include/platform/mir/graphics/display_buffer.h (+0/-22)
playground/demo-shell/demo_compositor.cpp (+0/-2)
src/platforms/android/server/configurable_display_buffer.h (+4/-1)
src/platforms/android/server/display.cpp (+1/-1)
src/platforms/android/server/display.h (+1/-1)
src/platforms/android/server/display_buffer.cpp (+9/-4)
src/platforms/android/server/display_buffer.h (+3/-1)
src/platforms/mesa/server/display.cpp (+2/-2)
src/platforms/mesa/server/display.h (+2/-2)
src/platforms/mesa/server/display_buffer.cpp (+23/-17)
src/platforms/mesa/server/display_buffer.h (+9/-4)
src/server/compositor/default_display_buffer_compositor.cpp (+1/-3)
src/server/compositor/multi_threaded_compositor.cpp (+46/-40)
src/server/compositor/screencast_display_buffer.cpp (+0/-4)
src/server/compositor/screencast_display_buffer.h (+0/-1)
src/server/graphics/nested/nested_display.cpp (+24/-7)
src/server/graphics/nested/nested_display.h (+12/-2)
src/server/graphics/nested/nested_output.cpp (+0/-4)
src/server/graphics/nested/nested_output.h (+0/-1)
src/server/graphics/offscreen/display.cpp (+22/-6)
src/server/graphics/offscreen/display.h (+12/-2)
src/server/graphics/offscreen/display_buffer.cpp (+0/-4)
src/server/graphics/offscreen/display_buffer.h (+0/-1)
src/server/input/display_input_region.cpp (+16/-10)
src/server/shell/graphics_display_layout.cpp (+15/-13)
tests/acceptance-tests/test_display_configuration.cpp (+4/-4)
tests/include/mir_test_doubles/mock_display.h (+1/-1)
tests/include/mir_test_doubles/mock_display_buffer.h (+0/-1)
tests/include/mir_test_doubles/null_display.h (+4/-4)
tests/include/mir_test_doubles/null_display_buffer.h (+0/-1)
tests/include/mir_test_doubles/null_display_sync_group.h (+81/-0)
tests/include/mir_test_doubles/stub_display.h (+26/-12)
tests/integration-tests/graphics/android/test_display_integration.cpp (+27/-23)
tests/integration-tests/test_display_info.cpp (+4/-3)
tests/integration-tests/test_display_server_main_loop_events.cpp (+2/-2)
tests/integration-tests/test_surface_first_frame_sync.cpp (+21/-11)
tests/integration-tests/test_surface_stack_with_compositor.cpp (+15/-22)
tests/integration-tests/test_surfaceloop.cpp (+6/-5)
tests/unit-tests/compositor/test_default_display_buffer_compositor.cpp (+0/-10)
tests/unit-tests/compositor/test_multi_threaded_compositor.cpp (+23/-28)
tests/unit-tests/graphics/android/test_display.cpp (+14/-11)
tests/unit-tests/graphics/android/test_display_buffer.cpp (+0/-1)
tests/unit-tests/graphics/mesa/test_display.cpp (+14/-12)
tests/unit-tests/graphics/mesa/test_display_buffer.cpp (+4/-4)
tests/unit-tests/graphics/mesa/test_display_multi_monitor.cpp (+4/-6)
tests/unit-tests/graphics/offscreen/test_offscreen_display.cpp (+9/-9)
tests/unit-tests/graphics/test_display.cpp (+6/-2)
tests/unit-tests/input/test_display_input_region.cpp (+7/-25)
To merge this branch: bzr merge lp:~kdub/mir/display-groups
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
Daniel van Vugt Needs Fixing
Robert Carr (community) Abstain
Alberto Aguirre (community) Approve
Andreas Pokorny (community) Approve
Alan Griffiths Approve
Alexandros Frantzis (community) Approve
Review via email: mp+249084@code.launchpad.net

Commit message

graphics: move the post() command from the DisplayBuffers to the new DisplayGroup interface. This allows platforms to designate which DisplayBuffers must be posted in a common function.

Description of the change

graphics: move the post() command from the DisplayBuffers to the new DisplayGroup interface. This allows platforms to designate which DisplayBuffers must be posted in a common function.

This is mostly for android, which requires us to figure out the fencing for the displays, and post one nonblocking function (https://android.googlesource.com/platform/hardware/libhardware/+/master/include/hardware/hwcomposer.h l587). To rephrase android's requirements, we're required to post 2 egl contexts in one post command. (ie, have 1 displaygroup with 2 buffers in it)

Mesa/offscreen/nested remain with one thread per displaybuffer, although the idea of a group of displays posting together isn't foreign to mesa and nested. Particularly these platforms do this in the case where the outputs overlap and share a common scanout buffer. Unfortunately android really needs 2 egl contexts backed by 2 bundles of framebuffers, so it cannot play a similar trick.

It also has the nice benefit that the DisplayBufferCompositor implementations becomes concerned with the content (GL/overlays), and the Compositor implementations now issue the post() command.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

262 +class ConfigurableDisplayBuffer : public graphics::DisplayBuffer,
263 + public graphics::DisplayGroup
...
469 +class DisplayBuffer : public graphics::DisplayBuffer,
470 + public graphics::DisplayGroup

I don't feel I understand the relationship between graphics::DisplayBuffer and graphics::DisplayGroup.

My gut says there's a composite pattern hidden in there somewhere, but that the interfaces are not quite right.

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

> I don't feel I understand the relationship between graphics::DisplayBuffer and
> graphics::DisplayGroup.

DisplayGroups are DisplayBuffers that share a common post() function. The DisplayBuffer becomes an abstraction for the content of the buffer, and the DisplayGroup becomes the abstraction for how that content is flipped to the frontbuffer. In lp:mir right now, DisplayBuffer is the abstraction for the content, and the posting.

> My gut says there's a composite pattern hidden in there somewhere, but that
> the interfaces are not quite right.

For android, I was intending to split the inheritance in the next round of updates. For mesa, I was intending on leaving it, but since having to look at it, the code does become more clean if I switch mgm::DisplayBuffer from inheritance to to composition. I am working on disentangling this too, will propose mesa and android in subsequent MP's, as they're both about 1k lines each.

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

Okay, so I made mgm::DisplayBuffer inherit only from mg::DisplayBuffer, and introduced a mgm::DisplayGroup in this branch lp:~kdub/mir/mesa-display-group. I'll propose that after this, and am working on the android branch (which might have the MM changes).

Revision history for this message
Alexandros Frantzis (afrantzis) wrote :

173 + * This may wait for based on hardware conditions and platform requirements */

Something missing.

178 +private:

(Nit) No need for private, protected will do.

629 + for (auto& compositor : compositors)
630 + {
631 + target.ensure_current(std::get<0>(compositor));
632 + std::get<1>(compositor)->composite(scene->scene_elements_for(comp_id));

(Non-blocking) As discussed on IRC, this approach serializes rendering of multiple outputs on Android (i.e., it's R1,R2,...,Rn,Post vs the ideal Parallel(R1,R2,...,Rn),Post). This could lead to some performance loss for the multimonitor case on Android, so hopefully there is a way to improve this if needed.

1711 +// display->for_each_mock_buffer([](mtd::MockDisplayBuffer& mock_buf)
1712 +// {
1713 +// EXPECT_CALL(mock_buf, make_current()).Times(1);
1714 +// EXPECT_CALL(mock_buf, view_area())
1715 +// .WillOnce(Return(geom::Rectangle()));
1716 +// });

Not needed?

1769 +#if 0
1770 TEST(MultiThreadedCompositor, makes_and_releases_display_buffer_current_target)

?

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

fixed nits/tests I forgot to reenable.

As for the performance concern, thats true. Ideally two threads would draw, and then one thread would post. This is forced on us somewhat by the android api, although its mitigated by:
* Android (surfaceflinger) would have the same multimonitor performance hit, as it drives rendering from one thread as well.
* mesa, and the other platforms are still driven in sensible way we have it.
* iirc, most android gpus have 2 or more banks of pipeline/contexts for this scenario
* The bottleneck is probably the system bus, instead of the cpu assembly of the gl commands (which is what the two threads would help with)

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

I'm a bit concerned that so much code has been rewritten here. Including many parts where a one line change means life-or-death for performance and multimonitor. Needs some serious manual testing...

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

(1) Found a regression on trunk when testing this. Will work on that now... bug 1420678.

(2) I'm concerned that "groups" are never useful or desirable with real displays;
If you use groups, support for displays with different refresh rates is lost. It's important that we never synchronize between different threads running different displays. Because one could easily be running much faster than the other (24Hz TV + 60Hz laptop in sidebyside mode) and still we need to be posting at the full 60Hz.

628 - display_buffer_compositor->composite(
629 - scene->scene_elements_for(comp_id));
630 + for (auto& compositor : compositors)
631 + {
632 + target.ensure_current(std::get<0>(compositor));
633 + std::get<1>(compositor)->composite(scene->scene_elements_for(comp_id));
634 + }
635 + group.post();

So groups in this way almost never make sense. I'm not sure it's a good idea for Android even. Definitely not desktop. Also, we already have a kind of grouping that we know is more annoying than useful; see bug 1395416.

Revision history for this message
Daniel van Vugt (vanvugt) 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.
It's important each display's flip() occurs on a separate thread asynchronously, so that its own rendering of the next frame can start immediately and we have a full frame time to render.

We need to reduce our use of grouping logic (bug 1395416) rather than extending it.

If you still need some sort of grouping to resolve Android problems, I suggest utilizing the grouping logic ("overlapping outputs") we already have, and only utilizing it in Android.

review: Disapprove
Revision history for this message
Alexandros Frantzis (afrantzis) wrote :

After conversations with Kevin, my understanding is that the Android APIs enforce synchronized posting of all outputs even if their contents are completely independent (i.e. different DisplayBuffers). That's _not_ the same problem we have with clone mode on desktop, where we have a single DisplayBuffer which we can post to different outputs.

I would, too, prefer it if we didn't need to have the DisplayGroup abstraction, but it seems that the Android display architecture forces this design choice. My preference would be use the current DisplayBuffer interface and hide all the synchronization inside the Android platform, but Kevin has tried this and found it to be both complicated and fragile.

> It's important each display's flip() occurs on a separate thread asynchronously, so that its
> own rendering of the next frame can start immediately and we have a full frame time to render.

Unfortunaetly, as described in the first paragraph, this is exactly what's not possible on Android. The bright side is that neither this concern, nor the one I noted in a previous comment affect the desktop, since on the desktop 1 DisplayGroup always contains 1 DisplayBuffer.

review: Approve
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

OK as a intermediate form

review: Approve
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

I think much greater care needs to be taken by all here.

This proposal can and should be improved. The DisplayGroup is not only undesirable for desktop but downright dangerous for a desktop compositor to use. Because there's no way to do so (e.g. with Mesa) without either causing frame skipping, reduced performance or lag.

As such, please hide DisplayGroup from the common code so that desktoppers can't accidentally find themselves using it.

review: Needs Fixing
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

At least one way springs to mind in which Android can be supported without DisplayGroup:
Implement a "MultiDisplayBuffer" containing multiple displays attached to a single DisplayBufferCompositor. That way they are composited together and posted together. And the code classes don't need modification.

Alternatively, please consider other ways to avoid touching so much common code. As it stands now, this increases the risk of short and long term regressions (confusion by future developers). And it scares me. We have the ability to pause and try to think of better solutions still.

review: Needs Fixing
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

-code classes
+common classes

Even if this seems like the only solution right now, we owe it to ourselves to ponder this a bit more. It's a 2000 line change that touches one of the most highly sensitive portions of Mir. It's only been up less than 48 hours. So just stop and think about other possibilities for a while...

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

@code rewrite:
The current interfaces are insufficient for android multimonitor, because android has a weird commit interface (line 587, https://android.googlesource.com/platform/hardware/libhardware/+/master/include/hardware/hwcomposer.h). Our current platform interfaces do not have explicit hints about threading, and this avoids this by saying "this group of monitors must post together in DisplayGroup".

I tried to work within the existing API and thread model in lp:~kdub/mir/hwc-sync-and-post-external, but this led to some complex, not-universal and heavy-handed synchronization. (eg, You couldn't drive that code from one thread, one thread per display was the only way. Furthermore, the displays were still synced together, making the second thread mostly useless)

(2) Its true that having two DisplayBuffers in one DisplayGroup ties the displays together, but mesa remain 1DisplayBuffer/1DisplayGroup, so mesa can still be driven in its preferred way, that is, one thread that syncs at the vsync rate of the respective monitor. Android also does not wait in mg::DisplayGroup::post(), so the disadvantages of having one thread drive 2 monitors is less severe. I share the sentiment that the android mm interface is strange, but we have to sync between the different display threads for the interface.

(3) Its important to remember that android's set() call does not block, so the timings are quite different than a model that relies on waiting on the vsync of a particular display. The vsync signal and the commit call are detached; the buffers are protected by fences. Its more that we're scheduling work in the kernel as opposed to controlling the timing carefully like in KMS/drm.

I did look into using OverlappingOutputGrouping in the same way that mesa does, that is, to model 2 monitors an 1 DisplayBuffer, with one common scanout buffer. This relies on being able set the scanout buffer of the monitors, which android doesn't let us do. We need one eglContext per display, and cannot force both monitors to read from one bundle of framebuffers.

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.

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

> At least one way springs to mind in which Android can be supported without
> DisplayGroup:
> Implement a "MultiDisplayBuffer" containing multiple displays attached to a
> single DisplayBufferCompositor. That way they are composited together and
> posted together. And the code classes don't need modification.

This is generally what this MP is intended to be, except that multiple displays are attached to the DisplayGroup, instead of having the grouping requirements somewhere in DisplayBufferCompositiors. I guess I'd have to see the MultiDisplayBuffer interface to figure out if android's mm requirements can fit within it.

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

> Even if this seems like the only solution right now, we owe it to ourselves to
> ponder this a bit more. It's a 2000 line change that touches one of the most
> highly sensitive portions of Mir. It's only been up less than 48 hours. So
> just stop and think about other possibilities for a while...

Fair enough, we can allot more time to review and consider, and I can chase more reviews as well.

Revision history for this message
Andreas Pokorny (andreas-pokorny) wrote :

Even if it could be done with a MultiDisplayBuffer approach, I think splitting out the post part looks reasonable.

review: Approve
Revision history for this message
Alberto Aguirre (albaguirre) wrote :

Looks reasonable and becomes clearer when peeking at the next mesa branch.

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Kevin DuBois (kdub) wrote :

downstream compatibility branch: lp:~kdub/qtmir/display-groups

Revision history for this message
Gerry Boland (gerboland) wrote :

I understand the motivation for DisplayGroup. The concept will make qtmir's multimonitor support complicated however, as QtMir will allocate one QWindow per Display, and expects to be able to call swap buffers on each Display individually.

Individuality being important: if the scene on monitor 2 changes, then its render loop will spin up, draw and call swap buffers for Display2. However if Display2 is part of a DisplayGroup, it would be presumptuous for it to swap the whole DisplayGroup.

What if Display1 (part of DisplayGroup too) was also being rendered, but finished later than Display2? It can't call swapBuffers on the whole DisplayGroup again, so instead need a sync point between the 2 threads where both are done, before calling swap.

But if Display1 doesn't need changing, then Display2 can legitimately call swap on the whole DisplayGroup.

I don't see how I can support this with Qt without some form of synchronization between the render threads which share a DisplayGroup. Is that a correct conclusion?

If so, it'll be work getting Qt to support this.

Revision history for this message
Daniel van Vugt (vanvugt) 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.

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

Revision history for this message
Andreas Pokorny (andreas-pokorny) wrote :

> I understand the motivation for DisplayGroup. The concept will make qtmir's
> multimonitor support complicated however, as QtMir will allocate one QWindow
> per Display, and expects to be able to call swap buffers on each Display
> individually.

Do you mean Display as in mir::graphics::Display? Or rather DisplayBuffer?

> Individuality being important: if the scene on monitor 2 changes, then its
> render loop will spin up, draw and call swap buffers for Display2. However if
> Display2 is part of a DisplayGroup, it would be presumptuous for it to swap
> the whole DisplayGroup.
>
> What if Display1 (part of DisplayGroup too) was also being rendered, but
> finished later than Display2? It can't call swapBuffers on the whole
> DisplayGroup again, so instead need a sync point between the 2 threads where
> both are done, before calling swap.

This problem is a subset of the problem we have with the underlying android API.
So even with this MP the problem still exists.. but it seems slightly simpler, we only need an optional sync point or optional sync time-window? (optional since for nested/offscreen/mesa DisplayGroup : DisplayBuffer will be a 1:1 relation, as I understood)..

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

> I understand the motivation for DisplayGroup. The concept will make qtmir's
> multimonitor support complicated however, as QtMir will allocate one QWindow
> per Display, and expects to be able to call swap buffers on each Display
> individually.
>
> Individuality being important: if the scene on monitor 2 changes, then its
> render loop will spin up, draw and call swap buffers for Display2. However if
> Display2 is part of a DisplayGroup, it would be presumptuous for it to swap
> the whole DisplayGroup.
> What if Display1 (part of DisplayGroup too) was also being rendered, but
> finished later than Display2? It can't call swapBuffers on the whole
> DisplayGroup again, so instead need a sync point between the 2 threads where
> both are done, before calling swap.
>
> But if Display1 doesn't need changing, then Display2 can legitimately call
> swap on the whole DisplayGroup.

Good points, but these are implications of how HWC/multimonitor are driven.
On android, it doesn't cause that much of a performance headache because swap_buffers() can be called independently on each "display buffer"/GL context, and the hwc functions themselves don't block, they just schedule work.

>
> I don't see how I can support this with Qt without some form of
> synchronization between the render threads which share a DisplayGroup. Is that
> a correct conclusion?
>
> If so, it'll be work getting Qt to support this.

Right, so, to rephrase an above comment, this change to DisplayBuffer implicitly gives a hint about how the threads of the compositor should be structured, now that we have a platform that does not prefer 1 thread per monitor, but rather prefers a synchronized submission for all DisplayBuffers.

So, my hope with QtMir and other compositors is that we have MultiThreadedCompositor handle the compositor threads and the compositor notification system, and have the compositors override DisplayBufferCompositor, with the already-existing stipulation that DisplayBufferCompositors have to be threadsafe, as composite() can be called from different threads with different DisplayBuffers.

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

@andreas, correct, this abstraction change in this MP is expanding to cover a goofy requirement (from a 1 thread/monitor world's perspective at least) that the HWC api has. A multithreaded setup on android would hit a lot of the same timing inconveniences in synchronizing, just down in the HwcDevice code, where it is not in the best position to make decisions to wait or continue.

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.

Revision history for this message
Robert Carr (robertcarr) wrote :

Lots of review already done so not going in depth. No high level objections though.

review: Abstain
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

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

Right, so this works for Android because Android isn't a low-level display system. It's a high-level abstraction of displays (and very much OpenGL-only). Native platforms (mesa, nvidia etc) will usually be low-level display systems that need asynchronous posting (hence never use DisplayGroup).

Although I can now imagine some virtual Mir platform in a window. If you wanted to represent multiple virtual outputs for visualization all in a single window then DisplayGroups would be used. But I doubt any such use case beyond Android will really ever be required.

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Wait a minute, post() is a no-op as is flip() for Android. You must be planning to change this in future if DisplayGroup is to have any function at all...?

Previously Android correctly had an empty flip() (now an empty post()), because it's a high-level platform abstraction that just needs to gl_swap_buffers.

I suggest part of the problem then is that you've misappropriated flip() on a platform that has no flip().

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

> Wait a minute, post() is a no-op as is flip() for Android. You must be
> planning to change this in future if DisplayGroup is to have any function at
> all...?
>

Yes, the subsequent branch uses the interface changes made here:
http://bazaar.launchpad.net/~kdub/mir/android-multimonitor/view/head:/src/platforms/android/server/display_group.cpp#L68

> Previously Android correctly had an empty flip() (now an empty post()),
> because it's a high-level platform abstraction that just needs to
> gl_swap_buffers.
>
> I suggest part of the problem then is that you've misappropriated flip() on a
> platform that has no flip().

Right, a no-op flip() seemed inappropriate for the android platform, as it did a "flip" operation from within gl_swap_buffers(). So, a user of the interface might be calling gl_swap_buffers(), which drives the display on android, and on mesa, it would just be rendering without posting to screen. It seems more appropriate to have gl_swap_buffers handle updating the context, and have flip/post() drive the content making it to the screen.

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

OK, I see the need. And I understand it's a painful thought to redesign the way I've suggested.

As a compromise how about just a rename of DisplayGroup so as to make it less likely that we don't accidentally use it on non-Android platforms; "SynchronousDisplayGroup" or "SyncGroup".

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

> OK, I see the need. And I understand it's a painful thought to redesign the
> way I've suggested.

Well, its not simply painful, but rather our current interfaces don't allow the android platform to present to the server code what the platform requires as its threading structure to post to the external monitor.

>
> As a compromise how about just a rename of DisplayGroup so as to make it less
> likely that we don't accidentally use it on non-Android platforms;
> "SynchronousDisplayGroup" or "SyncGroup".

Sounds good, how about DisplaySyncGroup?

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

include/platform/mir/graphics/display.h:
Documentation should be improved to point out DisplaySyncGroup is only for virtual display buffer implementations (VMs, Android) that have non-blocking post implementations. And most importantly it should not be used by native platform drivers.

Then I abstain.

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Also don't say "to their respective monitors" because nobody should flip to real monitors using a DisplaySyncGroup. That would result in bugs.

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

Updated!
I don't know if I agree that Android is somehow "virtual" while mesa is somehow "native". They just have different sync mechanisms.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'benchmarks/frame-uniformity/vsync_simulating_graphics_platform.cpp'
2--- benchmarks/frame-uniformity/vsync_simulating_graphics_platform.cpp 2015-02-13 06:12:34 +0000
3+++ benchmarks/frame-uniformity/vsync_simulating_graphics_platform.cpp 2015-02-23 13:13:47 +0000
4@@ -36,20 +36,21 @@
5 namespace
6 {
7
8-struct StubDisplayBuffer : mtd::StubDisplayBuffer
9+struct StubDisplaySyncGroup : mg::DisplaySyncGroup
10 {
11- StubDisplayBuffer(geom::Size output_size, int vsync_rate_in_hz)
12- : mtd::StubDisplayBuffer({{0, 0}, output_size}),
13- vsync_rate_in_hz(vsync_rate_in_hz),
14- last_sync(std::chrono::high_resolution_clock::now())
15- {
16- }
17-
18- void gl_swap_buffers() override
19- {
20- }
21-
22- void flip() override
23+ StubDisplaySyncGroup(geom::Size output_size, int vsync_rate_in_hz) :
24+ vsync_rate_in_hz(vsync_rate_in_hz),
25+ last_sync(std::chrono::high_resolution_clock::now()),
26+ buffer({{0, 0}, output_size})
27+ {
28+ }
29+
30+ void for_each_display_buffer(std::function<void(mg::DisplayBuffer&)> const& exec) override
31+ {
32+ exec(buffer);
33+ }
34+
35+ void post() override
36 {
37 auto now = std::chrono::high_resolution_clock::now();
38 auto next_sync = last_sync + std::chrono::seconds(1) / vsync_rate_in_hz;
39@@ -63,22 +64,24 @@
40 double const vsync_rate_in_hz;
41
42 std::chrono::high_resolution_clock::time_point last_sync;
43+
44+ mtd::StubDisplayBuffer buffer;
45 };
46
47 struct StubDisplay : public mtd::StubDisplay
48 {
49- StubDisplay(geom::Size output_size, int vsync_rate_in_hz)
50- : mtd::StubDisplay({{{0,0}, output_size}}),
51- buffer(output_size, vsync_rate_in_hz)
52+ StubDisplay(geom::Size output_size, int vsync_rate_in_hz) :
53+ mtd::StubDisplay({{{0,0}, output_size}}),
54+ group(output_size, vsync_rate_in_hz)
55 {
56 }
57
58- void for_each_display_buffer(std::function<void(mg::DisplayBuffer&)> const& exec) override
59+ void for_each_display_sync_group(std::function<void(mg::DisplaySyncGroup&)> const& exec) override
60 {
61- exec(buffer);
62+ exec(group);
63 }
64
65- StubDisplayBuffer buffer;
66+ StubDisplaySyncGroup group;
67 };
68
69 }
70
71=== modified file 'examples/render_overlays.cpp'
72--- examples/render_overlays.cpp 2015-01-21 09:03:53 +0000
73+++ examples/render_overlays.cpp 2015-02-23 13:13:47 +0000
74@@ -202,12 +202,16 @@
75
76 while (running)
77 {
78- display->for_each_display_buffer([&](mg::DisplayBuffer& buffer)
79+ client1->update_green_channel();
80+ client2->update_green_channel();
81+ display->for_each_display_sync_group([&](mg::DisplaySyncGroup& group)
82 {
83- buffer.make_current();
84- client1->update_green_channel();
85- client2->update_green_channel();
86- buffer.post_renderables_if_optimizable(renderlist);
87+ group.for_each_display_buffer([&](mg::DisplayBuffer& buffer)
88+ {
89+ buffer.make_current();
90+ buffer.post_renderables_if_optimizable(renderlist);
91+ });
92+ group.post();
93 });
94 }
95 }
96
97=== modified file 'examples/render_surfaces.cpp'
98--- examples/render_surfaces.cpp 2015-01-21 07:34:50 +0000
99+++ examples/render_surfaces.cpp 2015-02-23 13:13:47 +0000
100@@ -337,9 +337,12 @@
101
102 /* TODO: Get proper configuration */
103 geom::Rectangles view_area;
104- display->for_each_display_buffer([&view_area](mg::DisplayBuffer const& db)
105+ display->for_each_display_sync_group([&](mg::DisplaySyncGroup& group)
106 {
107- view_area.add(db.view_area());
108+ group.for_each_display_buffer([&](mg::DisplayBuffer& db)
109+ {
110+ view_area.add(db.view_area());
111+ });
112 });
113 geom::Size const display_size{view_area.bounding_rectangle().size};
114 uint32_t const surface_side{300};
115
116=== modified file 'examples/render_to_fb.cpp'
117--- examples/render_to_fb.cpp 2015-01-21 07:34:50 +0000
118+++ examples/render_to_fb.cpp 2015-02-23 13:13:47 +0000
119@@ -53,22 +53,26 @@
120
121 mir::draw::glAnimationBasic gl_animation;
122
123- display->for_each_display_buffer([&](mg::DisplayBuffer& buffer)
124+ display->for_each_display_sync_group([&](mg::DisplaySyncGroup& group)
125 {
126- buffer.make_current();
127- gl_animation.init_gl();
128+ group.for_each_display_buffer([&](mg::DisplayBuffer& buffer)
129+ {
130+ buffer.make_current();
131+ gl_animation.init_gl();
132+ });
133 });
134
135 while (running)
136 {
137- display->for_each_display_buffer([&](mg::DisplayBuffer& buffer)
138+ display->for_each_display_sync_group([&](mg::DisplaySyncGroup& group)
139 {
140- buffer.make_current();
141-
142- gl_animation.render_gl();
143-
144- buffer.gl_swap_buffers();
145- buffer.flip();
146+ group.for_each_display_buffer([&](mg::DisplayBuffer& buffer)
147+ {
148+ buffer.make_current();
149+ gl_animation.render_gl();
150+ buffer.gl_swap_buffers();
151+ });
152+ group.post();
153 });
154
155 gl_animation.step();
156
157=== modified file 'include/platform/mir/graphics/display.h'
158--- include/platform/mir/graphics/display.h 2015-01-21 07:34:50 +0000
159+++ include/platform/mir/graphics/display.h 2015-02-23 13:13:47 +0000
160@@ -39,6 +39,32 @@
161 typedef std::function<bool()> DisplayResumeHandler;
162 typedef std::function<void()> DisplayConfigurationChangeHandler;
163
164+/* DisplaySyncGroup allows for multiple displays to be posted together.
165+ * A DisplayGroup containing multiple DisplayBuffers should only be used
166+ * by platforms that have a nonblocking post implemenation.
167+ * One DisplayBuffer per DisplaySyncGroup is preferable for platforms that
168+ * wait for the post to complete.
169+ */
170+class DisplaySyncGroup
171+{
172+public:
173+ /**
174+ * Executes a functor that allows the DisplayBuffer contents to be updated
175+ **/
176+ virtual void for_each_display_buffer(std::function<void(DisplayBuffer&)> const& f) = 0;
177+
178+ /** Post the content of the DisplayBuffers associated with this DisplaySyncGroup.
179+ * The content of all the DisplayBuffers in this DisplaySyncGroup are guaranteed to be onscreen
180+ * in the near future. On some platforms, this may wait a potentially long time for vsync.
181+ **/
182+ virtual void post() = 0;
183+ virtual ~DisplaySyncGroup() = default;
184+protected:
185+ DisplaySyncGroup() = default;
186+ DisplaySyncGroup(DisplaySyncGroup const&) = delete;
187+ DisplaySyncGroup& operator=(DisplaySyncGroup const&) = delete;
188+};
189+
190 /**
191 * Interface to the display subsystem.
192 */
193@@ -46,9 +72,9 @@
194 {
195 public:
196 /**
197- * Executes a functor for each output framebuffer.
198+ * Executes a functor for each output group.
199 */
200- virtual void for_each_display_buffer(std::function<void(DisplayBuffer&)> const& f) = 0;
201+ virtual void for_each_display_sync_group(std::function<void(DisplaySyncGroup&)> const& f) = 0;
202
203 /**
204 * Gets a copy of the current output configuration.
205
206=== modified file 'include/platform/mir/graphics/display_buffer.h'
207--- include/platform/mir/graphics/display_buffer.h 2015-01-21 07:34:50 +0000
208+++ include/platform/mir/graphics/display_buffer.h 2015-02-23 13:13:47 +0000
209@@ -54,28 +54,6 @@
210 */
211 virtual void gl_swap_buffers() = 0;
212
213- /**
214- * After gl_swap_buffers, flip the new front buffer to the screen
215- * This most likely involves a wait for vblank so can be very time
216- * consuming. This function is separate to gl_swap_buffers() because in
217- * real display systems the act of scanning out (or flipping) the
218- * front buffer is a very separate step to the GL buffer swapping. Not
219- * least because "flipping" is a hardware operation that is independent
220- * of the graphics library (OpenGL or other). Also, flip() can be a
221- * dramatically slower operation than gl_swap_buffers() and it would be
222- * an unacceptable performance hit to wait for both before freeing
223- * GL resources.
224- */
225- virtual void flip() = 0;
226-
227- /**
228- * \deprecated Please try to implement separate gl_swap_buffers and
229- * flip functions instead. If not possible, just move your old
230- * post_update() logic into gl_swap_buffers.
231- */
232- __attribute__((__deprecated__("Use gl_swap_buffers() and flip(), remembering to release all compositor buffers in the middle.")))
233- void post_update() { gl_swap_buffers(); flip(); }
234-
235 /** This will render renderlist to the screen and post the result to the
236 * screen if there is a hardware optimization that can be done.
237 * \param [in] renderlist
238
239=== modified file 'playground/demo-shell/demo_compositor.cpp'
240--- playground/demo-shell/demo_compositor.cpp 2015-01-21 09:03:53 +0000
241+++ playground/demo-shell/demo_compositor.cpp 2015-02-23 13:13:47 +0000
242@@ -141,8 +141,6 @@
243 // flip() ...
244 // FIXME: This clear() call is blocking a little (LP: #1395421)
245 renderable_list.clear();
246-
247- display_buffer.flip();
248 }
249
250 report->finished_frame(this);
251
252=== modified file 'src/platforms/android/server/configurable_display_buffer.h'
253--- src/platforms/android/server/configurable_display_buffer.h 2015-01-22 09:00:14 +0000
254+++ src/platforms/android/server/configurable_display_buffer.h 2015-02-23 13:13:47 +0000
255@@ -19,6 +19,7 @@
256 #ifndef MIR_GRAPHICS_ANDROID_CONFIGURABLE_DISPLAY_BUFFER_H_
257 #define MIR_GRAPHICS_ANDROID_CONFIGURABLE_DISPLAY_BUFFER_H_
258
259+#include "mir/graphics/display.h"
260 #include "mir/graphics/display_buffer.h"
261 #include "mir/graphics/display_configuration.h"
262
263@@ -29,7 +30,9 @@
264 namespace android
265 {
266
267-class ConfigurableDisplayBuffer : public graphics::DisplayBuffer
268+//TODO: break this dependency, android displaybuffers shouldn't be their own DisplaySyncGroups
269+class ConfigurableDisplayBuffer : public graphics::DisplayBuffer,
270+ public graphics::DisplaySyncGroup
271 {
272 public:
273 virtual void configure(MirPowerMode power_mode, MirOrientation orientation) = 0;
274
275=== modified file 'src/platforms/android/server/display.cpp'
276--- src/platforms/android/server/display.cpp 2015-02-13 06:12:34 +0000
277+++ src/platforms/android/server/display.cpp 2015-02-23 13:13:47 +0000
278@@ -218,7 +218,7 @@
279
280 }
281
282-void mga::Display::for_each_display_buffer(std::function<void(mg::DisplayBuffer&)> const& f)
283+void mga::Display::for_each_display_sync_group(std::function<void(mg::DisplaySyncGroup&)> const& f)
284 {
285 std::lock_guard<decltype(configuration_mutex)> lock{configuration_mutex};
286 update_configuration(lock);
287
288=== modified file 'src/platforms/android/server/display.h'
289--- src/platforms/android/server/display.h 2015-02-13 06:12:34 +0000
290+++ src/platforms/android/server/display.h 2015-02-23 13:13:47 +0000
291@@ -57,7 +57,7 @@
292 OverlayOptimization overlay_option);
293 ~Display() noexcept;
294
295- void for_each_display_buffer(std::function<void(graphics::DisplayBuffer&)> const& f) override;
296+ void for_each_display_sync_group(std::function<void(graphics::DisplaySyncGroup&)> const& f) override;
297
298 std::unique_ptr<graphics::DisplayConfiguration> configuration() const override;
299 void configure(graphics::DisplayConfiguration const&) override;
300
301=== modified file 'src/platforms/android/server/display_buffer.cpp'
302--- src/platforms/android/server/display_buffer.cpp 2015-02-13 06:12:34 +0000
303+++ src/platforms/android/server/display_buffer.cpp 2015-02-23 13:13:47 +0000
304@@ -98,10 +98,6 @@
305 display_device->commit(display_name, *layer_list, gl_context, overlay_program);
306 }
307
308-void mga::DisplayBuffer::flip()
309-{
310-}
311-
312 MirOrientation mga::DisplayBuffer::orientation() const
313 {
314 /*
315@@ -118,6 +114,15 @@
316 return false;
317 }
318
319+void mga::DisplayBuffer::for_each_display_buffer(std::function<void(mg::DisplayBuffer&)> const& f)
320+{
321+ f(*this);
322+}
323+
324+void mga::DisplayBuffer::post()
325+{
326+}
327+
328 void mga::DisplayBuffer::configure(MirPowerMode power_mode, MirOrientation orientation)
329 {
330 if (power_mode != mir_power_mode_on)
331
332=== modified file 'src/platforms/android/server/display_buffer.h'
333--- src/platforms/android/server/display_buffer.h 2015-02-13 06:12:34 +0000
334+++ src/platforms/android/server/display_buffer.h 2015-02-23 13:13:47 +0000
335@@ -59,9 +59,11 @@
336 void make_current() override;
337 void release_current() override;
338 void gl_swap_buffers() override;
339- void flip() override;
340 bool post_renderables_if_optimizable(RenderableList const& renderlist) override;
341
342+ void for_each_display_buffer(std::function<void(graphics::DisplayBuffer&)> const& f) override;
343+ void post() override;
344+
345 MirOrientation orientation() const override;
346 bool uses_alpha() const override;
347 void configure(MirPowerMode power_mode, MirOrientation orientation) override;
348
349=== modified file 'src/platforms/mesa/server/display.cpp'
350--- src/platforms/mesa/server/display.cpp 2015-01-22 09:00:14 +0000
351+++ src/platforms/mesa/server/display.cpp 2015-02-23 13:13:47 +0000
352@@ -113,8 +113,8 @@
353 {
354 }
355
356-void mgm::Display::for_each_display_buffer(
357- std::function<void(graphics::DisplayBuffer&)> const& f)
358+void mgm::Display::for_each_display_sync_group(
359+ std::function<void(graphics::DisplaySyncGroup&)> const& f)
360 {
361 std::lock_guard<std::mutex> lg{configuration_mutex};
362
363
364=== modified file 'src/platforms/mesa/server/display.h'
365--- src/platforms/mesa/server/display.h 2015-01-22 09:00:14 +0000
366+++ src/platforms/mesa/server/display.h 2015-02-23 13:13:47 +0000
367@@ -61,8 +61,8 @@
368 ~Display();
369
370 geometry::Rectangle view_area() const;
371- void for_each_display_buffer(
372- std::function<void(graphics::DisplayBuffer&)> const& f) override;
373+ void for_each_display_sync_group(
374+ std::function<void(graphics::DisplaySyncGroup&)> const& f) override;
375
376 std::unique_ptr<DisplayConfiguration> configuration() const override;
377 void configure(DisplayConfiguration const& conf) override;
378
379=== modified file 'src/platforms/mesa/server/display_buffer.cpp'
380--- src/platforms/mesa/server/display_buffer.cpp 2015-02-12 18:08:46 +0000
381+++ src/platforms/mesa/server/display_buffer.cpp 2015-02-23 13:13:47 +0000
382@@ -204,11 +204,22 @@
383 auto bypass_it = std::find_if(renderable_list.rbegin(), renderable_list.rend(), bypass_match);
384 if (bypass_it != renderable_list.rend())
385 {
386- auto bypass_buf = (*bypass_it)->buffer();
387- if ((bypass_buf->native_buffer_handle()->flags & mir_buffer_flag_can_scanout) &&
388- bypass_buf->size() == geom::Size{fb_width,fb_height})
389- {
390- return flip(bypass_buf);
391+ auto bypass_buffer = (*bypass_it)->buffer();
392+ auto native = bypass_buffer->native_buffer_handle();
393+ auto gbm_native = static_cast<mgm::GBMNativeBuffer*>(native.get());
394+ auto bufobj = get_buffer_object(gbm_native->bo);
395+ if (bufobj &&
396+ native->flags & mir_buffer_flag_can_scanout &&
397+ bypass_buffer->size() == geom::Size{fb_width,fb_height})
398+ {
399+ bypass_buf = bypass_buffer;
400+ bypass_bufobj = bufobj;
401+ return true;
402+ }
403+ else
404+ {
405+ bypass_buf = nullptr;
406+ bypass_bufobj = nullptr;
407 }
408 }
409 }
410@@ -216,19 +227,21 @@
411 return false;
412 }
413
414-void mgm::DisplayBuffer::flip()
415+void mgm::DisplayBuffer::for_each_display_buffer(
416+ std::function<void(graphics::DisplayBuffer&)> const& f)
417 {
418- flip(nullptr);
419+ f(*this);
420 }
421
422 void mgm::DisplayBuffer::gl_swap_buffers()
423 {
424 if (!egl.swap_buffers())
425 fatal_error("Failed to perform buffer swap");
426+ bypass_buf = nullptr;
427+ bypass_bufobj = nullptr;
428 }
429
430-bool mgm::DisplayBuffer::flip(
431- std::shared_ptr<graphics::Buffer> bypass_buf)
432+void mgm::DisplayBuffer::post()
433 {
434 /*
435 * We might not have waited for the previous frame to page flip yet.
436@@ -257,12 +270,7 @@
437 mgm::BufferObject *bufobj;
438 if (bypass_buf)
439 {
440- auto native = bypass_buf->native_buffer_handle();
441- auto gbm_native = static_cast<mgm::GBMNativeBuffer*>(native.get());
442- bufobj = get_buffer_object(gbm_native->bo);
443- // If bypass fails, just fall back to compositing.
444- if (!bufobj)
445- return false;
446+ bufobj = bypass_bufobj;
447 }
448 else
449 {
450@@ -345,8 +353,6 @@
451
452 scheduled_bufobj = bufobj;
453 }
454-
455- return true;
456 }
457
458 mgm::BufferObject* mgm::DisplayBuffer::get_front_buffer_object()
459
460=== modified file 'src/platforms/mesa/server/display_buffer.h'
461--- src/platforms/mesa/server/display_buffer.h 2015-01-22 09:00:14 +0000
462+++ src/platforms/mesa/server/display_buffer.h 2015-02-23 13:13:47 +0000
463@@ -20,6 +20,7 @@
464 #define MIR_GRAPHICS_MESA_DISPLAY_BUFFER_H_
465
466 #include "mir/graphics/display_buffer.h"
467+#include "mir/graphics/display.h"
468 #include "display_helpers.h"
469
470 #include <vector>
471@@ -41,7 +42,8 @@
472 class BufferObject;
473 class KMSOutput;
474
475-class DisplayBuffer : public graphics::DisplayBuffer
476+class DisplayBuffer : public graphics::DisplayBuffer,
477+ public graphics::DisplaySyncGroup
478 {
479 public:
480 DisplayBuffer(std::shared_ptr<Platform> const& platform,
481@@ -58,9 +60,12 @@
482 void make_current() override;
483 void release_current() override;
484 void gl_swap_buffers() override;
485- void flip() override;
486 bool post_renderables_if_optimizable(RenderableList const& renderlist) override;
487
488+ void for_each_display_buffer(
489+ std::function<void(graphics::DisplayBuffer&)> const& f) override;
490+ void post() override;
491+
492 MirOrientation orientation() const override;
493 void set_orientation(MirOrientation const rot, geometry::Rectangle const& a);
494 bool uses_alpha() const override;
495@@ -68,8 +73,6 @@
496 void wait_for_page_flip();
497
498 private:
499- bool flip(std::shared_ptr<graphics::Buffer> bypass_buf);
500-
501 BufferObject* get_front_buffer_object();
502 BufferObject* get_buffer_object(struct gbm_bo *bo);
503 bool schedule_page_flip(BufferObject* bufobj);
504@@ -77,6 +80,8 @@
505 BufferObject* last_flipped_bufobj;
506 BufferObject* scheduled_bufobj;
507 std::shared_ptr<graphics::Buffer> last_flipped_bypass_buf;
508+ std::shared_ptr<Buffer> bypass_buf{nullptr};
509+ BufferObject* bypass_bufobj{nullptr};
510 std::shared_ptr<Platform> const platform;
511 std::shared_ptr<DisplayReport> const listener;
512 /* DRM helper from mgm::Platform */
513
514=== modified file 'src/server/compositor/default_display_buffer_compositor.cpp'
515--- src/server/compositor/default_display_buffer_compositor.cpp 2015-02-13 06:12:34 +0000
516+++ src/server/compositor/default_display_buffer_compositor.cpp 2015-02-23 13:13:47 +0000
517@@ -88,10 +88,8 @@
518
519 // Release the buffers we did use back to the clients, before starting
520 // on the potentially slow flip().
521- // FIXME: This clear() call is blocking a little (LP: #1395421)
522+ // FIXME: This clear() call is blocking a little because we drive IPC here (LP: #1395421)
523 renderable_list.clear();
524-
525- display_buffer.flip();
526 }
527
528 report->finished_frame(this);
529
530=== modified file 'src/server/compositor/multi_threaded_compositor.cpp'
531--- src/server/compositor/multi_threaded_compositor.cpp 2015-02-17 11:58:56 +0000
532+++ src/server/compositor/multi_threaded_compositor.cpp 2015-02-23 13:13:47 +0000
533@@ -71,34 +71,37 @@
534 class CurrentRenderingTarget
535 {
536 public:
537- CurrentRenderingTarget(mg::DisplayBuffer& buffer)
538- : buffer(buffer)
539+ CurrentRenderingTarget() = default;
540+ void ensure_current(mg::DisplayBuffer* buffer)
541 {
542- buffer.make_current();
543+ if ((buffer) && (buffer != current_buffer))
544+ buffer->make_current();
545+ current_buffer = buffer;
546 }
547
548 ~CurrentRenderingTarget()
549 {
550- buffer.release_current();
551+ if (current_buffer) current_buffer->release_current();
552 }
553
554 private:
555- mg::DisplayBuffer& buffer;
556+ mg::DisplayBuffer* current_buffer{nullptr};
557 };
558
559 class CompositingFunctor
560 {
561 public:
562- CompositingFunctor(std::shared_ptr<mc::DisplayBufferCompositorFactory> const& db_compositor_factory,
563- mg::DisplayBuffer& buffer,
564- std::shared_ptr<mc::Scene> const& scene,
565- std::shared_ptr<CompositorReport> const& report)
566- : display_buffer_compositor_factory{db_compositor_factory},
567- buffer(buffer),
568- scene(scene),
569- running{true},
570- frames_scheduled{0},
571- report{report}
572+ CompositingFunctor(
573+ std::shared_ptr<mc::DisplayBufferCompositorFactory> const& db_compositor_factory,
574+ mg::DisplaySyncGroup& group,
575+ std::shared_ptr<mc::Scene> const& scene,
576+ std::shared_ptr<CompositorReport> const& report) :
577+ compositor_factory{db_compositor_factory},
578+ group(group),
579+ scene(scene),
580+ running{true},
581+ frames_scheduled{0},
582+ report{report}
583 {
584 }
585
586@@ -107,26 +110,25 @@
587 {
588 mir::set_thread_name("Mir/Comp");
589
590- /*
591- * Make the buffer the current rendering target, and release
592- * it when the thread is finished.
593- */
594- CurrentRenderingTarget target{buffer};
595-
596- auto display_buffer_compositor = display_buffer_compositor_factory->create_compositor_for(buffer);
597- auto const comp_id = display_buffer_compositor.get();
598-
599- CompositorReport::SubCompositorId report_id =
600- display_buffer_compositor.get();
601-
602- const auto& r = buffer.view_area();
603- report->added_display(r.size.width.as_int(), r.size.height.as_int(),
604- r.top_left.x.as_int(), r.top_left.y.as_int(),
605- report_id);
606+ CurrentRenderingTarget target;
607+ auto const comp_id = this;
608+ std::vector<std::tuple<mg::DisplayBuffer*, std::unique_ptr<mc::DisplayBufferCompositor>>> compositors;
609+ group.for_each_display_buffer(
610+ [this, &compositors, &comp_id, &target](mg::DisplayBuffer& buffer)
611+ {
612+ target.ensure_current(&buffer);
613+ compositors.emplace_back(
614+ std::make_tuple(&buffer, compositor_factory->create_compositor_for(buffer)));
615+
616+ const auto& r = buffer.view_area();
617+ report->added_display(r.size.width.as_int(), r.size.height.as_int(),
618+ r.top_left.x.as_int(), r.top_left.y.as_int(),
619+ CompositorReport::SubCompositorId{comp_id});
620+ });
621
622 auto compositor_registration = mir::raii::paired_calls(
623- [this,&display_buffer_compositor]{scene->register_compositor(display_buffer_compositor.get());},
624- [this,&display_buffer_compositor]{scene->unregister_compositor(display_buffer_compositor.get());});
625+ [this,&comp_id]{scene->register_compositor(comp_id);},
626+ [this,&comp_id]{scene->unregister_compositor(comp_id);});
627
628 std::unique_lock<std::mutex> lock{run_mutex};
629 while (running)
630@@ -150,8 +152,12 @@
631 frames_scheduled--;
632 lock.unlock();
633
634- display_buffer_compositor->composite(
635- scene->scene_elements_for(comp_id));
636+ for (auto& compositor : compositors)
637+ {
638+ target.ensure_current(std::get<0>(compositor));
639+ std::get<1>(compositor)->composite(scene->scene_elements_for(comp_id));
640+ }
641+ group.post();
642
643 lock.lock();
644
645@@ -191,8 +197,8 @@
646 }
647
648 private:
649- std::shared_ptr<mc::DisplayBufferCompositorFactory> const display_buffer_compositor_factory;
650- mg::DisplayBuffer& buffer;
651+ std::shared_ptr<mc::DisplayBufferCompositorFactory> const compositor_factory;
652+ mg::DisplaySyncGroup& group;
653 std::shared_ptr<mc::Scene> const scene;
654 bool running;
655 int frames_scheduled;
656@@ -305,12 +311,12 @@
657 void mc::MultiThreadedCompositor::create_compositing_threads()
658 {
659 /* Start the display buffer compositing threads */
660- display->for_each_display_buffer([this](mg::DisplayBuffer& buffer)
661+ display->for_each_display_sync_group([this](mg::DisplaySyncGroup& group)
662 {
663 auto thread_functor = std::make_unique<mc::CompositingFunctor>(
664- display_buffer_compositor_factory, buffer, scene, report);
665+ display_buffer_compositor_factory, group, scene, report);
666
667- futures.push_back(thread_pool.run(std::ref(*thread_functor), &buffer));
668+ futures.push_back(thread_pool.run(std::ref(*thread_functor), &group));
669 thread_functors.push_back(std::move(thread_functor));
670 });
671
672
673=== modified file 'src/server/compositor/screencast_display_buffer.cpp'
674--- src/server/compositor/screencast_display_buffer.cpp 2015-01-21 07:34:50 +0000
675+++ src/server/compositor/screencast_display_buffer.cpp 2015-02-23 13:13:47 +0000
676@@ -95,10 +95,6 @@
677 glFinish();
678 }
679
680-void mc::ScreencastDisplayBuffer::flip()
681-{
682-}
683-
684 MirOrientation mc::ScreencastDisplayBuffer::orientation() const
685 {
686 return mir_orientation_normal;
687
688=== modified file 'src/server/compositor/screencast_display_buffer.h'
689--- src/server/compositor/screencast_display_buffer.h 2015-01-21 07:34:50 +0000
690+++ src/server/compositor/screencast_display_buffer.h 2015-02-23 13:13:47 +0000
691@@ -62,7 +62,6 @@
692 bool post_renderables_if_optimizable(graphics::RenderableList const&) override;
693
694 void gl_swap_buffers() override;
695- void flip() override;
696
697 MirOrientation orientation() const override;
698
699
700=== modified file 'src/server/graphics/nested/nested_display.cpp'
701--- src/server/graphics/nested/nested_display.cpp 2015-02-17 11:58:56 +0000
702+++ src/server/graphics/nested/nested_display.cpp 2015-02-23 13:13:47 +0000
703@@ -119,6 +119,22 @@
704 eglTerminate(egl_display);
705 }
706
707+mgn::detail::DisplaySyncGroup::DisplaySyncGroup(
708+ std::shared_ptr<mgn::detail::NestedOutput> const& output) :
709+ output(output)
710+{
711+}
712+
713+void mgn::detail::DisplaySyncGroup::for_each_display_buffer(
714+ std::function<void(DisplayBuffer&)> const& f)
715+{
716+ f(*output);
717+}
718+
719+void mgn::detail::DisplaySyncGroup::post()
720+{
721+}
722+
723 mgn::NestedDisplay::NestedDisplay(
724 std::shared_ptr<mg::Platform> const& platform,
725 std::shared_ptr<HostConnection> const& connection,
726@@ -144,7 +160,7 @@
727 eglMakeCurrent(egl_display, EGL_NO_SURFACE, EGL_NO_SURFACE, EGL_NO_CONTEXT);
728 }
729
730-void mgn::NestedDisplay::for_each_display_buffer(std::function<void(mg::DisplayBuffer&)> const& f)
731+void mgn::NestedDisplay::for_each_display_sync_group(std::function<void(mg::DisplaySyncGroup&)> const& f)
732 {
733 std::unique_lock<std::mutex> lock(outputs_mutex);
734 for (auto& i : outputs)
735@@ -205,12 +221,13 @@
736
737 auto const host_surface = connection->create_surface(request_params);
738
739- result[output.id] = std::make_shared<mgn::detail::NestedOutput>(
740- egl_display,
741- host_surface,
742- area,
743- dispatcher,
744- output.current_format);
745+ result[output.id] = std::make_shared<mgn::detail::DisplaySyncGroup>(
746+ std::make_shared<mgn::detail::NestedOutput>(
747+ egl_display,
748+ host_surface,
749+ area,
750+ dispatcher,
751+ output.current_format));
752 have_output_for_group = true;
753 }
754 });
755
756=== modified file 'src/server/graphics/nested/nested_display.h'
757--- src/server/graphics/nested/nested_display.h 2015-01-21 07:34:50 +0000
758+++ src/server/graphics/nested/nested_display.h 2015-02-23 13:13:47 +0000
759@@ -87,6 +87,16 @@
760
761 class NestedOutput;
762
763+class DisplaySyncGroup : public graphics::DisplaySyncGroup
764+{
765+public:
766+ DisplaySyncGroup(std::shared_ptr<detail::NestedOutput> const& output);
767+ void for_each_display_buffer(std::function<void(DisplayBuffer&)> const&) override;
768+ void post() override;
769+private:
770+ std::shared_ptr<detail::NestedOutput> const output;
771+};
772+
773 extern EGLint const nested_egl_context_attribs[];
774 }
775
776@@ -105,7 +115,7 @@
777
778 ~NestedDisplay() noexcept;
779
780- void for_each_display_buffer(std::function<void(DisplayBuffer&)>const& f) override;
781+ void for_each_display_sync_group(std::function<void(DisplaySyncGroup&)>const& f) override;
782
783 std::unique_ptr<DisplayConfiguration> configuration() const override;
784 void configure(DisplayConfiguration const&) override;
785@@ -133,7 +143,7 @@
786 detail::EGLDisplayHandle egl_display;
787
788 std::mutex outputs_mutex;
789- std::unordered_map<DisplayConfigurationOutputId, std::shared_ptr<detail::NestedOutput>> outputs;
790+ std::unordered_map<DisplayConfigurationOutputId, std::shared_ptr<detail::DisplaySyncGroup>> outputs;
791 DisplayConfigurationChangeHandler my_conf_change_handler;
792 void create_surfaces(mir::graphics::DisplayConfiguration const& configuration);
793 void apply_to_connection(mir::graphics::DisplayConfiguration const& configuration);
794
795=== modified file 'src/server/graphics/nested/nested_output.cpp'
796--- src/server/graphics/nested/nested_output.cpp 2015-02-13 06:12:34 +0000
797+++ src/server/graphics/nested/nested_output.cpp 2015-02-23 13:13:47 +0000
798@@ -70,10 +70,6 @@
799 eglSwapBuffers(egl_display, egl_surface);
800 }
801
802-void mgn::detail::NestedOutput::flip()
803-{
804-}
805-
806 bool mgn::detail::NestedOutput::post_renderables_if_optimizable(RenderableList const&)
807 {
808 return false;
809
810=== modified file 'src/server/graphics/nested/nested_output.h'
811--- src/server/graphics/nested/nested_output.h 2015-01-21 07:34:50 +0000
812+++ src/server/graphics/nested/nested_output.h 2015-02-23 13:13:47 +0000
813@@ -48,7 +48,6 @@
814 void make_current() override;
815 void release_current() override;
816 void gl_swap_buffers() override;
817- void flip() override;
818 MirOrientation orientation() const override;
819 bool uses_alpha() const override;
820
821
822=== modified file 'src/server/graphics/offscreen/display.cpp'
823--- src/server/graphics/offscreen/display.cpp 2015-02-17 11:58:56 +0000
824+++ src/server/graphics/offscreen/display.cpp 2015-02-23 13:13:47 +0000
825@@ -71,6 +71,21 @@
826 eglTerminate(egl_display);
827 }
828
829+mgo::detail::DisplaySyncGroup::DisplaySyncGroup(std::unique_ptr<mg::DisplayBuffer> output) :
830+ output(std::move(output))
831+{
832+}
833+
834+void mgo::detail::DisplaySyncGroup::for_each_display_buffer(
835+ std::function<void(mg::DisplayBuffer&)> const& f)
836+{
837+ f(*output);
838+}
839+
840+void mgo::detail::DisplaySyncGroup::post()
841+{
842+}
843+
844 mgo::Display::Display(
845 EGLNativeDisplayType egl_native_display,
846 std::shared_ptr<DisplayConfigurationPolicy> const& initial_conf_policy,
847@@ -94,13 +109,13 @@
848 {
849 }
850
851-void mgo::Display::for_each_display_buffer(
852- std::function<void(mg::DisplayBuffer&)> const& f)
853+void mgo::Display::for_each_display_sync_group(
854+ std::function<void(mg::DisplaySyncGroup&)> const& f)
855 {
856 std::lock_guard<std::mutex> lock{configuration_mutex};
857
858- for (auto& db_ptr : display_buffers)
859- f(*db_ptr);
860+ for (auto& dg_ptr : display_sync_groups)
861+ f(*dg_ptr);
862 }
863
864 std::unique_ptr<mg::DisplayConfiguration> mgo::Display::configuration() const
865@@ -120,7 +135,7 @@
866
867 std::lock_guard<std::mutex> lock{configuration_mutex};
868
869- display_buffers.clear();
870+ display_sync_groups.clear();
871
872 conf.for_each_output(
873 [this] (DisplayConfigurationOutput const& output)
874@@ -131,7 +146,8 @@
875 SurfacelessEGLContext{egl_display, egl_context_shared},
876 output.extents()};
877
878- display_buffers.push_back(std::unique_ptr<mg::DisplayBuffer>(raw_db));
879+ display_sync_groups.emplace_back(
880+ new mgo::detail::DisplaySyncGroup(std::unique_ptr<mg::DisplayBuffer>(raw_db)));
881 }
882 });
883 }
884
885=== modified file 'src/server/graphics/offscreen/display.h'
886--- src/server/graphics/offscreen/display.h 2015-01-21 07:34:50 +0000
887+++ src/server/graphics/offscreen/display.h 2015-02-23 13:13:47 +0000
888@@ -58,6 +58,16 @@
889 EGLDisplay egl_display;
890 };
891
892+class DisplaySyncGroup : public graphics::DisplaySyncGroup
893+{
894+public:
895+ DisplaySyncGroup(std::unique_ptr<DisplayBuffer> output);
896+ void for_each_display_buffer(std::function<void(DisplayBuffer&)> const&) override;
897+ void post() override;
898+private:
899+ std::unique_ptr<DisplayBuffer> const output;
900+};
901+
902 }
903
904 class Display : public graphics::Display
905@@ -68,7 +78,7 @@
906 std::shared_ptr<DisplayReport> const& listener);
907 ~Display() noexcept;
908
909- void for_each_display_buffer(std::function<void(DisplayBuffer&)> const& f) override;
910+ void for_each_display_sync_group(std::function<void(DisplaySyncGroup&)> const& f) override;
911
912 std::unique_ptr<graphics::DisplayConfiguration> configuration() const override;
913 void configure(graphics::DisplayConfiguration const& conf) override;
914@@ -93,7 +103,7 @@
915 SurfacelessEGLContext const egl_context_shared;
916 mutable std::mutex configuration_mutex;
917 DisplayConfiguration current_display_configuration;
918- std::vector<std::unique_ptr<DisplayBuffer>> display_buffers;
919+ std::vector<std::unique_ptr<DisplaySyncGroup>> display_sync_groups;
920 };
921
922 }
923
924=== modified file 'src/server/graphics/offscreen/display_buffer.cpp'
925--- src/server/graphics/offscreen/display_buffer.cpp 2015-01-21 07:34:50 +0000
926+++ src/server/graphics/offscreen/display_buffer.cpp 2015-02-23 13:13:47 +0000
927@@ -144,10 +144,6 @@
928 glFinish();
929 }
930
931-void mgo::DisplayBuffer::flip()
932-{
933-}
934-
935 bool mgo::DisplayBuffer::post_renderables_if_optimizable(RenderableList const&)
936 {
937 return false;
938
939=== modified file 'src/server/graphics/offscreen/display_buffer.h'
940--- src/server/graphics/offscreen/display_buffer.h 2015-01-21 07:34:50 +0000
941+++ src/server/graphics/offscreen/display_buffer.h 2015-02-23 13:13:47 +0000
942@@ -66,7 +66,6 @@
943 void make_current() override;
944 void release_current() override;
945 void gl_swap_buffers() override;
946- void flip() override;
947
948 MirOrientation orientation() const override;
949 bool uses_alpha() const override;
950
951=== modified file 'src/server/input/display_input_region.cpp'
952--- src/server/input/display_input_region.cpp 2014-03-06 06:05:17 +0000
953+++ src/server/input/display_input_region.cpp 2015-02-23 13:13:47 +0000
954@@ -37,11 +37,14 @@
955 {
956 geom::Rectangles rectangles;
957
958- display->for_each_display_buffer(
959- [&rectangles](mg::DisplayBuffer const& buffer)
960- {
961- rectangles.add(buffer.view_area());
962- });
963+ display->for_each_display_sync_group([&rectangles](mg::DisplaySyncGroup& group)
964+ {
965+ group.for_each_display_buffer(
966+ [&rectangles](mg::DisplayBuffer const& buffer)
967+ {
968+ rectangles.add(buffer.view_area());
969+ });
970+ });
971
972 return rectangles.bounding_rectangle();
973 }
974@@ -50,11 +53,14 @@
975 {
976 geom::Rectangles rectangles;
977
978- display->for_each_display_buffer(
979- [&rectangles](mg::DisplayBuffer const& buffer)
980- {
981- rectangles.add(buffer.view_area());
982- });
983+ display->for_each_display_sync_group([&rectangles](mg::DisplaySyncGroup& group)
984+ {
985+ group.for_each_display_buffer(
986+ [&rectangles](mg::DisplayBuffer const& buffer)
987+ {
988+ rectangles.add(buffer.view_area());
989+ });
990+ });
991
992 rectangles.confine(point);
993 }
994
995=== modified file 'src/server/shell/graphics_display_layout.cpp'
996--- src/server/shell/graphics_display_layout.cpp 2015-01-21 07:34:50 +0000
997+++ src/server/shell/graphics_display_layout.cpp 2015-02-23 13:13:47 +0000
998@@ -98,20 +98,22 @@
999 int max_area = -1;
1000 geometry::Rectangle best = rect;
1001
1002- display->for_each_display_buffer(
1003- [&](mg::DisplayBuffer const& db)
1004- {
1005- auto const& screen = db.view_area();
1006- auto const& overlap = rect.intersection_with(screen);
1007- int area = overlap.size.width.as_int() *
1008- overlap.size.height.as_int();
1009-
1010- if (area > max_area)
1011+ display->for_each_display_sync_group([&](mg::DisplaySyncGroup& group)
1012+ {
1013+ group.for_each_display_buffer([&](mg::DisplayBuffer const& db)
1014 {
1015- best = screen;
1016- max_area = area;
1017- }
1018- });
1019+ auto const& screen = db.view_area();
1020+ auto const& overlap = rect.intersection_with(screen);
1021+ int area = overlap.size.width.as_int() *
1022+ overlap.size.height.as_int();
1023+
1024+ if (area > max_area)
1025+ {
1026+ best = screen;
1027+ max_area = area;
1028+ }
1029+ });
1030+ });
1031
1032 return best;
1033 }
1034
1035=== modified file 'tests/acceptance-tests/test_display_configuration.cpp'
1036--- tests/acceptance-tests/test_display_configuration.cpp 2015-01-21 07:34:50 +0000
1037+++ tests/acceptance-tests/test_display_configuration.cpp 2015-02-23 13:13:47 +0000
1038@@ -24,7 +24,7 @@
1039 #include "mir_test_framework/connected_client_headless_server.h"
1040 #include "mir_test_doubles/null_platform.h"
1041 #include "mir_test_doubles/null_display.h"
1042-#include "mir_test_doubles/null_display_buffer.h"
1043+#include "mir_test_doubles/null_display_sync_group.h"
1044 #include "mir_test_doubles/null_platform.h"
1045 #include "mir_test/display_config_matchers.h"
1046 #include "mir_test_doubles/stub_display_configuration.h"
1047@@ -64,9 +64,9 @@
1048 {
1049 }
1050
1051- void for_each_display_buffer(std::function<void(mg::DisplayBuffer&)> const& f) override
1052+ void for_each_display_sync_group(std::function<void(mg::DisplaySyncGroup&)> const& f) override
1053 {
1054- f(display_buffer);
1055+ f(display_sync_group);
1056 }
1057
1058 std::unique_ptr<mg::DisplayConfiguration> configuration() const override
1059@@ -111,7 +111,7 @@
1060
1061 private:
1062 std::shared_ptr<mtd::StubDisplayConfig> config;
1063- mtd::NullDisplayBuffer display_buffer;
1064+ mtd::NullDisplaySyncGroup display_sync_group;
1065 mt::Pipe p;
1066 std::atomic<bool> handler_called;
1067 };
1068
1069=== modified file 'tests/include/mir_test_doubles/mock_display.h'
1070--- tests/include/mir_test_doubles/mock_display.h 2015-01-21 07:34:50 +0000
1071+++ tests/include/mir_test_doubles/mock_display.h 2015-02-23 13:13:47 +0000
1072@@ -35,7 +35,7 @@
1073 struct MockDisplay : public graphics::Display
1074 {
1075 public:
1076- MOCK_METHOD1(for_each_display_buffer, void (std::function<void(graphics::DisplayBuffer&)> const&));
1077+ MOCK_METHOD1(for_each_display_sync_group, void (std::function<void(graphics::DisplaySyncGroup&)> const&));
1078 MOCK_CONST_METHOD0(configuration, std::unique_ptr<graphics::DisplayConfiguration>());
1079 MOCK_METHOD1(configure, void(graphics::DisplayConfiguration const&));
1080 MOCK_METHOD2(register_configuration_change_handler,
1081
1082=== modified file 'tests/include/mir_test_doubles/mock_display_buffer.h'
1083--- tests/include/mir_test_doubles/mock_display_buffer.h 2015-01-21 07:34:50 +0000
1084+++ tests/include/mir_test_doubles/mock_display_buffer.h 2015-02-23 13:13:47 +0000
1085@@ -43,7 +43,6 @@
1086 MOCK_METHOD0(make_current, void());
1087 MOCK_METHOD0(release_current, void());
1088 MOCK_METHOD0(gl_swap_buffers, void());
1089- MOCK_METHOD0(flip, void());
1090 MOCK_METHOD1(post_renderables_if_optimizable, bool(graphics::RenderableList const&));
1091 MOCK_CONST_METHOD0(orientation, MirOrientation());
1092 MOCK_CONST_METHOD0(uses_alpha, bool());
1093
1094=== modified file 'tests/include/mir_test_doubles/null_display.h'
1095--- tests/include/mir_test_doubles/null_display.h 2015-01-21 07:34:50 +0000
1096+++ tests/include/mir_test_doubles/null_display.h 2015-02-23 13:13:47 +0000
1097@@ -22,7 +22,7 @@
1098 #include "mir/graphics/display.h"
1099 #include "null_gl_context.h"
1100 #include "null_display_configuration.h"
1101-#include <thread>
1102+#include "null_display_sync_group.h"
1103
1104 namespace mir
1105 {
1106@@ -34,10 +34,9 @@
1107 class NullDisplay : public graphics::Display
1108 {
1109 public:
1110- void for_each_display_buffer(std::function<void(graphics::DisplayBuffer&)> const&) override
1111+ void for_each_display_sync_group(std::function<void(graphics::DisplaySyncGroup&)> const& f) override
1112 {
1113- /* yield() is needed to ensure reasonable runtime under valgrind for some tests */
1114- std::this_thread::yield();
1115+ f(group);
1116 }
1117 std::unique_ptr<graphics::DisplayConfiguration> configuration() const override
1118 {
1119@@ -66,6 +65,7 @@
1120 {
1121 return std::unique_ptr<NullGLContext>{new NullGLContext()};
1122 }
1123+ NullDisplaySyncGroup group;
1124 };
1125
1126 }
1127
1128=== modified file 'tests/include/mir_test_doubles/null_display_buffer.h'
1129--- tests/include/mir_test_doubles/null_display_buffer.h 2015-01-21 07:34:50 +0000
1130+++ tests/include/mir_test_doubles/null_display_buffer.h 2015-02-23 13:13:47 +0000
1131@@ -35,7 +35,6 @@
1132 void make_current() override {}
1133 void release_current() override {}
1134 void gl_swap_buffers() override {}
1135- void flip() override {}
1136 bool post_renderables_if_optimizable(graphics::RenderableList const&) override { return false; }
1137 MirOrientation orientation() const override { return mir_orientation_normal; }
1138 bool uses_alpha() const override { return false; }
1139
1140=== added file 'tests/include/mir_test_doubles/null_display_sync_group.h'
1141--- tests/include/mir_test_doubles/null_display_sync_group.h 1970-01-01 00:00:00 +0000
1142+++ tests/include/mir_test_doubles/null_display_sync_group.h 2015-02-23 13:13:47 +0000
1143@@ -0,0 +1,81 @@
1144+/*
1145+ * Copyright © 2015 Canonical Ltd.
1146+ *
1147+ * This program is free software: you can redistribute it and/or modify it
1148+ * under the terms of the GNU General Public License version 3,
1149+ * as published by the Free Software Foundation.
1150+ *
1151+ * This program is distributed in the hope that it will be useful,
1152+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
1153+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
1154+ * GNU General Public License for more details.
1155+ *
1156+ * You should have received a copy of the GNU General Public License
1157+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
1158+ *
1159+ * Authored by: Kevin DuBois <kevin.dubois@canonical.com>
1160+ */
1161+
1162+#ifndef MIR_TEST_DOUBLES_NULL_DISPLAY_SYNC_GROUP_H_
1163+#define MIR_TEST_DOUBLES_NULL_DISPLAY_SYNC_GROUP_H_
1164+
1165+#include "mir/graphics/display.h"
1166+#include "mir/geometry/size.h"
1167+#include "null_display_buffer.h"
1168+#include "stub_display_buffer.h"
1169+#include <thread>
1170+
1171+namespace mir
1172+{
1173+namespace test
1174+{
1175+namespace doubles
1176+{
1177+
1178+struct StubDisplaySyncGroup : graphics::DisplaySyncGroup
1179+{
1180+public:
1181+ StubDisplaySyncGroup(std::vector<geometry::Rectangle> const& output_rects)
1182+ : output_rects{output_rects}
1183+ {
1184+ for (auto const& output_rect : output_rects)
1185+ display_buffers.emplace_back(output_rect);
1186+ }
1187+ StubDisplaySyncGroup(geometry::Size sz) : StubDisplaySyncGroup({{{0,0}, sz}}) {}
1188+
1189+ void for_each_display_buffer(std::function<void(graphics::DisplayBuffer&)> const& f) override
1190+ {
1191+ for (auto& db : display_buffers)
1192+ f(db);
1193+ }
1194+
1195+ void post() override
1196+ {
1197+ /* yield() is needed to ensure reasonable runtime under valgrind for some tests */
1198+ std::this_thread::yield();
1199+ }
1200+
1201+private:
1202+ std::vector<geometry::Rectangle> const output_rects;
1203+ std::vector<StubDisplayBuffer> display_buffers;
1204+};
1205+
1206+struct NullDisplaySyncGroup : graphics::DisplaySyncGroup
1207+{
1208+ void for_each_display_buffer(std::function<void(graphics::DisplayBuffer&)> const& f) override
1209+ {
1210+ f(db);
1211+ }
1212+ virtual void post() override
1213+ {
1214+ /* yield() is needed to ensure reasonable runtime under valgrind for some tests */
1215+ std::this_thread::yield();
1216+ }
1217+ NullDisplayBuffer db;
1218+};
1219+
1220+}
1221+}
1222+}
1223+
1224+#endif /* MIR_TEST_DOUBLES_NULL_DISPLAY_SYNC_GROUP_H_ */
1225
1226=== modified file 'tests/include/mir_test_doubles/stub_display.h'
1227--- tests/include/mir_test_doubles/stub_display.h 2015-01-21 07:34:50 +0000
1228+++ tests/include/mir_test_doubles/stub_display.h 2015-02-23 13:13:47 +0000
1229@@ -20,6 +20,7 @@
1230 #define MIR_TEST_DOUBLES_STUB_DISPLAY_H_
1231
1232 #include "null_display.h"
1233+#include "null_display_sync_group.h"
1234 #include "stub_display_buffer.h"
1235 #include "stub_display_configuration.h"
1236
1237@@ -37,17 +38,22 @@
1238 class StubDisplay : public NullDisplay
1239 {
1240 public:
1241- StubDisplay(std::vector<geometry::Rectangle> const& output_rects)
1242- : output_rects{output_rects}
1243- {
1244- for (auto const& output_rect : output_rects)
1245- display_buffers.emplace_back(output_rect);
1246- }
1247-
1248- void for_each_display_buffer(std::function<void(graphics::DisplayBuffer&)> const& f) override
1249- {
1250- for (auto& db : display_buffers)
1251- f(db);
1252+ StubDisplay(std::vector<geometry::Rectangle> const& output_rects) :
1253+ output_rects(output_rects)
1254+ {
1255+ for (auto const& rect : output_rects)
1256+ groups.emplace_back(new StubDisplaySyncGroup({rect}));
1257+ }
1258+
1259+ StubDisplay(unsigned int nbuffers) :
1260+ StubDisplay(generate_stub_rects(nbuffers))
1261+ {
1262+ }
1263+
1264+ void for_each_display_sync_group(std::function<void(graphics::DisplaySyncGroup&)> const& f) override
1265+ {
1266+ for (auto& group : groups)
1267+ f(*group);
1268 }
1269
1270 std::unique_ptr<graphics::DisplayConfiguration> configuration() const override
1271@@ -59,7 +65,15 @@
1272
1273 std::vector<geometry::Rectangle> const output_rects;
1274 private:
1275- std::vector<StubDisplayBuffer> display_buffers;
1276+ std::vector<geometry::Rectangle> generate_stub_rects(unsigned int nbuffers)
1277+ {
1278+ std::vector<geometry::Rectangle> rects;
1279+ for (auto i = 0u; i < nbuffers; i++)
1280+ rects.push_back(geometry::Rectangle{{0,0},{1,1}});
1281+ return rects;
1282+ }
1283+
1284+ std::vector<std::unique_ptr<StubDisplaySyncGroup>> groups;
1285 };
1286
1287 }
1288
1289=== modified file 'tests/integration-tests/graphics/android/test_display_integration.cpp'
1290--- tests/integration-tests/graphics/android/test_display_integration.cpp 2015-02-13 06:12:34 +0000
1291+++ tests/integration-tests/graphics/android/test_display_integration.cpp 2015-02-23 13:13:47 +0000
1292@@ -81,34 +81,38 @@
1293
1294 TEST_F(AndroidDisplay, display_can_post)
1295 {
1296- display->for_each_display_buffer([](mg::DisplayBuffer& buffer)
1297- {
1298- buffer.make_current();
1299- md::glAnimationBasic gl_animation;
1300- gl_animation.init_gl();
1301-
1302- gl_animation.render_gl();
1303- buffer.gl_swap_buffers();
1304- buffer.flip();
1305-
1306- gl_animation.render_gl();
1307- buffer.gl_swap_buffers();
1308- buffer.flip();
1309+ display->for_each_display_sync_group([](mg::DisplaySyncGroup& group) {
1310+ group.for_each_display_buffer([](mg::DisplayBuffer& buffer)
1311+ {
1312+ buffer.make_current();
1313+ md::glAnimationBasic gl_animation;
1314+ gl_animation.init_gl();
1315+
1316+ gl_animation.render_gl();
1317+ buffer.gl_swap_buffers();
1318+
1319+ gl_animation.render_gl();
1320+ buffer.gl_swap_buffers();
1321+ });
1322+ group.post();
1323 });
1324 }
1325
1326 TEST_F(AndroidDisplay, display_can_post_overlay)
1327 {
1328- display->for_each_display_buffer([](mg::DisplayBuffer& db)
1329- {
1330- db.make_current();
1331- auto area = db.view_area();
1332- auto buffer = buffer_allocator->alloc_buffer_platform(
1333- area.size, mir_pixel_format_abgr_8888, mga::BufferUsage::use_hardware);
1334- mg::RenderableList list{
1335- std::make_shared<mtd::StubRenderable>(buffer, area)
1336- };
1337+ display->for_each_display_sync_group([](mg::DisplaySyncGroup& group) {
1338+ group.for_each_display_buffer([](mg::DisplayBuffer& db)
1339+ {
1340+ db.make_current();
1341+ auto area = db.view_area();
1342+ auto buffer = buffer_allocator->alloc_buffer_platform(
1343+ area.size, mir_pixel_format_abgr_8888, mga::BufferUsage::use_hardware);
1344+ mg::RenderableList list{
1345+ std::make_shared<mtd::StubRenderable>(buffer, area)
1346+ };
1347
1348- db.post_renderables_if_optimizable(list);
1349+ db.post_renderables_if_optimizable(list);
1350+ });
1351+ group.post();
1352 });
1353 }
1354
1355=== modified file 'tests/integration-tests/test_display_info.cpp'
1356--- tests/integration-tests/test_display_info.cpp 2015-01-21 07:34:50 +0000
1357+++ tests/integration-tests/test_display_info.cpp 2015-02-23 13:13:47 +0000
1358@@ -29,6 +29,7 @@
1359 #include "mir_test_doubles/null_display.h"
1360 #include "mir_test_doubles/null_event_sink.h"
1361 #include "mir_test_doubles/null_display_changer.h"
1362+#include "mir_test_doubles/null_display_sync_group.h"
1363 #include "mir_test_doubles/stub_display_buffer.h"
1364 #include "mir_test_doubles/null_platform.h"
1365 #include "mir_test/display_config_matchers.h"
1366@@ -61,13 +62,13 @@
1367 {
1368 }
1369
1370- void for_each_display_buffer(std::function<void(mg::DisplayBuffer&)> const& f) override
1371+ void for_each_display_sync_group(std::function<void(mg::DisplaySyncGroup&)> const& f) override
1372 {
1373- f(display_buffer);
1374+ f(display_sync_group);
1375 }
1376
1377 private:
1378- mtd::NullDisplayBuffer display_buffer;
1379+ mtd::NullDisplaySyncGroup display_sync_group;
1380 };
1381
1382 class StubChanger : public mtd::NullDisplayChanger
1383
1384=== modified file 'tests/integration-tests/test_display_server_main_loop_events.cpp'
1385--- tests/integration-tests/test_display_server_main_loop_events.cpp 2015-02-13 06:12:34 +0000
1386+++ tests/integration-tests/test_display_server_main_loop_events.cpp 2015-02-23 13:13:47 +0000
1387@@ -86,9 +86,9 @@
1388 {
1389 }
1390
1391- void for_each_display_buffer(std::function<void(mg::DisplayBuffer&)> const& f) override
1392+ void for_each_display_sync_group(std::function<void(mg::DisplaySyncGroup&)> const& f) override
1393 {
1394- display->for_each_display_buffer(f);
1395+ display->for_each_display_sync_group(f);
1396 }
1397
1398 std::unique_ptr<mg::DisplayConfiguration> configuration() const
1399
1400=== modified file 'tests/integration-tests/test_surface_first_frame_sync.cpp'
1401--- tests/integration-tests/test_surface_first_frame_sync.cpp 2015-02-13 06:12:34 +0000
1402+++ tests/integration-tests/test_surface_first_frame_sync.cpp 2015-02-23 13:13:47 +0000
1403@@ -56,32 +56,42 @@
1404 public:
1405 SynchronousCompositor(std::shared_ptr<mg::Display> const& display_,
1406 std::shared_ptr<mc::Scene> const& s,
1407- std::shared_ptr<mc::DisplayBufferCompositorFactory> const& db_compositor_factory)
1408+ std::shared_ptr<mc::DisplayBufferCompositorFactory> const& dbc_factory)
1409 : display{display_},
1410 scene{s}
1411 {
1412- display->for_each_display_buffer(
1413- [this, &db_compositor_factory](mg::DisplayBuffer& display_buffer)
1414+ display->for_each_display_sync_group([this, &dbc_factory](mg::DisplaySyncGroup& group)
1415+ {
1416+ group.for_each_display_buffer([this, &dbc_factory](mg::DisplayBuffer& display_buffer)
1417 {
1418- auto dbc = db_compositor_factory->create_compositor_for(display_buffer);
1419+ auto dbc = dbc_factory->create_compositor_for(display_buffer);
1420 scene->register_compositor(dbc.get());
1421 display_buffer_compositor_map[&display_buffer] = std::move(dbc);
1422 });
1423-
1424+ });
1425+
1426 auto notify = [this]()
1427 {
1428- display->for_each_display_buffer([this](mg::DisplayBuffer& display_buffer)
1429+ display->for_each_display_sync_group([this](mg::DisplaySyncGroup& group)
1430 {
1431- auto& dbc = display_buffer_compositor_map[&display_buffer];
1432- dbc->composite(scene->scene_elements_for(dbc.get()));
1433+ group.for_each_display_buffer([this](mg::DisplayBuffer& display_buffer)
1434+ {
1435+ auto& dbc = display_buffer_compositor_map[&display_buffer];
1436+ dbc->composite(scene->scene_elements_for(dbc.get()));
1437+ });
1438+ group.post();
1439 });
1440 };
1441 auto notify2 = [this](int)
1442 {
1443- display->for_each_display_buffer([this](mg::DisplayBuffer& display_buffer)
1444+ display->for_each_display_sync_group([this](mg::DisplaySyncGroup& group)
1445 {
1446- auto& dbc = display_buffer_compositor_map[&display_buffer];
1447- dbc->composite(scene->scene_elements_for(dbc.get()));
1448+ group.for_each_display_buffer([this](mg::DisplayBuffer& display_buffer)
1449+ {
1450+ auto& dbc = display_buffer_compositor_map[&display_buffer];
1451+ dbc->composite(scene->scene_elements_for(dbc.get()));
1452+ });
1453+ group.post();
1454 });
1455 };
1456 observer = std::make_shared<ms::LegacySceneChangeNotification>(notify, notify2);
1457
1458=== modified file 'tests/integration-tests/test_surface_stack_with_compositor.cpp'
1459--- tests/integration-tests/test_surface_stack_with_compositor.cpp 2015-02-12 22:31:44 +0000
1460+++ tests/integration-tests/test_surface_stack_with_compositor.cpp 2015-02-23 13:13:47 +0000
1461@@ -30,6 +30,8 @@
1462 #include "mir_test_doubles/stub_display_buffer.h"
1463 #include "mir_test_doubles/stub_buffer.h"
1464 #include "mir_test_doubles/stub_input_sender.h"
1465+#include "mir_test_doubles/null_surface_configurator.h"
1466+#include "mir_test_doubles/null_display_sync_group.h"
1467
1468 #include <condition_variable>
1469 #include <mutex>
1470@@ -56,23 +58,14 @@
1471 }
1472 };
1473
1474-struct CountingDisplayBuffer : public mtd::StubDisplayBuffer
1475+struct CountingDisplaySyncGroup : public mtd::StubDisplaySyncGroup
1476 {
1477- CountingDisplayBuffer() :
1478- StubDisplayBuffer({{0,0}, {10, 10}})
1479- {
1480- }
1481-
1482- bool post_renderables_if_optimizable(mg::RenderableList const&) override
1483- {
1484- return false;
1485- }
1486-
1487- void gl_swap_buffers() override
1488- {
1489- }
1490-
1491- void flip() override
1492+ CountingDisplaySyncGroup() :
1493+ mtd::StubDisplaySyncGroup({100,100})
1494+ {
1495+ }
1496+
1497+ void post() override
1498 {
1499 increment_post_count();
1500 }
1501@@ -101,19 +94,19 @@
1502
1503 struct StubDisplay : public mtd::NullDisplay
1504 {
1505- StubDisplay(mg::DisplayBuffer& primary, mg::DisplayBuffer& secondary)
1506+ StubDisplay(mg::DisplaySyncGroup& primary, mg::DisplaySyncGroup& secondary)
1507 : primary(primary),
1508 secondary(secondary)
1509 {
1510 }
1511- void for_each_display_buffer(std::function<void(mg::DisplayBuffer&)> const& fn) override
1512+ void for_each_display_sync_group(std::function<void(mg::DisplaySyncGroup&)> const& fn) override
1513 {
1514 fn(primary);
1515 fn(secondary);
1516 }
1517 private:
1518- mg::DisplayBuffer& primary;
1519- mg::DisplayBuffer& secondary;
1520+ mg::DisplaySyncGroup& primary;
1521+ mg::DisplaySyncGroup& secondary;
1522 };
1523
1524 struct SurfaceStackCompositor : public testing::Test
1525@@ -144,8 +137,8 @@
1526 std::shared_ptr<ms::BasicSurface> stub_surface;
1527 ms::SurfaceCreationParameters default_params;
1528 mtd::StubBuffer stubbuf;
1529- CountingDisplayBuffer stub_primary_db;
1530- CountingDisplayBuffer stub_secondary_db;
1531+ CountingDisplaySyncGroup stub_primary_db;
1532+ CountingDisplaySyncGroup stub_secondary_db;
1533 StubDisplay stub_display{stub_primary_db, stub_secondary_db};
1534 mc::DefaultDisplayBufferCompositorFactory dbc_factory{
1535 mt::fake_shared(renderer_factory),
1536
1537=== modified file 'tests/integration-tests/test_surfaceloop.cpp'
1538--- tests/integration-tests/test_surfaceloop.cpp 2015-01-21 07:34:50 +0000
1539+++ tests/integration-tests/test_surfaceloop.cpp 2015-02-23 13:13:47 +0000
1540@@ -22,6 +22,7 @@
1541 #include "mir_test_doubles/stub_buffer_allocator.h"
1542 #include "mir_test_doubles/null_platform.h"
1543 #include "mir_test_doubles/null_display.h"
1544+#include "mir_test_doubles/null_display_sync_group.h"
1545 #include "mir_test_doubles/stub_display_buffer.h"
1546
1547 #include "mir_test_framework/stubbed_server_configuration.h"
1548@@ -75,18 +76,18 @@
1549 class StubDisplay : public mtd::NullDisplay
1550 {
1551 public:
1552- StubDisplay()
1553- : display_buffer{geom::Rectangle{geom::Point{0,0}, geom::Size{1600,1600}}}
1554+ StubDisplay() :
1555+ display_sync_group{geom::Size{1600,1600}}
1556 {
1557 }
1558
1559- void for_each_display_buffer(std::function<void(mg::DisplayBuffer&)> const& f) override
1560+ void for_each_display_sync_group(std::function<void(mg::DisplaySyncGroup&)> const& f) override
1561 {
1562- f(display_buffer);
1563+ f(display_sync_group);
1564 }
1565
1566 private:
1567- mtd::StubDisplayBuffer display_buffer;
1568+ mtd::StubDisplaySyncGroup display_sync_group;
1569 };
1570
1571 struct SurfaceSync
1572
1573=== modified file 'tests/unit-tests/compositor/test_default_display_buffer_compositor.cpp'
1574--- tests/unit-tests/compositor/test_default_display_buffer_compositor.cpp 2015-01-21 09:03:53 +0000
1575+++ tests/unit-tests/compositor/test_default_display_buffer_compositor.cpp 2015-02-23 13:13:47 +0000
1576@@ -124,8 +124,6 @@
1577 .Times(1);
1578 EXPECT_CALL(display_buffer, gl_swap_buffers())
1579 .Times(1);
1580- EXPECT_CALL(display_buffer, flip())
1581- .Times(1);
1582
1583 mc::DefaultDisplayBufferCompositor compositor(
1584 display_buffer,
1585@@ -205,8 +203,6 @@
1586 .InSequence(render_seq);
1587 EXPECT_CALL(display_buffer, gl_swap_buffers())
1588 .InSequence(render_seq);
1589- EXPECT_CALL(display_buffer, flip())
1590- .InSequence(render_seq);
1591
1592 mc::DefaultDisplayBufferCompositor compositor(
1593 display_buffer,
1594@@ -242,8 +238,6 @@
1595 .InSequence(seq);
1596 EXPECT_CALL(display_buffer, gl_swap_buffers())
1597 .InSequence(seq);
1598- EXPECT_CALL(display_buffer, flip())
1599- .InSequence(seq);
1600
1601 EXPECT_CALL(display_buffer, post_renderables_if_optimizable(_))
1602 .InSequence(seq)
1603@@ -263,8 +257,6 @@
1604 .InSequence(seq);
1605 EXPECT_CALL(display_buffer, gl_swap_buffers())
1606 .InSequence(seq);
1607- EXPECT_CALL(display_buffer, flip())
1608- .InSequence(seq);
1609
1610 mc::DefaultDisplayBufferCompositor compositor(
1611 display_buffer,
1612@@ -302,8 +294,6 @@
1613 .InSequence(seq);
1614 EXPECT_CALL(display_buffer, gl_swap_buffers())
1615 .InSequence(seq);
1616- EXPECT_CALL(display_buffer, flip())
1617- .InSequence(seq);
1618
1619 mc::DefaultDisplayBufferCompositor compositor(
1620 display_buffer,
1621
1622=== modified file 'tests/unit-tests/compositor/test_multi_threaded_compositor.cpp'
1623--- tests/unit-tests/compositor/test_multi_threaded_compositor.cpp 2015-02-13 06:12:34 +0000
1624+++ tests/unit-tests/compositor/test_multi_threaded_compositor.cpp 2015-02-23 13:13:47 +0000
1625@@ -31,6 +31,7 @@
1626 #include "mir_test_doubles/mock_compositor_report.h"
1627 #include "mir_test_doubles/mock_scene.h"
1628 #include "mir_test_doubles/stub_scene.h"
1629+#include "mir_test_doubles/stub_display.h"
1630 #include "mir_test_doubles/null_display_buffer_compositor_factory.h"
1631
1632 #include <boost/throw_exception.hpp>
1633@@ -52,30 +53,12 @@
1634 namespace mtd = mir::test::doubles;
1635 namespace mt = mir::test;
1636
1637-namespace
1638-{
1639-
1640-class StubDisplay : public mtd::NullDisplay
1641-{
1642- public:
1643- StubDisplay(unsigned int nbuffers) : buffers{nbuffers} {}
1644-
1645- void for_each_display_buffer(std::function<void(mg::DisplayBuffer&)> const& f) override
1646- {
1647- for (auto& db : buffers)
1648- f(db);
1649- }
1650-
1651-private:
1652- std::vector<mtd::NullDisplayBuffer> buffers;
1653-};
1654-
1655 class StubDisplayWithMockBuffers : public mtd::NullDisplay
1656 {
1657- public:
1658+public:
1659 StubDisplayWithMockBuffers(unsigned int nbuffers) : buffers{nbuffers} {}
1660
1661- void for_each_display_buffer(std::function<void(mg::DisplayBuffer&)> const& f)
1662+ void for_each_display_sync_group(std::function<void(mg::DisplaySyncGroup&)> const& f) override
1663 {
1664 for (auto& db : buffers)
1665 f(db);
1666@@ -84,11 +67,21 @@
1667 void for_each_mock_buffer(std::function<void(mtd::MockDisplayBuffer&)> const& f)
1668 {
1669 for (auto& db : buffers)
1670- f(db);
1671+ f(db.buffer);
1672 }
1673
1674 private:
1675- std::vector<testing::NiceMock<mtd::MockDisplayBuffer>> buffers;
1676+ struct StubDisplaySyncGroup : mg::DisplaySyncGroup
1677+ {
1678+ void for_each_display_buffer(std::function<void(mg::DisplayBuffer&)> const& f) override
1679+ {
1680+ f(buffer);
1681+ }
1682+ void post() override {}
1683+ testing::NiceMock<mtd::MockDisplayBuffer> buffer;
1684+ };
1685+
1686+ std::vector<StubDisplaySyncGroup> buffers;
1687 };
1688
1689 class StubScene : public mtd::StubScene
1690@@ -337,6 +330,8 @@
1691 std::vector<std::string> thread_names;
1692 };
1693
1694+namespace
1695+{
1696 auto const null_report = mr::null_compositor_report();
1697 unsigned int const composites_per_update{1};
1698 }
1699@@ -347,7 +342,7 @@
1700
1701 unsigned int const nbuffers{3};
1702
1703- auto display = std::make_shared<StubDisplay>(nbuffers);
1704+ auto display = std::make_shared<mtd::StubDisplay>(nbuffers);
1705 auto scene = std::make_shared<StubScene>();
1706 auto db_compositor_factory = std::make_shared<RecordingDisplayBufferCompositorFactory>();
1707 mc::MultiThreadedCompositor compositor{display, scene, db_compositor_factory, null_report, true};
1708@@ -422,7 +417,7 @@
1709
1710 unsigned int const nbuffers = 3;
1711
1712- auto display = std::make_shared<StubDisplay>(nbuffers);
1713+ auto display = std::make_shared<mtd::StubDisplay>(nbuffers);
1714 auto scene = std::make_shared<StubScene>();
1715 auto db_compositor_factory = std::make_shared<RecordingDisplayBufferCompositorFactory>();
1716 mc::MultiThreadedCompositor compositor{display, scene, db_compositor_factory, null_report, true};
1717@@ -482,7 +477,7 @@
1718
1719 unsigned int const nbuffers = 3;
1720
1721- auto display = std::make_shared<StubDisplay>(nbuffers);
1722+ auto display = std::make_shared<mtd::StubDisplay>(nbuffers);
1723 auto scene = std::make_shared<StubScene>();
1724 auto factory = std::make_shared<RecordingDisplayBufferCompositorFactory>();
1725 mc::MultiThreadedCompositor compositor{display, scene, factory,
1726@@ -555,7 +550,7 @@
1727
1728 unsigned int const nbuffers = 3;
1729
1730- auto display = std::make_shared<StubDisplay>(nbuffers);
1731+ auto display = std::make_shared<mtd::StubDisplay>(nbuffers);
1732 auto scene = std::make_shared<StubScene>();
1733 auto db_compositor_factory = std::make_shared<RecordingDisplayBufferCompositorFactory>();
1734 mc::MultiThreadedCompositor compositor{display, scene, db_compositor_factory, null_report, false};
1735@@ -578,7 +573,7 @@
1736
1737 unsigned int const nbuffers = 3;
1738
1739- auto display = std::make_shared<StubDisplay>(nbuffers);
1740+ auto display = std::make_shared<mtd::StubDisplay>(nbuffers);
1741 auto scene = std::make_shared<StubScene>();
1742 auto db_compositor_factory = std::make_shared<RecordingDisplayBufferCompositorFactory>();
1743 mc::MultiThreadedCompositor compositor{display, scene, db_compositor_factory, null_report, false};
1744@@ -613,7 +608,7 @@
1745
1746 unsigned int const nbuffers{3};
1747
1748- auto display = std::make_shared<StubDisplay>(nbuffers);
1749+ auto display = std::make_shared<mtd::StubDisplay>(nbuffers);
1750 auto scene = std::make_shared<StubScene>();
1751 auto db_compositor_factory = std::make_shared<SurfaceUpdatingDisplayBufferCompositorFactory>(scene);
1752 mc::MultiThreadedCompositor compositor{display, scene, db_compositor_factory, null_report, true};
1753
1754=== modified file 'tests/unit-tests/graphics/android/test_display.cpp'
1755--- tests/unit-tests/graphics/android/test_display.cpp 2015-02-13 06:12:34 +0000
1756+++ tests/unit-tests/graphics/android/test_display.cpp 2015-02-23 13:13:47 +0000
1757@@ -656,25 +656,28 @@
1758 null_display_report,
1759 mga::OverlayOptimization::enabled);
1760
1761- auto db_count = 0;
1762- display.for_each_display_buffer([&](mg::DisplayBuffer&){ db_count++; });
1763- EXPECT_THAT(db_count, Eq(2));
1764+ auto group_count = 0;
1765+ auto db_group_counter = [&](mg::DisplaySyncGroup&) {
1766+ group_count++;
1767+ };
1768+ display.for_each_display_sync_group(db_group_counter);
1769+ EXPECT_THAT(group_count, Eq(2));
1770
1771 //hotplug external away
1772 external_connected = false;
1773 hotplug_fn();
1774
1775- db_count = 0;
1776- display.for_each_display_buffer([&](mg::DisplayBuffer&){ db_count++; });
1777- EXPECT_THAT(db_count, Eq(1));
1778+ group_count = 0;
1779+ display.for_each_display_sync_group(db_group_counter);
1780+ EXPECT_THAT(group_count, Eq(1));
1781
1782 //hotplug external back
1783 external_connected = true;
1784 hotplug_fn();
1785
1786- db_count = 0;
1787- display.for_each_display_buffer([&](mg::DisplayBuffer&){ db_count++; });
1788- EXPECT_THAT(db_count, Eq(2));
1789+ group_count = 0;
1790+ display.for_each_display_sync_group(db_group_counter);
1791+ EXPECT_THAT(group_count, Eq(2));
1792 }
1793
1794 TEST_F(Display, turns_external_display_on_with_hotplug)
1795@@ -715,12 +718,12 @@
1796 //hotplug external away
1797 external_connected = false;
1798 hotplug_fn();
1799- display.for_each_display_buffer([](mg::DisplayBuffer&){});
1800+ display.for_each_display_sync_group([](mg::DisplaySyncGroup&){});
1801
1802 //hotplug external back
1803 external_connected = true;
1804 hotplug_fn();
1805- display.for_each_display_buffer([](mg::DisplayBuffer&){});
1806+ display.for_each_display_sync_group([](mg::DisplaySyncGroup&){});
1807 }
1808
1809 TEST_F(Display, configures_external_display)
1810
1811=== modified file 'tests/unit-tests/graphics/android/test_display_buffer.cpp'
1812--- tests/unit-tests/graphics/android/test_display_buffer.cpp 2015-02-13 06:12:34 +0000
1813+++ tests/unit-tests/graphics/android/test_display_buffer.cpp 2015-02-23 13:13:47 +0000
1814@@ -104,7 +104,6 @@
1815 mga::OverlayOptimization::enabled};
1816
1817 db.gl_swap_buffers();
1818- db.flip();
1819 }
1820
1821 TEST_F(DisplayBuffer, posts_overlay_list_returns_display_device_decision)
1822
1823=== modified file 'tests/unit-tests/graphics/mesa/test_display.cpp'
1824--- tests/unit-tests/graphics/mesa/test_display.cpp 2015-01-22 09:00:14 +0000
1825+++ tests/unit-tests/graphics/mesa/test_display.cpp 2015-02-23 13:13:47 +0000
1826@@ -438,10 +438,11 @@
1827
1828 auto display = create_display(create_platform());
1829
1830- display->for_each_display_buffer([](mg::DisplayBuffer& db)
1831- {
1832- db.gl_swap_buffers();
1833- db.flip();
1834+ display->for_each_display_sync_group([](mg::DisplaySyncGroup& group) {
1835+ group.for_each_display_buffer([](mg::DisplayBuffer& db) {
1836+ db.gl_swap_buffers();
1837+ });
1838+ group.post();
1839 });
1840 }
1841
1842@@ -476,11 +477,11 @@
1843 EXPECT_THROW(
1844 {
1845 auto display = create_display(create_platform());
1846-
1847- display->for_each_display_buffer([](mg::DisplayBuffer& db)
1848- {
1849- db.gl_swap_buffers();
1850- db.flip();
1851+ display->for_each_display_sync_group([](mg::DisplaySyncGroup& group) {
1852+ group.for_each_display_buffer([](mg::DisplayBuffer& db) {
1853+ db.gl_swap_buffers();
1854+ });
1855+ group.post();
1856 });
1857 }, std::runtime_error);
1858 }
1859@@ -622,9 +623,10 @@
1860
1861 int callback_count{0};
1862
1863- display->for_each_display_buffer([&](mg::DisplayBuffer&)
1864- {
1865- callback_count++;
1866+ display->for_each_display_sync_group([&](mg::DisplaySyncGroup& group) {
1867+ group.for_each_display_buffer([&](mg::DisplayBuffer&) {
1868+ callback_count++;
1869+ });
1870 });
1871
1872 EXPECT_NE(0, callback_count);
1873
1874=== modified file 'tests/unit-tests/graphics/mesa/test_display_buffer.cpp'
1875--- tests/unit-tests/graphics/mesa/test_display_buffer.cpp 2015-02-12 22:46:39 +0000
1876+++ tests/unit-tests/graphics/mesa/test_display_buffer.cpp 2015-02-23 13:13:47 +0000
1877@@ -312,7 +312,7 @@
1878 mock_egl.fake_egl_context);
1879
1880 db.gl_swap_buffers();
1881- db.flip();
1882+ db.post();
1883 }
1884
1885 TEST_F(MesaDisplayBufferTest, single_mode_first_post_flips_with_wait)
1886@@ -333,7 +333,7 @@
1887 mock_egl.fake_egl_context);
1888
1889 db.gl_swap_buffers();
1890- db.flip();
1891+ db.post();
1892 }
1893
1894 TEST_F(MesaDisplayBufferTest, clone_mode_waits_for_page_flip_on_second_flip)
1895@@ -362,10 +362,10 @@
1896 mock_egl.fake_egl_context);
1897
1898 db.gl_swap_buffers();
1899- db.flip();
1900+ db.post();
1901
1902 db.gl_swap_buffers();
1903- db.flip();
1904+ db.post();
1905 }
1906
1907 TEST_F(MesaDisplayBufferTest, skips_bypass_because_of_incompatible_list)
1908
1909=== modified file 'tests/unit-tests/graphics/mesa/test_display_multi_monitor.cpp'
1910--- tests/unit-tests/graphics/mesa/test_display_multi_monitor.cpp 2015-02-13 06:12:34 +0000
1911+++ tests/unit-tests/graphics/mesa/test_display_multi_monitor.cpp 2015-02-23 13:13:47 +0000
1912@@ -358,18 +358,16 @@
1913 auto display = create_display_cloned(create_platform());
1914
1915 /* First frame: Page flips are scheduled, but not waited for */
1916- display->for_each_display_buffer([](mg::DisplayBuffer& buffer)
1917+ display->for_each_display_sync_group([](mg::DisplaySyncGroup& group)
1918 {
1919- buffer.gl_swap_buffers();
1920- buffer.flip();
1921+ group.post();
1922 });
1923
1924 /* Second frame: Previous page flips finish (drmHandleEvent) and new ones
1925 are scheduled */
1926- display->for_each_display_buffer([](mg::DisplayBuffer& buffer)
1927+ display->for_each_display_sync_group([](mg::DisplaySyncGroup& group)
1928 {
1929- buffer.gl_swap_buffers();
1930- buffer.flip();
1931+ group.post();
1932 });
1933 }
1934
1935
1936=== modified file 'tests/unit-tests/graphics/offscreen/test_offscreen_display.cpp'
1937--- tests/unit-tests/graphics/offscreen/test_offscreen_display.cpp 2015-02-13 06:12:34 +0000
1938+++ tests/unit-tests/graphics/offscreen/test_offscreen_display.cpp 2015-02-23 13:13:47 +0000
1939@@ -68,12 +68,12 @@
1940 mr::null_display_report()};
1941
1942 int count = 0;
1943- display.for_each_display_buffer(
1944- [&](mg::DisplayBuffer& db)
1945- {
1946+ display.for_each_display_sync_group([&](mg::DisplaySyncGroup& group) {
1947+ group.for_each_display_buffer([&](mg::DisplayBuffer& db) {
1948 ++count;
1949 EXPECT_EQ(mir_orientation_normal, db.orientation());
1950 });
1951+ });
1952
1953 EXPECT_TRUE(count);
1954 }
1955@@ -107,9 +107,8 @@
1956 Mock::VerifyAndClearExpectations(&mock_gl);
1957
1958 /* Binds the GL framebuffer objects */
1959- display.for_each_display_buffer(
1960- [this](mg::DisplayBuffer& db)
1961- {
1962+ display.for_each_display_sync_group([&](mg::DisplaySyncGroup& group) {
1963+ group.for_each_display_buffer([&](mg::DisplayBuffer& db) {
1964 EXPECT_CALL(mock_egl, eglMakeCurrent(_,_,_,Ne(EGL_NO_CONTEXT)));
1965 EXPECT_CALL(mock_gl, glBindFramebuffer(_,Ne(0)));
1966
1967@@ -118,13 +117,14 @@
1968 Mock::VerifyAndClearExpectations(&mock_egl);
1969 Mock::VerifyAndClearExpectations(&mock_gl);
1970 });
1971+ });
1972
1973 /* Contexts are released at teardown */
1974- display.for_each_display_buffer(
1975- [this](mg::DisplayBuffer&)
1976- {
1977+ display.for_each_display_sync_group([&](mg::DisplaySyncGroup& group) {
1978+ group.for_each_display_buffer([&](mg::DisplayBuffer&) {
1979 EXPECT_CALL(mock_egl, eglMakeCurrent(_,_,_,EGL_NO_CONTEXT));
1980 });
1981+ });
1982 }
1983
1984 TEST_F(OffscreenDisplayTest, restores_previous_state_on_fbo_setup_failure)
1985
1986=== modified file 'tests/unit-tests/graphics/test_display.cpp'
1987--- tests/unit-tests/graphics/test_display.cpp 2015-01-21 07:34:50 +0000
1988+++ tests/unit-tests/graphics/test_display.cpp 2015-02-23 13:13:47 +0000
1989@@ -191,7 +191,9 @@
1990 auto display = create_display();
1991 int db_count{0};
1992
1993- display->for_each_display_buffer([&] (mg::DisplayBuffer&) { ++db_count; });
1994+ display->for_each_display_sync_group([&](mg::DisplaySyncGroup& group) {
1995+ group.for_each_display_buffer([&] (mg::DisplayBuffer&) { ++db_count; });
1996+ });
1997 EXPECT_THAT(db_count, Eq(1));
1998
1999 auto conf = display->configuration();
2000@@ -204,6 +206,8 @@
2001 display->configure(*conf);
2002
2003 db_count = 0;
2004- display->for_each_display_buffer([&] (mg::DisplayBuffer&) { ++db_count; });
2005+ display->for_each_display_sync_group([&](mg::DisplaySyncGroup& group) {
2006+ group.for_each_display_buffer([&] (mg::DisplayBuffer&) { ++db_count; });
2007+ });
2008 EXPECT_THAT(db_count, Eq(0));
2009 }
2010
2011=== modified file 'tests/unit-tests/input/test_display_input_region.cpp'
2012--- tests/unit-tests/input/test_display_input_region.cpp 2014-03-06 06:05:17 +0000
2013+++ tests/unit-tests/input/test_display_input_region.cpp 2015-02-23 13:13:47 +0000
2014@@ -19,7 +19,7 @@
2015 #include "src/server/input/display_input_region.h"
2016
2017 #include "mir_test_doubles/null_display.h"
2018-#include "mir_test_doubles/stub_display_buffer.h"
2019+#include "mir_test_doubles/stub_display.h"
2020
2021 #include <vector>
2022 #include <tuple>
2023@@ -33,35 +33,17 @@
2024
2025 namespace
2026 {
2027-
2028-class StubDisplay : public mtd::NullDisplay
2029-{
2030-public:
2031- StubDisplay()
2032- : display_buffers{
2033- mtd::StubDisplayBuffer{geom::Rectangle{geom::Point{0,0}, geom::Size{800,600}}},
2034- mtd::StubDisplayBuffer{geom::Rectangle{geom::Point{0,600}, geom::Size{100,100}}},
2035- mtd::StubDisplayBuffer{geom::Rectangle{geom::Point{800,0}, geom::Size{100,100}}}}
2036- {
2037-
2038- }
2039-
2040- void for_each_display_buffer(std::function<void(mg::DisplayBuffer&)> const& f) override
2041- {
2042- for (auto& db : display_buffers)
2043- f(db);
2044- }
2045-
2046-private:
2047- std::vector<mtd::StubDisplayBuffer> display_buffers;
2048+std::vector<geom::Rectangle> const rects{
2049+ geom::Rectangle{{0,0}, {800,600}},
2050+ geom::Rectangle{{0,600}, {100,100}},
2051+ geom::Rectangle{{800,0}, {100,100}}
2052 };
2053-
2054 }
2055
2056 TEST(DisplayInputRegionTest, returns_correct_bounding_rectangle)
2057 {
2058 geom::Rectangle const expected_bounding_rect{geom::Point{0,0}, geom::Size{900,700}};
2059- auto stub_display = std::make_shared<StubDisplay>();
2060+ auto stub_display = std::make_shared<mtd::StubDisplay>(rects);
2061
2062 mi::DisplayInputRegion input_region{stub_display};
2063
2064@@ -71,7 +53,7 @@
2065
2066 TEST(DisplayInputRegionTest, confines_point_to_closest_valid_position)
2067 {
2068- auto stub_display = std::make_shared<StubDisplay>();
2069+ auto stub_display = std::make_shared<mtd::StubDisplay>(rects);
2070
2071 mi::DisplayInputRegion input_region{stub_display};
2072

Subscribers

People subscribed via source and target branches