Mir

Merge lp:~vanvugt/mir/shape into lp:mir

Proposed by Daniel van Vugt
Status: Merged
Approved by: Daniel van Vugt
Approved revision: no longer in the source branch.
Merged at revision: 1113
Proposed branch: lp:~vanvugt/mir/shape
Merge into: lp:mir
Diff against target: 493 lines (+149/-32)
15 files modified
examples/eglapp.c (+43/-2)
examples/eglapp.h (+2/-0)
examples/eglflash.c (+11/-11)
examples/egltriangle.c (+1/-1)
include/server/mir/compositor/compositing_criteria.h (+1/-0)
include/shared/mir/geometry/pixel_format.h (+6/-0)
include/test/mir_test_doubles/mock_compositing_criteria.h (+1/-0)
include/test/mir_test_doubles/mock_surface_state.h (+1/-0)
include/test/mir_test_doubles/stub_compositing_criteria.h (+10/-2)
src/server/compositor/bypass.cpp (+1/-1)
src/server/surfaces/surface_allocator.cpp (+4/-1)
src/server/surfaces/surface_data.cpp (+7/-1)
src/server/surfaces/surface_data.h (+4/-1)
tests/unit-tests/compositor/test_bypass.cpp (+43/-0)
tests/unit-tests/surfaces/test_surface_data.cpp (+14/-12)
To merge this branch: bzr merge lp:~vanvugt/mir/shape
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
Kevin DuBois (community) Approve
Alan Griffiths Approve
Review via email: mp+189557@code.launchpad.net

Commit message

Ensure nonrectangular surfaces are never bypassed, even if they are
fullscreen and on top (e.g. Unity8 shell at present).
(LP: #1236264)

Description of the change

More importantly, the shape() method is required to fix bug 1227739 on the phone, soon.

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

145 + virtual bool shaped() const = 0; // meaning the pixel format has alpha

When comments and code disagree: trust neither!

Surely we can come up with clearer terminology?

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

I think there's some conflation of shaped/alpha/opacity but otherwise seems to work as advertised.

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

To me 'nonrectangular' doesn't immediately mean to me 'surface has alpha channel', (alpha_masked(), perhaps)? but its ok.

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

I agree the terminology is confusing. Present terminology:
Mir alpha() == Compiz opacity()
Mir shaped() == Compiz alpha()

Since they're distinctly different things, and both related to alpha blending, I suggest not mentioning "alpha" at all. Hence bug 1236224.

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

The failure is a bug which seems to already exist on the target branch. I just reproduced it using development-branch.

It has nothing to do with this proposal. Retry...

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

The failure is now logged as bug 1236698.

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 'examples/eglapp.c'
2--- examples/eglapp.c 2013-09-19 13:24:22 +0000
3+++ examples/eglapp.c 2013-10-07 10:02:32 +0000
4@@ -26,6 +26,8 @@
5
6 #include <xkbcommon/xkbcommon-keysyms.h>
7
8+float mir_eglapp_background_opacity = 1.0f;
9+
10 static const char appname[] = "egldemo";
11
12 static MirConnection *connection;
13@@ -182,6 +184,26 @@
14 {
15 switch (arg[1])
16 {
17+ case 'b':
18+ {
19+ float alpha = 1.0f;
20+ arg += 2;
21+ if (!arg[0] && i < argc-1)
22+ {
23+ i++;
24+ arg = argv[i];
25+ }
26+ if (sscanf(arg, "%f", &alpha) == 1)
27+ {
28+ mir_eglapp_background_opacity = alpha;
29+ }
30+ else
31+ {
32+ printf("Invalid opacity value: %s\n", arg);
33+ help = 1;
34+ }
35+ }
36+ break;
37 case 'n':
38 swapinterval = 0;
39 break;
40@@ -247,6 +269,7 @@
41 if (help)
42 {
43 printf("Usage: %s [<options>]\n"
44+ " -b Background opacity (0.0 - 1.0)\n"
45 " -h Show this help text\n"
46 " -f Force full screen\n"
47 " -o ID Force placement on output monitor ID\n"
48@@ -277,9 +300,27 @@
49
50 const MirDisplayMode *mode = &output->modes[output->current_mode];
51
52- unsigned int valid_formats;
53+ const unsigned int max_formats = 10;
54+ unsigned int format[max_formats];
55+ unsigned int nformats;
56+
57 mir_connection_get_available_surface_formats(connection,
58- &surfaceparm.pixel_format, 1, &valid_formats);
59+ format, max_formats, &nformats);
60+
61+ surfaceparm.pixel_format = format[0];
62+ for (unsigned int f = 0; f < nformats; f++)
63+ {
64+ const int opaque = (format[f] == mir_pixel_format_xbgr_8888 ||
65+ format[f] == mir_pixel_format_xrgb_8888 ||
66+ format[f] == mir_pixel_format_bgr_888);
67+
68+ if ((mir_eglapp_background_opacity == 1.0f && opaque) ||
69+ (mir_eglapp_background_opacity < 1.0f && !opaque))
70+ {
71+ surfaceparm.pixel_format = format[f];
72+ break;
73+ }
74+ }
75
76 printf("Connected to display: resolution (%dx%d), position(%dx%d), "
77 "supports %d pixel formats\n",
78
79=== modified file 'examples/eglapp.h'
80--- examples/eglapp.h 2013-09-19 13:24:22 +0000
81+++ examples/eglapp.h 2013-10-07 10:02:32 +0000
82@@ -27,6 +27,8 @@
83 struct MirConnection;
84 struct MirSurface;
85
86+extern float mir_eglapp_background_opacity;
87+
88 mir_eglapp_bool mir_eglapp_init(int argc, char *argv[],
89 unsigned int *width, unsigned int *height);
90 void mir_eglapp_swap_buffers(void);
91
92=== modified file 'examples/eglflash.c'
93--- examples/eglflash.c 2013-09-19 13:24:22 +0000
94+++ examples/eglflash.c 2013-10-07 10:02:32 +0000
95@@ -33,17 +33,17 @@
96 /* This is probably the simplest GL you can do */
97 while (mir_eglapp_running())
98 {
99- glClearColor(1.0f, 0.0f, 0.0f, 1.0f);
100- glClear(GL_COLOR_BUFFER_BIT);
101- mir_eglapp_swap_buffers();
102- sleep(1);
103-
104- glClearColor(0.0f, 1.0f, 0.0f, 1.0f);
105- glClear(GL_COLOR_BUFFER_BIT);
106- mir_eglapp_swap_buffers();
107- sleep(1);
108-
109- glClearColor(0.0f, 0.0f, 1.0f, 1.0f);
110+ glClearColor(1.0f, 0.0f, 0.0f, mir_eglapp_background_opacity);
111+ glClear(GL_COLOR_BUFFER_BIT);
112+ mir_eglapp_swap_buffers();
113+ sleep(1);
114+
115+ glClearColor(0.0f, 1.0f, 0.0f, mir_eglapp_background_opacity);
116+ glClear(GL_COLOR_BUFFER_BIT);
117+ mir_eglapp_swap_buffers();
118+ sleep(1);
119+
120+ glClearColor(0.0f, 0.0f, 1.0f, mir_eglapp_background_opacity);
121 glClear(GL_COLOR_BUFFER_BIT);
122 mir_eglapp_swap_buffers();
123 sleep(1);
124
125=== modified file 'examples/egltriangle.c'
126--- examples/egltriangle.c 2013-09-19 13:24:22 +0000
127+++ examples/egltriangle.c 2013-10-07 10:02:32 +0000
128@@ -105,7 +105,7 @@
129 return 2;
130 }
131
132- glClearColor(MID_AUBERGINE, 1.0);
133+ glClearColor(MID_AUBERGINE, mir_eglapp_background_opacity);
134 glViewport(0, 0, width, height);
135
136 glUseProgram(prog);
137
138=== modified file 'include/server/mir/compositor/compositing_criteria.h'
139--- include/server/mir/compositor/compositing_criteria.h 2013-08-28 03:41:48 +0000
140+++ include/server/mir/compositor/compositing_criteria.h 2013-10-07 10:02:32 +0000
141@@ -36,6 +36,7 @@
142 virtual float alpha() const = 0;
143 virtual glm::mat4 const& transformation() const = 0;
144 virtual bool should_be_rendered_in(geometry::Rectangle const& rect) const = 0;
145+ virtual bool shaped() const = 0; // meaning the pixel format has alpha
146
147 virtual ~CompositingCriteria() = default;
148
149
150=== modified file 'include/shared/mir/geometry/pixel_format.h'
151--- include/shared/mir/geometry/pixel_format.h 2013-06-12 09:36:20 +0000
152+++ include/shared/mir/geometry/pixel_format.h 2013-10-07 10:02:32 +0000
153@@ -42,6 +42,12 @@
154 return (fmt == PixelFormat::bgr_888) ? 3 : 4;
155 }
156
157+static inline bool has_alpha(PixelFormat fmt)
158+{
159+ return (fmt == PixelFormat::abgr_8888) ||
160+ (fmt == PixelFormat::argb_8888);
161+}
162+
163 }
164 }
165
166
167=== modified file 'include/test/mir_test_doubles/mock_compositing_criteria.h'
168--- include/test/mir_test_doubles/mock_compositing_criteria.h 2013-08-28 03:41:48 +0000
169+++ include/test/mir_test_doubles/mock_compositing_criteria.h 2013-10-07 10:02:32 +0000
170@@ -36,6 +36,7 @@
171 MOCK_CONST_METHOD0(alpha, float());
172 MOCK_CONST_METHOD0(transformation, glm::mat4 const&());
173 MOCK_CONST_METHOD1(should_be_rendered_in, bool(geometry::Rectangle const&));
174+ MOCK_CONST_METHOD0(shaped, bool());
175 };
176
177 }
178
179=== modified file 'include/test/mir_test_doubles/mock_surface_state.h'
180--- include/test/mir_test_doubles/mock_surface_state.h 2013-10-03 06:01:11 +0000
181+++ include/test/mir_test_doubles/mock_surface_state.h 2013-10-07 10:02:32 +0000
182@@ -60,6 +60,7 @@
183 MOCK_METHOD0(frame_posted, void());
184 MOCK_METHOD1(set_hidden, void(bool));
185 MOCK_CONST_METHOD1(should_be_rendered_in, bool(geometry::Rectangle const&));
186+ MOCK_CONST_METHOD0(shaped, bool());
187 };
188
189 typedef ::testing::NiceMock<MockSurfaceState> StubSurfaceState;
190
191=== modified file 'include/test/mir_test_doubles/stub_compositing_criteria.h'
192--- include/test/mir_test_doubles/stub_compositing_criteria.h 2013-08-13 07:07:12 +0000
193+++ include/test/mir_test_doubles/stub_compositing_criteria.h 2013-10-07 10:02:32 +0000
194@@ -34,9 +34,11 @@
195 {
196 public:
197 StubCompositingCriteria(int x, int y, int width, int height,
198- float opacity=1.0f)
199+ float opacity=1.0f,
200+ bool rectangular=true)
201 : rect{{x, y}, {width, height}},
202- opacity(opacity)
203+ opacity(opacity),
204+ rectangular(rectangular)
205 {
206 const glm::mat4 ident;
207 glm::vec3 size(width, height, 0.0f);
208@@ -59,10 +61,16 @@
209 return rect.overlaps(r);
210 }
211
212+ bool shaped() const override
213+ {
214+ return !rectangular;
215+ }
216+
217 private:
218 mir::geometry::Rectangle rect;
219 glm::mat4 trans;
220 float opacity;
221+ bool rectangular;
222 };
223
224 } // namespace doubles
225
226=== modified file 'src/server/compositor/bypass.cpp'
227--- src/server/compositor/bypass.cpp 2013-08-13 07:14:11 +0000
228+++ src/server/compositor/bypass.cpp 2013-10-07 10:02:32 +0000
229@@ -57,7 +57,7 @@
230
231 bool BypassFilter::operator()(const CompositingCriteria &criteria)
232 {
233- if (criteria.alpha() != 1.0f)
234+ if (criteria.alpha() != 1.0f || criteria.shaped())
235 return false;
236
237 if (!all_orthogonal)
238
239=== modified file 'src/server/surfaces/surface_allocator.cpp'
240--- src/server/surfaces/surface_allocator.cpp 2013-10-03 03:44:08 +0000
241+++ src/server/surfaces/surface_allocator.cpp 2013-10-07 10:02:32 +0000
242@@ -46,9 +46,12 @@
243 params.buffer_usage};
244 auto buffer_stream = buffer_stream_factory->create_buffer_stream(buffer_properties);
245 auto actual_size = geom::Rectangle{params.top_left, buffer_stream->stream_size()};
246+
247+ bool nonrectangular = has_alpha(params.pixel_format);
248 auto state = std::make_shared<ms::SurfaceData>(params.name,
249 actual_size,
250- change_callback);
251+ change_callback,
252+ nonrectangular);
253 auto input_channel = input_factory->make_input_channel();
254 return std::make_shared<ms::Surface>(state, buffer_stream, input_channel);
255 }
256
257=== modified file 'src/server/surfaces/surface_data.cpp'
258--- src/server/surfaces/surface_data.cpp 2013-10-03 03:44:08 +0000
259+++ src/server/surfaces/surface_data.cpp 2013-10-07 10:02:32 +0000
260@@ -22,7 +22,7 @@
261 namespace geom=mir::geometry;
262 namespace ms = mir::surfaces;
263
264-ms::SurfaceData::SurfaceData(std::string const& name, geom::Rectangle rect, std::function<void()> change_cb)
265+ms::SurfaceData::SurfaceData(std::string const& name, geom::Rectangle rect, std::function<void()> change_cb, bool nonrectangular)
266 : notify_change(change_cb),
267 surface_name(name),
268 surface_rect(rect),
269@@ -30,6 +30,7 @@
270 surface_alpha(1.0f),
271 first_frame_posted(false),
272 hidden(false),
273+ nonrectangular(nonrectangular),
274 input_rectangles{surface_rect}
275 {
276 }
277@@ -91,6 +92,11 @@
278 return rect.overlaps(surface_rect);
279 }
280
281+bool ms::SurfaceData::shaped() const
282+{
283+ return nonrectangular;
284+}
285+
286 void ms::SurfaceData::apply_alpha(float alpha)
287 {
288 {
289
290=== modified file 'src/server/surfaces/surface_data.h'
291--- src/server/surfaces/surface_data.h 2013-10-03 08:42:27 +0000
292+++ src/server/surfaces/surface_data.h 2013-10-07 10:02:32 +0000
293@@ -34,12 +34,14 @@
294 {
295 public:
296 SurfaceData(std::string const& name, geometry::Rectangle rect,
297- std::function<void()> change_cb);
298+ std::function<void()> change_cb,
299+ bool nonrectangular);
300
301 //mc::CompositingCriteria
302 glm::mat4 const& transformation() const;
303 float alpha() const;
304 bool should_be_rendered_in(geometry::Rectangle const& rect) const;
305+ bool shaped() const override;
306
307 //mi::Surface
308 std::string const& name() const;
309@@ -67,6 +69,7 @@
310 float surface_alpha;
311 bool first_frame_posted;
312 bool hidden;
313+ const bool nonrectangular;
314 std::vector<geometry::Rectangle> input_rectangles;
315 };
316
317
318=== modified file 'tests/unit-tests/compositor/test_bypass.cpp'
319--- tests/unit-tests/compositor/test_bypass.cpp 2013-08-13 07:18:22 +0000
320+++ tests/unit-tests/compositor/test_bypass.cpp 2013-10-07 10:02:32 +0000
321@@ -88,6 +88,16 @@
322 EXPECT_FALSE(filter.fullscreen_on_top());
323 }
324
325+TEST_F(BypassFilterTest, shaped_fullscreen_window_not_bypassed)
326+{
327+ BypassFilter filter(display_buffer[0]);
328+
329+ StubCompositingCriteria win(0, 0, 1920, 1200, 1.0f, false);
330+
331+ EXPECT_FALSE(filter(win));
332+ EXPECT_FALSE(filter.fullscreen_on_top());
333+}
334+
335 TEST_F(BypassFilterTest, offset_fullscreen_window_not_bypassed)
336 {
337 BypassFilter filter(display_buffer[0]);
338@@ -167,6 +177,39 @@
339 EXPECT_TRUE(filter.fullscreen_on_top());
340 }
341
342+TEST_F(BypassFilterTest, many_fullscreen_windows_only_bypass_top_rectangular)
343+{
344+ BypassFilter filter(display_buffer[0]);
345+
346+ StubCompositingCriteria a(0, 0, 1920, 1200, 1.0f, false);
347+ EXPECT_FALSE(filter(a));
348+ EXPECT_FALSE(filter.fullscreen_on_top());
349+
350+ StubCompositingCriteria b(1, 2, 3, 4);
351+ EXPECT_FALSE(filter(b));
352+ EXPECT_FALSE(filter.fullscreen_on_top());
353+
354+ StubCompositingCriteria c(0, 0, 1920, 1200);
355+ EXPECT_TRUE(filter(c));
356+ EXPECT_TRUE(filter.fullscreen_on_top());
357+
358+ StubCompositingCriteria d(5, 6, 7, 8);
359+ EXPECT_FALSE(filter(d));
360+ EXPECT_FALSE(filter.fullscreen_on_top());
361+
362+ StubCompositingCriteria e(0, 0, 1920, 1200, 1.0f, true);
363+ EXPECT_TRUE(filter(e));
364+ EXPECT_TRUE(filter.fullscreen_on_top());
365+
366+ StubCompositingCriteria f(9, 10, 11, 12);
367+ EXPECT_FALSE(filter(f));
368+ EXPECT_FALSE(filter.fullscreen_on_top());
369+
370+ StubCompositingCriteria g(0, 0, 1920, 1200, 0.5f, false);
371+ EXPECT_FALSE(filter(g));
372+ EXPECT_FALSE(filter.fullscreen_on_top());
373+}
374+
375 TEST_F(BypassFilterTest, multimonitor_one_bypassed)
376 {
377 BypassFilter left(display_buffer[0]);
378
379=== modified file 'tests/unit-tests/surfaces/test_surface_data.cpp'
380--- tests/unit-tests/surfaces/test_surface_data.cpp 2013-10-03 03:44:08 +0000
381+++ tests/unit-tests/surfaces/test_surface_data.cpp 2013-10-07 10:02:32 +0000
382@@ -67,10 +67,11 @@
383
384 TEST_F(SurfaceDataTest, basics)
385 {
386- ms::SurfaceData data{name, rect, null_change_cb};
387+ ms::SurfaceData data{name, rect, null_change_cb, false};
388 EXPECT_EQ(name, data.name());
389 EXPECT_EQ(rect.size, data.size());
390 EXPECT_EQ(rect.top_left, data.position());
391+ EXPECT_FALSE(data.shaped());
392 }
393
394 TEST_F(SurfaceDataTest, update_position)
395@@ -78,7 +79,7 @@
396 EXPECT_CALL(mock_callback, call())
397 .Times(1);
398
399- ms::SurfaceData storage{name, rect, mock_change_cb};
400+ ms::SurfaceData storage{name, rect, mock_change_cb, false};
401 EXPECT_EQ(rect.top_left, storage.position());
402
403 auto new_top_left = geom::Point{geom::X{6}, geom::Y{10}};
404@@ -91,7 +92,7 @@
405 EXPECT_CALL(mock_callback, call())
406 .Times(1);
407
408- ms::SurfaceData storage{name, rect, mock_change_cb};
409+ ms::SurfaceData storage{name, rect, mock_change_cb, false};
410 auto original_transformation = storage.transformation();
411
412 storage.apply_rotation(60.0f, glm::vec3{0.0f, 0.0f, 1.0f});
413@@ -106,7 +107,7 @@
414 const geom::Size sz{geom::Width{85}, geom::Height{43}};
415 const geom::Rectangle origin{geom::Point{geom::X{77}, geom::Y{88}}, sz};
416 const geom::Rectangle moved_pt{geom::Point{geom::X{55}, geom::Y{66}}, sz};
417- ms::SurfaceData storage{name, origin, null_change_cb};
418+ ms::SurfaceData storage{name, origin, null_change_cb, false};
419
420 glm::mat4 t0 = storage.transformation();
421 storage.move_to(moved_pt.top_left);
422@@ -125,7 +126,7 @@
423 EXPECT_CALL(mock_callback, call())
424 .Times(1);
425
426- ms::SurfaceData surface_state{name, rect, mock_change_cb};
427+ ms::SurfaceData surface_state{name, rect, mock_change_cb, false};
428
429 float alpha = 0.5f;
430 surface_state.apply_alpha(0.5f);
431@@ -135,8 +136,9 @@
432 TEST_F(SurfaceDataTest, test_surface_is_opaque_by_default)
433 {
434 using namespace testing;
435- ms::SurfaceData surface_state{name, rect, null_change_cb};
436+ ms::SurfaceData surface_state{name, rect, null_change_cb, false};
437 EXPECT_THAT(1.0f, FloatEq(surface_state.alpha()));
438+ EXPECT_FALSE(surface_state.shaped());
439 }
440
441 TEST_F(SurfaceDataTest, test_surface_apply_rotation)
442@@ -144,13 +146,13 @@
443 EXPECT_CALL(mock_callback, call())
444 .Times(1);
445
446- ms::SurfaceData surface_state{name, rect, mock_change_cb};
447+ ms::SurfaceData surface_state{name, rect, mock_change_cb, false};
448 surface_state.apply_rotation(60.0f, glm::vec3{0.0f, 0.0f, 1.0f});
449 }
450
451 TEST_F(SurfaceDataTest, test_surface_should_be_rendered_in)
452 {
453- ms::SurfaceData surface_state{name, rect, mock_change_cb};
454+ ms::SurfaceData surface_state{name, rect, mock_change_cb, false};
455 geom::Rectangle output_rect{geom::Point{0,0}, geom::Size{100, 100}};
456
457 //not renderable by default
458@@ -179,7 +181,7 @@
459 EXPECT_CALL(mock_callback, call())
460 .Times(1);
461
462- ms::SurfaceData surface_state{name, rect, mock_change_cb};
463+ ms::SurfaceData surface_state{name, rect, mock_change_cb, false};
464 surface_state.set_hidden(true);
465 }
466
467@@ -189,7 +191,7 @@
468 EXPECT_CALL(mock_callback, call())
469 .Times(1);
470
471- ms::SurfaceData surface_state{name, rect, mock_change_cb};
472+ ms::SurfaceData surface_state{name, rect, mock_change_cb, false};
473 surface_state.frame_posted();
474 }
475
476@@ -198,7 +200,7 @@
477 {
478 geom::Point pt(1,1);
479 geom::Size one_by_one{geom::Width{1}, geom::Height{1}};
480- ms::SurfaceData surface_state{name, geom::Rectangle{pt, one_by_one}, mock_change_cb};
481+ ms::SurfaceData surface_state{name, geom::Rectangle{pt, one_by_one}, mock_change_cb, false};
482
483 std::vector<geom::Point> contained_pt
484 {
485@@ -230,7 +232,7 @@
486 {{geom::X{1}, geom::Y{1}}, {geom::Width{1}, geom::Height{1}}} //region1
487 };
488
489- ms::SurfaceData surface_state{name, rect, mock_change_cb};
490+ ms::SurfaceData surface_state{name, rect, mock_change_cb, false};
491 surface_state.set_input_region(rectangles);
492
493 std::vector<geom::Point> contained_pt

Subscribers

People subscribed via source and target branches