Merge lp:~alan-griffiths/mir/fix-1506846 into lp:mir
- fix-1506846
- Merge into development-branch
Status: | Merged | ||||||||
---|---|---|---|---|---|---|---|---|---|
Merged at revision: | 3103 | ||||||||
Proposed branch: | lp:~alan-griffiths/mir/fix-1506846 | ||||||||
Merge into: | lp:mir | ||||||||
Diff against target: |
211 lines (+65/-14) 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 (+43/-1) tests/acceptance-tests/test_system_compositor_window_manager.cpp (+1/-1) |
||||||||
To merge this branch: | bzr merge lp:~alan-griffiths/mir/fix-1506846 | ||||||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
PS Jenkins bot (community) | continuous-integration | Needs Fixing | |
Alexandros Frantzis (community) | Approve | ||
Review via email: mp+277092@code.launchpad.net |
This proposal supersedes a proposal from 2015-11-10.
Commit message
shell: system compositors should update fullscreen windows when the output is reconfigured.
Description of the change
shell: system compositors should update fullscreen windows when the output is reconfigured.
This avoids the problem seen in lp:1506846 of the nested servers surfaces being placed according to a display configuration that has become obsolete.
Alexandros Frantzis (afrantzis) wrote : Posted in a previous version of this proposal | # |
Alan Griffiths (alan-griffiths) wrote : Posted in a previous version of this proposal | # |
> 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 SurfaceReadyObs
> manager.
...that would then break this fix.
Alexandros Frantzis (afrantzis) wrote : Posted in a previous version of this proposal | # |
... 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?
Alexandros Frantzis (afrantzis) wrote : Posted in a previous version of this proposal | # |
> > We could fix this by reintroducing similar
> > logic as the one removed (i.e. a SurfaceReadyObs
> > 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.
Alan Griffiths (alan-griffiths) wrote : Posted in a previous version of this proposal | # |
Alternatively, what does "fullscreen_
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal | # |
FAILED: Continuous integration, rev:3093
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
FAILURE: http://
FAILURE: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:3097
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
FAILURE: http://
FAILURE: http://
SUCCESS: http://
deb: http://
None: http://
SUCCESS: http://
deb: http://
FAILURE: http://
SUCCESS: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:3098
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
FAILURE: http://
FAILURE: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
deb: http://
FAILURE: http://
SUCCESS: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
Alexandros Frantzis (afrantzis) wrote : | # |
Looks good.
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:3099
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
FAILURE: http://
None: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
deb: http://
None: http://
Click here to trigger a rebuild:
http://
Alan Griffiths (alan-griffiths) wrote : | # |
Can't access http://
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Autolanding.
More details in the following jenkins job:
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
FAILURE: http://
FAILURE: http://
SUCCESS: http://
deb: http://
FAILURE: http://
SUCCESS: http://
deb: http://
FAILURE: http://
Preview Diff
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-12 18:48:46 +0000 | |||
4 | @@ -53,12 +53,14 @@ | |||
5 | 53 | virtual void size_to_output(geometry::Rectangle& rect) = 0; | 53 | virtual void size_to_output(geometry::Rectangle& rect) = 0; |
6 | 54 | 54 | ||
7 | 55 | /** | 55 | /** |
9 | 56 | * Places a rectangle in an particular output. | 56 | * Places a rectangle in an particular output if the display is known, |
10 | 57 | * otherwise does nothing. | ||
11 | 57 | * | 58 | * |
12 | 58 | * @param [in] id the id of the output to place the rectangle in | 59 | * @param [in] id the id of the output to place the rectangle in |
13 | 59 | * @param [in,out] rect the rectangle to place | 60 | * @param [in,out] rect the rectangle to place |
14 | 61 | * @return true iff the display id is recognised | ||
15 | 60 | */ | 62 | */ |
17 | 61 | virtual void place_in_output(graphics::DisplayConfigurationOutputId id, | 63 | virtual bool place_in_output(graphics::DisplayConfigurationOutputId id, |
18 | 62 | geometry::Rectangle& rect) = 0; | 64 | geometry::Rectangle& rect) = 0; |
19 | 63 | 65 | ||
20 | 64 | protected: | 66 | protected: |
21 | 65 | 67 | ||
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-12 18:48:46 +0000 | |||
25 | @@ -20,6 +20,10 @@ | |||
26 | 20 | #define MIR_SHELL_SYSTEM_COMPOSITOR_WINDOW_MANAGER_H_ | 20 | #define MIR_SHELL_SYSTEM_COMPOSITOR_WINDOW_MANAGER_H_ |
27 | 21 | 21 | ||
28 | 22 | #include "mir/shell/window_manager.h" | 22 | #include "mir/shell/window_manager.h" |
29 | 23 | #include "mir/graphics/display_configuration.h" | ||
30 | 24 | |||
31 | 25 | #include <map> | ||
32 | 26 | #include <mutex> | ||
33 | 23 | 27 | ||
34 | 24 | namespace mir | 28 | namespace mir |
35 | 25 | { | 29 | { |
36 | @@ -96,6 +100,11 @@ | |||
37 | 96 | std::shared_ptr<scene::Surface> const& surface, | 100 | std::shared_ptr<scene::Surface> const& surface, |
38 | 97 | MirSurfaceAttrib attrib, | 101 | MirSurfaceAttrib attrib, |
39 | 98 | int value) override; | 102 | int value) override; |
40 | 103 | |||
41 | 104 | using OutputMap = std::map<std::weak_ptr<scene::Surface>, graphics::DisplayConfigurationOutputId, std::owner_less<std::weak_ptr<scene::Surface>>>; | ||
42 | 105 | |||
43 | 106 | std::mutex mutable mutex; | ||
44 | 107 | OutputMap output_map; | ||
45 | 99 | }; | 108 | }; |
46 | 100 | } | 109 | } |
47 | 101 | } | 110 | } |
48 | 102 | 111 | ||
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-12 18:48:46 +0000 | |||
52 | @@ -5,7 +5,9 @@ | |||
53 | 5 | canonical_window_manager.cpp | 5 | canonical_window_manager.cpp |
54 | 6 | frontend_shell.cpp | 6 | frontend_shell.cpp |
55 | 7 | graphics_display_layout.cpp | 7 | graphics_display_layout.cpp |
56 | 8 | graphics_display_layout.h | ||
57 | 8 | default_configuration.cpp | 9 | default_configuration.cpp |
58 | 10 | null_host_lifecycle_event_listener.h | ||
59 | 9 | shell_wrapper.cpp | 11 | shell_wrapper.cpp |
60 | 10 | surface_ready_observer.cpp | 12 | surface_ready_observer.cpp |
61 | 11 | system_compositor_window_manager.cpp | 13 | system_compositor_window_manager.cpp |
62 | 12 | 14 | ||
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-12 18:48:46 +0000 | |||
66 | @@ -24,9 +24,6 @@ | |||
67 | 24 | #include "mir/geometry/rectangles.h" | 24 | #include "mir/geometry/rectangles.h" |
68 | 25 | #include "mir/geometry/displacement.h" | 25 | #include "mir/geometry/displacement.h" |
69 | 26 | 26 | ||
70 | 27 | #include <boost/throw_exception.hpp> | ||
71 | 28 | #include <stdexcept> | ||
72 | 29 | |||
73 | 30 | namespace msh = mir::shell; | 27 | namespace msh = mir::shell; |
74 | 31 | namespace mg = mir::graphics; | 28 | namespace mg = mir::graphics; |
75 | 32 | namespace geom = mir::geometry; | 29 | namespace geom = mir::geometry; |
76 | @@ -68,7 +65,7 @@ | |||
77 | 68 | rect = output; | 65 | rect = output; |
78 | 69 | } | 66 | } |
79 | 70 | 67 | ||
81 | 71 | void msh::GraphicsDisplayLayout::place_in_output( | 68 | bool msh::GraphicsDisplayLayout::place_in_output( |
82 | 72 | graphics::DisplayConfigurationOutputId id, | 69 | graphics::DisplayConfigurationOutputId id, |
83 | 73 | geometry::Rectangle& rect) | 70 | geometry::Rectangle& rect) |
84 | 74 | { | 71 | { |
85 | @@ -87,8 +84,7 @@ | |||
86 | 87 | } | 84 | } |
87 | 88 | }); | 85 | }); |
88 | 89 | 86 | ||
91 | 90 | if (!placed) | 87 | return placed; |
90 | 91 | BOOST_THROW_EXCEPTION(std::runtime_error("Failed to place surface in requested output")); | ||
92 | 92 | } | 88 | } |
93 | 93 | 89 | ||
94 | 94 | geom::Rectangle msh::GraphicsDisplayLayout::get_output_for(geometry::Rectangle& rect) | 90 | geom::Rectangle msh::GraphicsDisplayLayout::get_output_for(geometry::Rectangle& rect) |
95 | 95 | 91 | ||
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-12 18:48:46 +0000 | |||
99 | @@ -37,10 +37,10 @@ | |||
100 | 37 | public: | 37 | public: |
101 | 38 | GraphicsDisplayLayout(std::shared_ptr<graphics::Display> const& display); | 38 | GraphicsDisplayLayout(std::shared_ptr<graphics::Display> const& display); |
102 | 39 | 39 | ||
107 | 40 | void clip_to_output(geometry::Rectangle& rect); | 40 | void clip_to_output(geometry::Rectangle& rect) override; |
108 | 41 | void size_to_output(geometry::Rectangle& rect); | 41 | void size_to_output(geometry::Rectangle& rect) override; |
109 | 42 | void place_in_output(graphics::DisplayConfigurationOutputId output_id, | 42 | bool place_in_output(graphics::DisplayConfigurationOutputId output_id, |
110 | 43 | geometry::Rectangle& rect); | 43 | geometry::Rectangle& rect) override; |
111 | 44 | 44 | ||
112 | 45 | private: | 45 | private: |
113 | 46 | geometry::Rectangle get_output_for(geometry::Rectangle& rect); | 46 | geometry::Rectangle get_output_for(geometry::Rectangle& rect); |
114 | 47 | 47 | ||
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-12 18:48:46 +0000 | |||
118 | @@ -28,6 +28,8 @@ | |||
119 | 28 | #include "mir/scene/surface.h" | 28 | #include "mir/scene/surface.h" |
120 | 29 | #include "mir/scene/surface_creation_parameters.h" | 29 | #include "mir/scene/surface_creation_parameters.h" |
121 | 30 | 30 | ||
122 | 31 | #include <boost/throw_exception.hpp> | ||
123 | 32 | |||
124 | 31 | namespace mf = mir::frontend; | 33 | namespace mf = mir::frontend; |
125 | 32 | namespace ms = mir::scene; | 34 | namespace ms = mir::scene; |
126 | 33 | namespace msh = mir::shell; | 35 | namespace msh = mir::shell; |
127 | @@ -60,6 +62,9 @@ | |||
128 | 60 | { | 62 | { |
129 | 61 | mir::geometry::Rectangle rect{params.top_left, params.size}; | 63 | mir::geometry::Rectangle rect{params.top_left, params.size}; |
130 | 62 | 64 | ||
131 | 65 | if (!params.output_id.as_value()) | ||
132 | 66 | BOOST_THROW_EXCEPTION(std::runtime_error("An output ID must be specified")); | ||
133 | 67 | |||
134 | 63 | display_layout->place_in_output(params.output_id, rect); | 68 | display_layout->place_in_output(params.output_id, rect); |
135 | 64 | 69 | ||
136 | 65 | auto placed_parameters = params; | 70 | auto placed_parameters = params; |
137 | @@ -78,6 +83,9 @@ | |||
138 | 78 | surface); | 83 | surface); |
139 | 79 | 84 | ||
140 | 80 | surface->add_observer(session_ready_observer); | 85 | surface->add_observer(session_ready_observer); |
141 | 86 | |||
142 | 87 | std::lock_guard<decltype(mutex)> lock{mutex}; | ||
143 | 88 | output_map[surface] = params.output_id; | ||
144 | 81 | 89 | ||
145 | 82 | return result; | 90 | return result; |
146 | 83 | } | 91 | } |
147 | @@ -89,16 +97,50 @@ | |||
148 | 89 | { | 97 | { |
149 | 90 | if (modifications.name.is_set()) | 98 | if (modifications.name.is_set()) |
150 | 91 | surface->rename(modifications.name.value()); | 99 | surface->rename(modifications.name.value()); |
151 | 100 | |||
152 | 101 | if (modifications.output_id.is_set()) | ||
153 | 102 | { | ||
154 | 103 | auto const output_id = modifications.output_id.value(); | ||
155 | 104 | |||
156 | 105 | mir::geometry::Rectangle rect{surface->top_left(), surface->size()}; | ||
157 | 106 | |||
158 | 107 | if (display_layout->place_in_output(output_id, rect)) | ||
159 | 108 | { | ||
160 | 109 | surface->move_to(rect.top_left); | ||
161 | 110 | surface->resize(rect.size); | ||
162 | 111 | } | ||
163 | 112 | |||
164 | 113 | std::lock_guard<decltype(mutex)> lock{mutex}; | ||
165 | 114 | output_map[surface] = output_id; | ||
166 | 115 | } | ||
167 | 92 | } | 116 | } |
168 | 93 | 117 | ||
169 | 94 | void msh::SystemCompositorWindowManager::remove_surface( | 118 | void msh::SystemCompositorWindowManager::remove_surface( |
170 | 95 | std::shared_ptr<ms::Session> const& /*session*/, | 119 | std::shared_ptr<ms::Session> const& /*session*/, |
172 | 96 | std::weak_ptr<ms::Surface> const& /*surface*/) | 120 | std::weak_ptr<ms::Surface> const& surface) |
173 | 97 | { | 121 | { |
174 | 122 | output_map.erase(surface); | ||
175 | 98 | } | 123 | } |
176 | 99 | 124 | ||
177 | 100 | void msh::SystemCompositorWindowManager::add_display(mir::geometry::Rectangle const& /*area*/) | 125 | void msh::SystemCompositorWindowManager::add_display(mir::geometry::Rectangle const& /*area*/) |
178 | 101 | { | 126 | { |
179 | 127 | std::lock_guard<decltype(mutex)> lock{mutex}; | ||
180 | 128 | |||
181 | 129 | for (auto const& so : output_map) | ||
182 | 130 | { | ||
183 | 131 | if (auto surface = so.first.lock()) | ||
184 | 132 | { | ||
185 | 133 | auto const output_id = so.second; | ||
186 | 134 | |||
187 | 135 | mir::geometry::Rectangle rect{surface->top_left(), surface->size()}; | ||
188 | 136 | |||
189 | 137 | if (display_layout->place_in_output(output_id, rect)) | ||
190 | 138 | { | ||
191 | 139 | surface->move_to(rect.top_left); | ||
192 | 140 | surface->resize(rect.size); | ||
193 | 141 | } | ||
194 | 142 | } | ||
195 | 143 | } | ||
196 | 102 | } | 144 | } |
197 | 103 | 145 | ||
198 | 104 | void msh::SystemCompositorWindowManager::remove_display(mir::geometry::Rectangle const& /*area*/) | 146 | void msh::SystemCompositorWindowManager::remove_display(mir::geometry::Rectangle const& /*area*/) |
199 | 105 | 147 | ||
200 | === modified file 'tests/acceptance-tests/test_system_compositor_window_manager.cpp' | |||
201 | --- tests/acceptance-tests/test_system_compositor_window_manager.cpp 2015-10-02 04:27:35 +0000 | |||
202 | +++ tests/acceptance-tests/test_system_compositor_window_manager.cpp 2015-11-12 18:48:46 +0000 | |||
203 | @@ -167,7 +167,7 @@ | |||
204 | 167 | auto surface = client.create_surface(0); | 167 | auto surface = client.create_surface(0); |
205 | 168 | 168 | ||
206 | 169 | EXPECT_FALSE(mir_surface_is_valid(surface)); | 169 | EXPECT_FALSE(mir_surface_is_valid(surface)); |
208 | 170 | EXPECT_THAT(mir_surface_get_error_message(surface), HasSubstr("Failed to place surface")); | 170 | EXPECT_THAT(mir_surface_get_error_message(surface), HasSubstr("An output ID must be specified")); |
209 | 171 | } | 171 | } |
210 | 172 | 172 | ||
211 | 173 | TEST_F(SystemCompositorWindowManager, if_a_surface_posts_client_gets_focus) | 173 | TEST_F(SystemCompositorWindowManager, if_a_surface_posts_client_gets_focus) |
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 SurfaceReadyObs erver) in USC's window manager.