Mir

Merge lp:~alan-griffiths/mir/fix-1506846 into lp:mir

  1. fix-1506846
  2. Merge into development-branch
Proposed by Alan Griffiths
Status: Superseded
Proposed branch: lp:~alan-griffiths/mir/fix-1506846
Merge into: lp:mir
Diff against target: 206 lines (+63/-13)
7 files modified
include/server/mir/shell/display_layout.h (+4/-2)
include/server/mir/shell/system_compositor_window_manager.h (+9/-0)
src/server/shell/CMakeLists.txt (+2/-0)
src/server/shell/graphics_display_layout.cpp (+2/-6)
src/server/shell/graphics_display_layout.h (+4/-4)
src/server/shell/system_compositor_window_manager.cpp (+41/-0)
tests/acceptance-tests/test_system_compositor_window_manager.cpp (+1/-1)
To merge this branch: bzr merge lp:~alan-griffiths/mir/fix-1506846
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Needs Fixing
Alexandros Frantzis (community) Needs Information
Review via email: mp+277022@code.launchpad.net

This proposal has been superseded by a proposal from 2015-11-10.

Commit message

shell: system compositors should apply nested display configurations without waiting for buffers to be submitted

Description of the change

shell: system compositors should apply nested display configurations without waiting for buffers to be submitted

To post a comment you must log in.
Revision history for this message
Alexandros Frantzis (afrantzis) wrote :

I think this will break the phone startup by showing the unity8 session too early, while it is still blank. We could fix this by reintroducing similar logic as the one removed (i.e. a SurfaceReadyObserver) in USC's window manager.

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

> I think this will break the phone startup by showing the unity8 session too
> early, while it is still blank.

I see your point but...

> We could fix this by reintroducing similar
> logic as the one removed (i.e. a SurfaceReadyObserver) in USC's window
> manager.

...that would then break this fix.

Revision history for this message
Alexandros Frantzis (afrantzis) wrote :

... and this also brings up a few questions: What does it mean for a session to be ready? Does it make sense for readiness to differ depending on the WM?

review: Needs Information
Revision history for this message
Alexandros Frantzis (afrantzis) wrote :

> > We could fix this by reintroducing similar
> > logic as the one removed (i.e. a SurfaceReadyObserver) in USC's window
> > manager.
>
> ...that would then break this fix.

Ah, correct... The purpose of this fix is to focus the session early so its display config is applied.

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

Alternatively, what does "fullscreen_on_output" mean for a session that has set a display configuration that changes that output?

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

FAILED: Continuous integration, rev:3093
http://jenkins.qa.ubuntu.com/job/mir-ci/5529/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/4835
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/3742
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/4777
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-wily-touch/735/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/1683/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-i386-ci/735
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-i386-ci/735/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/4778
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/4778/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-touch/7378
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/25018
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-wily-armhf/736/console

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

review: Needs Fixing (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/display_layout.h'
2--- include/server/mir/shell/display_layout.h 2013-08-28 03:41:48 +0000
3+++ include/server/mir/shell/display_layout.h 2015-11-10 09:17:12 +0000
4@@ -53,12 +53,14 @@
5 virtual void size_to_output(geometry::Rectangle& rect) = 0;
6
7 /**
8- * Places a rectangle in an particular output.
9+ * Places a rectangle in an particular output if the display is known,
10+ * otherwise does nothing.
11 *
12 * @param [in] id the id of the output to place the rectangle in
13 * @param [in,out] rect the rectangle to place
14+ * @return true iff the display id is recognised
15 */
16- virtual void place_in_output(graphics::DisplayConfigurationOutputId id,
17+ virtual bool place_in_output(graphics::DisplayConfigurationOutputId id,
18 geometry::Rectangle& rect) = 0;
19
20 protected:
21
22=== modified file 'include/server/mir/shell/system_compositor_window_manager.h'
23--- include/server/mir/shell/system_compositor_window_manager.h 2015-10-29 22:07:47 +0000
24+++ include/server/mir/shell/system_compositor_window_manager.h 2015-11-10 09:17:12 +0000
25@@ -20,6 +20,10 @@
26 #define MIR_SHELL_SYSTEM_COMPOSITOR_WINDOW_MANAGER_H_
27
28 #include "mir/shell/window_manager.h"
29+#include "mir/graphics/display_configuration.h"
30+
31+#include <map>
32+#include <mutex>
33
34 namespace mir
35 {
36@@ -96,6 +100,11 @@
37 std::shared_ptr<scene::Surface> const& surface,
38 MirSurfaceAttrib attrib,
39 int value) override;
40+
41+ using OutputMap = std::map<std::weak_ptr<scene::Surface>, graphics::DisplayConfigurationOutputId, std::owner_less<std::weak_ptr<scene::Surface>>>;
42+
43+ std::mutex mutable mutex;
44+ OutputMap output_map;
45 };
46 }
47 }
48
49=== modified file 'src/server/shell/CMakeLists.txt'
50--- src/server/shell/CMakeLists.txt 2015-10-29 02:08:24 +0000
51+++ src/server/shell/CMakeLists.txt 2015-11-10 09:17:12 +0000
52@@ -5,7 +5,9 @@
53 canonical_window_manager.cpp
54 frontend_shell.cpp
55 graphics_display_layout.cpp
56+ graphics_display_layout.h
57 default_configuration.cpp
58+ null_host_lifecycle_event_listener.h
59 shell_wrapper.cpp
60 surface_ready_observer.cpp
61 system_compositor_window_manager.cpp
62
63=== modified file 'src/server/shell/graphics_display_layout.cpp'
64--- src/server/shell/graphics_display_layout.cpp 2015-04-28 07:54:10 +0000
65+++ src/server/shell/graphics_display_layout.cpp 2015-11-10 09:17:12 +0000
66@@ -24,9 +24,6 @@
67 #include "mir/geometry/rectangles.h"
68 #include "mir/geometry/displacement.h"
69
70-#include <boost/throw_exception.hpp>
71-#include <stdexcept>
72-
73 namespace msh = mir::shell;
74 namespace mg = mir::graphics;
75 namespace geom = mir::geometry;
76@@ -68,7 +65,7 @@
77 rect = output;
78 }
79
80-void msh::GraphicsDisplayLayout::place_in_output(
81+bool msh::GraphicsDisplayLayout::place_in_output(
82 graphics::DisplayConfigurationOutputId id,
83 geometry::Rectangle& rect)
84 {
85@@ -87,8 +84,7 @@
86 }
87 });
88
89- if (!placed)
90- BOOST_THROW_EXCEPTION(std::runtime_error("Failed to place surface in requested output"));
91+ return placed;
92 }
93
94 geom::Rectangle msh::GraphicsDisplayLayout::get_output_for(geometry::Rectangle& rect)
95
96=== modified file 'src/server/shell/graphics_display_layout.h'
97--- src/server/shell/graphics_display_layout.h 2014-03-06 06:05:17 +0000
98+++ src/server/shell/graphics_display_layout.h 2015-11-10 09:17:12 +0000
99@@ -37,10 +37,10 @@
100 public:
101 GraphicsDisplayLayout(std::shared_ptr<graphics::Display> const& display);
102
103- void clip_to_output(geometry::Rectangle& rect);
104- void size_to_output(geometry::Rectangle& rect);
105- void place_in_output(graphics::DisplayConfigurationOutputId output_id,
106- geometry::Rectangle& rect);
107+ void clip_to_output(geometry::Rectangle& rect) override;
108+ void size_to_output(geometry::Rectangle& rect) override;
109+ bool place_in_output(graphics::DisplayConfigurationOutputId output_id,
110+ geometry::Rectangle& rect) override;
111
112 private:
113 geometry::Rectangle get_output_for(geometry::Rectangle& rect);
114
115=== modified file 'src/server/shell/system_compositor_window_manager.cpp'
116--- src/server/shell/system_compositor_window_manager.cpp 2015-10-29 22:07:47 +0000
117+++ src/server/shell/system_compositor_window_manager.cpp 2015-11-10 09:17:12 +0000
118@@ -28,6 +28,8 @@
119 #include "mir/scene/surface.h"
120 #include "mir/scene/surface_creation_parameters.h"
121
122+#include <boost/throw_exception.hpp>
123+
124 namespace mf = mir::frontend;
125 namespace ms = mir::scene;
126 namespace msh = mir::shell;
127@@ -60,6 +62,9 @@
128 {
129 mir::geometry::Rectangle rect{params.top_left, params.size};
130
131+ if (!params.output_id.as_value())
132+ BOOST_THROW_EXCEPTION(std::runtime_error("An output ID must be specified"));
133+
134 display_layout->place_in_output(params.output_id, rect);
135
136 auto placed_parameters = params;
137@@ -78,6 +83,9 @@
138 surface);
139
140 surface->add_observer(session_ready_observer);
141+
142+ std::lock_guard<decltype(mutex)> lock{mutex};
143+ output_map[surface] = params.output_id;
144
145 return result;
146 }
147@@ -89,6 +97,22 @@
148 {
149 if (modifications.name.is_set())
150 surface->rename(modifications.name.value());
151+
152+ if (modifications.output_id.is_set())
153+ {
154+ auto const output_id = modifications.output_id.value();
155+
156+ mir::geometry::Rectangle rect{surface->top_left(), surface->size()};
157+
158+ if (display_layout->place_in_output(output_id, rect))
159+ {
160+ surface->move_to(rect.top_left);
161+ surface->resize(rect.size);
162+ }
163+
164+ std::lock_guard<decltype(mutex)> lock{mutex};
165+ output_map[surface] = output_id;
166+ }
167 }
168
169 void msh::SystemCompositorWindowManager::remove_surface(
170@@ -99,6 +123,23 @@
171
172 void msh::SystemCompositorWindowManager::add_display(mir::geometry::Rectangle const& /*area*/)
173 {
174+ std::lock_guard<decltype(mutex)> lock{mutex};
175+
176+ for (auto const& so : output_map)
177+ {
178+ if (auto surface = so.first.lock())
179+ {
180+ auto const output_id = so.second;
181+
182+ mir::geometry::Rectangle rect{surface->top_left(), surface->size()};
183+
184+ if (display_layout->place_in_output(output_id, rect))
185+ {
186+ surface->move_to(rect.top_left);
187+ surface->resize(rect.size);
188+ }
189+ }
190+ }
191 }
192
193 void msh::SystemCompositorWindowManager::remove_display(mir::geometry::Rectangle const& /*area*/)
194
195=== modified file 'tests/acceptance-tests/test_system_compositor_window_manager.cpp'
196--- tests/acceptance-tests/test_system_compositor_window_manager.cpp 2015-10-02 04:27:35 +0000
197+++ tests/acceptance-tests/test_system_compositor_window_manager.cpp 2015-11-10 09:17:12 +0000
198@@ -167,7 +167,7 @@
199 auto surface = client.create_surface(0);
200
201 EXPECT_FALSE(mir_surface_is_valid(surface));
202- EXPECT_THAT(mir_surface_get_error_message(surface), HasSubstr("Failed to place surface"));
203+ EXPECT_THAT(mir_surface_get_error_message(surface), HasSubstr("An output ID must be specified"));
204 }
205
206 TEST_F(SystemCompositorWindowManager, if_a_surface_posts_client_gets_focus)

Subscribers

People subscribed via source and target branches