Mir

Merge lp:~kdub/mir/fix-1503317 into lp:mir

  1. fix-1503317
  2. Merge into development-branch
Proposed by Kevin DuBois
Status: Merged
Approved by: Chris Halse Rogers
Approved revision: no longer in the source branch.
Merged at revision: 3009
Proposed branch: lp:~kdub/mir/fix-1503317
Merge into: lp:mir
Diff against target: 260 lines (+87/-18)
7 files modified
include/test/mir_test_framework/connected_client_with_a_surface.h (+3/-0)
src/server/scene/basic_surface.cpp (+1/-1)
tests/acceptance-tests/test_buffer_stream_arrangement.cpp (+47/-15)
tests/include/mir/test/doubles/mock_buffer_bundle.h (+3/-1)
tests/include/mir/test/doubles/mock_buffer_stream.h (+2/-0)
tests/mir_test_framework/connected_client_with_a_surface.cpp (+1/-1)
tests/unit-tests/scene/test_basic_surface.cpp (+30/-0)
To merge this branch: bzr merge lp:~kdub/mir/fix-1503317
Reviewer Review Type Date Requested Status
Chris Halse Rogers Approve
PS Jenkins bot (community) continuous-integration Approve
Alan Griffiths Approve
Review via email: mp+273694@code.launchpad.net

Commit message

In multibuffer stream scenarios, ensure that the size of the renderable generated from the stream is the size of the stream.

fixes lp: #1503317

Description of the change

In multibuffer stream scenarios, ensure that the size of the renderable generated from the stream is the size of the stream.

fixes lp: #1503317

problem came to my attention when working on switching the demo servers from the 'titlebars' being a surface to being a stream.

To post a comment you must log in.
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

Looks reasonable.

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

PASSED: Continuous integration, rev:3004
http://jenkins.qa.ubuntu.com/job/mir-ci/5195/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/4346
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/3253
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/4288
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-wily-touch/246/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/1349
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/1349/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-wily-i386-ci/246/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/4289
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/4289/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-touch/6948
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/24037
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-wily-armhf/247/console

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

review: Approve (continuous-integration)
Revision history for this message
Chris Halse Rogers (raof) wrote :

+ std::vector<std::vector<std::tuple<geom::Displacement, geom::Size>>> displacements;

This (in it's many positions) is no longer really a list of displacements. You're renamed it “positions” in one location, and that seems reasonable.

Also, tuple<Displacement, Size> is pretty much a geom::Rectangle. It looks like you could just drop that in (although if you wanted to type-encode you'd need a RelativeRectangle).

Otherwise looks good.

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

PASSED: Continuous integration, rev:3006
http://jenkins.qa.ubuntu.com/job/mir-ci/5209/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/4364
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/3271
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/4306
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-wily-touch/264/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/1363
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/1363/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-wily-i386-ci/264/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/4307
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/4307/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-touch/6961
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/24066
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-wily-armhf/265/console

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

review: Approve (continuous-integration)
Revision history for this message
Chris Halse Rogers (raof) wrote :

LGTM

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'include/test/mir_test_framework/connected_client_with_a_surface.h'
--- include/test/mir_test_framework/connected_client_with_a_surface.h 2015-06-22 03:04:56 +0000
+++ include/test/mir_test_framework/connected_client_with_a_surface.h 2015-10-08 12:19:30 +0000
@@ -20,6 +20,7 @@
20#define MIR_TEST_FRAMEWORK_CONNECTED_CLIENT_WITH_A_SURFACE_H_20#define MIR_TEST_FRAMEWORK_CONNECTED_CLIENT_WITH_A_SURFACE_H_
2121
22#include "mir_test_framework/connected_client_headless_server.h"22#include "mir_test_framework/connected_client_headless_server.h"
23#include "mir/geometry/size.h"
2324
24namespace mir_test_framework25namespace mir_test_framework
25{26{
@@ -30,6 +31,8 @@
30 void SetUp() override;31 void SetUp() override;
3132
32 void TearDown() override;33 void TearDown() override;
34
35 mir::geometry::Size const surface_size {640, 480};
33};36};
34}37}
3538
3639
=== modified file 'src/server/scene/basic_surface.cpp'
--- src/server/scene/basic_surface.cpp 2015-10-02 01:56:04 +0000
+++ src/server/scene/basic_surface.cpp 2015-10-08 12:19:30 +0000
@@ -877,7 +877,7 @@
877 {877 {
878 list.emplace_back(std::make_shared<SurfaceSnapshot>(878 list.emplace_back(std::make_shared<SurfaceSnapshot>(
879 info.stream, id,879 info.stream, id,
880 geom::Rectangle{surface_rect.top_left + info.displacement, surface_rect.size},880 geom::Rectangle{surface_rect.top_left + info.displacement, info.stream->stream_size()},
881 transformation_matrix, surface_alpha, nonrectangular, info.stream.get()));881 transformation_matrix, surface_alpha, nonrectangular, info.stream.get()));
882 }882 }
883 }883 }
884884
=== modified file 'tests/acceptance-tests/test_buffer_stream_arrangement.cpp'
--- tests/acceptance-tests/test_buffer_stream_arrangement.cpp 2015-06-22 03:04:56 +0000
+++ tests/acceptance-tests/test_buffer_stream_arrangement.cpp 2015-10-08 12:19:30 +0000
@@ -38,6 +38,23 @@
38namespace38namespace
39{39{
4040
41struct RelativeRectangle
42{
43 RelativeRectangle() = default;
44
45 RelativeRectangle(geom::Displacement const& displacement, geom::Size const& size)
46 : displacement{displacement}, size{size}
47 {
48 }
49
50 geom::Displacement displacement;
51 geom::Size size;
52};
53bool operator==(RelativeRectangle const& a, RelativeRectangle const& b)
54{
55 return (a.displacement == b.displacement) && (a.size == b.size);
56}
57
41MirPixelFormat an_available_format(MirConnection* connection)58MirPixelFormat an_available_format(MirConnection* connection)
42{59{
43 using namespace testing;60 using namespace testing;
@@ -49,9 +66,10 @@
4966
50struct Stream67struct Stream
51{68{
52 Stream(MirBufferStream* stream, geom::Point pt) :69 Stream(MirBufferStream* stream, geom::Point pt, geom::Size sz) :
53 stream(stream),70 stream(stream),
54 pos{pt},71 pos{pt},
72 stream_size{sz},
55 needs_release{false}73 needs_release{false}
56 {74 {
57 mir_buffer_stream_swap_buffers_sync(stream);75 mir_buffer_stream_swap_buffers_sync(stream);
@@ -65,6 +83,7 @@
65 an_available_format(connection),83 an_available_format(connection),
66 mir_buffer_usage_hardware)),84 mir_buffer_usage_hardware)),
67 pos{rect.top_left},85 pos{rect.top_left},
86 stream_size{rect.size},
68 needs_release{true}87 needs_release{true}
69 {88 {
70 mir_buffer_stream_swap_buffers_sync(stream);89 mir_buffer_stream_swap_buffers_sync(stream);
@@ -86,11 +105,17 @@
86 return pos;105 return pos;
87 }106 }
88107
108 geom::Size size()
109 {
110 return stream_size;
111 }
112
89 Stream(Stream const&) = delete;113 Stream(Stream const&) = delete;
90 Stream& operator=(Stream const&) = delete;114 Stream& operator=(Stream const&) = delete;
91private:115private:
92 MirBufferStream* stream;116 MirBufferStream* stream;
93 geom::Point const pos;117 geom::Point const pos;
118 geom::Size stream_size;
94 bool const needs_release;119 bool const needs_release;
95};120};
96121
@@ -102,24 +127,26 @@
102 return;127 return;
103128
104 std::unique_lock<decltype(mutex)> lk(mutex);129 std::unique_lock<decltype(mutex)> lk(mutex);
105 std::vector<geom::Displacement> displacement;130 std::vector<RelativeRectangle> position;
106 auto first_position = (*sequence.begin())->renderable()->screen_position().top_left;131 auto first_position = (*sequence.begin())->renderable()->screen_position().top_left;
107 for (auto const& element : sequence)132 for (auto const& element : sequence)
108 displacement.emplace_back(element->renderable()->screen_position().top_left - first_position);133 position.emplace_back(
109 displacements.push_back(displacement);134 element->renderable()->screen_position().top_left - first_position,
135 element->renderable()->screen_position().size);
136 positions.push_back(position);
110 cv.notify_all();137 cv.notify_all();
111 }138 }
112139
113 template<typename T, typename S>140 template<typename T, typename S>
114 bool wait_for_ordering_within(141 bool wait_for_positions_within(
115 std::vector<geom::Displacement> const& ordering,142 std::vector<RelativeRectangle> const& awaited_positions,
116 std::chrono::duration<T,S> duration)143 std::chrono::duration<T,S> duration)
117 {144 {
118 std::unique_lock<decltype(mutex)> lk(mutex);145 std::unique_lock<decltype(mutex)> lk(mutex);
119 return cv.wait_for(lk, duration, [this, ordering] {146 return cv.wait_for(lk, duration, [this, awaited_positions] {
120 for (auto& displacement : displacements)147 for (auto& position : positions)
121 if (displacement == ordering) return true;148 if (position == awaited_positions) return true;
122 displacements.clear();149 positions.clear();
123 return false;150 return false;
124 });151 });
125 }152 }
@@ -127,7 +154,7 @@
127private:154private:
128 std::mutex mutex;155 std::mutex mutex;
129 std::condition_variable cv;156 std::condition_variable cv;
130 std::vector<std::vector<geom::Displacement>> displacements;157 std::vector<std::vector<RelativeRectangle>> positions;
131};158};
132159
133struct OrderTrackingDBC : mc::DisplayBufferCompositor160struct OrderTrackingDBC : mc::DisplayBufferCompositor
@@ -186,7 +213,7 @@
186 server.the_cursor()->hide();213 server.the_cursor()->hide();
187214
188 streams.emplace_back(215 streams.emplace_back(
189 std::make_unique<Stream>(mir_surface_get_buffer_stream(surface), geom::Point{0,0}));216 std::make_unique<Stream>(mir_surface_get_buffer_stream(surface), geom::Point{0,0}, surface_size));
190 int const additional_streams{3};217 int const additional_streams{3};
191 for (auto i = 0; i < additional_streams; i++)218 for (auto i = 0; i < additional_streams; i++)
192 {219 {
@@ -226,12 +253,17 @@
226 mir_surface_apply_spec(surface, change_spec);253 mir_surface_apply_spec(surface, change_spec);
227 mir_surface_spec_release(change_spec);254 mir_surface_spec_release(change_spec);
228255
229 std::vector<geom::Displacement> displacements;256 std::vector<RelativeRectangle> positions;
257 i = 0;
230 for (auto& info : infos)258 for (auto& info : infos)
231 displacements.emplace_back(geom::Displacement{info.displacement_x, info.displacement_y});259 {
260 positions.emplace_back(
261 geom::Displacement{info.displacement_x, info.displacement_y},
262 streams[i++]->size());
263 }
232264
233 //check that the compositor rendered correctly265 //check that the compositor rendered correctly
234 using namespace std::literals::chrono_literals;266 using namespace std::literals::chrono_literals;
235 EXPECT_TRUE(ordering->wait_for_ordering_within(displacements, 5s))267 EXPECT_TRUE(ordering->wait_for_positions_within(positions, 5s))
236 << "timed out waiting to see the compositor post the streams in the right arrangement";268 << "timed out waiting to see the compositor post the streams in the right arrangement";
237}269}
238270
=== modified file 'tests/include/mir/test/doubles/mock_buffer_bundle.h'
--- tests/include/mir/test/doubles/mock_buffer_bundle.h 2015-09-14 16:46:44 +0000
+++ tests/include/mir/test/doubles/mock_buffer_bundle.h 2015-10-08 12:19:30 +0000
@@ -37,7 +37,9 @@
37 ON_CALL(*this, buffers_ready_for_compositor(testing::_))37 ON_CALL(*this, buffers_ready_for_compositor(testing::_))
38 .WillByDefault(testing::Return(1));38 .WillByDefault(testing::Return(1));
39 ON_CALL(*this, properties())39 ON_CALL(*this, properties())
40 .WillByDefault(testing::Return(graphics::BufferProperties()));40 .WillByDefault(testing::Return(
41 graphics::BufferProperties{geometry::Size{1,1},
42 mir_pixel_format_abgr_8888, graphics::BufferUsage::hardware}));
41 }43 }
4244
43 MOCK_METHOD1(client_acquire, void(std::function<void(graphics::Buffer*)>));45 MOCK_METHOD1(client_acquire, void(std::function<void(graphics::Buffer*)>));
4446
=== modified file 'tests/include/mir/test/doubles/mock_buffer_stream.h'
--- tests/include/mir/test/doubles/mock_buffer_stream.h 2015-09-14 14:45:36 +0000
+++ tests/include/mir/test/doubles/mock_buffer_stream.h 2015-10-08 12:19:30 +0000
@@ -53,6 +53,8 @@
53 .WillByDefault(testing::Return(true));53 .WillByDefault(testing::Return(true));
54 ON_CALL(*this, pixel_format())54 ON_CALL(*this, pixel_format())
55 .WillByDefault(testing::Return(mir_pixel_format_abgr_8888));55 .WillByDefault(testing::Return(mir_pixel_format_abgr_8888));
56 ON_CALL(*this, stream_size())
57 .WillByDefault(testing::Return(geometry::Size{0,0}));
56 }58 }
57 MOCK_METHOD1(acquire_client_buffer, void(std::function<void(graphics::Buffer* buffer)>));59 MOCK_METHOD1(acquire_client_buffer, void(std::function<void(graphics::Buffer* buffer)>));
58 MOCK_METHOD1(release_client_buffer, void(graphics::Buffer*));60 MOCK_METHOD1(release_client_buffer, void(graphics::Buffer*));
5961
=== modified file 'tests/mir_test_framework/connected_client_with_a_surface.cpp'
--- tests/mir_test_framework/connected_client_with_a_surface.cpp 2015-06-17 05:20:42 +0000
+++ tests/mir_test_framework/connected_client_with_a_surface.cpp 2015-10-08 12:19:30 +0000
@@ -25,7 +25,7 @@
25 ConnectedClientHeadlessServer::SetUp();25 ConnectedClientHeadlessServer::SetUp();
2626
27 auto const spec = mir_connection_create_spec_for_normal_surface(27 auto const spec = mir_connection_create_spec_for_normal_surface(
28 connection, 640, 480, mir_pixel_format_abgr_8888);28 connection, surface_size.width.as_int(), surface_size.height.as_int(), mir_pixel_format_abgr_8888);
29 mir_surface_spec_set_name(spec, "ConnectedClientWithASurfaceFixtureSurface");29 mir_surface_spec_set_name(spec, "ConnectedClientWithASurfaceFixtureSurface");
30 mir_surface_spec_set_buffer_usage(spec, mir_buffer_usage_hardware);30 mir_surface_spec_set_buffer_usage(spec, mir_buffer_usage_hardware);
3131
3232
=== modified file 'tests/unit-tests/scene/test_basic_surface.cpp'
--- tests/unit-tests/scene/test_basic_surface.cpp 2015-07-27 07:24:50 +0000
+++ tests/unit-tests/scene/test_basic_surface.cpp 2015-10-08 12:19:30 +0000
@@ -760,6 +760,11 @@
760 return (pos == arg->screen_position().top_left);760 return (pos == arg->screen_position().top_left);
761}761}
762762
763MATCHER_P(IsRenderableOfSize, size, "is renderable with size")
764{
765 return (size == arg->screen_position().size);
766}
767
763MATCHER_P(IsRenderableOfAlpha, alpha, "is renderable with alpha")768MATCHER_P(IsRenderableOfAlpha, alpha, "is renderable with alpha")
764{769{
765 EXPECT_THAT(static_cast<float>(alpha), testing::FloatEq(arg->alpha()));770 EXPECT_THAT(static_cast<float>(alpha), testing::FloatEq(arg->alpha()));
@@ -965,3 +970,28 @@
965 EXPECT_THAT(surface.buffers_ready_for_compositor(this), Eq(0));970 EXPECT_THAT(surface.buffers_ready_for_compositor(this), Eq(0));
966 EXPECT_THAT(surface.buffers_ready_for_compositor(this), Eq(2));971 EXPECT_THAT(surface.buffers_ready_for_compositor(this), Eq(2));
967}972}
973
974TEST_F(BasicSurfaceTest, buffer_streams_produce_correctly_sized_renderables)
975{
976 using namespace testing;
977 auto buffer_stream = std::make_shared<NiceMock<mtd::MockBufferStream>>();
978 geom::Displacement d0{0,0};
979 geom::Displacement d1{19,99};
980 geom::Size size0 {100, 101};
981 geom::Size size1 {102, 103};
982 ON_CALL(*mock_buffer_stream, stream_size())
983 .WillByDefault(Return(size0));
984 ON_CALL(*buffer_stream, stream_size())
985 .WillByDefault(Return(size1));
986
987 std::list<ms::StreamInfo> streams = {
988 { mock_buffer_stream, d0 },
989 { buffer_stream, d1 },
990 };
991 surface.set_streams(streams);
992
993 auto renderables = surface.generate_renderables(this);
994 ASSERT_THAT(renderables.size(), Eq(2));
995 EXPECT_THAT(renderables[0], IsRenderableOfSize(size0));
996 EXPECT_THAT(renderables[1], IsRenderableOfSize(size1));
997}

Subscribers

People subscribed via source and target branches