Mir

Merge lp:~kdub/mir/mali-client-render-support into lp:mir

Proposed by Kevin DuBois on 2013-09-24
Status: Merged
Approved by: Kevin DuBois on 2013-11-12
Approved revision: 1085
Merged at revision: 1220
Proposed branch: lp:~kdub/mir/mali-client-render-support
Merge into: lp:mir
Diff against target: 525 lines (+145/-67)
12 files modified
include/shared/mir/geometry/dimensions.h (+1/-0)
include/shared/mir/graphics/android/mir_native_window.h (+1/-0)
include/test/mir_test/draw/draw_pattern_checkered-inl.h (+10/-5)
src/client/android/android_client_buffer.cpp (+2/-0)
src/client/android/client_surface_interpreter.cpp (+1/-1)
src/shared/graphics/android/mir_native_window.cpp (+13/-6)
tests/draw/android_graphics.cpp (+2/-1)
tests/draw/patterns.cpp (+12/-10)
tests/integration-tests/client/test_client_render.cpp (+63/-33)
tests/unit-tests/client/android/test_android_native_window.cpp (+14/-0)
tests/unit-tests/client/android/test_client_surface_interpreter.cpp (+16/-0)
tests/unit-tests/draw/test_draw_patterns.cpp (+10/-11)
To merge this branch: bzr merge lp:~kdub/mir/mali-client-render-support
Reviewer Review Type Date Requested Status
Alan Griffiths 2013-09-24 Approve on 2013-11-12
Alexandros Frantzis (community) Approve on 2013-11-12
Daniel van Vugt Approve on 2013-11-12
Kevin DuBois (community) Abstain on 2013-11-11
PS Jenkins bot (community) continuous-integration 2013-09-24 Approve on 2013-11-05
Robert Carr 2013-09-24 Pending
Review via email: mp+187349@code.launchpad.net

This proposal supersedes a proposal from 2013-09-13.

Commit message

android: support driver hooks for the Mali T604 (present in nexus 10)

The exynos driver needed some function hooks implemented. This change implements those hooks and gets the TestClientIPCRender test to pass. This test sends buffers over IPC to a client, and the client then establishes an egl context, renders to the buffer, and checks the buffer content back on the server side.

Description of the change

android: support driver hooks for the Mali T604 (present in nexus 10)

The exynos driver needed some function hooks implemented. This change implements those hooks and gets the TestClientIPCRender test to pass. This test sends buffers over IPC to a client, and the client then establishes an egl context, renders to the buffer, and checks the buffer content back on the server side.

Please note: the nexus 10 display is still broken (so running a demo server won't go well), but supporting the hooks for the driver is the first step.

To post a comment you must log in.
Alan Griffiths (alan-griffiths) wrote : Posted in a previous version of this proposal

On android (N4) I get:
./unit-tests
...
[ PASSED ] 730 tests.
[ FAILED ] 1 test, listed below:
[ FAILED ] ClientAndroidBufferTest.buffer_packs_anativewindowbuffer_info

(But it fixes a hang in SwitchingBundleTest.client_framerate_matches_compositor - so I guess it doesn't make anything worse.)

Alan Griffiths (alan-griffiths) wrote : Posted in a previous version of this proposal

SwitchingBundleTest.client_framerate_matches_compositor is racy (and not fixed after all)

But this definitely breaks:

# ./unit-tests --gtest_filter=ClientAndroidBufferTest.buffer_packs_anativewindowbuffer_info
/home/alan/display_server/trunk-mir/tests/unit-tests/client/android/test_client_android_buffer.cpp:151: Failure
Value of: native_handle->stride
  Actual: 66
Expected: expected_stride_in_pixels
Which is: 16
[ FAILED ] ClientAndroidBufferTest.buffer_packs_anativewindowbuffer_info (2 ms)
[----------] 1 test from ClientAndroidBufferTest (2 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test case ran. (4 ms total)
[ PASSED ] 0 tests.
[ FAILED ] 1 test, listed below:
[ FAILED ] ClientAndroidBufferTest.buffer_packs_anativewindowbuffer_info

 1 FAILED TEST

review: Needs Fixing
Kevin DuBois (kdub) wrote : Posted in a previous version of this proposal

will have to take a look at stride on android again

Robert Carr (robertcarr) wrote : Posted in a previous version of this proposal

121 + static MirPixelFormat select_format_for_visual_id(int visual_id)
122 + {
123 + if (visual_id == 5)
124 + return mir_pixel_format_argb_8888;

It would be nice to see some constants for visual_id.

Haven't tested on nexus 4. Let me know if it needs some runs.

==
+ case NATIVE_WINDOW_MIN_UNDEQUEUED_BUFFERS:
43 + return 2;
===

Wondering if this has implications on existing hardware support (n4, etc...)

review: Needs Information
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Alan Griffiths (alan-griffiths) wrote :

102 +#include <iostream>

Not used

review: Needs Fixing
Kevin DuBois (kdub) wrote :

> + case NATIVE_WINDOW_MIN_UNDEQUEUED_BUFFERS:
> 43 + return 2;
> ===
> Wondering if this has implications on existing hardware support (n4, etc...)
the other drivers don't call this, shouldn't affect them.

Daniel van Vugt (vanvugt) wrote :

The Jenkins failure is bug 1227683 and probably not related to this proposal.

Kevin DuBois (kdub) wrote :

resurrecting this branch. commit msg/ description are still relevant. Issues mentioned above should have been fixed

1081. By Kevin DuBois on 2013-11-05

remove commented code

Alexandros Frantzis (afrantzis) wrote :

Looks good.

+ anwb->stride = stride.as_uint32_t();

So, is the ANativeWindowBuffer stride in bytes after all, or it's only there for the benefit of the users and the driver doesn't care about the value?

review: Needs Information
Alan Griffiths (alan-griffiths) wrote :

My previous concerns are addressed. But I don't understand the changes well enough to approve.

review: Abstain
Kevin DuBois (kdub) wrote :

> Looks good.
>
> + anwb->stride = stride.as_uint32_t();
>
> So, is the ANativeWindowBuffer stride in bytes after all, or it's only there
> for the benefit of the users and the driver doesn't care about the value?

Let me dig a bit on this question before we land this. I did a quick check on my devices... nexus4 seems to pass the stride in through the opaque handle (so it doesn't care about this field), nexus10 seems to prefer bytes, but maguro seems to prefer pixels... will post an update once i've sorted this out

review: Needs Fixing
1082. By Kevin DuBois on 2013-11-11

temporary commit

1083. By Kevin DuBois on 2013-11-11

merge trunk

1084. By Kevin DuBois on 2013-11-11

1more test

1085. By Kevin DuBois on 2013-11-11

all tests to pass, no stride changes, just test fixes

Kevin DuBois (kdub) wrote :

Okay, stride question on this MP solved, production code was correct, but the tests were outdated a bit.

We used to have the problem that android was using geom::Stride as pixel-stride, and gbm was using byte-stride. The pattern checkers surrounding TestClientIPCRender were written with pixel-stride in mind. Update the pattern checkers to use byte-stride.

Its a bit confusing that android's alloc functions (and ANativeWindowBuffer) use pixel stride, so I added comments where appropriate. Frankly, pixel stride makes little sense to me, but those are the android interfaces we have. :) Some drivers, like qualcomm's, works around this by passing byte-stride in their opaque type on the ANativeWindowBuffer (handle).

review: Abstain
Kevin DuBois (kdub) wrote :

> Let me dig a bit on this question before we land this. I did a quick check on
> my devices... nexus4 seems to pass the stride in through the opaque handle (so
> it doesn't care about this field), nexus10 seems to prefer bytes, but maguro
> seems to prefer pixels... will post an update once i've sorted this out

ANativeWindowBuffer::stride is pixel-stride, and gralloc's allocation function gives back pixel-stride on all devices.

exynos and powervr devices seem to use ANativeWindowBuffer::stride as-is, and its necessary that it has the right value.

qualcomm ignores ANativeWindowBuffer::stride, and prefers to pass byte-stride in its opaque ANativeWindowBuffer::handle. These drivers probably care about byte-alignment a bit more.

Daniel van Vugt (vanvugt) wrote :

Looks reasonable. Tested on trusty desktop and N4 to verify everything still works (at least as well as the dev branch).

review: Approve
Alexandros Frantzis (afrantzis) wrote :

Looks good.

review: Approve
Alan Griffiths (alan-griffiths) wrote :

70 + uint32_t *pixel = reinterpret_cast<uint32_t*>(&region.vaddr[j*region.stride + (i * bpp)]);

I know casts like this are in the original code - but I've had problems with them when moving between little-endian and big-endian platforms and when sizeof(int) != .

Looking around, I see that most usage of vaddr s as an array of uint32_t (there's TestGrallocMapper::graphic_region_from_handle() that spuriously uses it as int*, and some use as an array of bytes).

It isn't a blocker, but I have to wonder if there's a better way.

review: Approve
Kevin DuBois (kdub) wrote :

> 70 + uint32_t *pixel =
> reinterpret_cast<uint32_t*>(&region.vaddr[j*region.stride + (i * bpp)]);
>
> I know casts like this are in the original code - but I've had problems with
> them when moving between little-endian and big-endian platforms and when
> sizeof(int) != .
>
> Looking around, I see that most usage of vaddr s as an array of uint32_t
> (there's TestGrallocMapper::graphic_region_from_handle() that spuriously uses
> it as int*, and some use as an array of bytes).
>
> It isn't a blocker, but I have to wonder if there's a better way.

I gave consideration to making this part of the code shiny yesterday, however, its really just used by one android-specific integration test case at the moment. At the moment, expanding beyond that one test case is where I'd want to sink time into polishing.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'include/shared/mir/geometry/dimensions.h'
2--- include/shared/mir/geometry/dimensions.h 2013-06-06 09:27:25 +0000
3+++ include/shared/mir/geometry/dimensions.h 2013-11-11 21:03:32 +0000
4@@ -107,6 +107,7 @@
5
6 typedef detail::IntWrapper<detail::width> Width;
7 typedef detail::IntWrapper<detail::height> Height;
8+// Just to be clear, mir::geometry::Stride is the stride of the buffer in bytes
9 typedef detail::IntWrapper<detail::stride> Stride;
10
11 typedef detail::IntWrapper<detail::x> X;
12
13=== modified file 'include/shared/mir/graphics/android/mir_native_window.h'
14--- include/shared/mir/graphics/android/mir_native_window.h 2013-10-15 08:53:10 +0000
15+++ include/shared/mir/graphics/android/mir_native_window.h 2013-11-11 21:03:32 +0000
16@@ -40,6 +40,7 @@
17 int perform(int key, va_list args );
18 int dequeueBuffer(struct ANativeWindowBuffer** buffer, int* fence);
19 int queueBuffer(struct ANativeWindowBuffer* buffer, int fence);
20+ int cancelBuffer(struct ANativeWindowBuffer* buffer, int fence);
21 int setSwapInterval(int interval);
22 private:
23
24
25=== modified file 'include/test/mir_test/draw/draw_pattern_checkered-inl.h'
26--- include/test/mir_test/draw/draw_pattern_checkered-inl.h 2013-10-14 20:35:46 +0000
27+++ include/test/mir_test/draw/draw_pattern_checkered-inl.h 2013-11-11 21:03:32 +0000
28@@ -16,6 +16,9 @@
29 * Authored by: Kevin DuBois <kevin.dubois@canonical.com>
30 */
31
32+#include "mir/geometry/pixel_format.h"
33+namespace geom = mir::geometry;
34+
35 template<size_t Rows, size_t Cols>
36 DrawPatternCheckered<Rows,Cols>::DrawPatternCheckered(uint32_t (&pattern) [Rows][Cols])
37 {
38@@ -29,14 +32,15 @@
39 if (region.pixel_format != mir_pixel_format_abgr_8888)
40 throw(std::runtime_error("cannot draw region, incorrect format"));
41
42- uint32_t *pixel = (uint32_t*) region.vaddr;
43- for(int i=0; i< region.width; i++)
44+ auto bpp = geom::bytes_per_pixel(geom::PixelFormat::abgr_8888);
45+ for(int i=0; i < region.width; i++)
46 {
47 for(int j=0; j<region.height; j++)
48 {
49 int key_row = i % Rows;
50 int key_col = j % Cols;
51- pixel[j*region.stride + i] = color_pattern[key_row][key_col];
52+ uint32_t *pixel = reinterpret_cast<uint32_t*>(&region.vaddr[j*region.stride + (i * bpp)]);
53+ *pixel = color_pattern[key_row][key_col];
54 }
55 }
56 }
57@@ -47,14 +51,15 @@
58 if (region.pixel_format != mir_pixel_format_abgr_8888)
59 throw(std::runtime_error("cannot check region, incorrect format"));
60
61- uint32_t *pixel = (uint32_t*) region.vaddr;
62+ auto bpp = geom::bytes_per_pixel(geom::PixelFormat::abgr_8888);
63 for(int i=0; i< region.width; i++)
64 {
65 for(int j=0; j<region.height; j++)
66 {
67 int key_row = i % Rows;
68 int key_col = j % Cols;
69- if (pixel[j*region.stride + i] != color_pattern[key_row][key_col])
70+ uint32_t *pixel = reinterpret_cast<uint32_t*>(&region.vaddr[j*region.stride + (i * bpp)]);
71+ if ( *pixel != color_pattern[key_row][key_col] )
72 {
73 return false;
74 }
75
76=== modified file 'src/client/android/android_client_buffer.cpp'
77--- src/client/android/android_client_buffer.cpp 2013-10-15 08:53:10 +0000
78+++ src/client/android/android_client_buffer.cpp 2013-11-11 21:03:32 +0000
79@@ -45,6 +45,8 @@
80
81 anwb->height = static_cast<int32_t>(buffer_size.height.as_uint32_t());
82 anwb->width = static_cast<int32_t>(buffer_size.width.as_uint32_t());
83+ //note: mir uses stride in bytes, ANativeWindowBuffer needs it in pixel units. some drivers care
84+ //about byte-stride, they will pass stride via ANativeWindowBuffer::handle (which is opaque to us)
85 anwb->stride = stride.as_uint32_t() /
86 geom::bytes_per_pixel(buffer_pf);
87 anwb->usage = GRALLOC_USAGE_HW_TEXTURE | GRALLOC_USAGE_HW_RENDER;
88
89=== modified file 'src/client/android/client_surface_interpreter.cpp'
90--- src/client/android/client_surface_interpreter.cpp 2013-10-15 08:53:10 +0000
91+++ src/client/android/client_surface_interpreter.cpp 2013-11-11 21:03:32 +0000
92@@ -75,7 +75,7 @@
93 case NATIVE_WINDOW_TRANSFORM_HINT:
94 return 0;
95 case NATIVE_WINDOW_MIN_UNDEQUEUED_BUFFERS:
96- return 1;
97+ return 2;
98 default:
99 throw std::runtime_error("driver requested unsupported query");
100 }
101
102=== modified file 'src/shared/graphics/android/mir_native_window.cpp'
103--- src/shared/graphics/android/mir_native_window.cpp 2013-10-15 08:53:10 +0000
104+++ src/shared/graphics/android/mir_native_window.cpp 2013-11-11 21:03:32 +0000
105@@ -110,16 +110,17 @@
106 return 0;
107 }
108
109-int cancelBuffer_deprecated_static(struct ANativeWindow* /*window*/,
110- struct ANativeWindowBuffer* /*buffer*/)
111+int cancelBuffer_deprecated_static(struct ANativeWindow* window,
112+ struct ANativeWindowBuffer* buffer)
113 {
114- return 0;
115+ return cancelBuffer_static(window, buffer, -1);
116 }
117
118-int cancelBuffer_static(struct ANativeWindow* /*window*/,
119- struct ANativeWindowBuffer* /*buffer*/, int /*fence_fd*/)
120+int cancelBuffer_static(struct ANativeWindow* window,
121+ struct ANativeWindowBuffer* buffer, int fence_fd)
122 {
123- return 0;
124+ auto self = static_cast<mga::MirNativeWindow*>(window);
125+ return self->cancelBuffer(buffer, fence_fd);
126 }
127
128 }
129@@ -174,6 +175,12 @@
130 return 0;
131 }
132
133+int mga::MirNativeWindow::cancelBuffer(struct ANativeWindowBuffer* buffer, int fence)
134+{
135+ driver_interpreter->driver_returns_buffer(buffer, fence);
136+ return 0;
137+}
138+
139 int mga::MirNativeWindow::query(int key, int* value) const
140 {
141 *value = driver_interpreter->driver_requests_info(key);
142
143=== modified file 'tests/draw/android_graphics.cpp'
144--- tests/draw/android_graphics.cpp 2013-10-17 05:51:24 +0000
145+++ tests/draw/android_graphics.cpp 2013-11-11 21:03:32 +0000
146@@ -18,6 +18,7 @@
147
148
149 #include "mir_test/draw/android_graphics.h"
150+#include "mir/geometry/pixel_format.h"
151
152 #include <fstream>
153 #include <stdexcept>
154@@ -106,7 +107,7 @@
155 RegionDeleter del(module, package->handle);
156
157 region->vaddr = (char*) vaddr;
158- region->stride = package->stride;
159+ region->stride = package->stride * geom::bytes_per_pixel(geom::PixelFormat::abgr_8888);
160 region->width = package->width;
161 region->height = package->height;
162 region->pixel_format = mir_pixel_format_abgr_8888;
163
164=== modified file 'tests/draw/patterns.cpp'
165--- tests/draw/patterns.cpp 2013-10-14 20:35:46 +0000
166+++ tests/draw/patterns.cpp 2013-11-11 21:03:32 +0000
167@@ -16,9 +16,11 @@
168 * Authored by: Kevin DuBois <kevin.dubois@canonical.com>
169 */
170
171+#include "mir/geometry/pixel_format.h"
172 #include "mir_test/draw/patterns.h"
173
174 namespace mtd=mir::test::draw;
175+namespace geom=mir::geometry;
176
177 mtd::DrawPatternSolid::DrawPatternSolid(uint32_t color_value)
178 : color_value(color_value)
179@@ -30,13 +32,13 @@
180 if (region.pixel_format != mir_pixel_format_abgr_8888 )
181 throw(std::runtime_error("cannot draw region, incorrect format"));
182
183- uint32_t *pixel = (uint32_t*) region.vaddr;
184- int i,j;
185- for(i=0; i<region.height; i++)
186+ auto bpp = geom::bytes_per_pixel(geom::PixelFormat::abgr_8888);
187+ for(auto i = 0; i < region.height; i++)
188 {
189- for(j=0; j< region.width; j++)
190+ for(auto j = 0; j < region.width; j++)
191 {
192- pixel[i*region.stride + j] = color_value;
193+ uint32_t *pixel = (uint32_t*) &region.vaddr[i*region.stride + (j * bpp)];
194+ *pixel = color_value;
195 }
196 }
197 }
198@@ -46,13 +48,13 @@
199 if (region.pixel_format != mir_pixel_format_abgr_8888 )
200 throw(std::runtime_error("cannot check region, incorrect format"));
201
202- uint32_t *pixel = (uint32_t*) region.vaddr;
203- int i,j;
204- for(i=0; i< region.width; i++)
205+ auto bpp = geom::bytes_per_pixel(geom::PixelFormat::abgr_8888);
206+ for(auto i = 0; i < region.height; i++)
207 {
208- for(j=0; j<region.height; j++)
209+ for(auto j = 0; j < region.width; j++)
210 {
211- if (pixel[j*region.stride + i] != color_value)
212+ uint32_t *pixel = (uint32_t*) &region.vaddr[i*region.stride + (j * bpp)];
213+ if (*pixel != color_value)
214 {
215 return false;
216 }
217
218=== modified file 'tests/integration-tests/client/test_client_render.cpp'
219--- tests/integration-tests/client/test_client_render.cpp 2013-10-17 05:51:24 +0000
220+++ tests/integration-tests/client/test_client_render.cpp 2013-11-11 21:03:32 +0000
221@@ -50,7 +50,6 @@
222 {
223 static int test_width = 300;
224 static int test_height = 200;
225-
226 static uint32_t pattern0 [2][2] = {{0x12345678, 0x23456789},
227 {0x34567890, 0x45678901}};
228 static uint32_t pattern1 [2][2] = {{0xFFFFFFFF, 0xFFFF0000},
229@@ -61,6 +60,30 @@
230
231 struct TestClient
232 {
233+ static MirPixelFormat select_format_for_visual_id(int visual_id)
234+ {
235+ if (visual_id == 5)
236+ return mir_pixel_format_argb_8888;
237+ if (visual_id == 1)
238+ return mir_pixel_format_abgr_8888;
239+
240+ return mir_pixel_format_invalid;
241+ }
242+
243+ static MirSurface* create_mir_surface(MirConnection * connection, EGLDisplay display, EGLConfig config)
244+ {
245+ int visual_id;
246+ eglGetConfigAttrib(display, config, EGL_NATIVE_VISUAL_ID, &visual_id);
247+
248+ /* make surface */
249+ MirSurfaceParameters surface_parameters;
250+ surface_parameters.name = "testsurface";
251+ surface_parameters.width = test_width;
252+ surface_parameters.height = test_height;
253+ surface_parameters.pixel_format = select_format_for_visual_id(visual_id);
254+ return mir_connection_create_surface_sync(connection, &surface_parameters);
255+ }
256+
257 static int render_cpu_pattern(mtf::CrossProcessSync& process_sync, int num_frames)
258 {
259 process_sync.wait_for_signal_ready_for();
260@@ -107,27 +130,28 @@
261
262 /* set up egl context */
263 int major, minor, n;
264- EGLDisplay disp;
265 EGLContext context;
266 EGLSurface egl_surface;
267 EGLConfig egl_config;
268 EGLint attribs[] = {
269 EGL_SURFACE_TYPE, EGL_WINDOW_BIT,
270- EGL_GREEN_SIZE, 8,
271+ EGL_BUFFER_SIZE, 32,
272 EGL_RENDERABLE_TYPE, EGL_OPENGL_ES2_BIT,
273 EGL_NONE };
274 EGLint context_attribs[] = { EGL_CONTEXT_CLIENT_VERSION, 2, EGL_NONE };
275
276- EGLNativeDisplayType native_display = (EGLNativeDisplayType)mir_connection_get_egl_native_display(connection);
277- EGLNativeWindowType native_window = (EGLNativeWindowType) mir_surface_get_egl_native_window(surface);
278-
279- disp = eglGetDisplay(native_display);
280- eglInitialize(disp, &major, &minor);
281-
282- eglChooseConfig(disp, attribs, &egl_config, 1, &n);
283- egl_surface = eglCreateWindowSurface(disp, egl_config, native_window, NULL);
284- context = eglCreateContext(disp, egl_config, EGL_NO_CONTEXT, context_attribs);
285- eglMakeCurrent(disp, egl_surface, egl_surface, context);
286+ auto native_display = mir_connection_get_egl_native_display(connection);
287+ auto egl_display = eglGetDisplay(native_display);
288+ eglInitialize(egl_display, &major, &minor);
289+ eglChooseConfig(egl_display, attribs, &egl_config, 1, &n);
290+
291+ auto mir_surface = create_mir_surface(connection, egl_display, egl_config);
292+ auto native_window = static_cast<EGLNativeWindowType>(
293+ mir_surface_get_egl_native_window(mir_surface));
294+
295+ egl_surface = eglCreateWindowSurface(egl_display, egl_config, native_window, NULL);
296+ context = eglCreateContext(egl_display, egl_config, EGL_NO_CONTEXT, context_attribs);
297+ eglMakeCurrent(egl_display, egl_surface, egl_surface, context);
298
299 for (auto i=0; i < num_frames; i++)
300 {
301@@ -146,13 +170,12 @@
302 }
303 glClear(GL_COLOR_BUFFER_BIT);
304
305- eglSwapBuffers(disp, egl_surface);
306+ eglSwapBuffers(egl_display, egl_surface);
307 }
308
309 mir_surface_release_sync(surface);
310 mir_connection_release(connection);
311 return 0;
312-
313 }
314
315 static int exit_function()
316@@ -165,15 +188,13 @@
317 struct StubServerGenerator : public mt::StubServerTool
318 {
319 StubServerGenerator()
320- : next_received(false),
321- next_allowed(false)
322 {
323 auto initializer = std::make_shared<mg::NullBufferInitializer>();
324 allocator = std::make_shared<mga::AndroidGraphicBufferAllocator> (initializer);
325 auto size = geom::Size{test_width, test_height};
326- auto pf = geom::PixelFormat::abgr_8888;
327- last_posted = allocator->alloc_buffer_platform(size, pf, mga::BufferUsage::use_hardware);
328- client_buffer = allocator->alloc_buffer_platform(size, pf, mga::BufferUsage::use_hardware);
329+ surface_pf = geom::PixelFormat::abgr_8888;
330+ last_posted = allocator->alloc_buffer_platform(size, surface_pf, mga::BufferUsage::use_hardware);
331+ client_buffer = allocator->alloc_buffer_platform(size, surface_pf, mga::BufferUsage::use_hardware);
332 }
333
334 void create_surface(google::protobuf::RpcController* /*controller*/,
335@@ -184,17 +205,19 @@
336 response->mutable_id()->set_value(13);
337 response->set_width(test_width);
338 response->set_height(test_height);
339+ surface_pf = geom::PixelFormat(request->pixel_format());
340 response->set_pixel_format(request->pixel_format());
341 response->mutable_buffer()->set_buffer_id(client_buffer->id().as_uint32_t());
342
343 auto buf = client_buffer->native_buffer_handle();
344- response->mutable_buffer()->set_stride(buf->anwb()->stride);
345+ //note about the stride. Mir protocol sends stride in bytes, android uses stride in pixels
346+ response->mutable_buffer()->set_stride(client_buffer->stride().as_uint32_t());
347
348 response->mutable_buffer()->set_fds_on_side_channel(1);
349 native_handle_t const* native_handle = buf->handle();
350 for(auto i=0; i<native_handle->numFds; i++)
351 response->mutable_buffer()->add_fd(native_handle->data[i]);
352- for(auto i=0; i<native_handle->numInts; i++)
353+ for(auto i=0; i < native_handle->numInts; i++)
354 response->mutable_buffer()->add_data(native_handle->data[native_handle->numFds+i]);
355
356 std::unique_lock<std::mutex> lock(guard);
357@@ -216,19 +239,32 @@
358 response->set_buffer_id(client_buffer->id().as_uint32_t());
359
360 auto buf = client_buffer->native_buffer_handle();
361-
362 response->set_fds_on_side_channel(1);
363 native_handle_t const* native_handle = buf->handle();
364- response->set_stride(buf->anwb()->stride);
365+ response->set_stride(client_buffer->stride().as_uint32_t());
366 for(auto i=0; i<native_handle->numFds; i++)
367 response->add_fd(native_handle->data[i]);
368 for(auto i=0; i<native_handle->numInts; i++)
369 response->add_data(native_handle->data[native_handle->numFds+i]);
370-
371-
372 done->Run();
373 }
374
375+ uint32_t red_value_for_surface()
376+ {
377+ if ((surface_pf == geom::PixelFormat::abgr_8888) || (surface_pf == geom::PixelFormat::xbgr_8888))
378+ return 0xFF0000FF;
379+
380+ if ((surface_pf == geom::PixelFormat::argb_8888) || (surface_pf == geom::PixelFormat::xrgb_8888))
381+ return 0xFFFF0000;
382+
383+ return 0x0;
384+ }
385+
386+ uint32_t green_value_for_surface()
387+ {
388+ return 0xFF00FF00;
389+ }
390+
391 std::shared_ptr<mga::Buffer> second_to_last_posted_buffer()
392 {
393 std::unique_lock<std::mutex> lk(buffer_mutex);
394@@ -245,14 +281,8 @@
395 std::shared_ptr<mga::AndroidGraphicBufferAllocator> allocator;
396 std::shared_ptr<mga::Buffer> client_buffer;
397 std::shared_ptr<mga::Buffer> last_posted;
398-
399 std::mutex buffer_mutex;
400- std::condition_variable next_cv;
401- std::condition_variable allow_cv;
402- bool next_received;
403- bool next_allowed;
404-
405- int handle_id;
406+ geom::PixelFormat surface_pf;
407 };
408
409 }
410
411=== modified file 'tests/unit-tests/client/android/test_android_native_window.cpp'
412--- tests/unit-tests/client/android/test_android_native_window.cpp 2013-10-15 08:53:10 +0000
413+++ tests/unit-tests/client/android/test_android_native_window.cpp 2013-11-11 21:03:32 +0000
414@@ -288,3 +288,17 @@
415 auto ret = window->dequeueBuffer(window.get(), &tmp, &fencefd);
416 EXPECT_EQ(0, ret);
417 }
418+
419+TEST_F(AndroidNativeWindowTest, native_window_cancel_hook_behavior)
420+{
421+ using namespace testing;
422+ ANativeWindowBuffer buffer;
423+ int fence_fd = 33;
424+
425+ EXPECT_CALL(*mock_driver_interpreter, driver_returns_buffer(&buffer, _))
426+ .Times(1);
427+
428+ std::shared_ptr<ANativeWindow> window = std::make_shared<mga::MirNativeWindow>(mock_driver_interpreter);
429+ auto rc = window->cancelBuffer(window.get(), &buffer, fence_fd);
430+ EXPECT_EQ(0, rc);
431+}
432
433=== modified file 'tests/unit-tests/client/android/test_client_surface_interpreter.cpp'
434--- tests/unit-tests/client/android/test_client_surface_interpreter.cpp 2013-10-15 08:53:10 +0000
435+++ tests/unit-tests/client/android/test_client_surface_interpreter.cpp 2013-11-11 21:03:32 +0000
436@@ -207,3 +207,19 @@
437
438 EXPECT_EQ(surf_params.height, height);
439 }
440+
441+/* this is query key is a bit confusing from the system/window.h description.
442+ what it means is the minimum number of buffers that the server reserves for its own use in steady
443+ state. The drivers consider 'steady state' to begin after the first call to queueBuffer.
444+ So, for instance, if a driver requires 3 buffers to run at steady state, and the server needs
445+ to keep 2 buffers on hand at all time, the driver might dequeue 5 buffers, then cancel those 5 buffers.
446+ After the first call to queueBuffer however, the client may never own more than the number it has
447+ reserved (in this case, 3 buffers) */
448+TEST_F(AndroidInterpreterTest, native_window_minimum_undequeued_query_hook)
449+{
450+ testing::NiceMock<MockMirSurface> mock_surface{surf_params};
451+ mcla::ClientSurfaceInterpreter interpreter(mock_surface);
452+
453+ auto num_buffers = interpreter.driver_requests_info(NATIVE_WINDOW_MIN_UNDEQUEUED_BUFFERS);
454+ EXPECT_EQ(2, num_buffers);
455+}
456
457=== modified file 'tests/unit-tests/draw/test_draw_patterns.cpp'
458--- tests/unit-tests/draw/test_draw_patterns.cpp 2013-10-16 23:17:29 +0000
459+++ tests/unit-tests/draw/test_draw_patterns.cpp 2013-11-11 21:03:32 +0000
460@@ -28,17 +28,17 @@
461 {
462 protected:
463 DrawPatternsTest()
464- : stride_color(0x77777777)
465+ : stride_color(0x77)
466 {}
467
468 virtual void SetUp()
469 {
470
471 test_region.pixel_format = mir_pixel_format_abgr_8888;
472- int bytes_pp = 4;
473+ bytes_pp = 4;
474 test_region.width = 100;
475 /* misaligned stride to tease out stride problems */
476- test_region.stride = 102;
477+ test_region.stride = (test_region.width * bytes_pp) + 2;
478 test_region.height = 50;
479 auto region_size =
480 sizeof(char) * bytes_pp * test_region.height * test_region.stride;
481@@ -58,29 +58,28 @@
482
483 MirGraphicsRegion test_region;
484 uint32_t pattern_colors [2][2];
485+ int bytes_pp;
486 std::shared_ptr<char> region;
487 private:
488 /* check that no pattern writes or checks the stride region */
489 void write_stride_region()
490 {
491- uint32_t* region = (uint32_t*) test_region.vaddr;
492- for(auto i=0; i < test_region.height; i++)
493+ for(auto i = 0; i < test_region.height; i++)
494 {
495- for(auto j=test_region.width; j<test_region.stride; j++)
496+ for(auto j = test_region.width * bytes_pp; j < test_region.stride; j++)
497 {
498- region[ (i*test_region.stride) + j ] = stride_color;
499+ test_region.vaddr[ (i * test_region.stride) + j ] = stride_color;
500 }
501 }
502 }
503
504 bool check_stride_region_unaltered()
505 {
506- uint32_t* region = (uint32_t*) test_region.vaddr;
507 for(auto i=0; i < test_region.height; i++)
508 {
509- for(auto j=test_region.width; j<test_region.stride; j++)
510+ for(auto j=test_region.width * bytes_pp; j < test_region.stride; j++)
511 {
512- if(region[ ((i*test_region.stride) + j) ] != stride_color)
513+ if(test_region.vaddr[ ((i * test_region.stride) + j) ] != stride_color)
514 {
515 return false;
516 }
517@@ -89,7 +88,7 @@
518 return true;
519 }
520
521- const uint32_t stride_color;
522+ const char stride_color;
523 };
524
525 TEST_F(DrawPatternsTest, solid_color_unaccelerated)

Subscribers

People subscribed via source and target branches