Merge lp:~vanvugt/mir/deduplicate-NullDisplayReport into lp:mir
- deduplicate-NullDisplayReport
- Merge into development-branch
Status: | Work in progress |
---|---|
Proposed branch: | lp:~vanvugt/mir/deduplicate-NullDisplayReport |
Merge into: | lp:mir |
Diff against target: |
272 lines (+40/-63) 9 files modified
include/platform/mir/graphics/null_display_report.h (+9/-12) src/platform/graphics/CMakeLists.txt (+1/-0) src/platform/graphics/null_display_report.cpp (+19/-17) src/platform/symbols.map (+3/-0) src/platforms/android/server/device_quirks.cpp (+2/-14) src/platforms/android/utils/render_overlays.cpp (+2/-15) src/server/report/null/CMakeLists.txt (+0/-1) src/server/report/null/null_report_factory.cpp (+2/-2) tests/unit-tests/platforms/nested/test_nested_display.cpp (+2/-2) |
To merge this branch: | bzr merge lp:~vanvugt/mir/deduplicate-NullDisplayReport |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Mir CI Bot | continuous-integration | Approve | |
Alan Griffiths | Disapprove | ||
Review via email: mp+304905@code.launchpad.net |
Commit message
Deduplicate three null DisplayReports into one.
Description of the change
Mir CI Bot (mir-ci-bot) wrote : | # |
Alan Griffiths (alan-griffiths) wrote : | # |
Needing NullDisplayReport in mirplatform seems more like a problem than a solution.
Daniel van Vugt (vanvugt) wrote : | # |
Logically you make a good point. This isn't addressing the android code at the root of the issue.
However it's also worth remembering that DisplayReport is a report for Display. And Display is implemented in src/platforms/* so it makes sense that any commonality should exist in libmirplatform rather than libmirserver.
- 3686. By Daniel van Vugt
-
Ping Jenkins
Mir CI Bot (mir-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:3686
https:/
Executed test runs:
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
Click here to trigger a rebuild:
https:/
Daniel van Vugt (vanvugt) wrote : | # |
Come to think of it, any report class that legitimately has a null use case and that might only need a partial implementation in many cases really should not be pure virtual at all. A possibly cleaner solution to all this is to merge DisplayReport with null::DisplayRe
Daniel van Vugt (vanvugt) wrote : | # |
And it looks like I might be proposing four more places where NullDisplayReport gets used in future:
http://
- 3687. By Daniel van Vugt
-
Merge latest trunk
Mir CI Bot (mir-ci-bot) wrote : | # |
PASSED: Continuous integration, rev:3687
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
Daniel van Vugt (vanvugt) wrote : | # |
On second thoughts, mirserver is the only place in production where the report should be selected and constructed.
So ideally we should only see a null DisplayReport in mirserver and tests. I guess the current placement is correct if we can clean up and remove the two occurrences from src/platforms/
Unmerged revisions
- 3687. By Daniel van Vugt
-
Merge latest trunk
- 3686. By Daniel van Vugt
-
Ping Jenkins
- 3685. By Daniel van Vugt
-
It's LGPL, not GPL
- 3684. By Daniel van Vugt
-
First attempt
Preview Diff
1 | === renamed file 'src/server/report/null/display_report.h' => 'include/platform/mir/graphics/null_display_report.h' |
2 | --- src/server/report/null/display_report.h 2015-04-28 07:54:10 +0000 |
3 | +++ include/platform/mir/graphics/null_display_report.h 2016-09-07 03:00:58 +0000 |
4 | @@ -2,23 +2,23 @@ |
5 | * Copyright © 2012 Canonical Ltd. |
6 | * |
7 | * This program is free software: you can redistribute it and/or modify it |
8 | - * under the terms of the GNU General Public License version 3, |
9 | + * under the terms of the GNU Lesser General Public License version 3, |
10 | * as published by the Free Software Foundation. |
11 | * |
12 | * This program is distributed in the hope that it will be useful, |
13 | * but WITHOUT ANY WARRANTY; without even the implied warranty of |
14 | * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the |
15 | - * GNU General Public License for more details. |
16 | + * GNU Lesser General Public License for more details. |
17 | * |
18 | - * You should have received a copy of the GNU General Public License |
19 | + * You should have received a copy of the GNU Lesser General Public License |
20 | * along with this program. If not, see <http://www.gnu.org/licenses/>. |
21 | * |
22 | * Authored by: Alan Griffiths <alan@octopull.co.uk> |
23 | */ |
24 | |
25 | |
26 | -#ifndef MIR_REPORT_NULL_DISPLAY_REPORT_H_ |
27 | -#define MIR_REPORT_NULL_DISPLAY_REPORT_H_ |
28 | +#ifndef MIR_GRAPHICS_NULL_DISPLAY_REPORT_H_ |
29 | +#define MIR_GRAPHICS_NULL_DISPLAY_REPORT_H_ |
30 | |
31 | |
32 | #include "mir/graphics/display_report.h" |
33 | @@ -26,11 +26,9 @@ |
34 | |
35 | namespace mir |
36 | { |
37 | -namespace report |
38 | -{ |
39 | -namespace null |
40 | -{ |
41 | -class DisplayReport : public graphics::DisplayReport |
42 | +namespace graphics |
43 | +{ |
44 | +class NullDisplayReport : public graphics::DisplayReport |
45 | { |
46 | public: |
47 | |
48 | @@ -47,6 +45,5 @@ |
49 | }; |
50 | } |
51 | } |
52 | -} |
53 | |
54 | -#endif /* MIR_REPORT_NULL_DISPLAY_REPORT_H_ */ |
55 | +#endif /* MIR_GRAPHICS_NULL_DISPLAY_REPORT_H_ */ |
56 | |
57 | === modified file 'src/platform/graphics/CMakeLists.txt' |
58 | --- src/platform/graphics/CMakeLists.txt 2016-06-02 05:33:50 +0000 |
59 | +++ src/platform/graphics/CMakeLists.txt 2016-09-07 03:00:58 +0000 |
60 | @@ -11,6 +11,7 @@ |
61 | pixel_format_utils.cpp |
62 | overlapping_output_grouping.cpp |
63 | platform_probe.cpp |
64 | + null_display_report.cpp |
65 | ) |
66 | |
67 | add_library(mirplatformgraphicscommon OBJECT |
68 | |
69 | === renamed file 'src/server/report/null/display_report.cpp' => 'src/platform/graphics/null_display_report.cpp' |
70 | --- src/server/report/null/display_report.cpp 2015-04-28 07:54:10 +0000 |
71 | +++ src/platform/graphics/null_display_report.cpp 2016-09-07 03:00:58 +0000 |
72 | @@ -2,31 +2,33 @@ |
73 | * Copyright © 2013 Canonical Ltd. |
74 | * |
75 | * This program is free software: you can redistribute it and/or modify it |
76 | - * under the terms of the GNU General Public License version 3, |
77 | + * under the terms of the GNU Lesser General Public License version 3, |
78 | * as published by the Free Software Foundation. |
79 | * |
80 | * This program is distributed in the hope that it will be useful, |
81 | * but WITHOUT ANY WARRANTY; without even the implied warranty of |
82 | * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the |
83 | - * GNU General Public License for more details. |
84 | + * GNU Lesser General Public License for more details. |
85 | * |
86 | - * You should have received a copy of the GNU General Public License |
87 | + * You should have received a copy of the GNU Lesser General Public License |
88 | * along with this program. If not, see <http://www.gnu.org/licenses/>. |
89 | * |
90 | * Authored by: Alan Griffiths <alan@octopull.co.uk> |
91 | */ |
92 | |
93 | -#include "display_report.h" |
94 | - |
95 | -namespace mrn = mir::report::null; |
96 | - |
97 | -void mrn::DisplayReport::report_successful_setup_of_native_resources() {} |
98 | -void mrn::DisplayReport::report_successful_egl_make_current_on_construction() {} |
99 | -void mrn::DisplayReport::report_successful_egl_buffer_swap_on_construction() {} |
100 | -void mrn::DisplayReport::report_successful_drm_mode_set_crtc_on_construction() {} |
101 | -void mrn::DisplayReport::report_successful_display_construction() {} |
102 | -void mrn::DisplayReport::report_drm_master_failure(int) {} |
103 | -void mrn::DisplayReport::report_vt_switch_away_failure() {} |
104 | -void mrn::DisplayReport::report_vt_switch_back_failure() {} |
105 | -void mrn::DisplayReport::report_egl_configuration(EGLDisplay, EGLConfig) {} |
106 | -void mrn::DisplayReport::report_vsync(unsigned int) {} |
107 | +#include "mir/graphics/null_display_report.h" |
108 | + |
109 | +namespace mir { namespace graphics { |
110 | + |
111 | +void NullDisplayReport::report_successful_setup_of_native_resources() {} |
112 | +void NullDisplayReport::report_successful_egl_make_current_on_construction() {} |
113 | +void NullDisplayReport::report_successful_egl_buffer_swap_on_construction() {} |
114 | +void NullDisplayReport::report_successful_drm_mode_set_crtc_on_construction() {} |
115 | +void NullDisplayReport::report_successful_display_construction() {} |
116 | +void NullDisplayReport::report_drm_master_failure(int) {} |
117 | +void NullDisplayReport::report_vt_switch_away_failure() {} |
118 | +void NullDisplayReport::report_vt_switch_back_failure() {} |
119 | +void NullDisplayReport::report_egl_configuration(EGLDisplay, EGLConfig) {} |
120 | +void NullDisplayReport::report_vsync(unsigned int) {} |
121 | + |
122 | +}} // namespace mir::graphics |
123 | |
124 | === modified file 'src/platform/symbols.map' |
125 | --- src/platform/symbols.map 2016-08-22 14:36:40 +0000 |
126 | +++ src/platform/symbols.map 2016-09-07 03:00:58 +0000 |
127 | @@ -270,6 +270,9 @@ |
128 | android::isEligibleBuiltInKeyboard*; |
129 | __android_log_assert; |
130 | __android_log_print; |
131 | + |
132 | + vtable?for?mir::graphics::NullDisplayReport*; |
133 | + mir::graphics::NullDisplayReport*; |
134 | }; |
135 | local: *; |
136 | }; |
137 | |
138 | === modified file 'src/platforms/android/server/device_quirks.cpp' |
139 | --- src/platforms/android/server/device_quirks.cpp 2016-08-07 20:38:35 +0000 |
140 | +++ src/platforms/android/server/device_quirks.cpp 2016-09-07 03:00:58 +0000 |
141 | @@ -18,7 +18,7 @@ |
142 | |
143 | #include <GLES2/gl2.h> |
144 | #include <EGL/egl.h> |
145 | -#include "mir/graphics/display_report.h" |
146 | +#include "mir/graphics/null_display_report.h" |
147 | #include "mir/graphics/gl_config.h" |
148 | #include "gl_context.h" |
149 | #include "device_quirks.h" |
150 | @@ -100,19 +100,7 @@ |
151 | return false; |
152 | } |
153 | |
154 | -struct NullReport : mg::DisplayReport |
155 | -{ |
156 | - void report_successful_setup_of_native_resources() override {}; |
157 | - void report_successful_egl_make_current_on_construction() override {}; |
158 | - void report_successful_egl_buffer_swap_on_construction() override {}; |
159 | - void report_successful_display_construction() override {}; |
160 | - void report_egl_configuration(EGLDisplay, EGLConfig) override {}; |
161 | - void report_vsync(unsigned int) override {}; |
162 | - void report_successful_drm_mode_set_crtc_on_construction() override {}; |
163 | - void report_drm_master_failure(int) override {}; |
164 | - void report_vt_switch_away_failure() override {}; |
165 | - void report_vt_switch_back_failure() override {}; |
166 | -} report; |
167 | +mg::NullDisplayReport report; |
168 | |
169 | struct Config : mg::GLConfig |
170 | { |
171 | |
172 | === modified file 'src/platforms/android/utils/render_overlays.cpp' |
173 | --- src/platforms/android/utils/render_overlays.cpp 2016-08-16 19:34:28 +0000 |
174 | +++ src/platforms/android/utils/render_overlays.cpp 2016-09-07 03:00:58 +0000 |
175 | @@ -26,7 +26,7 @@ |
176 | #include "mir/graphics/graphic_buffer_allocator.h" |
177 | #include "mir/graphics/buffer_properties.h" |
178 | #include "mir/graphics/gl_config.h" |
179 | -#include "mir/graphics/display_report.h" |
180 | +#include "mir/graphics/null_display_report.h" |
181 | #include "mir/renderer/gl/render_target.h" |
182 | #include "mir/renderer/sw/pixel_source.h" |
183 | #include "mir_image.h" |
184 | @@ -226,19 +226,6 @@ |
185 | int stencil_buffer_bits() const override { return 0; } |
186 | }; |
187 | |
188 | -struct DisplayReport : mg::DisplayReport |
189 | -{ |
190 | - void report_successful_setup_of_native_resources() override {} |
191 | - void report_successful_egl_make_current_on_construction() override {} |
192 | - void report_successful_egl_buffer_swap_on_construction() override {} |
193 | - void report_successful_display_construction() override {} |
194 | - void report_egl_configuration(EGLDisplay, EGLConfig) override {} |
195 | - void report_vsync(unsigned int) override {} |
196 | - void report_successful_drm_mode_set_crtc_on_construction() override {} |
197 | - void report_drm_master_failure(int) override {} |
198 | - void report_vt_switch_away_failure() override {} |
199 | - void report_vt_switch_back_failure() override {} |
200 | -}; |
201 | } |
202 | |
203 | int main(int argc, char const** argv) |
204 | @@ -265,7 +252,7 @@ |
205 | |
206 | auto platform_fn = platform_library->load_function<mg::CreateHostPlatform>( |
207 | "create_host_platform", MIR_SERVER_GRAPHICS_PLATFORM_VERSION); |
208 | - auto platform = platform_fn(options, nullptr, std::make_shared<DisplayReport>()); |
209 | + auto platform = platform_fn(options, nullptr, std::make_shared<mg::NullDisplayReport>()); |
210 | |
211 | //Strange issues going on here with dlopen() + hybris (which uses gnu_indirect_functions) |
212 | //https://github.com/libhybris/libhybris/issues/315 |
213 | |
214 | === modified file 'src/server/report/null/CMakeLists.txt' |
215 | --- src/server/report/null/CMakeLists.txt 2016-07-28 23:00:22 +0000 |
216 | +++ src/server/report/null/CMakeLists.txt 2016-09-07 03:00:58 +0000 |
217 | @@ -3,7 +3,6 @@ |
218 | |
219 | compositor_report.cpp |
220 | connector_report.cpp |
221 | - display_report.cpp |
222 | input_report.cpp |
223 | message_processor_report.cpp |
224 | null_report_factory.cpp |
225 | |
226 | === modified file 'src/server/report/null/null_report_factory.cpp' |
227 | --- src/server/report/null/null_report_factory.cpp 2016-07-28 23:00:22 +0000 |
228 | +++ src/server/report/null/null_report_factory.cpp 2016-09-07 03:00:58 +0000 |
229 | @@ -22,11 +22,11 @@ |
230 | #include "connector_report.h" |
231 | #include "message_processor_report.h" |
232 | #include "session_mediator_report.h" |
233 | -#include "display_report.h" |
234 | #include "input_report.h" |
235 | #include "seat_report.h" |
236 | #include "shell_report.h" |
237 | #include "scene_report.h" |
238 | +#include "mir/graphics/null_display_report.h" |
239 | #include "mir/logging/null_shared_library_prober_report.h" |
240 | |
241 | std::shared_ptr<mir::compositor::CompositorReport> mir::report::NullReportFactory::create_compositor_report() |
242 | @@ -36,7 +36,7 @@ |
243 | |
244 | std::shared_ptr<mir::graphics::DisplayReport> mir::report::NullReportFactory::create_display_report() |
245 | { |
246 | - return std::make_shared<null::DisplayReport>(); |
247 | + return std::make_shared<mir::graphics::NullDisplayReport>(); |
248 | } |
249 | |
250 | std::shared_ptr<mir::scene::SceneReport> mir::report::NullReportFactory::create_scene_report() |
251 | |
252 | === modified file 'tests/unit-tests/platforms/nested/test_nested_display.cpp' |
253 | --- tests/unit-tests/platforms/nested/test_nested_display.cpp 2016-05-04 20:53:03 +0000 |
254 | +++ tests/unit-tests/platforms/nested/test_nested_display.cpp 2016-09-07 03:00:58 +0000 |
255 | @@ -19,7 +19,7 @@ |
256 | #include "src/server/graphics/nested/display.h" |
257 | #include "src/server/graphics/nested/host_connection.h" |
258 | #include "src/server/graphics/nested/host_surface.h" |
259 | -#include "src/server/report/null/display_report.h" |
260 | +#include "mir/graphics/null_display_report.h" |
261 | #include "mir/graphics/default_display_configuration_policy.h" |
262 | #include "src/server/input/null_input_dispatcher.h" |
263 | #include "mir_display_configuration_builder.h" |
264 | @@ -77,7 +77,7 @@ |
265 | |
266 | testing::NiceMock<mtd::MockEGL> mock_egl; |
267 | mi::NullInputDispatcher null_input_dispatcher; |
268 | - mir::report::null::DisplayReport null_display_report; |
269 | + mg::NullDisplayReport null_display_report; |
270 | mg::CloneDisplayConfigurationPolicy default_conf_policy; |
271 | mtd::StubGLConfig stub_gl_config; |
272 | std::shared_ptr<mtd::NullPlatform> null_platform{ |
FAILED: Continuous integration, rev:3685 /mir-jenkins. ubuntu. com/job/ mir-ci/ 1634/ /mir-jenkins. ubuntu. com/job/ build-mir/ 2044/console /mir-jenkins. ubuntu. com/job/ build-0- fetch/2106 /mir-jenkins. ubuntu. com/job/ build-1- sourcepkg/ release= vivid+overlay/ 2097 /mir-jenkins. ubuntu. com/job/ build-1- sourcepkg/ release= xenial+ overlay/ 2097 /mir-jenkins. ubuntu. com/job/ build-1- sourcepkg/ release= yakkety/ 2097 /mir-jenkins. ubuntu. com/job/ build-2- binpkg- mir/arch= amd64,compiler= clang,platform= mesa,release= yakkety/ 2070/console /mir-jenkins. ubuntu. com/job/ build-2- binpkg- mir/arch= amd64,compiler= gcc,platform= mesa,release= xenial+ overlay/ 2070 /mir-jenkins. ubuntu. com/job/ build-2- binpkg- mir/arch= amd64,compiler= gcc,platform= mesa,release= xenial+ overlay/ 2070/artifact/ output/ *zip*/output. zip /mir-jenkins. ubuntu. com/job/ build-2- binpkg- mir/arch= amd64,compiler= gcc,platform= mesa,release= yakkety/ 2070 /mir-jenkins. ubuntu. com/job/ build-2- binpkg- mir/arch= amd64,compiler= gcc,platform= mesa,release= yakkety/ 2070/artifact/ output/ *zip*/output. zip /mir-jenkins. ubuntu. com/job/ build-2- binpkg- mir/arch= cross-armhf, compiler= gcc,platform= android, release= vivid+overlay/ 2070 /mir-jenkins. ubuntu. com/job/ build-2- binpkg- mir/arch= cross-armhf, compiler= gcc,platform= android, release= vivid+overlay/ 2070/artifact/ output/ *zip*/output. zip /mir-jenkins. ubuntu. com/job/ build-2- binpkg- mir/arch= i386,compiler= gcc,platform= android, release= vivid+overlay/ 2070 /mir-jenkins. ubuntu. com/job/ build-2- binpkg- mir/arch= i386,compiler= gcc,platform= android, release= vivid+overlay/ 2070/artifact/ output/ *zip*/output. zip /mir-jenkins. ubuntu. com/job/ build-2- binpkg- mir/arch= i386,compiler= gcc,platform= mesa,release= xenial+ overlay/ 2070 /mir-jenkins. ubuntu. com/job/ build-2- binpkg- mir/arch= i386,compiler= gcc,platform= mesa,release= xenial+ overlay/ 2070/artifact/ output/ *zip*/output. zip
https:/
Executed test runs:
FAILURE: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
FAILURE: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild: /mir-jenkins. ubuntu. com/job/ mir-ci/ 1634/rebuild
https:/