Mir

Merge lp:~raof/mir/nested-one-surface-per-crtc into lp:mir

Proposed by Chris Halse Rogers
Status: Merged
Approved by: Chris Halse Rogers
Approved revision: no longer in the source branch.
Merged at revision: 1457
Proposed branch: lp:~raof/mir/nested-one-surface-per-crtc
Merge into: lp:mir
Diff against target: 278 lines (+54/-57)
7 files modified
include/platform/mir/graphics/overlapping_output_grouping.h (+3/-7)
src/platform/graphics/CMakeLists.txt (+1/-0)
src/platform/graphics/mesa/CMakeLists.txt (+0/-1)
src/platform/graphics/mesa/display.cpp (+1/-1)
src/platform/graphics/overlapping_output_grouping.cpp (+7/-8)
src/server/graphics/nested/nested_display.cpp (+39/-36)
tests/unit-tests/graphics/mesa/test_overlapping_output_grouping.cpp (+3/-4)
To merge this branch: bzr merge lp:~raof/mir/nested-one-surface-per-crtc
Reviewer Review Type Date Requested Status
Robert Carr (community) Approve
Kevin DuBois (community) Approve
PS Jenkins bot (community) continuous-integration Approve
Alan Griffiths Approve
Review via email: mp+209615@code.launchpad.net

Commit message

Use only a single MirSurface per distinct output in nested.

Otherwise for clone modes you get two surfaces, one entirely obscured by the other, and
the obscured surface rapidly (and correctly) blocks in eglSwapBuffers.

Fixes: https://bugs.launchpad.net/mir/+bug/1287282

Description of the change

Resolve FIXME in nested_output.cpp, which incidentally unbreaks
nested on multihead.

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

LGTM

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 :

okay

review: Approve
Revision history for this message
Robert Carr (robertcarr) wrote :

Yay LGTM.

Is it still a bug that one internal client blocking on swapping buffers will prevent another from swapping buffers.

review: Approve
Revision history for this message
Robert Carr (robertcarr) wrote :

I mean obviously this behavior still occurs due to the EGLDisplay mutex but is it acceptable, a bug, quirky behavior, etc.

Revision history for this message
Chris Halse Rogers (raof) wrote :

We'd need to have non-blocking eglSwapBuffers behaviour for that to work. This is something that we *might* want to do anyway...

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== renamed file 'src/platform/graphics/mesa/overlapping_output_grouping.h' => 'include/platform/mir/graphics/overlapping_output_grouping.h'
2--- src/platform/graphics/mesa/overlapping_output_grouping.h 2014-03-04 04:09:36 +0000
3+++ include/platform/mir/graphics/overlapping_output_grouping.h 2014-03-06 09:14:50 +0000
4@@ -16,8 +16,8 @@
5 * Authored by: Alexandros Frantzis <alexandros.frantzis@canonical.com>
6 */
7
8-#ifndef MIR_GRAPHICS_MESA_OVERLAPPING_OUTPUT_GROUPING_H_
9-#define MIR_GRAPHICS_MESA_OVERLAPPING_OUTPUT_GROUPING_H_
10+#ifndef MIR_GRAPHICS_OVERLAPPING_OUTPUT_GROUPING_H_
11+#define MIR_GRAPHICS_OVERLAPPING_OUTPUT_GROUPING_H_
12
13 #include <vector>
14 #include <functional>
15@@ -34,9 +34,6 @@
16 class DisplayConfiguration;
17 struct DisplayConfigurationOutput;
18
19-namespace mesa
20-{
21-
22 class OverlappingOutputGroup
23 {
24 public:
25@@ -67,6 +64,5 @@
26
27 }
28 }
29-}
30
31-#endif /* MIR_GRAPHICS_MESA_OVERLAPPING_OUTPUT_GROUPING_H_ */
32+#endif /* MIR_GRAPHICS_OVERLAPPING_OUTPUT_GROUPING_H_ */
33
34=== modified file 'src/platform/graphics/CMakeLists.txt'
35--- src/platform/graphics/CMakeLists.txt 2014-03-04 04:09:36 +0000
36+++ src/platform/graphics/CMakeLists.txt 2014-03-06 09:14:50 +0000
37@@ -10,6 +10,7 @@
38 display_configuration.cpp
39 buffer_basic.cpp
40 pixel_format_utils.cpp
41+ overlapping_output_grouping.cpp
42 )
43
44 add_library(
45
46=== modified file 'src/platform/graphics/mesa/CMakeLists.txt'
47--- src/platform/graphics/mesa/CMakeLists.txt 2014-03-05 02:07:13 +0000
48+++ src/platform/graphics/mesa/CMakeLists.txt 2014-03-06 09:14:50 +0000
49@@ -32,7 +32,6 @@
50 internal_native_surface.cpp
51 internal_client.cpp
52 drm_close_threadsafe.cpp
53- overlapping_output_grouping.cpp
54 native_platform.cpp
55 anonymous_shm_file.cpp
56 shm_buffer.cpp
57
58=== modified file 'src/platform/graphics/mesa/display.cpp'
59--- src/platform/graphics/mesa/display.cpp 2014-03-04 04:09:36 +0000
60+++ src/platform/graphics/mesa/display.cpp 2014-03-06 09:14:50 +0000
61@@ -24,7 +24,7 @@
62 #include "kms_output.h"
63 #include "kms_page_flipper.h"
64 #include "virtual_terminal.h"
65-#include "overlapping_output_grouping.h"
66+#include "mir/graphics/overlapping_output_grouping.h"
67
68 #include "mir/graphics/display_report.h"
69 #include "mir/graphics/gl_context.h"
70
71=== renamed file 'src/platform/graphics/mesa/overlapping_output_grouping.cpp' => 'src/platform/graphics/overlapping_output_grouping.cpp'
72--- src/platform/graphics/mesa/overlapping_output_grouping.cpp 2014-03-04 04:09:36 +0000
73+++ src/platform/graphics/overlapping_output_grouping.cpp 2014-03-06 09:14:50 +0000
74@@ -16,7 +16,7 @@
75 * Authored by: Alexandros Frantzis <alexandros.frantzis@canonical.com>
76 */
77
78-#include "overlapping_output_grouping.h"
79+#include "mir/graphics/overlapping_output_grouping.h"
80
81 #include "mir/graphics/display_configuration.h"
82 #include "mir/geometry/rectangle.h"
83@@ -25,7 +25,6 @@
84 #include <unordered_set>
85
86 namespace mg = mir::graphics;
87-namespace mgm = mir::graphics::mesa;
88 namespace geom = mir::geometry;
89
90 namespace
91@@ -33,7 +32,7 @@
92
93 struct DCOutputHash
94 {
95- size_t operator()(mg::DisplayConfigurationOutput const& o) const { return o.id.as_value(); };
96+ size_t operator()(mg::DisplayConfigurationOutput const& o) const { return o.id.as_value(); }
97 };
98
99 struct DCOutputEqual
100@@ -51,7 +50,7 @@
101 * OverlappingOutputGroup *
102 **************************/
103
104-geom::Rectangle mgm::OverlappingOutputGroup::bounding_rectangle() const
105+geom::Rectangle mg::OverlappingOutputGroup::bounding_rectangle() const
106 {
107 geom::Rectangles rectangles;
108
109@@ -61,7 +60,7 @@
110 return rectangles.bounding_rectangle();
111 }
112
113-void mgm::OverlappingOutputGroup::for_each_output(
114+void mg::OverlappingOutputGroup::for_each_output(
115 std::function<void(DisplayConfigurationOutput const&)> const& f) const
116 {
117 for (auto const& output : outputs)
118@@ -72,7 +71,7 @@
119 * OverlappingOutputGrouping *
120 *****************************/
121
122-mgm::OverlappingOutputGrouping::OverlappingOutputGrouping(DisplayConfiguration const& conf)
123+mg::OverlappingOutputGrouping::OverlappingOutputGrouping(DisplayConfiguration const& conf)
124 {
125 conf.for_each_output([&](DisplayConfigurationOutput const& conf_output)
126 {
127@@ -85,14 +84,14 @@
128
129 }
130
131-void mgm::OverlappingOutputGrouping::for_each_group(
132+void mg::OverlappingOutputGrouping::for_each_group(
133 std::function<void(OverlappingOutputGroup const& group)> const& f)
134 {
135 for (auto const& g : groups)
136 f(g);
137 }
138
139-void mgm::OverlappingOutputGrouping::add_output(DisplayConfigurationOutput const& conf_output)
140+void mg::OverlappingOutputGrouping::add_output(DisplayConfigurationOutput const& conf_output)
141 {
142 std::vector<size_t> overlapping_groups;
143
144
145=== modified file 'src/server/graphics/nested/nested_display.cpp'
146--- src/server/graphics/nested/nested_display.cpp 2014-03-04 04:09:36 +0000
147+++ src/server/graphics/nested/nested_display.cpp 2014-03-06 09:14:50 +0000
148@@ -26,6 +26,7 @@
149 #include "mir/graphics/gl_context.h"
150 #include "mir/graphics/surfaceless_egl_context.h"
151 #include "mir/graphics/display_configuration_policy.h"
152+#include "mir/graphics/overlapping_output_grouping.h"
153 #include "host_connection.h"
154
155 #include <boost/throw_exception.hpp>
156@@ -174,48 +175,50 @@
157 {
158 if (!configuration.valid())
159 {
160- BOOST_THROW_EXCEPTION(
161- std::logic_error("Invalid or inconsistent display configuration"));
162+ BOOST_THROW_EXCEPTION(std::logic_error("Invalid or inconsistent display configuration"));
163 }
164
165 decltype(outputs) result;
166+ OverlappingOutputGrouping unique_outputs{configuration};
167
168- // TODO for proper mirrored mode support we will need to detect overlapping outputs and
169- // TODO only use a single surface for them. The OverlappingOutputGrouping utility class
170- // TODO used by the Mesa backend for a similar purpose could help with this.
171- configuration.for_each_output(
172- [&](mg::DisplayConfigurationOutput const& output)
173+ unique_outputs.for_each_group(
174+ [&](mg::OverlappingOutputGroup const& group)
175 {
176- if (output.used)
177- {
178- geometry::Rectangle const& area = output.extents();
179-
180- auto const& egl_config_format = output.current_format;
181-
182- complete_display_initialization(egl_config_format);
183-
184- MirSurfaceParameters const request_params =
185+ bool have_output_for_group = false;
186+ geometry::Rectangle const& area = group.bounding_rectangle();
187+ group.for_each_output([&](mg::DisplayConfigurationOutput output)
188+ {
189+ if (!have_output_for_group)
190 {
191- "Mir nested display",
192- area.size.width.as_int(),
193- area.size.height.as_int(),
194- egl_config_format,
195- mir_buffer_usage_hardware,
196- static_cast<uint32_t>(output.id.as_value())
197- };
198-
199- auto const mir_surface = mir_connection_create_surface_sync(*connection, &request_params);
200-
201- if (!mir_surface_is_valid(mir_surface))
202- BOOST_THROW_EXCEPTION(std::runtime_error(mir_surface_get_error_message(mir_surface)));
203-
204- result[output.id] = std::make_shared<mgn::detail::NestedOutput>(
205- egl_display,
206- mir_surface,
207- area,
208- event_handler,
209- output.current_format);
210- }
211+ auto const& egl_config_format = output.current_format;
212+
213+ complete_display_initialization(egl_config_format);
214+
215+ MirSurfaceParameters const request_params = {
216+ "Mir nested display",
217+ area.size.width.as_int(),
218+ area.size.height.as_int(),
219+ egl_config_format,
220+ mir_buffer_usage_hardware,
221+ static_cast<uint32_t>(output.id.as_value())
222+ };
223+
224+ auto const mir_surface =
225+ mir_connection_create_surface_sync(*connection, &request_params);
226+
227+ if (!mir_surface_is_valid(mir_surface))
228+ BOOST_THROW_EXCEPTION(
229+ std::runtime_error(mir_surface_get_error_message(mir_surface)));
230+
231+ result[output.id] = std::make_shared<mgn::detail::NestedOutput>(
232+ egl_display,
233+ mir_surface,
234+ area,
235+ event_handler,
236+ output.current_format);
237+ have_output_for_group = true;
238+ }
239+ });
240 });
241
242 if (result.empty())
243
244=== modified file 'tests/unit-tests/graphics/mesa/test_overlapping_output_grouping.cpp'
245--- tests/unit-tests/graphics/mesa/test_overlapping_output_grouping.cpp 2014-03-04 04:09:36 +0000
246+++ tests/unit-tests/graphics/mesa/test_overlapping_output_grouping.cpp 2014-03-06 09:14:50 +0000
247@@ -16,7 +16,7 @@
248 * Authored by: Alexandros Frantzis <alexandros.frantzis@canonical.com>
249 */
250
251-#include "src/platform/graphics/mesa/overlapping_output_grouping.h"
252+#include "mir/graphics/overlapping_output_grouping.h"
253 #include "mir/graphics/display_configuration.h"
254 #include "mir/geometry/rectangle.h"
255 #include "mir/geometry/rectangles.h"
256@@ -27,7 +27,6 @@
257 #include <algorithm>
258
259 namespace mg = mir::graphics;
260-namespace mgm = mir::graphics::mesa;
261 namespace geom = mir::geometry;
262
263 namespace
264@@ -105,12 +104,12 @@
265 std::vector<geom::Rectangles> const& expected_groups)
266 {
267 StubDisplayConfiguration conf{info};
268- mgm::OverlappingOutputGrouping grouping{conf};
269+ mg::OverlappingOutputGrouping grouping{conf};
270
271 std::vector<std::vector<mg::DisplayConfigurationOutput>> grouping_results;
272
273 grouping.for_each_group(
274- [&](mgm::OverlappingOutputGroup const& group)
275+ [&](mg::OverlappingOutputGroup const& group)
276 {
277 std::vector<mg::DisplayConfigurationOutput> outputs;
278 group.for_each_output([&](mg::DisplayConfigurationOutput const& output)

Subscribers

People subscribed via source and target branches