Mir

Merge lp:~alan-griffiths/mir/improved-Rectangles into lp:mir

  1. improved-Rectangles
  2. Merge into development-branch
Proposed by Alan Griffiths on 2015-01-21
Status: Merged
Merged at revision: 2255
Proposed branch: lp:~alan-griffiths/mir/improved-Rectangles
Merge into: lp:mir
Diff against target: 406 lines (+122/-72)
8 files modified
common-ABI-sha1sums (+1/-1)
examples/server_example_window_management.cpp (+5/-10)
include/common/mir/geometry/rectangles.h (+4/-4)
platform-ABI-sha1sums (+1/-1)
server-ABI-sha1sums (+1/-1)
src/common/geometry/rectangles.cpp (+7/-1)
src/common/symbols.map (+1/-0)
tests/unit-tests/geometry/test-rectangles.cpp (+102/-54)
To merge this branch: bzr merge lp:~alan-griffiths/mir/improved-Rectangles
Reviewer Review Type Date Requested Status
PS Jenkins bot continuous-integration Needs Fixing on 2015-01-22
Kevin DuBois (community) Approve on 2015-01-22
Alexandros Frantzis (community) Approve on 2015-01-22
Daniel van Vugt Needs Fixing on 2015-01-22
Alberto Aguirre 2015-01-21 Approve on 2015-01-21
Review via email: mp+247161@code.launchpad.net

Commit Message

geometry: add a remove() method to the Rectangles class

Description of the Change

geometry: add a remove() method to the Rectangles class

This provides better support for tracking e.g. the display rectangles

To post a comment you must log in.
Alberto Aguirre (albaguirre) wrote :

LGTM

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

PASSED: Continuous integration, rev:2245
http://jenkins.qa.ubuntu.com/job/mir-ci/2745/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/971
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/971
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/933
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/742
        deb: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/742/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/933
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/933/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/4029
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/17292

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

review: Approve (continuous-integration)
Kevin DuBois (kdub) wrote :

It is was somewhat unclear what remove does if you add multiple same-sized-rectangles, maybe a test that checks that all rectangles of the same size are removed (or never double-added)?

review: Needs Fixing
Kevin DuBois (kdub) wrote :

> It is was somewhat unclear what remove does if you add multiple same-sized-
*It is somewhat...
https://www.youtube.com/watch?v=KzyCi1BFATA

Daniel van Vugt (vanvugt) wrote :

This potentially makes us sensitive to the order in which tests are run, so should probably be undone:

173 +struct TestRectangles : Test
174 +{
175 + Rectangles rectangles;

202 - Rectangles rectangles;
203 -
...

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

I think what Kevin is asking for is a: std::set<Rectangle>

Daniel van Vugt (vanvugt) wrote :

Hmm, why don't we just have a set<Rectangle> or unordered_set<Rectangle> ?

The required comparison operator for Rectangle to make it work would be simple to add.

Alexandros Frantzis (afrantzis) wrote :

Looks good.

> It is somewhat unclear what remove does if you add multiple same-sized-rectangles,
> maybe a test that checks that all rectangles of the same size are removed

It would be good to have a test and comment for this.

> This potentially makes us sensitive to the order in which tests are run,
> so should probably be undone:
>
> 173 +struct TestRectangles : Test
> 174 +{
> 175 + Rectangles rectangles;

A new fixture instance is created for each test case.

> Hmm, why don't we just have a set<Rectangle> or unordered_set<Rectangle> ?

Rectangles is designed to hold multiple same-sized-rectangles. We could use a (unordered_)multiset, but in terms of performance it's probably an overkill (vs vector) for the sizes we care about. I am fine either way.

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

> Looks good.
>
> > It is somewhat unclear what remove does if you add multiple same-sized-
> rectangles,
> > maybe a test that checks that all rectangles of the same size are removed
>
> It would be good to have a test and comment for this.

Done

Alan Griffiths (alan-griffiths) wrote :

> This potentially makes us sensitive to the order in which tests are run, so
> should probably be undone:
>
> 173 +struct TestRectangles : Test
> 174 +{
> 175 + Rectangles rectangles;
>
> 202 - Rectangles rectangles;
> 203 -
> ...

Oh no it doesn't.

Alexandros Frantzis (afrantzis) :
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/1047/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/1008
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/1007/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/969
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-autolanding/227
        deb: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-autolanding/227/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/969
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/969/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/4056
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/17352

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

The intended behavior is clearer with the tests, so lgtm!

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

[ 71%] Building CXX object tests/integration-tests/CMakeFiles/mir_integration_tests.dir/test_session_manager.cpp.o
FATAL: Unable to delete script file /tmp/hudson7970247705980537602.sh
hudson.util.IOException2: remote file operation failed: /tmp/hudson7970247705980537602.sh at hudson.remoting.Channel@3f39e40c:cloud-worker-10

reapproving

PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:2248
http://jenkins.qa.ubuntu.com/job/mir-ci/2775/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/1011
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/1010
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/972
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/772
        deb: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/772/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/972
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/972/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/4059
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/17359

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

review: Approve (continuous-integration)
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Daniel van Vugt (vanvugt) wrote :

I meant this increases the future risk of us becoming sensitive to the order of test case execution. Even if the current test cases guard against that, it's more defensive to make it impossible for future maintainers to make such a mistake. And would provide higher cohesion of tests too...

192 +struct TestRectangles : Test
193 +{
194 + Rectangles rectangles;

So TEST_F becomes TEST again.

Alan Griffiths (alan-griffiths) wrote :

> I meant this increases the future risk of us becoming sensitive to the order
> of test case execution. Even if the current test cases guard against that,
> it's more defensive to make it impossible for future maintainers to make such
> a mistake. And would provide higher cohesion of tests too...
>
> 192 +struct TestRectangles : Test
> 193 +{
> 194 + Rectangles rectangles;
>
> So TEST_F becomes TEST again.

Daniel, I really don't understand your concern. Are you misreading

> 194 + Rectangles rectangles;

as this?
               static Rectangles rectangles;

Moving rectangles from function scope to instance scope doesn't share it between different instances.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'common-ABI-sha1sums'
2--- common-ABI-sha1sums 2015-01-20 03:21:20 +0000
3+++ common-ABI-sha1sums 2015-01-22 10:38:23 +0000
4@@ -6,7 +6,7 @@
5 bfd87d9fb800c1c1b528c00185570abac569caf2 include/common/mir/geometry/length.h
6 d954464ef2d20c2876db68c94512a443186da09b include/common/mir/geometry/point.h
7 dc7c62d3916eec025e8e7deaf57e06077ce38928 include/common/mir/geometry/rectangle.h
8-5161774957e3ca4f5fa4e7db025d0978d2bbef06 include/common/mir/geometry/rectangles.h
9+41446a309ee25148a9b1a66ebee205c57e014a8f include/common/mir/geometry/rectangles.h
10 20fa71bf856b1f391ad84b0afd8c197e0ca9e982 include/common/mir/geometry/size.h
11 e1be9faee8b844ca2ce617f8fd82c9ee08d56bed include/common/mir/graphics/native_buffer.h
12 dcf8b8982f138bdde39a241825c610e955cd5e33 include/common/mir/input/input_platform.h
13
14=== modified file 'examples/server_example_window_management.cpp'
15--- examples/server_example_window_management.cpp 2015-01-16 02:57:31 +0000
16+++ examples/server_example_window_management.cpp 2015-01-22 10:38:23 +0000
17@@ -170,15 +170,14 @@
18 void add_display(Rectangle const& area) override
19 {
20 std::lock_guard<decltype(mutex)> lock(mutex);
21- displays.push_back(area);
22+ displays.add(area);
23 update_tiles();
24 }
25
26 void remove_display(Rectangle const& area) override
27 {
28 std::lock_guard<decltype(mutex)> lock(mutex);
29- auto const i = std::find(begin(displays), end(displays), area);
30- if (i != end(displays)) displays.erase(i);
31+ displays.remove(area);
32 update_tiles();
33 }
34
35@@ -308,12 +307,8 @@
36 if (session_info.size() < 1 || displays.size() < 1) return;
37
38 auto const sessions = session_info.size();
39- Rectangles view;
40-
41- for (auto const& display : displays)
42- view.add(display);
43-
44- auto const bounding_rect = view.bounding_rectangle();
45+
46+ auto const bounding_rect = displays.bounding_rectangle();
47
48 auto const total_width = bounding_rect.size.width.as_int();
49 auto const total_height = bounding_rect.size.height.as_int();
50@@ -558,7 +553,7 @@
51 FocusControllerFactory const focus_controller;
52
53 std::mutex mutex;
54- std::vector<Rectangle> displays;
55+ Rectangles displays;
56
57 std::map<ms::Session const*, SessionInfo> session_info;
58 std::map<std::weak_ptr<ms::Surface>, SurfaceInfo, std::owner_less<std::weak_ptr<ms::Surface>>> surface_info;
59
60=== modified file 'include/common/mir/geometry/rectangles.h'
61--- include/common/mir/geometry/rectangles.h 2015-01-14 06:39:13 +0000
62+++ include/common/mir/geometry/rectangles.h 2015-01-22 10:38:23 +0000
63@@ -1,5 +1,5 @@
64 /*
65- * Copyright © 2013 Canonical Ltd.
66+ * Copyright © 2013-2015 Canonical Ltd.
67 *
68 * This program is free software: you can redistribute it and/or modify it
69 * under the terms of the GNU Lesser General Public License version 3,
70@@ -14,9 +14,6 @@
71 * along with this program. If not, see <http://www.gnu.org/licenses/>.
72 *
73 * Authored by: Alexandros Frantzis <alexandros.frantzis@canonical.com>
74- *
75- * XXX This header is only used externally in one location in QtMir. It could
76- * possibly be made private with only minor changes.
77 */
78
79 #ifndef MIR_GEOMETRY_RECTANGLES_H_
80@@ -33,6 +30,7 @@
81 namespace geometry
82 {
83
84+/// A collection of rectangles (with possible duplicates).
85 class Rectangles
86 {
87 public:
88@@ -41,6 +39,8 @@
89 /* We want to keep implicit copy and move methods */
90
91 void add(Rectangle const& rect);
92+ /// removes at most one matching rectangle
93+ void remove(Rectangle const& rect);
94 void clear();
95 Rectangle bounding_rectangle() const;
96 void confine(Point& point) const;
97
98=== modified file 'platform-ABI-sha1sums'
99--- platform-ABI-sha1sums 2015-01-20 03:21:20 +0000
100+++ platform-ABI-sha1sums 2015-01-22 10:38:23 +0000
101@@ -6,7 +6,7 @@
102 bfd87d9fb800c1c1b528c00185570abac569caf2 include/common/mir/geometry/length.h
103 d954464ef2d20c2876db68c94512a443186da09b include/common/mir/geometry/point.h
104 dc7c62d3916eec025e8e7deaf57e06077ce38928 include/common/mir/geometry/rectangle.h
105-5161774957e3ca4f5fa4e7db025d0978d2bbef06 include/common/mir/geometry/rectangles.h
106+41446a309ee25148a9b1a66ebee205c57e014a8f include/common/mir/geometry/rectangles.h
107 20fa71bf856b1f391ad84b0afd8c197e0ca9e982 include/common/mir/geometry/size.h
108 e1be9faee8b844ca2ce617f8fd82c9ee08d56bed include/common/mir/graphics/native_buffer.h
109 dcf8b8982f138bdde39a241825c610e955cd5e33 include/common/mir/input/input_platform.h
110
111=== modified file 'server-ABI-sha1sums'
112--- server-ABI-sha1sums 2015-01-22 03:10:13 +0000
113+++ server-ABI-sha1sums 2015-01-22 10:38:23 +0000
114@@ -6,7 +6,7 @@
115 bfd87d9fb800c1c1b528c00185570abac569caf2 include/common/mir/geometry/length.h
116 d954464ef2d20c2876db68c94512a443186da09b include/common/mir/geometry/point.h
117 dc7c62d3916eec025e8e7deaf57e06077ce38928 include/common/mir/geometry/rectangle.h
118-5161774957e3ca4f5fa4e7db025d0978d2bbef06 include/common/mir/geometry/rectangles.h
119+41446a309ee25148a9b1a66ebee205c57e014a8f include/common/mir/geometry/rectangles.h
120 20fa71bf856b1f391ad84b0afd8c197e0ca9e982 include/common/mir/geometry/size.h
121 e1be9faee8b844ca2ce617f8fd82c9ee08d56bed include/common/mir/graphics/native_buffer.h
122 dcf8b8982f138bdde39a241825c610e955cd5e33 include/common/mir/input/input_platform.h
123
124=== modified file 'src/common/geometry/rectangles.cpp'
125--- src/common/geometry/rectangles.cpp 2015-01-14 06:39:13 +0000
126+++ src/common/geometry/rectangles.cpp 2015-01-22 10:38:23 +0000
127@@ -1,5 +1,5 @@
128 /*
129- * Copyright © 2013 Canonical Ltd.
130+ * Copyright © 2013-2015 Canonical Ltd.
131 *
132 * This program is free software: you can redistribute it and/or modify it
133 * under the terms of the GNU Lesser General Public License version 3,
134@@ -55,6 +55,12 @@
135 rectangles.push_back(rect);
136 }
137
138+void geom::Rectangles::remove(Rectangle const& rect)
139+{
140+ auto const i = std::find(rectangles.begin(), rectangles.end(), rect);
141+ if (i != rectangles.end()) rectangles.erase(i);
142+}
143+
144 void geom::Rectangles::clear()
145 {
146 rectangles.clear();
147
148=== modified file 'src/common/symbols.map'
149--- src/common/symbols.map 2015-01-14 06:39:13 +0000
150+++ src/common/symbols.map 2015-01-22 10:38:23 +0000
151@@ -31,6 +31,7 @@
152 mir::geometry::Rectangles::end*;
153 mir::geometry::Rectangles::operator*;
154 mir::geometry::Rectangles::Rectangles*;
155+ mir::geometry::Rectangles::remove*;
156 mir::geometry::Rectangles::size*;
157 mir::geometry::Rectangle::top_right*;
158 mir::geometry::Size::operator*;
159
160=== modified file 'tests/unit-tests/geometry/test-rectangles.cpp'
161--- tests/unit-tests/geometry/test-rectangles.cpp 2013-08-28 03:41:48 +0000
162+++ tests/unit-tests/geometry/test-rectangles.cpp 2015-01-22 10:38:23 +0000
163@@ -1,5 +1,5 @@
164 /*
165- * Copyright © 2013 Canonical Ltd.
166+ * Copyright © 2013-2015 Canonical Ltd.
167 *
168 * This program is free software: you can redistribute it and/or modify
169 * it under the terms of the GNU General Public License version 3 as
170@@ -19,34 +19,42 @@
171 #include "mir/geometry/rectangles.h"
172
173 #include <gtest/gtest.h>
174+#include <gmock/gmock.h>
175
176 #include <iterator>
177 #include <algorithm>
178
179-namespace geom = mir::geometry;
180-
181-TEST(geometry, rectangles_empty)
182-{
183- using namespace geom;
184-
185- Rectangles const rectangles;
186-
187+using namespace mir::geometry;
188+using namespace testing;
189+
190+namespace
191+{
192+struct TestRectangles : Test
193+{
194+ Rectangles rectangles;
195+
196+ auto contents_of(Rectangles const& rects) ->
197+ std::vector<Rectangle>
198+ {
199+ return {std::begin(rects), std::end(rects)};
200+ }
201+};
202+}
203+
204+TEST_F(TestRectangles, rectangles_empty)
205+{
206 EXPECT_EQ(0, std::distance(rectangles.begin(), rectangles.end()));
207 EXPECT_EQ(0u, rectangles.size());
208 }
209
210-TEST(geometry, rectangles_not_empty)
211+TEST_F(TestRectangles, rectangles_not_empty)
212 {
213- using namespace geom;
214-
215 std::vector<Rectangle> const rectangles_vec = {
216 {Point{1, 2}, Size{Width{10}, Height{11}}},
217 {Point{3, 4}, Size{Width{12}, Height{13}}},
218 {Point{5, 6}, Size{Width{14}, Height{15}}}
219 };
220
221- Rectangles rectangles;
222-
223 for (auto const& rect : rectangles_vec)
224 rectangles.add(rect);
225
226@@ -70,17 +78,14 @@
227 [](bool b){return b;}));
228 }
229
230-TEST(geometry, rectangles_clear)
231+TEST_F(TestRectangles, rectangles_clear)
232 {
233- using namespace geom;
234-
235 std::vector<Rectangle> const rectangles_vec = {
236 {Point{1, 2}, Size{Width{10}, Height{11}}},
237 {Point{3, 4}, Size{Width{12}, Height{13}}},
238 {Point{5, 6}, Size{Width{14}, Height{15}}}
239 };
240
241- Rectangles rectangles;
242 Rectangles const rectangles_empty;
243
244 for (auto const& rect : rectangles_vec)
245@@ -93,10 +98,8 @@
246 EXPECT_EQ(rectangles_empty, rectangles);
247 }
248
249-TEST(geometry, rectangles_bounding_rectangle)
250+TEST_F(TestRectangles, rectangles_bounding_rectangle)
251 {
252- using namespace geom;
253-
254 std::vector<Rectangle> const rectangles_vec = {
255 {Point{0, 0}, Size{Width{10}, Height{5}}},
256 {Point{2, 2}, Size{Width{4}, Height{1}}},
257@@ -115,8 +118,6 @@
258 {Point{-2, -3}, Size{Width{14}, Height{10}}}
259 };
260
261- Rectangles rectangles;
262-
263 EXPECT_EQ(Rectangle(), rectangles.bounding_rectangle());
264
265 for (size_t i = 0; i < rectangles_vec.size(); i++)
266@@ -126,10 +127,8 @@
267 }
268 }
269
270-TEST(geometry, rectangles_equality)
271+TEST_F(TestRectangles, rectangles_equality)
272 {
273- using namespace geom;
274-
275 std::vector<Rectangle> const rectangles_vec1 = {
276 {Point{0, 0}, Size{Width{10}, Height{5}}},
277 {Point{2, 2}, Size{Width{4}, Height{1}}},
278@@ -169,10 +168,8 @@
279 EXPECT_NE(rectangles1, rectangles_empty);
280 }
281
282-TEST(geometry, rectangles_copy_assign)
283+TEST_F(TestRectangles, rectangles_copy_assign)
284 {
285- using namespace geom;
286-
287 std::vector<Rectangle> const rectangles_vec = {
288 {Point{0, 0}, Size{Width{10}, Height{5}}},
289 {Point{2, 2}, Size{Width{4}, Height{1}}},
290@@ -200,40 +197,91 @@
291 EXPECT_EQ(rectangles2.bounding_rectangle(), rectangles3.bounding_rectangle());
292 }
293
294-TEST(geometry, rectangles_confine)
295+TEST_F(TestRectangles, rectangles_confine)
296 {
297- using namespace geom;
298-
299 std::vector<Rectangle> const rectangles_vec = {
300- {geom::Point{0,0}, geom::Size{800,600}},
301- {geom::Point{0,600}, geom::Size{100,100}},
302- {geom::Point{800,0}, geom::Size{100,100}}
303- };
304-
305- std::vector<std::tuple<geom::Point,geom::Point>> const point_tuples{
306- std::make_tuple(geom::Point{0,0}, geom::Point{0,0}),
307- std::make_tuple(geom::Point{900,50}, geom::Point{899,50}),
308- std::make_tuple(geom::Point{850,100}, geom::Point{850,99}),
309- std::make_tuple(geom::Point{801,100}, geom::Point{801,99}),
310- std::make_tuple(geom::Point{800,101}, geom::Point{799,101}),
311- std::make_tuple(geom::Point{800,600}, geom::Point{799,599}),
312- std::make_tuple(geom::Point{-1,700}, geom::Point{0,699}),
313- std::make_tuple(geom::Point{-1,-1}, geom::Point{0,0}),
314- std::make_tuple(geom::Point{-1,50}, geom::Point{0,50}),
315- std::make_tuple(geom::Point{799,-1}, geom::Point{799,0}),
316- std::make_tuple(geom::Point{800,-1}, geom::Point{800,0})
317- };
318-
319- Rectangles rectangles;
320+ {Point{0,0}, Size{800,600}},
321+ {Point{0,600}, Size{100,100}},
322+ {Point{800,0}, Size{100,100}}
323+ };
324+
325+ std::vector<std::tuple<Point,Point>> const point_tuples{
326+ std::make_tuple(Point{0,0}, Point{0,0}),
327+ std::make_tuple(Point{900,50}, Point{899,50}),
328+ std::make_tuple(Point{850,100}, Point{850,99}),
329+ std::make_tuple(Point{801,100}, Point{801,99}),
330+ std::make_tuple(Point{800,101}, Point{799,101}),
331+ std::make_tuple(Point{800,600}, Point{799,599}),
332+ std::make_tuple(Point{-1,700}, Point{0,699}),
333+ std::make_tuple(Point{-1,-1}, Point{0,0}),
334+ std::make_tuple(Point{-1,50}, Point{0,50}),
335+ std::make_tuple(Point{799,-1}, Point{799,0}),
336+ std::make_tuple(Point{800,-1}, Point{800,0})
337+ };
338
339 for (auto const& rect : rectangles_vec)
340 rectangles.add(rect);
341
342 for (auto const& t : point_tuples)
343 {
344- geom::Point confined_point{std::get<0>(t)};
345- geom::Point const expected_point{std::get<1>(t)};
346+ Point confined_point{std::get<0>(t)};
347+ Point const expected_point{std::get<1>(t)};
348 rectangles.confine(confined_point);
349 EXPECT_EQ(expected_point, confined_point);
350 }
351 }
352+
353+TEST_F(TestRectangles, tracks_add_and_remove)
354+{
355+ Rectangle const rect[]{
356+ {{ 0, 0}, {800, 600}},
357+ {{ 0, 600}, {100, 100}},
358+ {{800, 0}, {100, 100}}};
359+
360+ rectangles = Rectangles{rect[0], rect[1], rect[2]};
361+
362+ EXPECT_THAT(contents_of(rectangles), UnorderedElementsAre(rect[0], rect[1], rect[2]));
363+ EXPECT_THAT(rectangles.bounding_rectangle(), Eq(Rectangle{{0,0}, {900,700}}));
364+
365+ rectangles.remove(rect[1]);
366+
367+ EXPECT_THAT(contents_of(rectangles), UnorderedElementsAre(rect[0], rect[2]));
368+ EXPECT_THAT(rectangles.bounding_rectangle(), Eq(Rectangle{{0,0}, {900,600}}));
369+
370+ rectangles.add(rect[2]);
371+
372+ EXPECT_THAT(contents_of(rectangles), UnorderedElementsAre(rect[0], rect[2], rect[2]));
373+ EXPECT_THAT(rectangles.bounding_rectangle(), Eq(Rectangle{{0,0}, {900,600}}));
374+
375+ rectangles.add(rect[1]);
376+ rectangles.remove(rect[2]);
377+
378+ EXPECT_THAT(contents_of(rectangles), UnorderedElementsAre(rect[0], rect[1], rect[2]));
379+ EXPECT_THAT(rectangles.bounding_rectangle(), Eq(Rectangle{{0,0}, {900,700}}));
380+}
381+
382+TEST_F(TestRectangles, can_add_same_rectangle_many_times)
383+{
384+ int const many_times = 10;
385+ Rectangle const rectangle{{ 0, 0}, {800, 600}};
386+
387+ for (auto i = 0; i != many_times; ++i)
388+ rectangles.add(rectangle);
389+
390+ EXPECT_THAT(rectangles.size(), Eq(many_times));
391+}
392+
393+TEST_F(TestRectangles, remove_only_removes_one_instance)
394+{
395+ int const many_times = 10;
396+ Rectangle const rectangle{{ 0, 0}, {800, 600}};
397+
398+ for (auto i = 0; i != many_times; ++i)
399+ rectangles.add(rectangle);
400+
401+ for (auto i = many_times; i-- != 0;)
402+ {
403+ rectangles.remove(rectangle);
404+ EXPECT_THAT(rectangles.size(), Eq(i));
405+ }
406+}

Subscribers

People subscribed via source and target branches