Skip to content

Commit 434bc98

Browse files
jreckjreck
committed
Move PlaneLayout lookup to GraphicBuffer
This keeps GraphicBufferMapper's lock closer to a raw lock avoiding the PlaneLayout query if nobody is asking for it, reduces code duplication between Gralloc 4 & 5, and enables caching of the PlaneLayout to minimize queries Partial step towards an extended lock buffer API Also begin modernizing the API surface using a proper Result<T> along with an error enum class Test: make && atest Bug: 288142323 Change-Id: Ib4590b7a3b839a993f6c747c5f09a6312f7ab329
1 parent 19db87d commit 434bc98

6 files changed

Lines changed: 333 additions & 130 deletions

File tree

‎libs/ui/Gralloc4.cpp‎

Lines changed: 2 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -262,37 +262,8 @@ void Gralloc4Mapper::getTransportSize(buffer_handle_t bufferHandle, uint32_t* ou
262262
status_t Gralloc4Mapper::lock(buffer_handle_t bufferHandle, uint64_t usage, const Rect& bounds,
263263
int acquireFence, void** outData, int32_t* outBytesPerPixel,
264264
int32_t* outBytesPerStride) const {
265-
std::vector<ui::PlaneLayout> planeLayouts;
266-
status_t err = getPlaneLayouts(bufferHandle, &planeLayouts);
267-
268-
if (err == NO_ERROR && !planeLayouts.empty()) {
269-
if (outBytesPerPixel) {
270-
int32_t bitsPerPixel = planeLayouts.front().sampleIncrementInBits;
271-
for (const auto& planeLayout : planeLayouts) {
272-
if (bitsPerPixel != planeLayout.sampleIncrementInBits) {
273-
bitsPerPixel = -1;
274-
}
275-
}
276-
if (bitsPerPixel >= 0 && bitsPerPixel % 8 == 0) {
277-
*outBytesPerPixel = bitsPerPixel / 8;
278-
} else {
279-
*outBytesPerPixel = -1;
280-
}
281-
}
282-
if (outBytesPerStride) {
283-
int32_t bytesPerStride = planeLayouts.front().strideInBytes;
284-
for (const auto& planeLayout : planeLayouts) {
285-
if (bytesPerStride != planeLayout.strideInBytes) {
286-
bytesPerStride = -1;
287-
}
288-
}
289-
if (bytesPerStride >= 0) {
290-
*outBytesPerStride = bytesPerStride;
291-
} else {
292-
*outBytesPerStride = -1;
293-
}
294-
}
295-
}
265+
if (outBytesPerPixel) *outBytesPerPixel = -1;
266+
if (outBytesPerStride) *outBytesPerStride = -1;
296267

297268
auto buffer = const_cast<native_handle_t*>(bufferHandle);
298269

‎libs/ui/Gralloc5.cpp‎

Lines changed: 2 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -589,37 +589,8 @@ void Gralloc5Mapper::getTransportSize(buffer_handle_t bufferHandle, uint32_t *ou
589589
status_t Gralloc5Mapper::lock(buffer_handle_t bufferHandle, uint64_t usage, const Rect &bounds,
590590
int acquireFence, void **outData, int32_t *outBytesPerPixel,
591591
int32_t *outBytesPerStride) const {
592-
std::vector<ui::PlaneLayout> planeLayouts;
593-
status_t err = getPlaneLayouts(bufferHandle, &planeLayouts);
594-
595-
if (err == NO_ERROR && !planeLayouts.empty()) {
596-
if (outBytesPerPixel) {
597-
int32_t bitsPerPixel = planeLayouts.front().sampleIncrementInBits;
598-
for (const auto &planeLayout : planeLayouts) {
599-
if (bitsPerPixel != planeLayout.sampleIncrementInBits) {
600-
bitsPerPixel = -1;
601-
}
602-
}
603-
if (bitsPerPixel >= 0 && bitsPerPixel % 8 == 0) {
604-
*outBytesPerPixel = bitsPerPixel / 8;
605-
} else {
606-
*outBytesPerPixel = -1;
607-
}
608-
}
609-
if (outBytesPerStride) {
610-
int32_t bytesPerStride = planeLayouts.front().strideInBytes;
611-
for (const auto &planeLayout : planeLayouts) {
612-
if (bytesPerStride != planeLayout.strideInBytes) {
613-
bytesPerStride = -1;
614-
}
615-
}
616-
if (bytesPerStride >= 0) {
617-
*outBytesPerStride = bytesPerStride;
618-
} else {
619-
*outBytesPerStride = -1;
620-
}
621-
}
622-
}
592+
if (outBytesPerPixel) *outBytesPerPixel = -1;
593+
if (outBytesPerStride) *outBytesPerStride = -1;
623594

624595
auto status = mMapper->v5.lock(bufferHandle, usage, bounds, acquireFence, outData);
625596

‎libs/ui/GraphicBuffer.cpp‎

Lines changed: 86 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
#include <cutils/atomic.h>
2323

2424
#include <grallocusage/GrallocUsageConversion.h>
25-
25+
#include <sync/sync.h>
2626
#include <ui/GraphicBufferAllocator.h>
2727
#include <ui/GraphicBufferMapper.h>
2828
#include <utils/Trace.h>
@@ -40,6 +40,38 @@ static uint64_t getUniqueId() {
4040
return id;
4141
}
4242

43+
static void resolveLegacyByteLayoutFromPlaneLayout(const std::vector<ui::PlaneLayout>& planeLayouts,
44+
int32_t* outBytesPerPixel,
45+
int32_t* outBytesPerStride) {
46+
if (planeLayouts.empty()) return;
47+
if (outBytesPerPixel) {
48+
int32_t bitsPerPixel = planeLayouts.front().sampleIncrementInBits;
49+
for (const auto& planeLayout : planeLayouts) {
50+
if (bitsPerPixel != planeLayout.sampleIncrementInBits) {
51+
bitsPerPixel = -1;
52+
}
53+
}
54+
if (bitsPerPixel >= 0 && bitsPerPixel % 8 == 0) {
55+
*outBytesPerPixel = bitsPerPixel / 8;
56+
} else {
57+
*outBytesPerPixel = -1;
58+
}
59+
}
60+
if (outBytesPerStride) {
61+
int32_t bytesPerStride = planeLayouts.front().strideInBytes;
62+
for (const auto& planeLayout : planeLayouts) {
63+
if (bytesPerStride != planeLayout.strideInBytes) {
64+
bytesPerStride = -1;
65+
}
66+
}
67+
if (bytesPerStride >= 0) {
68+
*outBytesPerStride = bytesPerStride;
69+
} else {
70+
*outBytesPerStride = -1;
71+
}
72+
}
73+
}
74+
4375
sp<GraphicBuffer> GraphicBuffer::from(ANativeWindowBuffer* anwb) {
4476
return static_cast<GraphicBuffer *>(anwb);
4577
}
@@ -279,10 +311,7 @@ status_t GraphicBuffer::lock(uint32_t inUsage, const Rect& rect, void** vaddr,
279311
return BAD_VALUE;
280312
}
281313

282-
status_t res = getBufferMapper().lock(handle, inUsage, rect, vaddr, outBytesPerPixel,
283-
outBytesPerStride);
284-
285-
return res;
314+
return lockAsync(inUsage, rect, vaddr, -1, outBytesPerPixel, outBytesPerStride);
286315
}
287316

288317
status_t GraphicBuffer::lockYCbCr(uint32_t inUsage, android_ycbcr* ycbcr)
@@ -302,14 +331,12 @@ status_t GraphicBuffer::lockYCbCr(uint32_t inUsage, const Rect& rect,
302331
width, height);
303332
return BAD_VALUE;
304333
}
305-
status_t res = getBufferMapper().lockYCbCr(handle, inUsage, rect, ycbcr);
306-
return res;
334+
return lockAsyncYCbCr(inUsage, rect, ycbcr, -1);
307335
}
308336

309337
status_t GraphicBuffer::unlock()
310338
{
311-
status_t res = getBufferMapper().unlock(handle);
312-
return res;
339+
return unlockAsync(nullptr);
313340
}
314341

315342
status_t GraphicBuffer::lockAsync(uint32_t inUsage, void** vaddr, int fenceFd,
@@ -336,10 +363,49 @@ status_t GraphicBuffer::lockAsync(uint64_t inProducerUsage, uint64_t inConsumerU
336363
return BAD_VALUE;
337364
}
338365

339-
status_t res = getBufferMapper().lockAsync(handle, inProducerUsage, inConsumerUsage, rect,
340-
vaddr, fenceFd, outBytesPerPixel, outBytesPerStride);
366+
// Resolve the bpp & bps before doing a lock in case this fails we don't have to worry about
367+
// doing an unlock
368+
int32_t legacyBpp = -1, legacyBps = -1;
369+
if (outBytesPerPixel || outBytesPerStride) {
370+
const auto mapperVersion = getBufferMapperVersion();
371+
// For gralloc2 we need to guess at the bpp & bps
372+
// For gralloc3 the lock() call will return it
373+
// For gralloc4 & later the PlaneLayout metadata query is vastly superior and we
374+
// resolve bpp & bps just for compatibility
375+
376+
// TODO: See if we can't just remove gralloc2 support.
377+
if (mapperVersion == GraphicBufferMapper::GRALLOC_2) {
378+
legacyBpp = bytesPerPixel(format);
379+
if (legacyBpp > 0) {
380+
legacyBps = stride * legacyBpp;
381+
} else {
382+
legacyBpp = -1;
383+
}
384+
} else if (mapperVersion >= GraphicBufferMapper::GRALLOC_4) {
385+
auto planeLayout = getBufferMapper().getPlaneLayouts(handle);
386+
if (!planeLayout.has_value()) return planeLayout.asStatus();
387+
resolveLegacyByteLayoutFromPlaneLayout(planeLayout.value(), &legacyBpp, &legacyBps);
388+
}
389+
}
341390

342-
return res;
391+
const uint64_t usage = static_cast<uint64_t>(
392+
android_convertGralloc1To0Usage(inProducerUsage, inConsumerUsage));
393+
394+
auto result = getBufferMapper().lock(handle, usage, rect, base::unique_fd{fenceFd});
395+
396+
if (!result.has_value()) {
397+
return result.error().asStatus();
398+
}
399+
auto value = result.value();
400+
*vaddr = value.address;
401+
402+
if (outBytesPerPixel) {
403+
*outBytesPerPixel = legacyBpp != -1 ? legacyBpp : value.bytesPerPixel;
404+
}
405+
if (outBytesPerStride) {
406+
*outBytesPerStride = legacyBps != -1 ? legacyBps : value.bytesPerStride;
407+
}
408+
return OK;
343409
}
344410

345411
status_t GraphicBuffer::lockAsyncYCbCr(uint32_t inUsage, android_ycbcr* ycbcr,
@@ -360,14 +426,18 @@ status_t GraphicBuffer::lockAsyncYCbCr(uint32_t inUsage, const Rect& rect,
360426
width, height);
361427
return BAD_VALUE;
362428
}
363-
status_t res = getBufferMapper().lockAsyncYCbCr(handle, inUsage, rect, ycbcr, fenceFd);
364-
return res;
429+
auto result = getBufferMapper().lockYCbCr(handle, static_cast<int64_t>(inUsage), rect,
430+
base::unique_fd{fenceFd});
431+
if (!result.has_value()) {
432+
return result.error().asStatus();
433+
}
434+
*ycbcr = result.value();
435+
return OK;
365436
}
366437

367438
status_t GraphicBuffer::unlockAsync(int *fenceFd)
368439
{
369-
status_t res = getBufferMapper().unlockAsync(handle, fenceFd);
370-
return res;
440+
return getBufferMapper().unlockAsync(handle, fenceFd);
371441
}
372442

373443
status_t GraphicBuffer::isSupported(uint32_t inWidth, uint32_t inHeight, PixelFormat inFormat,

‎libs/ui/GraphicBufferMapper.cpp‎

Lines changed: 81 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,13 @@
4141

4242
#include <system/graphics.h>
4343

44+
using unique_fd = ::android::base::unique_fd;
45+
4446
namespace android {
4547
// ---------------------------------------------------------------------------
4648

49+
using LockResult = GraphicBufferMapper::LockResult;
50+
4751
ANDROID_SINGLETON_STATIC_INSTANCE( GraphicBufferMapper )
4852

4953
void GraphicBufferMapper::preloadHal() {
@@ -135,63 +139,86 @@ status_t GraphicBufferMapper::freeBuffer(buffer_handle_t handle)
135139
return NO_ERROR;
136140
}
137141

138-
status_t GraphicBufferMapper::lock(buffer_handle_t handle, uint32_t usage, const Rect& bounds,
139-
void** vaddr, int32_t* outBytesPerPixel,
140-
int32_t* outBytesPerStride) {
141-
return lockAsync(handle, usage, bounds, vaddr, -1, outBytesPerPixel, outBytesPerStride);
142-
}
143-
144-
status_t GraphicBufferMapper::lockYCbCr(buffer_handle_t handle, uint32_t usage,
145-
const Rect& bounds, android_ycbcr *ycbcr)
146-
{
147-
return lockAsyncYCbCr(handle, usage, bounds, ycbcr, -1);
148-
}
142+
ui::Result<LockResult> GraphicBufferMapper::lock(buffer_handle_t handle, int64_t usage,
143+
const Rect& bounds, unique_fd&& acquireFence) {
144+
ATRACE_CALL();
149145

150-
status_t GraphicBufferMapper::unlock(buffer_handle_t handle)
151-
{
152-
int32_t fenceFd = -1;
153-
status_t error = unlockAsync(handle, &fenceFd);
154-
if (error == NO_ERROR && fenceFd >= 0) {
155-
sync_wait(fenceFd, -1);
156-
close(fenceFd);
146+
LockResult result;
147+
status_t status = mMapper->lock(handle, usage, bounds, acquireFence.release(), &result.address,
148+
&result.bytesPerPixel, &result.bytesPerStride);
149+
if (status != OK) {
150+
return base::unexpected(ui::Error::statusToCode(status));
151+
} else {
152+
return result;
157153
}
158-
return error;
159-
}
160-
161-
status_t GraphicBufferMapper::lockAsync(buffer_handle_t handle, uint32_t usage, const Rect& bounds,
162-
void** vaddr, int fenceFd, int32_t* outBytesPerPixel,
163-
int32_t* outBytesPerStride) {
164-
return lockAsync(handle, usage, usage, bounds, vaddr, fenceFd, outBytesPerPixel,
165-
outBytesPerStride);
166154
}
167155

168-
status_t GraphicBufferMapper::lockAsync(buffer_handle_t handle, uint64_t producerUsage,
169-
uint64_t consumerUsage, const Rect& bounds, void** vaddr,
170-
int fenceFd, int32_t* outBytesPerPixel,
171-
int32_t* outBytesPerStride) {
156+
ui::Result<android_ycbcr> GraphicBufferMapper::lockYCbCr(buffer_handle_t handle, int64_t usage,
157+
const Rect& bounds,
158+
base::unique_fd&& acquireFence) {
172159
ATRACE_CALL();
173160

174-
const uint64_t usage = static_cast<uint64_t>(
175-
android_convertGralloc1To0Usage(producerUsage, consumerUsage));
176-
return mMapper->lock(handle, usage, bounds, fenceFd, vaddr, outBytesPerPixel,
177-
outBytesPerStride);
161+
android_ycbcr result = {};
162+
status_t status = mMapper->lock(handle, usage, bounds, acquireFence.release(), &result);
163+
if (status != OK) {
164+
return base::unexpected(ui::Error::statusToCode(status));
165+
} else {
166+
return result;
167+
}
178168
}
179169

180-
status_t GraphicBufferMapper::lockAsyncYCbCr(buffer_handle_t handle,
181-
uint32_t usage, const Rect& bounds, android_ycbcr *ycbcr, int fenceFd)
182-
{
170+
status_t GraphicBufferMapper::unlock(buffer_handle_t handle, base::unique_fd* outFence) {
183171
ATRACE_CALL();
172+
int fence = mMapper->unlock(handle);
173+
if (outFence) {
174+
*outFence = unique_fd{fence};
175+
} else {
176+
sync_wait(fence, -1);
177+
close(fence);
178+
}
179+
return OK;
180+
}
184181

185-
return mMapper->lock(handle, usage, bounds, fenceFd, ycbcr);
182+
status_t GraphicBufferMapper::lock(buffer_handle_t handle, uint32_t usage, const Rect& bounds,
183+
void** vaddr) {
184+
auto result = lock(handle, static_cast<int64_t>(usage), bounds);
185+
if (!result.has_value()) return result.asStatus();
186+
auto val = result.value();
187+
*vaddr = val.address;
188+
return OK;
186189
}
187190

188-
status_t GraphicBufferMapper::unlockAsync(buffer_handle_t handle, int *fenceFd)
189-
{
190-
ATRACE_CALL();
191+
status_t GraphicBufferMapper::lockYCbCr(buffer_handle_t handle, uint32_t usage, const Rect& bounds,
192+
android_ycbcr* ycbcr) {
193+
auto result = lockYCbCr(handle, static_cast<int64_t>(usage), bounds);
194+
if (!result.has_value()) return result.asStatus();
195+
*ycbcr = result.value();
196+
return OK;
197+
}
198+
199+
status_t GraphicBufferMapper::lockAsync(buffer_handle_t handle, uint32_t usage, const Rect& bounds,
200+
void** vaddr, int fenceFd) {
201+
auto result = lock(handle, static_cast<int64_t>(usage), bounds, unique_fd{fenceFd});
202+
if (!result.has_value()) return result.asStatus();
203+
auto val = result.value();
204+
*vaddr = val.address;
205+
return OK;
206+
}
191207

192-
*fenceFd = mMapper->unlock(handle);
208+
status_t GraphicBufferMapper::lockAsync(buffer_handle_t handle, uint64_t producerUsage,
209+
uint64_t consumerUsage, const Rect& bounds, void** vaddr,
210+
int fenceFd) {
211+
return lockAsync(handle, android_convertGralloc1To0Usage(producerUsage, consumerUsage), bounds,
212+
vaddr, fenceFd);
213+
}
193214

194-
return NO_ERROR;
215+
status_t GraphicBufferMapper::lockAsyncYCbCr(buffer_handle_t handle, uint32_t usage,
216+
const Rect& bounds, android_ycbcr* ycbcr,
217+
int fenceFd) {
218+
auto result = lockYCbCr(handle, static_cast<int64_t>(usage), bounds, unique_fd{fenceFd});
219+
if (!result.has_value()) return result.asStatus();
220+
*ycbcr = result.value();
221+
return OK;
195222
}
196223

197224
status_t GraphicBufferMapper::isSupported(uint32_t width, uint32_t height,
@@ -287,6 +314,17 @@ status_t GraphicBufferMapper::getPlaneLayouts(buffer_handle_t bufferHandle,
287314
return mMapper->getPlaneLayouts(bufferHandle, outPlaneLayouts);
288315
}
289316

317+
ui::Result<std::vector<ui::PlaneLayout>> GraphicBufferMapper::getPlaneLayouts(
318+
buffer_handle_t bufferHandle) {
319+
std::vector<ui::PlaneLayout> temp;
320+
status_t status = mMapper->getPlaneLayouts(bufferHandle, &temp);
321+
if (status == OK) {
322+
return std::move(temp);
323+
} else {
324+
return base::unexpected(ui::Error::statusToCode(status));
325+
}
326+
}
327+
290328
status_t GraphicBufferMapper::getDataspace(buffer_handle_t bufferHandle,
291329
ui::Dataspace* outDataspace) {
292330
return mMapper->getDataspace(bufferHandle, outDataspace);

0 commit comments

Comments
 (0)