Mir

Merge lp:~kdub/mir/current-buffer-id-fix into lp:mir

  1. current-buffer-id-fix
  2. Merge into development-branch
Proposed by Kevin DuBois
Status: Merged
Approved by: Kevin DuBois
Approved revision: no longer in the source branch.
Merged at revision: 3127
Proposed branch: lp:~kdub/mir/current-buffer-id-fix
Merge into: lp:mir
Diff against target: 531 lines (+97/-65)
10 files modified
src/client/buffer_stream.cpp (+11/-13)
src/client/buffer_vault.cpp (+4/-4)
src/client/buffer_vault.h (+9/-2)
src/platforms/android/server/android_buffer_allocator.cpp (+15/-10)
src/platforms/android/server/device_quirks.cpp (+9/-9)
src/platforms/android/server/device_quirks.h (+2/-2)
tests/integration-tests/test_buffer_scheduling.cpp (+1/-1)
tests/unit-tests/client/test_buffer_vault.cpp (+15/-15)
tests/unit-tests/client/test_client_buffer_stream.cpp (+22/-0)
tests/unit-tests/graphics/android/test_device_detection.cpp (+9/-9)
To merge this branch: bzr merge lp:~kdub/mir/current-buffer-id-fix
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
Alexandros Frantzis (community) Approve
Alan Griffiths Approve
Chris Halse Rogers Approve
Review via email: mp+275986@code.launchpad.net

Commit message

nbs: fix reporting of the current buffer id. We were previously reporting the last-client-bound id, not the last id the driver is using.

Description of the change

nbs: fix reporting of the current buffer id. We were previously reporting the last-client-bound id, not the last id the driver is using.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:3059
http://jenkins.qa.ubuntu.com/job/mir-ci/5423/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/4671
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/3578
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/4613
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-wily-touch/571/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/1577
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/1577/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-i386-ci/571
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-i386-ci/571/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/4614
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/4614/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-touch/7234
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/24642
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-wily-armhf/572/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/5423/rebuild

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

Yeah, sure.

The test at the end actually made it clear to me what you were trying to do here :)

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

Wouldn't it be more natural for the ClientBuffer to store and report its id (e.g., ClientBuffer::id())?

review: Needs Information
Revision history for this message
Kevin DuBois (kdub) wrote :

I'm sure I replied on tuesday, LP must have lost my response!

Revision history for this message
Kevin DuBois (kdub) wrote :

This was the first route I tried, but it required more plumbing and some platform abi breakage to plumb the ID down through the buffer so it can be accessed via a fn on mcl::ClientBuffer. This route is more in line with how BufferDepository works, where you can access the current id there.

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

I can live with this

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

OK.

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-autolanding/2098/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/4817
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/3724
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/4759/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-wily-touch/717/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-autolanding/653
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-autolanding/653/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-i386-ci/717
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-i386-ci/717/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/4760
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/4760/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-touch/7363/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/24966
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-wily-armhf/718/console

review: Needs Fixing (continuous-integration)
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-autolanding/2100/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/4819
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/3726/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/4761
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-wily-touch/719/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-autolanding/655
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-autolanding/655/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-i386-ci/719
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-i386-ci/719/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/4762
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/4762/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-touch/7365
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/24971
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-wily-armhf/720/console

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

11: [----------] 2 tests from ThreadedDispatcherDeathTest
11: [ RUN ] ThreadedDispatcherDeathTest.exceptions_in_threadpool_trigger_termination
11: Running main() from main.cpp
no failure on phone but instead on clang: maybe the thread was not scheduled:

11: [ OK ] ThreadedDispatcherDeathTest.exceptions_in_threadpool_trigger_termination (166 ms)
11: [ RUN ] ThreadedDispatcherDeathTest.destroying_dispatcher_from_a_callback_is_an_error
11:
11: [WARNING] /usr/src/gtest/src/gtest-death-test.cc:825:: Death tests use fork(), which is unsafe particularly in a threaded context. For this test, Google Test couldn't detect the number of threads.
11: /mir/tests/unit-tests/dispatch/test_threaded_dispatcher.cpp:385: Failure
11: Death test: { md::ThreadedDispatcher* dispatcher; auto dispatchable = std::make_shared<mt::TestDispatchable>([&dispatcher]() { delete dispatcher; }); dispatchable->trigger(); dispatcher = new md::ThreadedDispatcher("Death thread", dispatchable); std::this_thread::sleep_for(10s); }
11: Result: failed to die.
11: Error msg:
11: [ DEATH ]
11: [ FAILED ] ThreadedDispatcherDeathTest.destroying_dispatcher_from_a_callback_is_an_error (10006 ms)
11: [----------] 2 tests from ThreadedDispatcherDeathTest (10172 ms total)

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-autolanding/2104/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/4831
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/3738
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/4773
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-wily-touch/731/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-autolanding/659/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-i386-ci/731
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-i386-ci/731/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/4774
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/4774/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-touch/7374
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/25010
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-wily-armhf/732/console

review: Needs Fixing (continuous-integration)
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-autolanding/2111/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/4850/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/3757/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/4792/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-wily-touch/750/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-autolanding/666/console
    None: http://jenkins.qa.ubuntu.com/job/mir-wily-i386-ci/750/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/4793/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-wily-armhf/751/console

review: Needs Fixing (continuous-integration)
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-autolanding/2116/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/4861/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/3768/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/4803/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-wily-touch/761/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-autolanding/671/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-wily-i386-ci/761/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/4804/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-wily-armhf/762/console

review: Needs Fixing (continuous-integration)
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

Merge conflict:

Text conflict in src/client/buffer_stream.cpp
1 conflicts encountered.

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

FAILED: Continuous integration, rev:3060
http://jenkins.qa.ubuntu.com/job/mir-ci/5616/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/4965
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/3871
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/4911/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-wily-touch/867/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/1769
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/1769/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-i386-ci/865
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-i386-ci/865/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/4911
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/4911/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-touch/7475/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/25361
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-wily-armhf/868/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/5616/rebuild

review: Needs Fixing (continuous-integration)
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-autolanding/2159/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/4980
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/3886
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/4926/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-wily-touch/882/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-autolanding/712
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-autolanding/712/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-i386-ci/880
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-i386-ci/880/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/4926
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/4926/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-touch/7488/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/25378
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-wily-armhf/883/console

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

PASSED: Continuous integration, rev:3062
http://jenkins.qa.ubuntu.com/job/mir-ci/5664/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/5024
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/3930
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/4971
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-wily-touch/927/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/1816
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/1816/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-i386-ci/924
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-i386-ci/924/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/4971
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/4971/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-touch/7518
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/25496
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-wily-armhf/928/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/5664/rebuild

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/client/buffer_stream.cpp'
2--- src/client/buffer_stream.cpp 2015-11-18 12:42:24 +0000
3+++ src/client/buffer_stream.cpp 2015-11-25 13:03:44 +0000
4@@ -282,9 +282,6 @@
5 void deposit(mp::Buffer const& buffer, geom::Size, MirPixelFormat) override
6 {
7 vault.wire_transfer_inbound(buffer);
8-
9- std::lock_guard<std::mutex> lk(mutex);
10- current_buffer_id_ = buffer.buffer_id();
11 }
12
13 void advance_current_buffer(std::unique_lock<std::mutex>& lk)
14@@ -292,34 +289,36 @@
15 lk.unlock();
16 auto buffer = vault.withdraw().get();
17 lk.lock();
18- current_buffer_ = buffer;
19+ current = buffer;
20 }
21
22 std::shared_ptr<mir::client::ClientBuffer> current_buffer() override
23 {
24 std::unique_lock<std::mutex> lk(mutex);
25- if (!current_buffer_)
26+ if (!current.buffer)
27 advance_current_buffer(lk);
28- return current_buffer_;
29+ return current.buffer;
30 }
31
32 uint32_t current_buffer_id() override
33 {
34- std::lock_guard<std::mutex> lk(mutex);
35- return current_buffer_id_;
36+ std::unique_lock<std::mutex> lk(mutex);
37+ if (!current.buffer)
38+ advance_current_buffer(lk);
39+ return current.id;
40 }
41
42 MirWaitHandle* submit(std::function<void()> const& done, geom::Size, MirPixelFormat, int) override
43 {
44 std::unique_lock<std::mutex> lk(mutex);
45- if (!current_buffer_)
46+ if (!current.buffer)
47 advance_current_buffer(lk);
48 lk.unlock();
49
50- vault.deposit(current_buffer_);
51+ vault.deposit(current.buffer);
52
53 next_buffer_wait_handle.expect_result();
54- vault.wire_transfer_outbound(current_buffer_);
55+ vault.wire_transfer_outbound(current.buffer);
56 next_buffer_wait_handle.result_received();
57
58 lk.lock();
59@@ -343,8 +342,7 @@
60
61 mcl::BufferVault vault;
62 std::mutex mutex;
63- std::shared_ptr<mcl::ClientBuffer> current_buffer_;
64- int current_buffer_id_;
65+ mcl::BufferInfo current{nullptr, 0};
66 MirWaitHandle next_buffer_wait_handle;
67 };
68 }
69
70=== modified file 'src/client/buffer_vault.cpp'
71--- src/client/buffer_vault.cpp 2015-11-04 14:21:08 +0000
72+++ src/client/buffer_vault.cpp 2015-11-25 13:03:44 +0000
73@@ -55,10 +55,10 @@
74 server_requests->free_buffer(it.first);
75 }
76
77-mcl::NoTLSFuture<std::shared_ptr<mcl::ClientBuffer>> mcl::BufferVault::withdraw()
78+mcl::NoTLSFuture<mcl::BufferInfo> mcl::BufferVault::withdraw()
79 {
80 std::lock_guard<std::mutex> lk(mutex);
81- mcl::NoTLSPromise<std::shared_ptr<mcl::ClientBuffer>> promise;
82+ mcl::NoTLSPromise<mcl::BufferInfo> promise;
83 auto it = std::find_if(buffers.begin(), buffers.end(),
84 [this](std::pair<int, BufferEntry> const& entry) {
85 return ((entry.second.owner == Owner::Self) && (size == entry.second.buffer->size())); });
86@@ -67,7 +67,7 @@
87 if (it != buffers.end())
88 {
89 it->second.owner = Owner::ContentProducer;
90- promise.set_value(it->second.buffer);
91+ promise.set_value({it->second.buffer, it->first});
92 }
93 else
94 {
95@@ -148,7 +148,7 @@
96 if (!promises.empty())
97 {
98 buffers[protobuf_buffer.buffer_id()].owner = Owner::ContentProducer;
99- promises.front().set_value(buffers[protobuf_buffer.buffer_id()].buffer);
100+ promises.front().set_value({buffers[protobuf_buffer.buffer_id()].buffer, protobuf_buffer.buffer_id()});
101 promises.pop_front();
102 }
103 }
104
105=== modified file 'src/client/buffer_vault.h'
106--- src/client/buffer_vault.h 2015-10-20 03:30:00 +0000
107+++ src/client/buffer_vault.h 2015-11-25 13:03:44 +0000
108@@ -47,6 +47,13 @@
109 };
110
111 class ClientBufferFactory;
112+
113+struct BufferInfo
114+{
115+ std::shared_ptr<ClientBuffer> buffer;
116+ int id;
117+};
118+
119 class BufferVault
120 {
121 public:
122@@ -57,7 +64,7 @@
123 unsigned int initial_nbuffers);
124 ~BufferVault();
125
126- NoTLSFuture<std::shared_ptr<ClientBuffer>> withdraw();
127+ NoTLSFuture<BufferInfo> withdraw();
128 void deposit(std::shared_ptr<ClientBuffer> const& buffer);
129 void wire_transfer_inbound(protobuf::Buffer const&);
130 void wire_transfer_outbound(std::shared_ptr<ClientBuffer> const& buffer);
131@@ -78,7 +85,7 @@
132
133 std::mutex mutex;
134 std::map<int, BufferEntry> buffers;
135- std::deque<NoTLSPromise<std::shared_ptr<ClientBuffer>>> promises;
136+ std::deque<NoTLSPromise<BufferInfo>> promises;
137 geometry::Size size;
138 };
139 }
140
141=== modified file 'src/platforms/android/server/android_buffer_allocator.cpp'
142--- src/platforms/android/server/android_buffer_allocator.cpp 2015-08-07 15:55:22 +0000
143+++ src/platforms/android/server/android_buffer_allocator.cpp 2015-11-25 13:03:44 +0000
144@@ -25,6 +25,7 @@
145 #include "android_graphic_buffer_allocator.h"
146 #include "android_alloc_adaptor.h"
147 #include "buffer.h"
148+#include "device_quirks.h"
149
150 #include <boost/throw_exception.hpp>
151
152@@ -36,14 +37,17 @@
153
154 namespace
155 {
156-struct AllocDevDeleter
157-{
158- void operator()(alloc_device_t* t)
159- {
160- /* android takes care of delete for us */
161- t->common.close((hw_device_t*)t);
162- }
163-};
164+
165+void alloc_dev_deleter(alloc_device_t* t)
166+{
167+ /* android takes care of delete for us */
168+ t->common.close((hw_device_t*)t);
169+}
170+
171+void null_alloc_dev_deleter(alloc_device_t*)
172+{
173+}
174+
175 }
176
177 mga::AndroidGraphicBufferAllocator::AndroidGraphicBufferAllocator(std::shared_ptr<DeviceQuirks> const& quirks)
178@@ -63,8 +67,9 @@
179 /* note for future use: at this point, the hardware module should be filled with vendor information
180 that we can determine different courses of action based upon */
181
182- AllocDevDeleter del;
183- std::shared_ptr<struct alloc_device_t> alloc_dev_ptr(alloc_dev, del);
184+ std::shared_ptr<struct alloc_device_t> alloc_dev_ptr(
185+ alloc_dev,
186+ quirks->gralloc_cannot_be_closed_safely() ? null_alloc_dev_deleter : alloc_dev_deleter);
187 alloc_device = std::shared_ptr<mga::GraphicAllocAdaptor>(new AndroidAllocAdaptor(alloc_dev_ptr, quirks));
188 }
189
190
191=== modified file 'src/platforms/android/server/device_quirks.cpp'
192--- src/platforms/android/server/device_quirks.cpp 2015-06-24 11:33:02 +0000
193+++ src/platforms/android/server/device_quirks.cpp 2015-11-25 13:03:44 +0000
194@@ -35,7 +35,7 @@
195 namespace
196 {
197 char const* const num_framebuffers_opt = "enable-num-framebuffers-quirk";
198-char const* const single_gralloc_instance_opt = "enable-single-gralloc-instance-quirk";
199+char const* const gralloc_cannot_be_closed_safely_opt = "enable-gralloc-cannot-be-closed-safely-quirk";
200 char const* const width_alignment_opt = "enable-width-alignment-quirk";
201
202 std::string determine_device_name(mga::PropertiesWrapper const& properties)
203@@ -55,16 +55,16 @@
204 return 2;
205 }
206
207-unsigned int gralloc_reopenable_after_close_for(std::string const& device_name, bool quirk_enabled)
208+bool gralloc_cannot_be_closed_safely_for(std::string const& device_name, bool quirk_enabled)
209 {
210- return !(quirk_enabled && device_name == std::string{"krillin"});
211+ return quirk_enabled && device_name == std::string{"krillin"};
212 }
213 }
214
215 mga::DeviceQuirks::DeviceQuirks(PropertiesWrapper const& properties)
216 : device_name(determine_device_name(properties)),
217 num_framebuffers_(num_framebuffers_for(device_name, true)),
218- gralloc_reopenable_after_close_(gralloc_reopenable_after_close_for(device_name, true)),
219+ gralloc_cannot_be_closed_safely_(gralloc_cannot_be_closed_safely_for(device_name, true)),
220 enable_width_alignment_quirk{true}
221 {
222 }
223@@ -72,7 +72,7 @@
224 mga::DeviceQuirks::DeviceQuirks(PropertiesWrapper const& properties, mo::Option const& options)
225 : device_name(determine_device_name(properties)),
226 num_framebuffers_(num_framebuffers_for(device_name, options.get(num_framebuffers_opt, true))),
227- gralloc_reopenable_after_close_(gralloc_reopenable_after_close_for(device_name, options.get(single_gralloc_instance_opt, true))),
228+ gralloc_cannot_be_closed_safely_(gralloc_cannot_be_closed_safely_for(device_name, options.get(gralloc_cannot_be_closed_safely_opt, true))),
229 enable_width_alignment_quirk(options.get(width_alignment_opt, true))
230 {
231 }
232@@ -82,9 +82,9 @@
233 return num_framebuffers_;
234 }
235
236-bool mga::DeviceQuirks::gralloc_reopenable_after_close() const
237+bool mga::DeviceQuirks::gralloc_cannot_be_closed_safely() const
238 {
239- return gralloc_reopenable_after_close_;
240+ return gralloc_cannot_be_closed_safely_;
241 }
242
243 int mga::DeviceQuirks::aligned_width(int width) const
244@@ -100,9 +100,9 @@
245 (num_framebuffers_opt,
246 boost::program_options::value<bool>()->default_value(true),
247 "[platform-specific] Enable allocating 3 framebuffers (MX3 quirk) [{true,false}]")
248- (single_gralloc_instance_opt,
249+ (gralloc_cannot_be_closed_safely_opt,
250 boost::program_options::value<bool>()->default_value(true),
251- "[platform-specific] Allocate a single gralloc instance (krillin quirk) [{true,false}]")
252+ "[platform-specific] Only close gralloc if it is safe to do so (krillin quirk) [{true,false}]")
253 (width_alignment_opt,
254 boost::program_options::value<bool>()->default_value(true),
255 "[platform-specific] Enable width alignment (vegetahd quirk) [{true,false}]");
256
257=== modified file 'src/platforms/android/server/device_quirks.h'
258--- src/platforms/android/server/device_quirks.h 2015-06-24 11:33:02 +0000
259+++ src/platforms/android/server/device_quirks.h 2015-11-25 13:03:44 +0000
260@@ -61,7 +61,7 @@
261 DeviceQuirks(PropertiesWrapper const& properties, mir::options::Option const& options);
262
263 unsigned int num_framebuffers() const;
264- bool gralloc_reopenable_after_close() const;
265+ bool gralloc_cannot_be_closed_safely() const;
266 int aligned_width(int width) const;
267
268 static void add_options(boost::program_options::options_description& config);
269@@ -71,7 +71,7 @@
270 DeviceQuirks & operator=(DeviceQuirks const&) = delete;
271 std::string const device_name;
272 unsigned int const num_framebuffers_;
273- bool const gralloc_reopenable_after_close_;
274+ bool const gralloc_cannot_be_closed_safely_;
275 bool const enable_width_alignment_quirk;
276 };
277 }
278
279=== modified file 'tests/integration-tests/test_buffer_scheduling.cpp'
280--- tests/integration-tests/test_buffer_scheduling.cpp 2015-11-04 17:41:31 +0000
281+++ tests/integration-tests/test_buffer_scheduling.cpp 2015-11-25 13:03:44 +0000
282@@ -396,7 +396,7 @@
283 {
284 if (can_produce())
285 {
286- auto buffer = vault.withdraw().get();
287+ auto buffer = vault.withdraw().get().buffer;
288 vault.deposit(buffer);
289 vault.wire_transfer_outbound(buffer);
290 last_size_ = buffer->size();
291
292=== modified file 'tests/unit-tests/client/test_buffer_vault.cpp'
293--- tests/unit-tests/client/test_buffer_vault.cpp 2015-10-20 03:30:00 +0000
294+++ tests/unit-tests/client/test_buffer_vault.cpp 2015-11-25 13:03:44 +0000
295@@ -162,7 +162,7 @@
296 mcl::BufferVault vault(mt::fake_shared(mock_factory), mt::fake_shared(mock_requests),
297 size, format, usage, initial_nbuffers);
298 vault.wire_transfer_inbound(package);
299- auto b = vault.withdraw().get();
300+ auto b = vault.withdraw().get().buffer;
301 vault.deposit(b);
302 vault.wire_transfer_outbound(b);
303 vault.wire_transfer_inbound(package);
304@@ -183,12 +183,12 @@
305 {
306 auto buffer_future = vault.withdraw();
307 ASSERT_TRUE(buffer_future.valid());
308- EXPECT_THAT(buffer_future.get(), Ne(nullptr));;
309+ EXPECT_THAT(buffer_future.get().buffer, Ne(nullptr));;
310 }
311
312 TEST_F(StartedBufferVault, can_deposit_buffer)
313 {
314- auto buffer = vault.withdraw().get();
315+ auto buffer = vault.withdraw().get().buffer;
316 EXPECT_CALL(mock_requests, submit_buffer(_,Ref(*buffer)));
317 vault.deposit(buffer);
318 vault.wire_transfer_outbound(buffer);
319@@ -196,7 +196,7 @@
320
321 TEST_F(StartedBufferVault, cant_transfer_if_not_in_acct)
322 {
323- auto buffer = vault.withdraw().get();
324+ auto buffer = vault.withdraw().get().buffer;
325 EXPECT_THROW({
326 vault.wire_transfer_outbound(buffer);
327 }, std::logic_error);
328@@ -212,7 +212,7 @@
329
330 TEST_F(StartedBufferVault, attempt_to_redeposit_throws)
331 {
332- auto buffer = vault.withdraw().get();
333+ auto buffer = vault.withdraw().get().buffer;
334 vault.deposit(buffer);
335 vault.wire_transfer_outbound(buffer);
336 EXPECT_THROW({
337@@ -228,13 +228,13 @@
338 EXPECT_CALL(mock_factory, create_buffer(_,initial_properties.size,initial_properties.format))
339 .Times(Exactly(1));
340 vault.wire_transfer_inbound(package);
341- auto buffer = vault.withdraw().get();
342+ auto buffer = vault.withdraw().get().buffer;
343 vault.deposit(buffer);
344 vault.wire_transfer_outbound(buffer);
345
346 //should just activate, not create the buffer
347 vault.wire_transfer_inbound(package);
348- auto buffer2 = vault.withdraw().get();
349+ auto buffer2 = vault.withdraw().get().buffer;
350 EXPECT_THAT(buffer, Eq(buffer2));
351 }
352
353@@ -242,7 +242,7 @@
354 {
355 auto buffer1 = vault.withdraw().get();
356 auto buffer2 = vault.withdraw().get();
357- EXPECT_THAT(buffer1, Ne(buffer2));
358+ EXPECT_THAT(buffer1.buffer, Ne(buffer2.buffer));
359 }
360
361 TEST_F(BufferVault, multiple_withdrawals_during_wait_period_get_differing_buffers)
362@@ -257,13 +257,13 @@
363
364 auto buffer1 = f_buffer1.get();
365 auto buffer2 = f_buffer2.get();
366- EXPECT_THAT(buffer1, Ne(buffer2));
367+ EXPECT_THAT(buffer1.buffer, Ne(buffer2.buffer));
368 }
369
370 TEST_F(BufferVault, destruction_signals_futures)
371 {
372 using namespace std::literals::chrono_literals;
373- mcl::NoTLSFuture<std::shared_ptr<mcl::ClientBuffer>> fbuffer;
374+ mcl::NoTLSFuture<mcl::BufferInfo> fbuffer;
375 {
376 mcl::BufferVault vault(mt::fake_shared(mock_factory), mt::fake_shared(mock_requests),
377 size, format, usage, initial_nbuffers);
378@@ -282,7 +282,7 @@
379 mcl::BufferVault vault(mt::fake_shared(mock_factory), mt::fake_shared(mock_requests),
380 size, format, usage, initial_nbuffers);
381 vault.wire_transfer_inbound(package);
382- vault.deposit(vault.withdraw().get());
383+ vault.deposit(vault.withdraw().get().buffer);
384 }
385
386 TEST_F(BufferVault, marks_as_submitted_on_transfer)
387@@ -296,7 +296,7 @@
388 size, format, usage, initial_nbuffers);
389 vault.wire_transfer_inbound(package);
390
391- auto buffer = vault.withdraw().get();
392+ auto buffer = vault.withdraw().get().buffer;
393 vault.deposit(buffer);
394 vault.wire_transfer_outbound(buffer);
395 }
396@@ -308,7 +308,7 @@
397 std::vector<std::shared_ptr<mcl::ClientBuffer>> buffers(a_few_times);
398 for (auto i = 0u; i < a_few_times; i++)
399 {
400- buffers[i] = vault.withdraw().get();
401+ buffers[i] = vault.withdraw().get().buffer;
402 vault.deposit(buffers[i]);
403 }
404 EXPECT_THAT(buffers, Each(buffers[0]));
405@@ -349,7 +349,7 @@
406 vault.set_size(new_size);
407 vault.wire_transfer_inbound(package);
408 vault.wire_transfer_inbound(package4);
409- EXPECT_THAT(vault.withdraw().get()->size(), Eq(new_size));
410+ EXPECT_THAT(vault.withdraw().get().buffer->size(), Eq(new_size));
411 Mock::VerifyAndClearExpectations(&mock_requests);
412 }
413
414@@ -364,7 +364,7 @@
415 vault.set_size(new_size);
416 vault.wire_transfer_inbound(package4);
417
418- EXPECT_THAT(vault.withdraw().get()->size(), Eq(new_size));
419+ EXPECT_THAT(vault.withdraw().get().buffer->size(), Eq(new_size));
420 Mock::VerifyAndClearExpectations(&mock_requests);
421 }
422
423
424=== modified file 'tests/unit-tests/client/test_client_buffer_stream.cpp'
425--- tests/unit-tests/client/test_client_buffer_stream.cpp 2015-11-18 12:42:24 +0000
426+++ tests/unit-tests/client/test_client_buffer_stream.cpp 2015-11-25 13:03:44 +0000
427@@ -348,6 +348,7 @@
428 nullptr, mock_protobuf_server, mcl::BufferStreamMode::Consumer,
429 std::make_shared<StubClientPlatform>(mt::fake_shared(stub_factory)),
430 response, perf_report, "", size, nbuffers);
431+ service_requests_for(bs, mock_protobuf_server.alloc_count);
432 auto wh = bs.next_buffer([]{});
433 ASSERT_THAT(wh, NotNull());
434 EXPECT_FALSE(wh->is_pending());
435@@ -685,4 +686,25 @@
436 EXPECT_THAT(params.pixel_format, Eq(format));
437 }
438
439+TEST_P(ClientBufferStream, keeps_accurate_buffer_id)
440+{
441+ ON_CALL(mock_factory, create_buffer(_,_,_))
442+ .WillByDefault(Return(std::make_shared<mtd::NullClientBuffer>(size)));
443+ mcl::BufferStream stream(
444+ nullptr, mock_protobuf_server, mode,
445+ std::make_shared<StubClientPlatform>(mt::fake_shared(mock_factory)),
446+ response, perf_report, "", size, nbuffers);
447+ for(auto i = 0u; i < 2; i++)
448+ {
449+ mp::Buffer buffer;
450+ fill_protobuf_buffer_from_package(&buffer, a_buffer_package());
451+ buffer.set_buffer_id(i+10);
452+ buffer.set_width(size.width.as_int());
453+ buffer.set_height(size.height.as_int());
454+ stream.buffer_available(buffer);
455+ }
456+
457+ EXPECT_THAT(stream.get_current_buffer_id(), Eq(10));
458+}
459+
460 INSTANTIATE_TEST_CASE_P(BufferSemanticsMode, ClientBufferStream, Bool());
461
462=== modified file 'tests/unit-tests/graphics/android/test_device_detection.cpp'
463--- tests/unit-tests/graphics/android/test_device_detection.cpp 2015-06-24 11:33:02 +0000
464+++ tests/unit-tests/graphics/android/test_device_detection.cpp 2015-11-25 13:03:44 +0000
465@@ -75,7 +75,7 @@
466 }
467
468 //LP: 1371619, 1370555
469-TEST(DeviceDetection, reports_gralloc_reopenable_after_close_by_default)
470+TEST(DeviceDetection, reports_gralloc_can_be_closed_safely_by_default)
471 {
472 using namespace testing;
473 char const default_str[] = "";
474@@ -91,10 +91,10 @@
475 }));
476
477 mga::DeviceQuirks quirks(mock_ops);
478- EXPECT_TRUE(quirks.gralloc_reopenable_after_close());
479+ EXPECT_FALSE(quirks.gralloc_cannot_be_closed_safely());
480 }
481
482-TEST(DeviceDetection, reports_gralloc_not_reopenable_after_close_on_krillin)
483+TEST(DeviceDetection, reports_gralloc_cannot_be_closed_safely_on_krillin)
484 {
485 using namespace testing;
486 char const default_str[] = "";
487@@ -110,7 +110,7 @@
488 }));
489
490 mga::DeviceQuirks quirks(mock_ops);
491- EXPECT_FALSE(quirks.gralloc_reopenable_after_close());
492+ EXPECT_TRUE(quirks.gralloc_cannot_be_closed_safely());
493 }
494
495 TEST(DeviceDetection, aligns_width_on_vegetahd)
496@@ -150,12 +150,12 @@
497 options.parse_arguments(desc, argc, argv);
498 }
499
500- void disable_single_gralloc_instance_quirk()
501+ void disable_gralloc_cannot_be_closed_safely_quirk()
502 {
503 int const argc = 2;
504 char const* argv[argc] = {
505 __PRETTY_FUNCTION__,
506- "--enable-single-gralloc-instance-quirk=false"
507+ "--enable-gralloc-cannot-be-closed-safely-quirk=false"
508 };
509
510 options.parse_arguments(desc, argc, argv);
511@@ -197,7 +197,7 @@
512 EXPECT_THAT(quirks.num_framebuffers(), Ne(3));
513 }
514
515-TEST_F(DeviceQuirks, single_gralloc_instance_quirk_can_be_disabled)
516+TEST_F(DeviceQuirks, gralloc_cannot_be_closed_safely_quirk_can_be_disabled)
517 {
518 using namespace testing;
519 char const default_str[] = "";
520@@ -212,9 +212,9 @@
521 return 0;
522 }));
523
524- disable_single_gralloc_instance_quirk();
525+ disable_gralloc_cannot_be_closed_safely_quirk();
526 mga::DeviceQuirks quirks(mock_ops, options);
527- EXPECT_TRUE(quirks.gralloc_reopenable_after_close());
528+ EXPECT_FALSE(quirks.gralloc_cannot_be_closed_safely());
529 }
530
531 TEST_F(DeviceQuirks, width_alignment_quirk_can_be_disabled)

Subscribers

People subscribed via source and target branches