Mir

Merge lp:~kdub/mir/multistream-protobuf-additions into lp:mir

  1. multistream-protobuf-additions
  2. Merge into development-branch
Proposed by Kevin DuBois on 2015-05-22
Status: Merged
Approved by: Kevin DuBois on 2015-06-03
Approved revision: 2605
Merged at revision: 2617
Proposed branch: lp:~kdub/mir/multistream-protobuf-additions
Merge into: lp:mir
Diff against target: 144 lines (+76/-4)
4 files modified
include/server/mir/shell/surface_specification.h (+13/-4)
src/protobuf/mir_protobuf.proto (+6/-0)
src/server/frontend/session_mediator.cpp (+9/-0)
tests/unit-tests/frontend/test_session_mediator.cpp (+48/-0)
To merge this branch: bzr merge lp:~kdub/mir/multistream-protobuf-additions
Reviewer Review Type Date Requested Status
PS Jenkins bot continuous-integration Approve on 2015-06-03
Alan Griffiths Approve on 2015-06-02
Daniel van Vugt Abstain on 2015-06-02
Chris Halse Rogers Approve on 2015-06-01
Robert Carr (community) 2015-05-22 Approve on 2015-05-27
Review via email: mp+259970@code.launchpad.net

Commit Message

Add the small unit test that drives the protobuf additions for associating multiple buffer streams together. The request is sent over the 'modify_surface' rpc call.

Description of the Change

Add the small unit test that drives the protobuf additions for associating multiple buffer streams together. The request is sent over the 'modify_surface' rpc call.

To post a comment you must log in.
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:2601
http://jenkins.qa.ubuntu.com/job/mir-ci/3898/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/2576
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-wily-amd64-build/87/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/2524
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/54
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/54/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2524
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2524/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/5393
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/20743

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

review: Needs Fixing (continuous-integration)
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:2602
http://jenkins.qa.ubuntu.com/job/mir-ci/3901/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/2579
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-wily-amd64-build/90
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/2527
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/57
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/57/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2527
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2527/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/5396
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/20753

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

review: Approve (continuous-integration)
Robert Carr (robertcarr) wrote :

LGTM

review: Approve
Daniel van Vugt (vanvugt) wrote :

Difficult to maintain and unacceptably inflexible right now. More explanation and a better suggestion in: https://code.launchpad.net/~kdub/mir/add-streams-to-surface/+merge/259776

Daniel van Vugt (vanvugt) :
review: Disapprove
Daniel van Vugt (vanvugt) wrote :

Same needs fixing as the other branch.

review: Needs Fixing
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:2603
http://jenkins.qa.ubuntu.com/job/mir-ci/3945/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/2629
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-wily-amd64-build/140
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/2577
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/101
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/101/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2577
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2577/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/5428
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/20840

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

review: Approve (continuous-integration)
Chris Halse Rogers (raof) wrote :

Looks sensible to me.

review: Approve
Alan Griffiths (alan-griffiths) wrote :

Nit:

113 +TEST_F(SessionMediator, arrangement_of_bufferstreams)

The test name doesn't specify what feature is being tested.

review: Needs Fixing
Daniel van Vugt (vanvugt) wrote :

Still needs similar consideration as in:
https://code.launchpad.net/~kdub/mir/add-streams-to-surface/+merge/259776

If "displacement" exists then it should probably exist with some stream "type" enum. Because some shells will ignore some stream types and some stream types don't have a relative placement (e.g. the app icon).

Daniel van Vugt (vanvugt) wrote :

Apparently now, per the other proposal.

review: Abstain
Daniel van Vugt (vanvugt) wrote :

Apparently not.

Alan Griffiths (alan-griffiths) wrote :

OK

review: Approve
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/mir-autolanding/1491/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/2664
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-wily-amd64-build/175
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/2612/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-autolanding/46
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-autolanding/46/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2612
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2612/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/5461/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/20897

review: Needs Fixing (continuous-integration)
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:2605
http://jenkins.qa.ubuntu.com/job/mir-ci/3974/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/2666
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-wily-amd64-build/177
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/2614/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/130
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/130/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2614
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2614/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/5463/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/20902

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

review: Needs Fixing (continuous-integration)
Kevin DuBois (kdub) wrote :

failure seems to be the mircommon abi number issue

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 'include/server/mir/shell/surface_specification.h'
2--- include/server/mir/shell/surface_specification.h 2015-05-19 21:34:34 +0000
3+++ include/server/mir/shell/surface_specification.h 2015-06-02 14:02:03 +0000
4@@ -22,7 +22,9 @@
5 #include "mir/optional_value.h"
6 #include "mir_toolkit/common.h"
7 #include "mir/frontend/surface_id.h"
8+#include "mir/frontend/buffer_stream_id.h"
9 #include "mir/geometry/point.h"
10+#include "mir/geometry/displacement.h"
11 #include "mir/graphics/buffer_properties.h"
12 #include "mir/graphics/display_configuration.h"
13 #include "mir/scene/depth_id.h"
14@@ -35,6 +37,12 @@
15 {
16 struct SurfaceAspectRatio { unsigned width; unsigned height; };
17
18+struct StreamSpecification
19+{
20+ frontend::BufferStreamId stream_id;
21+ geometry::Displacement displacement;
22+};
23+
24 /// Specification of surface properties requested by client
25 struct SurfaceSpecification
26 {
27@@ -54,10 +62,11 @@
28 optional_value<geometry::Height> min_height;
29 optional_value<geometry::Width> max_width;
30 optional_value<geometry::Height> max_height;
31- mir::optional_value<geometry::DeltaX> width_inc;
32- mir::optional_value<geometry::DeltaY> height_inc;
33- mir::optional_value<SurfaceAspectRatio> min_aspect;
34- mir::optional_value<SurfaceAspectRatio> max_aspect;
35+ optional_value<geometry::DeltaX> width_inc;
36+ optional_value<geometry::DeltaY> height_inc;
37+ optional_value<SurfaceAspectRatio> min_aspect;
38+ optional_value<SurfaceAspectRatio> max_aspect;
39+ optional_value<std::vector<StreamSpecification>> streams;
40
41 // TODO scene::SurfaceCreationParameters overlaps this content but has additional fields:
42 // geometry::Point top_left;
43
44=== modified file 'src/protobuf/mir_protobuf.proto'
45--- src/protobuf/mir_protobuf.proto 2015-05-19 21:34:34 +0000
46+++ src/protobuf/mir_protobuf.proto 2015-06-02 14:02:03 +0000
47@@ -67,8 +67,14 @@
48 optional int32 height_inc = 18;
49 optional SurfaceAspectRatio min_aspect = 19;
50 optional SurfaceAspectRatio max_aspect = 20;
51+ repeated StreamConfiguration stream = 21;
52 }
53
54+message StreamConfiguration {
55+ optional BufferStreamId id = 1;
56+ optional int32 displacement_x = 2;
57+ optional int32 displacement_y = 3;
58+};
59
60 message SurfaceModifications {
61 required SurfaceId surface_id = 1;
62
63=== modified file 'src/server/frontend/session_mediator.cpp'
64--- src/server/frontend/session_mediator.cpp 2015-05-19 21:34:34 +0000
65+++ src/server/frontend/session_mediator.cpp 2015-06-02 14:02:03 +0000
66@@ -442,6 +442,15 @@
67 // max_aspect is a special case (below)
68
69 #undef COPY_IF_SET
70+ std::vector<msh::StreamSpecification> stream_spec;
71+ for(auto& stream : surface_specification.stream())
72+ {
73+ stream_spec.emplace_back(
74+ msh::StreamSpecification{
75+ mf::BufferStreamId{stream.id().value()},
76+ geom::Displacement{stream.displacement_x(), stream.displacement_y()}});
77+ }
78+ mods.streams = std::move(stream_spec);
79
80 if (surface_specification.has_aux_rect())
81 {
82
83=== modified file 'tests/unit-tests/frontend/test_session_mediator.cpp'
84--- tests/unit-tests/frontend/test_session_mediator.cpp 2015-05-19 21:34:34 +0000
85+++ tests/unit-tests/frontend/test_session_mediator.cpp 2015-06-02 14:02:03 +0000
86@@ -49,6 +49,7 @@
87 #include "mir_test/fake_shared.h"
88 #include "mir/frontend/connector.h"
89 #include "mir/frontend/event_sink.h"
90+#include "mir_protobuf.pb.h"
91
92 #include "gmock_set_arg.h"
93 #include <boost/exception/errinfo_errno.hpp>
94@@ -944,3 +945,50 @@
95 // call), but from the same thread that initiated the exchange_buffer operation
96 completion_func(&stub_buffer2);
97 }
98+
99+MATCHER(ConfigEq, "stream configurations are equivalent")
100+{
101+ return (std::get<0>(arg).stream_id == std::get<1>(arg).stream_id) &&
102+ (std::get<0>(arg).displacement == std::get<1>(arg).displacement);
103+}
104+
105+MATCHER_P(StreamsAre, value, "configuration streams match")
106+{
107+ if(!arg.streams.is_set())
108+ return false;
109+ EXPECT_THAT(arg.streams.value(), testing::Pointwise(ConfigEq(), value));
110+ return !(::testing::Test::HasFailure());
111+}
112+
113+TEST_F(SessionMediator, arranges_bufferstreams_via_shell)
114+{
115+ using namespace testing;
116+ mp::Void null;
117+ mp::SurfaceModifications mods;
118+ mp::BufferStreamParameters stream_request;
119+ std::array<mp::BufferStream,2> streams;
120+ std::array<geom::Displacement,2> displacement = { {
121+ geom::Displacement{-12,11}, geom::Displacement{4,-3} } };
122+
123+ mediator.connect(nullptr, &connect_parameters, &connection, null_callback.get());
124+ mediator.create_surface(nullptr, &surface_parameters, &surface_response, null_callback.get());
125+ for (auto &stream : streams)
126+ mediator.create_buffer_stream(nullptr, &stream_request, &stream, null_callback.get());
127+ mods.mutable_surface_id()->set_value(surface_response.id().value());
128+ for (auto i = 0u; i < streams.size(); i++)
129+ {
130+ auto stream = mods.mutable_surface_specification()->add_stream();
131+ stream->mutable_id()->set_value(streams[i].id().value());
132+ stream->set_displacement_x(displacement[i].dx.as_int());
133+ stream->set_displacement_y(displacement[i].dy.as_int());
134+ }
135+
136+ EXPECT_CALL(*shell, modify_surface(_,
137+ mf::SurfaceId{surface_response.id().value()},
138+ StreamsAre(std::vector<msh::StreamSpecification>{
139+ {mf::BufferStreamId(streams[0].id().value()), displacement[0]},
140+ {mf::BufferStreamId(streams[1].id().value()), displacement[1]}
141+ })));
142+
143+ mediator.modify_surface(nullptr, &mods, &null, null_callback.get());
144+}

Subscribers

People subscribed via source and target branches