Mir

Merge lp:~raof/mir/nested-preserve-display-buffer into lp:mir

  1. nested-preserve-display-buffer
  2. Merge into development-branch
Proposed by Chris Halse Rogers
Status: Merged
Approved by: Alan Griffiths
Approved revision: no longer in the source branch.
Merged at revision: 3844
Proposed branch: lp:~raof/mir/nested-preserve-display-buffer
Merge into: lp:mir
Diff against target: 453 lines (+272/-72)
4 files modified
src/server/graphics/nested/display.cpp (+127/-67)
src/server/graphics/nested/display.h (+1/-1)
tests/acceptance-tests/test_nested_mir.cpp (+12/-4)
tests/unit-tests/platforms/nested/test_nested_display.cpp (+132/-0)
To merge this branch: bzr merge lp:~raof/mir/nested-preserve-display-buffer
Reviewer Review Type Date Requested Status
Alan Griffiths Approve
Andreas Pokorny (community) Approve
Mir CI Bot continuous-integration Approve
Review via email: mp+310395@code.launchpad.net

Commit message

Implement Display::apply_if_configuration_preserves_framebuffers for the nested platform.

Final¹ part of the fix for bug #1556142.

¹: For now - mesa-x11 support is missing, as is eglstream support. X11 is purely for the ease of testing, and eglstream isn't ready for primetime.

To post a comment you must log in.
Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

FAILED: Continuous integration, rev:3813
https://mir-jenkins.ubuntu.com/job/mir-ci/2134/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/2762/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/2825
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/2817
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/2817
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/2817
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/2791/console
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/2791/console
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/2791/console
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/2791/console
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/2791/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/2791
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/2791/artifact/output/*zip*/output.zip
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/2791/console

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/2134/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Chris Halse Rogers (raof) wrote :

Hm, I've got a guess as to what's wrong here; I just need to work out how to test it...

Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

FAILED: Continuous integration, rev:3814
https://mir-jenkins.ubuntu.com/job/mir-ci/2156/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/2794/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/2859
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/2851
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/2851
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/2851
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/2823/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/2823
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/2823/artifact/output/*zip*/output.zip
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/2823/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/2823
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/2823/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/2823
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/2823/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/2823
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/2823/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/2156/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

FAILED: Continuous integration, rev:3814
https://mir-jenkins.ubuntu.com/job/mir-ci/2157/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/2795/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/2860
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/2852
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/2852
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/2852
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/2824/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/2824
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/2824/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/2824
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/2824/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/2824
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/2824/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/2824
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/2824/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/2824
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/2824/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/2157/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Chris Halse Rogers (raof) wrote :

07:28:38 9: [ FAILED ] TestClientInput.receives_one_touch_event_per_frame

REALTIME!!!!!!!!!!!!!!

Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

FAILED: Continuous integration, rev:3814
https://mir-jenkins.ubuntu.com/job/mir-ci/2166/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/2807/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/2872
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/2864
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/2864
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/2864
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/2836/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/2836
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/2836/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/2836
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/2836/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/2836
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/2836/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/2836
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/2836/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/2836
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/2836/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/2166/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

FAILED: Continuous integration, rev:3815
https://mir-jenkins.ubuntu.com/job/mir-ci/2168/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/2810/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/2875
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/2867
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/2867
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/2867
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/2839/console
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/2839/console
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/2839/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/2839
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/2839/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/2839
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/2839/artifact/output/*zip*/output.zip
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/2839/console

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/2168/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

PASSED: Continuous integration, rev:
https://mir-jenkins.ubuntu.com/job/mir-ci/2195/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/2841
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/2906
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/2898
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/2898
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/2898
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/2870
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/2870/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/2870
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/2870/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/2870
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/2870/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/2870
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/2870/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/2870
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/2870/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/2870
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/2870/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/2195/rebuild

review: Approve (continuous-integration)
Revision history for this message
Andreas Pokorny (andreas-pokorny) wrote :

lgtm

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

+ /* We need to attach a painted client before attempting to set the display mode,
+ * otherwise the nested server will not have rendered anything, so won't be focused,
+ * so the host server won't apply any set configuration
+ */

I don't see why we need to attach a painted client *before* attempting to set the display mode. AIUI the session display configuration gets set with or without focus - and will be (re)applied whenever the session (re)gains focus.

I.e. does this change in the test work around bug in this branch? Or am I missing something?

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

This works around a (deliberate) change in behaviour in this branch.

Previously, the nearby call to
nested_mir.server.the_display_configuration_controller()->set_base_configuration(initial_config)
would stop and resume the compositor around the display->configure() call. Compositor::start() has the side-effect of forcing a composition pass, which results in the nested server posting a buffer, which results in the nested server receiving focus in the host server, which results in the host server actually applying the requested display configuration.

The goal of this branch is to eliminate the stop()/start() cycle. This means the nested server doesn't (unnecessarily) post an empty buffer on set_base_configuration(), which means that the nested server isn't focused, which means the host server *doesn't* change the display configuration, which means the test times out waiting for the host server to change the display configuration.

So, the thing that you're missing is that while the session display configuration *will* get set without focus, the test is waiting for the display configuration to be applied, which only happens with focus.

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

OK, thanks.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/server/graphics/nested/display.cpp'
2--- src/server/graphics/nested/display.cpp 2016-11-11 07:56:09 +0000
3+++ src/server/graphics/nested/display.cpp 2016-11-17 00:14:50 +0000
4@@ -36,6 +36,7 @@
5
6 #include <boost/throw_exception.hpp>
7 #include <stdexcept>
8+#include <algorithm>
9
10 namespace mg = mir::graphics;
11 namespace mgn = mir::graphics::nested;
12@@ -175,6 +176,44 @@
13 return output->view_area();
14 }
15
16+namespace
17+{
18+long area_of(geom::Rectangle const& rect)
19+{
20+ return static_cast<long>(rect.size.width.as_int())*rect.size.height.as_int();
21+}
22+
23+std::vector<mg::DisplayConfigurationOutput> calculate_best_outputs(
24+ mg::DisplayConfiguration const& conf)
25+{
26+ std::vector<mg::DisplayConfigurationOutput> result;
27+
28+ mg::OverlappingOutputGrouping unique_outputs{conf};
29+
30+ unique_outputs.for_each_group(
31+ [&](mg::OverlappingOutputGroup const& group)
32+ {
33+ geom::Rectangle const area = group.bounding_rectangle();
34+
35+ long max_overlap_area = 0;
36+ mg::DisplayConfigurationOutput best_output;
37+
38+ group.for_each_output(
39+ [&](mg::DisplayConfigurationOutput const& output)
40+ {
41+ if (area_of(area.intersection_with(output.extents())) > max_overlap_area)
42+ {
43+ max_overlap_area = area_of(area.intersection_with(output.extents()));
44+ best_output = output;
45+ }
46+ });
47+ result.push_back(best_output);
48+ });
49+
50+ return result;
51+}
52+}
53+
54 mgn::Display::Display(
55 std::shared_ptr<mg::Platform> const& platform,
56 std::shared_ptr<HostConnection> const& connection,
57@@ -200,7 +239,7 @@
58 swap(current_configuration, conf);
59 }
60
61- create_surfaces(*current_configuration);
62+ create_surfaces(calculate_best_outputs(*current_configuration));
63 }
64
65 mgn::Display::~Display() noexcept
66@@ -233,82 +272,57 @@
67
68 void mgn::Display::configure(mg::DisplayConfiguration const& configuration)
69 {
70+ if (!configuration.valid())
71+ {
72+ BOOST_THROW_EXCEPTION(std::logic_error("Invalid or inconsistent display configuration"));
73+ }
74+
75 decltype(current_configuration) new_config{dynamic_cast<NestedDisplayConfiguration*>(configuration.clone().release())};
76
77 {
78 std::lock_guard<std::mutex> lock(configuration_mutex);
79
80 swap(current_configuration, new_config);
81- create_surfaces(*current_configuration);
82+ create_surfaces(calculate_best_outputs(*current_configuration));
83 }
84
85 connection->apply_display_config(**current_configuration);
86 }
87
88-namespace
89-{
90-long area_of(geom::Rectangle const& rect)
91-{
92- return static_cast<long>(rect.size.width.as_int())*rect.size.height.as_int();
93-}
94-}
95-
96-void mgn::Display::create_surfaces(mg::DisplayConfiguration const& configuration)
97-{
98- if (!configuration.valid())
99- {
100- BOOST_THROW_EXCEPTION(std::logic_error("Invalid or inconsistent display configuration"));
101- }
102-
103+void mgn::Display::create_surfaces(std::vector<mg::DisplayConfigurationOutput> const& output_list)
104+{
105 decltype(outputs) result;
106- OverlappingOutputGrouping unique_outputs{configuration};
107-
108- unique_outputs.for_each_group(
109- [&](mg::OverlappingOutputGroup const& group)
110- {
111- geometry::Rectangle const area = group.bounding_rectangle();
112-
113- long max_overlap_area = 0;
114- mg::DisplayConfigurationOutput best_output;
115-
116- group.for_each_output([&](mg::DisplayConfigurationOutput const& output)
117- {
118- if (area_of(area.intersection_with(output.extents())) > max_overlap_area)
119- {
120- max_overlap_area = area_of(area.intersection_with(output.extents()));
121- best_output = output;
122- }
123- });
124-
125- auto const& egl_config_format = best_output.current_format;
126- geometry::Rectangle const& extents = best_output.extents();
127-
128- auto& display_buffer = result[best_output.id];
129-
130- {
131- std::unique_lock<std::mutex> lock(outputs_mutex);
132- display_buffer = outputs[best_output.id];
133- }
134-
135- if (display_buffer)
136- {
137- if (display_buffer->view_area() != extents)
138- display_buffer.reset();
139- }
140-
141- if (!display_buffer)
142- {
143- complete_display_initialization(egl_config_format);
144-
145- eglBindAPI(MIR_SERVER_EGL_OPENGL_API);
146- display_buffer = std::make_shared<mgn::detail::DisplaySyncGroup>(
147- std::make_shared<mgn::detail::DisplayBuffer>(
148- egl_display,
149- best_output,
150- connection,
151- passthrough_option));
152- }
153- });
154+ for (auto const& output : output_list)
155+ {
156+ auto const& egl_config_format = output.current_format;
157+ geometry::Rectangle const& extents = output.extents();
158+
159+ auto& display_buffer = result[output.id];
160+
161+ {
162+ std::unique_lock<std::mutex> lock(outputs_mutex);
163+ display_buffer = outputs[output.id];
164+ }
165+
166+ if (display_buffer)
167+ {
168+ if (display_buffer->view_area() != extents)
169+ display_buffer.reset();
170+ }
171+
172+ if (!display_buffer)
173+ {
174+ complete_display_initialization(egl_config_format);
175+
176+ eglBindAPI(MIR_SERVER_EGL_OPENGL_API);
177+ display_buffer = std::make_shared<mgn::detail::DisplaySyncGroup>(
178+ std::make_shared<mgn::detail::DisplayBuffer>(
179+ egl_display,
180+ output,
181+ connection,
182+ passthrough_option));
183+ }
184+ }
185
186 {
187 std::unique_lock<std::mutex> lock(outputs_mutex);
188@@ -371,9 +385,55 @@
189 }
190
191 bool mgn::Display::apply_if_configuration_preserves_display_buffers(
192- mg::DisplayConfiguration const& /*conf*/)
193+ mg::DisplayConfiguration const& conf)
194 {
195- return false;
196+ auto new_outputs = calculate_best_outputs(conf);
197+ {
198+ std::lock_guard<decltype(configuration_mutex)> lock{configuration_mutex};
199+
200+ {
201+ std::lock_guard<decltype(outputs_mutex)> outputs_lock{outputs_mutex};
202+ for (auto const existing_output : outputs)
203+ {
204+ // O(n²) here, but n < 10 and this is isn't a hot path.
205+ if (!std::any_of(
206+ new_outputs.begin(),
207+ new_outputs.end(),
208+ [id = existing_output.first](auto const&output) { return output.id == id; }))
209+ {
210+ // At least one of the existing outputs isn't used in the ne
211+ return false;
212+ }
213+ }
214+
215+ for (auto const& output : new_outputs)
216+ {
217+ auto existing_output = outputs.find(output.id);
218+
219+ /*
220+ * If there's no existing output associated with this ID then we can't be
221+ * invalidating its DisplayBuffer.
222+ *
223+ * If there *is* an existing output associated with this ID then
224+ * create_surfaces() will invalidate its DisplayBuffer if and only if
225+ * the viewport doesn't exactly match.
226+ */
227+ if (existing_output != outputs.end() &&
228+ existing_output->second->view_area() != output.extents())
229+ {
230+ return false;
231+ }
232+ }
233+ }
234+
235+ current_configuration =
236+ decltype(current_configuration){dynamic_cast<NestedDisplayConfiguration*>(conf.clone().release())};
237+
238+ create_surfaces(new_outputs);
239+ }
240+ connection->apply_display_config(**current_configuration);
241+
242+ return true;
243 }
244
245 mg::Frame mgn::Display::last_frame_on(unsigned) const
246
247=== modified file 'src/server/graphics/nested/display.h'
248--- src/server/graphics/nested/display.h 2016-11-09 02:30:14 +0000
249+++ src/server/graphics/nested/display.h 2016-11-17 00:14:50 +0000
250@@ -167,7 +167,7 @@
251 std::mutex mutable configuration_mutex;
252 std::unique_ptr<NestedDisplayConfiguration> current_configuration;
253
254- void create_surfaces(mir::graphics::DisplayConfiguration const& configuration);
255+ void create_surfaces(std::vector<graphics::DisplayConfigurationOutput> const& output_list);
256 void complete_display_initialization(MirPixelFormat format);
257 };
258
259
260=== modified file 'tests/acceptance-tests/test_nested_mir.cpp'
261--- tests/acceptance-tests/test_nested_mir.cpp 2016-11-15 23:48:01 +0000
262+++ tests/acceptance-tests/test_nested_mir.cpp 2016-11-17 00:14:50 +0000
263@@ -1270,6 +1270,12 @@
264 NestedMirRunner nested_mir{new_connection()};
265 ignore_rebuild_of_egl_context();
266
267+ /* We need to attach a painted client before attempting to set the display mode,
268+ * otherwise the nested server will not have rendered anything, so won't be focused,
269+ * so the host server won't apply any set configuration
270+ */
271+ ClientWithAPaintedSurface client(nested_mir);
272+
273 {
274 std::shared_ptr<mg::DisplayConfiguration> const initial_config{nested_mir.server.the_display()->configuration()};
275 initial_config->for_each_output([](mg::UserDisplayConfigurationOutput& output)
276@@ -1288,8 +1294,6 @@
277 ASSERT_TRUE(initial_condition.raised());
278 }
279
280- ClientWithAPaintedSurface client(nested_mir);
281-
282 auto const expect_config = hw_display_config_for_unplug();
283
284 mt::Signal condition;
285@@ -1371,6 +1375,12 @@
286 NestedMirRunner nested_mir{new_connection()};
287 ignore_rebuild_of_egl_context();
288
289+ /* We need to attach a painted client before attempting to set the display mode,
290+ * otherwise the nested server will not have rendered anything, so won't be focused,
291+ * so the host server won't apply any set configuration
292+ */
293+ ClientWithAPaintedSurface client(nested_mir);
294+
295 {
296 std::shared_ptr<mg::DisplayConfiguration> const initial_config{nested_mir.server.the_display()->configuration()};
297 initial_config->for_each_output([](mg::UserDisplayConfigurationOutput& output)
298@@ -1389,8 +1399,6 @@
299 ASSERT_TRUE(initial_condition.raised());
300 }
301
302- ClientWithAPaintedSurface client(nested_mir);
303-
304 auto const new_config = hw_display_config_for_plugin();
305
306 // The default layout policy (for cloned displays) will be applied by the MediatingDisplayChanger.
307
308=== modified file 'tests/unit-tests/platforms/nested/test_nested_display.cpp'
309--- tests/unit-tests/platforms/nested/test_nested_display.cpp 2016-08-24 14:03:18 +0000
310+++ tests/unit-tests/platforms/nested/test_nested_display.cpp 2016-11-17 00:14:50 +0000
311@@ -31,6 +31,7 @@
312 #include "mir/test/doubles/stub_cursor_listener.h"
313 #include "mir/test/doubles/null_platform.h"
314 #include "mir/test/fake_shared.h"
315+#include "mir/test/display_config_matchers.h"
316
317 #include <gtest/gtest.h>
318 #include <gmock/gmock.h>
319@@ -244,3 +245,134 @@
320
321 auto dummy_context = nested_display->create_gl_context();
322 }
323+
324+TEST_F(NestedDisplay, preserves_framebuffers_for_metadata_changes)
325+{
326+ using namespace testing;
327+
328+ auto display = create_nested_display(
329+ null_platform,
330+ mt::fake_shared(stub_gl_config));
331+
332+ auto conf = display->configuration();
333+
334+ std::vector<mg::DisplayBuffer*> initial_dbs;
335+
336+ display->for_each_display_sync_group(
337+ [&initial_dbs](auto& sync_group)
338+ {
339+ sync_group.for_each_display_buffer(
340+ [&initial_dbs](auto& display_buffer)
341+ {
342+ initial_dbs.push_back(&display_buffer);
343+ });
344+ });
345+
346+ ASSERT_THAT(initial_dbs, Not(IsEmpty()));
347+
348+ conf->for_each_output(
349+ [](mg::UserDisplayConfigurationOutput& output)
350+ {
351+ output.scale *= 3.1415f;
352+ output.form_factor =
353+ output.form_factor == mir_form_factor_monitor ?
354+ mir_form_factor_phone :
355+ mir_form_factor_monitor;
356+ output.subpixel_arrangement =
357+ output.subpixel_arrangement == mir_subpixel_arrangement_horizontal_bgr ?
358+ mir_subpixel_arrangement_vertical_bgr :
359+ mir_subpixel_arrangement_horizontal_bgr;
360+ output.orientation =
361+ output.orientation == mir_orientation_normal ?
362+ mir_orientation_inverted :
363+ mir_orientation_normal;
364+ });
365+
366+ EXPECT_THAT(*conf, Not(mt::DisplayConfigMatches(std::cref(*display->configuration()))));
367+
368+ EXPECT_TRUE(display->apply_if_configuration_preserves_display_buffers(* conf));
369+
370+ // Touch each of the saved DisplayBuffers, so valgrind will complain if we've invalidated them
371+ for (auto db : initial_dbs)
372+ {
373+ EXPECT_THAT(db->native_display_buffer(), NotNull());
374+ }
375+}
376+
377+TEST_F(NestedDisplay, changing_output_does_not_preserve_framebuffers)
378+{
379+ using namespace testing;
380+
381+ class MultiDisplayHostConnection : public mtd::StubHostConnection
382+ {
383+ public:
384+ std::shared_ptr<MirDisplayConfiguration> create_display_config() override
385+ {
386+ // This builds a 3-monitor configuration...
387+ return mt::build_non_trivial_configuration();
388+ }
389+ };
390+
391+
392+ auto display = std::make_unique<mgn::Display>(
393+ null_platform,
394+ std::make_shared<MultiDisplayHostConnection>(),
395+ mt::fake_shared(null_display_report),
396+ mt::fake_shared(default_conf_policy),
397+ mt::fake_shared(stub_gl_config),
398+ mgn::PassthroughOption::disabled);
399+
400+ auto conf = display->configuration();
401+
402+ // Enable the first and second displays, have the third disabled.
403+ conf->for_each_output(
404+ [counter = 0](mg::UserDisplayConfigurationOutput& output) mutable
405+ {
406+ switch (counter)
407+ {
408+ case 0:
409+ output.used = true;
410+ output.top_left = {0, 0};
411+ break;
412+ case 1:
413+ output.used = true;
414+ output.top_left = {2000, 2000};
415+ break;
416+ case 2:
417+ output.used = false;
418+ break;
419+ }
420+ ++counter;
421+ });
422+
423+ display->configure(*conf);
424+
425+ conf = display->configuration();
426+
427+ // Enable the first and third displays, have the second disabled.
428+ conf->for_each_output(
429+ [counter = 0](mg::UserDisplayConfigurationOutput& output) mutable
430+ {
431+ switch (counter)
432+ {
433+ case 0:
434+ output.used = true;
435+ output.top_left = {0, 0};
436+ break;
437+ case 1:
438+ output.used = false;
439+ break;
440+ case 2:
441+ output.used = true;
442+ output.top_left = {2000, 2000};
443+ break;
444+ }
445+ ++counter;
446+ });
447+
448+ /*
449+ * This tests an implementation detail; it should be perfectly possible to preserve
450+ * display buffers in this case, but we don't, so we shouldn't pretend to.
451+ */
452+ EXPECT_FALSE(display->apply_if_configuration_preserves_display_buffers(*conf));
453+}

Subscribers

People subscribed via source and target branches