Mir

Merge lp:~kdub/mir/fix-1364637 into lp:mir

  1. fix-1364637
  2. Merge into development-branch
Proposed by Kevin DuBois
Status: Merged
Approved by: Daniel van Vugt
Approved revision: no longer in the source branch.
Merged at revision: 1902
Proposed branch: lp:~kdub/mir/fix-1364637
Merge into: lp:mir
Diff against target: 633 lines (+109/-80)
17 files modified
src/platform/graphics/android/android_platform.cpp (+2/-2)
src/platform/graphics/android/display_resource_factory.h (+3/-2)
src/platform/graphics/android/hwc_common_device.cpp (+10/-6)
src/platform/graphics/android/hwc_common_device.h (+3/-3)
src/platform/graphics/android/hwc_wrapper.h (+2/-1)
src/platform/graphics/android/output_builder.cpp (+6/-3)
src/platform/graphics/android/output_builder.h (+5/-1)
src/platform/graphics/android/real_hwc_wrapper.cpp (+4/-2)
src/platform/graphics/android/real_hwc_wrapper.h (+4/-1)
src/platform/graphics/android/resource_factory.cpp (+2/-9)
src/platform/graphics/android/resource_factory.h (+2/-7)
tests/include/mir_test_doubles/mock_hwc_device_wrapper.h (+1/-1)
tests/integration-tests/graphics/android/test_display_integration.cpp (+3/-3)
tests/unit-tests/graphics/android/test_hwc_common_device.cpp (+34/-16)
tests/unit-tests/graphics/android/test_hwc_wrapper.cpp (+13/-5)
tests/unit-tests/graphics/android/test_output_builder.cpp (+11/-8)
tests/unit-tests/graphics/android/test_resource_factory.cpp (+4/-10)
To merge this branch: bzr merge lp:~kdub/mir/fix-1364637
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
Alexandros Frantzis (community) Approve
Alberto Aguirre (community) Approve
Review via email: mp+233512@code.launchpad.net

Commit message

android: preserve the registered callback hooks until the hwc device has been closed.
lp: #1364637

Description of the change

android: preserve the registered callback hooks until the hwc device has been closed.

This seems to be an "incidental" part of the hwc api, as one can register the callback hooks, but cannot unregister or reregister hooks.
The problem manifested itself only on the krillin device.

I considered a few ways to shuffle this resource, including unique_ptrs and beefing up the RealHwcWrapper, this approach seemed the best for future plans (such as multimonitor) and keeping the code churn lower.

lp: #1364637

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

80 + callbacks->self = nullptr;

Doesn't 'self' need be atomic?

45 + hwc_wrapper = std::make_shared<mga::RealHwcWrapper>(hwc_native, logger);

Perhaps have a mga::ResourceFactory::create_hwc_wrapper() instead (and then we probably won't need ResourceFactory::create_hwc_native_device()).

Needs Fixing/Info.

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

PASSED: Continuous integration, rev:1894
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/2635/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/1632
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/1638
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/1614
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/1157
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/1157/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/551
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/551/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/2671
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/12744

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/2635/rebuild

review: Approve (continuous-integration)
Revision history for this message
Kevin DuBois (kdub) wrote :

> 80 + callbacks->self = nullptr;
>
> Doesn't 'self' need be atomic?
yes, it should be, fixed.

> 45 + hwc_wrapper = std::make_shared<mga::RealHwcWrapper>(hwc_native,
> logger);
>
> Perhaps have a mga::ResourceFactory::create_hwc_wrapper() instead (and then we
> probably won't need ResourceFactory::create_hwc_native_device()).

Yes, this is the direction I want to go, to hide the hwc type behind the wrapper. There are a few interfaces that still need to be plumbed out on HwcWrapper before getting rid of the create_hwc_native_device() function, namely detecting the version, and querying some of the other hwc properties. I considered that a bit beyond the scope of this bug fix, so will leave it for future improvement.

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

PASSED: Continuous integration, rev:1895
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/2636/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/1634
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/1640
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/1616
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/1158
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/1158/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/553
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/553/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/2673
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/12763

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/2636/rebuild

review: Approve (continuous-integration)
Revision history for this message
Alberto Aguirre (albaguirre) wrote :

Looks sensible.

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

> > Perhaps have a mga::ResourceFactory::create_hwc_wrapper() instead (and then we
> > probably won't need ResourceFactory::create_hwc_native_device()).
>
> I considered that a bit beyond the scope of this bug fix, so will leave it for future improvement.

OK, looking forward to that then.

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

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-autolanding/1048/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/1660
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/1666
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/1642
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-autolanding/432/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/579
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/579/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/2699
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/12900

review: Needs Fixing (continuous-integration)
Revision history for this message
Kevin DuBois (kdub) wrote :

not sure where the failure was, (seems an unrelated acceptance test stopped running). Will retrigger with regular CI, then top approve again

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

FAILED: Continuous integration, rev:1895
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/2655/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/1661
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/1667
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/1643/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/1177
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/1177/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/580
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/580/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/2700/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/12913

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/2655/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

The failure looks like it's caused by CI. Possibly due to /usr/bin/phablet-config crashing. So apparently not caused by this branch.

Revision history for this message
PS Jenkins bot (ps-jenkins) :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/platform/graphics/android/android_platform.cpp'
2--- src/platform/graphics/android/android_platform.cpp 2014-09-05 03:37:19 +0000
3+++ src/platform/graphics/android/android_platform.cpp 2014-09-05 18:49:42 +0000
4@@ -162,10 +162,10 @@
5 auto overlay_option = should_use_overlay_optimization(*options);
6 logger->log_overlay_optimization(overlay_option);
7 auto buffer_initializer = std::make_shared<mg::NullBufferInitializer>();
8- auto display_resource_factory = std::make_shared<mga::ResourceFactory>(logger);
9+ auto display_resource_factory = std::make_shared<mga::ResourceFactory>();
10 auto fb_allocator = std::make_shared<mga::AndroidGraphicBufferAllocator>(buffer_initializer);
11 auto display_builder = std::make_shared<mga::OutputBuilder>(
12- fb_allocator, display_resource_factory, display_report, overlay_option);
13+ fb_allocator, display_resource_factory, display_report, overlay_option, logger);
14 return std::make_shared<mga::AndroidPlatform>(display_builder, display_report);
15 }
16
17
18=== modified file 'src/platform/graphics/android/display_resource_factory.h'
19--- src/platform/graphics/android/display_resource_factory.h 2014-03-06 06:05:17 +0000
20+++ src/platform/graphics/android/display_resource_factory.h 2014-09-05 18:49:42 +0000
21@@ -34,6 +34,7 @@
22 {
23 class DisplayDevice;
24 class FramebufferBundle;
25+class HwcWrapper;
26
27 class DisplayResourceFactory
28 {
29@@ -49,9 +50,9 @@
30 virtual std::shared_ptr<DisplayDevice> create_fb_device(
31 std::shared_ptr<framebuffer_device_t> const& fb_native_device) const = 0;
32 virtual std::shared_ptr<DisplayDevice> create_hwc_device(
33- std::shared_ptr<hwc_composer_device_1> const& hwc_native_device) const = 0;
34+ std::shared_ptr<HwcWrapper> const& hwc_native_device) const = 0;
35 virtual std::shared_ptr<DisplayDevice> create_hwc_fb_device(
36- std::shared_ptr<hwc_composer_device_1> const& hwc_native_device,
37+ std::shared_ptr<HwcWrapper> const& hwc_native_device,
38 std::shared_ptr<framebuffer_device_t> const& fb_native_device) const = 0;
39
40 protected:
41
42=== modified file 'src/platform/graphics/android/hwc_common_device.cpp'
43--- src/platform/graphics/android/hwc_common_device.cpp 2014-09-05 03:37:19 +0000
44+++ src/platform/graphics/android/hwc_common_device.cpp 2014-09-05 18:49:42 +0000
45@@ -34,7 +34,9 @@
46
47 static void vsync_hook(const struct hwc_procs* procs, int /*disp*/, int64_t /*timestamp*/)
48 {
49- auto self = reinterpret_cast<mga::HWCCallbacks const*>(procs)->self;
50+ auto self = reinterpret_cast<mga::HWCCallbacks const*>(procs)->self.load();
51+ if (!self)
52+ return;
53 self->notify_vsync();
54 }
55
56@@ -46,15 +48,16 @@
57 mga::HWCCommonDevice::HWCCommonDevice(std::shared_ptr<HwcWrapper> const& hwc_device,
58 std::shared_ptr<HWCVsyncCoordinator> const& coordinator)
59 : coordinator(coordinator),
60+ callbacks(std::make_shared<mga::HWCCallbacks>()),
61 hwc_device(hwc_device),
62 current_mode(mir_power_mode_on)
63 {
64- callbacks.hooks.invalidate = invalidate_hook;
65- callbacks.hooks.vsync = vsync_hook;
66- callbacks.hooks.hotplug = hotplug_hook;
67- callbacks.self = this;
68+ callbacks->hooks.invalidate = invalidate_hook;
69+ callbacks->hooks.vsync = vsync_hook;
70+ callbacks->hooks.hotplug = hotplug_hook;
71+ callbacks->self = this;
72
73- hwc_device->register_hooks(&callbacks.hooks);
74+ hwc_device->register_hooks(callbacks);
75
76 try
77 {
78@@ -78,6 +81,7 @@
79 {
80 }
81 }
82+ callbacks->self = nullptr;
83 }
84
85 void mga::HWCCommonDevice::notify_vsync()
86
87=== modified file 'src/platform/graphics/android/hwc_common_device.h'
88--- src/platform/graphics/android/hwc_common_device.h 2014-09-05 03:37:19 +0000
89+++ src/platform/graphics/android/hwc_common_device.h 2014-09-05 18:49:42 +0000
90@@ -25,6 +25,7 @@
91 #include <memory>
92 #include <mutex>
93 #include <condition_variable>
94+#include <atomic>
95
96 namespace mir
97 {
98@@ -39,7 +40,7 @@
99 struct HWCCallbacks
100 {
101 hwc_procs_t hooks;
102- HWCCommonDevice* self;
103+ std::atomic<HWCCommonDevice*> self;
104 };
105
106 class HWCCommonDevice : public DisplayDevice
107@@ -63,8 +64,7 @@
108 void turn_screen_off();
109 virtual void turned_screen_off();
110
111- HWCCallbacks callbacks;
112-
113+ std::shared_ptr<HWCCallbacks> const callbacks;
114 std::shared_ptr<HwcWrapper> const hwc_device;
115
116 std::mutex blanked_mutex;
117
118=== modified file 'src/platform/graphics/android/hwc_wrapper.h'
119--- src/platform/graphics/android/hwc_wrapper.h 2014-09-05 03:37:19 +0000
120+++ src/platform/graphics/android/hwc_wrapper.h 2014-09-05 18:49:42 +0000
121@@ -20,6 +20,7 @@
122 #define MIR_GRAPHICS_ANDROID_HWC_WRAPPER_H_
123
124 #include <hardware/hwcomposer.h>
125+#include <memory>
126
127 namespace mir
128 {
129@@ -35,7 +36,7 @@
130
131 virtual void prepare(hwc_display_contents_1_t&) const = 0;
132 virtual void set(hwc_display_contents_1_t&) const = 0;
133- virtual void register_hooks(hwc_procs_t* callbacks) const = 0;
134+ virtual void register_hooks(std::shared_ptr<HWCCallbacks> const& callbacks) = 0;
135 virtual void vsync_signal_on() const = 0;
136 virtual void vsync_signal_off() const = 0;
137 virtual void display_on() const = 0;
138
139=== modified file 'src/platform/graphics/android/output_builder.cpp'
140--- src/platform/graphics/android/output_builder.cpp 2014-09-05 03:37:19 +0000
141+++ src/platform/graphics/android/output_builder.cpp 2014-09-05 18:49:42 +0000
142@@ -22,6 +22,7 @@
143 #include "display_buffer.h"
144 #include "display_device.h"
145 #include "framebuffers.h"
146+#include "real_hwc_wrapper.h"
147
148 #include "mir/graphics/display_buffer.h"
149 #include "mir/graphics/display_report.h"
150@@ -35,7 +36,8 @@
151 std::shared_ptr<mga::GraphicBufferAllocator> const& buffer_allocator,
152 std::shared_ptr<mga::DisplayResourceFactory> const& res_factory,
153 std::shared_ptr<mg::DisplayReport> const& display_report,
154- mga::OverlayOptimization overlay_optimization)
155+ mga::OverlayOptimization overlay_optimization,
156+ std::shared_ptr<HwcLogger> const& logger)
157 : buffer_allocator(buffer_allocator),
158 res_factory(res_factory),
159 display_report(display_report),
160@@ -45,6 +47,7 @@
161 try
162 {
163 hwc_native = res_factory->create_hwc_native_device();
164+ hwc_wrapper = std::make_shared<mga::RealHwcWrapper>(hwc_native, logger);
165 } catch (...)
166 {
167 force_backup_display = true;
168@@ -83,11 +86,11 @@
169 {
170 if (hwc_native->common.version == HWC_DEVICE_API_VERSION_1_0)
171 {
172- device = res_factory->create_hwc_fb_device(hwc_native, fb_native);
173+ device = res_factory->create_hwc_fb_device(hwc_wrapper, fb_native);
174 }
175 else //versions 1.1, 1.2
176 {
177- device = res_factory->create_hwc_device(hwc_native);
178+ device = res_factory->create_hwc_device(hwc_wrapper);
179 }
180
181 switch (hwc_native->common.version)
182
183=== modified file 'src/platform/graphics/android/output_builder.h'
184--- src/platform/graphics/android/output_builder.h 2014-09-05 03:37:19 +0000
185+++ src/platform/graphics/android/output_builder.h 2014-09-05 18:49:42 +0000
186@@ -35,6 +35,8 @@
187 class DisplayResourceFactory;
188 class GraphicBufferAllocator;
189 class DisplayDevice;
190+class HwcWrapper;
191+class HwcLogger;
192
193 class OutputBuilder : public DisplayBuilder
194 {
195@@ -43,7 +45,8 @@
196 std::shared_ptr<GraphicBufferAllocator> const& buffer_allocator,
197 std::shared_ptr<DisplayResourceFactory> const& res_factory,
198 std::shared_ptr<DisplayReport> const& display_report,
199- OverlayOptimization overlay_option);
200+ OverlayOptimization overlay_option,
201+ std::shared_ptr<HwcLogger> const& logger);
202
203 MirPixelFormat display_format();
204 std::unique_ptr<ConfigurableDisplayBuffer> create_display_buffer(
205@@ -58,6 +61,7 @@
206 std::shared_ptr<FramebufferBundle> framebuffers;
207 bool force_backup_display;
208
209+ std::shared_ptr<HwcWrapper> hwc_wrapper;
210 std::shared_ptr<hwc_composer_device_1> hwc_native;
211 std::shared_ptr<framebuffer_device_t> fb_native;
212 OverlayOptimization overlay_optimization;
213
214=== modified file 'src/platform/graphics/android/real_hwc_wrapper.cpp'
215--- src/platform/graphics/android/real_hwc_wrapper.cpp 2014-09-05 03:37:19 +0000
216+++ src/platform/graphics/android/real_hwc_wrapper.cpp 2014-09-05 18:49:42 +0000
217@@ -17,6 +17,7 @@
218 */
219
220 #include "real_hwc_wrapper.h"
221+#include "hwc_common_device.h"
222 #include "hwc_logger.h"
223 #include <boost/throw_exception.hpp>
224 #include <stdexcept>
225@@ -61,9 +62,10 @@
226 }
227 }
228
229-void mga::RealHwcWrapper::register_hooks(hwc_procs_t* callbacks) const
230+void mga::RealHwcWrapper::register_hooks(std::shared_ptr<HWCCallbacks> const& callbacks)
231 {
232- hwc_device->registerProcs(hwc_device.get(), callbacks);
233+ hwc_device->registerProcs(hwc_device.get(), reinterpret_cast<hwc_procs_t*>(callbacks.get()));
234+ registered_callbacks = callbacks;
235 }
236
237 void mga::RealHwcWrapper::vsync_signal_on() const
238
239=== modified file 'src/platform/graphics/android/real_hwc_wrapper.h'
240--- src/platform/graphics/android/real_hwc_wrapper.h 2014-09-05 03:37:19 +0000
241+++ src/platform/graphics/android/real_hwc_wrapper.h 2014-09-05 18:49:42 +0000
242@@ -39,13 +39,16 @@
243
244 void prepare(hwc_display_contents_1_t&) const override;
245 void set(hwc_display_contents_1_t&) const override;
246- void register_hooks(hwc_procs_t* callbacks) const override;
247+ void register_hooks(std::shared_ptr<HWCCallbacks> const& callbacks) override;
248 void vsync_signal_on() const override;
249 void vsync_signal_off() const override;
250 void display_on() const override;
251 void display_off() const override;
252 private:
253 static size_t const num_displays{3}; //primary, external, virtual
254+ //note: the callbacks have to extend past the lifetime of the hwc_composer_device_1 for some
255+ // devices (LP: 1364637)
256+ std::shared_ptr<HWCCallbacks> registered_callbacks;
257 std::shared_ptr<hwc_composer_device_1> const hwc_device;
258 std::shared_ptr<HwcLogger> const logger;
259 };
260
261=== modified file 'src/platform/graphics/android/resource_factory.cpp'
262--- src/platform/graphics/android/resource_factory.cpp 2014-09-05 03:37:19 +0000
263+++ src/platform/graphics/android/resource_factory.cpp 2014-09-05 18:49:42 +0000
264@@ -39,11 +39,6 @@
265 namespace mg = mir::graphics;
266 namespace mga=mir::graphics::android;
267
268-mga::ResourceFactory::ResourceFactory(std::shared_ptr<HwcLogger> const& logger)
269- : logger{logger}
270-{
271-}
272-
273 std::shared_ptr<framebuffer_device_t> mga::ResourceFactory::create_fb_native_device() const
274 {
275 hw_module_t const* module;
276@@ -95,19 +90,17 @@
277 }
278
279 std::shared_ptr<mga::DisplayDevice> mga::ResourceFactory::create_hwc_device(
280- std::shared_ptr<hwc_composer_device_1> const& hwc_native_device) const
281+ std::shared_ptr<HwcWrapper> const& wrapper) const
282 {
283 auto syncer = std::make_shared<mga::HWCVsync>();
284 auto file_ops = std::make_shared<mga::RealSyncFileOps>();
285- auto wrapper = std::make_shared<mga::RealHwcWrapper>(hwc_native_device, logger);
286 return std::make_shared<mga::HwcDevice>(wrapper, syncer, file_ops);
287 }
288
289 std::shared_ptr<mga::DisplayDevice> mga::ResourceFactory::create_hwc_fb_device(
290- std::shared_ptr<hwc_composer_device_1> const& hwc_native_device,
291+ std::shared_ptr<HwcWrapper> const& wrapper,
292 std::shared_ptr<framebuffer_device_t> const& fb_native_device) const
293 {
294 auto syncer = std::make_shared<mga::HWCVsync>();
295- auto wrapper = std::make_shared<mga::RealHwcWrapper>(hwc_native_device, logger);
296 return std::make_shared<mga::HwcFbDevice>(wrapper, fb_native_device, syncer);
297 }
298
299=== modified file 'src/platform/graphics/android/resource_factory.h'
300--- src/platform/graphics/android/resource_factory.h 2014-09-05 03:37:19 +0000
301+++ src/platform/graphics/android/resource_factory.h 2014-09-05 18:49:42 +0000
302@@ -27,13 +27,10 @@
303 {
304 namespace android
305 {
306-class HwcLogger;
307
308 class ResourceFactory : public DisplayResourceFactory
309 {
310 public:
311- ResourceFactory(std::shared_ptr<HwcLogger> const& logger);
312-
313 //native allocations
314 std::shared_ptr<hwc_composer_device_1> create_hwc_native_device() const;
315 std::shared_ptr<framebuffer_device_t> create_fb_native_device() const;
316@@ -42,15 +39,13 @@
317 std::shared_ptr<DisplayDevice> create_fb_device(
318 std::shared_ptr<framebuffer_device_t> const& fb_native_device) const;
319 std::shared_ptr<DisplayDevice> create_hwc_device(
320- std::shared_ptr<hwc_composer_device_1> const& hwc_native_device) const;
321+ std::shared_ptr<HwcWrapper> const& hwc_native_device) const;
322 std::shared_ptr<DisplayDevice> create_hwc_fb_device(
323- std::shared_ptr<hwc_composer_device_1> const& hwc_native_device,
324+ std::shared_ptr<HwcWrapper> const& hwc_native_device,
325 std::shared_ptr<framebuffer_device_t> const& fb_native_device) const;
326
327 std::shared_ptr<ANativeWindow> create_native_window(
328 std::shared_ptr<FramebufferBundle> const& fb_bundle) const;
329-private:
330- std::shared_ptr<HwcLogger> const logger;
331 };
332
333 }
334
335=== modified file 'tests/include/mir_test_doubles/mock_hwc_device_wrapper.h'
336--- tests/include/mir_test_doubles/mock_hwc_device_wrapper.h 2014-09-05 03:37:19 +0000
337+++ tests/include/mir_test_doubles/mock_hwc_device_wrapper.h 2014-09-05 18:49:42 +0000
338@@ -34,7 +34,7 @@
339 {
340 MOCK_CONST_METHOD1(prepare, void(hwc_display_contents_1_t&));
341 MOCK_CONST_METHOD1(set, void(hwc_display_contents_1_t&));
342- MOCK_CONST_METHOD1(register_hooks, void(hwc_procs_t*));
343+ MOCK_METHOD1(register_hooks, void(std::shared_ptr<graphics::android::HWCCallbacks> const&));
344 MOCK_CONST_METHOD0(vsync_signal_on, void());
345 MOCK_CONST_METHOD0(vsync_signal_off, void());
346 MOCK_CONST_METHOD0(display_on, void());
347
348=== modified file 'tests/integration-tests/graphics/android/test_display_integration.cpp'
349--- tests/integration-tests/graphics/android/test_display_integration.cpp 2014-09-05 03:37:19 +0000
350+++ tests/integration-tests/graphics/android/test_display_integration.cpp 2014-09-05 18:49:42 +0000
351@@ -53,8 +53,7 @@
352 /* note about fb_device: OMAP4 drivers seem to only be able to open fb once
353 per process (repeated framebuffer_{open,close}() doesn't seem to work). once we
354 figure out why, we can remove fb_device in the test fixture */
355- auto logger = std::make_shared<mga::NullHwcLogger>();
356- display_resource_factory = std::make_shared<mga::ResourceFactory>(logger);
357+ display_resource_factory = std::make_shared<mga::ResourceFactory>();
358 }
359
360 static void TearDownTestCase()
361@@ -79,8 +78,9 @@
362 auto stub_gl_config = std::make_shared<mtd::StubGLConfig>();
363 auto buffer_initializer = std::make_shared<mg::NullBufferInitializer>();
364 auto fb_allocator = std::make_shared<mga::AndroidGraphicBufferAllocator>(buffer_initializer);
365+ auto logger = std::make_shared<mga::NullHwcLogger>();
366 auto display_buffer_factory = std::make_shared<mga::OutputBuilder>(
367- fb_allocator, display_resource_factory, null_display_report, mga::OverlayOptimization::disabled);
368+ fb_allocator, display_resource_factory, null_display_report, mga::OverlayOptimization::disabled, logger);
369
370 auto program_factory = std::make_shared<mg::ProgramFactory>();
371 mga::AndroidDisplay display{display_buffer_factory, program_factory, stub_gl_config, null_display_report};
372
373=== modified file 'tests/unit-tests/graphics/android/test_hwc_common_device.cpp'
374--- tests/unit-tests/graphics/android/test_hwc_common_device.cpp 2014-09-05 03:37:19 +0000
375+++ tests/unit-tests/graphics/android/test_hwc_common_device.cpp 2014-09-05 18:49:42 +0000
376@@ -90,18 +90,33 @@
377 TYPED_TEST(HWCCommon, test_proc_registration)
378 {
379 using namespace testing;
380-
381- hwc_procs_t const* procs{nullptr};
382- EXPECT_CALL(*(this->mock_device), register_hooks(_))
383- .Times(1)
384- .WillOnce(SaveArg<0>(&procs));
385-
386- auto device = make_hwc_device<TypeParam>(this->mock_device, this->mock_fbdev, this->mock_vsync);
387-
388- ASSERT_THAT(procs, Ne(nullptr));
389- EXPECT_THAT(procs->invalidate, Ne(nullptr));
390- EXPECT_THAT(procs->vsync, Ne(nullptr));
391- EXPECT_THAT(procs->hotplug, Ne(nullptr));
392+ std::shared_ptr<mga::HWCCallbacks> callbacks;
393+ EXPECT_CALL(*(this->mock_device), register_hooks(_))
394+ .Times(1)
395+ .WillOnce(SaveArg<0>(&callbacks));
396+
397+ auto device = make_hwc_device<TypeParam>(this->mock_device, this->mock_fbdev, this->mock_vsync);
398+
399+ ASSERT_THAT(callbacks, Ne(nullptr));
400+ EXPECT_THAT(callbacks->hooks.invalidate, Ne(nullptr));
401+ EXPECT_THAT(callbacks->hooks.vsync, Ne(nullptr));
402+ EXPECT_THAT(callbacks->hooks.hotplug, Ne(nullptr));
403+}
404+
405+TYPED_TEST(HWCCommon, test_device_destruction_unregisters_self_from_hooks)
406+{
407+ using namespace testing;
408+ std::shared_ptr<mga::HWCCallbacks> callbacks;
409+ EXPECT_CALL(*(this->mock_device), register_hooks(_))
410+ .Times(1)
411+ .WillOnce(SaveArg<0>(&callbacks));
412+
413+ auto device = make_hwc_device<TypeParam>(this->mock_device, this->mock_fbdev, this->mock_vsync);
414+
415+ ASSERT_THAT(callbacks, Ne(nullptr));
416+ EXPECT_THAT(callbacks->self, Eq(device.get()));
417+ device = nullptr;
418+ EXPECT_THAT(callbacks->self, Eq(nullptr));
419 }
420
421 TYPED_TEST(HWCCommon, registerst_hooks_and_turns_on_display)
422@@ -174,17 +189,20 @@
423 TYPED_TEST(HWCCommon, callback_calls_hwcvsync)
424 {
425 using namespace testing;
426-
427- hwc_procs_t const* procs{nullptr};
428+ std::shared_ptr<mga::HWCCallbacks> callbacks;
429 EXPECT_CALL(*(this->mock_device), register_hooks(_))
430 .Times(1)
431- .WillOnce(SaveArg<0>(&procs));
432+ .WillOnce(SaveArg<0>(&callbacks));
433
434 auto device = make_hwc_device<TypeParam>(this->mock_device, this->mock_fbdev, this->mock_vsync);
435
436 EXPECT_CALL(*this->mock_vsync, notify_vsync())
437 .Times(1);
438- procs->vsync(procs, 0, 0);
439+ ASSERT_THAT(callbacks, Ne(nullptr));
440+ callbacks->hooks.vsync(&callbacks->hooks, 0, 0);
441+
442+ callbacks->self = nullptr;
443+ callbacks->hooks.vsync(&callbacks->hooks, 0, 0);
444 }
445
446 TYPED_TEST(HWCCommon, set_orientation)
447
448=== modified file 'tests/unit-tests/graphics/android/test_hwc_wrapper.cpp'
449--- tests/unit-tests/graphics/android/test_hwc_wrapper.cpp 2014-09-05 03:37:19 +0000
450+++ tests/unit-tests/graphics/android/test_hwc_wrapper.cpp 2014-09-05 18:49:42 +0000
451@@ -18,6 +18,7 @@
452
453 #include "src/platform/graphics/android/real_hwc_wrapper.h"
454 #include "src/platform/graphics/android/hwc_logger.h"
455+#include "src/platform/graphics/android/hwc_common_device.h"
456 #include "mir_test_doubles/mock_hwc_composer_device_1.h"
457 #include <gmock/gmock.h>
458 #include <gtest/gtest.h>
459@@ -144,14 +145,21 @@
460 }, std::runtime_error);
461 }
462
463-TEST_F(HwcWrapper, register_procs)
464+TEST_F(HwcWrapper, register_procs_registers_and_preserves_hooks_until_destruction)
465 {
466 using namespace testing;
467- hwc_procs_t procs;
468- EXPECT_CALL(*mock_device, registerProcs_interface(mock_device.get(), &procs))
469+ auto procs = std::make_shared<mga::HWCCallbacks>();
470+ EXPECT_CALL(*mock_device, registerProcs_interface(
471+ mock_device.get(), reinterpret_cast<hwc_procs_t*>(procs.get())))
472 .Times(1);
473- mga::RealHwcWrapper wrapper(mock_device, mock_logger);
474- wrapper.register_hooks(&procs);
475+
476+ auto use_count = procs.use_count();
477+ {
478+ mga::RealHwcWrapper wrapper(mock_device, mock_logger);
479+ wrapper.register_hooks(procs);
480+ EXPECT_THAT(procs.use_count(), Eq(use_count+1));
481+ }
482+ EXPECT_THAT(procs.use_count(), Eq(use_count));
483 }
484
485 TEST_F(HwcWrapper, turns_display_on)
486
487=== modified file 'tests/unit-tests/graphics/android/test_output_builder.cpp'
488--- tests/unit-tests/graphics/android/test_output_builder.cpp 2014-09-05 03:37:19 +0000
489+++ tests/unit-tests/graphics/android/test_output_builder.cpp 2014-09-05 18:49:42 +0000
490@@ -21,6 +21,7 @@
491 #include "src/platform/graphics/android/android_format_conversion-inl.h"
492 #include "src/platform/graphics/android/resource_factory.h"
493 #include "src/platform/graphics/android/graphic_buffer_allocator.h"
494+#include "src/platform/graphics/android/hwc_loggers.h"
495 #include "mir_test_doubles/mock_buffer.h"
496 #include "mir_test_doubles/mock_buffer_packer.h"
497 #include "mir_test_doubles/mock_display_report.h"
498@@ -79,10 +80,10 @@
499 MOCK_CONST_METHOD1(create_fb_device,
500 std::shared_ptr<mga::DisplayDevice>(std::shared_ptr<framebuffer_device_t> const&));
501 MOCK_CONST_METHOD1(create_hwc_device,
502- std::shared_ptr<mga::DisplayDevice>(std::shared_ptr<hwc_composer_device_1> const&));
503+ std::shared_ptr<mga::DisplayDevice>(std::shared_ptr<mga::HwcWrapper> const&));
504 MOCK_CONST_METHOD2(create_hwc_fb_device,
505 std::shared_ptr<mga::DisplayDevice>(
506- std::shared_ptr<hwc_composer_device_1> const&, std::shared_ptr<framebuffer_device_t> const&));
507+ std::shared_ptr<mga::HwcWrapper> const&, std::shared_ptr<framebuffer_device_t> const&));
508 };
509
510 class OutputBuilder : public ::testing::Test
511@@ -109,6 +110,7 @@
512 mga::PbufferGLContext gl_context{
513 mga::to_mir_format(mock_egl.fake_visual_id), stub_gl_config, mock_display_report};
514 mtd::StubGLProgramFactory const stub_program_factory;
515+ std::shared_ptr<mga::HwcLogger> logger{std::make_shared<mga::NullHwcLogger>()};
516 };
517 }
518 TEST_F(OutputBuilder, hwc_version_10_success)
519@@ -129,7 +131,7 @@
520 .Times(1);
521
522 mga::OutputBuilder factory(
523- mt::fake_shared(mock_buffer_allocator),mock_resource_factory, mt::fake_shared(mock_display_report), mga::OverlayOptimization::disabled);
524+ mt::fake_shared(mock_buffer_allocator),mock_resource_factory, mt::fake_shared(mock_display_report), mga::OverlayOptimization::disabled, logger);
525 factory.create_display_buffer(stub_program_factory, gl_context);
526 }
527
528@@ -152,7 +154,7 @@
529 .Times(1);
530
531 mga::OutputBuilder factory(
532- mt::fake_shared(mock_buffer_allocator),mock_resource_factory, mt::fake_shared(mock_display_report), mga::OverlayOptimization::disabled);
533+ mt::fake_shared(mock_buffer_allocator),mock_resource_factory, mt::fake_shared(mock_display_report), mga::OverlayOptimization::disabled, logger);
534 factory.create_display_buffer(stub_program_factory, gl_context);
535 }
536
537@@ -172,7 +174,7 @@
538 .Times(1);
539
540 mga::OutputBuilder factory(
541- mt::fake_shared(mock_buffer_allocator),mock_resource_factory, mt::fake_shared(mock_display_report), mga::OverlayOptimization::disabled);
542+ mt::fake_shared(mock_buffer_allocator),mock_resource_factory, mt::fake_shared(mock_display_report), mga::OverlayOptimization::disabled, logger);
543 factory.create_display_buffer(stub_program_factory, gl_context);
544 }
545
546@@ -195,7 +197,7 @@
547 .Times(1);
548
549 mga::OutputBuilder factory(
550- mt::fake_shared(mock_buffer_allocator),mock_resource_factory, mt::fake_shared(mock_display_report), mga::OverlayOptimization::disabled);
551+ mt::fake_shared(mock_buffer_allocator),mock_resource_factory, mt::fake_shared(mock_display_report), mga::OverlayOptimization::disabled, logger);
552 factory.create_display_buffer(stub_program_factory, gl_context);
553 }
554
555@@ -217,7 +219,8 @@
556 mt::fake_shared(mock_buffer_allocator),
557 mock_resource_factory,
558 mt::fake_shared(mock_display_report),
559- mga::OverlayOptimization::disabled);
560+ mga::OverlayOptimization::disabled,
561+ logger);
562 }, std::runtime_error);
563 }
564
565@@ -235,6 +238,6 @@
566 .Times(1);
567
568 mga::OutputBuilder factory(
569- mt::fake_shared(mock_buffer_allocator),mock_resource_factory, mt::fake_shared(mock_display_report), mga::OverlayOptimization::disabled);
570+ mt::fake_shared(mock_buffer_allocator),mock_resource_factory, mt::fake_shared(mock_display_report), mga::OverlayOptimization::disabled, logger);
571 factory.create_display_buffer(stub_program_factory, gl_context);
572 }
573
574=== modified file 'tests/unit-tests/graphics/android/test_resource_factory.cpp'
575--- tests/unit-tests/graphics/android/test_resource_factory.cpp 2014-09-05 03:37:19 +0000
576+++ tests/unit-tests/graphics/android/test_resource_factory.cpp 2014-09-05 18:49:42 +0000
577@@ -17,7 +17,6 @@
578 */
579
580 #include "src/platform/graphics/android/resource_factory.h"
581-#include "src/platform/graphics/android/hwc_loggers.h"
582 #include "mir_test_doubles/mock_android_hw.h"
583
584 #include <stdexcept>
585@@ -30,12 +29,7 @@
586
587 struct ResourceFactoryTest : public ::testing::Test
588 {
589- ResourceFactoryTest() :
590- logger{std::make_shared<mga::NullHwcLogger>()}
591- {
592- }
593 mtd::HardwareAccessMock hw_access_mock;
594- std::shared_ptr<mga::HwcLogger> const logger;
595 };
596
597 TEST_F(ResourceFactoryTest, fb_native_creation_opens_and_closes_gralloc)
598@@ -44,7 +38,7 @@
599 EXPECT_CALL(hw_access_mock, hw_get_module(StrEq(GRALLOC_HARDWARE_MODULE_ID), _))
600 .Times(1);
601
602- mga::ResourceFactory factory{logger};
603+ mga::ResourceFactory factory;
604 factory.create_fb_native_device();
605 EXPECT_TRUE(hw_access_mock.open_count_matches_close());
606 }
607@@ -52,7 +46,7 @@
608 TEST_F(ResourceFactoryTest, test_device_creation_throws_on_failure)
609 {
610 using namespace testing;
611- mga::ResourceFactory factory{logger};
612+ mga::ResourceFactory factory;
613
614 /* failure because of rc */
615 EXPECT_CALL(hw_access_mock, hw_get_module(StrEq(GRALLOC_HARDWARE_MODULE_ID), _))
616@@ -79,7 +73,7 @@
617 EXPECT_CALL(hw_access_mock, hw_get_module(StrEq(HWC_HARDWARE_MODULE_ID), _))
618 .Times(1);
619
620- mga::ResourceFactory factory{logger};
621+ mga::ResourceFactory factory;
622 factory.create_hwc_native_device();
623
624 EXPECT_TRUE(hw_access_mock.open_count_matches_close());
625@@ -96,7 +90,7 @@
626 .WillOnce(Return(-1))
627 .WillOnce(DoAll(SetArgPointee<1>(&failing_hwc_module_stub), Return(0)));
628
629- mga::ResourceFactory factory{logger};
630+ mga::ResourceFactory factory;
631
632 EXPECT_THROW({
633 factory.create_hwc_native_device();

Subscribers

People subscribed via source and target branches