Mir

Merge lp:~alan-griffiths/mir/refactoring-so-SwitchingBundle-can-control-completion-of-client_acquire into lp:mir

  1. refactoring-so-SwitchingBundle-can-control-completion-of-client_acquire
  2. Merge into development-branch
Proposed by Alan Griffiths
Status: Merged
Approved by: Alan Griffiths
Approved revision: no longer in the source branch.
Merged at revision: 1370
Proposed branch: lp:~alan-griffiths/mir/refactoring-so-SwitchingBundle-can-control-completion-of-client_acquire
Merge into: lp:mir
Diff against target: 1466 lines (+436/-205)
33 files modified
examples/render_surfaces.cpp (+3/-2)
include/server/mir/compositor/buffer_stream.h (+1/-1)
include/server/mir/frontend/surface.h (+8/-1)
include/test/mir_test_doubles/mock_buffer_bundle.h (+1/-1)
include/test/mir_test_doubles/mock_buffer_stream.h (+1/-1)
include/test/mir_test_doubles/mock_frontend_surface.h (+1/-1)
include/test/mir_test_doubles/stub_buffer_stream.h (+2/-2)
src/server/compositor/buffer_bundle.h (+1/-1)
src/server/compositor/buffer_stream_surfaces.cpp (+5/-4)
src/server/compositor/buffer_stream_surfaces.h (+1/-1)
src/server/compositor/switching_bundle.cpp (+2/-2)
src/server/compositor/switching_bundle.h (+1/-1)
src/server/frontend/protobuf_message_processor.cpp (+46/-0)
src/server/frontend/protobuf_message_processor.h (+1/-0)
src/server/frontend/session_mediator.cpp (+82/-77)
src/server/frontend/session_mediator.h (+2/-1)
src/server/frontend/surface.cpp (+25/-1)
src/server/scene/basic_surface.cpp (+3/-3)
src/server/scene/basic_surface.h (+1/-1)
src/server/scene/surface_impl.cpp (+2/-2)
src/server/scene/surface_impl.h (+1/-1)
tests/integration-tests/compositor/test_buffer_stream.cpp (+47/-21)
tests/integration-tests/compositor/test_swapping_swappers.cpp (+27/-2)
tests/integration-tests/graphics/android/test_buffer_integration.cpp (+25/-1)
tests/integration-tests/test_session.cpp (+1/-1)
tests/integration-tests/test_swapinterval.cpp (+15/-7)
tests/unit-tests/compositor/test_buffer_stream.cpp (+39/-9)
tests/unit-tests/compositor/test_switching_bundle.cpp (+58/-30)
tests/unit-tests/compositor/test_temporary_buffers.cpp (+2/-2)
tests/unit-tests/frontend/test_session_mediator.cpp (+10/-6)
tests/unit-tests/scene/test_surface.cpp (+19/-20)
tests/unit-tests/scene/test_surface_impl.cpp (+2/-1)
tests/unit-tests/scene/test_surface_stack.cpp (+1/-1)
To merge this branch: bzr merge lp:~alan-griffiths/mir/refactoring-so-SwitchingBundle-can-control-completion-of-client_acquire
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
Alexandros Frantzis (community) Approve
Kevin DuBois (community) Approve
Daniel van Vugt Needs Fixing
Review via email: mp+204244@code.launchpad.net

Commit message

frontend, compositor, scene: refactoring code to support a coming change to next_buffer implementation.

Description of the change

frontend, compositor, scene: refactoring code to support a coming change to next_buffer implementation.

This is the first of three passes over the the code - it is a pure refactoring and changes no functionality.

In the next pass I intend to change client_acquire to be non-blocking (as we are running on a frontend thread and need to return)

The final pass will be to clean up some of the mess left in the code - in particular we can get rid of some duplication this MP introduces and kill the (by then) legacy "force_requests_to_complete" code that forces unblocking of frontend threads hung in client_acquire.

I'm splitting it this way as it is easier to review in these chunks

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:1370
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/755/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-trusty-i386-build/775
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-trusty-amd64-build/772
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-trusty-touch/376
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/485/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/489
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/489/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/377
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/377/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/379
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/3534

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/755/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

"146/173 Test #146: memcheck(mir_unit_tests.SessionMediatorTest.*) .......................................***Failed 8.20 "

Rats! Can't reproduce quickly - will look again on Monday.

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

nit:
funny spacing around l518 (4 spaces, 8 spaces, 9spaces).

isnt the loop with the predicate lambda redundant?
628 + while (!done)
629 + cv.wait(lock, [&]{ return done; });

question:
This might be addressed in one of the follow ups, but

virtual void swap_client_buffers(graphics::Buffer*& buffer, std::function<void()> complete)

seems like its trying to mix a callback with a swapping function?

virtual void submit_produced_buffer_and_notify_on_available_buffer(graphics::Buffer* buffer, std::function<void(Buffer*)> new_buffer_callback)

makes a little more sense to me, because you're 'moving' the old buffer into the function, and then getting a new one via a callback. (naming is suspect :) )

alternatively,

set_available_buffer_callback(std::function<void(Buffer*)> new_buffer_callback);
submit_produced_buffer(Buffer*);

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

Interesting approach. I would not have immediately thought of moving the asynchronous logic all the way down to SwitchingBundle, but that makes sense.

I'm slightly concerned about the use of lambdas or callbacks though, because this means we don't really know what thread we're going to be called back in. At least not from an interface level. Seems overcomplicated in parts, which makes me further unsure.

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

I feel this is potentially a very difficult design to ensure thread safety for:

BufferStream:
    void swap_buffers(graphics::Buffer*&, std::function<void()> complete)

Because "complete" will likely be called from a different thread (a compositor thread) to that which is passing the buffers in and out (a frontend/protocol thread). Or did I miss something about the intended usage pattern?

You seem to have a better design in place for BufferBundle:
    void client_acquire(std::function<void(graphics::Buffer* buffer)> complete)

Why are they different?

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

It could be cleaner to register a callback once, to repeatedly receive all client-buffer-ready notifications. That would firstly simplify some code, only registering a callback once. Secondly you need to get away from calling client_acquire repeatedly in order to eventually resolve bug 1253868. Because we will need to notify clients of new buffers (multiple) being ready _before_ they ask for them (in fact they shouldn't ask at all eventually, they should be events). Waiting until you receive a request for a new buffer is too late, and too slow, limiting frame rates by being synchronously bound to protocol round trips plus context switching times on the ends.

I suggest for SwitchingBundle:
  1. Keep client_acquire() blocking, unchanged; and
  2. Add a new function like "on_client_buffer_ready()" which is called whenever it's guaranteed that client_acquire won't block.

That would be much more flexible.

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

Nits fixed. (And hopefully the test failure too.)

> question:
> This might be addressed in one of the follow ups, but
>
> virtual void swap_client_buffers(graphics::Buffer*& buffer,
> std::function<void()> complete)
>
> seems like its trying to mix a callback with a swapping function?

Yeah, I'm not entirely happy with that interface either.

I was hoping that a better idea would emerge as I clean up the code. But as you and Daniel have both commented on this I'll take another look.

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

> It could be cleaner to register a callback once, to repeatedly receive all
> client-buffer-ready notifications. That would firstly simplify some code,

The callbacks are carrying quite a bit of context relating to the stack of initial calls:

383 + [lock, this, response, done, session](graphics::Buffer* client_buffer, bool need_full_ipc)

297 + [&tracker, &client_buffer, complete]{

140 + [&buffer,complete](mg::Buffer* new_buffer)

New objects to carry this context into registered callbacks would be needed for this approach. I'm not convinced that it would be simpler.

> only registering a callback once. Secondly you need to get away from calling
> client_acquire repeatedly in order to eventually resolve bug 1253868. Because
> we will need to notify clients of new buffers (multiple) being ready _before_
> they ask for them (in fact they shouldn't ask at all eventually, they should
> be events). Waiting until you receive a request for a new buffer is too late,
> and too slow, limiting frame rates by being synchronously bound to protocol
> round trips plus context switching times on the ends.

It would be nice to address bug 1253868 but I don't feel that it really falls into the scope of the current changes.

> I suggest for SwitchingBundle:
> 1. Keep client_acquire() blocking, unchanged; and
> 2. Add a new function like "on_client_buffer_ready()" which is called
> whenever it's guaranteed that client_acquire won't block.
>
> That would be much more flexible.

I'm willing to be convinced - but I feel a design that avoids blocking is more flexible.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:1369
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/767/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-trusty-i386-build/793
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-trusty-amd64-build/790
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-trusty-touch/389
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/497
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/497/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/501
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/501/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/390
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/390/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/392
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/3587

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/767/rebuild

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

PASSED: Continuous integration, rev:1372
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/768/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-trusty-i386-build/796
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-trusty-amd64-build/793
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-trusty-touch/391
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/498
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/498/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/502
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/502/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/392
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/392/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/394
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/3606

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/768/rebuild

review: Approve (continuous-integration)
Revision history for this message
Kevin DuBois (kdub) wrote :

lgtm now, +1

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

I am OK with the proposed approach.

I would also like us to evaluate using a push mechanism for buffers at some point, but that's too radical a step to be taken lightly (and we want a solution for blocking clients soon).

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:1375
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/780/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-trusty-i386-build/810
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-trusty-amd64-build/807
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-trusty-touch/405/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/510/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/514
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/514/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/406/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/780/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

"Build timed out (after 120 minutes). Marking the build as failed." - looked like it was *almost* finished when time ran out.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:1375
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/785/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-android-trusty-i386-build/816/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-trusty-amd64-build/813/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-trusty-touch/411/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/515/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/519/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/412/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/785/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:1376
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/792/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-trusty-i386-build/823
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-trusty-amd64-build/820
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-trusty-touch/418
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/522
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/522/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/526
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/526/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/419
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/419/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/411
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/3679

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/792/rebuild

review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'examples/render_surfaces.cpp'
2--- examples/render_surfaces.cpp 2014-01-29 12:51:30 +0000
3+++ examples/render_surfaces.cpp 2014-02-06 09:52:02 +0000
4@@ -419,8 +419,9 @@
5 */
6 {
7 mg::Buffer* buffer{nullptr};
8- s->swap_buffers(buffer);
9- s->swap_buffers(buffer);
10+ auto const complete = [&](mg::Buffer* new_buf){ buffer = new_buf; };
11+ s->swap_buffers(buffer, complete);
12+ s->swap_buffers(buffer, complete);
13 }
14
15 /*
16
17=== modified file 'include/server/mir/compositor/buffer_stream.h'
18--- include/server/mir/compositor/buffer_stream.h 2014-01-13 06:12:33 +0000
19+++ include/server/mir/compositor/buffer_stream.h 2014-02-06 09:52:02 +0000
20@@ -41,7 +41,7 @@
21 public:
22 virtual ~BufferStream() = default;
23
24- virtual void swap_client_buffers(graphics::Buffer*& buffer) = 0;
25+ virtual void swap_client_buffers(graphics::Buffer* old_buffer, std::function<void(graphics::Buffer* new_buffer)> complete) = 0;
26 virtual std::shared_ptr<graphics::Buffer>
27 lock_compositor_buffer(unsigned long frameno) = 0;
28 virtual std::shared_ptr<graphics::Buffer> lock_snapshot_buffer() = 0;
29
30=== modified file 'include/server/mir/frontend/surface.h'
31--- include/server/mir/frontend/surface.h 2014-01-13 06:12:33 +0000
32+++ include/server/mir/frontend/surface.h 2014-02-06 09:52:02 +0000
33@@ -56,13 +56,20 @@
34 virtual geometry::Size size() const = 0;
35 virtual MirPixelFormat pixel_format() const = 0;
36
37- virtual void swap_buffers(graphics::Buffer*&) = 0;
38+ virtual void swap_buffers(graphics::Buffer* old_buffer, std::function<void(graphics::Buffer* new_buffer)> complete) = 0;
39
40 virtual bool supports_input() const = 0;
41 virtual int client_input_fd() const = 0;
42
43 virtual int configure(MirSurfaceAttrib attrib, int value) = 0;
44
45+ /**
46+ * swap_buffers_blocking() is a convenience wrapper around swap_buffers()
47+ * it forces the current thread to block until complete() is called.
48+ * Use with care!
49+ */
50+ void swap_buffers_blocking(graphics::Buffer*& buffer);
51+
52 protected:
53 Surface() = default;
54 Surface(Surface const&) = delete;
55
56=== modified file 'include/test/mir_test_doubles/mock_buffer_bundle.h'
57--- include/test/mir_test_doubles/mock_buffer_bundle.h 2014-01-13 06:12:33 +0000
58+++ include/test/mir_test_doubles/mock_buffer_bundle.h 2014-02-06 09:52:02 +0000
59@@ -37,7 +37,7 @@
60 ~MockBufferBundle() noexcept
61 {}
62
63- MOCK_METHOD0(client_acquire, graphics::Buffer*());
64+ MOCK_METHOD1(client_acquire, void(std::function<void(graphics::Buffer*)>));
65 MOCK_METHOD1(client_release, void(graphics::Buffer*));
66 MOCK_METHOD1(compositor_acquire, std::shared_ptr<graphics::Buffer>(unsigned long));
67 MOCK_METHOD1(compositor_release, void(std::shared_ptr<graphics::Buffer> const&));
68
69=== modified file 'include/test/mir_test_doubles/mock_buffer_stream.h'
70--- include/test/mir_test_doubles/mock_buffer_stream.h 2014-01-13 06:12:33 +0000
71+++ include/test/mir_test_doubles/mock_buffer_stream.h 2014-02-06 09:52:02 +0000
72@@ -31,7 +31,7 @@
73 {
74 struct MockBufferStream : public compositor::BufferStream
75 {
76- MOCK_METHOD1(swap_client_buffers, void(graphics::Buffer*&));
77+ MOCK_METHOD2(swap_client_buffers, void(graphics::Buffer*, std::function<void(graphics::Buffer*)> completee));
78 MOCK_METHOD1(lock_compositor_buffer,
79 std::shared_ptr<graphics::Buffer>(unsigned long));
80 MOCK_METHOD0(lock_snapshot_buffer, std::shared_ptr<graphics::Buffer>());
81
82=== modified file 'include/test/mir_test_doubles/mock_frontend_surface.h'
83--- include/test/mir_test_doubles/mock_frontend_surface.h 2014-01-13 06:12:33 +0000
84+++ include/test/mir_test_doubles/mock_frontend_surface.h 2014-02-06 09:52:02 +0000
85@@ -39,7 +39,7 @@
86
87 MOCK_METHOD0(destroy, void());
88 MOCK_METHOD0(force_requests_to_complete, void());
89- MOCK_METHOD1(swap_buffers, void(graphics::Buffer*&));
90+ MOCK_METHOD2(swap_buffers, void(graphics::Buffer*, std::function<void(graphics::Buffer*)> complete));
91
92 MOCK_CONST_METHOD0(size, geometry::Size());
93 MOCK_CONST_METHOD0(pixel_format, MirPixelFormat());
94
95=== modified file 'include/test/mir_test_doubles/stub_buffer_stream.h'
96--- include/test/mir_test_doubles/stub_buffer_stream.h 2014-01-13 06:12:33 +0000
97+++ include/test/mir_test_doubles/stub_buffer_stream.h 2014-02-06 09:52:02 +0000
98@@ -36,9 +36,9 @@
99 {
100 stub_compositor_buffer = std::make_shared<StubBuffer>();
101 }
102- void swap_client_buffers(graphics::Buffer*& buffer) override
103+ void swap_client_buffers(graphics::Buffer*, std::function<void(graphics::Buffer* new_buffer)> complete) override
104 {
105- buffer = &stub_client_buffer;
106+ complete(&stub_client_buffer);
107 }
108 std::shared_ptr<graphics::Buffer> lock_compositor_buffer(unsigned long) override
109 {
110
111=== modified file 'src/server/compositor/buffer_bundle.h'
112--- src/server/compositor/buffer_bundle.h 2014-01-13 06:12:33 +0000
113+++ src/server/compositor/buffer_bundle.h 2014-02-06 09:52:02 +0000
114@@ -34,7 +34,7 @@
115 {
116 public:
117 virtual ~BufferBundle() noexcept {}
118- virtual graphics::Buffer* client_acquire() = 0;
119+ virtual void client_acquire(std::function<void(graphics::Buffer* buffer)> complete) = 0;
120 virtual void client_release(graphics::Buffer*) = 0;
121 virtual std::shared_ptr<graphics::Buffer>
122 compositor_acquire(unsigned long frameno) = 0;
123
124=== modified file 'src/server/compositor/buffer_stream_surfaces.cpp'
125--- src/server/compositor/buffer_stream_surfaces.cpp 2014-01-13 06:12:33 +0000
126+++ src/server/compositor/buffer_stream_surfaces.cpp 2014-02-06 09:52:02 +0000
127@@ -49,13 +49,14 @@
128 return std::make_shared<mc::TemporarySnapshotBuffer>(buffer_bundle);
129 }
130
131-void mc::BufferStreamSurfaces::swap_client_buffers(mg::Buffer*& buffer)
132+void mc::BufferStreamSurfaces::swap_client_buffers(graphics::Buffer* old_buffer, std::function<void(graphics::Buffer* new_buffer)> complete)
133 {
134- if (buffer)
135+ if (old_buffer)
136 {
137- buffer_bundle->client_release(buffer);
138+ buffer_bundle->client_release(old_buffer);
139 }
140- buffer = buffer_bundle->client_acquire();
141+
142+ buffer_bundle->client_acquire(complete);
143 }
144
145 MirPixelFormat mc::BufferStreamSurfaces::get_stream_pixel_format()
146
147=== modified file 'src/server/compositor/buffer_stream_surfaces.h'
148--- src/server/compositor/buffer_stream_surfaces.h 2014-01-13 06:12:33 +0000
149+++ src/server/compositor/buffer_stream_surfaces.h 2014-02-06 09:52:02 +0000
150@@ -39,7 +39,7 @@
151 BufferStreamSurfaces(std::shared_ptr<BufferBundle> const& swapper);
152 ~BufferStreamSurfaces();
153
154- void swap_client_buffers(graphics::Buffer*& buffer) override;
155+ void swap_client_buffers(graphics::Buffer* old_buffer, std::function<void(graphics::Buffer* new_buffer)> complete) override;
156
157 std::shared_ptr<graphics::Buffer>
158 lock_compositor_buffer(unsigned long frameno) override;
159
160=== modified file 'src/server/compositor/switching_bundle.cpp'
161--- src/server/compositor/switching_bundle.cpp 2014-01-23 18:42:20 +0000
162+++ src/server/compositor/switching_bundle.cpp 2014-02-06 09:52:02 +0000
163@@ -177,7 +177,7 @@
164 return ring[slot].buf;
165 }
166
167-mg::Buffer* mc::SwitchingBundle::client_acquire()
168+void mc::SwitchingBundle::client_acquire(std::function<void(graphics::Buffer* buffer)> complete)
169 {
170 std::unique_lock<std::mutex> lock(guard);
171
172@@ -247,7 +247,7 @@
173 ring[client].buf = ret;
174 }
175
176- return ret.get();
177+ complete(ret.get());
178 }
179
180 void mc::SwitchingBundle::client_release(graphics::Buffer* released_buffer)
181
182=== modified file 'src/server/compositor/switching_bundle.h'
183--- src/server/compositor/switching_bundle.h 2014-01-23 18:42:20 +0000
184+++ src/server/compositor/switching_bundle.h 2014-02-06 09:52:02 +0000
185@@ -49,7 +49,7 @@
186
187 graphics::BufferProperties properties() const;
188
189- graphics::Buffer* client_acquire();
190+ void client_acquire(std::function<void(graphics::Buffer* buffer)> complete) override;
191 void client_release(graphics::Buffer* buffer);
192 std::shared_ptr<graphics::Buffer>
193 compositor_acquire(unsigned long frameno) override;
194
195=== modified file 'src/server/frontend/protobuf_message_processor.cpp'
196--- src/server/frontend/protobuf_message_processor.cpp 2014-02-05 15:37:16 +0000
197+++ src/server/frontend/protobuf_message_processor.cpp 2014-02-06 09:52:02 +0000
198@@ -57,6 +57,47 @@
199 template<> struct result_ptr_t<::mir::protobuf::Connection> { typedef ::mir::protobuf::Connection* type; };
200 template<> struct result_ptr_t<::mir::protobuf::Surface> { typedef ::mir::protobuf::Surface* type; };
201 template<> struct result_ptr_t<::mir::protobuf::Screencast> { typedef ::mir::protobuf::Screencast* type; };
202+
203+template<>
204+void invoke(
205+ ProtobufMessageProcessor* self,
206+ protobuf::DisplayServer* server,
207+ void (protobuf::DisplayServer::*function)(
208+ ::google::protobuf::RpcController* controller,
209+ const protobuf::SurfaceId* request,
210+ protobuf::Buffer* response,
211+ ::google::protobuf::Closure* done),
212+ Invocation const& invocation)
213+{
214+ protobuf::SurfaceId parameter_message;
215+ parameter_message.ParseFromString(invocation.parameters());
216+ auto const result_message = std::make_shared<protobuf::Buffer>();
217+
218+ auto const callback =
219+ google::protobuf::NewCallback<
220+ ProtobufMessageProcessor,
221+ ::google::protobuf::uint32,
222+ std::shared_ptr<protobuf::Buffer>>(
223+ self,
224+ &ProtobufMessageProcessor::send_response,
225+ invocation.id(),
226+ result_message);
227+
228+ try
229+ {
230+ (server->*function)(
231+ 0,
232+ &parameter_message,
233+ result_message.get(),
234+ callback);
235+ }
236+ catch (std::exception const& x)
237+ {
238+ delete callback;
239+ result_message->set_error(boost::diagnostic_information(x));
240+ self->send_response(invocation.id(), result_message);
241+ }
242+}
243 }
244 }
245 }
246@@ -165,6 +206,11 @@
247 sender->send_response(id, response, {fd});
248 }
249
250+void mfd::ProtobufMessageProcessor::send_response(::google::protobuf::uint32 id, std::shared_ptr<protobuf::Buffer> response)
251+{
252+ send_response(id, response.get());
253+}
254+
255 void mfd::ProtobufMessageProcessor::send_response(::google::protobuf::uint32 id, mir::protobuf::Connection* response)
256 {
257 const auto& fd = response->has_platform() ?
258
259=== modified file 'src/server/frontend/protobuf_message_processor.h'
260--- src/server/frontend/protobuf_message_processor.h 2014-02-05 15:37:16 +0000
261+++ src/server/frontend/protobuf_message_processor.h 2014-02-06 09:52:02 +0000
262@@ -52,6 +52,7 @@
263 void send_response(::google::protobuf::uint32 id, protobuf::Buffer* response);
264 void send_response(::google::protobuf::uint32 id, protobuf::Connection* response);
265 void send_response(::google::protobuf::uint32 id, protobuf::Surface* response);
266+ void send_response(::google::protobuf::uint32 id, std::shared_ptr<protobuf::Buffer> response);
267 void send_response(::google::protobuf::uint32 id, mir::protobuf::Screencast* response);
268
269 private:
270
271=== modified file 'src/server/frontend/session_mediator.cpp'
272--- src/server/frontend/session_mediator.cpp 2014-01-29 18:02:33 +0000
273+++ src/server/frontend/session_mediator.cpp 2014-02-06 09:52:02 +0000
274@@ -119,19 +119,25 @@
275 done->Run();
276 }
277
278-std::tuple<mg::Buffer*, bool>
279-mf::SessionMediator::advance_buffer(SurfaceId surf_id, Surface& surface)
280+void mf::SessionMediator::advance_buffer(
281+ SurfaceId surf_id,
282+ Surface& surface,
283+ std::function<void(graphics::Buffer*, bool)> complete)
284 {
285 auto& tracker = client_buffer_tracker[surf_id];
286 if (!tracker) tracker = std::make_shared<ClientBufferTracker>(client_buffer_cache_size);
287
288 auto& client_buffer = client_buffer_resource[surf_id];
289- surface.swap_buffers(client_buffer);
290- auto id = client_buffer->id();
291- auto need_full_ipc = !tracker->client_has(id);
292- tracker->add(id);
293+ surface.swap_buffers(client_buffer,
294+ [&tracker, &client_buffer, complete](mg::Buffer* new_buffer)
295+ {
296+ client_buffer = new_buffer;
297+ auto id = client_buffer->id();
298+ auto need_full_ipc = !tracker->client_has(id);
299+ tracker->add(id);
300
301- return std::tie(client_buffer, need_full_ipc);
302+ complete(client_buffer, need_full_ipc);
303+ });
304 }
305
306
307@@ -141,49 +147,48 @@
308 mir::protobuf::Surface* response,
309 google::protobuf::Closure* done)
310 {
311- bool need_full_ipc;
312- graphics::Buffer* client_buffer{nullptr};
313- std::shared_ptr<Session> session;
314-
315- {
316- std::unique_lock<std::mutex> lock(session_mutex);
317-
318- session = weak_session.lock();
319-
320- if (session.get() == nullptr)
321- BOOST_THROW_EXCEPTION(std::logic_error("Invalid application session"));
322-
323- report->session_create_surface_called(session->name());
324-
325- auto const surf_id = session->create_surface(msh::SurfaceCreationParameters()
326- .of_name(request->surface_name())
327- .of_size(request->width(), request->height())
328- .of_buffer_usage(static_cast<graphics::BufferUsage>(request->buffer_usage()))
329- .of_pixel_format(static_cast<MirPixelFormat>(request->pixel_format()))
330- .with_output_id(graphics::DisplayConfigurationOutputId(request->output_id())));
331-
332- auto surface = session->get_surface(surf_id);
333- response->mutable_id()->set_value(surf_id.as_value());
334- response->set_width(surface->size().width.as_uint32_t());
335- response->set_height(surface->size().height.as_uint32_t());
336- response->set_pixel_format((int)surface->pixel_format());
337- response->set_buffer_usage(request->buffer_usage());
338-
339- if (surface->supports_input())
340- response->add_fd(surface->client_input_fd());
341-
342- std::tie(client_buffer, need_full_ipc) = advance_buffer(surf_id, *surface);
343- }
344-
345- auto buffer = response->mutable_buffer();
346- pack_protobuf_buffer(*buffer, client_buffer, need_full_ipc);
347-
348- // TODO: NOTE: We use the ordering here to ensure the shell acts on the surface after the surface ID is sent over the wire.
349- // This guarantees that notifications such as, gained focus, etc, can be correctly interpreted by the client.
350- // To achieve this order we rely on done->Run() sending messages synchronously. As documented in mfd::SocketMessenger::send.
351- // this will require additional synchronization if mfd::SocketMessenger::send changes.
352- done->Run();
353- shell->handle_surface_created(session);
354+
355+ auto const lock = std::make_shared<std::unique_lock<std::mutex>>(session_mutex);
356+
357+ auto const session = weak_session.lock();
358+
359+ if (session.get() == nullptr)
360+ BOOST_THROW_EXCEPTION(std::logic_error("Invalid application session"));
361+
362+ report->session_create_surface_called(session->name());
363+
364+ auto const surf_id = session->create_surface(msh::SurfaceCreationParameters()
365+ .of_name(request->surface_name())
366+ .of_size(request->width(), request->height())
367+ .of_buffer_usage(static_cast<graphics::BufferUsage>(request->buffer_usage()))
368+ .of_pixel_format(static_cast<MirPixelFormat>(request->pixel_format()))
369+ .with_output_id(graphics::DisplayConfigurationOutputId(request->output_id())));
370+
371+ auto surface = session->get_surface(surf_id);
372+ response->mutable_id()->set_value(surf_id.as_value());
373+ response->set_width(surface->size().width.as_uint32_t());
374+ response->set_height(surface->size().height.as_uint32_t());
375+ response->set_pixel_format((int)surface->pixel_format());
376+ response->set_buffer_usage(request->buffer_usage());
377+
378+ if (surface->supports_input())
379+ response->add_fd(surface->client_input_fd());
380+
381+ advance_buffer(surf_id, *surface,
382+ [lock, this, response, done, session](graphics::Buffer* client_buffer, bool need_full_ipc)
383+ {
384+ lock->unlock();
385+
386+ auto buffer = response->mutable_buffer();
387+ pack_protobuf_buffer(*buffer, client_buffer, need_full_ipc);
388+
389+ // TODO: NOTE: We use the ordering here to ensure the shell acts on the surface after the surface ID is sent over the wire.
390+ // This guarantees that notifications such as, gained focus, etc, can be correctly interpreted by the client.
391+ // To achieve this order we rely on done->Run() sending messages synchronously. As documented in mfd::SocketMessenger::send.
392+ // this will require additional synchronization if mfd::SocketMessenger::send changes.
393+ done->Run();
394+ shell->handle_surface_created(session);
395+ });
396 }
397
398 void mf::SessionMediator::next_buffer(
399@@ -192,34 +197,34 @@
400 ::mir::protobuf::Buffer* response,
401 ::google::protobuf::Closure* done)
402 {
403- bool need_full_ipc;
404 SurfaceId const surf_id{request->value()};
405- graphics::Buffer* client_buffer{nullptr};
406-
407- {
408- std::unique_lock<std::mutex> lock(session_mutex);
409-
410- auto session = weak_session.lock();
411-
412- if (session.get() == nullptr)
413- BOOST_THROW_EXCEPTION(std::logic_error("Invalid application session"));
414-
415- report->session_next_buffer_called(session->name());
416-
417- // We ensure the client has not powered down the outputs, so that
418- // swap_buffer will not block indefinitely, leaving the client
419- // in a position where it can not turn back on the
420- // outputs.
421- display_changer->ensure_display_powered(session);
422-
423- auto surface = session->get_surface(surf_id);
424-
425- std::tie(client_buffer, need_full_ipc) = advance_buffer(surf_id, *surface);
426- }
427-
428- pack_protobuf_buffer(*response, client_buffer, need_full_ipc);
429-
430- done->Run();
431+
432+ auto const lock = std::make_shared<std::unique_lock<std::mutex>>(session_mutex);
433+
434+ auto const session = weak_session.lock();
435+
436+ if (session.get() == nullptr)
437+ BOOST_THROW_EXCEPTION(std::logic_error("Invalid application session"));
438+
439+ report->session_next_buffer_called(session->name());
440+
441+ // We ensure the client has not powered down the outputs, so that
442+ // swap_buffer will not block indefinitely, leaving the client
443+ // in a position where it can not turn back on the
444+ // outputs.
445+ display_changer->ensure_display_powered(session);
446+
447+ auto surface = session->get_surface(surf_id);
448+
449+ advance_buffer(surf_id, *surface,
450+ [lock, this, response, done, session](graphics::Buffer* client_buffer, bool need_full_ipc)
451+ {
452+ lock->unlock();
453+
454+ pack_protobuf_buffer(*response, client_buffer, need_full_ipc);
455+
456+ done->Run();
457+ });
458 }
459
460 void mf::SessionMediator::release_surface(
461
462=== modified file 'src/server/frontend/session_mediator.h'
463--- src/server/frontend/session_mediator.h 2014-01-29 18:02:33 +0000
464+++ src/server/frontend/session_mediator.h 2014-02-06 09:52:02 +0000
465@@ -24,6 +24,7 @@
466 #include "mir/frontend/surface_id.h"
467 #include "mir_toolkit/common.h"
468
469+#include <functional>
470 #include <unordered_map>
471 #include <memory>
472 #include <mutex>
473@@ -133,7 +134,7 @@
474 graphics::Buffer* graphics_buffer,
475 bool need_full_ipc);
476
477- std::tuple<graphics::Buffer*, bool> advance_buffer(SurfaceId surf_id, Surface& surface);
478+ void advance_buffer(SurfaceId surf_id, Surface& surface, std::function<void(graphics::Buffer*, bool)> complete);
479 std::shared_ptr<Shell> const shell;
480 std::shared_ptr<graphics::Platform> const graphics_platform;
481
482
483=== modified file 'src/server/frontend/surface.cpp'
484--- src/server/frontend/surface.cpp 2014-01-13 06:12:33 +0000
485+++ src/server/frontend/surface.cpp 2014-02-06 09:52:02 +0000
486@@ -25,6 +25,10 @@
487
488 #include "client_buffer_tracker.h"
489
490+#include <condition_variable>
491+#include <mutex>
492+#include <thread>
493+
494 namespace mg = mir::graphics;
495 namespace mf = mir::frontend;
496
497@@ -39,7 +43,7 @@
498 private:
499 void swap_buffers(graphics::Buffer*& buffer)
500 {
501- surface->swap_buffers(buffer);
502+ surface->swap_buffers_blocking(buffer);
503 }
504 virtual mir::geometry::Size size() const { return surface->size(); }
505 virtual MirPixelFormat pixel_format() const { return surface->pixel_format(); }
506@@ -49,3 +53,23 @@
507
508 return std::make_shared<ForwardingInternalSurface>(surface);
509 }
510+
511+void mf::Surface::swap_buffers_blocking(graphics::Buffer*& buffer)
512+{
513+ std::mutex mutex;
514+ std::condition_variable cv;
515+ bool done = false;
516+
517+ swap_buffers(buffer,
518+ [&](mg::Buffer* new_buffer)
519+ {
520+ std::unique_lock<decltype(mutex)> lock(mutex);
521+ buffer = new_buffer;
522+ done = true;
523+ cv.notify_one();
524+ });
525+
526+ std::unique_lock<decltype(mutex)> lock(mutex);
527+
528+ cv.wait(lock, [&]{ return done; });
529+}
530
531=== modified file 'src/server/scene/basic_surface.cpp'
532--- src/server/scene/basic_surface.cpp 2014-01-24 08:29:36 +0000
533+++ src/server/scene/basic_surface.cpp 2014-02-06 09:52:02 +0000
534@@ -114,11 +114,11 @@
535 return surface_buffer_stream->get_stream_pixel_format();
536 }
537
538-void ms::BasicSurface::swap_buffers(graphics::Buffer*& buffer)
539+void ms::BasicSurface::swap_buffers(mg::Buffer* old_buffer, std::function<void(mg::Buffer* new_buffer)> complete)
540 {
541- bool const posting{!!buffer};
542+ bool const posting{!!old_buffer};
543
544- surface_buffer_stream->swap_client_buffers(buffer);
545+ surface_buffer_stream->swap_client_buffers(old_buffer, complete);
546
547 if (posting)
548 {
549
550=== modified file 'src/server/scene/basic_surface.h'
551--- src/server/scene/basic_surface.h 2014-01-24 08:29:36 +0000
552+++ src/server/scene/basic_surface.h 2014-02-06 09:52:02 +0000
553@@ -73,7 +73,7 @@
554 virtual MirPixelFormat pixel_format() const;
555
556 virtual std::shared_ptr<graphics::Buffer> snapshot_buffer() const;
557- virtual void swap_buffers(graphics::Buffer*&);
558+ virtual void swap_buffers(graphics::Buffer* old_buffer, std::function<void(graphics::Buffer* new_buffer)> complete);
559 virtual void force_requests_to_complete();
560
561 virtual bool supports_input() const;
562
563=== modified file 'src/server/scene/surface_impl.cpp'
564--- src/server/scene/surface_impl.cpp 2014-01-24 08:29:36 +0000
565+++ src/server/scene/surface_impl.cpp 2014-02-06 09:52:02 +0000
566@@ -103,9 +103,9 @@
567 return surface->pixel_format();
568 }
569
570-void ms::SurfaceImpl::swap_buffers(graphics::Buffer*& buffer)
571+void ms::SurfaceImpl::swap_buffers(mg::Buffer* old_buffer, std::function<void(mg::Buffer* new_buffer)> complete)
572 {
573- surface->swap_buffers(buffer);
574+ surface->swap_buffers(old_buffer, complete);
575 }
576
577 void ms::SurfaceImpl::allow_framedropping(bool allow)
578
579=== modified file 'src/server/scene/surface_impl.h'
580--- src/server/scene/surface_impl.h 2014-01-13 06:12:33 +0000
581+++ src/server/scene/surface_impl.h 2014-02-06 09:52:02 +0000
582@@ -70,7 +70,7 @@
583
584 virtual void with_most_recent_buffer_do(
585 std::function<void(graphics::Buffer&)> const& exec);
586- virtual void swap_buffers(graphics::Buffer*& buffer);
587+ virtual void swap_buffers(graphics::Buffer* old_buffer, std::function<void(graphics::Buffer* new_buffer)> complete) override;
588
589 virtual bool supports_input() const;
590 virtual int client_input_fd() const;
591
592=== modified file 'tests/integration-tests/compositor/test_buffer_stream.cpp'
593--- tests/integration-tests/compositor/test_buffer_stream.cpp 2014-01-13 06:12:33 +0000
594+++ tests/integration-tests/compositor/test_buffer_stream.cpp 2014-02-06 09:52:02 +0000
595@@ -26,6 +26,7 @@
596
597 #include <gmock/gmock.h>
598
599+#include <condition_variable>
600 #include <thread>
601 #include <chrono>
602 #include <atomic>
603@@ -38,6 +39,31 @@
604
605 namespace
606 {
607+struct BufferStreamSurfaces : mc::BufferStreamSurfaces
608+{
609+ using mc::BufferStreamSurfaces::BufferStreamSurfaces;
610+
611+ // Convenient function to allow tests to be written in linear style
612+ void swap_client_buffers_blocking(mg::Buffer*& buffer)
613+ {
614+ std::mutex mutex;
615+ std::condition_variable cv;
616+ bool done = false;
617+
618+ swap_client_buffers(buffer,
619+ [&](mg::Buffer* new_buffer)
620+ {
621+ std::unique_lock<decltype(mutex)> lock(mutex);
622+ buffer = new_buffer;
623+ done = true;
624+ cv.notify_one();
625+ });
626+
627+ std::unique_lock<decltype(mutex)> lock(mutex);
628+
629+ cv.wait(lock, [&]{ return done; });
630+ }
631+};
632
633 struct BufferStreamTest : public ::testing::Test
634 {
635@@ -60,7 +86,7 @@
636 }
637
638 const int nbuffers;
639- mc::BufferStreamSurfaces buffer_stream;
640+ BufferStreamSurfaces buffer_stream;
641 };
642
643 }
644@@ -68,9 +94,9 @@
645 TEST_F(BufferStreamTest, gives_same_back_buffer_until_more_available)
646 {
647 mg::Buffer* client1{nullptr};
648- buffer_stream.swap_client_buffers(client1);
649+ buffer_stream.swap_client_buffers_blocking(client1);
650 auto client1_id = client1->id();
651- buffer_stream.swap_client_buffers(client1);
652+ buffer_stream.swap_client_buffers_blocking(client1);
653
654 auto comp1 = buffer_stream.lock_compositor_buffer(1);
655 auto comp2 = buffer_stream.lock_compositor_buffer(1);
656@@ -80,7 +106,7 @@
657
658 comp1.reset();
659
660- buffer_stream.swap_client_buffers(client1);
661+ buffer_stream.swap_client_buffers_blocking(client1);
662 auto comp3 = buffer_stream.lock_compositor_buffer(2);
663
664 EXPECT_NE(client1_id, comp3->id());
665@@ -97,7 +123,7 @@
666 {
667 mg::Buffer* client_buffer{nullptr};
668 for (int i = 0; i != nbuffers; i++)
669- buffer_stream.swap_client_buffers(client_buffer);
670+ buffer_stream.swap_client_buffers_blocking(client_buffer);
671
672 auto first_monitor = buffer_stream.lock_compositor_buffer(1);
673 auto first_compositor_id = first_monitor->id();
674@@ -113,14 +139,14 @@
675 TEST_F(BufferStreamTest, gives_different_back_buffer_asap)
676 {
677 mg::Buffer* client_buffer{nullptr};
678- buffer_stream.swap_client_buffers(client_buffer);
679+ buffer_stream.swap_client_buffers_blocking(client_buffer);
680
681 if (nbuffers > 1)
682 {
683- buffer_stream.swap_client_buffers(client_buffer);
684+ buffer_stream.swap_client_buffers_blocking(client_buffer);
685 auto comp1 = buffer_stream.lock_compositor_buffer(1);
686
687- buffer_stream.swap_client_buffers(client_buffer);
688+ buffer_stream.swap_client_buffers_blocking(client_buffer);
689 auto comp2 = buffer_stream.lock_compositor_buffer(2);
690
691 EXPECT_NE(comp1->id(), comp2->id());
692@@ -135,7 +161,7 @@
693 auto old_size = buffer_stream.stream_size();
694
695 mg::Buffer* client{nullptr};
696- buffer_stream.swap_client_buffers(client);
697+ buffer_stream.swap_client_buffers_blocking(client);
698 EXPECT_EQ(old_size, client->size());
699
700 geom::Size const new_size
701@@ -146,13 +172,13 @@
702 buffer_stream.resize(new_size);
703 EXPECT_EQ(new_size, buffer_stream.stream_size());
704
705- buffer_stream.swap_client_buffers(client);
706+ buffer_stream.swap_client_buffers_blocking(client);
707 EXPECT_EQ(new_size, client->size());
708
709 buffer_stream.resize(old_size);
710 EXPECT_EQ(old_size, buffer_stream.stream_size());
711
712- buffer_stream.swap_client_buffers(client);
713+ buffer_stream.swap_client_buffers_blocking(client);
714 EXPECT_EQ(old_size, client->size());
715 }
716
717@@ -161,7 +187,7 @@
718 auto old_size = buffer_stream.stream_size();
719
720 mg::Buffer* client{nullptr};
721- buffer_stream.swap_client_buffers(client);
722+ buffer_stream.swap_client_buffers_blocking(client);
723
724 geom::Size const new_size
725 {
726@@ -171,8 +197,8 @@
727 buffer_stream.resize(new_size);
728 EXPECT_EQ(new_size, buffer_stream.stream_size());
729
730- buffer_stream.swap_client_buffers(client);
731- buffer_stream.swap_client_buffers(client);
732+ buffer_stream.swap_client_buffers_blocking(client);
733+ buffer_stream.swap_client_buffers_blocking(client);
734
735 auto comp1 = buffer_stream.lock_compositor_buffer(1);
736 EXPECT_EQ(old_size, comp1->size());
737@@ -182,7 +208,7 @@
738 EXPECT_EQ(new_size, comp2->size());
739 comp2.reset();
740
741- buffer_stream.swap_client_buffers(client);
742+ buffer_stream.swap_client_buffers_blocking(client);
743
744 auto comp3 = buffer_stream.lock_compositor_buffer(3);
745 EXPECT_EQ(new_size, comp3->size());
746@@ -192,13 +218,13 @@
747 EXPECT_EQ(old_size, buffer_stream.stream_size());
748
749 // No client frames "drawn" since resize(old_size), so compositor gets new_size
750- buffer_stream.swap_client_buffers(client);
751+ buffer_stream.swap_client_buffers_blocking(client);
752 auto comp4 = buffer_stream.lock_compositor_buffer(4);
753 EXPECT_EQ(new_size, comp4->size());
754 comp4.reset();
755
756 // Generate a new frame, which should be back to old_size now
757- buffer_stream.swap_client_buffers(client);
758+ buffer_stream.swap_client_buffers_blocking(client);
759 auto comp5 = buffer_stream.lock_compositor_buffer(5);
760 EXPECT_EQ(old_size, comp5->size());
761 comp5.reset();
762@@ -207,8 +233,8 @@
763 TEST_F(BufferStreamTest, can_get_partly_released_back_buffer)
764 {
765 mg::Buffer* client{nullptr};
766- buffer_stream.swap_client_buffers(client);
767- buffer_stream.swap_client_buffers(client);
768+ buffer_stream.swap_client_buffers_blocking(client);
769+ buffer_stream.swap_client_buffers_blocking(client);
770
771 auto comp1 = buffer_stream.lock_compositor_buffer(123);
772 auto comp2 = buffer_stream.lock_compositor_buffer(123);
773@@ -225,12 +251,12 @@
774 namespace
775 {
776
777-void client_loop(int nframes, mc::BufferStream& stream)
778+void client_loop(int nframes, BufferStreamSurfaces& stream)
779 {
780 mg::Buffer* out_buffer{nullptr};
781 for (int f = 0; f < nframes; f++)
782 {
783- stream.swap_client_buffers(out_buffer);
784+ stream.swap_client_buffers_blocking(out_buffer);
785 ASSERT_NE(nullptr, out_buffer);
786 std::this_thread::yield();
787 }
788
789=== modified file 'tests/integration-tests/compositor/test_swapping_swappers.cpp'
790--- tests/integration-tests/compositor/test_swapping_swappers.cpp 2014-01-13 06:12:33 +0000
791+++ tests/integration-tests/compositor/test_swapping_swappers.cpp 2014-02-06 09:52:02 +0000
792@@ -51,6 +51,31 @@
793 std::shared_ptr<mc::SwitchingBundle> switching_bundle;
794 std::atomic<bool> client_thread_done;
795 };
796+
797+auto client_acquire_blocking(std::shared_ptr<mc::SwitchingBundle> const& switching_bundle)
798+-> mg::Buffer*
799+{
800+ std::mutex mutex;
801+ std::condition_variable cv;
802+ bool done = false;
803+
804+ mg::Buffer* result;
805+ switching_bundle->client_acquire(
806+ [&](mg::Buffer* new_buffer)
807+ {
808+ std::unique_lock<decltype(mutex)> lock(mutex);
809+
810+ result = new_buffer;
811+ done = true;
812+ cv.notify_one();
813+ });
814+
815+ std::unique_lock<decltype(mutex)> lock(mutex);
816+
817+ cv.wait(lock, [&]{ return done; });
818+
819+ return result;
820+}
821 }
822
823 TEST_F(SwapperSwappingStress, swapper)
824@@ -62,7 +87,7 @@
825 {
826 for(auto i=0u; i < 400; i++)
827 {
828- auto b = switching_bundle->client_acquire();
829+ auto b = client_acquire_blocking(switching_bundle);
830 std::this_thread::yield();
831 switching_bundle->client_release(b);
832 }
833@@ -107,7 +132,7 @@
834 {
835 for(auto i=0u; i < 400; i++)
836 {
837- auto b = switching_bundle->client_acquire();
838+ auto b = client_acquire_blocking(switching_bundle);
839 std::this_thread::yield();
840 switching_bundle->client_release(b);
841 }
842
843=== modified file 'tests/integration-tests/graphics/android/test_buffer_integration.cpp'
844--- tests/integration-tests/graphics/android/test_buffer_integration.cpp 2014-01-13 06:12:33 +0000
845+++ tests/integration-tests/graphics/android/test_buffer_integration.cpp 2014-02-06 09:52:02 +0000
846@@ -55,6 +55,30 @@
847 mtd::TestGrallocMapper sw_renderer;
848 };
849
850+auto client_acquire_blocking(mc::SwitchingBundle& switching_bundle)
851+-> mg::Buffer*
852+{
853+ std::mutex mutex;
854+ std::condition_variable cv;
855+ bool done = false;
856+
857+ mg::Buffer* result;
858+ switching_bundle.client_acquire(
859+ [&](mg::Buffer* new_buffer)
860+ {
861+ std::unique_lock<decltype(mutex)> lock(mutex);
862+
863+ result = new_buffer;
864+ done = true;
865+ cv.notify_one();
866+ });
867+
868+ std::unique_lock<decltype(mutex)> lock(mutex);
869+
870+ cv.wait(lock, [&]{ return done; });
871+
872+ return result;
873+}
874 }
875
876 TEST_F(AndroidBufferIntegration, allocator_can_create_sw_buffer)
877@@ -92,7 +116,7 @@
878
879 mc::SwitchingBundle swapper(2, allocator, buffer_properties);
880
881- auto returned_buffer = swapper.client_acquire();
882+ auto returned_buffer = client_acquire_blocking(swapper);
883
884 EXPECT_NE(nullptr, returned_buffer);
885 }
886
887=== modified file 'tests/integration-tests/test_session.cpp'
888--- tests/integration-tests/test_session.cpp 2014-01-14 07:02:27 +0000
889+++ tests/integration-tests/test_session.cpp 2014-02-06 09:52:02 +0000
890@@ -189,7 +189,7 @@
891 for (int i = 0; i < 500; ++i)
892 {
893 auto surface = session.default_surface();
894- surface->swap_buffers(buffer);
895+ surface->swap_buffers_blocking(buffer);
896 std::this_thread::sleep_for(std::chrono::microseconds{50});
897 }
898 }};
899
900=== modified file 'tests/integration-tests/test_swapinterval.cpp'
901--- tests/integration-tests/test_swapinterval.cpp 2014-01-13 06:12:33 +0000
902+++ tests/integration-tests/test_swapinterval.cpp 2014-02-06 09:52:02 +0000
903@@ -56,14 +56,22 @@
904 {
905 }
906
907- void swap_client_buffers(mg::Buffer*& buffer) { buffer = &stub_buffer; }
908- std::shared_ptr<mg::Buffer> lock_compositor_buffer(unsigned long) { return std::make_shared<mtd::StubBuffer>(); }
909- std::shared_ptr<mg::Buffer> lock_snapshot_buffer() { return std::make_shared<mtd::StubBuffer>(); }
910- MirPixelFormat get_stream_pixel_format() { return mir_pixel_format_abgr_8888; }
911- geom::Size stream_size() { return geom::Size{}; }
912+ void swap_client_buffers(mg::Buffer*, std::function<void(mg::Buffer* new_buffer)> complete) override
913+ { complete(&stub_buffer); }
914+
915+ std::shared_ptr<mg::Buffer> lock_compositor_buffer(unsigned long) override
916+ { return std::make_shared<mtd::StubBuffer>(); }
917+
918+ std::shared_ptr<mg::Buffer> lock_snapshot_buffer() override
919+ { return std::make_shared<mtd::StubBuffer>(); }
920+
921+ MirPixelFormat get_stream_pixel_format() override
922+ { return mir_pixel_format_abgr_8888; }
923+
924+ geom::Size stream_size() override { return geom::Size{}; }
925 void resize(geom::Size const&) override {}
926- void force_requests_to_complete() {}
927- void allow_framedropping(bool)
928+ void force_requests_to_complete() override {}
929+ void allow_framedropping(bool) override
930 {
931 while (write(render_operations_fd, "a", 1) != 1) continue;
932 }
933
934=== modified file 'tests/unit-tests/compositor/test_buffer_stream.cpp'
935--- tests/unit-tests/compositor/test_buffer_stream.cpp 2014-01-13 06:12:33 +0000
936+++ tests/unit-tests/compositor/test_buffer_stream.cpp 2014-02-06 09:52:02 +0000
937@@ -25,6 +25,8 @@
938 #include <gmock/gmock.h>
939 #include <gtest/gtest.h>
940
941+#include <condition_variable>
942+
943 namespace mc = mir::compositor;
944 namespace mg = mir::graphics;
945 namespace geom = mir::geometry;
946@@ -122,22 +124,50 @@
947
948 TEST_F(BufferStreamTest, get_buffer_for_client_releases_resources)
949 {
950- using namespace testing;
951- mc::BufferStreamSurfaces buffer_stream(mock_bundle);
952+ std::mutex mutex;
953+ std::condition_variable cv;
954 mg::Buffer* buffer{nullptr};
955+ bool done = false;
956+
957+ auto const callback =
958+ [&](mg::Buffer* new_buffer)
959+ {
960+ std::unique_lock<decltype(mutex)> lock(mutex);
961+ buffer = new_buffer;
962+ done = true;
963+ cv.notify_one();
964+ };
965+
966+ using namespace testing;
967+ mc::BufferStreamSurfaces buffer_stream(mock_bundle);
968
969 InSequence seq;
970- EXPECT_CALL(*mock_bundle, client_acquire())
971+ EXPECT_CALL(*mock_bundle, client_acquire(_))
972 .Times(1)
973- .WillOnce(Return(mock_buffer.get()));
974+ .WillOnce(InvokeArgument<0>(mock_buffer.get()));
975 EXPECT_CALL(*mock_bundle, client_release(_))
976 .Times(1);
977- EXPECT_CALL(*mock_bundle, client_acquire())
978+ EXPECT_CALL(*mock_bundle, client_acquire(_))
979 .Times(1)
980- .WillOnce(Return(mock_buffer.get()));
981-
982- buffer_stream.swap_client_buffers(buffer);
983- buffer_stream.swap_client_buffers(buffer);
984+ .WillOnce(InvokeArgument<0>(mock_buffer.get()));
985+
986+ buffer_stream.swap_client_buffers(buffer, callback);
987+
988+ {
989+ std::unique_lock<decltype(mutex)> lock(mutex);
990+
991+ cv.wait(lock, [&]{ return done; });
992+ }
993+
994+ done = false;
995+
996+ buffer_stream.swap_client_buffers(buffer, callback);
997+
998+ {
999+ std::unique_lock<decltype(mutex)> lock(mutex);
1000+
1001+ cv.wait(lock, [&]{ return done; });
1002+ }
1003 }
1004
1005 TEST_F(BufferStreamTest, allow_framedropping_device)
1006
1007=== modified file 'tests/unit-tests/compositor/test_switching_bundle.cpp'
1008--- tests/unit-tests/compositor/test_switching_bundle.cpp 2014-01-22 14:15:29 +0000
1009+++ tests/unit-tests/compositor/test_switching_bundle.cpp 2014-02-06 09:52:02 +0000
1010@@ -34,6 +34,8 @@
1011
1012 using namespace testing;
1013
1014+namespace
1015+{
1016 struct SwitchingBundleTest : public ::testing::Test
1017 {
1018 void SetUp()
1019@@ -52,6 +54,32 @@
1020 mg::BufferProperties basic_properties;
1021 };
1022
1023+auto client_acquire_blocking(mc::SwitchingBundle& switching_bundle)
1024+-> mg::Buffer*
1025+{
1026+ std::mutex mutex;
1027+ std::condition_variable cv;
1028+ bool done = false;
1029+
1030+ mg::Buffer* result;
1031+ switching_bundle.client_acquire(
1032+ [&](mg::Buffer* new_buffer)
1033+ {
1034+ std::unique_lock<decltype(mutex)> lock(mutex);
1035+
1036+ result = new_buffer;
1037+ done = true;
1038+ cv.notify_one();
1039+ });
1040+
1041+ std::unique_lock<decltype(mutex)> lock(mutex);
1042+
1043+ cv.wait(lock, [&]{ return done; });
1044+
1045+ return result;
1046+}
1047+}
1048+
1049 TEST_F(SwitchingBundleTest, sync_swapper_by_default)
1050 {
1051 mg::BufferProperties properties{geom::Size{7, 8},
1052@@ -100,7 +128,7 @@
1053 {
1054 mc::SwitchingBundle bundle(nbuffers, allocator, basic_properties);
1055
1056- auto buffer = bundle.client_acquire();
1057+ auto buffer = client_acquire_blocking(bundle);
1058 bundle.client_release(buffer);
1059 }
1060 }
1061@@ -132,7 +160,7 @@
1062
1063 for (int i = 0; i < 20; i++)
1064 {
1065- auto client1 = bundle.client_acquire();
1066+ auto client1 = client_acquire_blocking(bundle);
1067 mg::BufferID expect_id = client1->id(), composited_id;
1068 bundle.client_release(client1);
1069
1070@@ -173,7 +201,7 @@
1071 {
1072 for (int j = 0; j < 100; j++)
1073 {
1074- auto client = bundle.client_acquire();
1075+ auto client = client_acquire_blocking(bundle);
1076 last_client_id = client->id();
1077 bundle.client_release(client);
1078 }
1079@@ -197,11 +225,11 @@
1080 { // Regression test for LP: #1210042
1081 mc::SwitchingBundle bundle(3, allocator, basic_properties);
1082
1083- auto client1 = bundle.client_acquire();
1084+ auto client1 = client_acquire_blocking(bundle);
1085 auto client1_id = client1->id();
1086 bundle.client_release(client1);
1087
1088- auto client2 = bundle.client_acquire();
1089+ auto client2 = client_acquire_blocking(bundle);
1090 bundle.client_release(client2);
1091
1092 auto compositor = bundle.compositor_acquire(1);
1093@@ -217,8 +245,8 @@
1094 {
1095 mc::SwitchingBundle bundle(nbuffers, allocator, basic_properties);
1096
1097- auto client1 = bundle.client_acquire();
1098- auto client2 = bundle.client_acquire();
1099+ auto client1 = client_acquire_blocking(bundle);
1100+ auto client2 = client_acquire_blocking(bundle);
1101 EXPECT_THROW(
1102 bundle.client_release(client2),
1103 std::logic_error
1104@@ -241,7 +269,7 @@
1105 mc::SwitchingBundle bundle(nbuffers, allocator, basic_properties);
1106 unsigned long frameno = 0;
1107
1108- auto client = bundle.client_acquire();
1109+ auto client = client_acquire_blocking(bundle);
1110 auto client_id = client->id();
1111 bundle.client_release(client);
1112
1113@@ -291,7 +319,7 @@
1114 {
1115 if (i % 10 == 0)
1116 {
1117- auto client = bundle.client_acquire();
1118+ auto client = client_acquire_blocking(bundle);
1119 client_id = client->id();
1120 bundle.client_release(client);
1121 }
1122@@ -317,7 +345,7 @@
1123 mc::SwitchingBundle bundle(nbuffers, allocator, basic_properties);
1124 unsigned long frameno = 0;
1125
1126- auto client = bundle.client_acquire();
1127+ auto client = client_acquire_blocking(bundle);
1128 bundle.client_release(client);
1129
1130 auto compositor1 = bundle.compositor_acquire(++frameno);
1131@@ -342,11 +370,11 @@
1132 mg::Buffer* client_buffer = nullptr;
1133 std::shared_ptr<mg::Buffer> compositor_buffer = nullptr;
1134
1135- client_buffer = bundle.client_acquire();
1136+ client_buffer = client_acquire_blocking(bundle);
1137 mg::BufferID first_ready_buffer_id = client_buffer->id();
1138 bundle.client_release(client_buffer);
1139
1140- client_buffer = bundle.client_acquire();
1141+ client_buffer = client_acquire_blocking(bundle);
1142
1143 // in the original bug, compositor would be given the wrong buffer here
1144 compositor_buffer = bundle.compositor_acquire(0 /*frameno*/);
1145@@ -371,11 +399,11 @@
1146
1147 std::shared_ptr<mg::Buffer> compositor[2];
1148
1149- bundle.client_release(bundle.client_acquire());
1150+ bundle.client_release(client_acquire_blocking(bundle));
1151 compositor[0] = bundle.compositor_acquire(frameno);
1152
1153 frameno++;
1154- bundle.client_release(bundle.client_acquire());
1155+ bundle.client_release(client_acquire_blocking(bundle));
1156 compositor[1] = bundle.compositor_acquire(frameno);
1157
1158 for (int i = 0; i < 20; i++)
1159@@ -386,7 +414,7 @@
1160 // One of the compositors (the oldest one) gets a new buffer...
1161 int oldest = i & 1;
1162 bundle.compositor_release(compositor[oldest]);
1163- bundle.client_release(bundle.client_acquire());
1164+ bundle.client_release(client_acquire_blocking(bundle));
1165 frameno++;
1166 compositor[oldest] = bundle.compositor_acquire(frameno);
1167 }
1168@@ -445,7 +473,7 @@
1169 std::logic_error
1170 );
1171
1172- auto client = bundle.client_acquire();
1173+ auto client = client_acquire_blocking(bundle);
1174 auto snapshot = bundle.snapshot_acquire();
1175
1176 EXPECT_EQ(compositor->id(), snapshot->id());
1177@@ -488,7 +516,7 @@
1178 {
1179 for (int i = 0; i < nframes; i++)
1180 {
1181- bundle.client_release(bundle.client_acquire());
1182+ bundle.client_release(client_acquire_blocking(bundle));
1183 std::this_thread::yield();
1184 }
1185 }
1186@@ -499,12 +527,12 @@
1187 {
1188 bundle.allow_framedropping(false);
1189 for (int j = 0; j < 5; j++)
1190- bundle.client_release(bundle.client_acquire());
1191+ bundle.client_release(client_acquire_blocking(bundle));
1192 std::this_thread::yield();
1193
1194 bundle.allow_framedropping(true);
1195 for (int j = 0; j < 5; j++)
1196- bundle.client_release(bundle.client_acquire());
1197+ bundle.client_release(client_acquire_blocking(bundle));
1198 std::this_thread::yield();
1199 }
1200 }
1201@@ -602,11 +630,11 @@
1202
1203 std::shared_ptr<mg::Buffer> compositor[2];
1204
1205- bundle.client_release(bundle.client_acquire());
1206+ bundle.client_release(client_acquire_blocking(bundle));
1207 compositor[0] = bundle.compositor_acquire(frameno);
1208
1209 frameno++;
1210- bundle.client_release(bundle.client_acquire());
1211+ bundle.client_release(client_acquire_blocking(bundle));
1212 compositor[1] = bundle.compositor_acquire(frameno);
1213
1214 for (int i = 0; i < 20; i++)
1215@@ -614,7 +642,7 @@
1216 // Two compositors acquired, and they're always different...
1217 ASSERT_NE(compositor[0]->id(), compositor[1]->id());
1218
1219- auto client = bundle.client_acquire();
1220+ auto client = client_acquire_blocking(bundle);
1221 ASSERT_NE(compositor[0]->id(), client->id());
1222 ASSERT_NE(compositor[1]->id(), client->id());
1223 bundle.client_release(client);
1224@@ -648,7 +676,7 @@
1225
1226 for (int b = 0; b < nbuffers; b++)
1227 {
1228- buf[b] = bundle.client_acquire();
1229+ buf[b] = client_acquire_blocking(bundle);
1230 expect[b] = buf[b]->id();
1231
1232 for (int p = 0; p < b; p++)
1233@@ -660,7 +688,7 @@
1234
1235 for (int frame = 0; frame < nframes; frame++)
1236 {
1237- auto client = bundle.client_acquire();
1238+ auto client = client_acquire_blocking(bundle);
1239 ASSERT_EQ(expect[frame % nbuffers], client->id());
1240 bundle.client_release(client);
1241 }
1242@@ -739,11 +767,11 @@
1243 }
1244 });
1245
1246- bundle.client_release(bundle.client_acquire());
1247+ bundle.client_release(client_acquire_blocking(bundle));
1248
1249 while (!done.load())
1250 {
1251- bundle.client_release(bundle.client_acquire());
1252+ bundle.client_release(client_acquire_blocking(bundle));
1253 client_frames++;
1254 }
1255
1256@@ -789,13 +817,13 @@
1257 }
1258 });
1259
1260- bundle.client_release(bundle.client_acquire());
1261+ bundle.client_release(client_acquire_blocking(bundle));
1262
1263 while (!done.load())
1264 {
1265 sync.lock();
1266 sync.unlock();
1267- auto buf = bundle.client_acquire();
1268+ auto buf = client_acquire_blocking(bundle);
1269 std::this_thread::sleep_for(frame_time);
1270 bundle.client_release(buf);
1271 client_frames++;
1272@@ -825,7 +853,7 @@
1273 for (int subframe = 0; subframe < 3; ++subframe)
1274 {
1275 bundle.resize(expect_size);
1276- auto client = bundle.client_acquire();
1277+ auto client = client_acquire_blocking(bundle);
1278 ASSERT_EQ(expect_size, client->size());
1279 bundle.client_release(client);
1280
1281@@ -861,7 +889,7 @@
1282 height += dy;
1283
1284 bundle.resize(new_size);
1285- auto client = bundle.client_acquire();
1286+ auto client = client_acquire_blocking(bundle);
1287 history[produce] = client->id();
1288 ASSERT_EQ(new_size, client->size());
1289 bundle.client_release(client);
1290
1291=== modified file 'tests/unit-tests/compositor/test_temporary_buffers.cpp'
1292--- tests/unit-tests/compositor/test_temporary_buffers.cpp 2014-01-13 06:12:33 +0000
1293+++ tests/unit-tests/compositor/test_temporary_buffers.cpp 2014-02-06 09:52:02 +0000
1294@@ -52,8 +52,8 @@
1295 {
1296 using namespace testing;
1297
1298- ON_CALL(*mock_bundle, client_acquire())
1299- .WillByDefault(Return(mock_buffer.get()));
1300+ ON_CALL(*mock_bundle, client_acquire(_))
1301+ .WillByDefault(InvokeArgument<0>(mock_buffer.get()));
1302 ON_CALL(*mock_bundle, compositor_acquire(_))
1303 .WillByDefault(Return(mock_buffer));
1304 }
1305
1306=== modified file 'tests/unit-tests/frontend/test_session_mediator.cpp'
1307--- tests/unit-tests/frontend/test_session_mediator.cpp 2014-01-29 18:02:33 +0000
1308+++ tests/unit-tests/frontend/test_session_mediator.cpp 2014-02-06 09:52:02 +0000
1309@@ -104,7 +104,8 @@
1310
1311 EXPECT_CALL(*mock_surface, size()).Times(AnyNumber()).WillRepeatedly(Return(geom::Size()));
1312 EXPECT_CALL(*mock_surface, pixel_format()).Times(AnyNumber()).WillRepeatedly(Return(MirPixelFormat()));
1313- EXPECT_CALL(*mock_surface, swap_buffers(_)).Times(AnyNumber()).WillRepeatedly(SetArg<0>(mock_buffer.get()));
1314+ EXPECT_CALL(*mock_surface, swap_buffers(_, _)).Times(AnyNumber())
1315+ .WillRepeatedly(InvokeArgument<1>(mock_buffer.get()));
1316
1317 EXPECT_CALL(*mock_surface, supports_input()).Times(AnyNumber()).WillRepeatedly(Return(true));
1318 EXPECT_CALL(*mock_surface, client_input_fd()).Times(AnyNumber()).WillRepeatedly(Return(testing_client_input_fd));
1319@@ -124,7 +125,8 @@
1320
1321 EXPECT_CALL(*mock_surfaces[id], size()).Times(AnyNumber()).WillRepeatedly(Return(geom::Size()));
1322 EXPECT_CALL(*mock_surfaces[id], pixel_format()).Times(AnyNumber()).WillRepeatedly(Return(MirPixelFormat()));
1323- EXPECT_CALL(*mock_surfaces[id], swap_buffers(_)).Times(AnyNumber()).WillRepeatedly(SetArg<0>(mock_buffer.get()));
1324+ EXPECT_CALL(*mock_surfaces[id], swap_buffers(_, _)).Times(AnyNumber())
1325+ .WillRepeatedly(InvokeArgument<1>(mock_buffer.get()));
1326
1327 EXPECT_CALL(*mock_surfaces[id], supports_input()).Times(AnyNumber()).WillRepeatedly(Return(true));
1328 EXPECT_CALL(*mock_surfaces[id], client_input_fd()).Times(AnyNumber()).WillRepeatedly(Return(testing_client_input_fd));
1329@@ -474,6 +476,8 @@
1330 mediator.connect(nullptr, &connect_parameters, &connection, null_callback.get());
1331
1332 {
1333+ // AFAICS these values are stubs to set up the test condition,
1334+ // the exact calls here are not a *requirement* on SessionMediator
1335 EXPECT_CALL(*stubbed_session->mock_buffer, id())
1336 .WillOnce(Return(mg::BufferID{4}))
1337 .WillOnce(Return(mg::BufferID{4}))
1338@@ -532,8 +536,8 @@
1339 * the pre-created stubbed_session->mock_surface. Further create_surface()
1340 * invocations create new surfaces in stubbed_session->mock_surfaces[].
1341 */
1342- EXPECT_CALL(*stubbed_session->mock_surface, swap_buffers(_))
1343- .WillOnce(SetArg<0>(&buffer));
1344+ EXPECT_CALL(*stubbed_session->mock_surface, swap_buffers(_, _))
1345+ .WillOnce(InvokeArgument<1>(&buffer));
1346
1347 mediator.create_surface(nullptr, &surface_request, &surface_response, null_callback.get());
1348 mp::SurfaceId our_surface{surface_response.id()};
1349@@ -541,7 +545,7 @@
1350 Mock::VerifyAndClearExpectations(stubbed_session->mock_surface.get());
1351
1352 /* Creating a new surface should not affect our surfaces' buffers */
1353- EXPECT_CALL(*stubbed_session->mock_surface, swap_buffers(_)).Times(0);
1354+ EXPECT_CALL(*stubbed_session->mock_surface, swap_buffers(_, _)).Times(0);
1355 mediator.create_surface(nullptr, &surface_request, &surface_response, null_callback.get());
1356
1357 mp::SurfaceId new_surface{surface_response.id()};
1358@@ -553,7 +557,7 @@
1359 Mock::VerifyAndClearExpectations(stubbed_session->mock_surface.get());
1360
1361 /* Getting the next buffer of our surface should post the original */
1362- EXPECT_CALL(*stubbed_session->mock_surface, swap_buffers(Eq(&buffer))).Times(1);
1363+ EXPECT_CALL(*stubbed_session->mock_surface, swap_buffers(Eq(&buffer), _)).Times(1);
1364
1365 mediator.next_buffer(nullptr, &our_surface, &buffer_response, null_callback.get());
1366 mediator.disconnect(nullptr, nullptr, nullptr, null_callback.get());
1367
1368=== modified file 'tests/unit-tests/scene/test_surface.cpp'
1369--- tests/unit-tests/scene/test_surface.cpp 2014-01-24 08:29:36 +0000
1370+++ tests/unit-tests/scene/test_surface.cpp 2014-02-06 09:52:02 +0000
1371@@ -175,8 +175,8 @@
1372 stub_data = std::make_shared<ms::SurfaceData>(
1373 surface_name, rect, change_notification, false);
1374
1375- ON_CALL(*mock_buffer_stream, swap_client_buffers(_))
1376- .WillByDefault(SetArg<0>(&stub_buffer));
1377+ ON_CALL(*mock_buffer_stream, swap_client_buffers(_, _))
1378+ .WillByDefault(InvokeArgument<1>(&stub_buffer));
1379 }
1380
1381 std::shared_ptr<ms::SurfaceData> stub_data;
1382@@ -233,14 +233,13 @@
1383 ms::BasicSurface surf(stub_data, mock_buffer_stream, std::shared_ptr<mi::InputChannel>(), report);
1384 mtd::StubBuffer graphics_resource;
1385
1386- EXPECT_CALL(*mock_buffer_stream, swap_client_buffers(_))
1387+ EXPECT_CALL(*mock_buffer_stream, swap_client_buffers(_, _))
1388 .Times(1)
1389- .WillOnce(SetArg<0>(&graphics_resource));
1390-
1391- mg::Buffer* result{nullptr};
1392- surf.swap_buffers(result);
1393-
1394- EXPECT_EQ(&graphics_resource, result);
1395+ .WillOnce(InvokeArgument<1>(&graphics_resource));
1396+
1397+ surf.swap_buffers(
1398+ nullptr,
1399+ [&graphics_resource](mg::Buffer* result){ EXPECT_THAT(result, Eq(&graphics_resource)); });
1400 }
1401
1402 TEST_F(SurfaceCreation, test_surface_gets_ipc_from_stream)
1403@@ -250,14 +249,13 @@
1404 mtd::StubBuffer stub_buffer;
1405
1406 ms::BasicSurface surf(stub_data, mock_buffer_stream, std::shared_ptr<mi::InputChannel>(), report);
1407- EXPECT_CALL(*mock_buffer_stream, swap_client_buffers(_))
1408+ EXPECT_CALL(*mock_buffer_stream, swap_client_buffers(_, _))
1409 .Times(1)
1410- .WillOnce(SetArg<0>(&stub_buffer));
1411-
1412- mg::Buffer* result{nullptr};
1413- surf.swap_buffers(result);
1414-
1415- EXPECT_EQ(&stub_buffer, result);
1416+ .WillOnce(InvokeArgument<1>(&stub_buffer));
1417+
1418+ surf.swap_buffers(
1419+ nullptr,
1420+ [&stub_buffer](mg::Buffer* result){ EXPECT_THAT(result, Eq(&stub_buffer)); });
1421 }
1422
1423 TEST_F(SurfaceCreation, test_surface_gets_top_left)
1424@@ -405,10 +403,11 @@
1425 ms::BasicSurface surf(stub_data, mock_buffer_stream, std::shared_ptr<mi::InputChannel>(), report);
1426 mg::Buffer* buffer{nullptr};
1427
1428- surf.swap_buffers(buffer);
1429- surf.swap_buffers(buffer);
1430- surf.swap_buffers(buffer);
1431- surf.swap_buffers(buffer);
1432+ auto const complete = [&buffer](mg::Buffer* new_buffer){ buffer = new_buffer; };
1433+ surf.swap_buffers(buffer, complete);
1434+ surf.swap_buffers(buffer, complete);
1435+ surf.swap_buffers(buffer, complete);
1436+ surf.swap_buffers(buffer, complete);
1437
1438 EXPECT_EQ(3, notification_count);
1439 }
1440
1441=== modified file 'tests/unit-tests/scene/test_surface_impl.cpp'
1442--- tests/unit-tests/scene/test_surface_impl.cpp 2014-01-24 08:29:36 +0000
1443+++ tests/unit-tests/scene/test_surface_impl.cpp 2014-02-06 09:52:02 +0000
1444@@ -149,7 +149,8 @@
1445
1446 ON_CALL(*buffer_stream, stream_size()).WillByDefault(Return(geom::Size()));
1447 ON_CALL(*buffer_stream, get_stream_pixel_format()).WillByDefault(Return(mir_pixel_format_abgr_8888));
1448- ON_CALL(*buffer_stream, swap_client_buffers(_)).WillByDefault(SetArg<0>(nullptr));
1449+ ON_CALL(*buffer_stream, swap_client_buffers(_, _))
1450+ .WillByDefault(InvokeArgument<1>(nullptr));
1451 }
1452 mf::SurfaceId stub_id;
1453 std::shared_ptr<mf::EventSink> stub_sender;
1454
1455=== modified file 'tests/unit-tests/scene/test_surface_stack.cpp'
1456--- tests/unit-tests/scene/test_surface_stack.cpp 2014-01-13 06:12:33 +0000
1457+++ tests/unit-tests/scene/test_surface_stack.cpp 2014-02-06 09:52:02 +0000
1458@@ -67,7 +67,7 @@
1459 class NullBufferBundle : public mc::BufferBundle
1460 {
1461 public:
1462- virtual mg::Buffer* client_acquire() { return nullptr; }
1463+ virtual void client_acquire(std::function<void(mg::Buffer* buffer)> complete) { complete(nullptr); }
1464 virtual void client_release(mg::Buffer*) {}
1465 virtual std::shared_ptr<mg::Buffer> compositor_acquire(unsigned long)
1466 { return std::shared_ptr<mg::Buffer>(); };

Subscribers

People subscribed via source and target branches