Merge lp:~kdub/mir/add-sync-support into lp:~mir-team/mir/trunk
- add-sync-support
- Merge into trunk
Status: | Merged |
---|---|
Approved by: | Alexandros Frantzis |
Approved revision: | no longer in the source branch. |
Merged at revision: | 501 |
Proposed branch: | lp:~kdub/mir/add-sync-support |
Merge into: | lp:~mir-team/mir/trunk |
Diff against target: |
478 lines (+275/-15) 12 files modified
src/client/CMakeLists.txt (+1/-0) src/client/android/android_driver_interpreter.h (+1/-1) src/client/android/client_surface_interpreter.cpp (+1/-1) src/client/android/client_surface_interpreter.h (+1/-1) src/client/android/mir_native_window.cpp (+27/-4) src/client/android/mir_native_window.h (+2/-1) src/client/android/syncfence.cpp (+44/-0) src/client/android/syncfence.h (+76/-0) tests/unit-tests/client/CMakeLists.txt (+1/-0) tests/unit-tests/client/test_android_native_window.cpp (+29/-5) tests/unit-tests/client/test_android_syncfence.cpp (+91/-0) tests/unit-tests/client/test_client_surface_interpreter.cpp (+1/-2) |
To merge this branch: | bzr merge lp:~kdub/mir/add-sync-support |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Alexandros Frantzis (community) | Approve | ||
PS Jenkins bot (community) | continuous-integration | Approve | |
Alan Griffiths | Abstain | ||
Review via email: mp+153017@code.launchpad.net |
Commit message
add sync driver support to the android client
Description of the change
android uses the 'sync' driver available on the android kernel for additional synchronization with the driver. This support was added in the upgrade to the native window type for android. The requirement on us is that we wait on the sync object (via ioctl) and that we close the sync object. After we do this, we get stable client accelerated rendering.
The sync driver is open source in the android kernel. However, the android kernel headers are not available as a package (correct me if I'm wrong :) ), so I scraped the ioctl value and put it in the source, like the android sources do.
PS Jenkins bot (ps-jenkins) wrote : | # |
Alexandros Frantzis (afrantzis) wrote : | # |
243 + SyncFence(int fd);
244 +
245 + //override using real ioctl's with something else. useful for injecting mocks
246 + SyncFence(int fd, std::shared_
My preference (as stated in other MPs, too), is to not use default arguments or overloaded constructors as a way to provide default dependencies. The reasons are reduced design clarity (does SyncFence depend on IoctlWrapper or not?), and the introduction of different (even if trivially different) code paths, some of which are not as well tested as the others.
My suggestion is to keep only the "full" constructor and provide an IoctlWrapper implementation at SyncFence construction in mir_native_
225 +class IoctlWrapper
226 +{
233 +class AndroidFence
234 +{
Do we need the copy constructor and assignment operator for these?
252 + std::shared_
shared_ptr const
466 +
Nitpick: Do we need this added space here?
Alexandros Frantzis (afrantzis) wrote : | # |
Also, I wonder if something like this would be a better fit than IoctlWrapper:
FenceFDWrapper
{
virtual void sync() = 0;
virtual void close() = 0;
};
FenceFDImpl : FenceFDWrapper
{
FenceFDImpl(int fd) : fd{fd} {}
void sync() { if (fd >= 0) ioctl(fd, SYNC_IOC_WAIT, &timeout); }
void close() { close(fd) }
};
Alan Griffiths (alan-griffiths) wrote : | # |
> 243 + SyncFence(int fd);
> 244 +
> 245 + //override using real ioctl's with something else. useful for
> injecting mocks
> 246 + SyncFence(int fd, std::shared_
>
> My preference (as stated in other MPs, too), is to not use default arguments
> or overloaded constructors as a way to provide default dependencies. The
> reasons are reduced design clarity (does SyncFence depend on IoctlWrapper or
> not?), and the introduction of different (even if trivially different) code
> paths, some of which are not as well tested as the others.
>
> My suggestion is to keep only the "full" constructor and provide an
> IoctlWrapper implementation at SyncFence construction in
> mir_native_
>
> 225 +class IoctlWrapper
> 226 +{
> 233 +class AndroidFence
> 234 +{
>
> Do we need the copy constructor and assignment operator for these?
>
> 252 + std::shared_
>
> shared_ptr const
>
> 466 +
>
> Nitpick: Do we need this added space here?
+1
Also, not directly aimed at this MP - why don't we have a FileHandle type instead of passing naked ints around?
Kevin DuBois (kdub) wrote : | # |
> 243 + SyncFence(int fd);
> 244 +
> 245 + //override using real ioctl's with something else. useful for
> injecting mocks
> 246 + SyncFence(int fd, std::shared_
>
> My preference (as stated in other MPs, too), is to not use default arguments
> or overloaded constructors as a way to provide default dependencies. The
> reasons are reduced design clarity (does SyncFence depend on IoctlWrapper or
> not?), and the introduction of different (even if trivially different) code
> paths, some of which are not as well tested as the others.
>
> My suggestion is to keep only the "full" constructor and provide an
> IoctlWrapper implementation at SyncFence construction in
> mir_native_
I went with this suggestion over the FenceFDWrapper suggestion. I wanted to test right at the point that we call close/ioctl, so we can test how we handle invalid fd's directly.
>
> 225 +class IoctlWrapper
> 226 +{
> 233 +class AndroidFence
> 234 +{
>
> Do we need the copy constructor and assignment operator for these?
no, i have deleted copy/assign
>
> 252 + std::shared_
>
> shared_ptr const
>
> 466 +
>
> Nitpick: Do we need this added space here?
addressed, (plus made the 'int fd' const)
Kevin DuBois (kdub) wrote : | # |
>
> +1
>
> Also, not directly aimed at this MP - why don't we have a FileHandle type
> instead of passing naked ints around?
My intention with SyncFence was to wrap the FD as soon as we got it. SyncFence is more specialized than a generic type, but I think that's ok for now
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:507
http://
Executed test runs:
SUCCESS: http://
deb: http://
Click here to trigger a rebuild:
http://
Alexandros Frantzis (afrantzis) wrote : | # |
Looks good overall. Some style nitpicks: https:/
(It was quicker to fix the code than propose the fixes :))
Alan Griffiths (alan-griffiths) wrote : | # |
> Looks good overall. Some style nitpicks: https:/
> team/mir/
>
> (It was quicker to fix the code than propose the fixes :))
With that tidy-up I'd be satisfied.
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:508
http://
Executed test runs:
SUCCESS: http://
deb: http://
Click here to trigger a rebuild:
http://
Alexandros Frantzis (afrantzis) wrote : | # |
Looks good.
Preview Diff
1 | === modified file 'src/client/CMakeLists.txt' |
2 | --- src/client/CMakeLists.txt 2013-03-13 11:24:59 +0000 |
3 | +++ src/client/CMakeLists.txt 2013-03-14 15:40:25 +0000 |
4 | @@ -59,6 +59,7 @@ |
5 | android/android_client_platform.cpp |
6 | android/mir_native_window.cpp |
7 | android/client_surface_interpreter.cpp |
8 | + android/syncfence.cpp |
9 | ${CLIENT_SOURCES} |
10 | ) |
11 | endif() |
12 | |
13 | === modified file 'src/client/android/android_driver_interpreter.h' |
14 | --- src/client/android/android_driver_interpreter.h 2013-03-12 15:27:46 +0000 |
15 | +++ src/client/android/android_driver_interpreter.h 2013-03-14 15:40:25 +0000 |
16 | @@ -31,7 +31,7 @@ |
17 | { |
18 | public: |
19 | virtual ANativeWindowBuffer* driver_requests_buffer() = 0; |
20 | - virtual void driver_returns_buffer(ANativeWindowBuffer*, int fence_fd) = 0; |
21 | + virtual void driver_returns_buffer(ANativeWindowBuffer*) = 0; |
22 | virtual void dispatch_driver_request_format(int format) = 0; |
23 | virtual int driver_requests_info(int key) const = 0; |
24 | protected: |
25 | |
26 | === modified file 'src/client/android/client_surface_interpreter.cpp' |
27 | --- src/client/android/client_surface_interpreter.cpp 2013-03-12 15:27:46 +0000 |
28 | +++ src/client/android/client_surface_interpreter.cpp 2013-03-14 15:40:25 +0000 |
29 | @@ -39,7 +39,7 @@ |
30 | |
31 | static void empty(MirSurface * /*surface*/, void * /*client_context*/) |
32 | {} |
33 | -void mcla::ClientSurfaceInterpreter::driver_returns_buffer(ANativeWindowBuffer*, int /*fence_fd*/) |
34 | +void mcla::ClientSurfaceInterpreter::driver_returns_buffer(ANativeWindowBuffer*) |
35 | { |
36 | mir_wait_for(surface.next_buffer(empty, NULL)); |
37 | } |
38 | |
39 | === modified file 'src/client/android/client_surface_interpreter.h' |
40 | --- src/client/android/client_surface_interpreter.h 2013-03-12 15:27:46 +0000 |
41 | +++ src/client/android/client_surface_interpreter.h 2013-03-14 15:40:25 +0000 |
42 | @@ -35,7 +35,7 @@ |
43 | explicit ClientSurfaceInterpreter(ClientSurface& surface); |
44 | |
45 | ANativeWindowBuffer* driver_requests_buffer(); |
46 | - void driver_returns_buffer(ANativeWindowBuffer*, int fence_fd); |
47 | + void driver_returns_buffer(ANativeWindowBuffer*); |
48 | void dispatch_driver_request_format(int format); |
49 | int driver_requests_info(int key) const; |
50 | private: |
51 | |
52 | === modified file 'src/client/android/mir_native_window.cpp' |
53 | --- src/client/android/mir_native_window.cpp 2013-03-12 16:43:03 +0000 |
54 | +++ src/client/android/mir_native_window.cpp 2013-03-14 15:40:25 +0000 |
55 | @@ -18,12 +18,30 @@ |
56 | |
57 | #include "mir_native_window.h" |
58 | #include "android_driver_interpreter.h" |
59 | +#include "syncfence.h" |
60 | + |
61 | +#include <unistd.h> |
62 | +#include <sys/ioctl.h> |
63 | |
64 | namespace mcl=mir::client; |
65 | namespace mcla=mir::client::android; |
66 | |
67 | namespace |
68 | { |
69 | + |
70 | +class IoctlControl : public mcla::IoctlWrapper |
71 | +{ |
72 | +public: |
73 | + int ioctl(int fd, unsigned long int request, int* timeout) const |
74 | + { |
75 | + return ::ioctl(fd, request, timeout); |
76 | + } |
77 | + int close(int fd) const |
78 | + { |
79 | + return ::close(fd); |
80 | + } |
81 | +}; |
82 | + |
83 | static int query_static(const ANativeWindow* anw, int key, int* value); |
84 | static int perform_static(ANativeWindow* anw, int key, ...); |
85 | static int setSwapInterval_static (struct ANativeWindow* window, int interval); |
86 | @@ -82,14 +100,18 @@ |
87 | struct ANativeWindowBuffer* buffer) |
88 | { |
89 | auto self = static_cast<mcla::MirNativeWindow*>(window); |
90 | - return self->queueBuffer(buffer, -1); |
91 | + auto ioctl_control = std::make_shared<IoctlControl>(); |
92 | + auto fence = std::make_shared<mcla::SyncFence>(-1, ioctl_control); |
93 | + return self->queueBuffer(buffer, fence); |
94 | } |
95 | |
96 | int queueBuffer_static(struct ANativeWindow* window, |
97 | struct ANativeWindowBuffer* buffer, int fence_fd) |
98 | { |
99 | auto self = static_cast<mcla::MirNativeWindow*>(window); |
100 | - return self->queueBuffer(buffer, fence_fd); |
101 | + auto ioctl_control = std::make_shared<IoctlControl>(); |
102 | + auto fence = std::make_shared<mcla::SyncFence>(fence_fd, ioctl_control); |
103 | + return self->queueBuffer(buffer, fence); |
104 | |
105 | } |
106 | |
107 | @@ -146,9 +168,10 @@ |
108 | return 0; |
109 | } |
110 | |
111 | -int mcla::MirNativeWindow::queueBuffer(struct ANativeWindowBuffer* buffer, int fence_fd) |
112 | +int mcla::MirNativeWindow::queueBuffer(struct ANativeWindowBuffer* buffer, std::shared_ptr<mcla::AndroidFence> const& fence) |
113 | { |
114 | - driver_interpreter->driver_returns_buffer(buffer, fence_fd); |
115 | + fence->wait(); |
116 | + driver_interpreter->driver_returns_buffer(buffer); |
117 | return 0; |
118 | } |
119 | |
120 | |
121 | === modified file 'src/client/android/mir_native_window.h' |
122 | --- src/client/android/mir_native_window.h 2013-03-12 16:43:03 +0000 |
123 | +++ src/client/android/mir_native_window.h 2013-03-14 15:40:25 +0000 |
124 | @@ -30,6 +30,7 @@ |
125 | { |
126 | namespace android |
127 | { |
128 | +class AndroidFence; |
129 | class AndroidDriverInterpreter; |
130 | |
131 | class MirNativeWindow : public ANativeWindow |
132 | @@ -40,7 +41,7 @@ |
133 | int query(int key, int* value) const; |
134 | int perform(int key, va_list args ); |
135 | int dequeueBuffer(struct ANativeWindowBuffer** buffer); |
136 | - int queueBuffer(struct ANativeWindowBuffer* buffer, int fence_fd); |
137 | + int queueBuffer(struct ANativeWindowBuffer* buffer, std::shared_ptr<AndroidFence> const& fence); |
138 | private: |
139 | |
140 | std::shared_ptr<AndroidDriverInterpreter> const driver_interpreter; |
141 | |
142 | === added file 'src/client/android/syncfence.cpp' |
143 | --- src/client/android/syncfence.cpp 1970-01-01 00:00:00 +0000 |
144 | +++ src/client/android/syncfence.cpp 2013-03-14 15:40:25 +0000 |
145 | @@ -0,0 +1,44 @@ |
146 | +/* |
147 | + * Copyright © 2013 Canonical Ltd. |
148 | + * |
149 | + * This program is free software: you can redistribute it and/or modify |
150 | + * it under the terms of the GNU General Public License version 3 as |
151 | + * published by the Free Software Foundation. |
152 | + * |
153 | + * This program is distributed in the hope that it will be useful, |
154 | + * but WITHOUT ANY WARRANTY; without even the implied warranty of |
155 | + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the |
156 | + * GNU General Public License for more details. |
157 | + * |
158 | + * You should have received a copy of the GNU General Public License |
159 | + * along with this program. If not, see <http://www.gnu.org/licenses/>. |
160 | + * |
161 | + * Authored by: Kevin DuBois <kevin.dubois@canonical.com> |
162 | + */ |
163 | + |
164 | +#include "syncfence.h" |
165 | + |
166 | +namespace mcla = mir::client::android; |
167 | + |
168 | +mcla::SyncFence::SyncFence(int fd, std::shared_ptr<IoctlWrapper> const& wrapper) |
169 | + : ioctl_wrapper(wrapper), |
170 | + fence_fd(fd) |
171 | +{ |
172 | +} |
173 | + |
174 | +mcla::SyncFence::~SyncFence() |
175 | +{ |
176 | + if (fence_fd > 0) |
177 | + { |
178 | + ioctl_wrapper->close(fence_fd); |
179 | + } |
180 | +} |
181 | + |
182 | +void mcla::SyncFence::wait() |
183 | +{ |
184 | + if (fence_fd > 0) |
185 | + { |
186 | + int timeout = -1; |
187 | + ioctl_wrapper->ioctl(fence_fd, SYNC_IOC_WAIT, &timeout); |
188 | + } |
189 | +} |
190 | |
191 | === added file 'src/client/android/syncfence.h' |
192 | --- src/client/android/syncfence.h 1970-01-01 00:00:00 +0000 |
193 | +++ src/client/android/syncfence.h 2013-03-14 15:40:25 +0000 |
194 | @@ -0,0 +1,76 @@ |
195 | +/* |
196 | + * Copyright © 2013 Canonical Ltd. |
197 | + * |
198 | + * This program is free software: you can redistribute it and/or modify |
199 | + * it under the terms of the GNU General Public License version 3 as |
200 | + * published by the Free Software Foundation. |
201 | + * |
202 | + * This program is distributed in the hope that it will be useful, |
203 | + * but WITHOUT ANY WARRANTY; without even the implied warranty of |
204 | + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the |
205 | + * GNU General Public License for more details. |
206 | + * |
207 | + * You should have received a copy of the GNU General Public License |
208 | + * along with this program. If not, see <http://www.gnu.org/licenses/>. |
209 | + * |
210 | + * Authored by: Kevin DuBois <kevin.dubois@canonical.com> |
211 | + */ |
212 | +#ifndef MIR_CLIENT_ANDROID_SYNCFENCE_H_ |
213 | +#define MIR_CLIENT_ANDROID_SYNCFENCE_H_ |
214 | + |
215 | +#include <memory> |
216 | + |
217 | +#define SYNC_IOC_WAIT 0x40043E00 |
218 | + |
219 | +namespace mir |
220 | +{ |
221 | +namespace client |
222 | +{ |
223 | +namespace android |
224 | +{ |
225 | + |
226 | +class IoctlWrapper |
227 | +{ |
228 | +public: |
229 | + virtual ~IoctlWrapper() {} |
230 | + virtual int ioctl(int fd, unsigned long int request, int* timeout) const = 0; |
231 | + virtual int close(int fd) const = 0; |
232 | + |
233 | +protected: |
234 | + IoctlWrapper() = default; |
235 | + IoctlWrapper(IoctlWrapper const&) = delete; |
236 | + IoctlWrapper& operator=(IoctlWrapper const&) = delete; |
237 | +}; |
238 | + |
239 | +class AndroidFence |
240 | +{ |
241 | +public: |
242 | + virtual ~AndroidFence() {} |
243 | + virtual void wait() = 0; |
244 | + |
245 | +protected: |
246 | + AndroidFence() = default; |
247 | + AndroidFence(AndroidFence const&) = delete; |
248 | + AndroidFence& operator=(AndroidFence const&) = delete; |
249 | +}; |
250 | + |
251 | +class SyncFence : public AndroidFence |
252 | +{ |
253 | +public: |
254 | + SyncFence(int fd, std::shared_ptr<IoctlWrapper> const& wrapper); |
255 | + ~SyncFence(); |
256 | + |
257 | + void wait(); |
258 | + |
259 | +private: |
260 | + SyncFence(SyncFence const&) = delete; |
261 | + SyncFence& operator=(SyncFence const&) = delete; |
262 | + |
263 | + std::shared_ptr<IoctlWrapper> const ioctl_wrapper; |
264 | + int const fence_fd; |
265 | +}; |
266 | + |
267 | +} |
268 | +} |
269 | +} |
270 | +#endif /* MIR_CLIENT_ANDROID_SYNCFENCE_H_ */ |
271 | |
272 | === modified file 'tests/unit-tests/client/CMakeLists.txt' |
273 | --- tests/unit-tests/client/CMakeLists.txt 2013-03-12 16:43:03 +0000 |
274 | +++ tests/unit-tests/client/CMakeLists.txt 2013-03-14 15:40:25 +0000 |
275 | @@ -14,6 +14,7 @@ |
276 | ${CMAKE_CURRENT_SOURCE_DIR}/test_android_client_depository.cpp |
277 | ${CMAKE_CURRENT_SOURCE_DIR}/test_android_client_platform.cpp |
278 | ${CMAKE_CURRENT_SOURCE_DIR}/test_client_surface_interpreter.cpp |
279 | + ${CMAKE_CURRENT_SOURCE_DIR}/test_android_syncfence.cpp |
280 | ) |
281 | endif() |
282 | |
283 | |
284 | === modified file 'tests/unit-tests/client/test_android_native_window.cpp' |
285 | --- tests/unit-tests/client/test_android_native_window.cpp 2013-03-12 16:43:03 +0000 |
286 | +++ tests/unit-tests/client/test_android_native_window.cpp 2013-03-14 15:40:25 +0000 |
287 | @@ -19,6 +19,7 @@ |
288 | #include "src/client/mir_client_surface.h" |
289 | #include "src/client/android/mir_native_window.h" |
290 | #include "src/client/android/android_driver_interpreter.h" |
291 | +#include "src/client/android/syncfence.h" |
292 | |
293 | #include <gmock/gmock.h> |
294 | #include <gtest/gtest.h> |
295 | @@ -27,11 +28,17 @@ |
296 | |
297 | namespace |
298 | { |
299 | + |
300 | +struct MockSyncFence : public mcla::AndroidFence |
301 | +{ |
302 | + MOCK_METHOD0(wait, void()); |
303 | +}; |
304 | + |
305 | class MockAndroidDriverInterpreter : public mcla::AndroidDriverInterpreter |
306 | { |
307 | public: |
308 | MOCK_METHOD0(driver_requests_buffer, ANativeWindowBuffer*()); |
309 | - MOCK_METHOD2(driver_returns_buffer, void(ANativeWindowBuffer*, int)); |
310 | + MOCK_METHOD1(driver_returns_buffer, void(ANativeWindowBuffer*)); |
311 | MOCK_METHOD1(dispatch_driver_request_format, void(int)); |
312 | MOCK_CONST_METHOD1(driver_requests_info, int(int)); |
313 | }; |
314 | @@ -166,13 +173,30 @@ |
315 | window->queueBuffer(window.get(), tmp, -1); |
316 | } |
317 | |
318 | -TEST_F(AndroidNativeWindowTest, native_window_queue_passes_buffer_and_fence_back) |
319 | +TEST_F(AndroidNativeWindowTest, native_window_queue_waits_for_sync_before_returning_buffer) |
320 | +{ |
321 | + using namespace testing; |
322 | + ANativeWindowBuffer* tmp = nullptr; |
323 | + auto window = std::make_shared<mcla::MirNativeWindow>(mock_driver_interpreter); |
324 | + |
325 | + auto mock_fence = std::make_shared<testing::NiceMock<MockSyncFence>>(); |
326 | + |
327 | + InSequence s; |
328 | + EXPECT_CALL(*mock_fence, wait()) |
329 | + .Times(1); |
330 | + EXPECT_CALL(*mock_driver_interpreter, driver_returns_buffer(_)) |
331 | + .Times(1); |
332 | + |
333 | + window->queueBuffer(tmp, mock_fence); |
334 | +} |
335 | + |
336 | +TEST_F(AndroidNativeWindowTest, native_window_queue_passes_buffer_back) |
337 | { |
338 | ANativeWindowBuffer buffer; |
339 | int fence_fd = 33; |
340 | std::shared_ptr<ANativeWindow> window = std::make_shared<mcla::MirNativeWindow>(mock_driver_interpreter); |
341 | |
342 | - EXPECT_CALL(*mock_driver_interpreter, driver_returns_buffer(&buffer, fence_fd)) |
343 | + EXPECT_CALL(*mock_driver_interpreter, driver_returns_buffer(&buffer)) |
344 | .Times(1); |
345 | |
346 | window->queueBuffer(window.get(), &buffer, fence_fd); |
347 | @@ -187,12 +211,12 @@ |
348 | window->queueBuffer_DEPRECATED(window.get(), tmp); |
349 | } |
350 | |
351 | -TEST_F(AndroidNativeWindowTest, native_window_queue_deprecated_always_indicates_no_wait) |
352 | +TEST_F(AndroidNativeWindowTest, native_window_queue_deprecated_passes_buffer_back) |
353 | { |
354 | ANativeWindowBuffer buffer; |
355 | std::shared_ptr<ANativeWindow> window = std::make_shared<mcla::MirNativeWindow>(mock_driver_interpreter); |
356 | |
357 | - EXPECT_CALL(*mock_driver_interpreter, driver_returns_buffer(&buffer, -1)) |
358 | + EXPECT_CALL(*mock_driver_interpreter, driver_returns_buffer(&buffer)) |
359 | .Times(1); |
360 | |
361 | window->queueBuffer_DEPRECATED(window.get(), &buffer); |
362 | |
363 | === added file 'tests/unit-tests/client/test_android_syncfence.cpp' |
364 | --- tests/unit-tests/client/test_android_syncfence.cpp 1970-01-01 00:00:00 +0000 |
365 | +++ tests/unit-tests/client/test_android_syncfence.cpp 2013-03-14 15:40:25 +0000 |
366 | @@ -0,0 +1,91 @@ |
367 | +/* |
368 | + * Copyright © 2013 Canonical Ltd. |
369 | + * |
370 | + * This program is free software: you can redistribute it and/or modify |
371 | + * it under the terms of the GNU General Public License version 3 as |
372 | + * published by the Free Software Foundation. |
373 | + * |
374 | + * This program is distributed in the hope that it will be useful, |
375 | + * but WITHOUT ANY WARRANTY; without even the implied warranty of |
376 | + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the |
377 | + * GNU General Public License for more details. |
378 | + * |
379 | + * You should have received a copy of the GNU General Public License |
380 | + * along with this program. If not, see <http://www.gnu.org/licenses/>. |
381 | + * |
382 | + * Authored by: Kevin DuBois <kevin.dubois@canonical.com> |
383 | + */ |
384 | + |
385 | +#include "src/client/android/syncfence.h" |
386 | + |
387 | +#include <gmock/gmock.h> |
388 | +#include <gtest/gtest.h> |
389 | + |
390 | +namespace mcla = mir::client::android; |
391 | + |
392 | +namespace |
393 | +{ |
394 | +class MockIoctlWrapper : public mcla::IoctlWrapper |
395 | +{ |
396 | +public: |
397 | + MOCK_CONST_METHOD3(ioctl, int(int, unsigned long int, int*)); |
398 | + MOCK_CONST_METHOD1(close, int(int)); |
399 | +}; |
400 | +} |
401 | + |
402 | +class SyncFenceTest : public ::testing::Test |
403 | +{ |
404 | +public: |
405 | + SyncFenceTest() |
406 | + : ioctl_mock(std::make_shared<testing::NiceMock<MockIoctlWrapper>>()), |
407 | + dummyfd(44), invalid_fd(-1) |
408 | + { |
409 | + } |
410 | + |
411 | + std::shared_ptr<MockIoctlWrapper> const ioctl_mock; |
412 | + int const dummyfd; |
413 | + int const invalid_fd; |
414 | +}; |
415 | + |
416 | +TEST_F(SyncFenceTest, test_valid_fd_wait) |
417 | +{ |
418 | + using namespace testing; |
419 | + |
420 | + int timeout_val; |
421 | + mcla::SyncFence fence(dummyfd, ioctl_mock); |
422 | + EXPECT_CALL(*ioctl_mock, ioctl(dummyfd, SYNC_IOC_WAIT, _)) |
423 | + .Times(1) |
424 | + .WillOnce(DoAll(SaveArgPointee<2>(&timeout_val), |
425 | + Return(0))); |
426 | + |
427 | + fence.wait(); |
428 | + EXPECT_GT(0, timeout_val); |
429 | +} |
430 | + |
431 | +TEST_F(SyncFenceTest, test_valid_fd_destruction_closes_fd) |
432 | +{ |
433 | + EXPECT_CALL(*ioctl_mock, close(dummyfd)) |
434 | + .Times(1); |
435 | + |
436 | + mcla::SyncFence fence(dummyfd, ioctl_mock); |
437 | +} |
438 | + |
439 | +TEST_F(SyncFenceTest, test_invalid_fd_does_not_call_ioctl_on_wait) |
440 | +{ |
441 | + using namespace testing; |
442 | + |
443 | + mcla::SyncFence fence(invalid_fd, ioctl_mock); |
444 | + EXPECT_CALL(*ioctl_mock, ioctl(_, _, _)) |
445 | + .Times(0); |
446 | + |
447 | + fence.wait(); |
448 | +} |
449 | + |
450 | +TEST_F(SyncFenceTest, test_invalid_fd_destruction_does_not_close_fd) |
451 | +{ |
452 | + using namespace testing; |
453 | + EXPECT_CALL(*ioctl_mock, close(_)) |
454 | + .Times(0); |
455 | + |
456 | + mcla::SyncFence fence(invalid_fd, ioctl_mock); |
457 | +} |
458 | |
459 | === modified file 'tests/unit-tests/client/test_client_surface_interpreter.cpp' |
460 | --- tests/unit-tests/client/test_client_surface_interpreter.cpp 2013-03-12 15:27:46 +0000 |
461 | +++ tests/unit-tests/client/test_client_surface_interpreter.cpp 2013-03-14 15:40:25 +0000 |
462 | @@ -125,7 +125,6 @@ |
463 | { |
464 | using namespace testing; |
465 | ANativeWindowBuffer buffer; |
466 | - int fence_fd = -1; |
467 | |
468 | MockMirSurface mock_surface{surf_params}; |
469 | mcla::ClientSurfaceInterpreter interpreter(mock_surface); |
470 | @@ -133,7 +132,7 @@ |
471 | EXPECT_CALL(mock_surface, next_buffer(_,_)) |
472 | .Times(1); |
473 | |
474 | - interpreter.driver_returns_buffer(&buffer, fence_fd); |
475 | + interpreter.driver_returns_buffer(&buffer); |
476 | } |
477 | |
478 | /* format is an int that is set by the driver. these are not the HAL_PIXEL_FORMATS in android */ |
PASSED: Continuous integration, rev:504 jenkins. qa.ubuntu. com/job/ mir-ci/ 60/ jenkins. qa.ubuntu. com/job/ mir-quantal- amd64-ci/ 61//console
http://
Executed test runs:
SUCCESS: http://
Click here to trigger a rebuild: jenkins. qa.ubuntu. com/job/ mir-ci/ 60//rebuild/?
http://