From dcc9d9b5cf697d238cbdca93a99a7a3aa48281fe Mon Sep 17 00:00:00 2001 From: Marzia Favaro Date: Wed, 10 Jan 2024 10:17:00 +0000 Subject: Edge extension effect: shader reimplementation X-axis activity transitions require the translation of the surfaces involved. As this movement would create unwanted see-through, we would have added side windows anchored to the moving activities, and textured them by clamping their parents. We are replacing the additional windows with the extension of the surfaces, and filling the area without buffer with a shader. See go/edge-extension-sksl for more info. Bug: 322036393 Test: LayerSnapshotTest Flag: EXEMPT (calls will be protected by wm shell with com.android.window.flags.edge_extension_shader) Change-Id: I3682efd16a1b311d67a522bb85960f100948b2ea --- libs/gui/LayerState.cpp | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'libs/gui/LayerState.cpp') diff --git a/libs/gui/LayerState.cpp b/libs/gui/LayerState.cpp index 3745805aa3..307ae3990e 100644 --- a/libs/gui/LayerState.cpp +++ b/libs/gui/LayerState.cpp @@ -177,6 +177,7 @@ status_t layer_state_t::write(Parcel& output) const } SAFE_PARCEL(output.write, stretchEffect); + SAFE_PARCEL(output.writeParcelable, edgeExtensionParameters); SAFE_PARCEL(output.write, bufferCrop); SAFE_PARCEL(output.write, destinationFrame); SAFE_PARCEL(output.writeInt32, static_cast(trustedOverlay)); @@ -306,6 +307,7 @@ status_t layer_state_t::read(const Parcel& input) } SAFE_PARCEL(input.read, stretchEffect); + SAFE_PARCEL(input.readParcelable, &edgeExtensionParameters); SAFE_PARCEL(input.read, bufferCrop); SAFE_PARCEL(input.read, destinationFrame); uint32_t trustedOverlayInt; @@ -682,6 +684,10 @@ void layer_state_t::merge(const layer_state_t& other) { what |= eStretchChanged; stretchEffect = other.stretchEffect; } + if (other.what & eEdgeExtensionChanged) { + what |= eEdgeExtensionChanged; + edgeExtensionParameters = other.edgeExtensionParameters; + } if (other.what & eBufferCropChanged) { what |= eBufferCropChanged; bufferCrop = other.bufferCrop; @@ -783,6 +789,7 @@ uint64_t layer_state_t::diff(const layer_state_t& other) const { CHECK_DIFF(diff, eAutoRefreshChanged, other, autoRefresh); CHECK_DIFF(diff, eTrustedOverlayChanged, other, trustedOverlay); CHECK_DIFF(diff, eStretchChanged, other, stretchEffect); + CHECK_DIFF(diff, eEdgeExtensionChanged, other, edgeExtensionParameters); CHECK_DIFF(diff, eBufferCropChanged, other, bufferCrop); CHECK_DIFF(diff, eDestinationFrameChanged, other, destinationFrame); if (other.what & eProducerDisconnect) diff |= eProducerDisconnect; -- cgit v1.2.3-59-g8ed1b From ac70bc579028cdc00a2f14b77718080b26b93c7d Mon Sep 17 00:00:00 2001 From: Patrick Williams Date: Tue, 9 Jul 2024 17:11:28 -0500 Subject: Optimize BLAST buffer releases via Unix sockets Bug: 294133380 Flag: com.android.graphics.libgui.flags.buffer_release_channel Test: BLASTBufferQueueTest Change-Id: Ia183452198dadc7f8e540f7219bd44d8b5823458 --- libs/gui/Android.bp | 1 + libs/gui/BLASTBufferQueue.cpp | 326 ++++++++++++++++-- libs/gui/BufferReleaseChannel.cpp | 364 +++++++++++++++++++++ libs/gui/LayerState.cpp | 17 + libs/gui/SurfaceComposerClient.cpp | 16 + libs/gui/include/gui/BLASTBufferQueue.h | 56 +++- libs/gui/include/gui/BufferReleaseChannel.h | 127 +++++++ libs/gui/include/gui/LayerState.h | 4 + libs/gui/include/gui/SurfaceComposerClient.h | 5 + libs/gui/libgui_flags.aconfig | 8 + libs/gui/tests/Android.bp | 1 + libs/gui/tests/BufferReleaseChannel_test.cpp | 121 +++++++ services/surfaceflinger/Layer.cpp | 15 +- services/surfaceflinger/Layer.h | 3 + services/surfaceflinger/SurfaceFlinger.cpp | 4 + .../surfaceflinger/TransactionCallbackInvoker.cpp | 11 + .../surfaceflinger/TransactionCallbackInvoker.h | 9 + 17 files changed, 1062 insertions(+), 26 deletions(-) create mode 100644 libs/gui/BufferReleaseChannel.cpp create mode 100644 libs/gui/include/gui/BufferReleaseChannel.h create mode 100644 libs/gui/tests/BufferReleaseChannel_test.cpp (limited to 'libs/gui/LayerState.cpp') diff --git a/libs/gui/Android.bp b/libs/gui/Android.bp index 51d2e5305a..1243b214d3 100644 --- a/libs/gui/Android.bp +++ b/libs/gui/Android.bp @@ -255,6 +255,7 @@ filegroup { "BitTube.cpp", "BLASTBufferQueue.cpp", "BufferItemConsumer.cpp", + "BufferReleaseChannel.cpp", "Choreographer.cpp", "CompositorTiming.cpp", "ConsumerBase.cpp", diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp index 739c3c2a41..f13d499b50 100644 --- a/libs/gui/BLASTBufferQueue.cpp +++ b/libs/gui/BLASTBufferQueue.cpp @@ -38,13 +38,17 @@ #include #include +#include #include +#include +#include #include #include using namespace com::android::graphics::libgui; using namespace std::chrono_literals; +using android::base::unique_fd; namespace { inline const char* boolToString(bool b) { @@ -179,8 +183,6 @@ BLASTBufferQueue::BLASTBufferQueue(const std::string& name, bool updateDestinati // explicitly so that dequeueBuffer will block mProducer->setDequeueTimeout(std::numeric_limits::max()); - // safe default, most producers are expected to override this - mProducer->setMaxDequeuedBufferCount(2); mBufferItemConsumer = new BLASTBufferItemConsumer(mConsumer, GraphicBuffer::USAGE_HW_COMPOSER | GraphicBuffer::USAGE_HW_TEXTURE, @@ -210,6 +212,12 @@ BLASTBufferQueue::BLASTBufferQueue(const std::string& name, bool updateDestinati }, this); +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL) + std::unique_ptr bufferReleaseConsumer; + gui::BufferReleaseChannel::open(mName, bufferReleaseConsumer, mBufferReleaseProducer); + mBufferReleaseReader.emplace(std::move(bufferReleaseConsumer)); +#endif + BQA_LOGV("BLASTBufferQueue created"); } @@ -259,6 +267,9 @@ void BLASTBufferQueue::update(const sp& surface, uint32_t width, if (surfaceControlChanged) { t.setFlags(mSurfaceControl, layer_state_t::eEnableBackpressure, layer_state_t::eEnableBackpressure); + if (mBufferReleaseProducer) { + t.setBufferReleaseChannel(mSurfaceControl, mBufferReleaseProducer); + } applyTransaction = true; } mTransformHint = mSurfaceControl->getTransformHint(); @@ -439,6 +450,21 @@ void BLASTBufferQueue::releaseBufferCallback( BBQ_TRACE(); releaseBufferCallbackLocked(id, releaseFence, currentMaxAcquiredBufferCount, false /* fakeRelease */); + if (!mBufferReleaseReader) { + return; + } + // Drain the buffer release channel socket + while (true) { + ReleaseCallbackId releaseCallbackId; + sp releaseFence; + if (status_t status = + mBufferReleaseReader->readNonBlocking(releaseCallbackId, releaseFence); + status != OK) { + break; + } + releaseBufferCallbackLocked(releaseCallbackId, releaseFence, std::nullopt, + false /* fakeRelease */); + } } void BLASTBufferQueue::releaseBufferCallbackLocked( @@ -495,11 +521,11 @@ void BLASTBufferQueue::releaseBuffer(const ReleaseCallbackId& callbackId, const sp& releaseFence) { auto it = mSubmitted.find(callbackId); if (it == mSubmitted.end()) { - BQA_LOGE("ERROR: releaseBufferCallback without corresponding submitted buffer %s", - callbackId.to_string().c_str()); return; } mNumAcquired--; + updateDequeueShouldBlockLocked(); + mBufferReleaseReader->interruptBlockingRead(); BBQ_TRACE("frame=%" PRIu64, callbackId.framenumber); BQA_LOGV("released %s", callbackId.to_string().c_str()); mBufferItemConsumer->releaseBuffer(it->second, releaseFence); @@ -564,6 +590,7 @@ status_t BLASTBufferQueue::acquireNextBufferLocked( auto buffer = bufferItem.mGraphicBuffer; mNumFrameAvailable--; + updateDequeueShouldBlockLocked(); BBQ_TRACE("frame=%" PRIu64, bufferItem.mFrameNumber); if (buffer == nullptr) { @@ -582,6 +609,7 @@ status_t BLASTBufferQueue::acquireNextBufferLocked( } mNumAcquired++; + updateDequeueShouldBlockLocked(); mLastAcquiredFrameNumber = bufferItem.mFrameNumber; ReleaseCallbackId releaseCallbackId(buffer->getId(), mLastAcquiredFrameNumber); mSubmitted[releaseCallbackId] = bufferItem; @@ -708,6 +736,7 @@ void BLASTBufferQueue::acquireAndReleaseBuffer() { return; } mNumFrameAvailable--; + updateDequeueShouldBlockLocked(); mBufferItemConsumer->releaseBuffer(bufferItem, bufferItem.mFence); } @@ -761,7 +790,9 @@ void BLASTBufferQueue::onFrameAvailable(const BufferItem& item) { } // add to shadow queue + mNumDequeued--; mNumFrameAvailable++; + updateDequeueShouldBlockLocked(); if (waitForTransactionCallback && mNumFrameAvailable >= 2) { acquireAndReleaseBuffer(); } @@ -812,11 +843,21 @@ void BLASTBufferQueue::onFrameReplaced(const BufferItem& item) { void BLASTBufferQueue::onFrameDequeued(const uint64_t bufferId) { std::lock_guard _lock{mTimestampMutex}; mDequeueTimestamps[bufferId] = systemTime(); -}; + mNumDequeued++; +} void BLASTBufferQueue::onFrameCancelled(const uint64_t bufferId) { - std::lock_guard _lock{mTimestampMutex}; - mDequeueTimestamps.erase(bufferId); + { + std::lock_guard _lock{mTimestampMutex}; + mDequeueTimestamps.erase(bufferId); + } + + { + std::lock_guard lock{mMutex}; + mNumDequeued--; + updateDequeueShouldBlockLocked(); + } + mBufferReleaseReader->interruptBlockingRead(); }; bool BLASTBufferQueue::syncNextTransaction( @@ -888,6 +929,22 @@ bool BLASTBufferQueue::rejectBuffer(const BufferItem& item) { return mSize != bufferSize; } +void BLASTBufferQueue::updateDequeueShouldBlockLocked() { + int32_t buffersInUse = mNumDequeued + mNumFrameAvailable + mNumAcquired; + int32_t maxBufferCount = std::min(mMaxAcquiredBuffers + mMaxDequeuedBuffers, kMaxBufferCount); + bool bufferAvailable = buffersInUse < maxBufferCount; + // BLASTBufferQueueProducer should block until a buffer is released if + // (1) There are no free buffers available. + // (2) We're not in async mode. In async mode, BufferQueueProducer::dequeueBuffer returns + // WOULD_BLOCK instead of blocking when there are no free buffers. + // (3) We're not in shared buffer mode. In shared buffer mode, both the producer and consumer + // can access the same buffer simultaneously. BufferQueueProducer::dequeueBuffer returns + // the shared buffer immediately instead of blocking. + mDequeueShouldBlock = !(bufferAvailable || mAsyncMode || mSharedBufferMode); + ATRACE_INT("Dequeued", mNumDequeued); + ATRACE_INT("DequeueShouldBlock", mDequeueShouldBlock); +} + class BBQSurface : public Surface { private: std::mutex mMutex; @@ -1116,24 +1173,58 @@ public: producerControlledByApp, output); } + status_t disconnect(int api, DisconnectMode mode) override { + if (status_t status = BufferQueueProducer::disconnect(api, mode); status != OK) { + return status; + } + + sp bbq = mBLASTBufferQueue.promote(); + if (!bbq) { + return OK; + } + + { + std::lock_guard lock{bbq->mMutex}; + bbq->mNumDequeued = 0; + bbq->mNumFrameAvailable = 0; + bbq->mNumAcquired = 0; + bbq->mSubmitted.clear(); + bbq->updateDequeueShouldBlockLocked(); + } + bbq->mBufferReleaseReader->interruptBlockingRead(); + + return OK; + } + // We want to resize the frame history when changing the size of the buffer queue status_t setMaxDequeuedBufferCount(int maxDequeuedBufferCount) override { int maxBufferCount; status_t status = BufferQueueProducer::setMaxDequeuedBufferCount(maxDequeuedBufferCount, &maxBufferCount); - // if we can't determine the max buffer count, then just skip growing the history size - if (status == OK) { - size_t newFrameHistorySize = maxBufferCount + 2; // +2 because triple buffer rendering - // optimize away resizing the frame history unless it will grow - if (newFrameHistorySize > FrameEventHistory::INITIAL_MAX_FRAME_HISTORY) { - sp bbq = mBLASTBufferQueue.promote(); - if (bbq != nullptr) { - ALOGV("increasing frame history size to %zu", newFrameHistorySize); - bbq->resizeFrameEventHistory(newFrameHistorySize); - } - } + if (status != OK) { + return status; } - return status; + + sp bbq = mBLASTBufferQueue.promote(); + if (!bbq) { + return OK; + } + + { + std::lock_guard lock{bbq->mMutex}; + bbq->mMaxDequeuedBuffers = maxDequeuedBufferCount; + bbq->updateDequeueShouldBlockLocked(); + } + bbq->mBufferReleaseReader->interruptBlockingRead(); + + size_t newFrameHistorySize = maxBufferCount + 2; // +2 because triple buffer rendering + // optimize away resizing the frame history unless it will grow + if (newFrameHistorySize > FrameEventHistory::INITIAL_MAX_FRAME_HISTORY) { + ALOGV("increasing frame history size to %zu", newFrameHistorySize); + bbq->resizeFrameEventHistory(newFrameHistorySize); + } + + return OK; } int query(int what, int* value) override { @@ -1144,6 +1235,125 @@ public: return BufferQueueProducer::query(what, value); } + status_t setAsyncMode(bool asyncMode) override { + if (status_t status = BufferQueueProducer::setAsyncMode(asyncMode); status != NO_ERROR) { + return status; + } + + sp bbq = mBLASTBufferQueue.promote(); + if (!bbq) { + return NO_ERROR; + } + + { + std::lock_guard lock{bbq->mMutex}; + bbq->mAsyncMode = asyncMode; + bbq->updateDequeueShouldBlockLocked(); + } + + bbq->mBufferReleaseReader->interruptBlockingRead(); + return NO_ERROR; + } + + status_t setSharedBufferMode(bool sharedBufferMode) override { + if (status_t status = BufferQueueProducer::setSharedBufferMode(sharedBufferMode); + status != NO_ERROR) { + return status; + } + + sp bbq = mBLASTBufferQueue.promote(); + if (!bbq) { + return NO_ERROR; + } + + { + std::lock_guard lock{bbq->mMutex}; + bbq->mSharedBufferMode = sharedBufferMode; + bbq->updateDequeueShouldBlockLocked(); + } + + bbq->mBufferReleaseReader->interruptBlockingRead(); + return NO_ERROR; + } + + status_t detachBuffer(int slot) override { + if (status_t status = BufferQueueProducer::detachBuffer(slot); status != NO_ERROR) { + return status; + } + + sp bbq = mBLASTBufferQueue.promote(); + if (!bbq) { + return NO_ERROR; + } + + { + std::lock_guard lock{bbq->mMutex}; + bbq->mNumDequeued--; + bbq->updateDequeueShouldBlockLocked(); + } + + bbq->mBufferReleaseReader->interruptBlockingRead(); + return NO_ERROR; + } + + // Override dequeueBuffer to block if there are no free buffers. + // + // Buffer releases are communicated via the BufferReleaseChannel. When dequeueBuffer determines + // a free buffer is not available, it blocks on an epoll file descriptor. Epoll is configured to + // detect messages on the BufferReleaseChannel's socket and an eventfd. The eventfd is signaled + // whenever an event other than a buffer release occurs that may change the number of free + // buffers. dequeueBuffer uses epoll in a similar manner as a condition variable by testing for + // the availability of a free buffer in a loop, breaking the loop once a free buffer is + // available. + // + // This is an optimization implemented to reduce thread scheduling delays in the previously + // existing binder release callback. The binder buffer release callback is still used and there + // are no guarantees around order between buffer releases via binder and the + // BufferReleaseChannel. If we attempt to a release a buffer here that has already been released + // via binder, the release is ignored. + status_t dequeueBuffer(int* outSlot, sp* outFence, uint32_t width, uint32_t height, + PixelFormat format, uint64_t usage, uint64_t* outBufferAge, + FrameEventHistoryDelta* outTimestamps) { + sp bbq = mBLASTBufferQueue.promote(); + if (!bbq || !bbq->mBufferReleaseReader) { + return BufferQueueProducer::dequeueBuffer(outSlot, outFence, width, height, format, + usage, outBufferAge, outTimestamps); + } + + if (bbq->mDequeueShouldBlock) { + ATRACE_FORMAT("waiting for free buffer"); + auto maxWaitTime = std::chrono::steady_clock::now() + 1s; + do { + auto timeout = std::chrono::duration_cast( + maxWaitTime - std::chrono::steady_clock::now()); + if (timeout <= 0ms) { + break; + } + + ReleaseCallbackId releaseCallbackId; + sp releaseFence; + status_t status = bbq->mBufferReleaseReader->readBlocking(releaseCallbackId, + releaseFence, timeout); + if (status == WOULD_BLOCK) { + // readBlocking was interrupted. The loop will test if we have a free buffer. + continue; + } + + if (status != OK) { + // An error occurred or readBlocking timed out. + break; + } + + std::lock_guard lock{bbq->mMutex}; + bbq->releaseBufferCallbackLocked(releaseCallbackId, releaseFence, std::nullopt, + false); + } while (bbq->mDequeueShouldBlock); + } + + return BufferQueueProducer::dequeueBuffer(outSlot, outFence, width, height, format, usage, + outBufferAge, outTimestamps); + } + private: const wp mBLASTBufferQueue; }; @@ -1173,6 +1383,16 @@ void BLASTBufferQueue::createBufferQueue(sp* outProducer *outConsumer = consumer; } +void BLASTBufferQueue::onFirstRef() { + // safe default, most producers are expected to override this + // + // This is done in onFirstRef instead of BLASTBufferQueue's constructor because + // BBQBufferQueueProducer::setMaxDequeuedBufferCount promotes a weak pointer to BLASTBufferQueue + // to a strong pointer. If this is done in the constructor, then when the strong pointer goes + // out of scope, it's the last reference so BLASTBufferQueue is deleted. + mProducer->setMaxDequeuedBufferCount(2); +} + void BLASTBufferQueue::resizeFrameEventHistory(size_t newSize) { // This can be null during creation of the buffer queue, but resizing won't do anything at that // point in time, so just ignore. This can go away once the class relationships and lifetimes of @@ -1222,4 +1442,72 @@ void BLASTBufferQueue::setTransactionHangCallback( mTransactionHangCallback = callback; } +BLASTBufferQueue::BufferReleaseReader::BufferReleaseReader( + std::unique_ptr endpoint) + : mEndpoint(std::move(endpoint)) { + mEpollFd = android::base::unique_fd(epoll_create1(0)); + if (!mEpollFd.ok()) { + ALOGE("Failed to create buffer release epoll file descriptor. errno=%d message='%s'", errno, + strerror(errno)); + } + + epoll_event event; + event.events = EPOLLIN; + event.data.fd = mEndpoint->getFd(); + if (epoll_ctl(mEpollFd.get(), EPOLL_CTL_ADD, mEndpoint->getFd(), &event) == -1) { + ALOGE("Failed to register buffer release consumer file descriptor with epoll. errno=%d " + "message='%s'", + errno, strerror(errno)); + } + + mEventFd = android::base::unique_fd(eventfd(0, EFD_NONBLOCK)); + event.data.fd = mEventFd.get(); + if (epoll_ctl(mEpollFd.get(), EPOLL_CTL_ADD, mEventFd.get(), &event) == -1) { + ALOGE("Failed to register buffer release eventfd with epoll. errno=%d message='%s'", errno, + strerror(errno)); + } +} + +status_t BLASTBufferQueue::BufferReleaseReader::readNonBlocking(ReleaseCallbackId& outId, + sp& outFence) { + std::lock_guard lock{mMutex}; + return mEndpoint->readReleaseFence(outId, outFence); +} + +status_t BLASTBufferQueue::BufferReleaseReader::readBlocking(ReleaseCallbackId& outId, + sp& outFence, + std::chrono::milliseconds timeout) { + epoll_event event; + int eventCount = epoll_wait(mEpollFd.get(), &event, 1 /* maxevents */, timeout.count()); + + if (eventCount == -1) { + ALOGE("epoll_wait error while waiting for buffer release. errno=%d message='%s'", errno, + strerror(errno)); + return UNKNOWN_ERROR; + } + + if (eventCount == 0) { + return TIMED_OUT; + } + + if (event.data.fd == mEventFd.get()) { + uint64_t value; + if (read(mEventFd.get(), &value, sizeof(uint64_t)) == -1 && errno != EWOULDBLOCK) { + ALOGE("error while reading from eventfd. errno=%d message='%s'", errno, + strerror(errno)); + } + return WOULD_BLOCK; + } + + std::lock_guard lock{mMutex}; + return mEndpoint->readReleaseFence(outId, outFence); +} + +void BLASTBufferQueue::BufferReleaseReader::interruptBlockingRead() { + uint64_t value = 1; + if (write(mEventFd.get(), &value, sizeof(uint64_t)) == -1) { + ALOGE("failed to notify dequeue event. errno=%d message='%s'", errno, strerror(errno)); + } +} + } // namespace android diff --git a/libs/gui/BufferReleaseChannel.cpp b/libs/gui/BufferReleaseChannel.cpp new file mode 100644 index 0000000000..183b25a4cb --- /dev/null +++ b/libs/gui/BufferReleaseChannel.cpp @@ -0,0 +1,364 @@ +/* + * Copyright (C) 2024 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#define LOG_TAG "BufferReleaseChannel" +#define ATRACE_TAG ATRACE_TAG_GRAPHICS + +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include + +#include + +using android::base::Result; + +namespace android::gui { + +namespace { + +template +static void readAligned(const void*& buffer, size_t& size, T& value) { + size -= FlattenableUtils::align(buffer); + FlattenableUtils::read(buffer, size, value); +} + +template +static void writeAligned(void*& buffer, size_t& size, T value) { + size -= FlattenableUtils::align(buffer); + FlattenableUtils::write(buffer, size, value); +} + +template +static void addAligned(size_t& size, T /* value */) { + size = FlattenableUtils::align(size); + size += sizeof(T); +} + +template +static inline constexpr uint32_t low32(const T n) { + return static_cast(static_cast(n)); +} + +template +static inline constexpr uint32_t high32(const T n) { + return static_cast(static_cast(n) >> 32); +} + +template +static inline constexpr T to64(const uint32_t lo, const uint32_t hi) { + return static_cast(static_cast(hi) << 32 | lo); +} + +} // namespace + +size_t BufferReleaseChannel::Message::getPodSize() const { + size_t size = 0; + addAligned(size, low32(releaseCallbackId.bufferId)); + addAligned(size, high32(releaseCallbackId.bufferId)); + addAligned(size, low32(releaseCallbackId.framenumber)); + addAligned(size, high32(releaseCallbackId.framenumber)); + return size; +} + +size_t BufferReleaseChannel::Message::getFlattenedSize() const { + size_t size = releaseFence->getFlattenedSize(); + size = FlattenableUtils::align<4>(size); + size += getPodSize(); + return size; +} + +status_t BufferReleaseChannel::Message::flatten(void*& buffer, size_t& size, int*& fds, + size_t& count) const { + if (status_t err = releaseFence->flatten(buffer, size, fds, count); err != OK) { + return err; + } + size -= FlattenableUtils::align<4>(buffer); + + // Check we still have enough space + if (size < getPodSize()) { + return NO_MEMORY; + } + + writeAligned(buffer, size, low32(releaseCallbackId.bufferId)); + writeAligned(buffer, size, high32(releaseCallbackId.bufferId)); + writeAligned(buffer, size, low32(releaseCallbackId.framenumber)); + writeAligned(buffer, size, high32(releaseCallbackId.framenumber)); + return OK; +} + +status_t BufferReleaseChannel::Message::unflatten(void const*& buffer, size_t& size, + int const*& fds, size_t& count) { + releaseFence = new Fence(); + if (status_t err = releaseFence->unflatten(buffer, size, fds, count); err != OK) { + return err; + } + size -= FlattenableUtils::align<4>(buffer); + + // Check we still have enough space + if (size < getPodSize()) { + return OK; + } + + uint32_t bufferIdLo = 0, bufferIdHi = 0; + uint32_t frameNumberLo = 0, frameNumberHi = 0; + + readAligned(buffer, size, bufferIdLo); + readAligned(buffer, size, bufferIdHi); + releaseCallbackId.bufferId = to64(bufferIdLo, bufferIdHi); + readAligned(buffer, size, frameNumberLo); + readAligned(buffer, size, frameNumberHi); + releaseCallbackId.framenumber = to64(frameNumberLo, frameNumberHi); + + return NO_ERROR; +} + +status_t BufferReleaseChannel::ConsumerEndpoint::readReleaseFence( + ReleaseCallbackId& outReleaseCallbackId, sp& outReleaseFence) { + Message releaseBufferMessage; + mFlattenedBuffer.resize(releaseBufferMessage.getFlattenedSize()); + std::array controlMessageBuffer; + + iovec iov{ + .iov_base = mFlattenedBuffer.data(), + .iov_len = mFlattenedBuffer.size(), + }; + + msghdr msg{ + .msg_iov = &iov, + .msg_iovlen = 1, + .msg_control = controlMessageBuffer.data(), + .msg_controllen = controlMessageBuffer.size(), + }; + + int result; + do { + result = recvmsg(mFd, &msg, 0); + } while (result == -1 && errno == EINTR); + if (result == -1) { + if (errno == EWOULDBLOCK || errno == EAGAIN) { + return WOULD_BLOCK; + } + ALOGE("Error reading release fence from socket: error %#x (%s)", errno, strerror(errno)); + return -errno; + } + + if (msg.msg_iovlen != 1) { + ALOGE("Error reading release fence from socket: bad data length"); + return UNKNOWN_ERROR; + } + + if (msg.msg_controllen % sizeof(int) != 0) { + ALOGE("Error reading release fence from socket: bad fd length"); + return UNKNOWN_ERROR; + } + + size_t dataLen = msg.msg_iov->iov_len; + const void* data = static_cast(msg.msg_iov->iov_base); + if (!data) { + ALOGE("Error reading release fence from socket: no buffer data"); + return UNKNOWN_ERROR; + } + + size_t fdCount = 0; + const int* fdData = nullptr; + if (cmsghdr* cmsg = CMSG_FIRSTHDR(&msg)) { + fdData = reinterpret_cast(CMSG_DATA(cmsg)); + fdCount = (cmsg->cmsg_len - CMSG_LEN(0)) / sizeof(int); + } + + if (status_t err = releaseBufferMessage.unflatten(data, dataLen, fdData, fdCount); err != OK) { + return err; + } + + outReleaseCallbackId = releaseBufferMessage.releaseCallbackId; + outReleaseFence = std::move(releaseBufferMessage.releaseFence); + + return OK; +} + +int BufferReleaseChannel::ProducerEndpoint::writeReleaseFence(const ReleaseCallbackId& callbackId, + const sp& fence) { + Message releaseBufferMessage(callbackId, fence ? fence : Fence::NO_FENCE); + mFlattenedBuffer.resize(releaseBufferMessage.getFlattenedSize()); + int flattenedFd; + { + // Make copies of needed items since flatten modifies them, and we don't + // want to send anything if there's an error during flatten. + void* flattenedBufferPtr = mFlattenedBuffer.data(); + size_t flattenedBufferSize = mFlattenedBuffer.size(); + int* flattenedFdPtr = &flattenedFd; + size_t flattenedFdCount = 1; + if (status_t err = releaseBufferMessage.flatten(flattenedBufferPtr, flattenedBufferSize, + flattenedFdPtr, flattenedFdCount); + err != OK) { + ALOGE("Failed to flatten BufferReleaseChannel message."); + return err; + } + } + + iovec iov{ + .iov_base = mFlattenedBuffer.data(), + .iov_len = mFlattenedBuffer.size(), + }; + + msghdr msg{ + .msg_iov = &iov, + .msg_iovlen = 1, + }; + + std::array controlMessageBuffer; + if (fence && fence->isValid()) { + msg.msg_control = controlMessageBuffer.data(); + msg.msg_controllen = controlMessageBuffer.size(); + + cmsghdr* cmsg = CMSG_FIRSTHDR(&msg); + cmsg->cmsg_level = SOL_SOCKET; + cmsg->cmsg_type = SCM_RIGHTS; + cmsg->cmsg_len = CMSG_LEN(sizeof(int)); + memcpy(CMSG_DATA(cmsg), &flattenedFd, sizeof(int)); + } + + int result; + do { + result = sendmsg(mFd, &msg, 0); + } while (result == -1 && errno == EINTR); + if (result == -1) { + ALOGD("Error writing release fence to socket: error %#x (%s)", errno, strerror(errno)); + return -errno; + } + + return OK; +} + +status_t BufferReleaseChannel::ProducerEndpoint::readFromParcel(const android::Parcel* parcel) { + if (!parcel) return STATUS_BAD_VALUE; + SAFE_PARCEL(parcel->readUtf8FromUtf16, &mName); + SAFE_PARCEL(parcel->readUniqueFileDescriptor, &mFd); + return STATUS_OK; +} + +status_t BufferReleaseChannel::ProducerEndpoint::writeToParcel(android::Parcel* parcel) const { + if (!parcel) return STATUS_BAD_VALUE; + SAFE_PARCEL(parcel->writeUtf8AsUtf16, mName); + SAFE_PARCEL(parcel->writeUniqueFileDescriptor, mFd); + return STATUS_OK; +} + +status_t BufferReleaseChannel::open(std::string name, + std::unique_ptr& outConsumer, + std::shared_ptr& outProducer) { + outConsumer.reset(); + outProducer.reset(); + + int sockets[2]; + if (socketpair(AF_UNIX, SOCK_SEQPACKET, 0, sockets)) { + ALOGE("[%s] Failed to create socket pair. errorno=%d message='%s'", name.c_str(), errno, + strerror(errno)); + return -errno; + } + + android::base::unique_fd consumerFd(sockets[0]); + android::base::unique_fd producerFd(sockets[1]); + + // Socket buffer size. The default is typically about 128KB, which is much larger than + // we really need. So we make it smaller. It just needs to be big enough to hold + // a few dozen release fences. + const int bufferSize = 32 * 1024; + if (setsockopt(consumerFd.get(), SOL_SOCKET, SO_SNDBUF, &bufferSize, sizeof(bufferSize)) == + -1) { + ALOGE("[%s] Failed to set consumer socket send buffer size. errno=%d message='%s'", + name.c_str(), errno, strerror(errno)); + return -errno; + } + if (setsockopt(consumerFd.get(), SOL_SOCKET, SO_RCVBUF, &bufferSize, sizeof(bufferSize)) == + -1) { + ALOGE("[%s] Failed to set consumer socket receive buffer size. errno=%d " + "message='%s'", + name.c_str(), errno, strerror(errno)); + return -errno; + } + if (setsockopt(producerFd.get(), SOL_SOCKET, SO_SNDBUF, &bufferSize, sizeof(bufferSize)) == + -1) { + ALOGE("[%s] Failed to set producer socket send buffer size. errno=%d message='%s'", + name.c_str(), errno, strerror(errno)); + return -errno; + } + if (setsockopt(producerFd.get(), SOL_SOCKET, SO_RCVBUF, &bufferSize, sizeof(bufferSize)) == + -1) { + ALOGE("[%s] Failed to set producer socket receive buffer size. errno=%d " + "message='%s'", + name.c_str(), errno, strerror(errno)); + return -errno; + } + + // Configure the consumer socket to be non-blocking. + int flags = fcntl(consumerFd.get(), F_GETFL, 0); + if (flags == -1) { + ALOGE("[%s] Failed to get consumer socket flags. errno=%d message='%s'", name.c_str(), + errno, strerror(errno)); + return -errno; + } + if (fcntl(consumerFd.get(), F_SETFL, flags | O_NONBLOCK) == -1) { + ALOGE("[%s] Failed to set consumer socket to non-blocking mode. errno=%d " + "message='%s'", + name.c_str(), errno, strerror(errno)); + return -errno; + } + + // Configure a timeout for the producer socket. + const timeval timeout{.tv_sec = 1, .tv_usec = 0}; + if (setsockopt(producerFd.get(), SOL_SOCKET, SO_RCVTIMEO, &timeout, sizeof(timeval)) == -1) { + ALOGE("[%s] Failed to set producer socket timeout. errno=%d message='%s'", name.c_str(), + errno, strerror(errno)); + return -errno; + } + + // Make the consumer read-only + if (shutdown(consumerFd.get(), SHUT_WR) == -1) { + ALOGE("[%s] Failed to shutdown writing on consumer socket. errno=%d message='%s'", + name.c_str(), errno, strerror(errno)); + return -errno; + } + + // Make the producer write-only + if (shutdown(producerFd.get(), SHUT_RD) == -1) { + ALOGE("[%s] Failed to shutdown reading on producer socket. errno=%d message='%s'", + name.c_str(), errno, strerror(errno)); + return -errno; + } + + outConsumer = std::make_unique(name, std::move(consumerFd)); + outProducer = std::make_shared(std::move(name), std::move(producerFd)); + return STATUS_OK; +} + +} // namespace android::gui diff --git a/libs/gui/LayerState.cpp b/libs/gui/LayerState.cpp index 307ae3990e..adf1646b83 100644 --- a/libs/gui/LayerState.cpp +++ b/libs/gui/LayerState.cpp @@ -194,6 +194,12 @@ status_t layer_state_t::write(Parcel& output) const SAFE_PARCEL(output.writeFloat, currentHdrSdrRatio); SAFE_PARCEL(output.writeFloat, desiredHdrSdrRatio); SAFE_PARCEL(output.writeInt32, static_cast(cachingHint)); + + const bool hasBufferReleaseChannel = (bufferReleaseChannel != nullptr); + SAFE_PARCEL(output.writeBool, hasBufferReleaseChannel); + if (hasBufferReleaseChannel) { + SAFE_PARCEL(output.writeParcelable, *bufferReleaseChannel); + } return NO_ERROR; } @@ -339,6 +345,12 @@ status_t layer_state_t::read(const Parcel& input) SAFE_PARCEL(input.readInt32, &tmpInt32); cachingHint = static_cast(tmpInt32); + bool hasBufferReleaseChannel; + SAFE_PARCEL(input.readBool, &hasBufferReleaseChannel); + if (hasBufferReleaseChannel) { + bufferReleaseChannel = std::make_shared(); + SAFE_PARCEL(input.readParcelable, bufferReleaseChannel.get()); + } return NO_ERROR; } @@ -718,6 +730,10 @@ void layer_state_t::merge(const layer_state_t& other) { if (other.what & eFlushJankData) { what |= eFlushJankData; } + if (other.what & eBufferReleaseChannelChanged) { + what |= eBufferReleaseChannelChanged; + bufferReleaseChannel = other.bufferReleaseChannel; + } if ((other.what & what) != other.what) { ALOGE("Unmerged SurfaceComposer Transaction properties. LayerState::merge needs updating? " "other.what=0x%" PRIX64 " what=0x%" PRIX64 " unmerged flags=0x%" PRIX64, @@ -797,6 +813,7 @@ uint64_t layer_state_t::diff(const layer_state_t& other) const { CHECK_DIFF(diff, eColorChanged, other, color.rgb); CHECK_DIFF(diff, eColorSpaceAgnosticChanged, other, colorSpaceAgnostic); CHECK_DIFF(diff, eDimmingEnabledChanged, other, dimmingEnabled); + if (other.what & eBufferReleaseChannelChanged) diff |= eBufferReleaseChannelChanged; return diff; } diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp index 2821b510c3..5c87772587 100644 --- a/libs/gui/SurfaceComposerClient.cpp +++ b/libs/gui/SurfaceComposerClient.cpp @@ -2394,6 +2394,22 @@ SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setDropI return *this; } +SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setBufferReleaseChannel( + const sp& sc, + const std::shared_ptr& channel) { + layer_state_t* s = getLayerState(sc); + if (!s) { + mStatus = BAD_INDEX; + return *this; + } + + s->what |= layer_state_t::eBufferReleaseChannelChanged; + s->bufferReleaseChannel = channel; + + registerSurfaceControlForCallback(sc); + return *this; +} + // --------------------------------------------------------------------------- DisplayState& SurfaceComposerClient::Transaction::getDisplayState(const sp& token) { diff --git a/libs/gui/include/gui/BLASTBufferQueue.h b/libs/gui/include/gui/BLASTBufferQueue.h index 0e1a505c69..e9a350c301 100644 --- a/libs/gui/include/gui/BLASTBufferQueue.h +++ b/libs/gui/include/gui/BLASTBufferQueue.h @@ -19,7 +19,7 @@ #include #include - +#include #include #include @@ -28,7 +28,6 @@ #include #include -#include #include #include @@ -131,6 +130,8 @@ public: virtual ~BLASTBufferQueue(); + void onFirstRef() override; + private: friend class BLASTBufferQueueHelper; friend class BBQBufferQueueProducer; @@ -170,11 +171,23 @@ private: // BufferQueue internally allows 1 more than // the max to be acquired - int32_t mMaxAcquiredBuffers = 1; - + int32_t mMaxAcquiredBuffers GUARDED_BY(mMutex) = 1; + int32_t mMaxDequeuedBuffers GUARDED_BY(mMutex) = 1; + static constexpr int32_t kMaxBufferCount = BufferQueueDefs::NUM_BUFFER_SLOTS; + + // mNumDequeued is an atomic instead of being guarded by mMutex so that it can be incremented in + // onFrameDequeued while avoiding lock contention. updateDequeueShouldBlockLocked is not called + // after mNumDequeued is incremented for this reason. This means mDequeueShouldBlock may be + // temporarily false when it should be true. This can happen if multiple threads are dequeuing + // buffers or if dequeueBuffers is called multiple times in a row without queuing a buffer in + // between. As mDequeueShouldBlock is only used for optimization, this is OK. + std::atomic_int mNumDequeued; int32_t mNumFrameAvailable GUARDED_BY(mMutex) = 0; int32_t mNumAcquired GUARDED_BY(mMutex) = 0; + bool mAsyncMode GUARDED_BY(mMutex) = false; + bool mSharedBufferMode GUARDED_BY(mMutex) = false; + // A value used to identify if a producer has been changed for the same SurfaceControl. // This is needed to know when the frame number has been reset to make sure we don't // latch stale buffers and that we don't wait on barriers from an old producer. @@ -300,6 +313,41 @@ private: std::function mTransactionHangCallback; std::unordered_set mSyncedFrameNumbers GUARDED_BY(mMutex); + + class BufferReleaseReader { + public: + BufferReleaseReader(std::unique_ptr); + + BufferReleaseReader(const BufferReleaseReader&) = delete; + BufferReleaseReader& operator=(const BufferReleaseReader&) = delete; + + // Block until we can release a buffer. + // + // Returns: + // * OK if a ReleaseCallbackId and Fence were successfully read. + // * TIMED_OUT if the specified timeout was reached. + // * WOULD_BLOCK if the blocking read was interrupted by interruptBlockingRead. + // * UNKNOWN_ERROR if something went wrong. + status_t readBlocking(ReleaseCallbackId&, sp&, std::chrono::milliseconds timeout); + + status_t readNonBlocking(ReleaseCallbackId&, sp&); + + void interruptBlockingRead(); + + private: + std::mutex mMutex; + std::unique_ptr mEndpoint GUARDED_BY(mMutex); + android::base::unique_fd mEpollFd; + android::base::unique_fd mEventFd; + }; + + // BufferReleaseChannel is used to communicate buffer releases from SurfaceFlinger to + // the client. See BBQBufferQueueProducer::dequeueBuffer for details. + std::optional mBufferReleaseReader; + std::shared_ptr mBufferReleaseProducer; + + std::atomic_bool mDequeueShouldBlock{false}; + void updateDequeueShouldBlockLocked() REQUIRES(mMutex); }; } // namespace android diff --git a/libs/gui/include/gui/BufferReleaseChannel.h b/libs/gui/include/gui/BufferReleaseChannel.h new file mode 100644 index 0000000000..cb0b261240 --- /dev/null +++ b/libs/gui/include/gui/BufferReleaseChannel.h @@ -0,0 +1,127 @@ +/* + * Copyright (C) 2024 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include + +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +namespace android::gui { + +/** + * IPC wrapper to pass release fences from SurfaceFlinger to apps via a local unix domain socket. + */ +class BufferReleaseChannel { +private: + class Endpoint { + public: + Endpoint(std::string name, android::base::unique_fd fd) + : mName(std::move(name)), mFd(std::move(fd)) {} + Endpoint() {} + + Endpoint(Endpoint&&) noexcept = default; + Endpoint& operator=(Endpoint&&) noexcept = default; + + Endpoint(const Endpoint&) = delete; + void operator=(const Endpoint&) = delete; + + const android::base::unique_fd& getFd() const { return mFd; } + + protected: + std::string mName; + android::base::unique_fd mFd; + }; + +public: + class ConsumerEndpoint : public Endpoint { + public: + ConsumerEndpoint(std::string name, android::base::unique_fd fd) + : Endpoint(std::move(name), std::move(fd)) {} + + /** + * Reads a release fence from the BufferReleaseChannel. + * + * Returns OK on success. + * Returns WOULD_BLOCK if there is no fence present. + * Other errors probably indicate that the channel is broken. + */ + status_t readReleaseFence(ReleaseCallbackId& outReleaseCallbackId, + sp& outReleaseFence); + + private: + std::vector mFlattenedBuffer; + }; + + class ProducerEndpoint : public Endpoint, public Parcelable { + public: + ProducerEndpoint(std::string name, android::base::unique_fd fd) + : Endpoint(std::move(name), std::move(fd)) {} + ProducerEndpoint() {} + + status_t readFromParcel(const android::Parcel* parcel) override; + status_t writeToParcel(android::Parcel* parcel) const override; + + status_t writeReleaseFence(const ReleaseCallbackId&, const sp& releaseFence); + + private: + std::vector mFlattenedBuffer; + }; + + /** + * Create two endpoints that make up the BufferReleaseChannel. + * + * Return OK on success. + */ + static status_t open(const std::string name, std::unique_ptr& outConsumer, + std::shared_ptr& outProducer); + + struct Message : public Flattenable { + ReleaseCallbackId releaseCallbackId; + sp releaseFence = Fence::NO_FENCE; + + Message() = default; + Message(ReleaseCallbackId releaseCallbackId, sp releaseFence) + : releaseCallbackId(releaseCallbackId), releaseFence(std::move(releaseFence)) {} + + // Flattenable protocol + size_t getFlattenedSize() const; + + size_t getFdCount() const { return releaseFence->getFdCount(); } + + status_t flatten(void*& buffer, size_t& size, int*& fds, size_t& count) const; + + status_t unflatten(void const*& buffer, size_t& size, int const*& fds, size_t& count); + + private: + size_t getPodSize() const; + }; +}; + +} // namespace android::gui diff --git a/libs/gui/include/gui/LayerState.h b/libs/gui/include/gui/LayerState.h index 3fb1894585..d41994589f 100644 --- a/libs/gui/include/gui/LayerState.h +++ b/libs/gui/include/gui/LayerState.h @@ -34,6 +34,7 @@ #include #include +#include #include #include #include @@ -220,6 +221,7 @@ struct layer_state_t { eDropInputModeChanged = 0x8000'00000000, eExtendedRangeBrightnessChanged = 0x10000'00000000, eEdgeExtensionChanged = 0x20000'00000000, + eBufferReleaseChannelChanged = 0x40000'00000000, }; layer_state_t(); @@ -412,6 +414,8 @@ struct layer_state_t { TrustedPresentationThresholds trustedPresentationThresholds; TrustedPresentationListener trustedPresentationListener; + + std::shared_ptr bufferReleaseChannel; }; class ComposerState { diff --git a/libs/gui/include/gui/SurfaceComposerClient.h b/libs/gui/include/gui/SurfaceComposerClient.h index 95574ee30e..8c1d6443c5 100644 --- a/libs/gui/include/gui/SurfaceComposerClient.h +++ b/libs/gui/include/gui/SurfaceComposerClient.h @@ -45,6 +45,7 @@ #include #include +#include #include #include #include @@ -763,6 +764,10 @@ public: const Rect& destinationFrame); Transaction& setDropInputMode(const sp& sc, gui::DropInputMode mode); + Transaction& setBufferReleaseChannel( + const sp& sc, + const std::shared_ptr& channel); + status_t setDisplaySurface(const sp& token, const sp& bufferProducer); diff --git a/libs/gui/libgui_flags.aconfig b/libs/gui/libgui_flags.aconfig index 74d806ec79..e8f2b76b9b 100644 --- a/libs/gui/libgui_flags.aconfig +++ b/libs/gui/libgui_flags.aconfig @@ -51,3 +51,11 @@ flag { bug: "322036393" is_fixed_read_only: true } # edge_extension_shader + +flag { + name: "buffer_release_channel" + namespace: "window_surfaces" + description: "Enable BufferReleaseChannel to optimize buffer releases" + bug: "294133380" + is_fixed_read_only: true +} # buffer_release_channel diff --git a/libs/gui/tests/Android.bp b/libs/gui/tests/Android.bp index ea8acbbb72..daaddfbc15 100644 --- a/libs/gui/tests/Android.bp +++ b/libs/gui/tests/Android.bp @@ -32,6 +32,7 @@ cc_test { "BLASTBufferQueue_test.cpp", "BufferItemConsumer_test.cpp", "BufferQueue_test.cpp", + "BufferReleaseChannel_test.cpp", "Choreographer_test.cpp", "CompositorTiming_test.cpp", "CpuConsumer_test.cpp", diff --git a/libs/gui/tests/BufferReleaseChannel_test.cpp b/libs/gui/tests/BufferReleaseChannel_test.cpp new file mode 100644 index 0000000000..f3e962c192 --- /dev/null +++ b/libs/gui/tests/BufferReleaseChannel_test.cpp @@ -0,0 +1,121 @@ +/* + * Copyright (C) 2024 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include +#include + +#include +#include + +using namespace std::string_literals; +using android::gui::BufferReleaseChannel; + +namespace android { + +namespace { + +// Helper function to check if two file descriptors point to the same file. +bool is_same_file(int fd1, int fd2) { + struct stat stat1; + if (fstat(fd1, &stat1) != 0) { + return false; + } + struct stat stat2; + if (fstat(fd2, &stat2) != 0) { + return false; + } + return (stat1.st_dev == stat2.st_dev) && (stat1.st_ino == stat2.st_ino); +} + +} // namespace + +TEST(BufferReleaseChannelTest, MessageFlattenable) { + ReleaseCallbackId releaseCallbackId{1, 2}; + sp releaseFence = sp::make(memfd_create("fake-fence-fd", 0)); + + std::vector dataBuffer; + std::vector fdBuffer; + + // Verify that we can flatten a message + { + BufferReleaseChannel::Message message{releaseCallbackId, releaseFence}; + + dataBuffer.resize(message.getFlattenedSize()); + void* dataPtr = dataBuffer.data(); + size_t dataSize = dataBuffer.size(); + + fdBuffer.resize(message.getFdCount()); + int* fdPtr = fdBuffer.data(); + size_t fdSize = fdBuffer.size(); + + ASSERT_EQ(OK, message.flatten(dataPtr, dataSize, fdPtr, fdSize)); + + // Fence's unique_fd uses fdsan to check ownership of the file descriptor. Normally the file + // descriptor is passed through the Unix socket and duplicated (and sent to another process) + // so there's no problem with duplicate file descriptor ownership. For this unit test, we + // need to set up a duplicate file descriptor to avoid crashing due to duplicate ownership. + ASSERT_EQ(releaseFence->get(), fdBuffer[0]); + fdBuffer[0] = message.releaseFence->dup(); + } + + // Verify that we can unflatten a message + { + BufferReleaseChannel::Message message; + + const void* dataPtr = dataBuffer.data(); + size_t dataSize = dataBuffer.size(); + + const int* fdPtr = fdBuffer.data(); + size_t fdSize = fdBuffer.size(); + + ASSERT_EQ(OK, message.unflatten(dataPtr, dataSize, fdPtr, fdSize)); + ASSERT_EQ(releaseCallbackId, message.releaseCallbackId); + ASSERT_TRUE(is_same_file(releaseFence->get(), message.releaseFence->get())); + } +} + +// Verify that the BufferReleaseChannel consume returns WOULD_BLOCK when there's no message +// available. +TEST(BufferReleaseChannelTest, ConsumerEndpointIsNonBlocking) { + std::unique_ptr consumer; + std::shared_ptr producer; + ASSERT_EQ(OK, BufferReleaseChannel::open("test-channel"s, consumer, producer)); + + ReleaseCallbackId releaseCallbackId; + sp releaseFence; + ASSERT_EQ(WOULD_BLOCK, consumer->readReleaseFence(releaseCallbackId, releaseFence)); +} + +// Verify that we can write a message to the BufferReleaseChannel producer and read that message +// using the BufferReleaseChannel consumer. +TEST(BufferReleaseChannelTest, ProduceAndConsume) { + std::unique_ptr consumer; + std::shared_ptr producer; + ASSERT_EQ(OK, BufferReleaseChannel::open("test-channel"s, consumer, producer)); + + ReleaseCallbackId producerReleaseCallbackId{1, 2}; + sp producerReleaseFence = sp::make(memfd_create("fake-fence-fd", 0)); + ASSERT_EQ(OK, producer->writeReleaseFence(producerReleaseCallbackId, producerReleaseFence)); + + ReleaseCallbackId consumerReleaseCallbackId; + sp consumerReleaseFence; + ASSERT_EQ(OK, consumer->readReleaseFence(consumerReleaseCallbackId, consumerReleaseFence)); + + ASSERT_EQ(producerReleaseCallbackId, consumerReleaseCallbackId); + ASSERT_TRUE(is_same_file(producerReleaseFence->get(), consumerReleaseFence->get())); +} + +} // namespace android diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp index a6d2b1529a..082d8aa424 100644 --- a/services/surfaceflinger/Layer.cpp +++ b/services/surfaceflinger/Layer.cpp @@ -2783,11 +2783,14 @@ void Layer::callReleaseBufferCallback(const sp& l return; } SFTRACE_FORMAT_INSTANT("callReleaseBufferCallback %s - %" PRIu64, getDebugName(), framenumber); + ReleaseCallbackId callbackId{buffer->getId(), framenumber}; + const sp& fence = releaseFence ? releaseFence : Fence::NO_FENCE; uint32_t currentMaxAcquiredBufferCount = mFlinger->getMaxAcquiredBufferCountForCurrentRefreshRate(mOwnerUid); - listener->onReleaseBuffer({buffer->getId(), framenumber}, - releaseFence ? releaseFence : Fence::NO_FENCE, - currentMaxAcquiredBufferCount); + listener->onReleaseBuffer(callbackId, fence, currentMaxAcquiredBufferCount); + if (mBufferReleaseChannel) { + mBufferReleaseChannel->writeReleaseFence(callbackId, fence); + } } sp Layer::findCallbackHandle() { @@ -2905,6 +2908,7 @@ void Layer::onLayerDisplayed(ftl::SharedFuture futureFenceResult, void Layer::releasePendingBuffer(nsecs_t dequeueReadyTime) { for (const auto& handle : mDrawingState.callbackHandles) { + handle->bufferReleaseChannel = mBufferReleaseChannel; handle->transformHint = mTransformHint; handle->dequeueReadyTime = dequeueReadyTime; handle->currentMaxAcquiredBufferCount = @@ -4376,6 +4380,11 @@ bool Layer::setTrustedPresentationInfo(TrustedPresentationThresholds const& thre return haveTrustedPresentationListener; } +void Layer::setBufferReleaseChannel( + const std::shared_ptr& channel) { + mBufferReleaseChannel = channel; +} + void Layer::updateLastLatchTime(nsecs_t latchTime) { mLastLatchTime = latchTime; } diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h index 52f169ebee..d4707388b2 100644 --- a/services/surfaceflinger/Layer.h +++ b/services/surfaceflinger/Layer.h @@ -550,6 +550,7 @@ public: }; BufferInfo mBufferInfo; + std::shared_ptr mBufferReleaseChannel; // implements compositionengine::LayerFE const compositionengine::LayerFECompositionState* getCompositionState() const; @@ -813,6 +814,8 @@ public: bool setTrustedPresentationInfo(TrustedPresentationThresholds const& thresholds, TrustedPresentationListener const& listener); + void setBufferReleaseChannel( + const std::shared_ptr& channel); // Creates a new handle each time, so we only expect // this to be called once. diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 1d3573017d..beb05b9780 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -5951,6 +5951,10 @@ uint32_t SurfaceFlinger::updateLayerCallbacksAndStats(const FrameTimelineInfo& f } } + if (what & layer_state_t::eBufferReleaseChannelChanged) { + layer->setBufferReleaseChannel(s.bufferReleaseChannel); + } + const auto& requestedLayerState = mLayerLifecycleManager.getLayerFromId(layer->getSequence()); bool willPresentCurrentTransaction = requestedLayerState && (requestedLayerState->hasReadyFrame() || diff --git a/services/surfaceflinger/TransactionCallbackInvoker.cpp b/services/surfaceflinger/TransactionCallbackInvoker.cpp index 881bf35b58..9ea0f5f4ca 100644 --- a/services/surfaceflinger/TransactionCallbackInvoker.cpp +++ b/services/surfaceflinger/TransactionCallbackInvoker.cpp @@ -149,6 +149,12 @@ status_t TransactionCallbackInvoker::addCallbackHandle(const sp& handle->transformHint, handle->currentMaxAcquiredBufferCount, eventStats, handle->previousReleaseCallbackId); + if (handle->bufferReleaseChannel && + handle->previousReleaseCallbackId != ReleaseCallbackId::INVALID_ID) { + mBufferReleases.emplace_back(handle->bufferReleaseChannel, + handle->previousReleaseCallbackId, + handle->previousReleaseFence); + } } return NO_ERROR; } @@ -158,6 +164,11 @@ void TransactionCallbackInvoker::addPresentFence(sp presentFence) { } void TransactionCallbackInvoker::sendCallbacks(bool onCommitOnly) { + for (const auto& bufferRelease : mBufferReleases) { + bufferRelease.channel->writeReleaseFence(bufferRelease.callbackId, bufferRelease.fence); + } + mBufferReleases.clear(); + // For each listener auto completedTransactionsItr = mCompletedTransactions.begin(); ftl::SmallVector listenerStatsToSend; diff --git a/services/surfaceflinger/TransactionCallbackInvoker.h b/services/surfaceflinger/TransactionCallbackInvoker.h index 7853a9f359..f74f9644b6 100644 --- a/services/surfaceflinger/TransactionCallbackInvoker.h +++ b/services/surfaceflinger/TransactionCallbackInvoker.h @@ -28,6 +28,7 @@ #include #include #include +#include #include #include #include @@ -59,6 +60,7 @@ public: uint64_t frameNumber = 0; uint64_t previousFrameNumber = 0; ReleaseCallbackId previousReleaseCallbackId = ReleaseCallbackId::INVALID_ID; + std::shared_ptr bufferReleaseChannel; }; class TransactionCallbackInvoker { @@ -86,6 +88,13 @@ private: std::unordered_map, std::deque, IListenerHash> mCompletedTransactions; + struct BufferRelease { + std::shared_ptr channel; + ReleaseCallbackId callbackId; + sp fence; + }; + std::vector mBufferReleases; + sp mPresentFence; }; -- cgit v1.2.3-59-g8ed1b From 4b9507d5a4586ecd2e14db88eb8674e204387ddd Mon Sep 17 00:00:00 2001 From: Patrick Williams Date: Thu, 25 Jul 2024 09:55:52 -0500 Subject: Revert "Optimize BLAST buffer releases via Unix sockets" Reverting due to b/355260320 Change-Id: I8a32f73b6805d3f2bceb2948925be6a9baaa3015 --- libs/gui/Android.bp | 1 - libs/gui/BLASTBufferQueue.cpp | 343 ++----------------- libs/gui/BufferReleaseChannel.cpp | 364 --------------------- libs/gui/LayerState.cpp | 17 - libs/gui/SurfaceComposerClient.cpp | 16 - libs/gui/include/gui/BLASTBufferQueue.h | 56 +--- libs/gui/include/gui/BufferReleaseChannel.h | 127 ------- libs/gui/include/gui/LayerState.h | 4 - libs/gui/include/gui/SurfaceComposerClient.h | 5 - libs/gui/tests/Android.bp | 1 - libs/gui/tests/BufferReleaseChannel_test.cpp | 121 ------- services/surfaceflinger/Layer.cpp | 15 +- services/surfaceflinger/Layer.h | 3 - services/surfaceflinger/SurfaceFlinger.cpp | 4 - .../surfaceflinger/TransactionCallbackInvoker.cpp | 11 - .../surfaceflinger/TransactionCallbackInvoker.h | 9 - 16 files changed, 26 insertions(+), 1071 deletions(-) delete mode 100644 libs/gui/BufferReleaseChannel.cpp delete mode 100644 libs/gui/include/gui/BufferReleaseChannel.h delete mode 100644 libs/gui/tests/BufferReleaseChannel_test.cpp (limited to 'libs/gui/LayerState.cpp') diff --git a/libs/gui/Android.bp b/libs/gui/Android.bp index 1243b214d3..51d2e5305a 100644 --- a/libs/gui/Android.bp +++ b/libs/gui/Android.bp @@ -255,7 +255,6 @@ filegroup { "BitTube.cpp", "BLASTBufferQueue.cpp", "BufferItemConsumer.cpp", - "BufferReleaseChannel.cpp", "Choreographer.cpp", "CompositorTiming.cpp", "ConsumerBase.cpp", diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp index 767f3e8c16..739c3c2a41 100644 --- a/libs/gui/BLASTBufferQueue.cpp +++ b/libs/gui/BLASTBufferQueue.cpp @@ -38,17 +38,13 @@ #include #include -#include #include -#include -#include #include #include using namespace com::android::graphics::libgui; using namespace std::chrono_literals; -using android::base::unique_fd; namespace { inline const char* boolToString(bool b) { @@ -183,6 +179,8 @@ BLASTBufferQueue::BLASTBufferQueue(const std::string& name, bool updateDestinati // explicitly so that dequeueBuffer will block mProducer->setDequeueTimeout(std::numeric_limits::max()); + // safe default, most producers are expected to override this + mProducer->setMaxDequeuedBufferCount(2); mBufferItemConsumer = new BLASTBufferItemConsumer(mConsumer, GraphicBuffer::USAGE_HW_COMPOSER | GraphicBuffer::USAGE_HW_TEXTURE, @@ -212,12 +210,6 @@ BLASTBufferQueue::BLASTBufferQueue(const std::string& name, bool updateDestinati }, this); -#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL) - std::unique_ptr bufferReleaseConsumer; - gui::BufferReleaseChannel::open(mName, bufferReleaseConsumer, mBufferReleaseProducer); - mBufferReleaseReader.emplace(std::move(bufferReleaseConsumer)); -#endif - BQA_LOGV("BLASTBufferQueue created"); } @@ -267,9 +259,6 @@ void BLASTBufferQueue::update(const sp& surface, uint32_t width, if (surfaceControlChanged) { t.setFlags(mSurfaceControl, layer_state_t::eEnableBackpressure, layer_state_t::eEnableBackpressure); - if (mBufferReleaseProducer) { - t.setBufferReleaseChannel(mSurfaceControl, mBufferReleaseProducer); - } applyTransaction = true; } mTransformHint = mSurfaceControl->getTransformHint(); @@ -450,21 +439,6 @@ void BLASTBufferQueue::releaseBufferCallback( BBQ_TRACE(); releaseBufferCallbackLocked(id, releaseFence, currentMaxAcquiredBufferCount, false /* fakeRelease */); - if (!mBufferReleaseReader) { - return; - } - // Drain the buffer release channel socket - while (true) { - ReleaseCallbackId releaseCallbackId; - sp releaseFence; - if (status_t status = - mBufferReleaseReader->readNonBlocking(releaseCallbackId, releaseFence); - status != OK) { - break; - } - releaseBufferCallbackLocked(releaseCallbackId, releaseFence, std::nullopt, - false /* fakeRelease */); - } } void BLASTBufferQueue::releaseBufferCallbackLocked( @@ -521,13 +495,11 @@ void BLASTBufferQueue::releaseBuffer(const ReleaseCallbackId& callbackId, const sp& releaseFence) { auto it = mSubmitted.find(callbackId); if (it == mSubmitted.end()) { + BQA_LOGE("ERROR: releaseBufferCallback without corresponding submitted buffer %s", + callbackId.to_string().c_str()); return; } mNumAcquired--; - updateDequeueShouldBlockLocked(); - if (mBufferReleaseReader) { - mBufferReleaseReader->interruptBlockingRead(); - } BBQ_TRACE("frame=%" PRIu64, callbackId.framenumber); BQA_LOGV("released %s", callbackId.to_string().c_str()); mBufferItemConsumer->releaseBuffer(it->second, releaseFence); @@ -592,7 +564,6 @@ status_t BLASTBufferQueue::acquireNextBufferLocked( auto buffer = bufferItem.mGraphicBuffer; mNumFrameAvailable--; - updateDequeueShouldBlockLocked(); BBQ_TRACE("frame=%" PRIu64, bufferItem.mFrameNumber); if (buffer == nullptr) { @@ -611,7 +582,6 @@ status_t BLASTBufferQueue::acquireNextBufferLocked( } mNumAcquired++; - updateDequeueShouldBlockLocked(); mLastAcquiredFrameNumber = bufferItem.mFrameNumber; ReleaseCallbackId releaseCallbackId(buffer->getId(), mLastAcquiredFrameNumber); mSubmitted[releaseCallbackId] = bufferItem; @@ -738,7 +708,6 @@ void BLASTBufferQueue::acquireAndReleaseBuffer() { return; } mNumFrameAvailable--; - updateDequeueShouldBlockLocked(); mBufferItemConsumer->releaseBuffer(bufferItem, bufferItem.mFence); } @@ -792,9 +761,7 @@ void BLASTBufferQueue::onFrameAvailable(const BufferItem& item) { } // add to shadow queue - mNumDequeued--; mNumFrameAvailable++; - updateDequeueShouldBlockLocked(); if (waitForTransactionCallback && mNumFrameAvailable >= 2) { acquireAndReleaseBuffer(); } @@ -845,24 +812,11 @@ void BLASTBufferQueue::onFrameReplaced(const BufferItem& item) { void BLASTBufferQueue::onFrameDequeued(const uint64_t bufferId) { std::lock_guard _lock{mTimestampMutex}; mDequeueTimestamps[bufferId] = systemTime(); - mNumDequeued++; -} +}; void BLASTBufferQueue::onFrameCancelled(const uint64_t bufferId) { - { - std::lock_guard _lock{mTimestampMutex}; - mDequeueTimestamps.erase(bufferId); - } - - { - std::lock_guard lock{mMutex}; - mNumDequeued--; - updateDequeueShouldBlockLocked(); - } - - if (mBufferReleaseReader) { - mBufferReleaseReader->interruptBlockingRead(); - } + std::lock_guard _lock{mTimestampMutex}; + mDequeueTimestamps.erase(bufferId); }; bool BLASTBufferQueue::syncNextTransaction( @@ -934,22 +888,6 @@ bool BLASTBufferQueue::rejectBuffer(const BufferItem& item) { return mSize != bufferSize; } -void BLASTBufferQueue::updateDequeueShouldBlockLocked() { - int32_t buffersInUse = mNumDequeued + mNumFrameAvailable + mNumAcquired; - int32_t maxBufferCount = std::min(mMaxAcquiredBuffers + mMaxDequeuedBuffers, kMaxBufferCount); - bool bufferAvailable = buffersInUse < maxBufferCount; - // BLASTBufferQueueProducer should block until a buffer is released if - // (1) There are no free buffers available. - // (2) We're not in async mode. In async mode, BufferQueueProducer::dequeueBuffer returns - // WOULD_BLOCK instead of blocking when there are no free buffers. - // (3) We're not in shared buffer mode. In shared buffer mode, both the producer and consumer - // can access the same buffer simultaneously. BufferQueueProducer::dequeueBuffer returns - // the shared buffer immediately instead of blocking. - mDequeueShouldBlock = !(bufferAvailable || mAsyncMode || mSharedBufferMode); - ATRACE_INT("Dequeued", mNumDequeued); - ATRACE_INT("DequeueShouldBlock", mDequeueShouldBlock); -} - class BBQSurface : public Surface { private: std::mutex mMutex; @@ -1178,64 +1116,24 @@ public: producerControlledByApp, output); } - status_t disconnect(int api, DisconnectMode mode) override { - if (status_t status = BufferQueueProducer::disconnect(api, mode); status != OK) { - return status; - } - - sp bbq = mBLASTBufferQueue.promote(); - if (!bbq) { - return OK; - } - - { - std::lock_guard lock{bbq->mMutex}; - bbq->mNumDequeued = 0; - bbq->mNumFrameAvailable = 0; - bbq->mNumAcquired = 0; - bbq->mSubmitted.clear(); - bbq->updateDequeueShouldBlockLocked(); - } - - if (bbq->mBufferReleaseReader) { - bbq->mBufferReleaseReader->interruptBlockingRead(); - } - - return OK; - } - // We want to resize the frame history when changing the size of the buffer queue status_t setMaxDequeuedBufferCount(int maxDequeuedBufferCount) override { int maxBufferCount; status_t status = BufferQueueProducer::setMaxDequeuedBufferCount(maxDequeuedBufferCount, &maxBufferCount); - if (status != OK) { - return status; - } - - sp bbq = mBLASTBufferQueue.promote(); - if (!bbq) { - return OK; - } - - { - std::lock_guard lock{bbq->mMutex}; - bbq->mMaxDequeuedBuffers = maxDequeuedBufferCount; - bbq->updateDequeueShouldBlockLocked(); - } - - if (bbq->mBufferReleaseReader) { - bbq->mBufferReleaseReader->interruptBlockingRead(); - } - - size_t newFrameHistorySize = maxBufferCount + 2; // +2 because triple buffer rendering - // optimize away resizing the frame history unless it will grow - if (newFrameHistorySize > FrameEventHistory::INITIAL_MAX_FRAME_HISTORY) { - ALOGV("increasing frame history size to %zu", newFrameHistorySize); - bbq->resizeFrameEventHistory(newFrameHistorySize); + // if we can't determine the max buffer count, then just skip growing the history size + if (status == OK) { + size_t newFrameHistorySize = maxBufferCount + 2; // +2 because triple buffer rendering + // optimize away resizing the frame history unless it will grow + if (newFrameHistorySize > FrameEventHistory::INITIAL_MAX_FRAME_HISTORY) { + sp bbq = mBLASTBufferQueue.promote(); + if (bbq != nullptr) { + ALOGV("increasing frame history size to %zu", newFrameHistorySize); + bbq->resizeFrameEventHistory(newFrameHistorySize); + } + } } - - return OK; + return status; } int query(int what, int* value) override { @@ -1246,131 +1144,6 @@ public: return BufferQueueProducer::query(what, value); } - status_t setAsyncMode(bool asyncMode) override { - if (status_t status = BufferQueueProducer::setAsyncMode(asyncMode); status != NO_ERROR) { - return status; - } - - sp bbq = mBLASTBufferQueue.promote(); - if (!bbq) { - return NO_ERROR; - } - - { - std::lock_guard lock{bbq->mMutex}; - bbq->mAsyncMode = asyncMode; - bbq->updateDequeueShouldBlockLocked(); - } - - if (bbq->mBufferReleaseReader) { - bbq->mBufferReleaseReader->interruptBlockingRead(); - } - return NO_ERROR; - } - - status_t setSharedBufferMode(bool sharedBufferMode) override { - if (status_t status = BufferQueueProducer::setSharedBufferMode(sharedBufferMode); - status != NO_ERROR) { - return status; - } - - sp bbq = mBLASTBufferQueue.promote(); - if (!bbq) { - return NO_ERROR; - } - - { - std::lock_guard lock{bbq->mMutex}; - bbq->mSharedBufferMode = sharedBufferMode; - bbq->updateDequeueShouldBlockLocked(); - } - - if (bbq->mBufferReleaseReader) { - bbq->mBufferReleaseReader->interruptBlockingRead(); - } - return NO_ERROR; - } - - status_t detachBuffer(int slot) override { - if (status_t status = BufferQueueProducer::detachBuffer(slot); status != NO_ERROR) { - return status; - } - - sp bbq = mBLASTBufferQueue.promote(); - if (!bbq) { - return NO_ERROR; - } - - { - std::lock_guard lock{bbq->mMutex}; - bbq->mNumDequeued--; - bbq->updateDequeueShouldBlockLocked(); - } - - if (bbq->mBufferReleaseReader) { - bbq->mBufferReleaseReader->interruptBlockingRead(); - } - return NO_ERROR; - } - - // Override dequeueBuffer to block if there are no free buffers. - // - // Buffer releases are communicated via the BufferReleaseChannel. When dequeueBuffer determines - // a free buffer is not available, it blocks on an epoll file descriptor. Epoll is configured to - // detect messages on the BufferReleaseChannel's socket and an eventfd. The eventfd is signaled - // whenever an event other than a buffer release occurs that may change the number of free - // buffers. dequeueBuffer uses epoll in a similar manner as a condition variable by testing for - // the availability of a free buffer in a loop, breaking the loop once a free buffer is - // available. - // - // This is an optimization implemented to reduce thread scheduling delays in the previously - // existing binder release callback. The binder buffer release callback is still used and there - // are no guarantees around order between buffer releases via binder and the - // BufferReleaseChannel. If we attempt to a release a buffer here that has already been released - // via binder, the release is ignored. - status_t dequeueBuffer(int* outSlot, sp* outFence, uint32_t width, uint32_t height, - PixelFormat format, uint64_t usage, uint64_t* outBufferAge, - FrameEventHistoryDelta* outTimestamps) { - sp bbq = mBLASTBufferQueue.promote(); - if (!bbq || !bbq->mBufferReleaseReader) { - return BufferQueueProducer::dequeueBuffer(outSlot, outFence, width, height, format, - usage, outBufferAge, outTimestamps); - } - - if (bbq->mDequeueShouldBlock) { - ATRACE_FORMAT("waiting for free buffer"); - auto maxWaitTime = std::chrono::steady_clock::now() + 1s; - do { - auto timeout = std::chrono::duration_cast( - maxWaitTime - std::chrono::steady_clock::now()); - if (timeout <= 0ms) { - break; - } - - ReleaseCallbackId releaseCallbackId; - sp releaseFence; - status_t status = bbq->mBufferReleaseReader->readBlocking(releaseCallbackId, - releaseFence, timeout); - if (status == WOULD_BLOCK) { - // readBlocking was interrupted. The loop will test if we have a free buffer. - continue; - } - - if (status != OK) { - // An error occurred or readBlocking timed out. - break; - } - - std::lock_guard lock{bbq->mMutex}; - bbq->releaseBufferCallbackLocked(releaseCallbackId, releaseFence, std::nullopt, - false); - } while (bbq->mDequeueShouldBlock); - } - - return BufferQueueProducer::dequeueBuffer(outSlot, outFence, width, height, format, usage, - outBufferAge, outTimestamps); - } - private: const wp mBLASTBufferQueue; }; @@ -1400,16 +1173,6 @@ void BLASTBufferQueue::createBufferQueue(sp* outProducer *outConsumer = consumer; } -void BLASTBufferQueue::onFirstRef() { - // safe default, most producers are expected to override this - // - // This is done in onFirstRef instead of BLASTBufferQueue's constructor because - // BBQBufferQueueProducer::setMaxDequeuedBufferCount promotes a weak pointer to BLASTBufferQueue - // to a strong pointer. If this is done in the constructor, then when the strong pointer goes - // out of scope, it's the last reference so BLASTBufferQueue is deleted. - mProducer->setMaxDequeuedBufferCount(2); -} - void BLASTBufferQueue::resizeFrameEventHistory(size_t newSize) { // This can be null during creation of the buffer queue, but resizing won't do anything at that // point in time, so just ignore. This can go away once the class relationships and lifetimes of @@ -1459,72 +1222,4 @@ void BLASTBufferQueue::setTransactionHangCallback( mTransactionHangCallback = callback; } -BLASTBufferQueue::BufferReleaseReader::BufferReleaseReader( - std::unique_ptr endpoint) - : mEndpoint(std::move(endpoint)) { - mEpollFd = android::base::unique_fd(epoll_create1(0)); - if (!mEpollFd.ok()) { - ALOGE("Failed to create buffer release epoll file descriptor. errno=%d message='%s'", errno, - strerror(errno)); - } - - epoll_event event; - event.events = EPOLLIN; - event.data.fd = mEndpoint->getFd(); - if (epoll_ctl(mEpollFd.get(), EPOLL_CTL_ADD, mEndpoint->getFd(), &event) == -1) { - ALOGE("Failed to register buffer release consumer file descriptor with epoll. errno=%d " - "message='%s'", - errno, strerror(errno)); - } - - mEventFd = android::base::unique_fd(eventfd(0, EFD_NONBLOCK)); - event.data.fd = mEventFd.get(); - if (epoll_ctl(mEpollFd.get(), EPOLL_CTL_ADD, mEventFd.get(), &event) == -1) { - ALOGE("Failed to register buffer release eventfd with epoll. errno=%d message='%s'", errno, - strerror(errno)); - } -} - -status_t BLASTBufferQueue::BufferReleaseReader::readNonBlocking(ReleaseCallbackId& outId, - sp& outFence) { - std::lock_guard lock{mMutex}; - return mEndpoint->readReleaseFence(outId, outFence); -} - -status_t BLASTBufferQueue::BufferReleaseReader::readBlocking(ReleaseCallbackId& outId, - sp& outFence, - std::chrono::milliseconds timeout) { - epoll_event event; - int eventCount = epoll_wait(mEpollFd.get(), &event, 1 /* maxevents */, timeout.count()); - - if (eventCount == -1) { - ALOGE("epoll_wait error while waiting for buffer release. errno=%d message='%s'", errno, - strerror(errno)); - return UNKNOWN_ERROR; - } - - if (eventCount == 0) { - return TIMED_OUT; - } - - if (event.data.fd == mEventFd.get()) { - uint64_t value; - if (read(mEventFd.get(), &value, sizeof(uint64_t)) == -1 && errno != EWOULDBLOCK) { - ALOGE("error while reading from eventfd. errno=%d message='%s'", errno, - strerror(errno)); - } - return WOULD_BLOCK; - } - - std::lock_guard lock{mMutex}; - return mEndpoint->readReleaseFence(outId, outFence); -} - -void BLASTBufferQueue::BufferReleaseReader::interruptBlockingRead() { - uint64_t value = 1; - if (write(mEventFd.get(), &value, sizeof(uint64_t)) == -1) { - ALOGE("failed to notify dequeue event. errno=%d message='%s'", errno, strerror(errno)); - } -} - } // namespace android diff --git a/libs/gui/BufferReleaseChannel.cpp b/libs/gui/BufferReleaseChannel.cpp deleted file mode 100644 index 183b25a4cb..0000000000 --- a/libs/gui/BufferReleaseChannel.cpp +++ /dev/null @@ -1,364 +0,0 @@ -/* - * Copyright (C) 2024 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#define LOG_TAG "BufferReleaseChannel" -#define ATRACE_TAG ATRACE_TAG_GRAPHICS - -#include -#include -#include -#include -#include -#include - -#include -#include -#include -#include -#include -#include -#include -#include -#include - -#include -#include - -#include - -using android::base::Result; - -namespace android::gui { - -namespace { - -template -static void readAligned(const void*& buffer, size_t& size, T& value) { - size -= FlattenableUtils::align(buffer); - FlattenableUtils::read(buffer, size, value); -} - -template -static void writeAligned(void*& buffer, size_t& size, T value) { - size -= FlattenableUtils::align(buffer); - FlattenableUtils::write(buffer, size, value); -} - -template -static void addAligned(size_t& size, T /* value */) { - size = FlattenableUtils::align(size); - size += sizeof(T); -} - -template -static inline constexpr uint32_t low32(const T n) { - return static_cast(static_cast(n)); -} - -template -static inline constexpr uint32_t high32(const T n) { - return static_cast(static_cast(n) >> 32); -} - -template -static inline constexpr T to64(const uint32_t lo, const uint32_t hi) { - return static_cast(static_cast(hi) << 32 | lo); -} - -} // namespace - -size_t BufferReleaseChannel::Message::getPodSize() const { - size_t size = 0; - addAligned(size, low32(releaseCallbackId.bufferId)); - addAligned(size, high32(releaseCallbackId.bufferId)); - addAligned(size, low32(releaseCallbackId.framenumber)); - addAligned(size, high32(releaseCallbackId.framenumber)); - return size; -} - -size_t BufferReleaseChannel::Message::getFlattenedSize() const { - size_t size = releaseFence->getFlattenedSize(); - size = FlattenableUtils::align<4>(size); - size += getPodSize(); - return size; -} - -status_t BufferReleaseChannel::Message::flatten(void*& buffer, size_t& size, int*& fds, - size_t& count) const { - if (status_t err = releaseFence->flatten(buffer, size, fds, count); err != OK) { - return err; - } - size -= FlattenableUtils::align<4>(buffer); - - // Check we still have enough space - if (size < getPodSize()) { - return NO_MEMORY; - } - - writeAligned(buffer, size, low32(releaseCallbackId.bufferId)); - writeAligned(buffer, size, high32(releaseCallbackId.bufferId)); - writeAligned(buffer, size, low32(releaseCallbackId.framenumber)); - writeAligned(buffer, size, high32(releaseCallbackId.framenumber)); - return OK; -} - -status_t BufferReleaseChannel::Message::unflatten(void const*& buffer, size_t& size, - int const*& fds, size_t& count) { - releaseFence = new Fence(); - if (status_t err = releaseFence->unflatten(buffer, size, fds, count); err != OK) { - return err; - } - size -= FlattenableUtils::align<4>(buffer); - - // Check we still have enough space - if (size < getPodSize()) { - return OK; - } - - uint32_t bufferIdLo = 0, bufferIdHi = 0; - uint32_t frameNumberLo = 0, frameNumberHi = 0; - - readAligned(buffer, size, bufferIdLo); - readAligned(buffer, size, bufferIdHi); - releaseCallbackId.bufferId = to64(bufferIdLo, bufferIdHi); - readAligned(buffer, size, frameNumberLo); - readAligned(buffer, size, frameNumberHi); - releaseCallbackId.framenumber = to64(frameNumberLo, frameNumberHi); - - return NO_ERROR; -} - -status_t BufferReleaseChannel::ConsumerEndpoint::readReleaseFence( - ReleaseCallbackId& outReleaseCallbackId, sp& outReleaseFence) { - Message releaseBufferMessage; - mFlattenedBuffer.resize(releaseBufferMessage.getFlattenedSize()); - std::array controlMessageBuffer; - - iovec iov{ - .iov_base = mFlattenedBuffer.data(), - .iov_len = mFlattenedBuffer.size(), - }; - - msghdr msg{ - .msg_iov = &iov, - .msg_iovlen = 1, - .msg_control = controlMessageBuffer.data(), - .msg_controllen = controlMessageBuffer.size(), - }; - - int result; - do { - result = recvmsg(mFd, &msg, 0); - } while (result == -1 && errno == EINTR); - if (result == -1) { - if (errno == EWOULDBLOCK || errno == EAGAIN) { - return WOULD_BLOCK; - } - ALOGE("Error reading release fence from socket: error %#x (%s)", errno, strerror(errno)); - return -errno; - } - - if (msg.msg_iovlen != 1) { - ALOGE("Error reading release fence from socket: bad data length"); - return UNKNOWN_ERROR; - } - - if (msg.msg_controllen % sizeof(int) != 0) { - ALOGE("Error reading release fence from socket: bad fd length"); - return UNKNOWN_ERROR; - } - - size_t dataLen = msg.msg_iov->iov_len; - const void* data = static_cast(msg.msg_iov->iov_base); - if (!data) { - ALOGE("Error reading release fence from socket: no buffer data"); - return UNKNOWN_ERROR; - } - - size_t fdCount = 0; - const int* fdData = nullptr; - if (cmsghdr* cmsg = CMSG_FIRSTHDR(&msg)) { - fdData = reinterpret_cast(CMSG_DATA(cmsg)); - fdCount = (cmsg->cmsg_len - CMSG_LEN(0)) / sizeof(int); - } - - if (status_t err = releaseBufferMessage.unflatten(data, dataLen, fdData, fdCount); err != OK) { - return err; - } - - outReleaseCallbackId = releaseBufferMessage.releaseCallbackId; - outReleaseFence = std::move(releaseBufferMessage.releaseFence); - - return OK; -} - -int BufferReleaseChannel::ProducerEndpoint::writeReleaseFence(const ReleaseCallbackId& callbackId, - const sp& fence) { - Message releaseBufferMessage(callbackId, fence ? fence : Fence::NO_FENCE); - mFlattenedBuffer.resize(releaseBufferMessage.getFlattenedSize()); - int flattenedFd; - { - // Make copies of needed items since flatten modifies them, and we don't - // want to send anything if there's an error during flatten. - void* flattenedBufferPtr = mFlattenedBuffer.data(); - size_t flattenedBufferSize = mFlattenedBuffer.size(); - int* flattenedFdPtr = &flattenedFd; - size_t flattenedFdCount = 1; - if (status_t err = releaseBufferMessage.flatten(flattenedBufferPtr, flattenedBufferSize, - flattenedFdPtr, flattenedFdCount); - err != OK) { - ALOGE("Failed to flatten BufferReleaseChannel message."); - return err; - } - } - - iovec iov{ - .iov_base = mFlattenedBuffer.data(), - .iov_len = mFlattenedBuffer.size(), - }; - - msghdr msg{ - .msg_iov = &iov, - .msg_iovlen = 1, - }; - - std::array controlMessageBuffer; - if (fence && fence->isValid()) { - msg.msg_control = controlMessageBuffer.data(); - msg.msg_controllen = controlMessageBuffer.size(); - - cmsghdr* cmsg = CMSG_FIRSTHDR(&msg); - cmsg->cmsg_level = SOL_SOCKET; - cmsg->cmsg_type = SCM_RIGHTS; - cmsg->cmsg_len = CMSG_LEN(sizeof(int)); - memcpy(CMSG_DATA(cmsg), &flattenedFd, sizeof(int)); - } - - int result; - do { - result = sendmsg(mFd, &msg, 0); - } while (result == -1 && errno == EINTR); - if (result == -1) { - ALOGD("Error writing release fence to socket: error %#x (%s)", errno, strerror(errno)); - return -errno; - } - - return OK; -} - -status_t BufferReleaseChannel::ProducerEndpoint::readFromParcel(const android::Parcel* parcel) { - if (!parcel) return STATUS_BAD_VALUE; - SAFE_PARCEL(parcel->readUtf8FromUtf16, &mName); - SAFE_PARCEL(parcel->readUniqueFileDescriptor, &mFd); - return STATUS_OK; -} - -status_t BufferReleaseChannel::ProducerEndpoint::writeToParcel(android::Parcel* parcel) const { - if (!parcel) return STATUS_BAD_VALUE; - SAFE_PARCEL(parcel->writeUtf8AsUtf16, mName); - SAFE_PARCEL(parcel->writeUniqueFileDescriptor, mFd); - return STATUS_OK; -} - -status_t BufferReleaseChannel::open(std::string name, - std::unique_ptr& outConsumer, - std::shared_ptr& outProducer) { - outConsumer.reset(); - outProducer.reset(); - - int sockets[2]; - if (socketpair(AF_UNIX, SOCK_SEQPACKET, 0, sockets)) { - ALOGE("[%s] Failed to create socket pair. errorno=%d message='%s'", name.c_str(), errno, - strerror(errno)); - return -errno; - } - - android::base::unique_fd consumerFd(sockets[0]); - android::base::unique_fd producerFd(sockets[1]); - - // Socket buffer size. The default is typically about 128KB, which is much larger than - // we really need. So we make it smaller. It just needs to be big enough to hold - // a few dozen release fences. - const int bufferSize = 32 * 1024; - if (setsockopt(consumerFd.get(), SOL_SOCKET, SO_SNDBUF, &bufferSize, sizeof(bufferSize)) == - -1) { - ALOGE("[%s] Failed to set consumer socket send buffer size. errno=%d message='%s'", - name.c_str(), errno, strerror(errno)); - return -errno; - } - if (setsockopt(consumerFd.get(), SOL_SOCKET, SO_RCVBUF, &bufferSize, sizeof(bufferSize)) == - -1) { - ALOGE("[%s] Failed to set consumer socket receive buffer size. errno=%d " - "message='%s'", - name.c_str(), errno, strerror(errno)); - return -errno; - } - if (setsockopt(producerFd.get(), SOL_SOCKET, SO_SNDBUF, &bufferSize, sizeof(bufferSize)) == - -1) { - ALOGE("[%s] Failed to set producer socket send buffer size. errno=%d message='%s'", - name.c_str(), errno, strerror(errno)); - return -errno; - } - if (setsockopt(producerFd.get(), SOL_SOCKET, SO_RCVBUF, &bufferSize, sizeof(bufferSize)) == - -1) { - ALOGE("[%s] Failed to set producer socket receive buffer size. errno=%d " - "message='%s'", - name.c_str(), errno, strerror(errno)); - return -errno; - } - - // Configure the consumer socket to be non-blocking. - int flags = fcntl(consumerFd.get(), F_GETFL, 0); - if (flags == -1) { - ALOGE("[%s] Failed to get consumer socket flags. errno=%d message='%s'", name.c_str(), - errno, strerror(errno)); - return -errno; - } - if (fcntl(consumerFd.get(), F_SETFL, flags | O_NONBLOCK) == -1) { - ALOGE("[%s] Failed to set consumer socket to non-blocking mode. errno=%d " - "message='%s'", - name.c_str(), errno, strerror(errno)); - return -errno; - } - - // Configure a timeout for the producer socket. - const timeval timeout{.tv_sec = 1, .tv_usec = 0}; - if (setsockopt(producerFd.get(), SOL_SOCKET, SO_RCVTIMEO, &timeout, sizeof(timeval)) == -1) { - ALOGE("[%s] Failed to set producer socket timeout. errno=%d message='%s'", name.c_str(), - errno, strerror(errno)); - return -errno; - } - - // Make the consumer read-only - if (shutdown(consumerFd.get(), SHUT_WR) == -1) { - ALOGE("[%s] Failed to shutdown writing on consumer socket. errno=%d message='%s'", - name.c_str(), errno, strerror(errno)); - return -errno; - } - - // Make the producer write-only - if (shutdown(producerFd.get(), SHUT_RD) == -1) { - ALOGE("[%s] Failed to shutdown reading on producer socket. errno=%d message='%s'", - name.c_str(), errno, strerror(errno)); - return -errno; - } - - outConsumer = std::make_unique(name, std::move(consumerFd)); - outProducer = std::make_shared(std::move(name), std::move(producerFd)); - return STATUS_OK; -} - -} // namespace android::gui diff --git a/libs/gui/LayerState.cpp b/libs/gui/LayerState.cpp index adf1646b83..307ae3990e 100644 --- a/libs/gui/LayerState.cpp +++ b/libs/gui/LayerState.cpp @@ -194,12 +194,6 @@ status_t layer_state_t::write(Parcel& output) const SAFE_PARCEL(output.writeFloat, currentHdrSdrRatio); SAFE_PARCEL(output.writeFloat, desiredHdrSdrRatio); SAFE_PARCEL(output.writeInt32, static_cast(cachingHint)); - - const bool hasBufferReleaseChannel = (bufferReleaseChannel != nullptr); - SAFE_PARCEL(output.writeBool, hasBufferReleaseChannel); - if (hasBufferReleaseChannel) { - SAFE_PARCEL(output.writeParcelable, *bufferReleaseChannel); - } return NO_ERROR; } @@ -345,12 +339,6 @@ status_t layer_state_t::read(const Parcel& input) SAFE_PARCEL(input.readInt32, &tmpInt32); cachingHint = static_cast(tmpInt32); - bool hasBufferReleaseChannel; - SAFE_PARCEL(input.readBool, &hasBufferReleaseChannel); - if (hasBufferReleaseChannel) { - bufferReleaseChannel = std::make_shared(); - SAFE_PARCEL(input.readParcelable, bufferReleaseChannel.get()); - } return NO_ERROR; } @@ -730,10 +718,6 @@ void layer_state_t::merge(const layer_state_t& other) { if (other.what & eFlushJankData) { what |= eFlushJankData; } - if (other.what & eBufferReleaseChannelChanged) { - what |= eBufferReleaseChannelChanged; - bufferReleaseChannel = other.bufferReleaseChannel; - } if ((other.what & what) != other.what) { ALOGE("Unmerged SurfaceComposer Transaction properties. LayerState::merge needs updating? " "other.what=0x%" PRIX64 " what=0x%" PRIX64 " unmerged flags=0x%" PRIX64, @@ -813,7 +797,6 @@ uint64_t layer_state_t::diff(const layer_state_t& other) const { CHECK_DIFF(diff, eColorChanged, other, color.rgb); CHECK_DIFF(diff, eColorSpaceAgnosticChanged, other, colorSpaceAgnostic); CHECK_DIFF(diff, eDimmingEnabledChanged, other, dimmingEnabled); - if (other.what & eBufferReleaseChannelChanged) diff |= eBufferReleaseChannelChanged; return diff; } diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp index cdf57ffd61..e86e13d3ee 100644 --- a/libs/gui/SurfaceComposerClient.cpp +++ b/libs/gui/SurfaceComposerClient.cpp @@ -2395,22 +2395,6 @@ SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setDropI return *this; } -SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setBufferReleaseChannel( - const sp& sc, - const std::shared_ptr& channel) { - layer_state_t* s = getLayerState(sc); - if (!s) { - mStatus = BAD_INDEX; - return *this; - } - - s->what |= layer_state_t::eBufferReleaseChannelChanged; - s->bufferReleaseChannel = channel; - - registerSurfaceControlForCallback(sc); - return *this; -} - // --------------------------------------------------------------------------- DisplayState& SurfaceComposerClient::Transaction::getDisplayState(const sp& token) { diff --git a/libs/gui/include/gui/BLASTBufferQueue.h b/libs/gui/include/gui/BLASTBufferQueue.h index e9a350c301..0e1a505c69 100644 --- a/libs/gui/include/gui/BLASTBufferQueue.h +++ b/libs/gui/include/gui/BLASTBufferQueue.h @@ -19,7 +19,7 @@ #include #include -#include + #include #include @@ -28,6 +28,7 @@ #include #include +#include #include #include @@ -130,8 +131,6 @@ public: virtual ~BLASTBufferQueue(); - void onFirstRef() override; - private: friend class BLASTBufferQueueHelper; friend class BBQBufferQueueProducer; @@ -171,23 +170,11 @@ private: // BufferQueue internally allows 1 more than // the max to be acquired - int32_t mMaxAcquiredBuffers GUARDED_BY(mMutex) = 1; - int32_t mMaxDequeuedBuffers GUARDED_BY(mMutex) = 1; - static constexpr int32_t kMaxBufferCount = BufferQueueDefs::NUM_BUFFER_SLOTS; - - // mNumDequeued is an atomic instead of being guarded by mMutex so that it can be incremented in - // onFrameDequeued while avoiding lock contention. updateDequeueShouldBlockLocked is not called - // after mNumDequeued is incremented for this reason. This means mDequeueShouldBlock may be - // temporarily false when it should be true. This can happen if multiple threads are dequeuing - // buffers or if dequeueBuffers is called multiple times in a row without queuing a buffer in - // between. As mDequeueShouldBlock is only used for optimization, this is OK. - std::atomic_int mNumDequeued; + int32_t mMaxAcquiredBuffers = 1; + int32_t mNumFrameAvailable GUARDED_BY(mMutex) = 0; int32_t mNumAcquired GUARDED_BY(mMutex) = 0; - bool mAsyncMode GUARDED_BY(mMutex) = false; - bool mSharedBufferMode GUARDED_BY(mMutex) = false; - // A value used to identify if a producer has been changed for the same SurfaceControl. // This is needed to know when the frame number has been reset to make sure we don't // latch stale buffers and that we don't wait on barriers from an old producer. @@ -313,41 +300,6 @@ private: std::function mTransactionHangCallback; std::unordered_set mSyncedFrameNumbers GUARDED_BY(mMutex); - - class BufferReleaseReader { - public: - BufferReleaseReader(std::unique_ptr); - - BufferReleaseReader(const BufferReleaseReader&) = delete; - BufferReleaseReader& operator=(const BufferReleaseReader&) = delete; - - // Block until we can release a buffer. - // - // Returns: - // * OK if a ReleaseCallbackId and Fence were successfully read. - // * TIMED_OUT if the specified timeout was reached. - // * WOULD_BLOCK if the blocking read was interrupted by interruptBlockingRead. - // * UNKNOWN_ERROR if something went wrong. - status_t readBlocking(ReleaseCallbackId&, sp&, std::chrono::milliseconds timeout); - - status_t readNonBlocking(ReleaseCallbackId&, sp&); - - void interruptBlockingRead(); - - private: - std::mutex mMutex; - std::unique_ptr mEndpoint GUARDED_BY(mMutex); - android::base::unique_fd mEpollFd; - android::base::unique_fd mEventFd; - }; - - // BufferReleaseChannel is used to communicate buffer releases from SurfaceFlinger to - // the client. See BBQBufferQueueProducer::dequeueBuffer for details. - std::optional mBufferReleaseReader; - std::shared_ptr mBufferReleaseProducer; - - std::atomic_bool mDequeueShouldBlock{false}; - void updateDequeueShouldBlockLocked() REQUIRES(mMutex); }; } // namespace android diff --git a/libs/gui/include/gui/BufferReleaseChannel.h b/libs/gui/include/gui/BufferReleaseChannel.h deleted file mode 100644 index cb0b261240..0000000000 --- a/libs/gui/include/gui/BufferReleaseChannel.h +++ /dev/null @@ -1,127 +0,0 @@ -/* - * Copyright (C) 2024 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#pragma once - -#include - -#include -#include -#include - -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include - -namespace android::gui { - -/** - * IPC wrapper to pass release fences from SurfaceFlinger to apps via a local unix domain socket. - */ -class BufferReleaseChannel { -private: - class Endpoint { - public: - Endpoint(std::string name, android::base::unique_fd fd) - : mName(std::move(name)), mFd(std::move(fd)) {} - Endpoint() {} - - Endpoint(Endpoint&&) noexcept = default; - Endpoint& operator=(Endpoint&&) noexcept = default; - - Endpoint(const Endpoint&) = delete; - void operator=(const Endpoint&) = delete; - - const android::base::unique_fd& getFd() const { return mFd; } - - protected: - std::string mName; - android::base::unique_fd mFd; - }; - -public: - class ConsumerEndpoint : public Endpoint { - public: - ConsumerEndpoint(std::string name, android::base::unique_fd fd) - : Endpoint(std::move(name), std::move(fd)) {} - - /** - * Reads a release fence from the BufferReleaseChannel. - * - * Returns OK on success. - * Returns WOULD_BLOCK if there is no fence present. - * Other errors probably indicate that the channel is broken. - */ - status_t readReleaseFence(ReleaseCallbackId& outReleaseCallbackId, - sp& outReleaseFence); - - private: - std::vector mFlattenedBuffer; - }; - - class ProducerEndpoint : public Endpoint, public Parcelable { - public: - ProducerEndpoint(std::string name, android::base::unique_fd fd) - : Endpoint(std::move(name), std::move(fd)) {} - ProducerEndpoint() {} - - status_t readFromParcel(const android::Parcel* parcel) override; - status_t writeToParcel(android::Parcel* parcel) const override; - - status_t writeReleaseFence(const ReleaseCallbackId&, const sp& releaseFence); - - private: - std::vector mFlattenedBuffer; - }; - - /** - * Create two endpoints that make up the BufferReleaseChannel. - * - * Return OK on success. - */ - static status_t open(const std::string name, std::unique_ptr& outConsumer, - std::shared_ptr& outProducer); - - struct Message : public Flattenable { - ReleaseCallbackId releaseCallbackId; - sp releaseFence = Fence::NO_FENCE; - - Message() = default; - Message(ReleaseCallbackId releaseCallbackId, sp releaseFence) - : releaseCallbackId(releaseCallbackId), releaseFence(std::move(releaseFence)) {} - - // Flattenable protocol - size_t getFlattenedSize() const; - - size_t getFdCount() const { return releaseFence->getFdCount(); } - - status_t flatten(void*& buffer, size_t& size, int*& fds, size_t& count) const; - - status_t unflatten(void const*& buffer, size_t& size, int const*& fds, size_t& count); - - private: - size_t getPodSize() const; - }; -}; - -} // namespace android::gui diff --git a/libs/gui/include/gui/LayerState.h b/libs/gui/include/gui/LayerState.h index d41994589f..3fb1894585 100644 --- a/libs/gui/include/gui/LayerState.h +++ b/libs/gui/include/gui/LayerState.h @@ -34,7 +34,6 @@ #include #include -#include #include #include #include @@ -221,7 +220,6 @@ struct layer_state_t { eDropInputModeChanged = 0x8000'00000000, eExtendedRangeBrightnessChanged = 0x10000'00000000, eEdgeExtensionChanged = 0x20000'00000000, - eBufferReleaseChannelChanged = 0x40000'00000000, }; layer_state_t(); @@ -414,8 +412,6 @@ struct layer_state_t { TrustedPresentationThresholds trustedPresentationThresholds; TrustedPresentationListener trustedPresentationListener; - - std::shared_ptr bufferReleaseChannel; }; class ComposerState { diff --git a/libs/gui/include/gui/SurfaceComposerClient.h b/libs/gui/include/gui/SurfaceComposerClient.h index 8c1d6443c5..95574ee30e 100644 --- a/libs/gui/include/gui/SurfaceComposerClient.h +++ b/libs/gui/include/gui/SurfaceComposerClient.h @@ -45,7 +45,6 @@ #include #include -#include #include #include #include @@ -764,10 +763,6 @@ public: const Rect& destinationFrame); Transaction& setDropInputMode(const sp& sc, gui::DropInputMode mode); - Transaction& setBufferReleaseChannel( - const sp& sc, - const std::shared_ptr& channel); - status_t setDisplaySurface(const sp& token, const sp& bufferProducer); diff --git a/libs/gui/tests/Android.bp b/libs/gui/tests/Android.bp index daaddfbc15..ea8acbbb72 100644 --- a/libs/gui/tests/Android.bp +++ b/libs/gui/tests/Android.bp @@ -32,7 +32,6 @@ cc_test { "BLASTBufferQueue_test.cpp", "BufferItemConsumer_test.cpp", "BufferQueue_test.cpp", - "BufferReleaseChannel_test.cpp", "Choreographer_test.cpp", "CompositorTiming_test.cpp", "CpuConsumer_test.cpp", diff --git a/libs/gui/tests/BufferReleaseChannel_test.cpp b/libs/gui/tests/BufferReleaseChannel_test.cpp deleted file mode 100644 index f3e962c192..0000000000 --- a/libs/gui/tests/BufferReleaseChannel_test.cpp +++ /dev/null @@ -1,121 +0,0 @@ -/* - * Copyright (C) 2024 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#include -#include - -#include -#include - -using namespace std::string_literals; -using android::gui::BufferReleaseChannel; - -namespace android { - -namespace { - -// Helper function to check if two file descriptors point to the same file. -bool is_same_file(int fd1, int fd2) { - struct stat stat1; - if (fstat(fd1, &stat1) != 0) { - return false; - } - struct stat stat2; - if (fstat(fd2, &stat2) != 0) { - return false; - } - return (stat1.st_dev == stat2.st_dev) && (stat1.st_ino == stat2.st_ino); -} - -} // namespace - -TEST(BufferReleaseChannelTest, MessageFlattenable) { - ReleaseCallbackId releaseCallbackId{1, 2}; - sp releaseFence = sp::make(memfd_create("fake-fence-fd", 0)); - - std::vector dataBuffer; - std::vector fdBuffer; - - // Verify that we can flatten a message - { - BufferReleaseChannel::Message message{releaseCallbackId, releaseFence}; - - dataBuffer.resize(message.getFlattenedSize()); - void* dataPtr = dataBuffer.data(); - size_t dataSize = dataBuffer.size(); - - fdBuffer.resize(message.getFdCount()); - int* fdPtr = fdBuffer.data(); - size_t fdSize = fdBuffer.size(); - - ASSERT_EQ(OK, message.flatten(dataPtr, dataSize, fdPtr, fdSize)); - - // Fence's unique_fd uses fdsan to check ownership of the file descriptor. Normally the file - // descriptor is passed through the Unix socket and duplicated (and sent to another process) - // so there's no problem with duplicate file descriptor ownership. For this unit test, we - // need to set up a duplicate file descriptor to avoid crashing due to duplicate ownership. - ASSERT_EQ(releaseFence->get(), fdBuffer[0]); - fdBuffer[0] = message.releaseFence->dup(); - } - - // Verify that we can unflatten a message - { - BufferReleaseChannel::Message message; - - const void* dataPtr = dataBuffer.data(); - size_t dataSize = dataBuffer.size(); - - const int* fdPtr = fdBuffer.data(); - size_t fdSize = fdBuffer.size(); - - ASSERT_EQ(OK, message.unflatten(dataPtr, dataSize, fdPtr, fdSize)); - ASSERT_EQ(releaseCallbackId, message.releaseCallbackId); - ASSERT_TRUE(is_same_file(releaseFence->get(), message.releaseFence->get())); - } -} - -// Verify that the BufferReleaseChannel consume returns WOULD_BLOCK when there's no message -// available. -TEST(BufferReleaseChannelTest, ConsumerEndpointIsNonBlocking) { - std::unique_ptr consumer; - std::shared_ptr producer; - ASSERT_EQ(OK, BufferReleaseChannel::open("test-channel"s, consumer, producer)); - - ReleaseCallbackId releaseCallbackId; - sp releaseFence; - ASSERT_EQ(WOULD_BLOCK, consumer->readReleaseFence(releaseCallbackId, releaseFence)); -} - -// Verify that we can write a message to the BufferReleaseChannel producer and read that message -// using the BufferReleaseChannel consumer. -TEST(BufferReleaseChannelTest, ProduceAndConsume) { - std::unique_ptr consumer; - std::shared_ptr producer; - ASSERT_EQ(OK, BufferReleaseChannel::open("test-channel"s, consumer, producer)); - - ReleaseCallbackId producerReleaseCallbackId{1, 2}; - sp producerReleaseFence = sp::make(memfd_create("fake-fence-fd", 0)); - ASSERT_EQ(OK, producer->writeReleaseFence(producerReleaseCallbackId, producerReleaseFence)); - - ReleaseCallbackId consumerReleaseCallbackId; - sp consumerReleaseFence; - ASSERT_EQ(OK, consumer->readReleaseFence(consumerReleaseCallbackId, consumerReleaseFence)); - - ASSERT_EQ(producerReleaseCallbackId, consumerReleaseCallbackId); - ASSERT_TRUE(is_same_file(producerReleaseFence->get(), consumerReleaseFence->get())); -} - -} // namespace android diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp index 0682fdb639..0eb6cc32c5 100644 --- a/services/surfaceflinger/Layer.cpp +++ b/services/surfaceflinger/Layer.cpp @@ -2775,14 +2775,11 @@ void Layer::callReleaseBufferCallback(const sp& l return; } SFTRACE_FORMAT_INSTANT("callReleaseBufferCallback %s - %" PRIu64, getDebugName(), framenumber); - ReleaseCallbackId callbackId{buffer->getId(), framenumber}; - const sp& fence = releaseFence ? releaseFence : Fence::NO_FENCE; uint32_t currentMaxAcquiredBufferCount = mFlinger->getMaxAcquiredBufferCountForCurrentRefreshRate(mOwnerUid); - listener->onReleaseBuffer(callbackId, fence, currentMaxAcquiredBufferCount); - if (mBufferReleaseChannel) { - mBufferReleaseChannel->writeReleaseFence(callbackId, fence); - } + listener->onReleaseBuffer({buffer->getId(), framenumber}, + releaseFence ? releaseFence : Fence::NO_FENCE, + currentMaxAcquiredBufferCount); } sp Layer::findCallbackHandle() { @@ -2900,7 +2897,6 @@ void Layer::onLayerDisplayed(ftl::SharedFuture futureFenceResult, void Layer::releasePendingBuffer(nsecs_t dequeueReadyTime) { for (const auto& handle : mDrawingState.callbackHandles) { - handle->bufferReleaseChannel = mBufferReleaseChannel; handle->transformHint = mTransformHint; handle->dequeueReadyTime = dequeueReadyTime; handle->currentMaxAcquiredBufferCount = @@ -4372,11 +4368,6 @@ bool Layer::setTrustedPresentationInfo(TrustedPresentationThresholds const& thre return haveTrustedPresentationListener; } -void Layer::setBufferReleaseChannel( - const std::shared_ptr& channel) { - mBufferReleaseChannel = channel; -} - void Layer::updateLastLatchTime(nsecs_t latchTime) { mLastLatchTime = latchTime; } diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h index d4707388b2..52f169ebee 100644 --- a/services/surfaceflinger/Layer.h +++ b/services/surfaceflinger/Layer.h @@ -550,7 +550,6 @@ public: }; BufferInfo mBufferInfo; - std::shared_ptr mBufferReleaseChannel; // implements compositionengine::LayerFE const compositionengine::LayerFECompositionState* getCompositionState() const; @@ -814,8 +813,6 @@ public: bool setTrustedPresentationInfo(TrustedPresentationThresholds const& thresholds, TrustedPresentationListener const& listener); - void setBufferReleaseChannel( - const std::shared_ptr& channel); // Creates a new handle each time, so we only expect // this to be called once. diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index f61763880e..84553cab39 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -5789,10 +5789,6 @@ uint32_t SurfaceFlinger::updateLayerCallbacksAndStats(const FrameTimelineInfo& f } } - if (what & layer_state_t::eBufferReleaseChannelChanged) { - layer->setBufferReleaseChannel(s.bufferReleaseChannel); - } - const auto& requestedLayerState = mLayerLifecycleManager.getLayerFromId(layer->getSequence()); bool willPresentCurrentTransaction = requestedLayerState && (requestedLayerState->hasReadyFrame() || diff --git a/services/surfaceflinger/TransactionCallbackInvoker.cpp b/services/surfaceflinger/TransactionCallbackInvoker.cpp index 9ea0f5f4ca..881bf35b58 100644 --- a/services/surfaceflinger/TransactionCallbackInvoker.cpp +++ b/services/surfaceflinger/TransactionCallbackInvoker.cpp @@ -149,12 +149,6 @@ status_t TransactionCallbackInvoker::addCallbackHandle(const sp& handle->transformHint, handle->currentMaxAcquiredBufferCount, eventStats, handle->previousReleaseCallbackId); - if (handle->bufferReleaseChannel && - handle->previousReleaseCallbackId != ReleaseCallbackId::INVALID_ID) { - mBufferReleases.emplace_back(handle->bufferReleaseChannel, - handle->previousReleaseCallbackId, - handle->previousReleaseFence); - } } return NO_ERROR; } @@ -164,11 +158,6 @@ void TransactionCallbackInvoker::addPresentFence(sp presentFence) { } void TransactionCallbackInvoker::sendCallbacks(bool onCommitOnly) { - for (const auto& bufferRelease : mBufferReleases) { - bufferRelease.channel->writeReleaseFence(bufferRelease.callbackId, bufferRelease.fence); - } - mBufferReleases.clear(); - // For each listener auto completedTransactionsItr = mCompletedTransactions.begin(); ftl::SmallVector listenerStatsToSend; diff --git a/services/surfaceflinger/TransactionCallbackInvoker.h b/services/surfaceflinger/TransactionCallbackInvoker.h index f74f9644b6..7853a9f359 100644 --- a/services/surfaceflinger/TransactionCallbackInvoker.h +++ b/services/surfaceflinger/TransactionCallbackInvoker.h @@ -28,7 +28,6 @@ #include #include #include -#include #include #include #include @@ -60,7 +59,6 @@ public: uint64_t frameNumber = 0; uint64_t previousFrameNumber = 0; ReleaseCallbackId previousReleaseCallbackId = ReleaseCallbackId::INVALID_ID; - std::shared_ptr bufferReleaseChannel; }; class TransactionCallbackInvoker { @@ -88,13 +86,6 @@ private: std::unordered_map, std::deque, IListenerHash> mCompletedTransactions; - struct BufferRelease { - std::shared_ptr channel; - ReleaseCallbackId callbackId; - sp fence; - }; - std::vector mBufferReleases; - sp mPresentFence; }; -- cgit v1.2.3-59-g8ed1b From f693bcf2ee13391f99b8ede5562047981e1f5968 Mon Sep 17 00:00:00 2001 From: Patrick Williams Date: Fri, 2 Aug 2024 09:55:23 -0500 Subject: Add BufferReleaseChannel BufferReleaseChannel will be used to communicate buffer releases from SurfaceFlinger to BLASTBufferQueue. Bug: 294133380 Flag: com.android.graphics.libgui.flags.buffer_release_channel Test: BufferReleaseChannelTest Change-Id: Ic38e8eefc96abc0b2bbe780115b7628413e8b829 --- libs/gui/Android.bp | 1 + libs/gui/BufferReleaseChannel.cpp | 358 +++++++++++++++++++++++++++ libs/gui/LayerState.cpp | 20 +- libs/gui/SurfaceComposerClient.cpp | 16 ++ libs/gui/include/gui/BufferReleaseChannel.h | 125 ++++++++++ libs/gui/include/gui/LayerState.h | 4 + libs/gui/include/gui/SurfaceComposerClient.h | 5 + libs/gui/tests/Android.bp | 1 + libs/gui/tests/BufferReleaseChannel_test.cpp | 138 +++++++++++ 9 files changed, 667 insertions(+), 1 deletion(-) create mode 100644 libs/gui/BufferReleaseChannel.cpp create mode 100644 libs/gui/include/gui/BufferReleaseChannel.h create mode 100644 libs/gui/tests/BufferReleaseChannel_test.cpp (limited to 'libs/gui/LayerState.cpp') diff --git a/libs/gui/Android.bp b/libs/gui/Android.bp index 51d2e5305a..1243b214d3 100644 --- a/libs/gui/Android.bp +++ b/libs/gui/Android.bp @@ -255,6 +255,7 @@ filegroup { "BitTube.cpp", "BLASTBufferQueue.cpp", "BufferItemConsumer.cpp", + "BufferReleaseChannel.cpp", "Choreographer.cpp", "CompositorTiming.cpp", "ConsumerBase.cpp", diff --git a/libs/gui/BufferReleaseChannel.cpp b/libs/gui/BufferReleaseChannel.cpp new file mode 100644 index 0000000000..27367aa83f --- /dev/null +++ b/libs/gui/BufferReleaseChannel.cpp @@ -0,0 +1,358 @@ +/* + * Copyright (C) 2024 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#define LOG_TAG "BufferReleaseChannel" + +#include +#include +#include + +#include +#include +#include +#include + +#include +#include + +using android::base::Result; + +namespace android::gui { + +namespace { + +template +static void readAligned(const void*& buffer, size_t& size, T& value) { + size -= FlattenableUtils::align(buffer); + FlattenableUtils::read(buffer, size, value); +} + +template +static void writeAligned(void*& buffer, size_t& size, T value) { + size -= FlattenableUtils::align(buffer); + FlattenableUtils::write(buffer, size, value); +} + +template +static void addAligned(size_t& size, T /* value */) { + size = FlattenableUtils::align(size); + size += sizeof(T); +} + +template +static inline constexpr uint32_t low32(const T n) { + return static_cast(static_cast(n)); +} + +template +static inline constexpr uint32_t high32(const T n) { + return static_cast(static_cast(n) >> 32); +} + +template +static inline constexpr T to64(const uint32_t lo, const uint32_t hi) { + return static_cast(static_cast(hi) << 32 | lo); +} + +} // namespace + +size_t BufferReleaseChannel::Message::getPodSize() const { + size_t size = 0; + addAligned(size, low32(releaseCallbackId.bufferId)); + addAligned(size, high32(releaseCallbackId.bufferId)); + addAligned(size, low32(releaseCallbackId.framenumber)); + addAligned(size, high32(releaseCallbackId.framenumber)); + addAligned(size, maxAcquiredBufferCount); + return size; +} + +size_t BufferReleaseChannel::Message::getFlattenedSize() const { + size_t size = releaseFence->getFlattenedSize(); + size = FlattenableUtils::align<4>(size); + size += getPodSize(); + return size; +} + +status_t BufferReleaseChannel::Message::flatten(void*& buffer, size_t& size, int*& fds, + size_t& count) const { + if (status_t err = releaseFence->flatten(buffer, size, fds, count); err != OK) { + return err; + } + size -= FlattenableUtils::align<4>(buffer); + + // Check we still have enough space + if (size < getPodSize()) { + return NO_MEMORY; + } + + writeAligned(buffer, size, low32(releaseCallbackId.bufferId)); + writeAligned(buffer, size, high32(releaseCallbackId.bufferId)); + writeAligned(buffer, size, low32(releaseCallbackId.framenumber)); + writeAligned(buffer, size, high32(releaseCallbackId.framenumber)); + writeAligned(buffer, size, maxAcquiredBufferCount); + return OK; +} + +status_t BufferReleaseChannel::Message::unflatten(void const*& buffer, size_t& size, + int const*& fds, size_t& count) { + releaseFence = new Fence(); + if (status_t err = releaseFence->unflatten(buffer, size, fds, count); err != OK) { + return err; + } + size -= FlattenableUtils::align<4>(buffer); + + // Check we still have enough space + if (size < getPodSize()) { + return OK; + } + + uint32_t bufferIdLo = 0, bufferIdHi = 0; + uint32_t frameNumberLo = 0, frameNumberHi = 0; + + readAligned(buffer, size, bufferIdLo); + readAligned(buffer, size, bufferIdHi); + releaseCallbackId.bufferId = to64(bufferIdLo, bufferIdHi); + readAligned(buffer, size, frameNumberLo); + readAligned(buffer, size, frameNumberHi); + releaseCallbackId.framenumber = to64(frameNumberLo, frameNumberHi); + readAligned(buffer, size, maxAcquiredBufferCount); + + return OK; +} + +status_t BufferReleaseChannel::ConsumerEndpoint::readReleaseFence( + ReleaseCallbackId& outReleaseCallbackId, sp& outReleaseFence, + uint32_t& outMaxAcquiredBufferCount) { + Message message; + mFlattenedBuffer.resize(message.getFlattenedSize()); + std::array controlMessageBuffer; + + iovec iov{ + .iov_base = mFlattenedBuffer.data(), + .iov_len = mFlattenedBuffer.size(), + }; + + msghdr msg{ + .msg_iov = &iov, + .msg_iovlen = 1, + .msg_control = controlMessageBuffer.data(), + .msg_controllen = controlMessageBuffer.size(), + }; + + int result; + do { + result = recvmsg(mFd, &msg, 0); + } while (result == -1 && errno == EINTR); + if (result == -1) { + if (errno == EWOULDBLOCK || errno == EAGAIN) { + return WOULD_BLOCK; + } + ALOGE("Error reading release fence from socket: error %#x (%s)", errno, strerror(errno)); + return UNKNOWN_ERROR; + } + + if (msg.msg_iovlen != 1) { + ALOGE("Error reading release fence from socket: bad data length"); + return UNKNOWN_ERROR; + } + + if (msg.msg_controllen % sizeof(int) != 0) { + ALOGE("Error reading release fence from socket: bad fd length"); + return UNKNOWN_ERROR; + } + + size_t dataLen = msg.msg_iov->iov_len; + const void* data = static_cast(msg.msg_iov->iov_base); + if (!data) { + ALOGE("Error reading release fence from socket: no buffer data"); + return UNKNOWN_ERROR; + } + + size_t fdCount = 0; + const int* fdData = nullptr; + if (cmsghdr* cmsg = CMSG_FIRSTHDR(&msg)) { + fdData = reinterpret_cast(CMSG_DATA(cmsg)); + fdCount = (cmsg->cmsg_len - CMSG_LEN(0)) / sizeof(int); + } + + if (status_t err = message.unflatten(data, dataLen, fdData, fdCount); err != OK) { + return err; + } + + outReleaseCallbackId = message.releaseCallbackId; + outReleaseFence = std::move(message.releaseFence); + outMaxAcquiredBufferCount = message.maxAcquiredBufferCount; + + return OK; +} + +int BufferReleaseChannel::ProducerEndpoint::writeReleaseFence(const ReleaseCallbackId& callbackId, + const sp& fence, + uint32_t maxAcquiredBufferCount) { + Message message{callbackId, fence ? fence : Fence::NO_FENCE, maxAcquiredBufferCount}; + mFlattenedBuffer.resize(message.getFlattenedSize()); + int flattenedFd; + { + // Make copies of needed items since flatten modifies them, and we don't + // want to send anything if there's an error during flatten. + void* flattenedBufferPtr = mFlattenedBuffer.data(); + size_t flattenedBufferSize = mFlattenedBuffer.size(); + int* flattenedFdPtr = &flattenedFd; + size_t flattenedFdCount = 1; + if (status_t err = message.flatten(flattenedBufferPtr, flattenedBufferSize, flattenedFdPtr, + flattenedFdCount); + err != OK) { + ALOGE("Failed to flatten BufferReleaseChannel message."); + return err; + } + } + + iovec iov{ + .iov_base = mFlattenedBuffer.data(), + .iov_len = mFlattenedBuffer.size(), + }; + + msghdr msg{ + .msg_iov = &iov, + .msg_iovlen = 1, + }; + + std::array controlMessageBuffer; + if (fence && fence->isValid()) { + msg.msg_control = controlMessageBuffer.data(); + msg.msg_controllen = controlMessageBuffer.size(); + + cmsghdr* cmsg = CMSG_FIRSTHDR(&msg); + cmsg->cmsg_level = SOL_SOCKET; + cmsg->cmsg_type = SCM_RIGHTS; + cmsg->cmsg_len = CMSG_LEN(sizeof(int)); + memcpy(CMSG_DATA(cmsg), &flattenedFd, sizeof(int)); + } + + int result; + do { + result = sendmsg(mFd, &msg, 0); + } while (result == -1 && errno == EINTR); + if (result == -1) { + ALOGD("Error writing release fence to socket: error %#x (%s)", errno, strerror(errno)); + return -errno; + } + + return OK; +} + +status_t BufferReleaseChannel::ProducerEndpoint::readFromParcel(const android::Parcel* parcel) { + if (!parcel) return STATUS_BAD_VALUE; + SAFE_PARCEL(parcel->readUtf8FromUtf16, &mName); + SAFE_PARCEL(parcel->readUniqueFileDescriptor, &mFd); + return STATUS_OK; +} + +status_t BufferReleaseChannel::ProducerEndpoint::writeToParcel(android::Parcel* parcel) const { + if (!parcel) return STATUS_BAD_VALUE; + SAFE_PARCEL(parcel->writeUtf8AsUtf16, mName); + SAFE_PARCEL(parcel->writeUniqueFileDescriptor, mFd); + return STATUS_OK; +} + +status_t BufferReleaseChannel::open(std::string name, + std::unique_ptr& outConsumer, + std::shared_ptr& outProducer) { + outConsumer.reset(); + outProducer.reset(); + + int sockets[2]; + if (socketpair(AF_UNIX, SOCK_SEQPACKET, 0, sockets)) { + ALOGE("[%s] Failed to create socket pair. errorno=%d message='%s'", name.c_str(), errno, + strerror(errno)); + return -errno; + } + + android::base::unique_fd consumerFd(sockets[0]); + android::base::unique_fd producerFd(sockets[1]); + + // Socket buffer size. The default is typically about 128KB, which is much larger than + // we really need. + size_t bufferSize = 32 * 1024; + if (setsockopt(consumerFd.get(), SOL_SOCKET, SO_SNDBUF, &bufferSize, sizeof(bufferSize)) == + -1) { + ALOGE("[%s] Failed to set consumer socket send buffer size. errno=%d message='%s'", + name.c_str(), errno, strerror(errno)); + return -errno; + } + if (setsockopt(consumerFd.get(), SOL_SOCKET, SO_RCVBUF, &bufferSize, sizeof(bufferSize)) == + -1) { + ALOGE("[%s] Failed to set consumer socket receive buffer size. errno=%d " + "message='%s'", + name.c_str(), errno, strerror(errno)); + return -errno; + } + if (setsockopt(producerFd.get(), SOL_SOCKET, SO_SNDBUF, &bufferSize, sizeof(bufferSize)) == + -1) { + ALOGE("[%s] Failed to set producer socket send buffer size. errno=%d message='%s'", + name.c_str(), errno, strerror(errno)); + return -errno; + } + if (setsockopt(producerFd.get(), SOL_SOCKET, SO_RCVBUF, &bufferSize, sizeof(bufferSize)) == + -1) { + ALOGE("[%s] Failed to set producer socket receive buffer size. errno=%d " + "message='%s'", + name.c_str(), errno, strerror(errno)); + return -errno; + } + + // Configure the consumer socket to be non-blocking. + int flags = fcntl(consumerFd.get(), F_GETFL, 0); + if (flags == -1) { + ALOGE("[%s] Failed to get consumer socket flags. errno=%d message='%s'", name.c_str(), + errno, strerror(errno)); + return -errno; + } + if (fcntl(consumerFd.get(), F_SETFL, flags | O_NONBLOCK) == -1) { + ALOGE("[%s] Failed to set consumer socket to non-blocking mode. errno=%d " + "message='%s'", + name.c_str(), errno, strerror(errno)); + return -errno; + } + + // Configure a timeout for the producer socket. + const timeval timeout{.tv_sec = 1, .tv_usec = 0}; + if (setsockopt(producerFd.get(), SOL_SOCKET, SO_RCVTIMEO, &timeout, sizeof(timeval)) == -1) { + ALOGE("[%s] Failed to set producer socket timeout. errno=%d message='%s'", name.c_str(), + errno, strerror(errno)); + return -errno; + } + + // Make the consumer read-only + if (shutdown(consumerFd.get(), SHUT_WR) == -1) { + ALOGE("[%s] Failed to shutdown writing on consumer socket. errno=%d message='%s'", + name.c_str(), errno, strerror(errno)); + return -errno; + } + + // Make the producer write-only + if (shutdown(producerFd.get(), SHUT_RD) == -1) { + ALOGE("[%s] Failed to shutdown reading on producer socket. errno=%d message='%s'", + name.c_str(), errno, strerror(errno)); + return -errno; + } + + outConsumer = std::make_unique(name, std::move(consumerFd)); + outProducer = std::make_shared(std::move(name), std::move(producerFd)); + return STATUS_OK; +} + +} // namespace android::gui \ No newline at end of file diff --git a/libs/gui/LayerState.cpp b/libs/gui/LayerState.cpp index 307ae3990e..0714fd9501 100644 --- a/libs/gui/LayerState.cpp +++ b/libs/gui/LayerState.cpp @@ -17,7 +17,6 @@ #define LOG_TAG "LayerState" #include -#include #include #include @@ -194,6 +193,13 @@ status_t layer_state_t::write(Parcel& output) const SAFE_PARCEL(output.writeFloat, currentHdrSdrRatio); SAFE_PARCEL(output.writeFloat, desiredHdrSdrRatio); SAFE_PARCEL(output.writeInt32, static_cast(cachingHint)); + + const bool hasBufferReleaseChannel = (bufferReleaseChannel != nullptr); + SAFE_PARCEL(output.writeBool, hasBufferReleaseChannel); + if (hasBufferReleaseChannel) { + SAFE_PARCEL(output.writeParcelable, *bufferReleaseChannel); + } + return NO_ERROR; } @@ -339,6 +345,13 @@ status_t layer_state_t::read(const Parcel& input) SAFE_PARCEL(input.readInt32, &tmpInt32); cachingHint = static_cast(tmpInt32); + bool hasBufferReleaseChannel; + SAFE_PARCEL(input.readBool, &hasBufferReleaseChannel); + if (hasBufferReleaseChannel) { + bufferReleaseChannel = std::make_shared(); + SAFE_PARCEL(input.readParcelable, bufferReleaseChannel.get()); + } + return NO_ERROR; } @@ -718,6 +731,10 @@ void layer_state_t::merge(const layer_state_t& other) { if (other.what & eFlushJankData) { what |= eFlushJankData; } + if (other.what & eBufferReleaseChannelChanged) { + what |= eBufferReleaseChannelChanged; + bufferReleaseChannel = other.bufferReleaseChannel; + } if ((other.what & what) != other.what) { ALOGE("Unmerged SurfaceComposer Transaction properties. LayerState::merge needs updating? " "other.what=0x%" PRIX64 " what=0x%" PRIX64 " unmerged flags=0x%" PRIX64, @@ -797,6 +814,7 @@ uint64_t layer_state_t::diff(const layer_state_t& other) const { CHECK_DIFF(diff, eColorChanged, other, color.rgb); CHECK_DIFF(diff, eColorSpaceAgnosticChanged, other, colorSpaceAgnostic); CHECK_DIFF(diff, eDimmingEnabledChanged, other, dimmingEnabled); + if (other.what & eBufferReleaseChannelChanged) diff |= eBufferReleaseChannelChanged; return diff; } diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp index f663600f9e..b5d9366185 100644 --- a/libs/gui/SurfaceComposerClient.cpp +++ b/libs/gui/SurfaceComposerClient.cpp @@ -2390,6 +2390,22 @@ SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setDropI return *this; } +SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setBufferReleaseChannel( + const sp& sc, + const std::shared_ptr& channel) { + layer_state_t* s = getLayerState(sc); + if (!s) { + mStatus = BAD_INDEX; + return *this; + } + + s->what |= layer_state_t::eBufferReleaseChannelChanged; + s->bufferReleaseChannel = channel; + + registerSurfaceControlForCallback(sc); + return *this; +} + // --------------------------------------------------------------------------- DisplayState& SurfaceComposerClient::Transaction::getDisplayState(const sp& token) { diff --git a/libs/gui/include/gui/BufferReleaseChannel.h b/libs/gui/include/gui/BufferReleaseChannel.h new file mode 100644 index 0000000000..51fe0b6fab --- /dev/null +++ b/libs/gui/include/gui/BufferReleaseChannel.h @@ -0,0 +1,125 @@ +/* + * Copyright (C) 2024 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include +#include + +#include + +#include +#include +#include +#include + +namespace android::gui { + +/** + * IPC wrapper to pass release fences from SurfaceFlinger to apps via a local unix domain socket. + */ +class BufferReleaseChannel { +private: + class Endpoint { + public: + Endpoint(std::string name, android::base::unique_fd fd) + : mName(std::move(name)), mFd(std::move(fd)) {} + Endpoint() {} + + Endpoint(Endpoint&&) noexcept = default; + Endpoint& operator=(Endpoint&&) noexcept = default; + + Endpoint(const Endpoint&) = delete; + void operator=(const Endpoint&) = delete; + + const android::base::unique_fd& getFd() const { return mFd; } + + protected: + std::string mName; + android::base::unique_fd mFd; + }; + +public: + class ConsumerEndpoint : public Endpoint { + public: + ConsumerEndpoint(std::string name, android::base::unique_fd fd) + : Endpoint(std::move(name), std::move(fd)) {} + + /** + * Reads a release fence from the BufferReleaseChannel. + * + * Returns OK on success. + * Returns WOULD_BLOCK if there is no fence present. + * Other errors probably indicate that the channel is broken. + */ + status_t readReleaseFence(ReleaseCallbackId& outReleaseCallbackId, + sp& outReleaseFence, uint32_t& maxAcquiredBufferCount); + + private: + std::vector mFlattenedBuffer; + }; + + class ProducerEndpoint : public Endpoint, public Parcelable { + public: + ProducerEndpoint(std::string name, android::base::unique_fd fd) + : Endpoint(std::move(name), std::move(fd)) {} + ProducerEndpoint() {} + + status_t readFromParcel(const android::Parcel* parcel) override; + status_t writeToParcel(android::Parcel* parcel) const override; + + status_t writeReleaseFence(const ReleaseCallbackId&, const sp& releaseFence, + uint32_t maxAcquiredBufferCount); + + private: + std::vector mFlattenedBuffer; + }; + + /** + * Create two endpoints that make up the BufferReleaseChannel. + * + * Return OK on success. + */ + static status_t open(const std::string name, std::unique_ptr& outConsumer, + std::shared_ptr& outProducer); + + struct Message : public Flattenable { + ReleaseCallbackId releaseCallbackId; + sp releaseFence = Fence::NO_FENCE; + uint32_t maxAcquiredBufferCount; + + Message() = default; + Message(ReleaseCallbackId releaseCallbackId, sp releaseFence, + uint32_t maxAcquiredBufferCount) + : releaseCallbackId{releaseCallbackId}, + releaseFence{std::move(releaseFence)}, + maxAcquiredBufferCount{maxAcquiredBufferCount} {} + + // Flattenable protocol + size_t getFlattenedSize() const; + + size_t getFdCount() const { return releaseFence->getFdCount(); } + + status_t flatten(void*& buffer, size_t& size, int*& fds, size_t& count) const; + + status_t unflatten(void const*& buffer, size_t& size, int const*& fds, size_t& count); + + private: + size_t getPodSize() const; + }; +}; + +} // namespace android::gui diff --git a/libs/gui/include/gui/LayerState.h b/libs/gui/include/gui/LayerState.h index 3fb1894585..d41994589f 100644 --- a/libs/gui/include/gui/LayerState.h +++ b/libs/gui/include/gui/LayerState.h @@ -34,6 +34,7 @@ #include #include +#include #include #include #include @@ -220,6 +221,7 @@ struct layer_state_t { eDropInputModeChanged = 0x8000'00000000, eExtendedRangeBrightnessChanged = 0x10000'00000000, eEdgeExtensionChanged = 0x20000'00000000, + eBufferReleaseChannelChanged = 0x40000'00000000, }; layer_state_t(); @@ -412,6 +414,8 @@ struct layer_state_t { TrustedPresentationThresholds trustedPresentationThresholds; TrustedPresentationListener trustedPresentationListener; + + std::shared_ptr bufferReleaseChannel; }; class ComposerState { diff --git a/libs/gui/include/gui/SurfaceComposerClient.h b/libs/gui/include/gui/SurfaceComposerClient.h index 61a65bb29f..4f9af16826 100644 --- a/libs/gui/include/gui/SurfaceComposerClient.h +++ b/libs/gui/include/gui/SurfaceComposerClient.h @@ -45,6 +45,7 @@ #include #include +#include #include #include #include @@ -762,6 +763,10 @@ public: const Rect& destinationFrame); Transaction& setDropInputMode(const sp& sc, gui::DropInputMode mode); + Transaction& setBufferReleaseChannel( + const sp& sc, + const std::shared_ptr& channel); + status_t setDisplaySurface(const sp& token, const sp& bufferProducer); diff --git a/libs/gui/tests/Android.bp b/libs/gui/tests/Android.bp index b342a7db0d..9558edab87 100644 --- a/libs/gui/tests/Android.bp +++ b/libs/gui/tests/Android.bp @@ -33,6 +33,7 @@ cc_test { "BLASTBufferQueue_test.cpp", "BufferItemConsumer_test.cpp", "BufferQueue_test.cpp", + "BufferReleaseChannel_test.cpp", "Choreographer_test.cpp", "CompositorTiming_test.cpp", "CpuConsumer_test.cpp", diff --git a/libs/gui/tests/BufferReleaseChannel_test.cpp b/libs/gui/tests/BufferReleaseChannel_test.cpp new file mode 100644 index 0000000000..11d122b525 --- /dev/null +++ b/libs/gui/tests/BufferReleaseChannel_test.cpp @@ -0,0 +1,138 @@ +/* + * Copyright (C) 2024 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include +#include + +#include +#include + +using namespace std::string_literals; +using android::gui::BufferReleaseChannel; + +namespace android { + +namespace { + +// Helper function to check if two file descriptors point to the same file. +bool is_same_file(int fd1, int fd2) { + struct stat stat1; + if (fstat(fd1, &stat1) != 0) { + return false; + } + struct stat stat2; + if (fstat(fd2, &stat2) != 0) { + return false; + } + return (stat1.st_dev == stat2.st_dev) && (stat1.st_ino == stat2.st_ino); +} + +} // namespace + +TEST(BufferReleaseChannelTest, MessageFlattenable) { + ReleaseCallbackId releaseCallbackId{1, 2}; + sp releaseFence = sp::make(memfd_create("fake-fence-fd", 0)); + uint32_t maxAcquiredBufferCount = 5; + + std::vector dataBuffer; + std::vector fdBuffer; + + // Verify that we can flatten a message + { + BufferReleaseChannel::Message message{releaseCallbackId, releaseFence, + maxAcquiredBufferCount}; + + dataBuffer.resize(message.getFlattenedSize()); + void* dataPtr = dataBuffer.data(); + size_t dataSize = dataBuffer.size(); + + fdBuffer.resize(message.getFdCount()); + int* fdPtr = fdBuffer.data(); + size_t fdSize = fdBuffer.size(); + + ASSERT_EQ(OK, message.flatten(dataPtr, dataSize, fdPtr, fdSize)); + + // Fence's unique_fd uses fdsan to check ownership of the file descriptor. Normally the file + // descriptor is passed through the Unix socket and duplicated (and sent to another process) + // so there's no problem with duplicate file descriptor ownership. For this unit test, we + // need to set up a duplicate file descriptor to avoid crashing due to duplicate ownership. + ASSERT_EQ(releaseFence->get(), fdBuffer[0]); + fdBuffer[0] = message.releaseFence->dup(); + } + + // Verify that we can unflatten a message + { + BufferReleaseChannel::Message message; + + const void* dataPtr = dataBuffer.data(); + size_t dataSize = dataBuffer.size(); + + const int* fdPtr = fdBuffer.data(); + size_t fdSize = fdBuffer.size(); + + ASSERT_EQ(OK, message.unflatten(dataPtr, dataSize, fdPtr, fdSize)); + ASSERT_EQ(releaseCallbackId, message.releaseCallbackId); + ASSERT_TRUE(is_same_file(releaseFence->get(), message.releaseFence->get())); + ASSERT_EQ(maxAcquiredBufferCount, message.maxAcquiredBufferCount); + } +} + +// Verify that the BufferReleaseChannel consume returns WOULD_BLOCK when there's no message +// available. +TEST(BufferReleaseChannelTest, ConsumerEndpointIsNonBlocking) { + std::unique_ptr consumer; + std::shared_ptr producer; + ASSERT_EQ(OK, BufferReleaseChannel::open("test-channel"s, consumer, producer)); + + ReleaseCallbackId releaseCallbackId; + sp releaseFence; + uint32_t maxAcquiredBufferCount; + ASSERT_EQ(WOULD_BLOCK, + consumer->readReleaseFence(releaseCallbackId, releaseFence, maxAcquiredBufferCount)); +} + +// Verify that we can write a message to the BufferReleaseChannel producer and read that message +// using the BufferReleaseChannel consumer. +TEST(BufferReleaseChannelTest, ProduceAndConsume) { + std::unique_ptr consumer; + std::shared_ptr producer; + ASSERT_EQ(OK, BufferReleaseChannel::open("test-channel"s, consumer, producer)); + + sp fence = sp::make(memfd_create("fake-fence-fd", 0)); + + for (uint64_t i = 0; i < 64; i++) { + ReleaseCallbackId producerId{i, i + 1}; + uint32_t maxAcquiredBufferCount = i + 2; + ASSERT_EQ(OK, producer->writeReleaseFence(producerId, fence, maxAcquiredBufferCount)); + } + + for (uint64_t i = 0; i < 64; i++) { + ReleaseCallbackId expectedId{i, i + 1}; + uint32_t expectedMaxAcquiredBufferCount = i + 2; + + ReleaseCallbackId consumerId; + sp consumerFence; + uint32_t maxAcquiredBufferCount; + ASSERT_EQ(OK, + consumer->readReleaseFence(consumerId, consumerFence, maxAcquiredBufferCount)); + + ASSERT_EQ(expectedId, consumerId); + ASSERT_TRUE(is_same_file(fence->get(), consumerFence->get())); + ASSERT_EQ(expectedMaxAcquiredBufferCount, maxAcquiredBufferCount); + } +} + +} // namespace android \ No newline at end of file -- cgit v1.2.3-59-g8ed1b From bae678660e8bcbfe92d2b6c2c46f112f5fd97514 Mon Sep 17 00:00:00 2001 From: Alec Mouri Date: Tue, 6 Aug 2024 14:53:46 +0000 Subject: Move CaptureArgs + friends to structured aidl No more manual parceling! Bug: 329465218 Flag: EXEMPT mechanical refactor Test: builds Test: courage Change-Id: I9f5eba12db615d6b02de0686193381f7a63bb043 --- libs/gui/LayerState.cpp | 82 --------------------- libs/gui/aidl/Android.bp | 3 - libs/gui/aidl/android/gui/CaptureArgs.aidl | 56 ++++++++++++++- libs/gui/aidl/android/gui/DisplayCaptureArgs.aidl | 15 +++- libs/gui/aidl/android/gui/LayerCaptureArgs.aidl | 13 +++- libs/gui/include/gui/AidlStatusUtil.h | 26 ++++++- libs/gui/include/gui/DisplayCaptureArgs.h | 84 ---------------------- libs/gui/include/gui/ISurfaceComposer.h | 9 +-- libs/gui/include/gui/LayerCaptureArgs.h | 34 --------- libs/gui/include/gui/LayerState.h | 4 +- libs/gui/rust/aidl_types/src/lib.rs | 3 - libs/gui/tests/BLASTBufferQueue_test.cpp | 3 +- services/surfaceflinger/SurfaceFlinger.cpp | 68 ++++++++++-------- services/surfaceflinger/tests/Credentials_test.cpp | 2 +- services/surfaceflinger/tests/LayerState_test.cpp | 60 ---------------- .../tests/LayerTypeTransaction_test.cpp | 3 +- services/surfaceflinger/tests/MirrorLayer_test.cpp | 3 +- .../surfaceflinger/tests/ScreenCapture_test.cpp | 59 +++++++-------- .../surfaceflinger/tests/TextureFiltering_test.cpp | 33 ++++----- .../surfaceflinger/tests/utils/ScreenshotUtils.h | 4 +- 20 files changed, 206 insertions(+), 358 deletions(-) delete mode 100644 libs/gui/include/gui/DisplayCaptureArgs.h delete mode 100644 libs/gui/include/gui/LayerCaptureArgs.h (limited to 'libs/gui/LayerState.cpp') diff --git a/libs/gui/LayerState.cpp b/libs/gui/LayerState.cpp index 0714fd9501..b10996951b 100644 --- a/libs/gui/LayerState.cpp +++ b/libs/gui/LayerState.cpp @@ -892,88 +892,6 @@ status_t InputWindowCommands::read(const Parcel& input) { // ---------------------------------------------------------------------------- -namespace gui { - -status_t CaptureArgs::writeToParcel(Parcel* output) const { - SAFE_PARCEL(output->writeInt32, static_cast(pixelFormat)); - SAFE_PARCEL(output->write, sourceCrop); - SAFE_PARCEL(output->writeFloat, frameScaleX); - SAFE_PARCEL(output->writeFloat, frameScaleY); - SAFE_PARCEL(output->writeBool, captureSecureLayers); - SAFE_PARCEL(output->writeInt32, uid); - SAFE_PARCEL(output->writeInt32, static_cast(dataspace)); - SAFE_PARCEL(output->writeBool, allowProtected); - SAFE_PARCEL(output->writeBool, grayscale); - SAFE_PARCEL(output->writeInt32, excludeHandles.size()); - for (auto& excludeHandle : excludeHandles) { - SAFE_PARCEL(output->writeStrongBinder, excludeHandle); - } - SAFE_PARCEL(output->writeBool, hintForSeamlessTransition); - return NO_ERROR; -} - -status_t CaptureArgs::readFromParcel(const Parcel* input) { - int32_t value = 0; - SAFE_PARCEL(input->readInt32, &value); - pixelFormat = static_cast(value); - SAFE_PARCEL(input->read, sourceCrop); - SAFE_PARCEL(input->readFloat, &frameScaleX); - SAFE_PARCEL(input->readFloat, &frameScaleY); - SAFE_PARCEL(input->readBool, &captureSecureLayers); - SAFE_PARCEL(input->readInt32, &uid); - SAFE_PARCEL(input->readInt32, &value); - dataspace = static_cast(value); - SAFE_PARCEL(input->readBool, &allowProtected); - SAFE_PARCEL(input->readBool, &grayscale); - int32_t numExcludeHandles = 0; - SAFE_PARCEL_READ_SIZE(input->readInt32, &numExcludeHandles, input->dataSize()); - excludeHandles.reserve(numExcludeHandles); - for (int i = 0; i < numExcludeHandles; i++) { - sp binder; - SAFE_PARCEL(input->readStrongBinder, &binder); - excludeHandles.emplace(binder); - } - SAFE_PARCEL(input->readBool, &hintForSeamlessTransition); - return NO_ERROR; -} - -status_t DisplayCaptureArgs::writeToParcel(Parcel* output) const { - SAFE_PARCEL(CaptureArgs::writeToParcel, output); - - SAFE_PARCEL(output->writeStrongBinder, displayToken); - SAFE_PARCEL(output->writeUint32, width); - SAFE_PARCEL(output->writeUint32, height); - return NO_ERROR; -} - -status_t DisplayCaptureArgs::readFromParcel(const Parcel* input) { - SAFE_PARCEL(CaptureArgs::readFromParcel, input); - - SAFE_PARCEL(input->readStrongBinder, &displayToken); - SAFE_PARCEL(input->readUint32, &width); - SAFE_PARCEL(input->readUint32, &height); - return NO_ERROR; -} - -status_t LayerCaptureArgs::writeToParcel(Parcel* output) const { - SAFE_PARCEL(CaptureArgs::writeToParcel, output); - - SAFE_PARCEL(output->writeStrongBinder, layerHandle); - SAFE_PARCEL(output->writeBool, childrenOnly); - return NO_ERROR; -} - -status_t LayerCaptureArgs::readFromParcel(const Parcel* input) { - SAFE_PARCEL(CaptureArgs::readFromParcel, input); - - SAFE_PARCEL(input->readStrongBinder, &layerHandle); - - SAFE_PARCEL(input->readBool, &childrenOnly); - return NO_ERROR; -} - -}; // namespace gui - ReleaseCallbackId BufferData::generateReleaseCallbackId() const { uint64_t bufferId; if (buffer) { diff --git a/libs/gui/aidl/Android.bp b/libs/gui/aidl/Android.bp index 8ed08c2644..fd035f60f5 100644 --- a/libs/gui/aidl/Android.bp +++ b/libs/gui/aidl/Android.bp @@ -28,9 +28,6 @@ filegroup { ":libgui_extra_unstructured_aidl_files", "android/gui/BitTube.aidl", - "android/gui/CaptureArgs.aidl", - "android/gui/DisplayCaptureArgs.aidl", - "android/gui/LayerCaptureArgs.aidl", "android/gui/LayerMetadata.aidl", "android/gui/ParcelableVsyncEventData.aidl", "android/gui/ScreenCaptureResults.aidl", diff --git a/libs/gui/aidl/android/gui/CaptureArgs.aidl b/libs/gui/aidl/android/gui/CaptureArgs.aidl index 9f198cae10..2bbed2b9d6 100644 --- a/libs/gui/aidl/android/gui/CaptureArgs.aidl +++ b/libs/gui/aidl/android/gui/CaptureArgs.aidl @@ -16,4 +16,58 @@ package android.gui; -parcelable CaptureArgs cpp_header "gui/DisplayCaptureArgs.h" rust_type "gui_aidl_types_rs::CaptureArgs"; +import android.gui.ARect; + +// Common arguments for capturing content on-screen +parcelable CaptureArgs { + const int UNSET_UID = -1; + + // Desired pixel format of the final screenshotted buffer + int /*ui::PixelFormat*/ pixelFormat = 1; + + // Crop in layer space: all content outside of the crop will not be captured. + ARect sourceCrop; + + // Scale in the x-direction for the screenshotted result. + float frameScaleX = 1.0f; + + // Scale in the y-direction for the screenshotted result. + float frameScaleY = 1.0f; + + // True if capturing secure layers is permitted + boolean captureSecureLayers = false; + + // UID whose content we want to screenshot + int uid = UNSET_UID; + + // Force capture to be in a color space. If the value is ui::Dataspace::UNKNOWN, the captured + // result will be in a colorspace appropriate for capturing the display contents + // The display may use non-RGB dataspace (ex. displayP3) that could cause pixel data could be + // different from SRGB (byte per color), and failed when checking colors in tests. + // NOTE: In normal cases, we want the screen to be captured in display's colorspace. + int /*ui::Dataspace*/ dataspace = 0; + + // The receiver of the capture can handle protected buffer. A protected buffer has + // GRALLOC_USAGE_PROTECTED usage bit and must not be accessed unprotected behaviour. + // Any read/write access from unprotected context will result in undefined behaviour. + // Protected contents are typically DRM contents. This has no direct implication to the + // secure property of the surface, which is specified by the application explicitly to avoid + // the contents being accessed/captured by screenshot or unsecure display. + boolean allowProtected = false; + + // True if the content should be captured in grayscale + boolean grayscale = false; + + // List of layers to exclude capturing from + IBinder[] excludeHandles; + + // Hint that the caller will use the screenshot animation as part of a transition animation. + // The canonical example would be screen rotation - in such a case any color shift in the + // screenshot is a detractor so composition in the display's colorspace is required. + // Otherwise, the system may choose a colorspace that is more appropriate for use-cases + // such as file encoding or for blending HDR content into an ap's UI, where the display's + // exact colorspace is not an appropriate intermediate result. + // Note that if the caller is requesting a specific dataspace, this hint does nothing. + boolean hintForSeamlessTransition = false; +} + diff --git a/libs/gui/aidl/android/gui/DisplayCaptureArgs.aidl b/libs/gui/aidl/android/gui/DisplayCaptureArgs.aidl index fc97dbf03d..e00a2dfa82 100644 --- a/libs/gui/aidl/android/gui/DisplayCaptureArgs.aidl +++ b/libs/gui/aidl/android/gui/DisplayCaptureArgs.aidl @@ -16,5 +16,18 @@ package android.gui; -parcelable DisplayCaptureArgs cpp_header "gui/DisplayCaptureArgs.h" rust_type "gui_aidl_types_rs::DisplayCaptureArgs"; +import android.gui.CaptureArgs; + +// Arguments for screenshotting an entire display +parcelable DisplayCaptureArgs { + CaptureArgs captureArgs; + + // The display that we want to screenshot + IBinder displayToken; + + // The width of the render area when we screenshot + int width = 0; + // The length of the render area when we screenshot + int height = 0; +} diff --git a/libs/gui/aidl/android/gui/LayerCaptureArgs.aidl b/libs/gui/aidl/android/gui/LayerCaptureArgs.aidl index 18d293f211..004c35a5ce 100644 --- a/libs/gui/aidl/android/gui/LayerCaptureArgs.aidl +++ b/libs/gui/aidl/android/gui/LayerCaptureArgs.aidl @@ -16,4 +16,15 @@ package android.gui; -parcelable LayerCaptureArgs cpp_header "gui/LayerCaptureArgs.h" rust_type "gui_aidl_types_rs::LayerCaptureArgs"; +import android.gui.CaptureArgs; + +// Arguments for capturing a layer and/or its children +parcelable LayerCaptureArgs { + CaptureArgs captureArgs; + + // The Layer that we may want to capture. We would also capture its children + IBinder layerHandle; + // True if we don't actually want to capture the layer and want to capture + // its children instead. + boolean childrenOnly = false; +} diff --git a/libs/gui/include/gui/AidlStatusUtil.h b/libs/gui/include/gui/AidlStatusUtil.h index 55be27bf35..159e0329ac 100644 --- a/libs/gui/include/gui/AidlStatusUtil.h +++ b/libs/gui/include/gui/AidlStatusUtil.h @@ -16,9 +16,11 @@ #pragma once +#include #include +#include -// Extracted from frameworks/av/media/libaudioclient/include/media/AidlConversionUtil.h +// Originally extracted from frameworks/av/media/libaudioclient/include/media/AidlConversionUtil.h namespace android::gui::aidl_utils { /** @@ -111,4 +113,26 @@ static inline ::android::binder::Status binderStatusFromStatusT( return Status::fromServiceSpecificError(status, emptyIfNull); } +static inline Rect fromARect(ARect rect) { + return Rect(rect.left, rect.top, rect.right, rect.bottom); +} + +static inline ARect toARect(Rect rect) { + ARect aRect; + + aRect.left = rect.left; + aRect.top = rect.top; + aRect.right = rect.right; + aRect.bottom = rect.bottom; + return aRect; +} + +static inline ARect toARect(int32_t left, int32_t top, int32_t right, int32_t bottom) { + return toARect(Rect(left, top, right, bottom)); +} + +static inline ARect toARect(int32_t width, int32_t height) { + return toARect(Rect(width, height)); +} + } // namespace android::gui::aidl_utils diff --git a/libs/gui/include/gui/DisplayCaptureArgs.h b/libs/gui/include/gui/DisplayCaptureArgs.h deleted file mode 100644 index e29ce41bd5..0000000000 --- a/libs/gui/include/gui/DisplayCaptureArgs.h +++ /dev/null @@ -1,84 +0,0 @@ -/* - * Copyright (C) 2022 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#pragma once - -#include -#include - -#include -#include -#include -#include -#include -#include -#include -#include - -namespace android::gui { - -struct CaptureArgs : public Parcelable { - const static int32_t UNSET_UID = -1; - virtual ~CaptureArgs() = default; - - ui::PixelFormat pixelFormat{ui::PixelFormat::RGBA_8888}; - Rect sourceCrop; - float frameScaleX{1}; - float frameScaleY{1}; - bool captureSecureLayers{false}; - int32_t uid{UNSET_UID}; - // Force capture to be in a color space. If the value is ui::Dataspace::UNKNOWN, the captured - // result will be in a colorspace appropriate for capturing the display contents - // The display may use non-RGB dataspace (ex. displayP3) that could cause pixel data could be - // different from SRGB (byte per color), and failed when checking colors in tests. - // NOTE: In normal cases, we want the screen to be captured in display's colorspace. - ui::Dataspace dataspace = ui::Dataspace::UNKNOWN; - - // The receiver of the capture can handle protected buffer. A protected buffer has - // GRALLOC_USAGE_PROTECTED usage bit and must not be accessed unprotected behaviour. - // Any read/write access from unprotected context will result in undefined behaviour. - // Protected contents are typically DRM contents. This has no direct implication to the - // secure property of the surface, which is specified by the application explicitly to avoid - // the contents being accessed/captured by screenshot or unsecure display. - bool allowProtected = false; - - bool grayscale = false; - - std::unordered_set, SpHash> excludeHandles; - - // Hint that the caller will use the screenshot animation as part of a transition animation. - // The canonical example would be screen rotation - in such a case any color shift in the - // screenshot is a detractor so composition in the display's colorspace is required. - // Otherwise, the system may choose a colorspace that is more appropriate for use-cases - // such as file encoding or for blending HDR content into an ap's UI, where the display's - // exact colorspace is not an appropriate intermediate result. - // Note that if the caller is requesting a specific dataspace, this hint does nothing. - bool hintForSeamlessTransition = false; - - virtual status_t writeToParcel(Parcel* output) const; - virtual status_t readFromParcel(const Parcel* input); -}; - -struct DisplayCaptureArgs : CaptureArgs { - sp displayToken; - uint32_t width{0}; - uint32_t height{0}; - - status_t writeToParcel(Parcel* output) const override; - status_t readFromParcel(const Parcel* input) override; -}; - -}; // namespace android::gui diff --git a/libs/gui/include/gui/ISurfaceComposer.h b/libs/gui/include/gui/ISurfaceComposer.h index 1ecc216dff..9a422fd808 100644 --- a/libs/gui/include/gui/ISurfaceComposer.h +++ b/libs/gui/include/gui/ISurfaceComposer.h @@ -18,6 +18,7 @@ #include #include +#include #include #include #include @@ -27,6 +28,7 @@ #include #include #include +#include #include #include #include @@ -70,13 +72,6 @@ using gui::IRegionSamplingListener; using gui::IScreenCaptureListener; using gui::SpHash; -namespace gui { - -struct DisplayCaptureArgs; -struct LayerCaptureArgs; - -} // namespace gui - namespace ui { struct DisplayMode; diff --git a/libs/gui/include/gui/LayerCaptureArgs.h b/libs/gui/include/gui/LayerCaptureArgs.h deleted file mode 100644 index fae2bcc787..0000000000 --- a/libs/gui/include/gui/LayerCaptureArgs.h +++ /dev/null @@ -1,34 +0,0 @@ -/* - * Copyright (C) 2022 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#pragma once - -#include -#include - -#include - -namespace android::gui { - -struct LayerCaptureArgs : CaptureArgs { - sp layerHandle; - bool childrenOnly{false}; - - status_t writeToParcel(Parcel* output) const override; - status_t readFromParcel(const Parcel* input) override; -}; - -}; // namespace android::gui diff --git a/libs/gui/include/gui/LayerState.h b/libs/gui/include/gui/LayerState.h index d41994589f..2cdde3255e 100644 --- a/libs/gui/include/gui/LayerState.h +++ b/libs/gui/include/gui/LayerState.h @@ -21,7 +21,9 @@ #include #include +#include #include +#include #include #include #include @@ -35,9 +37,7 @@ #include #include -#include #include -#include #include #include #include diff --git a/libs/gui/rust/aidl_types/src/lib.rs b/libs/gui/rust/aidl_types/src/lib.rs index fead018bbf..2351df0318 100644 --- a/libs/gui/rust/aidl_types/src/lib.rs +++ b/libs/gui/rust/aidl_types/src/lib.rs @@ -42,10 +42,7 @@ macro_rules! stub_unstructured_parcelable { } stub_unstructured_parcelable!(BitTube); -stub_unstructured_parcelable!(CaptureArgs); -stub_unstructured_parcelable!(DisplayCaptureArgs); stub_unstructured_parcelable!(DisplayInfo); -stub_unstructured_parcelable!(LayerCaptureArgs); stub_unstructured_parcelable!(LayerDebugInfo); stub_unstructured_parcelable!(LayerMetadata); stub_unstructured_parcelable!(ParcelableVsyncEventData); diff --git a/libs/gui/tests/BLASTBufferQueue_test.cpp b/libs/gui/tests/BLASTBufferQueue_test.cpp index 6852589e32..46a19c273d 100644 --- a/libs/gui/tests/BLASTBufferQueue_test.cpp +++ b/libs/gui/tests/BLASTBufferQueue_test.cpp @@ -229,7 +229,8 @@ protected: ISurfaceComposerClient::eFXSurfaceBufferState, /*parent*/ mRootSurfaceControl->getHandle()); - mCaptureArgs.sourceCrop = Rect(ui::Size(mDisplayWidth, mDisplayHeight)); + mCaptureArgs.captureArgs.sourceCrop = + gui::aidl_utils::toARect(mDisplayWidth, mDisplayHeight); mCaptureArgs.layerHandle = mRootSurfaceControl->getHandle(); } diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 9be9fee9ee..1e374c0a05 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -7105,7 +7105,8 @@ void SurfaceFlinger::captureDisplay(const DisplayCaptureArgs& args, const sp& captureListener) { SFTRACE_CALL(); - status_t validate = validateScreenshotPermissions(args); + const auto& captureArgs = args.captureArgs; + status_t validate = validateScreenshotPermissions(captureArgs); if (validate != OK) { ALOGD("Permission denied to captureDisplay"); invokeScreenCaptureError(validate, captureListener); @@ -7118,7 +7119,7 @@ void SurfaceFlinger::captureDisplay(const DisplayCaptureArgs& args, return; } - if (args.captureSecureLayers && !hasCaptureBlackoutContentPermission()) { + if (captureArgs.captureSecureLayers && !hasCaptureBlackoutContentPermission()) { ALOGD("Attempting to capture secure layers without CAPTURE_BLACKOUT_CONTENT"); invokeScreenCaptureError(PERMISSION_DENIED, captureListener); return; @@ -7144,7 +7145,7 @@ void SurfaceFlinger::captureDisplay(const DisplayCaptureArgs& args, reqSize = display->getLayerStackSpaceRect().getSize(); } - for (const auto& handle : args.excludeHandles) { + for (const auto& handle : captureArgs.excludeHandles) { uint32_t excludeLayer = LayerHandle::getLayerId(handle); if (excludeLayer != UNASSIGNED_LAYER_ID) { excludeLayerIds.emplace(excludeLayer); @@ -7157,17 +7158,21 @@ void SurfaceFlinger::captureDisplay(const DisplayCaptureArgs& args, } GetLayerSnapshotsFunction getLayerSnapshotsFn = - getLayerSnapshotsForScreenshots(layerStack, args.uid, std::move(excludeLayerIds)); + getLayerSnapshotsForScreenshots(layerStack, captureArgs.uid, + std::move(excludeLayerIds)); ftl::Flags options; - if (args.captureSecureLayers) options |= RenderArea::Options::CAPTURE_SECURE_LAYERS; - if (args.hintForSeamlessTransition) + if (captureArgs.captureSecureLayers) options |= RenderArea::Options::CAPTURE_SECURE_LAYERS; + if (captureArgs.hintForSeamlessTransition) options |= RenderArea::Options::HINT_FOR_SEAMLESS_TRANSITION; captureScreenCommon(RenderAreaBuilderVariant(std::in_place_type, - args.sourceCrop, reqSize, args.dataspace, + gui::aidl_utils::fromARect(captureArgs.sourceCrop), + reqSize, + static_cast(captureArgs.dataspace), displayWeak, options), - getLayerSnapshotsFn, reqSize, args.pixelFormat, args.allowProtected, - args.grayscale, captureListener); + getLayerSnapshotsFn, reqSize, + static_cast(captureArgs.pixelFormat), + captureArgs.allowProtected, captureArgs.grayscale, captureListener); } void SurfaceFlinger::captureDisplay(DisplayId displayId, const CaptureArgs& args, @@ -7220,10 +7225,11 @@ void SurfaceFlinger::captureDisplay(DisplayId displayId, const CaptureArgs& args if (args.hintForSeamlessTransition) options |= RenderArea::Options::HINT_FOR_SEAMLESS_TRANSITION; captureScreenCommon(RenderAreaBuilderVariant(std::in_place_type, - Rect(), size, args.dataspace, displayWeak, - options), - getLayerSnapshotsFn, size, args.pixelFormat, kAllowProtected, kGrayscale, - captureListener); + Rect(), size, + static_cast(args.dataspace), + displayWeak, options), + getLayerSnapshotsFn, size, static_cast(args.pixelFormat), + kAllowProtected, kGrayscale, captureListener); } ScreenCaptureResults SurfaceFlinger::captureLayersSync(const LayerCaptureArgs& args) { @@ -7236,20 +7242,23 @@ void SurfaceFlinger::captureLayers(const LayerCaptureArgs& args, const sp& captureListener) { SFTRACE_CALL(); - status_t validate = validateScreenshotPermissions(args); + const auto& captureArgs = args.captureArgs; + + status_t validate = validateScreenshotPermissions(captureArgs); if (validate != OK) { ALOGD("Permission denied to captureLayers"); invokeScreenCaptureError(validate, captureListener); return; } + auto crop = gui::aidl_utils::fromARect(captureArgs.sourceCrop); + ui::Size reqSize; sp parent; - Rect crop(args.sourceCrop); std::unordered_set excludeLayerIds; - ui::Dataspace dataspace = args.dataspace; + ui::Dataspace dataspace = static_cast(captureArgs.dataspace); - if (args.captureSecureLayers && !hasCaptureBlackoutContentPermission()) { + if (captureArgs.captureSecureLayers && !hasCaptureBlackoutContentPermission()) { ALOGD("Attempting to capture secure layers without CAPTURE_BLACKOUT_CONTENT"); invokeScreenCaptureError(PERMISSION_DENIED, captureListener); return; @@ -7266,26 +7275,27 @@ void SurfaceFlinger::captureLayers(const LayerCaptureArgs& args, } Rect parentSourceBounds = parent->getCroppedBufferSize(parent->getDrawingState()); - if (args.sourceCrop.width() <= 0) { + if (crop.width() <= 0) { crop.left = 0; crop.right = parentSourceBounds.getWidth(); } - if (args.sourceCrop.height() <= 0) { + if (crop.height() <= 0) { crop.top = 0; crop.bottom = parentSourceBounds.getHeight(); } - if (crop.isEmpty() || args.frameScaleX <= 0.0f || args.frameScaleY <= 0.0f) { + if (crop.isEmpty() || captureArgs.frameScaleX <= 0.0f || captureArgs.frameScaleY <= 0.0f) { // Error out if the layer has no source bounds (i.e. they are boundless) and a source // crop was not specified, or an invalid frame scale was provided. ALOGD("Boundless layer, unspecified crop, or invalid frame scale to captureLayers"); invokeScreenCaptureError(BAD_VALUE, captureListener); return; } - reqSize = ui::Size(crop.width() * args.frameScaleX, crop.height() * args.frameScaleY); + reqSize = ui::Size(crop.width() * captureArgs.frameScaleX, + crop.height() * captureArgs.frameScaleY); - for (const auto& handle : args.excludeHandles) { + for (const auto& handle : captureArgs.excludeHandles) { uint32_t excludeLayer = LayerHandle::getLayerId(handle); if (excludeLayer != UNASSIGNED_LAYER_ID) { excludeLayerIds.emplace(excludeLayer); @@ -7311,8 +7321,9 @@ void SurfaceFlinger::captureLayers(const LayerCaptureArgs& args, } GetLayerSnapshotsFunction getLayerSnapshotsFn = - getLayerSnapshotsForScreenshots(parent->sequence, args.uid, std::move(excludeLayerIds), - args.childrenOnly, parentCrop); + getLayerSnapshotsForScreenshots(parent->sequence, captureArgs.uid, + std::move(excludeLayerIds), args.childrenOnly, + parentCrop); if (captureListener == nullptr) { ALOGD("capture screen must provide a capture listener callback"); @@ -7321,14 +7332,15 @@ void SurfaceFlinger::captureLayers(const LayerCaptureArgs& args, } ftl::Flags options; - if (args.captureSecureLayers) options |= RenderArea::Options::CAPTURE_SECURE_LAYERS; - if (args.hintForSeamlessTransition) + if (captureArgs.captureSecureLayers) options |= RenderArea::Options::CAPTURE_SECURE_LAYERS; + if (captureArgs.hintForSeamlessTransition) options |= RenderArea::Options::HINT_FOR_SEAMLESS_TRANSITION; captureScreenCommon(RenderAreaBuilderVariant(std::in_place_type, crop, reqSize, dataspace, parent, args.childrenOnly, options), - getLayerSnapshotsFn, reqSize, args.pixelFormat, args.allowProtected, - args.grayscale, captureListener); + getLayerSnapshotsFn, reqSize, + static_cast(captureArgs.pixelFormat), + captureArgs.allowProtected, captureArgs.grayscale, captureListener); } // Creates a Future release fence for a layer and keeps track of it in a list to diff --git a/services/surfaceflinger/tests/Credentials_test.cpp b/services/surfaceflinger/tests/Credentials_test.cpp index d355e720d1..babbcd4e35 100644 --- a/services/surfaceflinger/tests/Credentials_test.cpp +++ b/services/surfaceflinger/tests/Credentials_test.cpp @@ -280,7 +280,7 @@ TEST_F(CredentialsTest, CaptureLayersTest) { std::function condition = [=, this]() { LayerCaptureArgs captureArgs; captureArgs.layerHandle = mBGSurfaceControl->getHandle(); - captureArgs.sourceCrop = {0, 0, 1, 1}; + captureArgs.captureArgs.sourceCrop = gui::aidl_utils::toARect(0, 0, 1, 1); ScreenCaptureResults captureResults; return ScreenCapture::captureLayers(captureArgs, captureResults); diff --git a/services/surfaceflinger/tests/LayerState_test.cpp b/services/surfaceflinger/tests/LayerState_test.cpp index 15a98df275..cc57e11206 100644 --- a/services/surfaceflinger/tests/LayerState_test.cpp +++ b/services/surfaceflinger/tests/LayerState_test.cpp @@ -28,66 +28,6 @@ using gui::ScreenCaptureResults; namespace test { -TEST(LayerStateTest, ParcellingDisplayCaptureArgs) { - DisplayCaptureArgs args; - args.pixelFormat = ui::PixelFormat::RGB_565; - args.sourceCrop = Rect(0, 0, 500, 200); - args.frameScaleX = 2; - args.frameScaleY = 4; - args.captureSecureLayers = true; - args.displayToken = sp::make(); - args.width = 10; - args.height = 20; - args.grayscale = true; - - Parcel p; - args.writeToParcel(&p); - p.setDataPosition(0); - - DisplayCaptureArgs args2; - args2.readFromParcel(&p); - - ASSERT_EQ(args.pixelFormat, args2.pixelFormat); - ASSERT_EQ(args.sourceCrop, args2.sourceCrop); - ASSERT_EQ(args.frameScaleX, args2.frameScaleX); - ASSERT_EQ(args.frameScaleY, args2.frameScaleY); - ASSERT_EQ(args.captureSecureLayers, args2.captureSecureLayers); - ASSERT_EQ(args.displayToken, args2.displayToken); - ASSERT_EQ(args.width, args2.width); - ASSERT_EQ(args.height, args2.height); - ASSERT_EQ(args.grayscale, args2.grayscale); -} - -TEST(LayerStateTest, ParcellingLayerCaptureArgs) { - LayerCaptureArgs args; - args.pixelFormat = ui::PixelFormat::RGB_565; - args.sourceCrop = Rect(0, 0, 500, 200); - args.frameScaleX = 2; - args.frameScaleY = 4; - args.captureSecureLayers = true; - args.layerHandle = sp::make(); - args.excludeHandles = {sp::make(), sp::make()}; - args.childrenOnly = false; - args.grayscale = true; - - Parcel p; - args.writeToParcel(&p); - p.setDataPosition(0); - - LayerCaptureArgs args2; - args2.readFromParcel(&p); - - ASSERT_EQ(args.pixelFormat, args2.pixelFormat); - ASSERT_EQ(args.sourceCrop, args2.sourceCrop); - ASSERT_EQ(args.frameScaleX, args2.frameScaleX); - ASSERT_EQ(args.frameScaleY, args2.frameScaleY); - ASSERT_EQ(args.captureSecureLayers, args2.captureSecureLayers); - ASSERT_EQ(args.layerHandle, args2.layerHandle); - ASSERT_EQ(args.excludeHandles, args2.excludeHandles); - ASSERT_EQ(args.childrenOnly, args2.childrenOnly); - ASSERT_EQ(args.grayscale, args2.grayscale); -} - TEST(LayerStateTest, ParcellingScreenCaptureResultsWithFence) { ScreenCaptureResults results; results.buffer = sp::make(100u, 200u, PIXEL_FORMAT_RGBA_8888, 1u, 0u); diff --git a/services/surfaceflinger/tests/LayerTypeTransaction_test.cpp b/services/surfaceflinger/tests/LayerTypeTransaction_test.cpp index f9b4bbac22..e655df7695 100644 --- a/services/surfaceflinger/tests/LayerTypeTransaction_test.cpp +++ b/services/surfaceflinger/tests/LayerTypeTransaction_test.cpp @@ -15,6 +15,7 @@ */ // TODO(b/129481165): remove the #pragma below and fix conversion issues +#include "gui/AidlStatusUtil.h" #pragma clang diagnostic push #pragma clang diagnostic ignored "-Wconversion" @@ -64,7 +65,7 @@ TEST_P(LayerTypeTransactionTest, SetRelativeZNegative) { // only layerB is in this range LayerCaptureArgs captureArgs; captureArgs.layerHandle = parent->getHandle(); - captureArgs.sourceCrop = {0, 0, 32, 32}; + captureArgs.captureArgs.sourceCrop = gui::aidl_utils::toARect(32, 32); ScreenCapture::captureLayers(&screenshot, captureArgs); screenshot->expectColor(Rect(0, 0, 32, 32), Color::BLUE); } diff --git a/services/surfaceflinger/tests/MirrorLayer_test.cpp b/services/surfaceflinger/tests/MirrorLayer_test.cpp index d97d433160..2a535881b2 100644 --- a/services/surfaceflinger/tests/MirrorLayer_test.cpp +++ b/services/surfaceflinger/tests/MirrorLayer_test.cpp @@ -20,6 +20,7 @@ #include #include +#include #include #include "LayerTransactionTest.h" #include "utils/TransactionUtils.h" @@ -350,7 +351,7 @@ TEST_F(MirrorLayerTest, OffscreenMirrorScreenshot) { // Capture just the mirror layer and child. LayerCaptureArgs captureArgs; captureArgs.layerHandle = mirrorParent->getHandle(); - captureArgs.sourceCrop = childBounds; + captureArgs.captureArgs.sourceCrop = gui::aidl_utils::toARect(childBounds); std::unique_ptr shot; ScreenCapture::captureLayers(&shot, captureArgs); shot->expectSize(childBounds.width(), childBounds.height()); diff --git a/services/surfaceflinger/tests/ScreenCapture_test.cpp b/services/surfaceflinger/tests/ScreenCapture_test.cpp index 9a78550d00..069f199cab 100644 --- a/services/surfaceflinger/tests/ScreenCapture_test.cpp +++ b/services/surfaceflinger/tests/ScreenCapture_test.cpp @@ -20,6 +20,7 @@ #pragma clang diagnostic push #pragma clang diagnostic ignored "-Wconversion" +#include #include #include @@ -65,7 +66,7 @@ protected: .show(mFGSurfaceControl); }); - mCaptureArgs.sourceCrop = mDisplayRect; + mCaptureArgs.captureArgs.sourceCrop = gui::aidl_utils::toARect(mDisplayRect); mCaptureArgs.layerHandle = mRootSurfaceControl->getHandle(); } @@ -112,7 +113,7 @@ TEST_F(ScreenCaptureTest, SetFlagsSecureEUidSystem) { shot->expectColor(Rect(0, 0, 32, 32), Color::BLACK); } - mCaptureArgs.captureSecureLayers = true; + mCaptureArgs.captureArgs.captureSecureLayers = true; // AID_SYSTEM is allowed to capture secure content. ASSERT_EQ(NO_ERROR, ScreenCapture::captureLayers(mCaptureArgs, mCaptureResults)); ASSERT_TRUE(mCaptureResults.capturedSecureLayers); @@ -164,7 +165,7 @@ TEST_F(ScreenCaptureTest, CaptureChildSetParentFlagsSecureEUidSystem) { // Here we pass captureSecureLayers = true and since we are AID_SYSTEM we should be able // to receive them...we are expected to take care with the results. - mCaptureArgs.captureSecureLayers = true; + mCaptureArgs.captureArgs.captureSecureLayers = true; ASSERT_EQ(NO_ERROR, ScreenCapture::captureLayers(mCaptureArgs, mCaptureResults)); ASSERT_TRUE(mCaptureResults.capturedSecureLayers); ScreenCapture sc(mCaptureResults.buffer, mCaptureResults.capturedHdrLayers); @@ -198,8 +199,8 @@ TEST_F(ScreenCaptureTest, CaptureChildRespectsParentSecureFlag) { .apply(); LayerCaptureArgs captureArgs; captureArgs.layerHandle = childLayer->getHandle(); - captureArgs.sourceCrop = size; - captureArgs.captureSecureLayers = false; + captureArgs.captureArgs.sourceCrop = gui::aidl_utils::toARect(size); + captureArgs.captureArgs.captureSecureLayers = false; { SCOPED_TRACE("parent hidden"); ASSERT_EQ(NO_ERROR, ScreenCapture::captureLayers(captureArgs, mCaptureResults)); @@ -208,7 +209,7 @@ TEST_F(ScreenCaptureTest, CaptureChildRespectsParentSecureFlag) { sc.expectColor(size, Color::BLACK); } - captureArgs.captureSecureLayers = true; + captureArgs.captureArgs.captureSecureLayers = true; { SCOPED_TRACE("capture secure parent not visible"); ASSERT_EQ(NO_ERROR, ScreenCapture::captureLayers(captureArgs, mCaptureResults)); @@ -218,7 +219,7 @@ TEST_F(ScreenCaptureTest, CaptureChildRespectsParentSecureFlag) { } Transaction().show(parentLayer).apply(); - captureArgs.captureSecureLayers = false; + captureArgs.captureArgs.captureSecureLayers = false; { SCOPED_TRACE("parent visible"); ASSERT_EQ(NO_ERROR, ScreenCapture::captureLayers(captureArgs, mCaptureResults)); @@ -227,7 +228,7 @@ TEST_F(ScreenCaptureTest, CaptureChildRespectsParentSecureFlag) { sc.expectColor(size, Color::BLACK); } - captureArgs.captureSecureLayers = true; + captureArgs.captureArgs.captureSecureLayers = true; { SCOPED_TRACE("capture secure parent visible"); ASSERT_EQ(NO_ERROR, ScreenCapture::captureLayers(captureArgs, mCaptureResults)); @@ -259,8 +260,8 @@ TEST_F(ScreenCaptureTest, CaptureOffscreenChildRespectsParentSecureFlag) { .apply(); LayerCaptureArgs captureArgs; captureArgs.layerHandle = childLayer->getHandle(); - captureArgs.sourceCrop = size; - captureArgs.captureSecureLayers = false; + captureArgs.captureArgs.sourceCrop = gui::aidl_utils::toARect(size); + captureArgs.captureArgs.captureSecureLayers = false; { SCOPED_TRACE("parent hidden"); ASSERT_EQ(NO_ERROR, ScreenCapture::captureLayers(captureArgs, mCaptureResults)); @@ -269,7 +270,7 @@ TEST_F(ScreenCaptureTest, CaptureOffscreenChildRespectsParentSecureFlag) { sc.expectColor(size, Color::BLACK); } - captureArgs.captureSecureLayers = true; + captureArgs.captureArgs.captureSecureLayers = true; { SCOPED_TRACE("capture secure parent not visible"); ASSERT_EQ(NO_ERROR, ScreenCapture::captureLayers(captureArgs, mCaptureResults)); @@ -279,7 +280,7 @@ TEST_F(ScreenCaptureTest, CaptureOffscreenChildRespectsParentSecureFlag) { } Transaction().show(parentLayer).apply(); - captureArgs.captureSecureLayers = false; + captureArgs.captureArgs.captureSecureLayers = false; { SCOPED_TRACE("parent visible"); ASSERT_EQ(NO_ERROR, ScreenCapture::captureLayers(captureArgs, mCaptureResults)); @@ -288,7 +289,7 @@ TEST_F(ScreenCaptureTest, CaptureOffscreenChildRespectsParentSecureFlag) { sc.expectColor(size, Color::BLACK); } - captureArgs.captureSecureLayers = true; + captureArgs.captureArgs.captureSecureLayers = true; { SCOPED_TRACE("capture secure parent visible"); ASSERT_EQ(NO_ERROR, ScreenCapture::captureLayers(captureArgs, mCaptureResults)); @@ -361,14 +362,14 @@ TEST_F(ScreenCaptureTest, CaptureLayerExclude) { LayerCaptureArgs captureArgs; captureArgs.layerHandle = fgHandle; captureArgs.childrenOnly = true; - captureArgs.excludeHandles = {child2->getHandle()}; + captureArgs.captureArgs.excludeHandles = {child2->getHandle()}; ScreenCapture::captureLayers(&mCapture, captureArgs); mCapture->checkPixel(10, 10, 0, 0, 0); mCapture->checkPixel(0, 0, 200, 200, 200); } TEST_F(ScreenCaptureTest, CaptureLayerExcludeThroughDisplayArgs) { - mCaptureArgs.excludeHandles = {mFGSurfaceControl->getHandle()}; + mCaptureArgs.captureArgs.excludeHandles = {mFGSurfaceControl->getHandle()}; ScreenCapture::captureLayers(&mCapture, mCaptureArgs); mCapture->expectBGColor(0, 0); // Doesn't capture FG layer which is at 64, 64 @@ -401,7 +402,7 @@ TEST_F(ScreenCaptureTest, CaptureLayerExcludeTree) { LayerCaptureArgs captureArgs; captureArgs.layerHandle = fgHandle; captureArgs.childrenOnly = true; - captureArgs.excludeHandles = {child2->getHandle()}; + captureArgs.captureArgs.excludeHandles = {child2->getHandle()}; ScreenCapture::captureLayers(&mCapture, captureArgs); mCapture->checkPixel(10, 10, 0, 0, 0); mCapture->checkPixel(0, 0, 200, 200, 200); @@ -418,7 +419,7 @@ TEST_F(ScreenCaptureTest, CaptureTransparent) { // Captures child LayerCaptureArgs captureArgs; captureArgs.layerHandle = child->getHandle(); - captureArgs.sourceCrop = {0, 0, 10, 20}; + captureArgs.captureArgs.sourceCrop = gui::aidl_utils::toARect(10, 20); ScreenCapture::captureLayers(&mCapture, captureArgs); mCapture->expectColor(Rect(0, 0, 9, 9), {200, 200, 200, 255}); // Area outside of child's bounds is transparent. @@ -481,7 +482,7 @@ TEST_F(ScreenCaptureTest, CaptureBoundlessLayerWithSourceCrop) { LayerCaptureArgs captureArgs; captureArgs.layerHandle = child->getHandle(); - captureArgs.sourceCrop = {0, 0, 10, 10}; + captureArgs.captureArgs.sourceCrop = gui::aidl_utils::toARect(10, 10); ScreenCapture::captureLayers(&mCapture, captureArgs); mCapture->expectColor(Rect(0, 0, 9, 9), Color::RED); @@ -623,7 +624,7 @@ TEST_F(ScreenCaptureTest, CaptureCrop) { // red area to the right of the blue area mCapture->expectColor(Rect(30, 0, 59, 59), Color::RED); - captureArgs.sourceCrop = {0, 0, 30, 30}; + captureArgs.captureArgs.sourceCrop = gui::aidl_utils::toARect(30, 30); ScreenCapture::captureLayers(&mCapture, captureArgs); // Capturing the cropped screen, cropping out the shown red area, should leave only the blue // area visible. @@ -658,8 +659,8 @@ TEST_F(ScreenCaptureTest, CaptureSize) { // red area to the right of the blue area mCapture->expectColor(Rect(30, 0, 59, 59), Color::RED); - captureArgs.frameScaleX = 0.5f; - captureArgs.frameScaleY = 0.5f; + captureArgs.captureArgs.frameScaleX = 0.5f; + captureArgs.captureArgs.frameScaleY = 0.5f; sleep(1); ScreenCapture::captureLayers(&mCapture, captureArgs); @@ -689,8 +690,8 @@ TEST_F(ScreenCaptureTest, CaptureTooLargeLayer) { LayerCaptureArgs captureArgs; captureArgs.layerHandle = redLayer->getHandle(); - captureArgs.frameScaleX = INT32_MAX / 60; - captureArgs.frameScaleY = INT32_MAX / 60; + captureArgs.captureArgs.frameScaleX = INT32_MAX / 60; + captureArgs.captureArgs.frameScaleY = INT32_MAX / 60; ScreenCaptureResults captureResults; ASSERT_EQ(BAD_VALUE, ScreenCapture::captureLayers(captureArgs, captureResults)); @@ -736,7 +737,7 @@ TEST_F(ScreenCaptureTest, CaptureSecureLayer) { mCapture->expectColor(Rect(30, 30, 60, 60), Color::RED); // Passing flag secure so the blue layer should be screenshot too. - args.captureSecureLayers = true; + args.captureArgs.captureSecureLayers = true; ScreenCapture::captureLayers(&mCapture, args); mCapture->expectColor(Rect(0, 0, 30, 30), Color::BLUE); mCapture->expectColor(Rect(30, 30, 60, 60), Color::RED); @@ -780,7 +781,7 @@ TEST_F(ScreenCaptureTest, ScreenshotProtectedBuffer) { // Reading color data will expectedly result in crash, only check usage bit // b/309965549 Checking that the usage bit is protected does not work for // devices that do not support usage protected. - mCaptureArgs.allowProtected = true; + mCaptureArgs.captureArgs.allowProtected = true; ASSERT_EQ(NO_ERROR, ScreenCapture::captureLayers(mCaptureArgs, captureResults)); // ASSERT_EQ(GRALLOC_USAGE_PROTECTED, GRALLOC_USAGE_PROTECTED & // captureResults.buffer->getUsage()); @@ -898,7 +899,7 @@ TEST_F(ScreenCaptureTest, CaptureLayerWithUid) { // Make screenshot request with current uid set. No layers were created with the current // uid so screenshot will be black. - captureArgs.uid = fakeUid; + captureArgs.captureArgs.uid = fakeUid; ScreenCapture::captureLayers(&mCapture, captureArgs); mCapture->expectColor(Rect(0, 0, 32, 32), Color::TRANSPARENT); mCapture->expectBorder(Rect(0, 0, 32, 32), Color::TRANSPARENT); @@ -935,7 +936,7 @@ TEST_F(ScreenCaptureTest, CaptureLayerWithUid) { mCapture->expectBorder(Rect(128, 128, 160, 160), Color::TRANSPARENT); // Screenshot from the fakeUid caller with no uid requested allows everything to be screenshot. - captureArgs.uid = -1; + captureArgs.captureArgs.uid = -1; ScreenCapture::captureLayers(&mCapture, captureArgs); mCapture->expectColor(Rect(128, 128, 160, 160), Color::RED); mCapture->expectBorder(Rect(128, 128, 160, 160), {63, 63, 195, 255}); @@ -955,7 +956,7 @@ TEST_F(ScreenCaptureTest, CaptureWithGrayscale) { ScreenCapture::captureLayers(&mCapture, captureArgs); mCapture->expectColor(Rect(0, 0, 32, 32), Color::RED); - captureArgs.grayscale = true; + captureArgs.captureArgs.grayscale = true; const uint8_t tolerance = 1; @@ -1052,7 +1053,7 @@ TEST_F(ScreenCaptureTest, captureOffscreenNullSnapshot) { LayerCaptureArgs captureArgs; captureArgs.layerHandle = mirroredLayer->getHandle(); - captureArgs.sourceCrop = Rect(0, 0, 1, 1); + captureArgs.captureArgs.sourceCrop = gui::aidl_utils::toARect(1, 1); // Screenshot path should only use the children of the layer hierarchy so // that it will not create a new snapshot. A snapshot would otherwise be diff --git a/services/surfaceflinger/tests/TextureFiltering_test.cpp b/services/surfaceflinger/tests/TextureFiltering_test.cpp index c5d118c1aa..f8c536430a 100644 --- a/services/surfaceflinger/tests/TextureFiltering_test.cpp +++ b/services/surfaceflinger/tests/TextureFiltering_test.cpp @@ -14,9 +14,10 @@ * limitations under the License. */ +#include #include #include -#include +#include #include #include @@ -84,7 +85,7 @@ protected: }; TEST_F(TextureFilteringTest, NoFiltering) { - captureArgs.sourceCrop = Rect{0, 0, 100, 100}; + captureArgs.captureArgs.sourceCrop = gui::aidl_utils::toARect(100, 100); captureArgs.layerHandle = mParent->getHandle(); ScreenCapture::captureLayers(&mCapture, captureArgs); @@ -93,7 +94,7 @@ TEST_F(TextureFilteringTest, NoFiltering) { } TEST_F(TextureFilteringTest, BufferCropNoFiltering) { - captureArgs.sourceCrop = Rect{0, 0, 100, 100}; + captureArgs.captureArgs.sourceCrop = gui::aidl_utils::toARect(100, 100); captureArgs.layerHandle = mParent->getHandle(); ScreenCapture::captureLayers(&mCapture, captureArgs); @@ -105,7 +106,7 @@ TEST_F(TextureFilteringTest, BufferCropNoFiltering) { TEST_F(TextureFilteringTest, BufferCropIsFiltered) { Transaction().setBufferCrop(mLayer, Rect{25, 25, 75, 75}).apply(); - captureArgs.sourceCrop = Rect{0, 0, 100, 100}; + captureArgs.captureArgs.sourceCrop = gui::aidl_utils::toARect(100, 100); captureArgs.layerHandle = mParent->getHandle(); ScreenCapture::captureLayers(&mCapture, captureArgs); @@ -114,9 +115,9 @@ TEST_F(TextureFilteringTest, BufferCropIsFiltered) { // Expect filtering because the output source crop is stretched to the output buffer's size. TEST_F(TextureFilteringTest, OutputSourceCropIsFiltered) { - captureArgs.frameScaleX = 2; - captureArgs.frameScaleY = 2; - captureArgs.sourceCrop = Rect{25, 25, 75, 75}; + captureArgs.captureArgs.frameScaleX = 2; + captureArgs.captureArgs.frameScaleY = 2; + captureArgs.captureArgs.sourceCrop = gui::aidl_utils::toARect(25, 25, 75, 75); captureArgs.layerHandle = mParent->getHandle(); ScreenCapture::captureLayers(&mCapture, captureArgs); @@ -127,9 +128,9 @@ TEST_F(TextureFilteringTest, OutputSourceCropIsFiltered) { // buffer's size. TEST_F(TextureFilteringTest, LayerCropOutputSourceCropIsFiltered) { Transaction().setCrop(mLayer, Rect{25, 25, 75, 75}).apply(); - captureArgs.frameScaleX = 2; - captureArgs.frameScaleY = 2; - captureArgs.sourceCrop = Rect{25, 25, 75, 75}; + captureArgs.captureArgs.frameScaleX = 2; + captureArgs.captureArgs.frameScaleY = 2; + captureArgs.captureArgs.sourceCrop = gui::aidl_utils::toARect(25, 25, 75, 75); captureArgs.layerHandle = mParent->getHandle(); ScreenCapture::captureLayers(&mCapture, captureArgs); @@ -139,8 +140,8 @@ TEST_F(TextureFilteringTest, LayerCropOutputSourceCropIsFiltered) { // Expect filtering because the layer is scaled up. TEST_F(TextureFilteringTest, LayerCaptureWithScalingIsFiltered) { captureArgs.layerHandle = mLayer->getHandle(); - captureArgs.frameScaleX = 2; - captureArgs.frameScaleY = 2; + captureArgs.captureArgs.frameScaleX = 2; + captureArgs.captureArgs.frameScaleY = 2; ScreenCapture::captureLayers(&mCapture, captureArgs); expectFiltered({0, 0, 100, 200}, {100, 0, 200, 200}); @@ -149,7 +150,7 @@ TEST_F(TextureFilteringTest, LayerCaptureWithScalingIsFiltered) { // Expect no filtering because the output buffer's size matches the source crop. TEST_F(TextureFilteringTest, LayerCaptureOutputSourceCropNoFiltering) { captureArgs.layerHandle = mLayer->getHandle(); - captureArgs.sourceCrop = Rect{25, 25, 75, 75}; + captureArgs.captureArgs.sourceCrop = gui::aidl_utils::toARect(25, 25, 75, 75); ScreenCapture::captureLayers(&mCapture, captureArgs); mCapture->expectColor(Rect{0, 0, 25, 50}, Color::RED); @@ -162,7 +163,7 @@ TEST_F(TextureFilteringTest, LayerCaptureWithCropNoFiltering) { Transaction().setCrop(mLayer, Rect{10, 10, 90, 90}).apply(); captureArgs.layerHandle = mLayer->getHandle(); - captureArgs.sourceCrop = Rect{25, 25, 75, 75}; + captureArgs.captureArgs.sourceCrop = gui::aidl_utils::toARect(25, 25, 75, 75); ScreenCapture::captureLayers(&mCapture, captureArgs); mCapture->expectColor(Rect{0, 0, 25, 50}, Color::RED); @@ -172,7 +173,7 @@ TEST_F(TextureFilteringTest, LayerCaptureWithCropNoFiltering) { // Expect no filtering because the output source crop and output buffer are the same size. TEST_F(TextureFilteringTest, OutputSourceCropDisplayFrameMatchNoFiltering) { captureArgs.layerHandle = mLayer->getHandle(); - captureArgs.sourceCrop = Rect{25, 25, 75, 75}; + captureArgs.captureArgs.sourceCrop = gui::aidl_utils::toARect(25, 25, 75, 75); ScreenCapture::captureLayers(&mCapture, captureArgs); mCapture->expectColor(Rect{0, 0, 25, 50}, Color::RED); @@ -206,7 +207,7 @@ TEST_F(TextureFilteringTest, ParentHasTransformNoFiltering) { Transaction().setPosition(mParent, 100, 100).apply(); captureArgs.layerHandle = mParent->getHandle(); - captureArgs.sourceCrop = Rect{0, 0, 100, 100}; + captureArgs.captureArgs.sourceCrop = gui::aidl_utils::toARect(100, 100); ScreenCapture::captureLayers(&mCapture, captureArgs); mCapture->expectColor(Rect{0, 0, 50, 100}, Color::RED); diff --git a/services/surfaceflinger/tests/utils/ScreenshotUtils.h b/services/surfaceflinger/tests/utils/ScreenshotUtils.h index 1675584f5a..efce6b6b44 100644 --- a/services/surfaceflinger/tests/utils/ScreenshotUtils.h +++ b/services/surfaceflinger/tests/utils/ScreenshotUtils.h @@ -39,7 +39,7 @@ public: const auto sf = ComposerServiceAIDL::getComposerService(); SurfaceComposerClient::Transaction().apply(true); - captureArgs.dataspace = ui::Dataspace::V0_SRGB; + captureArgs.captureArgs.dataspace = static_cast(ui::Dataspace::V0_SRGB); const sp captureListener = sp::make(); binder::Status status = sf->captureDisplay(captureArgs, captureListener); status_t err = statusTFromBinderStatus(status); @@ -77,7 +77,7 @@ public: const auto sf = ComposerServiceAIDL::getComposerService(); SurfaceComposerClient::Transaction().apply(true); - captureArgs.dataspace = ui::Dataspace::V0_SRGB; + captureArgs.captureArgs.dataspace = static_cast(ui::Dataspace::V0_SRGB); const sp captureListener = sp::make(); binder::Status status = sf->captureLayers(captureArgs, captureListener); status_t err = statusTFromBinderStatus(status); -- cgit v1.2.3-59-g8ed1b