Merge lp:~kdub/mir/server-sent-buffer into lp:mir
- server-sent-buffer
- Merge into development-branch
Status: | Merged |
---|---|
Approved by: | Chris Halse Rogers |
Approved revision: | no longer in the source branch. |
Merged at revision: | 2693 |
Proposed branch: | lp:~kdub/mir/server-sent-buffer |
Merge into: | lp:mir |
Prerequisite: | lp:~kdub/mir/client-track-bufferstream-ids |
Diff against target: |
559 lines (+253/-10) 15 files modified
src/client/buffer_stream.cpp (+54/-1) src/client/buffer_stream.h (+11/-0) src/client/client_buffer_stream.h (+2/-1) src/client/rpc/mir_protobuf_rpc_channel.cpp (+8/-0) src/include/server/mir/frontend/event_sink.h (+3/-0) src/protobuf/mir_protobuf.proto (+2/-0) src/server/frontend/event_sender.cpp (+11/-0) src/server/frontend/event_sender.h (+1/-0) src/server/scene/global_event_sender.cpp (+4/-0) src/server/scene/global_event_sender.h (+1/-0) tests/include/mir_test_doubles/mock_client_buffer_stream.h (+1/-0) tests/include/mir_test_doubles/mock_event_sink.h (+1/-0) tests/include/mir_test_doubles/null_event_sink.h (+1/-0) tests/integration-tests/test_exchange_buffer.cpp (+86/-3) tests/unit-tests/client/test_client_buffer_stream.cpp (+67/-5) |
To merge this branch: | bzr merge lp:~kdub/mir/server-sent-buffer |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
PS Jenkins bot (community) | continuous-integration | Approve | |
Chris Halse Rogers | Approve | ||
Andreas Pokorny (community) | Approve | ||
Alexandros Frantzis (community) | Approve | ||
Alan Griffiths | Approve | ||
Alberto Aguirre (community) | Approve | ||
Review via email: mp+261421@code.launchpad.net |
Commit message
Give the server the ability to send a buffer to the client without a client request. (part of the buffer semantics changes). The buffer is sent as an event; as its the part of the system that's suited to sending unrequested messages to the server
Description of the change
Give the server the ability to send a buffer to the client without a client request. (part of the buffer semantics changes). The buffer is sent as an event; as its the part of the system that's suited to sending unrequested messages to the server.
Note: this can really only send the 'partial' buffer updates now, a bit more plumbing is needed to hook up the platform-specific packing operations for the full buffer sending.
The transition strategy here is that the client will use the exchange_buffer() call to submit its buffers, until it receives a buffer asynchronously. After receiving the first 'async' buffer, it submits its buffers via the submit_buffer() call.
PS Jenkins bot (ps-jenkins) wrote : | # |
Kevin DuBois (kdub) wrote : | # |
^^doesnt look related at first glance, but haven't ruled out that is yet (and cant reproduce locally)...
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:2628
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
Chris Halse Rogers (raof) wrote : | # |
Is there a document with the proposed new buffer semantics anywhere? This doesn't match my understanding from Dallas. (Not that it's *wrong*, just that it's unexpected ☺)
Kevin DuBois (kdub) wrote : | # |
Notes are in this card: https:/
There were two ways that I could think of to do this, the buffer gets sent when its ready (the approach in the MP), or the client has to call some sort of acquire-buffer rpc call. The send-to-
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:2629
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
Alberto Aguirre (albaguirre) wrote : | # |
Yeah I guess from the Dallas discussion I assumed this to be an event FD (buffer writable event) that would be get populated at the time of creation to avoid rpc.
My understanding was something akin to:
During buffer create, a couple of fence fds are created - one for the server to signal when the buffer is writable by the client again (i.e buffer is no longer being read) and another for the client to signal the server that the buffer is now readable (i.e. finished writing to the buffer).
An RPC call was needed for the client to "schedule" a buffer - as the fence fd did not have enough context for scheduling (i.e. timestamping)
An fd signaling a buffer is writable however, seems like there's enough context there to avoid explicit RPC calls.
Kevin DuBois (kdub) wrote : | # |
There's also the problem that we'd have to send fences (in the android sense) with every signal of the fd pair, which is why I thought we shied away from wanting to do this.
Chris Halse Rogers (raof) wrote : | # |
My understanding was that submit_buffer was going to take a {bufferid, when-readable-
So, I guess the takeaway point is that we're not clear as a team how we expect this to work :)
Kevin DuBois (kdub) wrote : | # |
Just returning one fence that signals (perhaps we would epoll on it?) still has the problem that we have to go and get the FD that indicates when the buffer is ready. (the 'standard' android fences). The android fences could be (and, without confirming, probably are) a different FD each time a buffer fence is returned from the driver, so we have to send that fd via ipc each time we get one.
There's two timelines going on... one is from the driver, and that fence protects the buffers content (when the GPU/display thinks the buffer is ready). The second timeline is when mir thinks its ready (perhaps there's screenshot going on, or we have to hold the buffer a bit longer for a convoluted multimonitor scenario), and both have to be clear before the gpu can go and write in the buffer on the client side.
Alberto Aguirre (albaguirre) wrote : | # |
In the android platform I was envisioning returning the fence all the way to the driver not explicitly epolling.
ANW->dequeueBuffer would get any of the buffers that are not currently used by the driver along with its "this buffer is writable now" fence; that way no explicit waiting is done by the client code.
But I guess yeah the android fences could be different each time so an RPC would still be needed to update the fence on the client side.
Also the consumer side needs to be able to make the client wait until before retrieving a buffer (during dequeueBuffer) otherwise there would be a race getting the right sync fence fd.
Given that I guess we do need something like this MP.
Alberto Aguirre (albaguirre) wrote : | # |
OK for transitioning from exchange buffer.
Alan Griffiths (alan-griffiths) wrote : | # |
I guess this is OK as an intermediate form but...
21 + if (using_
22 + return exchange(done, std::move(lock));
23 + return submit(done, std::move(lock));
...
74 + using_exchange_
...
127 + bool using_exchange_
This feels like there's an abstraction missing. C.f.
return state->
Or perhaps a pointer to member function instead of a bool?
~~~~
35 + //in both cases, submit what we have
It would be helpful to explicitly state what the two cases are.
Alexandros Frantzis (afrantzis) wrote : | # |
OK.
Nits:
432 + bool satisfied = false;
Clearer as 'stub_buffer_
429 + //spin-wait for the id to become the current one.
Could use mt::spin_
498 + bs->next_
Could use just '[]{}'.
'[](){}' is a nice collection of brackets... C++ should introduce angle brackets in lambdas '[](){}⟨⟩' to become the first bracket-complete language! :)
Andreas Pokorny (andreas-pokorny) wrote : | # |
ok looks good
Kevin DuBois (kdub) wrote : | # |
@abstraction, There is one to be made, but we want to deprecate exchange_buffer soon anyways, so it didn't seem worth it to make this abstraction temporarily
Kevin DuBois (kdub) wrote : | # |
@Alexandros, stopped shy of using mt::spin_
@Chris, still hesitant? :)
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:2632
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
Chris Halse Rogers (raof) wrote : | # |
Ah, right. Yes. The problem with returning the GPU fence from submit_buffer is that server ownership of that buffer quite likely extends past the GPU read.
Although, hm.
Under the assumption that fences are represented as pollable fds *and* that pollable fds can be used as fences¹, we *could* do the fence-is-
This requires fences that have a signalled→
I'm mostly happy with this; if it turns out we can do better then we can do that extra funky stuff later.
¹: Or, alternatively, that we can do composite fences; the main requirement is that we can post-hoc combine fences and Mir can create and signal a fence itself.
Chris Halse Rogers (raof) wrote : | # |
Now, for the mostly happy bit:
submit_buffer() shouldn't block in that fashion. Why does it need to block at all?
I'm also not entirely sure why submit_buffer returns a MirWaitHandle, and even if it does I don't think that the MirWaitHandle it's returning should be the ‘there's a buffer available’ wait handle.
Kevin DuBois (kdub) wrote : | # |
mir_buffer_
Once the protocol changes have completed, we'll have to have to have some exposition of the buffer semantics in the client api (so that nested and multimedia can use the feature), and we'll have to support mir_buffer_
For now, its working this way just in the name of compatibility, and because we haven't rooted out the uni-buffer concept from other client side classes.
Chris Halse Rogers (raof) wrote : | # |
Isn't that the purpose of the done callback? Why is there the additional submitting condition variable, and the wait on that?
-----Original Message-----
From: "Kevin DuBois" <email address hidden>
Sent: 16/06/2015 21:35
To: "Kevin DuBois" <email address hidden>
Subject: Re: [Merge] lp:~kdub/mir/server-sent-buffer into lp:mir
mir_buffer_
Once the protocol changes have completed, we'll have to have to have some exposition of the buffer semantics in the client api (so that nested and multimedia can use the feature), and we'll have to support mir_buffer_
For now, its working this way just in the name of compatibility, and because we haven't rooted out the uni-buffer concept from other client side classes.
--
https:/
You are reviewing the proposed merge of lp:~kdub/mir/server-sent-buffer into lp:mir.
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:2634
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
Kevin DuBois (kdub) wrote : | # |
> Isn't that the purpose of the done callback? Why is there the additional
> submitting condition variable, and the wait on that?
For now (in the intermediate form), before mcl::BufferStre
Chris Halse Rogers (raof) wrote : | # |
Why do we have to wait for submit_buffer to finish?
We process RPC requests in order on the server, so as soon as it's submitted we know that it's going to be processed at the appropriate time.
As far as I can see, the client never needs to know when the server has acknowledged the submission; and for the transition it only needs to know when there's a buffer available to render into.
I think of any behaviour that would be incorrect if you treated submit_buffer as not having any response at all. If you tried it, what went wrong?
Kevin DuBois (kdub) wrote : | # |
@Chris, I see now what you're saying, makes sense. We don't have to wait for the /completion/ of submit_buffer on the server side, we just have to accomplish the sending-off of the submit_buffer request, and then get another buffer in the client side before returning.
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:2636
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:2637
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
FAILURE: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
Kevin DuBois (kdub) wrote : | # |
seems like CI just stopped before getting around to unit/integration?
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:2637
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
FAILURE: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
deb: http://
FAILURE: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
Kevin DuBois (kdub) wrote : | # |
Err http://
Temporary failure resolving 'ports.ubuntu.com'
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:2637
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
PS Jenkins bot (ps-jenkins) : | # |
Preview Diff
1 | === modified file 'src/client/buffer_stream.cpp' |
2 | --- src/client/buffer_stream.cpp 2015-06-17 18:24:42 +0000 |
3 | +++ src/client/buffer_stream.cpp 2015-06-19 17:03:24 +0000 |
4 | @@ -167,7 +167,11 @@ |
5 | void mcl::BufferStream::process_buffer(mp::Buffer const& buffer) |
6 | { |
7 | std::unique_lock<decltype(mutex)> lock(mutex); |
8 | - |
9 | + process_buffer(buffer, lock); |
10 | +} |
11 | + |
12 | +void mcl::BufferStream::process_buffer(protobuf::Buffer const& buffer, std::unique_lock<std::mutex> const&) |
13 | +{ |
14 | auto buffer_package = std::make_shared<MirBufferPackage>(); |
15 | populate_buffer_package(*buffer_package, buffer); |
16 | |
17 | @@ -202,6 +206,37 @@ |
18 | |
19 | secured_region.reset(); |
20 | |
21 | + if (using_exchange_buffer) |
22 | + return exchange(done, std::move(lock)); |
23 | + return submit(done, std::move(lock)); |
24 | +} |
25 | + |
26 | +MirWaitHandle* mcl::BufferStream::submit(std::function<void()> const& done, std::unique_lock<std::mutex> lock) |
27 | +{ |
28 | + //always submit what we have, whether we have a buffer, or will have to wait for an async reply |
29 | + auto request = mcl::make_protobuf_object<mp::BufferRequest>(); |
30 | + request->mutable_id()->set_value(protobuf_bs->id().value()); |
31 | + request->mutable_buffer()->set_buffer_id(protobuf_bs->buffer().buffer_id()); |
32 | + display_server.submit_buffer(nullptr, request.get(), protobuf_void.get(), |
33 | + google::protobuf::NewCallback(google::protobuf::DoNothing)); |
34 | + |
35 | + if (incoming_buffers.empty()) |
36 | + { |
37 | + next_buffer_wait_handle.expect_result(); |
38 | + on_incoming_buffer = done; |
39 | + } |
40 | + else |
41 | + { |
42 | + process_buffer(incoming_buffers.front(), lock); |
43 | + incoming_buffers.pop(); |
44 | + done(); |
45 | + } |
46 | + return &next_buffer_wait_handle; |
47 | +} |
48 | + |
49 | +MirWaitHandle* mcl::BufferStream::exchange( |
50 | + std::function<void()> const& done, std::unique_lock<std::mutex> lock) |
51 | +{ |
52 | // TODO: We can fix the strange "ID casting" used below in the second phase |
53 | // of buffer stream which generalizes and clarifies the server side logic. |
54 | if (mode == mcl::BufferStreamMode::Producer) |
55 | @@ -395,3 +430,21 @@ |
56 | { |
57 | buffer_depository.set_max_buffers(cache_size); |
58 | } |
59 | + |
60 | +void mcl::BufferStream::buffer_available(mir::protobuf::Buffer const& buffer) |
61 | +{ |
62 | + std::unique_lock<decltype(mutex)> lock(mutex); |
63 | + using_exchange_buffer = false; //this stream uses async exchange from here on out |
64 | + |
65 | + if (on_incoming_buffer) |
66 | + { |
67 | + process_buffer(buffer, lock); |
68 | + next_buffer_wait_handle.result_received(); |
69 | + on_incoming_buffer(); |
70 | + on_incoming_buffer = std::function<void()>{}; |
71 | + } |
72 | + else |
73 | + { |
74 | + incoming_buffers.push(buffer); |
75 | + } |
76 | +} |
77 | |
78 | === modified file 'src/client/buffer_stream.h' |
79 | --- src/client/buffer_stream.h 2015-06-17 18:24:42 +0000 |
80 | +++ src/client/buffer_stream.h 2015-06-19 17:03:24 +0000 |
81 | @@ -31,6 +31,7 @@ |
82 | |
83 | #include <EGL/eglplatform.h> |
84 | |
85 | +#include <queue> |
86 | #include <memory> |
87 | #include <mutex> |
88 | |
89 | @@ -104,6 +105,9 @@ |
90 | |
91 | frontend::BufferStreamId rpc_id() const override; |
92 | bool valid() const override; |
93 | + |
94 | + void buffer_available(mir::protobuf::Buffer const& buffer) override; |
95 | + |
96 | protected: |
97 | BufferStream(BufferStream const&) = delete; |
98 | BufferStream& operator=(BufferStream const&) = delete; |
99 | @@ -111,13 +115,20 @@ |
100 | private: |
101 | void created(mir_buffer_stream_callback callback, void* context); |
102 | void process_buffer(protobuf::Buffer const& buffer); |
103 | + void process_buffer(protobuf::Buffer const& buffer, std::unique_lock<std::mutex> const&); |
104 | void next_buffer_received( |
105 | std::function<void()> done); |
106 | void on_configured(); |
107 | void release_cpu_region(); |
108 | + MirWaitHandle* exchange(std::function<void()> const& done, std::unique_lock<std::mutex> lk); |
109 | + MirWaitHandle* submit(std::function<void()> const& done, std::unique_lock<std::mutex> lk); |
110 | |
111 | mutable std::mutex mutex; // Protects all members of *this |
112 | |
113 | + bool using_exchange_buffer = true; |
114 | + std::function<void()> on_incoming_buffer; |
115 | + std::queue<mir::protobuf::Buffer> incoming_buffers; |
116 | + |
117 | MirConnection* connection; |
118 | mir::protobuf::DisplayServer& display_server; |
119 | |
120 | |
121 | === modified file 'src/client/client_buffer_stream.h' |
122 | --- src/client/client_buffer_stream.h 2015-06-17 05:20:42 +0000 |
123 | +++ src/client/client_buffer_stream.h 2015-06-19 17:03:24 +0000 |
124 | @@ -63,7 +63,8 @@ |
125 | virtual MirWaitHandle* release(mir_buffer_stream_callback callback, void* context) = 0; |
126 | |
127 | virtual bool valid() const = 0; |
128 | - |
129 | + virtual void buffer_available(mir::protobuf::Buffer const& buffer) = 0; |
130 | + |
131 | protected: |
132 | ClientBufferStream() = default; |
133 | ClientBufferStream(const ClientBufferStream&) = delete; |
134 | |
135 | === modified file 'src/client/rpc/mir_protobuf_rpc_channel.cpp' |
136 | --- src/client/rpc/mir_protobuf_rpc_channel.cpp 2015-06-17 18:24:42 +0000 |
137 | +++ src/client/rpc/mir_protobuf_rpc_channel.cpp 2015-06-19 17:03:24 +0000 |
138 | @@ -251,6 +251,14 @@ |
139 | lifecycle_control->call_lifecycle_event_handler(seq->lifecycle_event().new_state()); |
140 | } |
141 | |
142 | + if (seq->has_buffer_request()) |
143 | + { |
144 | + surface_map->with_stream_do(mf::BufferStreamId(seq->buffer_request().id().value()), |
145 | + [&] (mcl::ClientBufferStream* stream) { |
146 | + stream->buffer_available(seq->buffer_request().buffer()); |
147 | + }); |
148 | + } |
149 | + |
150 | int const nevents = seq->event_size(); |
151 | for (int i = 0; i != nevents; ++i) |
152 | { |
153 | |
154 | === modified file 'src/include/server/mir/frontend/event_sink.h' |
155 | --- src/include/server/mir/frontend/event_sink.h 2015-02-22 07:46:25 +0000 |
156 | +++ src/include/server/mir/frontend/event_sink.h 2015-06-19 17:03:24 +0000 |
157 | @@ -20,12 +20,14 @@ |
158 | #define MIR_EVENTS_EVENT_SINK_H_ |
159 | |
160 | #include "mir_toolkit/event.h" |
161 | +#include "mir/frontend/buffer_stream_id.h" |
162 | |
163 | namespace mir |
164 | { |
165 | namespace graphics |
166 | { |
167 | class DisplayConfiguration; |
168 | +class Buffer; |
169 | } |
170 | namespace frontend |
171 | { |
172 | @@ -37,6 +39,7 @@ |
173 | virtual void handle_event(MirEvent const& e) = 0; |
174 | virtual void handle_lifecycle_event(MirLifecycleState state) = 0; |
175 | virtual void handle_display_config_change(graphics::DisplayConfiguration const& config) = 0; |
176 | + virtual void send_buffer(frontend::BufferStreamId id, graphics::Buffer& buffer) = 0; |
177 | |
178 | protected: |
179 | EventSink() = default; |
180 | |
181 | === modified file 'src/protobuf/mir_protobuf.proto' |
182 | --- src/protobuf/mir_protobuf.proto 2015-06-17 19:16:03 +0000 |
183 | +++ src/protobuf/mir_protobuf.proto 2015-06-19 17:03:24 +0000 |
184 | @@ -245,6 +245,8 @@ |
185 | repeated Event event = 1; |
186 | optional DisplayConfiguration display_configuration = 2; |
187 | optional LifecycleEvent lifecycle_event = 3; |
188 | + optional BufferRequest buffer_request = 4; |
189 | + |
190 | optional string error = 127; |
191 | } |
192 | |
193 | |
194 | === modified file 'src/server/frontend/event_sender.cpp' |
195 | --- src/server/frontend/event_sender.cpp 2015-06-17 05:20:42 +0000 |
196 | +++ src/server/frontend/event_sender.cpp 2015-06-19 17:03:24 +0000 |
197 | @@ -24,6 +24,8 @@ |
198 | #include "message_sender.h" |
199 | #include "protobuf_buffer_packer.h" |
200 | |
201 | +#include "mir/graphics/buffer.h" |
202 | + |
203 | #include "mir_protobuf_wire.pb.h" |
204 | #include "mir_protobuf.pb.h" |
205 | |
206 | @@ -95,3 +97,12 @@ |
207 | (void) error; |
208 | } |
209 | } |
210 | + |
211 | +void mfd::EventSender::send_buffer(frontend::BufferStreamId id, graphics::Buffer& buffer) |
212 | +{ |
213 | + mp::EventSequence seq; |
214 | + auto request = seq.mutable_buffer_request(); |
215 | + request->mutable_id()->set_value(id.as_value()); |
216 | + request->mutable_buffer()->set_buffer_id(buffer.id().as_value()); |
217 | + send_event_sequence(seq); |
218 | +} |
219 | |
220 | === modified file 'src/server/frontend/event_sender.h' |
221 | --- src/server/frontend/event_sender.h 2015-02-22 07:46:25 +0000 |
222 | +++ src/server/frontend/event_sender.h 2015-06-19 17:03:24 +0000 |
223 | @@ -41,6 +41,7 @@ |
224 | void handle_event(MirEvent const& e); |
225 | void handle_lifecycle_event(MirLifecycleState state); |
226 | void handle_display_config_change(graphics::DisplayConfiguration const& config); |
227 | + void send_buffer(frontend::BufferStreamId id, graphics::Buffer& buffer); |
228 | |
229 | private: |
230 | void send_event_sequence(protobuf::EventSequence&); |
231 | |
232 | === modified file 'src/server/scene/global_event_sender.cpp' |
233 | --- src/server/scene/global_event_sender.cpp 2015-02-22 07:46:25 +0000 |
234 | +++ src/server/scene/global_event_sender.cpp 2015-06-19 17:03:24 +0000 |
235 | @@ -45,3 +45,7 @@ |
236 | session->send_display_config(config); |
237 | }); |
238 | } |
239 | + |
240 | +void ms::GlobalEventSender::send_buffer(mir::frontend::BufferStreamId, mg::Buffer&) |
241 | +{ |
242 | +} |
243 | |
244 | === modified file 'src/server/scene/global_event_sender.h' |
245 | --- src/server/scene/global_event_sender.h 2015-02-22 07:46:25 +0000 |
246 | +++ src/server/scene/global_event_sender.h 2015-06-19 17:03:24 +0000 |
247 | @@ -36,6 +36,7 @@ |
248 | void handle_event(MirEvent const& e); |
249 | void handle_lifecycle_event(MirLifecycleState state); |
250 | void handle_display_config_change(graphics::DisplayConfiguration const& config); |
251 | + void send_buffer(frontend::BufferStreamId id, graphics::Buffer& buffer); |
252 | |
253 | private: |
254 | std::shared_ptr<SessionContainer> const sessions; |
255 | |
256 | === modified file 'tests/include/mir_test_doubles/mock_client_buffer_stream.h' |
257 | --- tests/include/mir_test_doubles/mock_client_buffer_stream.h 2015-06-17 05:20:42 +0000 |
258 | +++ tests/include/mir_test_doubles/mock_client_buffer_stream.h 2015-06-19 17:03:24 +0000 |
259 | @@ -45,6 +45,7 @@ |
260 | MOCK_METHOD0(get_create_wait_handle, MirWaitHandle*(void)); |
261 | MOCK_CONST_METHOD0(rpc_id, frontend::BufferStreamId(void)); |
262 | MOCK_CONST_METHOD0(valid, bool(void)); |
263 | + MOCK_METHOD1(buffer_available, void(mir::protobuf::Buffer const&)); |
264 | }; |
265 | |
266 | } |
267 | |
268 | === modified file 'tests/include/mir_test_doubles/mock_event_sink.h' |
269 | --- tests/include/mir_test_doubles/mock_event_sink.h 2015-02-11 22:10:31 +0000 |
270 | +++ tests/include/mir_test_doubles/mock_event_sink.h 2015-06-19 17:03:24 +0000 |
271 | @@ -34,6 +34,7 @@ |
272 | MOCK_METHOD1(handle_event, void(MirEvent const&)); |
273 | MOCK_METHOD1(handle_lifecycle_event, void(MirLifecycleState)); |
274 | MOCK_METHOD1(handle_display_config_change, void(graphics::DisplayConfiguration const&)); |
275 | + MOCK_METHOD2(send_buffer, void(frontend::BufferStreamId, graphics::Buffer&)); |
276 | }; |
277 | } |
278 | } |
279 | |
280 | === modified file 'tests/include/mir_test_doubles/null_event_sink.h' |
281 | --- tests/include/mir_test_doubles/null_event_sink.h 2015-02-22 07:46:25 +0000 |
282 | +++ tests/include/mir_test_doubles/null_event_sink.h 2015-06-19 17:03:24 +0000 |
283 | @@ -32,6 +32,7 @@ |
284 | void handle_event(MirEvent const&) {} |
285 | void handle_lifecycle_event(MirLifecycleState) {} |
286 | void handle_display_config_change(graphics::DisplayConfiguration const&) {} |
287 | + void send_buffer(frontend::BufferStreamId, graphics::Buffer&) {} |
288 | }; |
289 | } |
290 | } |
291 | |
292 | === modified file 'tests/integration-tests/test_exchange_buffer.cpp' |
293 | --- tests/integration-tests/test_exchange_buffer.cpp 2015-06-17 19:16:03 +0000 |
294 | +++ tests/integration-tests/test_exchange_buffer.cpp 2015-06-19 17:03:24 +0000 |
295 | @@ -28,10 +28,13 @@ |
296 | #include "mir/graphics/buffer_ipc_message.h" |
297 | #include "mir/graphics/platform_operation_message.h" |
298 | #include "mir/scene/buffer_stream_factory.h" |
299 | +#include "mir/scene/session_coordinator.h" |
300 | +#include "mir/frontend/event_sink.h" |
301 | #include "mir/compositor/buffer_stream.h" |
302 | #include "src/server/compositor/buffer_bundle.h" |
303 | #include "src/server/compositor/buffer_stream_surfaces.h" |
304 | #include "mir_toolkit/mir_client_library.h" |
305 | +#include "mir_toolkit/debug/surface.h" |
306 | #include "src/client/mir_connection.h" |
307 | #include <chrono> |
308 | #include <mutex> |
309 | @@ -49,6 +52,7 @@ |
310 | namespace mc = mir::compositor; |
311 | namespace geom = mir::geometry; |
312 | namespace mp = mir::protobuf; |
313 | +namespace mf = mir::frontend; |
314 | |
315 | namespace |
316 | { |
317 | @@ -176,6 +180,47 @@ |
318 | std::shared_ptr<mg::PlatformIpcOperations> const ipc_ops; |
319 | }; |
320 | |
321 | +class SinkSkimmingCoordinator : public msc::SessionCoordinator |
322 | +{ |
323 | +public: |
324 | + SinkSkimmingCoordinator(std::shared_ptr<msc::SessionCoordinator> const& wrapped) : |
325 | + wrapped(wrapped) |
326 | + { |
327 | + } |
328 | + |
329 | + void set_focus_to(std::shared_ptr<msc::Session> const& focus) override |
330 | + { |
331 | + wrapped->set_focus_to(focus); |
332 | + } |
333 | + |
334 | + void unset_focus() override |
335 | + { |
336 | + wrapped->unset_focus(); |
337 | + } |
338 | + |
339 | + std::shared_ptr<msc::Session> open_session( |
340 | + pid_t client_pid, |
341 | + std::string const& name, |
342 | + std::shared_ptr<mf::EventSink> const& sink) override |
343 | + { |
344 | + last_sink = sink; |
345 | + return wrapped->open_session(client_pid, name, sink); |
346 | + } |
347 | + |
348 | + void close_session(std::shared_ptr<msc::Session> const& session) override |
349 | + { |
350 | + wrapped->close_session(session); |
351 | + } |
352 | + |
353 | + std::shared_ptr<msc::Session> successor_of(std::shared_ptr<msc::Session> const& session) const override |
354 | + { |
355 | + return wrapped->successor_of(session); |
356 | + } |
357 | + |
358 | + std::shared_ptr<msc::SessionCoordinator> const wrapped; |
359 | + std::weak_ptr<mf::EventSink> last_sink; |
360 | +}; |
361 | + |
362 | struct ExchangeServerConfiguration : mtf::StubbedServerConfiguration |
363 | { |
364 | ExchangeServerConfiguration( |
365 | @@ -186,6 +231,14 @@ |
366 | { |
367 | } |
368 | |
369 | + std::shared_ptr<msc::SessionCoordinator> the_session_coordinator() override |
370 | + { |
371 | + return session_coordinator([this]{ |
372 | + coordinator = std::make_shared<SinkSkimmingCoordinator>( |
373 | + DefaultServerConfiguration::the_session_coordinator()); |
374 | + return coordinator; |
375 | + }); |
376 | + } |
377 | std::shared_ptr<mg::Platform> the_graphics_platform() override |
378 | { |
379 | return platform; |
380 | @@ -196,8 +249,9 @@ |
381 | return stream_factory; |
382 | } |
383 | |
384 | - std::shared_ptr<msc::BufferStreamFactory> const stream_factory; |
385 | + std::shared_ptr<StubBundleFactory> const stream_factory; |
386 | std::shared_ptr<mg::Platform> const platform; |
387 | + std::shared_ptr<SinkSkimmingCoordinator> coordinator; |
388 | }; |
389 | |
390 | struct ExchangeBufferTest : mir_test_framework::InProcessServer |
391 | @@ -217,8 +271,6 @@ |
392 | cv.notify_all(); |
393 | } |
394 | |
395 | - //TODO: once the next_buffer rpc is deprecated, change this code out for the |
396 | - // mir_surface_next_buffer() api call |
397 | bool exchange_buffer(mp::DisplayServer& server) |
398 | { |
399 | std::unique_lock<decltype(mutex)> lk(mutex); |
400 | @@ -359,6 +411,37 @@ |
401 | mir_connection_release(connection); |
402 | } |
403 | |
404 | +TEST_F(ExchangeBufferTest, server_can_send_buffer) |
405 | +{ |
406 | + using namespace testing; |
407 | + using namespace std::literals::chrono_literals; |
408 | + mtd::StubBuffer stub_buffer; |
409 | + auto connection = mir_connect_sync(new_connection().c_str(), __PRETTY_FUNCTION__); |
410 | + auto surface = mtf::make_any_surface(connection); |
411 | + auto sink = server_configuration.coordinator->last_sink.lock(); |
412 | + sink->send_buffer(mf::BufferStreamId{0}, stub_buffer); |
413 | + |
414 | + //spin-wait for the id to become the current one. |
415 | + //The notification doesn't generate a client-facing callback on the stream yet |
416 | + //(although probably should, seems something a media decoder would need |
417 | + bool buffer_arrived = false; |
418 | + auto timeout = std::chrono::steady_clock::now() + 5s; |
419 | + while(!buffer_arrived && std::chrono::steady_clock::now() < timeout) |
420 | + { |
421 | + mir_buffer_stream_swap_buffers_sync(mir_surface_get_buffer_stream(surface)); |
422 | + if (mir_debug_surface_current_buffer_id(surface) == stub_buffer.id().as_value()) |
423 | + { |
424 | + buffer_arrived = true; |
425 | + break; |
426 | + } |
427 | + std::this_thread::yield(); |
428 | + } |
429 | + EXPECT_THAT(buffer_arrived, Eq(true)) << "failed to see the sent buffer become the current one"; |
430 | + |
431 | + mir_surface_release_sync(surface); |
432 | + mir_connection_release(connection); |
433 | +} |
434 | + |
435 | //TODO: check that a buffer arrives asynchronously. |
436 | TEST_F(ExchangeBufferTest, allocate_buffers_doesnt_time_out) |
437 | { |
438 | |
439 | === modified file 'tests/unit-tests/client/test_client_buffer_stream.cpp' |
440 | --- tests/unit-tests/client/test_client_buffer_stream.cpp 2015-06-17 05:20:42 +0000 |
441 | +++ tests/unit-tests/client/test_client_buffer_stream.cpp 2015-06-19 17:03:24 +0000 |
442 | @@ -50,6 +50,11 @@ |
443 | mp::ScreencastId const* /*request*/, |
444 | mp::Buffer* /*response*/, |
445 | google::protobuf::Closure* /*done*/)); |
446 | + MOCK_METHOD4(submit_buffer, |
447 | + void(google::protobuf::RpcController* /*controller*/, |
448 | + mp::BufferRequest const* /*request*/, |
449 | + mp::Void* /*response*/, |
450 | + google::protobuf::Closure* /*done*/)); |
451 | MOCK_METHOD4(exchange_buffer, |
452 | void(google::protobuf::RpcController* /*controller*/, |
453 | mp::BufferRequest const* /*request*/, |
454 | @@ -113,7 +118,7 @@ |
455 | |
456 | struct ClientBufferStreamTest : public testing::Test |
457 | { |
458 | - mtd::MockClientBufferFactory mock_client_buffer_factory; |
459 | + testing::NiceMock<mtd::MockClientBufferFactory> mock_client_buffer_factory; |
460 | mtd::StubClientBufferFactory stub_client_buffer_factory; |
461 | |
462 | MockProtobufServer mock_protobuf_server; |
463 | @@ -283,7 +288,7 @@ |
464 | |
465 | auto bs = make_buffer_stream(protobuf_bs, mcl::BufferStreamMode::Producer); |
466 | |
467 | - bs->next_buffer([](){}); |
468 | + bs->next_buffer([]{}); |
469 | } |
470 | |
471 | TEST_F(ClientBufferStreamTest, consumer_streams_call_screencast_buffer_on_next_buffer) |
472 | @@ -298,7 +303,7 @@ |
473 | |
474 | auto bs = make_buffer_stream(protobuf_bs, mcl::BufferStreamMode::Consumer); |
475 | |
476 | - bs->next_buffer([](){}); |
477 | + bs->next_buffer([]{}); |
478 | } |
479 | |
480 | TEST_F(ClientBufferStreamTest, invokes_callback_on_next_buffer) |
481 | @@ -363,7 +368,7 @@ |
482 | auto bs = make_buffer_stream(protobuf_bs, mock_client_buffer_factory); |
483 | |
484 | EXPECT_EQ(client_buffer_1, bs->get_current_buffer()); |
485 | - bs->next_buffer([](){}); |
486 | + bs->next_buffer([]{}); |
487 | EXPECT_EQ(client_buffer_2, bs->get_current_buffer()); |
488 | } |
489 | |
490 | @@ -397,7 +402,7 @@ |
491 | auto bs = make_buffer_stream(protobuf_bs, mock_client_buffer_factory); |
492 | |
493 | EXPECT_EQ(client_buffer_1, bs->get_current_buffer()); |
494 | - bs->next_buffer([](){}); |
495 | + bs->next_buffer([]{}); |
496 | EXPECT_EQ(client_buffer_2, bs->get_current_buffer()); |
497 | } |
498 | |
499 | @@ -453,3 +458,60 @@ |
500 | std::make_shared<StubClientPlatform>(mt::fake_shared(stub_client_buffer_factory)), |
501 | protobuf_bs, mt::fake_shared(mock_perf_report), name); |
502 | } |
503 | + |
504 | +TEST_F(ClientBufferStreamTest, receives_unsolicited_buffer) |
505 | +{ |
506 | + using namespace ::testing; |
507 | + int id = 88; |
508 | + MockClientBuffer mock_client_buffer; |
509 | + MockClientBuffer second_mock_client_buffer; |
510 | + MirBufferPackage buffer_package = a_buffer_package(); |
511 | + auto protobuf_bs = a_protobuf_buffer_stream(default_pixel_format, default_buffer_usage, buffer_package); |
512 | + EXPECT_CALL(mock_client_buffer_factory, create_buffer(BufferPackageMatches(buffer_package),_,_)) |
513 | + .WillOnce(Return(mt::fake_shared(mock_client_buffer))); |
514 | + auto bs = make_buffer_stream(protobuf_bs, mock_client_buffer_factory); |
515 | + |
516 | + mir::protobuf::Buffer another_buffer_package; |
517 | + another_buffer_package.set_buffer_id(id); |
518 | + EXPECT_CALL(mock_client_buffer_factory, create_buffer(_,_,_)) |
519 | + .WillOnce(Return(mt::fake_shared(second_mock_client_buffer))); |
520 | + EXPECT_CALL(mock_protobuf_server, submit_buffer(_,_,_,_)) |
521 | + .WillOnce(RunProtobufClosure()); |
522 | + bs->buffer_available(another_buffer_package); |
523 | + bs->next_buffer([]{}); |
524 | + |
525 | + EXPECT_THAT(bs->get_current_buffer().get(), Eq(&second_mock_client_buffer)); |
526 | + EXPECT_THAT(bs->get_current_buffer_id(), Eq(id)); |
527 | +} |
528 | + |
529 | +//useful only in transitioning from exchange to async |
530 | +TEST_F(ClientBufferStreamTest, after_receiving_an_unsolicited_buffer_exchange_buffer_return_are_ignored) |
531 | +{ |
532 | + using namespace ::testing; |
533 | + int id = 88; |
534 | + MockClientBuffer mock_client_buffer; |
535 | + MockClientBuffer second_mock_client_buffer; |
536 | + MirBufferPackage buffer_package = a_buffer_package(); |
537 | + mir::protobuf::Buffer another_buffer_package; |
538 | + another_buffer_package.set_buffer_id(id); |
539 | + auto protobuf_bs = a_protobuf_buffer_stream(default_pixel_format, default_buffer_usage, buffer_package); |
540 | + ON_CALL(mock_client_buffer_factory, create_buffer(_,_,_)) |
541 | + .WillByDefault(Return(mt::fake_shared(mock_client_buffer))); |
542 | + auto bs = make_buffer_stream(protobuf_bs, mock_client_buffer_factory); |
543 | + |
544 | + int a_few_times = 11; |
545 | + EXPECT_CALL(mock_protobuf_server, exchange_buffer(_,_,_,_)) |
546 | + .Times(a_few_times) |
547 | + .WillRepeatedly(RunProtobufClosure()); |
548 | + EXPECT_CALL(mock_protobuf_server, submit_buffer(_,_,_,_)) |
549 | + .Times(a_few_times) |
550 | + .WillRepeatedly(RunProtobufClosure()); |
551 | + |
552 | + for(auto i = 0; i < a_few_times; i++) |
553 | + bs->next_buffer([]{}); |
554 | + |
555 | + bs->buffer_available(another_buffer_package); |
556 | + |
557 | + for(auto i = 0; i < a_few_times; i++) |
558 | + bs->next_buffer([]{}); |
559 | +} |
FAILED: Continuous integration, rev:2628 jenkins. qa.ubuntu. com/job/ mir-ci/ 4034/ jenkins. qa.ubuntu. com/job/ mir-android- vivid-i386- build/2753 jenkins. qa.ubuntu. com/job/ mir-clang- wily-amd64- build/266 jenkins. qa.ubuntu. com/job/ mir-mediumtests -vivid- touch/2701/ console jenkins. qa.ubuntu. com/job/ mir-wily- amd64-ci/ 190/console jenkins. qa.ubuntu. com/job/ mir-mediumtests -builder- vivid-armhf/ 2701/console
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild: s-jenkins. ubuntu- ci:8080/ job/mir- ci/4034/ rebuild
http://