Mir

Merge lp:~albaguirre/mir/back-to-stack-alloc-mir-protobuf into lp:mir

  1. back-to-stack-alloc-mir-protobuf
  2. Merge into development-branch
Proposed by Alberto Aguirre
Status: Merged
Approved by: Alberto Aguirre
Approved revision: no longer in the source branch.
Merged at revision: 2801
Proposed branch: lp:~albaguirre/mir/back-to-stack-alloc-mir-protobuf
Merge into: lp:mir
Prerequisite: lp:~albaguirre/mir/enable-versioning-of-mirprotobuf
Diff against target: 897 lines (+226/-196)
13 files modified
src/client/buffer_stream.cpp (+15/-14)
src/client/buffer_stream.h (+6/-2)
src/client/client_buffer_stream.h (+4/-0)
src/client/mir_connection.cpp (+26/-24)
src/client/mir_connection.h (+8/-2)
src/client/mir_prompt_session.cpp (+4/-3)
src/client/mir_prompt_session.h (+6/-1)
src/client/mir_screencast.cpp (+13/-12)
src/client/mir_screencast.h (+5/-1)
src/client/mir_surface.cpp (+101/-103)
src/client/mir_surface.h (+8/-3)
src/client/rpc/mir_protobuf_rpc_channel.cpp (+30/-29)
src/client/rpc/mir_protobuf_rpc_channel.h (+0/-2)
To merge this branch: bzr merge lp:~albaguirre/mir/back-to-stack-alloc-mir-protobuf
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
Alan Griffiths Approve
Kevin DuBois (community) Approve
Review via email: mp+266275@code.launchpad.net

Commit message

client: Use stack allocated mir::protobuf objects when possible

Description of the change

client: Use stack allocated mir::protobuf objects when possible

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

FAILED: Continuous integration, rev:2789
http://jenkins.qa.ubuntu.com/job/mir-ci/4439/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/3347
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/mir-clang-ts-wily-amd64-build/200
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-wily-amd64-build/872/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/3295
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/588
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/588/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/3295
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/3295/artifact/work/output/*zip*/output.zip
    None: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/6088/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/4439/rebuild

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

FAILED: Continuous integration, rev:2789
http://jenkins.qa.ubuntu.com/job/mir-ci/4438/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/3346/console
    FAILURE: http://s-jenkins.ubuntu-ci:8080/job/mir-clang-ts-wily-amd64-build/199/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-wily-amd64-build/871/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/3294/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/587/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/3294/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/4438/rebuild

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

PASSED: Continuous integration, rev:2790
http://jenkins.qa.ubuntu.com/job/mir-ci/4440/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/3348
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/mir-clang-ts-wily-amd64-build/201
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-wily-amd64-build/873
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/3296
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/589
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/589/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/3296
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/3296/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/6089
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/22198

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/4440/rebuild

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

I know I asked for this. Although now I realize it's actually less efficient than using Arenas. Because on the stack your object is indeed allocated super fast (adjusting the stack pointer has always taken one clock tick or less). But stack allocation doesn't solve the problem for the subobjects (variable length lists, strings) that are still allocated on the heap. Arenas however solve both problems with the speed of a stack:

   https://developers.google.com/protocol-buffers/docs/reference/arenas?hl=en

So I've got nothing against this proposal right now. But if it makes the syntax less like the Arena syntax then it might be harder for us to get that optimal performance with Arenas in the long run.

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

281 class SharedLibrary;
282 -
283 +namespace protobuf
284 +{

I've a mild preference for a blank line before the namespace block.

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

+1

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

> I know I asked for this. Although now I realize it's actually less efficient
> than using Arenas. Because on the stack your object is indeed allocated super
> fast (adjusting the stack pointer has always taken one clock tick or less).
> But stack allocation doesn't solve the problem for the subobjects (variable
> length lists, strings) that are still allocated on the heap. Arenas however
> solve both problems with the speed of a stack:
>
> https://developers.google.com/protocol-buffers/docs/reference/arenas?hl=en
>
> So I've got nothing against this proposal right now. But if it makes the
> syntax less like the Arena syntax then it might be harder for us to get that
> optimal performance with Arenas in the long run.

Ok, it shouldn't be too difficult to change to Arena based allocation even if this lands, but I need to investigate the code generated when "option cc_enable_arenas = true;" is used.

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

OK

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

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/mir-autolanding/1726/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/3368
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/mir-clang-ts-wily-amd64-build/221
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-wily-amd64-build/893
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/3316
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-autolanding/279/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/3316
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/3316/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/6107
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/22253

review: Needs Fixing (continuous-integration)
Revision history for this message
Alberto Aguirre (albaguirre) wrote :

^--Failure seems to be lp:1480422, lp:1480420

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

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/mir-autolanding/1727/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/3373
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/mir-clang-ts-wily-amd64-build/226
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-wily-amd64-build/898
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/3321/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-autolanding/280
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-autolanding/280/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/3321
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/3321/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/6112/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/22262

review: Needs Fixing (continuous-integration)
Revision history for this message
Alberto Aguirre (albaguirre) wrote :

"E: Failed to fetch http://ports.ubuntu.com/ubuntu-ports/pool/universe/g/glmark2/glmark2-data_2014.03+git20150611.fa71af2d-0ubuntu2_all.deb Temporary failure resolving 'ports.ubuntu.com'"

Arrghh....Re-trying.

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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/client/buffer_stream.cpp'
2--- src/client/buffer_stream.cpp 2015-07-29 17:33:10 +0000
3+++ src/client/buffer_stream.cpp 2015-07-29 17:33:10 +0000
4@@ -24,6 +24,7 @@
5 #include "perf_report.h"
6 #include "logging/perf_report.h"
7 #include "rpc/mir_display_server.h"
8+#include "mir_protobuf.pb.h"
9
10 #include "mir/log.h"
11 #include "mir/client_platform.h"
12@@ -201,12 +202,12 @@
13 MirWaitHandle* mcl::BufferStream::submit(std::function<void()> const& done, std::unique_lock<std::mutex> lock)
14 {
15 //always submit what we have, whether we have a buffer, or will have to wait for an async reply
16- auto request = mcl::make_protobuf_object<mp::BufferRequest>();
17- request->mutable_id()->set_value(protobuf_bs->id().value());
18- request->mutable_buffer()->set_buffer_id(buffer_depository.current_buffer_id());
19+ mp::BufferRequest request;
20+ request.mutable_id()->set_value(protobuf_bs->id().value());
21+ request.mutable_buffer()->set_buffer_id(buffer_depository.current_buffer_id());
22 lock.unlock();
23
24- display_server.submit_buffer(request.get(), protobuf_void.get(),
25+ display_server.submit_buffer(&request, protobuf_void.get(),
26 google::protobuf::NewCallback(google::protobuf::DoNothing));
27
28 lock.lock();
29@@ -241,14 +242,14 @@
30 }
31 else
32 {
33- auto screencast_id = mcl::make_protobuf_object<mp::ScreencastId>();
34- screencast_id->set_value(protobuf_bs->id().value());
35+ mp::ScreencastId screencast_id;
36+ screencast_id.set_value(protobuf_bs->id().value());
37
38 lock.unlock();
39 next_buffer_wait_handle.expect_result();
40
41 display_server.screencast_buffer(
42- screencast_id.get(),
43+ &screencast_id,
44 protobuf_bs->mutable_buffer(),
45 google::protobuf::NewCallback(
46 this, &mcl::BufferStream::next_buffer_received,
47@@ -337,21 +338,21 @@
48 BOOST_THROW_EXCEPTION(std::logic_error("Attempt to set swap interval on screencast is invalid"));
49 }
50
51- auto setting = mcl::make_protobuf_object<mp::SurfaceSetting>();
52- auto result = mcl::make_protobuf_object<mp::SurfaceSetting>();
53- setting->mutable_surfaceid()->set_value(protobuf_bs->id().value());
54- setting->set_attrib(attrib);
55- setting->set_ivalue(value);
56+ mp::SurfaceSetting setting;
57+ mp::SurfaceSetting result;
58+ setting.mutable_surfaceid()->set_value(protobuf_bs->id().value());
59+ setting.set_attrib(attrib);
60+ setting.set_ivalue(value);
61 lock.unlock();
62
63 configure_wait_handle.expect_result();
64- display_server.configure_surface(setting.get(), result.get(),
65+ display_server.configure_surface(&setting, &result,
66 google::protobuf::NewCallback(this, &mcl::BufferStream::on_configured));
67
68 configure_wait_handle.wait_for_all();
69
70 lock.lock();
71- swap_interval_ = result->ivalue();
72+ swap_interval_ = result.ivalue();
73 }
74
75 uint32_t mcl::BufferStream::get_current_buffer_id()
76
77=== modified file 'src/client/buffer_stream.h'
78--- src/client/buffer_stream.h 2015-07-29 17:33:10 +0000
79+++ src/client/buffer_stream.h 2015-07-29 17:33:10 +0000
80@@ -19,8 +19,6 @@
81 #ifndef MIR_CLIENT_BUFFER_STREAM_H
82 #define MIR_CLIENT_BUFFER_STREAM_H
83
84-#include "mir_protobuf.pb.h"
85-
86 #include "mir_wait_handle.h"
87 #include "mir/egl_native_surface.h"
88 #include "mir/client_buffer.h"
89@@ -41,6 +39,12 @@
90 {
91 class Logger;
92 }
93+namespace protobuf
94+{
95+class BufferStream;
96+class BufferStreamParameters;
97+class Void;
98+}
99 namespace client
100 {
101 namespace rpc
102
103=== modified file 'src/client/client_buffer_stream.h'
104--- src/client/client_buffer_stream.h 2015-07-16 07:03:19 +0000
105+++ src/client/client_buffer_stream.h 2015-07-29 17:33:10 +0000
106@@ -32,6 +32,10 @@
107
108 namespace mir
109 {
110+namespace protobuf
111+{
112+class Buffer;
113+}
114 namespace client
115 {
116 class ClientBuffer;
117
118=== modified file 'src/client/mir_connection.cpp'
119--- src/client/mir_connection.cpp 2015-07-29 17:33:10 +0000
120+++ src/client/mir_connection.cpp 2015-07-29 17:33:10 +0000
121@@ -19,6 +19,7 @@
122 #include "mir_connection.h"
123 #include "mir_surface.h"
124 #include "mir_prompt_session.h"
125+#include "mir_protobuf.pb.h"
126 #include "default_client_buffer_stream_factory.h"
127 #include "make_protobuf_object.h"
128 #include "mir_toolkit/mir_platform_message.h"
129@@ -48,6 +49,7 @@
130 namespace mev = mir::events;
131 namespace gp = google::protobuf;
132 namespace mf = mir::frontend;
133+namespace mp = mir::protobuf;
134
135 namespace
136 {
137@@ -229,8 +231,8 @@
138
139 SurfaceRelease surf_release{surface, new_wait_handle, callback, context};
140
141- auto message = mcl::make_protobuf_object<mir::protobuf::SurfaceId>();
142- message->set_value(surface->id());
143+ mp::SurfaceId message;
144+ message.set_value(surface->id());
145
146 {
147 std::lock_guard<decltype(release_wait_handle_guard)> rel_lock(release_wait_handle_guard);
148@@ -238,7 +240,7 @@
149 }
150
151 new_wait_handle->expect_result();
152- server.release_surface(message.get(), void_response.get(),
153+ server.release_surface(&message, void_response.get(),
154 gp::NewCallback(this, &MirConnection::released, surf_release));
155
156
157@@ -391,19 +393,19 @@
158 return nullptr;
159 }
160
161- auto protobuf_request = mcl::make_protobuf_object<mir::protobuf::PlatformOperationMessage>();
162+ mp::PlatformOperationMessage protobuf_request;
163
164- protobuf_request->set_opcode(mir_platform_message_get_opcode(request));
165+ protobuf_request.set_opcode(mir_platform_message_get_opcode(request));
166 auto const request_data = mir_platform_message_get_data(request);
167 auto const request_fds = mir_platform_message_get_fds(request);
168
169- protobuf_request->set_data(request_data.data, request_data.size);
170+ protobuf_request.set_data(request_data.data, request_data.size);
171 for (size_t i = 0; i != request_fds.num_fds; ++i)
172- protobuf_request->add_fd(request_fds.fds[i]);
173+ protobuf_request.add_fd(request_fds.fds[i]);
174
175 platform_operation_wait_handle.expect_result();
176 server.platform_operation(
177- protobuf_request.get(),
178+ &protobuf_request,
179 platform_operation_reply.get(),
180 google::protobuf::NewCallback(this, &MirConnection::done_platform_operation,
181 callback, context));
182@@ -499,17 +501,17 @@
183 mir_buffer_stream_callback callback,
184 void *context)
185 {
186- auto params = mcl::make_protobuf_object<mir::protobuf::BufferStreamParameters>();
187- params->set_width(width);
188- params->set_height(height);
189- params->set_pixel_format(format);
190- params->set_buffer_usage(buffer_usage);
191+ mp::BufferStreamParameters params;
192+ params.set_width(width);
193+ params.set_height(height);
194+ params.set_pixel_format(format);
195+ params.set_buffer_usage(buffer_usage);
196
197- return buffer_stream_factory->make_producer_stream(this, server, *params, callback, context);
198+ return buffer_stream_factory->make_producer_stream(this, server, params, callback, context);
199 }
200
201 std::shared_ptr<mir::client::ClientBufferStream> MirConnection::make_consumer_stream(
202- mir::protobuf::BufferStream const& protobuf_bs, std::string const& surface_name)
203+ mp::BufferStream const& protobuf_bs, std::string const& surface_name)
204 {
205 return buffer_stream_factory->make_consumer_stream(this, server, protobuf_bs, surface_name);
206 }
207@@ -549,9 +551,9 @@
208
209 void MirConnection::pong(int32_t serial)
210 {
211- auto pong = mcl::make_protobuf_object<mir::protobuf::PingEvent>();
212- pong->set_serial(serial);
213- server.pong(pong.get(), void_response.get(), pong_callback.get());
214+ mp::PingEvent pong;
215+ pong.set_serial(serial);
216+ server.pong(&pong, void_response.get(), pong_callback.get());
217 }
218
219 void MirConnection::register_display_change_callback(mir_display_config_callback callback, void* context)
220@@ -603,14 +605,14 @@
221 return NULL;
222 }
223
224- auto request = mcl::make_protobuf_object<mir::protobuf::DisplayConfiguration>();
225+ mp::DisplayConfiguration request;
226 {
227 std::lock_guard<decltype(mutex)> lock(mutex);
228
229 for (auto i=0u; i < config->num_outputs; i++)
230 {
231 auto output = config->outputs[i];
232- auto display_request = request->add_display_output();
233+ auto display_request = request.add_display_output();
234 display_request->set_output_id(output.output_id);
235 display_request->set_used(output.used);
236 display_request->set_current_mode(output.current_mode);
237@@ -623,7 +625,7 @@
238 }
239
240 configure_display_wait_handle.expect_result();
241- server.configure_display(request.get(), display_configuration_response.get(),
242+ server.configure_display(&request, display_configuration_response.get(),
243 google::protobuf::NewCallback(this, &MirConnection::done_display_configure));
244
245 return &configure_display_wait_handle;
246@@ -648,8 +650,8 @@
247
248 StreamRelease stream_release{stream, new_wait_handle, callback, context};
249
250- auto buffer_stream_id = mcl::make_protobuf_object<mir::protobuf::BufferStreamId>();
251- buffer_stream_id->set_value(stream->rpc_id().as_value());
252+ mp::BufferStreamId buffer_stream_id;
253+ buffer_stream_id.set_value(stream->rpc_id().as_value());
254
255 {
256 std::lock_guard<decltype(release_wait_handle_guard)> rel_lock(release_wait_handle_guard);
257@@ -658,7 +660,7 @@
258
259 new_wait_handle->expect_result();
260 server.release_buffer_stream(
261- buffer_stream_id.get(), void_response.get(),
262+ &buffer_stream_id, void_response.get(),
263 google::protobuf::NewCallback(this, &MirConnection::released, stream_release));
264 return new_wait_handle;
265 }
266
267=== modified file 'src/client/mir_connection.h'
268--- src/client/mir_connection.h 2015-07-29 17:33:10 +0000
269+++ src/client/mir_connection.h 2015-07-29 17:33:10 +0000
270@@ -18,7 +18,6 @@
271 #ifndef MIR_CLIENT_MIR_CONNECTION_H_
272 #define MIR_CLIENT_MIR_CONNECTION_H_
273
274-#include "mir_protobuf.pb.h"
275 #include "mir_wait_handle.h"
276 #include "lifecycle_control.h"
277 #include "ping_handler.h"
278@@ -39,7 +38,14 @@
279 namespace mir
280 {
281 class SharedLibrary;
282-
283+namespace protobuf
284+{
285+class BufferStream;
286+class Connection;
287+class ConnectParameters;
288+class PlatformOperationMessage;
289+class DisplayConfiguration;
290+}
291 /// The client-side library implementation namespace
292 namespace client
293 {
294
295=== modified file 'src/client/mir_prompt_session.cpp'
296--- src/client/mir_prompt_session.cpp 2015-07-29 17:33:10 +0000
297+++ src/client/mir_prompt_session.cpp 2015-07-29 17:33:10 +0000
298@@ -20,6 +20,7 @@
299 #include "event_handler_register.h"
300 #include "make_protobuf_object.h"
301 #include "rpc/mir_display_server.h"
302+#include "mir_protobuf.pb.h"
303
304 namespace mp = mir::protobuf;
305 namespace mcl = mir::client;
306@@ -146,13 +147,13 @@
307 mir_client_fd_callback callback,
308 void * context)
309 {
310- auto request = mcl::make_protobuf_object<mir::protobuf::SocketFDRequest>();
311- request->set_number(no_of_fds);
312+ mp::SocketFDRequest request;
313+ request.set_number(no_of_fds);
314
315 fds_for_prompt_providers_wait_handle.expect_result();
316
317 server.new_fds_for_prompt_providers(
318- request.get(),
319+ &request,
320 socket_fd_response.get(),
321 google::protobuf::NewCallback(this, &MirPromptSession::done_fds_for_prompt_providers,
322 callback, context));
323
324=== modified file 'src/client/mir_prompt_session.h'
325--- src/client/mir_prompt_session.h 2015-07-29 17:33:10 +0000
326+++ src/client/mir_prompt_session.h 2015-07-29 17:33:10 +0000
327@@ -21,7 +21,6 @@
328
329 #include "mir_toolkit/mir_client_library.h"
330
331-#include "mir_protobuf.pb.h"
332 #include "mir_wait_handle.h"
333
334 #include <mutex>
335@@ -30,6 +29,12 @@
336
337 namespace mir
338 {
339+namespace protobuf
340+{
341+class PromptSessionParameters;
342+class SocketFD;
343+class Void;
344+}
345 /// The client-side library implementation namespace
346 namespace client
347 {
348
349=== modified file 'src/client/mir_screencast.cpp'
350--- src/client/mir_screencast.cpp 2015-07-29 17:33:10 +0000
351+++ src/client/mir_screencast.cpp 2015-07-29 17:33:10 +0000
352@@ -18,6 +18,7 @@
353
354 #include "mir_screencast.h"
355 #include "mir_connection.h"
356+#include "mir_protobuf.pb.h"
357 #include "make_protobuf_object.h"
358 #include "client_buffer_stream.h"
359 #include "mir/frontend/client_constants.h"
360@@ -53,19 +54,19 @@
361 }
362 protobuf_screencast->set_error("Not initialized");
363
364- auto parameters = mcl::make_protobuf_object<mir::protobuf::ScreencastParameters>();
365+ mp::ScreencastParameters parameters;
366
367- parameters->mutable_region()->set_left(region.top_left.x.as_int());
368- parameters->mutable_region()->set_top(region.top_left.y.as_int());
369- parameters->mutable_region()->set_width(region.size.width.as_uint32_t());
370- parameters->mutable_region()->set_height(region.size.height.as_uint32_t());
371- parameters->set_width(output_size.width.as_uint32_t());
372- parameters->set_height(output_size.height.as_uint32_t());
373- parameters->set_pixel_format(pixel_format);
374+ parameters.mutable_region()->set_left(region.top_left.x.as_int());
375+ parameters.mutable_region()->set_top(region.top_left.y.as_int());
376+ parameters.mutable_region()->set_width(region.size.width.as_uint32_t());
377+ parameters.mutable_region()->set_height(region.size.height.as_uint32_t());
378+ parameters.set_width(output_size.width.as_uint32_t());
379+ parameters.set_height(output_size.height.as_uint32_t());
380+ parameters.set_pixel_format(pixel_format);
381
382 create_screencast_wait_handle.expect_result();
383 server.create_screencast(
384- parameters.get(),
385+ &parameters,
386 protobuf_screencast.get(),
387 google::protobuf::NewCallback(
388 this, &MirScreencast::screencast_created,
389@@ -85,12 +86,12 @@
390 MirWaitHandle* MirScreencast::release(
391 mir_screencast_callback callback, void* context)
392 {
393- auto screencast_id = mcl::make_protobuf_object<mir::protobuf::ScreencastId>();
394- screencast_id->set_value(protobuf_screencast->screencast_id().value());
395+ mp::ScreencastId screencast_id;
396+ screencast_id.set_value(protobuf_screencast->screencast_id().value());
397
398 release_wait_handle.expect_result();
399 server.release_screencast(
400- screencast_id.get(),
401+ &screencast_id,
402 protobuf_void.get(),
403 google::protobuf::NewCallback(
404 this, &MirScreencast::released, callback, context));
405
406=== modified file 'src/client/mir_screencast.h'
407--- src/client/mir_screencast.h 2015-07-29 17:33:10 +0000
408+++ src/client/mir_screencast.h 2015-07-29 17:33:10 +0000
409@@ -21,7 +21,6 @@
410
411 #include "mir_wait_handle.h"
412 #include "mir_toolkit/client_types.h"
413-#include "mir_protobuf.pb.h"
414 #include "mir/geometry/size.h"
415 #include "mir/geometry/rectangle.h"
416
417@@ -31,6 +30,11 @@
418
419 namespace mir
420 {
421+namespace protobuf
422+{
423+class Screencast;
424+class Void;
425+}
426 namespace client
427 {
428 namespace rpc
429
430=== modified file 'src/client/mir_surface.cpp'
431--- src/client/mir_surface.cpp 2015-07-29 17:33:10 +0000
432+++ src/client/mir_surface.cpp 2015-07-29 17:33:10 +0000
433@@ -22,6 +22,7 @@
434 #include "client_buffer_stream.h"
435 #include "client_buffer_stream_factory.h"
436 #include "make_protobuf_object.h"
437+#include "mir_protobuf.pb.h"
438
439 #include "mir_toolkit/mir_client_library.h"
440 #include "mir/frontend/client_constants.h"
441@@ -43,14 +44,81 @@
442 namespace gp = google::protobuf;
443 namespace md = mir::dispatch;
444
445-#define SERIALIZE_OPTION_IF_SET(option, message) \
446- if (option.is_set()) \
447- message->set_##option(option.value());
448-
449 namespace
450 {
451 std::mutex handle_mutex;
452 std::unordered_set<MirSurface*> valid_surfaces;
453+
454+mir::protobuf::SurfaceParameters serialize_spec(MirSurfaceSpec const& spec)
455+{
456+ mp::SurfaceParameters message;
457+
458+#define SERIALIZE_OPTION_IF_SET(option) \
459+ if (spec.option.is_set()) \
460+ message.set_##option(spec.option.value());
461+
462+ SERIALIZE_OPTION_IF_SET(width);
463+ SERIALIZE_OPTION_IF_SET(height);
464+ SERIALIZE_OPTION_IF_SET(pixel_format);
465+ SERIALIZE_OPTION_IF_SET(buffer_usage);
466+ SERIALIZE_OPTION_IF_SET(surface_name);
467+ SERIALIZE_OPTION_IF_SET(output_id);
468+ SERIALIZE_OPTION_IF_SET(type);
469+ SERIALIZE_OPTION_IF_SET(state);
470+ SERIALIZE_OPTION_IF_SET(pref_orientation);
471+ SERIALIZE_OPTION_IF_SET(edge_attachment);
472+ SERIALIZE_OPTION_IF_SET(min_width);
473+ SERIALIZE_OPTION_IF_SET(min_height);
474+ SERIALIZE_OPTION_IF_SET(max_width);
475+ SERIALIZE_OPTION_IF_SET(max_height);
476+ SERIALIZE_OPTION_IF_SET(width_inc);
477+ SERIALIZE_OPTION_IF_SET(height_inc);
478+ // min_aspect is a special case (below)
479+ // max_aspect is a special case (below)
480+
481+ if (spec.parent.is_set() && spec.parent.value() != nullptr)
482+ message.set_parent_id(spec.parent.value()->id());
483+
484+ if (spec.parent_id)
485+ {
486+ auto id = message.mutable_parent_persistent_id();
487+ id->set_value(spec.parent_id->as_string());
488+ }
489+
490+ if (spec.aux_rect.is_set())
491+ {
492+ message.mutable_aux_rect()->set_left(spec.aux_rect.value().left);
493+ message.mutable_aux_rect()->set_top(spec.aux_rect.value().top);
494+ message.mutable_aux_rect()->set_width(spec.aux_rect.value().width);
495+ message.mutable_aux_rect()->set_height(spec.aux_rect.value().height);
496+ }
497+
498+ if (spec.min_aspect.is_set())
499+ {
500+ message.mutable_min_aspect()->set_width(spec.min_aspect.value().width);
501+ message.mutable_min_aspect()->set_height(spec.min_aspect.value().height);
502+ }
503+
504+ if (spec.max_aspect.is_set())
505+ {
506+ message.mutable_max_aspect()->set_width(spec.max_aspect.value().width);
507+ message.mutable_max_aspect()->set_height(spec.max_aspect.value().height);
508+ }
509+
510+ if (spec.input_shape.is_set())
511+ {
512+ for (auto const& rect : spec.input_shape.value())
513+ {
514+ auto const new_shape = message.add_input_shape();
515+ new_shape->set_left(rect.left);
516+ new_shape->set_top(rect.top);
517+ new_shape->set_width(rect.width);
518+ new_shape->set_height(rect.height);
519+ }
520+ }
521+
522+ return message;
523+}
524 }
525
526 MirSurfaceSpec::MirSurfaceSpec(
527@@ -78,77 +146,7 @@
528 }
529 }
530
531-MirSurfaceSpec::MirSurfaceSpec()
532-{
533-}
534-
535-std::unique_ptr<mir::protobuf::SurfaceParameters> MirSurfaceSpec::serialize() const
536-{
537- //std::unique_ptr<mp::SurfaceParameters> message{mp::SurfaceParameters::default_instance().New()};
538- auto message = mcl::make_protobuf_object<mp::SurfaceParameters>();
539-
540- SERIALIZE_OPTION_IF_SET(width, message);
541- SERIALIZE_OPTION_IF_SET(height, message);
542- SERIALIZE_OPTION_IF_SET(pixel_format, message);
543- SERIALIZE_OPTION_IF_SET(buffer_usage, message);
544- SERIALIZE_OPTION_IF_SET(surface_name, message);
545- SERIALIZE_OPTION_IF_SET(output_id, message);
546- SERIALIZE_OPTION_IF_SET(type, message);
547- SERIALIZE_OPTION_IF_SET(state, message);
548- SERIALIZE_OPTION_IF_SET(pref_orientation, message);
549- SERIALIZE_OPTION_IF_SET(edge_attachment, message);
550- SERIALIZE_OPTION_IF_SET(min_width, message);
551- SERIALIZE_OPTION_IF_SET(min_height, message);
552- SERIALIZE_OPTION_IF_SET(max_width, message);
553- SERIALIZE_OPTION_IF_SET(max_height, message);
554- SERIALIZE_OPTION_IF_SET(width_inc, message);
555- SERIALIZE_OPTION_IF_SET(height_inc, message);
556- // min_aspect is a special case (below)
557- // max_aspect is a special case (below)
558-
559- if (parent.is_set() && parent.value() != nullptr)
560- message->set_parent_id(parent.value()->id());
561-
562- if (parent_id)
563- {
564- auto id = message->mutable_parent_persistent_id();
565- id->set_value(parent_id->as_string());
566- }
567-
568- if (aux_rect.is_set())
569- {
570- message->mutable_aux_rect()->set_left(aux_rect.value().left);
571- message->mutable_aux_rect()->set_top(aux_rect.value().top);
572- message->mutable_aux_rect()->set_width(aux_rect.value().width);
573- message->mutable_aux_rect()->set_height(aux_rect.value().height);
574- }
575-
576- if (min_aspect.is_set())
577- {
578- message->mutable_min_aspect()->set_width(min_aspect.value().width);
579- message->mutable_min_aspect()->set_height(min_aspect.value().height);
580- }
581-
582- if (max_aspect.is_set())
583- {
584- message->mutable_max_aspect()->set_width(max_aspect.value().width);
585- message->mutable_max_aspect()->set_height(max_aspect.value().height);
586- }
587-
588- if (input_shape.is_set())
589- {
590- for (auto const& rect : input_shape.value())
591- {
592- auto const new_shape = message->add_input_shape();
593- new_shape->set_left(rect.left);
594- new_shape->set_top(rect.top);
595- new_shape->set_width(rect.width);
596- new_shape->set_height(rect.height);
597- }
598- }
599-
600- return message;
601-}
602+MirSurfaceSpec::MirSurfaceSpec() = default;
603
604 MirPersistentId::MirPersistentId(std::string const& string_id)
605 : string_id{string_id}
606@@ -193,11 +191,11 @@
607 for (int i = 0; i < mir_surface_attribs; i++)
608 attrib_cache[i] = -1;
609
610- auto const message = spec.serialize();
611+ auto const message = serialize_spec(spec);
612 create_wait_handle.expect_result();
613 try
614 {
615- server->create_surface(message.get(), surface.get(), gp::NewCallback(this, &MirSurface::created, callback, context));
616+ server->create_surface(&message, surface.get(), gp::NewCallback(this, &MirSurface::created, callback, context));
617 }
618 catch (std::exception const& ex)
619 {
620@@ -383,28 +381,28 @@
621
622 MirWaitHandle* MirSurface::configure_cursor(MirCursorConfiguration const* cursor)
623 {
624- auto setting = mcl::make_protobuf_object<mp::CursorSetting>();
625+ mp::CursorSetting setting;
626
627 {
628 std::unique_lock<decltype(mutex)> lock(mutex);
629- setting->mutable_surfaceid()->CopyFrom(surface->id());
630+ setting.mutable_surfaceid()->CopyFrom(surface->id());
631 if (cursor)
632 {
633 if (cursor->stream != nullptr)
634 {
635- setting->mutable_buffer_stream()->set_value(cursor->stream->rpc_id().as_value());
636- setting->set_hotspot_x(cursor->hotspot_x);
637- setting->set_hotspot_y(cursor->hotspot_y);
638+ setting.mutable_buffer_stream()->set_value(cursor->stream->rpc_id().as_value());
639+ setting.set_hotspot_x(cursor->hotspot_x);
640+ setting.set_hotspot_y(cursor->hotspot_y);
641 }
642 else if (cursor->name != mir_disabled_cursor_name)
643 {
644- setting->set_name(cursor->name.c_str());
645+ setting.set_name(cursor->name.c_str());
646 }
647 }
648 }
649
650 configure_cursor_wait_handle.expect_result();
651- server->configure_cursor(setting.get(), void_response.get(),
652+ server->configure_cursor(&setting, void_response.get(),
653 google::protobuf::NewCallback(this, &MirSurface::on_cursor_configured));
654
655 return &configure_cursor_wait_handle;
656@@ -424,14 +422,14 @@
657
658 std::unique_lock<decltype(mutex)> lock(mutex);
659
660- auto setting = mcl::make_protobuf_object<mp::SurfaceSetting>();
661- setting->mutable_surfaceid()->CopyFrom(surface->id());
662- setting->set_attrib(at);
663- setting->set_ivalue(value);
664+ mp::SurfaceSetting setting;
665+ setting.mutable_surfaceid()->CopyFrom(surface->id());
666+ setting.set_attrib(at);
667+ setting.set_ivalue(value);
668 lock.unlock();
669
670 configure_wait_handle.expect_result();
671- server->configure_surface(setting.get(), configure_result.get(),
672+ server->configure_surface(&setting, configure_result.get(),
673 google::protobuf::NewCallback(this, &MirSurface::on_configured));
674
675 return &configure_wait_handle;
676@@ -453,12 +451,12 @@
677 return false;
678 }
679
680- auto request = mcl::make_protobuf_object<mp::CoordinateTranslationRequest>();
681+ mp::CoordinateTranslationRequest request;
682
683- request->set_x(x);
684- request->set_y(y);
685- *request->mutable_surfaceid() = surface->id();
686- auto response = mcl::make_protobuf_object<mp::CoordinateTranslationResponse>();
687+ request.set_x(x);
688+ request.set_y(y);
689+ *request.mutable_surfaceid() = surface->id();
690+ mp::CoordinateTranslationResponse response;
691
692 MirWaitHandle signal;
693 signal.expect_result();
694@@ -467,16 +465,16 @@
695 std::lock_guard<decltype(mutex)> lock(mutex);
696
697 debug->translate_surface_to_screen(
698- request.get(),
699- response.get(),
700+ &request,
701+ &response,
702 google::protobuf::NewCallback(&signal_response_received, &signal));
703 }
704
705 signal.wait_for_one();
706
707- *screen_x = response->x();
708- *screen_y = response->y();
709- return !response->has_error();
710+ *screen_x = response.x();
711+ *screen_y = response.y();
712+ return !response.has_error();
713 }
714
715 void MirSurface::on_configured()
716@@ -628,14 +626,14 @@
717
718 MirWaitHandle* MirSurface::modify(MirSurfaceSpec const& spec)
719 {
720- auto mods = mcl::make_protobuf_object<mp::SurfaceModifications>();
721+ mp::SurfaceModifications mods;
722
723 {
724 std::unique_lock<decltype(mutex)> lock(mutex);
725- mods->mutable_surface_id()->set_value(surface->id().value());
726+ mods.mutable_surface_id()->set_value(surface->id().value());
727 }
728
729- auto const surface_specification = mods->mutable_surface_specification();
730+ auto const surface_specification = mods.mutable_surface_specification();
731
732 #define COPY_IF_SET(field)\
733 if (spec.field.is_set())\
734@@ -727,7 +725,7 @@
735 }
736
737 modify_wait_handle.expect_result();
738- server->modify_surface(mods.get(), modify_result.get(),
739+ server->modify_surface(&mods, modify_result.get(),
740 google::protobuf::NewCallback(this, &MirSurface::on_modified));
741
742 return &modify_wait_handle;
743
744=== modified file 'src/client/mir_surface.h'
745--- src/client/mir_surface.h 2015-07-29 17:33:10 +0000
746+++ src/client/mir_surface.h 2015-07-29 17:33:10 +0000
747@@ -18,7 +18,6 @@
748 #ifndef MIR_CLIENT_MIR_SURFACE_H_
749 #define MIR_CLIENT_MIR_SURFACE_H_
750
751-#include "mir_protobuf.pb.h"
752 #include "client_buffer_depository.h"
753 #include "client_buffer_stream.h"
754 #include "mir_wait_handle.h"
755@@ -51,6 +50,14 @@
756 class XKBMapper;
757 }
758 }
759+namespace protobuf
760+{
761+class PersistentSurfaceId;
762+class Surface;
763+class SurfaceParameters;
764+class SurfaceSetting;
765+class Void;
766+}
767 namespace client
768 {
769 namespace rpc
770@@ -73,8 +80,6 @@
771 MirSurfaceSpec(MirConnection* connection, int width, int height, MirPixelFormat format);
772 MirSurfaceSpec(MirConnection* connection, MirSurfaceParameters const& params);
773
774- std::unique_ptr<mir::protobuf::SurfaceParameters> serialize() const;
775-
776 struct AspectRatio { unsigned width; unsigned height; };
777
778 // Required parameters
779
780=== modified file 'src/client/rpc/mir_protobuf_rpc_channel.cpp'
781--- src/client/rpc/mir_protobuf_rpc_channel.cpp 2015-07-29 17:33:10 +0000
782+++ src/client/rpc/mir_protobuf_rpc_channel.cpp 2015-07-29 17:33:10 +0000
783@@ -40,6 +40,7 @@
784 namespace mcl = mir::client;
785 namespace mclr = mir::client::rpc;
786 namespace md = mir::dispatch;
787+namespace mp = mir::protobuf;
788
789 namespace
790 {
791@@ -241,48 +242,48 @@
792
793 void mclr::MirProtobufRpcChannel::process_event_sequence(std::string const& event)
794 {
795- auto seq = mcl::make_protobuf_object<mir::protobuf::EventSequence>();
796-
797- seq->ParseFromString(event);
798-
799- if (seq->has_display_configuration())
800- {
801- display_configuration->update_configuration(seq->display_configuration());
802- }
803-
804- if (seq->has_lifecycle_event())
805- {
806- (*lifecycle_control)(static_cast<MirLifecycleState>(seq->lifecycle_event().new_state()));
807- }
808-
809- if (seq->has_ping_event())
810- {
811- (*ping_handler)(seq->ping_event().serial());
812- }
813-
814- if (seq->has_buffer_request())
815+ mp::EventSequence seq;
816+
817+ seq.ParseFromString(event);
818+
819+ if (seq.has_display_configuration())
820+ {
821+ display_configuration->update_configuration(seq.display_configuration());
822+ }
823+
824+ if (seq.has_lifecycle_event())
825+ {
826+ (*lifecycle_control)(static_cast<MirLifecycleState>(seq.lifecycle_event().new_state()));
827+ }
828+
829+ if (seq.has_ping_event())
830+ {
831+ (*ping_handler)(seq.ping_event().serial());
832+ }
833+
834+ if (seq.has_buffer_request())
835 {
836 std::array<char, 1> dummy;
837- auto const num_fds = seq->mutable_buffer_request()->mutable_buffer()->fds_on_side_channel();
838+ auto const num_fds = seq.mutable_buffer_request()->mutable_buffer()->fds_on_side_channel();
839 std::vector<mir::Fd> fds(num_fds);
840 if (num_fds > 0)
841 {
842 transport->receive_data(dummy.data(), dummy.size(), fds);
843- seq->mutable_buffer_request()->mutable_buffer()->clear_fd();
844+ seq.mutable_buffer_request()->mutable_buffer()->clear_fd();
845 for(auto& fd : fds)
846- seq->mutable_buffer_request()->mutable_buffer()->add_fd(fd);
847+ seq.mutable_buffer_request()->mutable_buffer()->add_fd(fd);
848 }
849
850- surface_map->with_stream_do(mf::BufferStreamId(seq->buffer_request().id().value()),
851+ surface_map->with_stream_do(mf::BufferStreamId(seq.buffer_request().id().value()),
852 [&] (mcl::ClientBufferStream* stream) {
853- stream->buffer_available(seq->buffer_request().buffer());
854+ stream->buffer_available(seq.buffer_request().buffer());
855 });
856 }
857
858- int const nevents = seq->event_size();
859+ int const nevents = seq.event_size();
860 for (int i = 0; i != nevents; ++i)
861 {
862- mir::protobuf::Event const& event = seq->event(i);
863+ mp::Event const& event = seq.event(i);
864 if (event.has_raw())
865 {
866 std::string const& raw_event = event.raw();
867@@ -349,7 +350,7 @@
868 */
869 std::lock_guard<decltype(read_mutex)> lock(read_mutex);
870
871- auto result = mcl::make_protobuf_object<mir::protobuf::wire::Result>();
872+ auto result = mcl::make_protobuf_object<mp::wire::Result>();
873 try
874 {
875 uint16_t message_size;
876@@ -393,7 +394,7 @@
877 {
878 // It's too difficult to convince C++ to move this lambda everywhere, so
879 // just give up and let it pretend its a shared_ptr.
880- std::shared_ptr<mir::protobuf::wire::Result> appeaser{std::move(result)};
881+ std::shared_ptr<mp::wire::Result> appeaser{std::move(result)};
882 delayed_processor->enqueue([delayed_result = std::move(appeaser), this]() mutable
883 {
884 pending_calls.complete_response(*delayed_result);
885
886=== modified file 'src/client/rpc/mir_protobuf_rpc_channel.h'
887--- src/client/rpc/mir_protobuf_rpc_channel.h 2015-07-29 17:33:10 +0000
888+++ src/client/rpc/mir_protobuf_rpc_channel.h 2015-07-29 17:33:10 +0000
889@@ -28,8 +28,6 @@
890 #include "../lifecycle_control.h"
891 #include "../ping_handler.h"
892
893-#include "mir_protobuf_wire.pb.h"
894-
895 #include <thread>
896 #include <atomic>
897 #include <experimental/optional>

Subscribers

People subscribed via source and target branches