Mir

Merge lp:~albaguirre/mir/zero-sized-images-hide-cursor into lp:mir

  1. zero-sized-images-hide-cursor
  2. Merge into development-branch
Proposed by Alberto Aguirre
Status: Merged
Approved by: Alan Griffiths
Approved revision: no longer in the source branch.
Merged at revision: 3146
Proposed branch: lp:~albaguirre/mir/zero-sized-images-hide-cursor
Merge into: lp:mir
Diff against target: 345 lines (+67/-35)
4 files modified
src/server/input/cursor_controller.cpp (+12/-6)
tests/acceptance-tests/throwback/test_client_cursor_api.cpp (+1/-1)
tests/unit-tests/graphics/nested/test_nested_cursor.cpp (+1/-1)
tests/unit-tests/input/test_cursor_controller.cpp (+53/-27)
To merge this branch: bzr merge lp:~albaguirre/mir/zero-sized-images-hide-cursor
Reviewer Review Type Date Requested Status
Alan Griffiths Approve
Alexandros Frantzis (community) Approve
PS Jenkins bot (community) continuous-integration Approve
Review via email: mp+278863@code.launchpad.net

Commit message

cursor controller: zero sized images hide cursor

A given surface may have an attached zero sized cursor image.
Make the cursor controller hide the cursor in such cases.

Description of the change

cursor controller: zero sized images hide cursor

A given surface may have an attached zero sized cursor image.
Make the cursor controller hide the cursor in such cases.

This also helps qtmir upcoming surface cursor support which attaches named but zero-sized cursor images to surfaces, which before this MP led to the nested cursor attempt to allocate a zero-sized buffer stream instead of hiding the cursor.

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

PASSED: Continuous integration, rev:3135
http://jenkins.qa.ubuntu.com/job/mir-ci/5707/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/5083
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/3989
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/5030
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-xenial-touch/38/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-xenial-amd64-ci/37
        deb: http://jenkins.qa.ubuntu.com/job/mir-xenial-amd64-ci/37/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-xenial-i386-ci/37
        deb: http://jenkins.qa.ubuntu.com/job/mir-xenial-i386-ci/37/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5030
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5030/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-touch/7569
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/25602
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-xenial-armhf/36/console

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

review: Approve (continuous-integration)
Revision history for this message
Alexandros Frantzis (afrantzis) wrote :

Looks good.

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

OK

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/server/input/cursor_controller.cpp'
2--- src/server/input/cursor_controller.cpp 2015-10-02 01:56:04 +0000
3+++ src/server/input/cursor_controller.cpp 2015-11-27 18:50:49 +0000
4@@ -21,6 +21,7 @@
5 #include "mir/input/scene.h"
6 #include "mir/input/surface.h"
7 #include "mir/graphics/cursor.h"
8+#include "mir/graphics/cursor_image.h"
9 #include "mir/scene/observer.h"
10 #include "mir/scene/null_surface_observer.h"
11 #include "mir/scene/surface.h"
12@@ -105,7 +106,7 @@
13 : cursor_controller(cursor_controller)
14 {
15 }
16-
17+
18 void add_surface_observer(ms::Surface* surface)
19 {
20 auto const observer = std::make_shared<UpdateCursorOnSurfaceChanges>(cursor_controller);
21@@ -116,7 +117,7 @@
22 surface_observers[surface] = observer;
23 }
24 }
25-
26+
27 void surface_added(ms::Surface *surface)
28 {
29 add_surface_observer(surface);
30@@ -144,13 +145,13 @@
31 {
32 cursor_controller->update_cursor_image();
33 }
34-
35+
36 void surface_exists(ms::Surface *surface)
37 {
38 add_surface_observer(surface);
39 cursor_controller->update_cursor_image();
40 }
41-
42+
43 void end_observation()
44 {
45 std::unique_lock<decltype(surface_observers_guard)> lg(surface_observers_guard);
46@@ -162,7 +163,7 @@
47 }
48 surface_observers.clear();
49 }
50-
51+
52 private:
53 mi::CursorController* const cursor_controller;
54
55@@ -183,6 +184,11 @@
56 return top_surface_at_point;
57 }
58
59+bool is_empty(std::shared_ptr<mg::CursorImage> const& image)
60+{
61+ auto const size = image->size();
62+ return size.width.as_int() == 0 || size.height.as_int() == 0;
63+}
64 }
65
66 mi::CursorController::CursorController(std::shared_ptr<mi::Scene> const& input_targets,
67@@ -224,7 +230,7 @@
68
69 lock.unlock();
70
71- if (image)
72+ if (image && !is_empty(image))
73 cursor->show(*image);
74 else
75 cursor->hide();
76
77=== modified file 'tests/acceptance-tests/throwback/test_client_cursor_api.cpp'
78--- tests/acceptance-tests/throwback/test_client_cursor_api.cpp 2015-11-05 11:51:04 +0000
79+++ tests/acceptance-tests/throwback/test_client_cursor_api.cpp 2015-11-27 18:50:49 +0000
80@@ -75,7 +75,7 @@
81 }
82
83 void const* as_argb_8888() const override { return nullptr; }
84- geom::Size size() const override { return geom::Size{}; }
85+ geom::Size size() const override { return geom::Size{16, 16}; }
86 geom::Displacement hotspot() const override { return geom::Displacement{0, 0}; }
87
88 std::string const cursor_name;
89
90=== modified file 'tests/unit-tests/graphics/nested/test_nested_cursor.cpp'
91--- tests/unit-tests/graphics/nested/test_nested_cursor.cpp 2015-06-25 03:00:08 +0000
92+++ tests/unit-tests/graphics/nested/test_nested_cursor.cpp 2015-11-27 18:50:49 +0000
93@@ -43,7 +43,7 @@
94 struct StubCursorImage : public mg::CursorImage
95 {
96 void const* as_argb_8888() const { return this; }
97- geom::Size size() const { return geom::Size{}; }
98+ geom::Size size() const { return geom::Size{16, 16}; }
99 geom::Displacement hotspot() const { return geom::Displacement{0, 0}; }
100 };
101
102
103=== modified file 'tests/unit-tests/input/test_cursor_controller.cpp'
104--- tests/unit-tests/input/test_cursor_controller.cpp 2015-07-16 08:13:28 +0000
105+++ tests/unit-tests/input/test_cursor_controller.cpp 2015-11-27 18:50:49 +0000
106@@ -58,19 +58,26 @@
107 : cursor_name(name)
108 {
109 }
110-
111- void const* as_argb_8888() const { return nullptr; }
112- geom::Size size() const { return geom::Size{}; }
113- geom::Displacement hotspot() const { return geom::Displacement{0, 0}; }
114+
115+ void const* as_argb_8888() const override { return nullptr; }
116+ geom::Size size() const override { return geom::Size{16, 16}; }
117+ geom::Displacement hotspot() const override { return geom::Displacement{0, 0}; }
118
119 std::string const cursor_name;
120 };
121
122+struct ZeroSizedCursorImage : public mg::CursorImage
123+{
124+ void const* as_argb_8888() const override { return nullptr; }
125+ geom::Size size() const override { return geom::Size{}; }
126+ geom::Displacement hotspot() const override { return geom::Displacement{0, 0}; }
127+};
128+
129 bool cursor_is_named(mg::CursorImage const& i, std::string const& name)
130 {
131 auto image = dynamic_cast<NamedCursorImage const*>(&i);
132 assert(image);
133-
134+
135 return image->cursor_name == name;
136 }
137
138@@ -89,7 +96,7 @@
139 MOCK_METHOD0(show, void());
140 MOCK_METHOD1(show, void(mg::CursorImage const&));
141 MOCK_METHOD0(hide, void());
142-
143+
144 MOCK_METHOD1(move_to, void(geom::Point));
145 };
146
147@@ -103,29 +110,29 @@
148 cursor_image_(cursor_image)
149 {
150 }
151-
152+
153 std::string name() const override
154 {
155 return std::string();
156 }
157-
158+
159 geom::Rectangle input_bounds() const override
160 {
161 // We could return bounds here but lets make sure the cursor controller
162 // is only using input_area_contains.
163 return geom::Rectangle();
164 }
165-
166+
167 bool input_area_contains(geom::Point const& point) const override
168 {
169 return bounds.contains(point);
170 }
171-
172+
173 std::shared_ptr<mi::InputChannel> input_channel() const override
174 {
175 return nullptr;
176 }
177-
178+
179 mi::InputReceptionMode reception_mode() const override
180 {
181 return mi::InputReceptionMode::normal;
182@@ -139,7 +146,7 @@
183 void set_cursor_image(std::shared_ptr<mg::CursorImage> const& image) override
184 {
185 cursor_image_ = image;
186-
187+
188 {
189 std::unique_lock<decltype(observer_guard)> lk(observer_guard);
190 for (auto o : observers)
191@@ -178,7 +185,7 @@
192
193 geom::Rectangle const bounds;
194 std::shared_ptr<mg::CursorImage> cursor_image_;
195-
196+
197 std::mutex observer_guard;
198 std::vector<std::shared_ptr<ms::SurfaceObserver>> observers;
199 };
200@@ -189,13 +196,13 @@
201 : targets(targets.begin(), targets.end())
202 {
203 }
204-
205+
206 void for_each(std::function<void(std::shared_ptr<mi::Surface> const&)> const& callback) override
207 {
208 for (auto const& target : targets)
209 callback(target);
210 }
211-
212+
213 void add_observer(std::shared_ptr<ms::Observer> const& observer) override
214 {
215 observers.add(observer);
216@@ -216,7 +223,7 @@
217
218 observers.remove(o);
219 }
220-
221+
222 void add_surface(std::shared_ptr<StubInputSurface> const& surface)
223 {
224 targets.push_back(surface);
225@@ -225,7 +232,7 @@
226 observer->surface_added(surface.get());
227 });
228 }
229-
230+
231 // TODO: Should be mi::Surface. See comment on StubInputSurface.
232 std::vector<std::shared_ptr<ms::Surface>> targets;
233
234@@ -242,7 +249,7 @@
235 geom::Rectangle const rect_1_1_1_1{{1, 1}, {1, 1}};
236 std::string const cursor_name_1 = "test-cursor-1";
237 std::string const cursor_name_2 = "test-cursor-2";
238-
239+
240 MockCursor cursor;
241 std::shared_ptr<mg::CursorImage> const default_cursor_image;
242 };
243@@ -254,14 +261,14 @@
244 using namespace ::testing;
245
246 StubScene targets({});
247-
248+
249 mi::CursorController controller(mt::fake_shared(targets),
250 mt::fake_shared(cursor), default_cursor_image);
251
252 InSequence seq;
253 EXPECT_CALL(cursor, move_to(geom::Point{geom::X{1.0f}, geom::Y{1.0f}}));
254 EXPECT_CALL(cursor, move_to(geom::Point{geom::X{0.0f}, geom::Y{0.0f}}));
255-
256+
257 controller.cursor_moved_to(1.0f, 1.0f);
258 controller.cursor_moved_to(0.0f, 0.0f);
259 }
260@@ -273,7 +280,7 @@
261 StubInputSurface surface{rect_1_1_1_1,
262 std::make_shared<NamedCursorImage>(cursor_name_1)};
263 StubScene targets({mt::fake_shared(surface)});
264-
265+
266 mi::CursorController controller(mt::fake_shared(targets),
267 mt::fake_shared(cursor), default_cursor_image);
268
269@@ -290,7 +297,7 @@
270 StubInputSurface surface{rect_1_1_1_1,
271 nullptr};
272 StubScene targets({mt::fake_shared(surface)});
273-
274+
275 mi::CursorController controller(mt::fake_shared(targets),
276 mt::fake_shared(cursor), default_cursor_image);
277
278@@ -307,7 +314,7 @@
279 StubInputSurface surface_1{rect_1_1_1_1, std::make_shared<NamedCursorImage>(cursor_name_1)};
280 StubInputSurface surface_2{rect_1_1_1_1, std::make_shared<NamedCursorImage>(cursor_name_2)};
281 StubScene targets({mt::fake_shared(surface_1), mt::fake_shared(surface_2)});
282-
283+
284 mi::CursorController controller(mt::fake_shared(targets),
285 mt::fake_shared(cursor), default_cursor_image);
286
287@@ -324,7 +331,7 @@
288 StubInputSurface surface{rect_1_1_1_1,
289 std::make_shared<NamedCursorImage>(cursor_name_1)};
290 StubScene targets({mt::fake_shared(surface)});
291-
292+
293 mi::CursorController controller(mt::fake_shared(targets),
294 mt::fake_shared(cursor), default_cursor_image);
295
296@@ -347,7 +354,7 @@
297 StubInputSurface surface{rect_1_1_1_1,
298 std::make_shared<NamedCursorImage>(cursor_name_1)};
299 StubScene targets({mt::fake_shared(surface)});
300-
301+
302 mi::CursorController controller(mt::fake_shared(targets),
303 mt::fake_shared(cursor), default_cursor_image);
304
305@@ -370,7 +377,7 @@
306 StubInputSurface surface{rect_0_0_1_1,
307 std::make_shared<NamedCursorImage>(cursor_name_1)};
308 StubScene targets({});
309-
310+
311 mi::CursorController controller(mt::fake_shared(targets),
312 mt::fake_shared(cursor), default_cursor_image);
313
314@@ -390,7 +397,7 @@
315 StubInputSurface surface1{rect_0_0_1_1, image};
316 StubInputSurface surface2{rect_0_0_1_1, image};
317 StubScene targets({});
318-
319+
320 mi::CursorController controller(mt::fake_shared(targets),
321 mt::fake_shared(cursor), default_cursor_image);
322
323@@ -430,3 +437,22 @@
324 EXPECT_CALL(cursor, show(_)).Times(0);
325 surface.post_frame();
326 }
327+
328+TEST_F(TestCursorController, zero_sized_image_hides_cursor)
329+{
330+ using namespace ::testing;
331+
332+ auto image = std::make_shared<ZeroSizedCursorImage>();
333+
334+ // Here we also demonstrate that the cursor begins at 0,0.
335+ StubInputSurface surface{rect_0_0_1_1, image};
336+ StubScene targets({});
337+
338+ mi::CursorController controller(mt::fake_shared(targets),
339+ mt::fake_shared(cursor), default_cursor_image);
340+
341+ EXPECT_CALL(cursor, move_to(_)).Times(AnyNumber());
342+ EXPECT_CALL(cursor, hide()).Times(1);
343+
344+ targets.add_surface(mt::fake_shared(surface));
345+}

Subscribers

People subscribed via source and target branches