diff options
79 files changed, 906 insertions, 1592 deletions
diff --git a/cmds/servicemanager/ServiceManager.cpp b/cmds/servicemanager/ServiceManager.cpp index d85182d915..c44e540c6d 100644 --- a/cmds/servicemanager/ServiceManager.cpp +++ b/cmds/servicemanager/ServiceManager.cpp @@ -29,6 +29,7 @@ #include <thread> #if !defined(VENDORSERVICEMANAGER) && !defined(__ANDROID_RECOVERY__) +#include "perfetto/public/protos/trace/android/android_track_event.pzc.h" #include "perfetto/public/te_category_macros.h" #include "perfetto/public/te_macros.h" #endif // !defined(VENDORSERVICEMANAGER) && !defined(__ANDROID_RECOVERY__) @@ -57,6 +58,12 @@ PERFETTO_TE_CATEGORIES_DEFINE(PERFETTO_SM_CATEGORIES); #define SM_PERFETTO_TRACE_FUNC(...) \ PERFETTO_TE_SCOPED(servicemanager, PERFETTO_TE_SLICE_BEGIN(__func__) __VA_OPT__(, ) __VA_ARGS__) +constexpr uint32_t kProtoServiceName = + perfetto_protos_AndroidTrackEvent_binder_service_name_field_number; +constexpr uint32_t kProtoInterfaceName = + perfetto_protos_AndroidTrackEvent_binder_interface_name_field_number; +constexpr uint32_t kProtoApexName = perfetto_protos_AndroidTrackEvent_apex_name_field_number; + #endif // !(defined(VENDORSERVICEMANAGER) || defined(__ANDROID_RECOVERY__)) bool is_multiuser_uid_isolated(uid_t uid) { @@ -386,7 +393,8 @@ ServiceManager::~ServiceManager() { } Status ServiceManager::getService(const std::string& name, os::Service* outService) { - SM_PERFETTO_TRACE_FUNC(PERFETTO_TE_ARG_STRING("name", name.c_str())); + SM_PERFETTO_TRACE_FUNC(PERFETTO_TE_PROTO_FIELDS( + PERFETTO_TE_PROTO_FIELD_CSTR(kProtoServiceName, name.c_str()))); *outService = tryGetService(name, true); // returns ok regardless of result for legacy reasons @@ -394,7 +402,8 @@ Status ServiceManager::getService(const std::string& name, os::Service* outServi } Status ServiceManager::checkService(const std::string& name, os::Service* outService) { - SM_PERFETTO_TRACE_FUNC(PERFETTO_TE_ARG_STRING("name", name.c_str())); + SM_PERFETTO_TRACE_FUNC(PERFETTO_TE_PROTO_FIELDS( + PERFETTO_TE_PROTO_FIELD_CSTR(kProtoServiceName, name.c_str()))); *outService = tryGetService(name, false); // returns ok regardless of result for legacy reasons @@ -419,7 +428,8 @@ os::Service ServiceManager::tryGetService(const std::string& name, bool startIfN } sp<IBinder> ServiceManager::tryGetBinder(const std::string& name, bool startIfNotFound) { - SM_PERFETTO_TRACE_FUNC(PERFETTO_TE_ARG_STRING("name", name.c_str())); + SM_PERFETTO_TRACE_FUNC(PERFETTO_TE_PROTO_FIELDS( + PERFETTO_TE_PROTO_FIELD_CSTR(kProtoServiceName, name.c_str()))); auto ctx = mAccess->getCallingContext(); @@ -459,7 +469,8 @@ sp<IBinder> ServiceManager::tryGetBinder(const std::string& name, bool startIfNo } bool isValidServiceName(const std::string& name) { - SM_PERFETTO_TRACE_FUNC(PERFETTO_TE_ARG_STRING("name", name.c_str())); + SM_PERFETTO_TRACE_FUNC(PERFETTO_TE_PROTO_FIELDS( + PERFETTO_TE_PROTO_FIELD_CSTR(kProtoServiceName, name.c_str()))); if (name.size() == 0) return false; if (name.size() > 127) return false; @@ -476,7 +487,8 @@ bool isValidServiceName(const std::string& name) { } Status ServiceManager::addService(const std::string& name, const sp<IBinder>& binder, bool allowIsolated, int32_t dumpPriority) { - SM_PERFETTO_TRACE_FUNC(PERFETTO_TE_ARG_STRING("name", name.c_str())); + SM_PERFETTO_TRACE_FUNC(PERFETTO_TE_PROTO_FIELDS( + PERFETTO_TE_PROTO_FIELD_CSTR(kProtoServiceName, name.c_str()))); auto ctx = mAccess->getCallingContext(); @@ -599,7 +611,8 @@ Status ServiceManager::listServices(int32_t dumpPriority, std::vector<std::strin Status ServiceManager::registerForNotifications( const std::string& name, const sp<IServiceCallback>& callback) { - SM_PERFETTO_TRACE_FUNC(PERFETTO_TE_ARG_STRING("name", name.c_str())); + SM_PERFETTO_TRACE_FUNC(PERFETTO_TE_PROTO_FIELDS( + PERFETTO_TE_PROTO_FIELD_CSTR(kProtoServiceName, name.c_str()))); auto ctx = mAccess->getCallingContext(); @@ -650,7 +663,8 @@ Status ServiceManager::registerForNotifications( } Status ServiceManager::unregisterForNotifications( const std::string& name, const sp<IServiceCallback>& callback) { - SM_PERFETTO_TRACE_FUNC(PERFETTO_TE_ARG_STRING("name", name.c_str())); + SM_PERFETTO_TRACE_FUNC(PERFETTO_TE_PROTO_FIELDS( + PERFETTO_TE_PROTO_FIELD_CSTR(kProtoServiceName, name.c_str()))); auto ctx = mAccess->getCallingContext(); @@ -676,7 +690,8 @@ Status ServiceManager::unregisterForNotifications( } Status ServiceManager::isDeclared(const std::string& name, bool* outReturn) { - SM_PERFETTO_TRACE_FUNC(PERFETTO_TE_ARG_STRING("name", name.c_str())); + SM_PERFETTO_TRACE_FUNC(PERFETTO_TE_PROTO_FIELDS( + PERFETTO_TE_PROTO_FIELD_CSTR(kProtoServiceName, name.c_str()))); auto ctx = mAccess->getCallingContext(); @@ -694,7 +709,8 @@ Status ServiceManager::isDeclared(const std::string& name, bool* outReturn) { } binder::Status ServiceManager::getDeclaredInstances(const std::string& interface, std::vector<std::string>* outReturn) { - SM_PERFETTO_TRACE_FUNC(PERFETTO_TE_ARG_STRING("interface", interface.c_str())); + SM_PERFETTO_TRACE_FUNC(PERFETTO_TE_PROTO_FIELDS( + PERFETTO_TE_PROTO_FIELD_CSTR(kProtoInterfaceName, interface.c_str()))); auto ctx = mAccess->getCallingContext(); @@ -722,7 +738,8 @@ binder::Status ServiceManager::getDeclaredInstances(const std::string& interface Status ServiceManager::updatableViaApex(const std::string& name, std::optional<std::string>* outReturn) { - SM_PERFETTO_TRACE_FUNC(PERFETTO_TE_ARG_STRING("name", name.c_str())); + SM_PERFETTO_TRACE_FUNC(PERFETTO_TE_PROTO_FIELDS( + PERFETTO_TE_PROTO_FIELD_CSTR(kProtoServiceName, name.c_str()))); auto ctx = mAccess->getCallingContext(); @@ -741,7 +758,8 @@ Status ServiceManager::updatableViaApex(const std::string& name, Status ServiceManager::getUpdatableNames([[maybe_unused]] const std::string& apexName, std::vector<std::string>* outReturn) { - SM_PERFETTO_TRACE_FUNC(PERFETTO_TE_ARG_STRING("apexName", apexName.c_str())); + SM_PERFETTO_TRACE_FUNC(PERFETTO_TE_PROTO_FIELDS( + PERFETTO_TE_PROTO_FIELD_CSTR(kProtoApexName, apexName.c_str()))); auto ctx = mAccess->getCallingContext(); @@ -767,7 +785,8 @@ Status ServiceManager::getUpdatableNames([[maybe_unused]] const std::string& ape Status ServiceManager::getConnectionInfo(const std::string& name, std::optional<ConnectionInfo>* outReturn) { - SM_PERFETTO_TRACE_FUNC(PERFETTO_TE_ARG_STRING("name", name.c_str())); + SM_PERFETTO_TRACE_FUNC(PERFETTO_TE_PROTO_FIELDS( + PERFETTO_TE_PROTO_FIELD_CSTR(kProtoServiceName, name.c_str()))); auto ctx = mAccess->getCallingContext(); @@ -852,7 +871,8 @@ void ServiceManager::tryStartService(const Access::CallingContext& ctx, const st Status ServiceManager::registerClientCallback(const std::string& name, const sp<IBinder>& service, const sp<IClientCallback>& cb) { - SM_PERFETTO_TRACE_FUNC(PERFETTO_TE_ARG_STRING("name", name.c_str())); + SM_PERFETTO_TRACE_FUNC(PERFETTO_TE_PROTO_FIELDS( + PERFETTO_TE_PROTO_FIELD_CSTR(kProtoServiceName, name.c_str()))); if (cb == nullptr) { return Status::fromExceptionCode(Status::EX_NULL_POINTER, "Callback null."); @@ -1014,7 +1034,8 @@ void ServiceManager::sendClientCallbackNotifications(const std::string& serviceN } Status ServiceManager::tryUnregisterService(const std::string& name, const sp<IBinder>& binder) { - SM_PERFETTO_TRACE_FUNC(PERFETTO_TE_ARG_STRING("name", name.c_str())); + SM_PERFETTO_TRACE_FUNC(PERFETTO_TE_PROTO_FIELDS( + PERFETTO_TE_PROTO_FIELD_CSTR(kProtoServiceName, name.c_str()))); if (binder == nullptr) { return Status::fromExceptionCode(Status::EX_NULL_POINTER, "Null service."); diff --git a/include/ftl/fake_guard.h b/include/ftl/fake_guard.h index e6012516fc..0bf2870b69 100644 --- a/include/ftl/fake_guard.h +++ b/include/ftl/fake_guard.h @@ -76,12 +76,8 @@ struct [[clang::scoped_lockable]] FakeGuard final { FTL_ATTRIBUTE(release_capability(mutex)) #endif -// The parentheses around `expr` are needed to deduce an lvalue or rvalue reference. -#define FTL_FAKE_GUARD2(mutex, expr) \ - [&]() -> decltype(auto) { \ - const android::ftl::FakeGuard guard(mutex); \ - return (expr); \ - }() +#define FTL_FAKE_GUARD2(mutex, expr) \ + (android::ftl::FakeGuard(mutex), expr) #define FTL_MAKE_FAKE_GUARD(arg1, arg2, guard, ...) guard diff --git a/libs/binder/tests/Android.bp b/libs/binder/tests/Android.bp index 4c7684ccb7..dd50fbdc38 100644 --- a/libs/binder/tests/Android.bp +++ b/libs/binder/tests/Android.bp @@ -235,6 +235,16 @@ cc_defaults { "binder_test_defaults", ], + compile_multilib: "both", + multilib: { + lib32: { + suffix: "32", + }, + lib64: { + suffix: "64", + }, + }, + static_libs: [ "libbinder_test_utils", "libbinder_tls_static", @@ -267,7 +277,6 @@ cc_defaults { defaults: [ "binderRpcTest_common_defaults", ], - compile_multilib: "first", srcs: [ "binderRpcTest.cpp", diff --git a/libs/binder/tests/binderRpcTest.cpp b/libs/binder/tests/binderRpcTest.cpp index 3efd84f145..cd78e82322 100644 --- a/libs/binder/tests/binderRpcTest.cpp +++ b/libs/binder/tests/binderRpcTest.cpp @@ -19,6 +19,12 @@ #include <aidl/IBinderRpcTest.h> #endif +#if defined(__LP64__) +#define TEST_FILE_SUFFIX "64" +#else +#define TEST_FILE_SUFFIX "32" +#endif + #include <chrono> #include <cstdlib> #include <iostream> @@ -259,7 +265,8 @@ std::unique_ptr<ProcessSession> BinderRpc::createRpcTestSocketServerProcessEtc( std::string path = GetExecutableDirectory(); auto servicePath = path + "/binder_rpc_test_service" + - (singleThreaded ? "_single_threaded" : "") + (noKernel ? "_no_kernel" : ""); + (singleThreaded ? "_single_threaded" : "") + (noKernel ? "_no_kernel" : "") + + TEST_FILE_SUFFIX; unique_fd bootstrapClientFd, socketFd; diff --git a/libs/cputimeinstate/testtimeinstate.cpp b/libs/cputimeinstate/testtimeinstate.cpp index 81f6a585ab..44cdc02c0e 100644 --- a/libs/cputimeinstate/testtimeinstate.cpp +++ b/libs/cputimeinstate/testtimeinstate.cpp @@ -41,7 +41,7 @@ static constexpr uint64_t NSEC_PER_SEC = 1000000000; static constexpr uint64_t NSEC_PER_YEAR = NSEC_PER_SEC * 60 * 60 * 24 * 365; // Declare busy loop variable globally to prevent removal during optimization -static long sum __attribute__((used)) = 0; +static volatile long sum __attribute__((used)) = 1; using std::vector; @@ -579,8 +579,8 @@ uint64_t timeNanos() { // Keeps CPU busy with some number crunching void useCpu() { - sum = 0; - for (int i = 0; i < 100000; i++) { + sum = 1; + for (int i = 1; i < 100000; i++) { sum *= i; } } 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 <private/gui/ComposerService.h> #include <private/gui/ComposerServiceAIDL.h> -#include <android-base/stringprintf.h> #include <android-base/thread_annotations.h> -#include <sys/epoll.h> -#include <sys/eventfd.h> #include <chrono> #include <com_android_graphics_libgui_flags.h> 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<int64_t>::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<gui::BufferReleaseChannel::ConsumerEndpoint> 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<SurfaceControl>& 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<Fence> 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<Fence>& 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<BLASTBufferQueue> 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<BLASTBufferQueue> 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<BLASTBufferQueue> 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<BLASTBufferQueue> 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<BLASTBufferQueue> 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<BLASTBufferQueue> 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<Fence>* outFence, uint32_t width, uint32_t height, - PixelFormat format, uint64_t usage, uint64_t* outBufferAge, - FrameEventHistoryDelta* outTimestamps) { - sp<BLASTBufferQueue> 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<std::chrono::milliseconds>( - maxWaitTime - std::chrono::steady_clock::now()); - if (timeout <= 0ms) { - break; - } - - ReleaseCallbackId releaseCallbackId; - sp<Fence> 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<BLASTBufferQueue> mBLASTBufferQueue; }; @@ -1400,16 +1173,6 @@ void BLASTBufferQueue::createBufferQueue(sp<IGraphicBufferProducer>* 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<gui::BufferReleaseChannel::ConsumerEndpoint> 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<Fence>& outFence) { - std::lock_guard lock{mMutex}; - return mEndpoint->readReleaseFence(outId, outFence); -} - -status_t BLASTBufferQueue::BufferReleaseReader::readBlocking(ReleaseCallbackId& outId, - sp<Fence>& 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 <errno.h> -#include <fcntl.h> -#include <math.h> -#include <sys/socket.h> -#include <sys/types.h> -#include <unistd.h> - -#include <android-base/logging.h> -#include <android-base/properties.h> -#include <android-base/stringprintf.h> -#include <android/binder_status.h> -#include <binder/Parcel.h> -#include <cutils/properties.h> -#include <ftl/enum.h> -#include <log/log.h> -#include <utils/Trace.h> - -#include <gui/TraceUtils.h> -#include <private/gui/ParcelUtils.h> - -#include <gui/BufferReleaseChannel.h> - -using android::base::Result; - -namespace android::gui { - -namespace { - -template <typename T> -static void readAligned(const void*& buffer, size_t& size, T& value) { - size -= FlattenableUtils::align<alignof(T)>(buffer); - FlattenableUtils::read(buffer, size, value); -} - -template <typename T> -static void writeAligned(void*& buffer, size_t& size, T value) { - size -= FlattenableUtils::align<alignof(T)>(buffer); - FlattenableUtils::write(buffer, size, value); -} - -template <typename T> -static void addAligned(size_t& size, T /* value */) { - size = FlattenableUtils::align<sizeof(T)>(size); - size += sizeof(T); -} - -template <typename T> -static inline constexpr uint32_t low32(const T n) { - return static_cast<uint32_t>(static_cast<uint64_t>(n)); -} - -template <typename T> -static inline constexpr uint32_t high32(const T n) { - return static_cast<uint32_t>(static_cast<uint64_t>(n) >> 32); -} - -template <typename T> -static inline constexpr T to64(const uint32_t lo, const uint32_t hi) { - return static_cast<T>(static_cast<uint64_t>(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<int64_t>(bufferIdLo, bufferIdHi); - readAligned(buffer, size, frameNumberLo); - readAligned(buffer, size, frameNumberHi); - releaseCallbackId.framenumber = to64<uint64_t>(frameNumberLo, frameNumberHi); - - return NO_ERROR; -} - -status_t BufferReleaseChannel::ConsumerEndpoint::readReleaseFence( - ReleaseCallbackId& outReleaseCallbackId, sp<Fence>& outReleaseFence) { - Message releaseBufferMessage; - mFlattenedBuffer.resize(releaseBufferMessage.getFlattenedSize()); - std::array<uint8_t, CMSG_SPACE(sizeof(int))> 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<const void*>(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<const int*>(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>& 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<uint8_t, CMSG_SPACE(sizeof(int))> 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<ConsumerEndpoint>& outConsumer, - std::shared_ptr<ProducerEndpoint>& 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<ConsumerEndpoint>(name, std::move(consumerFd)); - outProducer = std::make_shared<ProducerEndpoint>(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<int32_t>(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<gui::CachingHint>(tmpInt32); - bool hasBufferReleaseChannel; - SAFE_PARCEL(input.readBool, &hasBufferReleaseChannel); - if (hasBufferReleaseChannel) { - bufferReleaseChannel = std::make_shared<gui::BufferReleaseChannel::ProducerEndpoint>(); - 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/OWNERS b/libs/gui/OWNERS index 070f6bf532..5a9644b77f 100644 --- a/libs/gui/OWNERS +++ b/libs/gui/OWNERS @@ -9,10 +9,10 @@ per-file EndToEndNativeInputTest.cpp = svv@google.com # BufferQueue is feature-frozen per-file BufferQueue* = set noparent -per-file BufferQueue* = jreck@google.com, sumir@google.com, alecmouri@google.com +per-file BufferQueue* = jreck@google.com, sumir@google.com, alecmouri@google.com, jshargo@google.com, carlosmr@google.com per-file IGraphicBuffer* = set noparent -per-file IGraphicBuffer* = jreck@google.com, sumir@google.com, alecmouri@google.com +per-file IGraphicBuffer* = jreck@google.com, sumir@google.com, alecmouri@google.com, jshargo@google.com, carlosmr@google.com per-file include/gui/BufferQueue* = set noparent -per-file include/gui/BufferQueue* = jreck@google.com, sumir@google.com, alecmouri@google.com +per-file include/gui/BufferQueue* = jreck@google.com, sumir@google.com, alecmouri@google.com, jshargo@google.com, carlosmr@google.com per-file include/gui/IGraphicBuffer* = set noparent -per-file include/gui/IGraphicBuffer* = jreck@google.com, sumir@google.com, alecmouri@google.com +per-file include/gui/IGraphicBuffer* = jreck@google.com, sumir@google.com, alecmouri@google.com, jshargo@google.com, carlosmr@google.com diff --git a/libs/gui/Surface.cpp b/libs/gui/Surface.cpp index 685391effc..eb9faf584b 100644 --- a/libs/gui/Surface.cpp +++ b/libs/gui/Surface.cpp @@ -21,6 +21,8 @@ #include <gui/Surface.h> #include <condition_variable> +#include <cstddef> +#include <cstdint> #include <deque> #include <mutex> #include <thread> @@ -161,6 +163,12 @@ void Surface::allocateBuffers() { mReqFormat, mReqUsage); } +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_PLATFORM_API_IMPROVEMENTS) +status_t Surface::allowAllocation(bool allowAllocation) { + return mGraphicBufferProducer->allowAllocation(allowAllocation); +} +#endif // COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_PLATFORM_API_IMPROVEMENTS) + status_t Surface::setGenerationNumber(uint32_t generation) { status_t result = mGraphicBufferProducer->setGenerationNumber(generation); if (result == NO_ERROR) { @@ -693,6 +701,50 @@ int Surface::dequeueBuffer(android_native_buffer_t** buffer, int* fenceFd) { return OK; } +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_PLATFORM_API_IMPROVEMENTS) + +status_t Surface::dequeueBuffer(sp<GraphicBuffer>* buffer, sp<Fence>* outFence) { + if (buffer == nullptr || outFence == nullptr) { + return BAD_VALUE; + } + + android_native_buffer_t* anb; + int fd = -1; + status_t res = dequeueBuffer(&anb, &fd); + *buffer = GraphicBuffer::from(anb); + *outFence = sp<Fence>::make(fd); + return res; +} + +status_t Surface::queueBuffer(const sp<GraphicBuffer>& buffer, const sp<Fence>& fd) { + if (buffer == nullptr) { + return BAD_VALUE; + } + return queueBuffer(buffer.get(), fd ? fd->get() : -1); +} + +status_t Surface::detachBuffer(const sp<GraphicBuffer>& buffer) { + if (nullptr == buffer) { + return BAD_VALUE; + } + + Mutex::Autolock lock(mMutex); + + uint64_t bufferId = buffer->getId(); + for (int slot = 0; slot < Surface::NUM_BUFFER_SLOTS; ++slot) { + auto& bufferSlot = mSlots[slot]; + if (bufferSlot.buffer != nullptr && bufferSlot.buffer->getId() == bufferId) { + bufferSlot.buffer = nullptr; + bufferSlot.dirtyRegion = Region::INVALID_REGION; + return mGraphicBufferProducer->detachBuffer(slot); + } + } + + return BAD_VALUE; +} + +#endif // COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_PLATFORM_API_IMPROVEMENTS) + int Surface::dequeueBuffers(std::vector<BatchBuffer>* buffers) { using DequeueBufferInput = IGraphicBufferProducer::DequeueBufferInput; using DequeueBufferOutput = IGraphicBufferProducer::DequeueBufferOutput; 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<SurfaceControl>& sc, - const std::shared_ptr<gui::BufferReleaseChannel::ProducerEndpoint>& 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<IBinder>& 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 <gui/BufferItem.h> #include <gui/BufferItemConsumer.h> -#include <gui/BufferReleaseChannel.h> + #include <gui/IGraphicBufferProducer.h> #include <gui/SurfaceComposerClient.h> @@ -28,6 +28,7 @@ #include <utils/RefBase.h> #include <system/window.h> +#include <thread> #include <queue> #include <com_android_graphics_libgui_flags.h> @@ -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<void(const std::string&)> mTransactionHangCallback; std::unordered_set<uint64_t> mSyncedFrameNumbers GUARDED_BY(mMutex); - - class BufferReleaseReader { - public: - BufferReleaseReader(std::unique_ptr<gui::BufferReleaseChannel::ConsumerEndpoint>); - - 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<Fence>&, std::chrono::milliseconds timeout); - - status_t readNonBlocking(ReleaseCallbackId&, sp<Fence>&); - - void interruptBlockingRead(); - - private: - std::mutex mMutex; - std::unique_ptr<gui::BufferReleaseChannel::ConsumerEndpoint> 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<BufferReleaseReader> mBufferReleaseReader; - std::shared_ptr<gui::BufferReleaseChannel::ProducerEndpoint> 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 <string> - -#include <android-base/chrono_utils.h> -#include <android-base/result.h> -#include <android-base/unique_fd.h> - -#include <binder/IBinder.h> -#include <binder/Parcelable.h> -#include <gui/ITransactionCompletedListener.h> -#include <sys/stat.h> -#include <ui/Fence.h> -#include <ui/Transform.h> -#include <utils/BitSet.h> -#include <utils/Errors.h> -#include <utils/RefBase.h> -#include <utils/Timers.h> - -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<Fence>& outReleaseFence); - - private: - std::vector<uint8_t> 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<Fence>& releaseFence); - - private: - std::vector<uint8_t> mFlattenedBuffer; - }; - - /** - * Create two endpoints that make up the BufferReleaseChannel. - * - * Return OK on success. - */ - static status_t open(const std::string name, std::unique_ptr<ConsumerEndpoint>& outConsumer, - std::shared_ptr<ProducerEndpoint>& outProducer); - - struct Message : public Flattenable<Message> { - ReleaseCallbackId releaseCallbackId; - sp<Fence> releaseFence = Fence::NO_FENCE; - - Message() = default; - Message(ReleaseCallbackId releaseCallbackId, sp<Fence> 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 <android/gui/TrustedOverlay.h> #include <ftl/flags.h> -#include <gui/BufferReleaseChannel.h> #include <gui/DisplayCaptureArgs.h> #include <gui/ISurfaceComposer.h> #include <gui/LayerCaptureArgs.h> @@ -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<gui::BufferReleaseChannel::ProducerEndpoint> bufferReleaseChannel; }; class ComposerState { diff --git a/libs/gui/include/gui/Surface.h b/libs/gui/include/gui/Surface.h index 124550113e..d07e121d6e 100644 --- a/libs/gui/include/gui/Surface.h +++ b/libs/gui/include/gui/Surface.h @@ -18,6 +18,7 @@ #define ANDROID_GUI_SURFACE_H #include <android/gui/FrameTimelineInfo.h> +#include <com_android_graphics_libgui_flags.h> #include <gui/BufferQueueDefs.h> #include <gui/HdrMetadata.h> #include <gui/IGraphicBufferProducer.h> @@ -35,6 +36,8 @@ namespace android { +class GraphicBuffer; + namespace gui { class ISurfaceComposer; } // namespace gui @@ -164,6 +167,11 @@ public: */ virtual void allocateBuffers(); +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_PLATFORM_API_IMPROVEMENTS) + // See IGraphicBufferProducer::allowAllocation + status_t allowAllocation(bool allowAllocation); +#endif // COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_PLATFORM_API_IMPROVEMENTS) + /* Sets the generation number on the IGraphicBufferProducer and updates the * generation number on any buffers attached to the Surface after this call. * See IGBP::setGenerationNumber for more information. */ @@ -395,6 +403,20 @@ public: static status_t attachAndQueueBufferWithDataspace(Surface* surface, sp<GraphicBuffer> buffer, ui::Dataspace dataspace); +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_PLATFORM_API_IMPROVEMENTS) + // Dequeues a buffer and its outFence, which must be signalled before the buffer can be used. + status_t dequeueBuffer(sp<GraphicBuffer>* buffer, sp<Fence>* outFence); + + // Queues a buffer, with an optional fd fence that captures pending work on the buffer. This + // buffer must have been returned by dequeueBuffer or associated with this Surface via an + // attachBuffer operation. + status_t queueBuffer(const sp<GraphicBuffer>& buffer, const sp<Fence>& fd = Fence::NO_FENCE); + + // Detaches this buffer, dissociating it from this Surface. This buffer must have been returned + // by queueBuffer or associated with this Surface via an attachBuffer operation. + status_t detachBuffer(const sp<GraphicBuffer>& buffer); +#endif // COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_PLATFORM_API_IMPROVEMENTS) + // Batch version of dequeueBuffer, cancelBuffer and queueBuffer // Note that these batched operations are not supported when shared buffer mode is being used. struct BatchBuffer { 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 <android/gui/BnJankListener.h> #include <android/gui/ISurfaceComposerClient.h> -#include <gui/BufferReleaseChannel.h> #include <gui/CpuConsumer.h> #include <gui/ISurfaceComposer.h> #include <gui/ITransactionCompletedListener.h> @@ -764,10 +763,6 @@ public: const Rect& destinationFrame); Transaction& setDropInputMode(const sp<SurfaceControl>& sc, gui::DropInputMode mode); - Transaction& setBufferReleaseChannel( - const sp<SurfaceControl>& sc, - const std::shared_ptr<gui::BufferReleaseChannel::ProducerEndpoint>& channel); - status_t setDisplaySurface(const sp<IBinder>& token, const sp<IGraphicBufferProducer>& bufferProducer); diff --git a/libs/gui/tests/Android.bp b/libs/gui/tests/Android.bp index daaddfbc15..b342a7db0d 100644 --- a/libs/gui/tests/Android.bp +++ b/libs/gui/tests/Android.bp @@ -25,6 +25,7 @@ cc_test { "-Wthread-safety", "-DCOM_ANDROID_GRAPHICS_LIBGUI_FLAGS_BQ_SETFRAMERATE=true", "-DCOM_ANDROID_GRAPHICS_LIBGUI_FLAGS_BQ_EXTENDEDALLOCATE=true", + "-DCOM_ANDROID_GRAPHICS_LIBGUI_FLAGS_WB_PLATFORM_API_IMPROVEMENTS=true", ], srcs: [ @@ -32,7 +33,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 <string> -#include <vector> - -#include <gtest/gtest.h> -#include <gui/BufferReleaseChannel.h> - -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<Fence> releaseFence = sp<Fence>::make(memfd_create("fake-fence-fd", 0)); - - std::vector<uint8_t> dataBuffer; - std::vector<int> 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<BufferReleaseChannel::ConsumerEndpoint> consumer; - std::shared_ptr<BufferReleaseChannel::ProducerEndpoint> producer; - ASSERT_EQ(OK, BufferReleaseChannel::open("test-channel"s, consumer, producer)); - - ReleaseCallbackId releaseCallbackId; - sp<Fence> 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<BufferReleaseChannel::ConsumerEndpoint> consumer; - std::shared_ptr<BufferReleaseChannel::ProducerEndpoint> producer; - ASSERT_EQ(OK, BufferReleaseChannel::open("test-channel"s, consumer, producer)); - - ReleaseCallbackId producerReleaseCallbackId{1, 2}; - sp<Fence> producerReleaseFence = sp<Fence>::make(memfd_create("fake-fence-fd", 0)); - ASSERT_EQ(OK, producer->writeReleaseFence(producerReleaseCallbackId, producerReleaseFence)); - - ReleaseCallbackId consumerReleaseCallbackId; - sp<Fence> 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/libs/gui/tests/Surface_test.cpp b/libs/gui/tests/Surface_test.cpp index 88168e3955..8d6917f3a6 100644 --- a/libs/gui/tests/Surface_test.cpp +++ b/libs/gui/tests/Surface_test.cpp @@ -24,23 +24,26 @@ #include <android/gui/ISurfaceComposer.h> #include <android/hardware/configstore/1.0/ISurfaceFlingerConfigs.h> #include <binder/ProcessState.h> +#include <com_android_graphics_libgui_flags.h> #include <configstore/Utils.h> #include <gui/AidlStatusUtil.h> #include <gui/BufferItemConsumer.h> +#include <gui/CpuConsumer.h> #include <gui/ISurfaceComposer.h> #include <gui/Surface.h> #include <gui/SurfaceComposerClient.h> #include <gui/SyncScreenCaptureListener.h> -#include <inttypes.h> #include <private/gui/ComposerService.h> #include <private/gui/ComposerServiceAIDL.h> #include <sys/types.h> #include <ui/BufferQueueDefs.h> #include <ui/DisplayMode.h> +#include <ui/GraphicBuffer.h> #include <ui/Rect.h> #include <utils/Errors.h> #include <utils/String8.h> +#include <cstddef> #include <limits> #include <thread> @@ -2225,4 +2228,90 @@ TEST_F(SurfaceTest, BatchIllegalOperations) { ASSERT_EQ(NO_ERROR, surface->disconnect(NATIVE_WINDOW_API_CPU)); } +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_PLATFORM_API_IMPROVEMENTS) + +TEST_F(SurfaceTest, PlatformBufferMethods) { + sp<IGraphicBufferProducer> producer; + sp<IGraphicBufferConsumer> consumer; + BufferQueue::createBufferQueue(&producer, &consumer); + + sp<CpuConsumer> cpuConsumer = sp<CpuConsumer>::make(consumer, 1); + sp<Surface> surface = sp<Surface>::make(producer); + sp<StubSurfaceListener> listener = sp<StubSurfaceListener>::make(); + sp<GraphicBuffer> buffer; + sp<Fence> fence; + + EXPECT_EQ(OK, + surface->connect(NATIVE_WINDOW_API_CPU, listener, /* reportBufferRemoval */ false)); + + // + // Verify nullptrs are handled safely: + // + + EXPECT_EQ(BAD_VALUE, surface->dequeueBuffer((sp<GraphicBuffer>*)nullptr, nullptr)); + EXPECT_EQ(BAD_VALUE, surface->dequeueBuffer((sp<GraphicBuffer>*)nullptr, &fence)); + EXPECT_EQ(BAD_VALUE, surface->dequeueBuffer(&buffer, nullptr)); + EXPECT_EQ(BAD_VALUE, surface->queueBuffer(nullptr, nullptr)); + EXPECT_EQ(BAD_VALUE, surface->detachBuffer(nullptr)); + + // + // Verify dequeue/queue: + // + + EXPECT_EQ(OK, surface->dequeueBuffer(&buffer, &fence)); + EXPECT_NE(nullptr, buffer); + EXPECT_EQ(OK, surface->queueBuffer(buffer, fence)); + + // + // Verify dequeue/detach: + // + + wp<GraphicBuffer> weakBuffer; + { + EXPECT_EQ(OK, surface->dequeueBuffer(&buffer, &fence)); + + EXPECT_EQ(OK, surface->detachBuffer(buffer)); + + weakBuffer = buffer; + buffer = nullptr; + } + EXPECT_EQ(nullptr, weakBuffer.promote()) << "Weak buffer still held by Surface."; + + // + // Verify detach without borrowing the buffer does not work: + // + + sp<GraphicBuffer> heldTooLongBuffer; + EXPECT_EQ(OK, surface->dequeueBuffer(&heldTooLongBuffer, &fence)); + EXPECT_EQ(OK, surface->queueBuffer(heldTooLongBuffer)); + EXPECT_EQ(BAD_VALUE, surface->detachBuffer(heldTooLongBuffer)); +} + +TEST_F(SurfaceTest, AllowAllocation) { + sp<IGraphicBufferProducer> producer; + sp<IGraphicBufferConsumer> consumer; + BufferQueue::createBufferQueue(&producer, &consumer); + + // controlledByApp must be true to disable blocking + sp<CpuConsumer> cpuConsumer = sp<CpuConsumer>::make(consumer, 1, /*controlledByApp*/ true); + sp<Surface> surface = sp<Surface>::make(producer, /*controlledByApp*/ true); + sp<StubSurfaceListener> listener = sp<StubSurfaceListener>::make(); + sp<GraphicBuffer> buffer; + sp<Fence> fence; + + EXPECT_EQ(OK, + surface->connect(NATIVE_WINDOW_API_CPU, listener, /* reportBufferRemoval */ false)); + EXPECT_EQ(OK, surface->allowAllocation(false)); + + EXPECT_EQ(OK, surface->setDequeueTimeout(-1)); + EXPECT_EQ(WOULD_BLOCK, surface->dequeueBuffer(&buffer, &fence)); + + EXPECT_EQ(OK, surface->setDequeueTimeout(10)); + EXPECT_EQ(TIMED_OUT, surface->dequeueBuffer(&buffer, &fence)); + + EXPECT_EQ(OK, surface->allowAllocation(true)); + EXPECT_EQ(OK, surface->dequeueBuffer(&buffer, &fence)); +} +#endif // COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_PLATFORM_API_IMPROVEMENTS) + } // namespace android diff --git a/services/inputflinger/reader/EventHub.cpp b/services/inputflinger/reader/EventHub.cpp index e48e94f372..e11adb8c76 100644 --- a/services/inputflinger/reader/EventHub.cpp +++ b/services/inputflinger/reader/EventHub.cpp @@ -517,10 +517,10 @@ ftl::Flags<InputDeviceClass> getAbsAxisUsage(int32_t axis, // --- RawAbsoluteAxisInfo --- -std::ostream& operator<<(std::ostream& out, const RawAbsoluteAxisInfo& info) { - if (info.valid) { - out << "min=" << info.minValue << ", max=" << info.maxValue << ", flat=" << info.flat - << ", fuzz=" << info.fuzz << ", resolution=" << info.resolution; +std::ostream& operator<<(std::ostream& out, const std::optional<RawAbsoluteAxisInfo>& info) { + if (info) { + out << "min=" << info->minValue << ", max=" << info->maxValue << ", flat=" << info->flat + << ", fuzz=" << info->fuzz << ", resolution=" << info->resolution; } else { out << "unknown range"; } @@ -649,7 +649,6 @@ void EventHub::Device::populateAbsoluteAxisStates() { continue; } auto& [axisInfo, value] = absState[axis]; - axisInfo.valid = true; axisInfo.minValue = info.minimum; axisInfo.maxValue = info.maximum; axisInfo.flat = info.flat; diff --git a/services/inputflinger/reader/include/EventHub.h b/services/inputflinger/reader/include/EventHub.h index feae6b6afc..657126a825 100644 --- a/services/inputflinger/reader/include/EventHub.h +++ b/services/inputflinger/reader/include/EventHub.h @@ -71,18 +71,14 @@ struct RawEvent { /* Describes an absolute axis. */ struct RawAbsoluteAxisInfo { - bool valid{false}; // true if the information is valid, false otherwise - int32_t minValue{}; // minimum value int32_t maxValue{}; // maximum value int32_t flat{}; // center flat position, eg. flat == 8 means center is between -8 and 8 int32_t fuzz{}; // error tolerance, eg. fuzz == 4 means value is +/- 4 due to noise int32_t resolution{}; // resolution in units per mm or radians per mm - - inline void clear() { *this = RawAbsoluteAxisInfo(); } }; -std::ostream& operator<<(std::ostream& out, const RawAbsoluteAxisInfo& info); +std::ostream& operator<<(std::ostream& out, const std::optional<RawAbsoluteAxisInfo>& info); /* * Input device classes. diff --git a/services/inputflinger/reader/include/InputDevice.h b/services/inputflinger/reader/include/InputDevice.h index 086c26f5e2..1aed8a8394 100644 --- a/services/inputflinger/reader/include/InputDevice.h +++ b/services/inputflinger/reader/include/InputDevice.h @@ -43,7 +43,7 @@ class InputDevice { public: InputDevice(InputReaderContext* context, int32_t id, int32_t generation, const InputDeviceIdentifier& identifier); - ~InputDevice(); + virtual ~InputDevice(); inline InputReaderContext* getContext() { return mContext; } inline int32_t getId() const { return mId; } @@ -56,7 +56,7 @@ public: } inline const std::string getLocation() const { return mIdentifier.location; } inline ftl::Flags<InputDeviceClass> getClasses() const { return mClasses; } - inline uint32_t getSources() const { return mSources; } + inline virtual uint32_t getSources() const { return mSources; } inline bool hasEventHubDevices() const { return !mDevices.empty(); } inline bool isExternal() { return mIsExternal; } @@ -132,7 +132,7 @@ public: [[nodiscard]] NotifyDeviceResetArgs notifyReset(nsecs_t when); - inline const PropertyMap& getConfiguration() { return mConfiguration; } + inline virtual const PropertyMap& getConfiguration() const { return mConfiguration; } inline EventHubInterface* getEventHub() { return mContext->getEventHub(); } std::optional<ui::LogicalDisplayId> getAssociatedDisplayId(); @@ -299,28 +299,24 @@ public: inline ftl::Flags<InputDeviceClass> getDeviceClasses() const { return mEventHub->getDeviceClasses(mId); } + inline uint32_t getDeviceSources() const { return mDevice.getSources(); } inline InputDeviceIdentifier getDeviceIdentifier() const { return mEventHub->getDeviceIdentifier(mId); } inline int32_t getDeviceControllerNumber() const { return mEventHub->getDeviceControllerNumber(mId); } - inline status_t getAbsoluteAxisInfo(int32_t code, RawAbsoluteAxisInfo* axisInfo) const { + inline std::optional<RawAbsoluteAxisInfo> getAbsoluteAxisInfo(int32_t code) const { std::optional<RawAbsoluteAxisInfo> info = mEventHub->getAbsoluteAxisInfo(mId, code); - if (!info.has_value()) { - axisInfo->clear(); - return NAME_NOT_FOUND; - } - *axisInfo = *info; // Validate axis info for InputDevice. - if (axisInfo->valid && axisInfo->minValue == axisInfo->maxValue) { + if (info && info->minValue == info->maxValue) { // Historically, we deem axes with the same min and max values as invalid to avoid // dividing by zero when scaling by max - min. // TODO(b/291772515): Perform axis info validation on a per-axis basis when it is used. - axisInfo->valid = false; + return std::nullopt; } - return OK; + return info; } inline bool hasRelativeAxis(int32_t code) const { return mEventHub->hasRelativeAxis(mId, code); @@ -435,8 +431,7 @@ public: } inline bool hasAbsoluteAxis(int32_t code) const { - std::optional<RawAbsoluteAxisInfo> info = mEventHub->getAbsoluteAxisInfo(mId, code); - return info.has_value() && info->valid; + return mEventHub->getAbsoluteAxisInfo(mId, code).has_value(); } inline bool isKeyPressed(int32_t scanCode) const { return mEventHub->getScanCodeState(mId, scanCode) == AKEY_STATE_DOWN; diff --git a/services/inputflinger/reader/mapper/CapturedTouchpadEventConverter.cpp b/services/inputflinger/reader/mapper/CapturedTouchpadEventConverter.cpp index 90685dec2b..c8e7790c86 100644 --- a/services/inputflinger/reader/mapper/CapturedTouchpadEventConverter.cpp +++ b/services/inputflinger/reader/mapper/CapturedTouchpadEventConverter.cpp @@ -16,6 +16,7 @@ #include "CapturedTouchpadEventConverter.h" +#include <optional> #include <sstream> #include <android-base/stringprintf.h> @@ -53,32 +54,33 @@ CapturedTouchpadEventConverter::CapturedTouchpadEventConverter( mMotionAccumulator(motionAccumulator), mHasTouchMinor(deviceContext.hasAbsoluteAxis(ABS_MT_TOUCH_MINOR)), mHasToolMinor(deviceContext.hasAbsoluteAxis(ABS_MT_WIDTH_MINOR)) { - RawAbsoluteAxisInfo orientationInfo; - deviceContext.getAbsoluteAxisInfo(ABS_MT_ORIENTATION, &orientationInfo); - if (orientationInfo.valid) { - if (orientationInfo.maxValue > 0) { - mOrientationScale = M_PI_2 / orientationInfo.maxValue; - } else if (orientationInfo.minValue < 0) { - mOrientationScale = -M_PI_2 / orientationInfo.minValue; + if (std::optional<RawAbsoluteAxisInfo> orientation = + deviceContext.getAbsoluteAxisInfo(ABS_MT_ORIENTATION); + orientation) { + if (orientation->maxValue > 0) { + mOrientationScale = M_PI_2 / orientation->maxValue; + } else if (orientation->minValue < 0) { + mOrientationScale = -M_PI_2 / orientation->minValue; } } // TODO(b/275369880): support touch.pressure.calibration and .scale properties when captured. - RawAbsoluteAxisInfo pressureInfo; - deviceContext.getAbsoluteAxisInfo(ABS_MT_PRESSURE, &pressureInfo); - if (pressureInfo.valid && pressureInfo.maxValue > 0) { - mPressureScale = 1.0 / pressureInfo.maxValue; + if (std::optional<RawAbsoluteAxisInfo> pressure = + deviceContext.getAbsoluteAxisInfo(ABS_MT_PRESSURE); + pressure && pressure->maxValue > 0) { + mPressureScale = 1.0 / pressure->maxValue; } - RawAbsoluteAxisInfo touchMajorInfo, toolMajorInfo; - deviceContext.getAbsoluteAxisInfo(ABS_MT_TOUCH_MAJOR, &touchMajorInfo); - deviceContext.getAbsoluteAxisInfo(ABS_MT_WIDTH_MAJOR, &toolMajorInfo); - mHasTouchMajor = touchMajorInfo.valid; - mHasToolMajor = toolMajorInfo.valid; - if (mHasTouchMajor && touchMajorInfo.maxValue != 0) { - mSizeScale = 1.0f / touchMajorInfo.maxValue; - } else if (mHasToolMajor && toolMajorInfo.maxValue != 0) { - mSizeScale = 1.0f / toolMajorInfo.maxValue; + std::optional<RawAbsoluteAxisInfo> touchMajor = + deviceContext.getAbsoluteAxisInfo(ABS_MT_TOUCH_MAJOR); + std::optional<RawAbsoluteAxisInfo> toolMajor = + deviceContext.getAbsoluteAxisInfo(ABS_MT_WIDTH_MAJOR); + mHasTouchMajor = touchMajor.has_value(); + mHasToolMajor = toolMajor.has_value(); + if (mHasTouchMajor && touchMajor->maxValue != 0) { + mSizeScale = 1.0f / touchMajor->maxValue; + } else if (mHasToolMajor && toolMajor->maxValue != 0) { + mSizeScale = 1.0f / toolMajor->maxValue; } } @@ -113,15 +115,13 @@ void CapturedTouchpadEventConverter::populateMotionRanges(InputDeviceInfo& info) tryAddRawMotionRange(/*byref*/ info, AMOTION_EVENT_AXIS_TOOL_MAJOR, ABS_MT_WIDTH_MAJOR); tryAddRawMotionRange(/*byref*/ info, AMOTION_EVENT_AXIS_TOOL_MINOR, ABS_MT_WIDTH_MINOR); - RawAbsoluteAxisInfo pressureInfo; - mDeviceContext.getAbsoluteAxisInfo(ABS_MT_PRESSURE, &pressureInfo); - if (pressureInfo.valid) { + if (mDeviceContext.hasAbsoluteAxis(ABS_MT_PRESSURE)) { info.addMotionRange(AMOTION_EVENT_AXIS_PRESSURE, SOURCE, 0, 1, 0, 0, 0); } - RawAbsoluteAxisInfo orientationInfo; - mDeviceContext.getAbsoluteAxisInfo(ABS_MT_ORIENTATION, &orientationInfo); - if (orientationInfo.valid && (orientationInfo.maxValue > 0 || orientationInfo.minValue < 0)) { + if (std::optional<RawAbsoluteAxisInfo> orientation = + mDeviceContext.getAbsoluteAxisInfo(ABS_MT_ORIENTATION); + orientation && (orientation->maxValue > 0 || orientation->minValue < 0)) { info.addMotionRange(AMOTION_EVENT_AXIS_ORIENTATION, SOURCE, -M_PI_2, M_PI_2, 0, 0, 0); } @@ -133,11 +133,10 @@ void CapturedTouchpadEventConverter::populateMotionRanges(InputDeviceInfo& info) void CapturedTouchpadEventConverter::tryAddRawMotionRange(InputDeviceInfo& deviceInfo, int32_t androidAxis, int32_t evdevAxis) const { - RawAbsoluteAxisInfo info; - mDeviceContext.getAbsoluteAxisInfo(evdevAxis, &info); - if (info.valid) { - deviceInfo.addMotionRange(androidAxis, SOURCE, info.minValue, info.maxValue, info.flat, - info.fuzz, info.resolution); + std::optional<RawAbsoluteAxisInfo> info = mDeviceContext.getAbsoluteAxisInfo(evdevAxis); + if (info) { + deviceInfo.addMotionRange(androidAxis, SOURCE, info->minValue, info->maxValue, info->flat, + info->fuzz, info->resolution); } } diff --git a/services/inputflinger/reader/mapper/ExternalStylusInputMapper.cpp b/services/inputflinger/reader/mapper/ExternalStylusInputMapper.cpp index 3af1d04073..7cc8940379 100644 --- a/services/inputflinger/reader/mapper/ExternalStylusInputMapper.cpp +++ b/services/inputflinger/reader/mapper/ExternalStylusInputMapper.cpp @@ -33,7 +33,7 @@ uint32_t ExternalStylusInputMapper::getSources() const { void ExternalStylusInputMapper::populateDeviceInfo(InputDeviceInfo& info) { InputMapper::populateDeviceInfo(info); - if (mRawPressureAxis.valid) { + if (mRawPressureAxis) { info.addMotionRange(AMOTION_EVENT_AXIS_PRESSURE, AINPUT_SOURCE_STYLUS, 0.0f, 1.0f, 0.0f, 0.0f, 0.0f); } @@ -50,7 +50,7 @@ void ExternalStylusInputMapper::dump(std::string& dump) { std::list<NotifyArgs> ExternalStylusInputMapper::reconfigure(nsecs_t when, const InputReaderConfiguration& config, ConfigurationChanges changes) { - getAbsoluteAxisInfo(ABS_PRESSURE, &mRawPressureAxis); + mRawPressureAxis = getAbsoluteAxisInfo(ABS_PRESSURE); mTouchButtonAccumulator.configure(); return {}; } @@ -82,10 +82,10 @@ std::list<NotifyArgs> ExternalStylusInputMapper::sync(nsecs_t when) { mStylusState.toolType = ToolType::STYLUS; } - if (mRawPressureAxis.valid) { + if (mRawPressureAxis) { auto rawPressure = static_cast<float>(mSingleTouchMotionAccumulator.getAbsolutePressure()); - mStylusState.pressure = (rawPressure - mRawPressureAxis.minValue) / - static_cast<float>(mRawPressureAxis.maxValue - mRawPressureAxis.minValue); + mStylusState.pressure = (rawPressure - mRawPressureAxis->minValue) / + static_cast<float>(mRawPressureAxis->maxValue - mRawPressureAxis->minValue); } else if (mTouchButtonAccumulator.hasButtonTouch()) { mStylusState.pressure = mTouchButtonAccumulator.isHovering() ? 0.0f : 1.0f; } diff --git a/services/inputflinger/reader/mapper/ExternalStylusInputMapper.h b/services/inputflinger/reader/mapper/ExternalStylusInputMapper.h index c040a7b996..d48fd9b469 100644 --- a/services/inputflinger/reader/mapper/ExternalStylusInputMapper.h +++ b/services/inputflinger/reader/mapper/ExternalStylusInputMapper.h @@ -16,6 +16,8 @@ #pragma once +#include <optional> + #include "InputMapper.h" #include "SingleTouchMotionAccumulator.h" @@ -43,7 +45,7 @@ public: private: SingleTouchMotionAccumulator mSingleTouchMotionAccumulator; - RawAbsoluteAxisInfo mRawPressureAxis; + std::optional<RawAbsoluteAxisInfo> mRawPressureAxis; TouchButtonAccumulator mTouchButtonAccumulator; StylusState mStylusState; diff --git a/services/inputflinger/reader/mapper/InputMapper.cpp b/services/inputflinger/reader/mapper/InputMapper.cpp index b6c5c9806c..c44c48c0bf 100644 --- a/services/inputflinger/reader/mapper/InputMapper.cpp +++ b/services/inputflinger/reader/mapper/InputMapper.cpp @@ -18,6 +18,7 @@ #include "InputMapper.h" +#include <optional> #include <sstream> #include <ftl/enum.h> @@ -116,15 +117,16 @@ std::list<NotifyArgs> InputMapper::updateExternalStylusState(const StylusState& return {}; } -status_t InputMapper::getAbsoluteAxisInfo(int32_t axis, RawAbsoluteAxisInfo* axisInfo) { - return getDeviceContext().getAbsoluteAxisInfo(axis, axisInfo); +std::optional<RawAbsoluteAxisInfo> InputMapper::getAbsoluteAxisInfo(int32_t axis) { + return getDeviceContext().getAbsoluteAxisInfo(axis); } void InputMapper::bumpGeneration() { getDeviceContext().bumpGeneration(); } -void InputMapper::dumpRawAbsoluteAxisInfo(std::string& dump, const RawAbsoluteAxisInfo& axis, +void InputMapper::dumpRawAbsoluteAxisInfo(std::string& dump, + const std::optional<RawAbsoluteAxisInfo>& axis, const char* name) { std::stringstream out; out << INDENT4 << name << ": " << axis << "\n"; diff --git a/services/inputflinger/reader/mapper/InputMapper.h b/services/inputflinger/reader/mapper/InputMapper.h index 2c51448b4e..e5afcc798e 100644 --- a/services/inputflinger/reader/mapper/InputMapper.h +++ b/services/inputflinger/reader/mapper/InputMapper.h @@ -16,6 +16,8 @@ #pragma once +#include <optional> + #include "EventHub.h" #include "InputDevice.h" #include "InputListener.h" @@ -126,10 +128,11 @@ protected: explicit InputMapper(InputDeviceContext& deviceContext, const InputReaderConfiguration& readerConfig); - status_t getAbsoluteAxisInfo(int32_t axis, RawAbsoluteAxisInfo* axisInfo); + std::optional<RawAbsoluteAxisInfo> getAbsoluteAxisInfo(int32_t axis); void bumpGeneration(); - static void dumpRawAbsoluteAxisInfo(std::string& dump, const RawAbsoluteAxisInfo& axis, + static void dumpRawAbsoluteAxisInfo(std::string& dump, + const std::optional<RawAbsoluteAxisInfo>& axis, const char* name); static void dumpStylusState(std::string& dump, const StylusState& state); }; diff --git a/services/inputflinger/reader/mapper/JoystickInputMapper.cpp b/services/inputflinger/reader/mapper/JoystickInputMapper.cpp index 41e018d392..3091714e00 100644 --- a/services/inputflinger/reader/mapper/JoystickInputMapper.cpp +++ b/services/inputflinger/reader/mapper/JoystickInputMapper.cpp @@ -117,9 +117,8 @@ std::list<NotifyArgs> JoystickInputMapper::reconfigure(nsecs_t when, continue; // axis must be claimed by a different device } - RawAbsoluteAxisInfo rawAxisInfo; - getAbsoluteAxisInfo(abs, &rawAxisInfo); - if (rawAxisInfo.valid) { + if (std::optional<RawAbsoluteAxisInfo> rawAxisInfo = getAbsoluteAxisInfo(abs); + rawAxisInfo) { // Map axis. AxisInfo axisInfo; const bool explicitlyMapped = !getDeviceContext().mapAxis(abs, &axisInfo); @@ -129,7 +128,7 @@ std::list<NotifyArgs> JoystickInputMapper::reconfigure(nsecs_t when, axisInfo.mode = AxisInfo::MODE_NORMAL; axisInfo.axis = -1; } - mAxes.insert({abs, createAxis(axisInfo, rawAxisInfo, explicitlyMapped)}); + mAxes.insert({abs, createAxis(axisInfo, rawAxisInfo.value(), explicitlyMapped)}); } } diff --git a/services/inputflinger/reader/mapper/KeyboardInputMapper.cpp b/services/inputflinger/reader/mapper/KeyboardInputMapper.cpp index 4a21e48f8f..38dcd65a81 100644 --- a/services/inputflinger/reader/mapper/KeyboardInputMapper.cpp +++ b/services/inputflinger/reader/mapper/KeyboardInputMapper.cpp @@ -98,10 +98,10 @@ static bool isMediaKey(int32_t keyCode) { KeyboardInputMapper::KeyboardInputMapper(InputDeviceContext& deviceContext, const InputReaderConfiguration& readerConfig, uint32_t source) - : InputMapper(deviceContext, readerConfig), mSource(source) {} + : InputMapper(deviceContext, readerConfig), mMapperSource(source) {} uint32_t KeyboardInputMapper::getSources() const { - return mSource; + return mMapperSource; } ui::Rotation KeyboardInputMapper::getOrientation() { @@ -351,8 +351,8 @@ std::list<NotifyArgs> KeyboardInputMapper::processKey(nsecs_t when, nsecs_t read policyFlags |= POLICY_FLAG_DISABLE_KEY_REPEAT; } - out.emplace_back(NotifyKeyArgs(getContext()->getNextId(), when, readTime, deviceId, mSource, - getDisplayId(), policyFlags, + out.emplace_back(NotifyKeyArgs(getContext()->getNextId(), when, readTime, deviceId, + getEventSource(), getDisplayId(), policyFlags, down ? AKEY_EVENT_ACTION_DOWN : AKEY_EVENT_ACTION_UP, flags, keyCode, scanCode, keyMetaState, downTime)); return out; @@ -478,12 +478,12 @@ std::list<NotifyArgs> KeyboardInputMapper::cancelAllDownKeys(nsecs_t when) { std::list<NotifyArgs> out; size_t n = mKeyDowns.size(); for (size_t i = 0; i < n; i++) { - out.emplace_back(NotifyKeyArgs(getContext()->getNextId(), when, - systemTime(SYSTEM_TIME_MONOTONIC), getDeviceId(), mSource, - getDisplayId(), /*policyFlags=*/0, AKEY_EVENT_ACTION_UP, - mKeyDowns[i].flags | AKEY_EVENT_FLAG_CANCELED, - mKeyDowns[i].keyCode, mKeyDowns[i].scanCode, AMETA_NONE, - mKeyDowns[i].downTime)); + out.emplace_back( + NotifyKeyArgs(getContext()->getNextId(), when, systemTime(SYSTEM_TIME_MONOTONIC), + getDeviceId(), getEventSource(), getDisplayId(), /*policyFlags=*/0, + AKEY_EVENT_ACTION_UP, mKeyDowns[i].flags | AKEY_EVENT_FLAG_CANCELED, + mKeyDowns[i].keyCode, mKeyDowns[i].scanCode, AMETA_NONE, + mKeyDowns[i].downTime)); } mKeyDowns.clear(); mMetaState = AMETA_NONE; @@ -495,4 +495,14 @@ void KeyboardInputMapper::onKeyDownProcessed(nsecs_t downTime) { context.setLastKeyDownTimestamp(downTime); } +uint32_t KeyboardInputMapper::getEventSource() const { + // For all input events generated by this mapper, use the source that's shared across all + // KeyboardInputMappers for this device in case there are more than one. + static constexpr auto ALL_KEYBOARD_SOURCES = + AINPUT_SOURCE_KEYBOARD | AINPUT_SOURCE_DPAD | AINPUT_SOURCE_GAMEPAD; + const auto deviceSources = getDeviceContext().getDeviceSources(); + LOG_ALWAYS_FATAL_IF((deviceSources & mMapperSource) != mMapperSource); + return deviceSources & ALL_KEYBOARD_SOURCES; +} + } // namespace android diff --git a/services/inputflinger/reader/mapper/KeyboardInputMapper.h b/services/inputflinger/reader/mapper/KeyboardInputMapper.h index c7df558caf..2df0b85e21 100644 --- a/services/inputflinger/reader/mapper/KeyboardInputMapper.h +++ b/services/inputflinger/reader/mapper/KeyboardInputMapper.h @@ -60,7 +60,10 @@ private: int32_t flags{}; }; - uint32_t mSource{}; + // The keyboard source for this mapper. Events generated should use the source shared + // by all KeyboardInputMappers for this input device. + uint32_t mMapperSource{}; + std::optional<KeyboardLayoutInfo> mKeyboardLayoutInfo; std::vector<KeyDown> mKeyDowns{}; // keys that are down @@ -106,6 +109,7 @@ private: std::optional<DisplayViewport> findViewport(const InputReaderConfiguration& readerConfig); [[nodiscard]] std::list<NotifyArgs> cancelAllDownKeys(nsecs_t when); void onKeyDownProcessed(nsecs_t downTime); + uint32_t getEventSource() const; }; } // namespace android diff --git a/services/inputflinger/reader/mapper/MultiTouchInputMapper.cpp b/services/inputflinger/reader/mapper/MultiTouchInputMapper.cpp index 1986fe286a..fd8224a608 100644 --- a/services/inputflinger/reader/mapper/MultiTouchInputMapper.cpp +++ b/services/inputflinger/reader/mapper/MultiTouchInputMapper.cpp @@ -133,7 +133,7 @@ void MultiTouchInputMapper::syncTouch(nsecs_t when, RawState* outState) { bool isHovering = mTouchButtonAccumulator.getToolType() != ToolType::MOUSE && (mTouchButtonAccumulator.isHovering() || - (mRawPointerAxes.pressure.valid && inSlot.getPressure() <= 0)); + (mRawPointerAxes.pressure && inSlot.getPressure() <= 0)); outPointer.isHovering = isHovering; // Assign pointer id using tracking id if available. @@ -189,21 +189,27 @@ std::list<NotifyArgs> MultiTouchInputMapper::reconfigure(nsecs_t when, void MultiTouchInputMapper::configureRawPointerAxes() { TouchInputMapper::configureRawPointerAxes(); - getAbsoluteAxisInfo(ABS_MT_POSITION_X, &mRawPointerAxes.x); - getAbsoluteAxisInfo(ABS_MT_POSITION_Y, &mRawPointerAxes.y); - getAbsoluteAxisInfo(ABS_MT_TOUCH_MAJOR, &mRawPointerAxes.touchMajor); - getAbsoluteAxisInfo(ABS_MT_TOUCH_MINOR, &mRawPointerAxes.touchMinor); - getAbsoluteAxisInfo(ABS_MT_WIDTH_MAJOR, &mRawPointerAxes.toolMajor); - getAbsoluteAxisInfo(ABS_MT_WIDTH_MINOR, &mRawPointerAxes.toolMinor); - getAbsoluteAxisInfo(ABS_MT_ORIENTATION, &mRawPointerAxes.orientation); - getAbsoluteAxisInfo(ABS_MT_PRESSURE, &mRawPointerAxes.pressure); - getAbsoluteAxisInfo(ABS_MT_DISTANCE, &mRawPointerAxes.distance); - getAbsoluteAxisInfo(ABS_MT_TRACKING_ID, &mRawPointerAxes.trackingId); - getAbsoluteAxisInfo(ABS_MT_SLOT, &mRawPointerAxes.slot); - - if (mRawPointerAxes.trackingId.valid && mRawPointerAxes.slot.valid && - mRawPointerAxes.slot.minValue == 0 && mRawPointerAxes.slot.maxValue > 0) { - size_t slotCount = mRawPointerAxes.slot.maxValue + 1; + // TODO(b/351870641): Investigate why we are sometime not getting valid axis infos for the x/y + // axes, even though those axes are required to be supported. + if (const auto xInfo = getAbsoluteAxisInfo(ABS_MT_POSITION_X); xInfo.has_value()) { + mRawPointerAxes.x = *xInfo; + } + if (const auto yInfo = getAbsoluteAxisInfo(ABS_MT_POSITION_Y); yInfo.has_value()) { + mRawPointerAxes.y = *yInfo; + } + mRawPointerAxes.touchMajor = getAbsoluteAxisInfo(ABS_MT_TOUCH_MAJOR); + mRawPointerAxes.touchMinor = getAbsoluteAxisInfo(ABS_MT_TOUCH_MINOR); + mRawPointerAxes.toolMajor = getAbsoluteAxisInfo(ABS_MT_WIDTH_MAJOR); + mRawPointerAxes.toolMinor = getAbsoluteAxisInfo(ABS_MT_WIDTH_MINOR); + mRawPointerAxes.orientation = getAbsoluteAxisInfo(ABS_MT_ORIENTATION); + mRawPointerAxes.pressure = getAbsoluteAxisInfo(ABS_MT_PRESSURE); + mRawPointerAxes.distance = getAbsoluteAxisInfo(ABS_MT_DISTANCE); + mRawPointerAxes.trackingId = getAbsoluteAxisInfo(ABS_MT_TRACKING_ID); + mRawPointerAxes.slot = getAbsoluteAxisInfo(ABS_MT_SLOT); + + if (mRawPointerAxes.trackingId && mRawPointerAxes.slot && mRawPointerAxes.slot->minValue == 0 && + mRawPointerAxes.slot->maxValue > 0) { + size_t slotCount = mRawPointerAxes.slot->maxValue + 1; if (slotCount > MAX_SLOTS) { ALOGW("MultiTouch Device %s reported %zu slots but the framework " "only supports a maximum of %zu slots at this time.", diff --git a/services/inputflinger/reader/mapper/SensorInputMapper.cpp b/services/inputflinger/reader/mapper/SensorInputMapper.cpp index d7f2993daa..4233f789d6 100644 --- a/services/inputflinger/reader/mapper/SensorInputMapper.cpp +++ b/services/inputflinger/reader/mapper/SensorInputMapper.cpp @@ -133,9 +133,8 @@ std::list<NotifyArgs> SensorInputMapper::reconfigure(nsecs_t when, .test(InputDeviceClass::SENSOR))) { continue; } - RawAbsoluteAxisInfo rawAxisInfo; - getAbsoluteAxisInfo(abs, &rawAxisInfo); - if (rawAxisInfo.valid) { + if (std::optional<RawAbsoluteAxisInfo> rawAxisInfo = getAbsoluteAxisInfo(abs); + rawAxisInfo) { AxisInfo axisInfo; // Axis doesn't need to be mapped, as sensor mapper doesn't generate any motion // input events @@ -146,7 +145,7 @@ std::list<NotifyArgs> SensorInputMapper::reconfigure(nsecs_t when, if (ret.ok()) { InputDeviceSensorType sensorType = (*ret).first; int32_t sensorDataIndex = (*ret).second; - const Axis& axis = createAxis(axisInfo, rawAxisInfo); + const Axis& axis = createAxis(axisInfo, rawAxisInfo.value()); parseSensorConfiguration(sensorType, abs, sensorDataIndex, axis); mAxes.insert({abs, axis}); diff --git a/services/inputflinger/reader/mapper/SingleTouchInputMapper.cpp b/services/inputflinger/reader/mapper/SingleTouchInputMapper.cpp index 140bb0c4ed..cef18375d4 100644 --- a/services/inputflinger/reader/mapper/SingleTouchInputMapper.cpp +++ b/services/inputflinger/reader/mapper/SingleTouchInputMapper.cpp @@ -44,7 +44,7 @@ void SingleTouchInputMapper::syncTouch(nsecs_t when, RawState* outState) { bool isHovering = mTouchButtonAccumulator.getToolType() != ToolType::MOUSE && (mTouchButtonAccumulator.isHovering() || - (mRawPointerAxes.pressure.valid && + (mRawPointerAxes.pressure && mSingleTouchMotionAccumulator.getAbsolutePressure() <= 0)); outState->rawPointerData.markIdBit(0, isHovering); @@ -72,13 +72,19 @@ void SingleTouchInputMapper::syncTouch(nsecs_t when, RawState* outState) { void SingleTouchInputMapper::configureRawPointerAxes() { TouchInputMapper::configureRawPointerAxes(); - getAbsoluteAxisInfo(ABS_X, &mRawPointerAxes.x); - getAbsoluteAxisInfo(ABS_Y, &mRawPointerAxes.y); - getAbsoluteAxisInfo(ABS_PRESSURE, &mRawPointerAxes.pressure); - getAbsoluteAxisInfo(ABS_TOOL_WIDTH, &mRawPointerAxes.toolMajor); - getAbsoluteAxisInfo(ABS_DISTANCE, &mRawPointerAxes.distance); - getAbsoluteAxisInfo(ABS_TILT_X, &mRawPointerAxes.tiltX); - getAbsoluteAxisInfo(ABS_TILT_Y, &mRawPointerAxes.tiltY); + // TODO(b/351870641): Investigate why we are sometime not getting valid axis infos for the x/y + // axes, even though those axes are required to be supported. + if (const auto xInfo = getAbsoluteAxisInfo(ABS_X); xInfo.has_value()) { + mRawPointerAxes.x = *xInfo; + } + if (const auto yInfo = getAbsoluteAxisInfo(ABS_Y); yInfo.has_value()) { + mRawPointerAxes.y = *yInfo; + } + mRawPointerAxes.pressure = getAbsoluteAxisInfo(ABS_PRESSURE); + mRawPointerAxes.toolMajor = getAbsoluteAxisInfo(ABS_TOOL_WIDTH); + mRawPointerAxes.distance = getAbsoluteAxisInfo(ABS_DISTANCE); + mRawPointerAxes.tiltX = getAbsoluteAxisInfo(ABS_TILT_X); + mRawPointerAxes.tiltY = getAbsoluteAxisInfo(ABS_TILT_Y); } bool SingleTouchInputMapper::hasStylus() const { diff --git a/services/inputflinger/reader/mapper/TouchInputMapper.cpp b/services/inputflinger/reader/mapper/TouchInputMapper.cpp index 2d892087f6..984e217efc 100644 --- a/services/inputflinger/reader/mapper/TouchInputMapper.cpp +++ b/services/inputflinger/reader/mapper/TouchInputMapper.cpp @@ -600,10 +600,10 @@ void TouchInputMapper::initializeSizeRanges() { const float diagonalSize = hypotf(mDisplayBounds.width, mDisplayBounds.height); // Size factors. - if (mRawPointerAxes.touchMajor.valid && mRawPointerAxes.touchMajor.maxValue != 0) { - mSizeScale = 1.0f / mRawPointerAxes.touchMajor.maxValue; - } else if (mRawPointerAxes.toolMajor.valid && mRawPointerAxes.toolMajor.maxValue != 0) { - mSizeScale = 1.0f / mRawPointerAxes.toolMajor.maxValue; + if (mRawPointerAxes.touchMajor && mRawPointerAxes.touchMajor->maxValue != 0) { + mSizeScale = 1.0f / mRawPointerAxes.touchMajor->maxValue; + } else if (mRawPointerAxes.toolMajor && mRawPointerAxes.toolMajor->maxValue != 0) { + mSizeScale = 1.0f / mRawPointerAxes.toolMajor->maxValue; } else { mSizeScale = 0.0f; } @@ -618,18 +618,18 @@ void TouchInputMapper::initializeSizeRanges() { .resolution = 0, }; - if (mRawPointerAxes.touchMajor.valid) { - mRawPointerAxes.touchMajor.resolution = - clampResolution("touchMajor", mRawPointerAxes.touchMajor.resolution); - mOrientedRanges.touchMajor->resolution = mRawPointerAxes.touchMajor.resolution; + if (mRawPointerAxes.touchMajor) { + mRawPointerAxes.touchMajor->resolution = + clampResolution("touchMajor", mRawPointerAxes.touchMajor->resolution); + mOrientedRanges.touchMajor->resolution = mRawPointerAxes.touchMajor->resolution; } mOrientedRanges.touchMinor = mOrientedRanges.touchMajor; mOrientedRanges.touchMinor->axis = AMOTION_EVENT_AXIS_TOUCH_MINOR; - if (mRawPointerAxes.touchMinor.valid) { - mRawPointerAxes.touchMinor.resolution = - clampResolution("touchMinor", mRawPointerAxes.touchMinor.resolution); - mOrientedRanges.touchMinor->resolution = mRawPointerAxes.touchMinor.resolution; + if (mRawPointerAxes.touchMinor) { + mRawPointerAxes.touchMinor->resolution = + clampResolution("touchMinor", mRawPointerAxes.touchMinor->resolution); + mOrientedRanges.touchMinor->resolution = mRawPointerAxes.touchMinor->resolution; } mOrientedRanges.toolMajor = InputDeviceInfo::MotionRange{ @@ -641,18 +641,18 @@ void TouchInputMapper::initializeSizeRanges() { .fuzz = 0, .resolution = 0, }; - if (mRawPointerAxes.toolMajor.valid) { - mRawPointerAxes.toolMajor.resolution = - clampResolution("toolMajor", mRawPointerAxes.toolMajor.resolution); - mOrientedRanges.toolMajor->resolution = mRawPointerAxes.toolMajor.resolution; + if (mRawPointerAxes.toolMajor) { + mRawPointerAxes.toolMajor->resolution = + clampResolution("toolMajor", mRawPointerAxes.toolMajor->resolution); + mOrientedRanges.toolMajor->resolution = mRawPointerAxes.toolMajor->resolution; } mOrientedRanges.toolMinor = mOrientedRanges.toolMajor; mOrientedRanges.toolMinor->axis = AMOTION_EVENT_AXIS_TOOL_MINOR; - if (mRawPointerAxes.toolMinor.valid) { - mRawPointerAxes.toolMinor.resolution = - clampResolution("toolMinor", mRawPointerAxes.toolMinor.resolution); - mOrientedRanges.toolMinor->resolution = mRawPointerAxes.toolMinor.resolution; + if (mRawPointerAxes.toolMinor) { + mRawPointerAxes.toolMinor->resolution = + clampResolution("toolMinor", mRawPointerAxes.toolMinor->resolution); + mOrientedRanges.toolMinor->resolution = mRawPointerAxes.toolMinor->resolution; } if (mCalibration.sizeCalibration == Calibration::SizeCalibration::GEOMETRIC) { @@ -704,9 +704,10 @@ void TouchInputMapper::initializeOrientedRanges() { mCalibration.pressureCalibration == Calibration::PressureCalibration::AMPLITUDE) { if (mCalibration.pressureScale) { mPressureScale = *mCalibration.pressureScale; - pressureMax = mPressureScale * mRawPointerAxes.pressure.maxValue; - } else if (mRawPointerAxes.pressure.valid && mRawPointerAxes.pressure.maxValue != 0) { - mPressureScale = 1.0f / mRawPointerAxes.pressure.maxValue; + pressureMax = mPressureScale * + (mRawPointerAxes.pressure ? mRawPointerAxes.pressure->maxValue : 0); + } else if (mRawPointerAxes.pressure && mRawPointerAxes.pressure->maxValue != 0) { + mPressureScale = 1.0f / mRawPointerAxes.pressure->maxValue; } } @@ -725,18 +726,18 @@ void TouchInputMapper::initializeOrientedRanges() { mTiltXScale = 0; mTiltYCenter = 0; mTiltYScale = 0; - mHaveTilt = mRawPointerAxes.tiltX.valid && mRawPointerAxes.tiltY.valid; + mHaveTilt = mRawPointerAxes.tiltX && mRawPointerAxes.tiltY; if (mHaveTilt) { - mTiltXCenter = avg(mRawPointerAxes.tiltX.minValue, mRawPointerAxes.tiltX.maxValue); - mTiltYCenter = avg(mRawPointerAxes.tiltY.minValue, mRawPointerAxes.tiltY.maxValue); + mTiltXCenter = avg(mRawPointerAxes.tiltX->minValue, mRawPointerAxes.tiltX->maxValue); + mTiltYCenter = avg(mRawPointerAxes.tiltY->minValue, mRawPointerAxes.tiltY->maxValue); mTiltXScale = M_PI / 180; mTiltYScale = M_PI / 180; - if (mRawPointerAxes.tiltX.resolution) { - mTiltXScale = 1.0 / mRawPointerAxes.tiltX.resolution; + if (mRawPointerAxes.tiltX->resolution) { + mTiltXScale = 1.0 / mRawPointerAxes.tiltX->resolution; } - if (mRawPointerAxes.tiltY.resolution) { - mTiltYScale = 1.0 / mRawPointerAxes.tiltY.resolution; + if (mRawPointerAxes.tiltY->resolution) { + mTiltYScale = 1.0 / mRawPointerAxes.tiltY->resolution; } mOrientedRanges.tilt = InputDeviceInfo::MotionRange{ @@ -766,11 +767,11 @@ void TouchInputMapper::initializeOrientedRanges() { } else if (mCalibration.orientationCalibration != Calibration::OrientationCalibration::NONE) { if (mCalibration.orientationCalibration == Calibration::OrientationCalibration::INTERPOLATED) { - if (mRawPointerAxes.orientation.valid) { - if (mRawPointerAxes.orientation.maxValue > 0) { - mOrientationScale = M_PI_2 / mRawPointerAxes.orientation.maxValue; - } else if (mRawPointerAxes.orientation.minValue < 0) { - mOrientationScale = -M_PI_2 / mRawPointerAxes.orientation.minValue; + if (mRawPointerAxes.orientation) { + if (mRawPointerAxes.orientation->maxValue > 0) { + mOrientationScale = M_PI_2 / mRawPointerAxes.orientation->maxValue; + } else if (mRawPointerAxes.orientation->minValue < 0) { + mOrientationScale = -M_PI_2 / mRawPointerAxes.orientation->minValue; } else { mOrientationScale = 0; } @@ -795,14 +796,14 @@ void TouchInputMapper::initializeOrientedRanges() { mDistanceScale = mCalibration.distanceScale.value_or(1.0f); } + const bool hasDistance = mRawPointerAxes.distance.has_value(); mOrientedRanges.distance = InputDeviceInfo::MotionRange{ - .axis = AMOTION_EVENT_AXIS_DISTANCE, .source = mSource, - .min = mRawPointerAxes.distance.minValue * mDistanceScale, - .max = mRawPointerAxes.distance.maxValue * mDistanceScale, + .min = hasDistance ? mRawPointerAxes.distance->minValue * mDistanceScale : 0, + .max = hasDistance ? mRawPointerAxes.distance->maxValue * mDistanceScale : 0, .flat = 0, - .fuzz = mRawPointerAxes.distance.fuzz * mDistanceScale, + .fuzz = hasDistance ? mRawPointerAxes.distance->fuzz * mDistanceScale : 0, .resolution = 0, }; } @@ -943,12 +944,7 @@ void TouchInputMapper::configureInputDevice(nsecs_t when, bool* outResetNeeded) const std::optional<DisplayViewport> newViewportOpt = findViewport(); // Ensure the device is valid and can be used. - if (!mRawPointerAxes.x.valid || !mRawPointerAxes.y.valid) { - ALOGW("Touch device '%s' did not report support for X or Y axis! " - "The device will be inoperable.", - getDeviceName().c_str()); - mDeviceMode = DeviceMode::DISABLED; - } else if (!newViewportOpt) { + if (!newViewportOpt) { ALOGI("Touch device '%s' could not query the properties of its associated " "display. The device will be inoperable until the display size " "becomes available.", @@ -1237,7 +1233,7 @@ void TouchInputMapper::parseCalibration() { void TouchInputMapper::resolveCalibration() { // Size - if (mRawPointerAxes.touchMajor.valid || mRawPointerAxes.toolMajor.valid) { + if (mRawPointerAxes.touchMajor || mRawPointerAxes.toolMajor) { if (mCalibration.sizeCalibration == Calibration::SizeCalibration::DEFAULT) { mCalibration.sizeCalibration = Calibration::SizeCalibration::GEOMETRIC; } @@ -1246,7 +1242,7 @@ void TouchInputMapper::resolveCalibration() { } // Pressure - if (mRawPointerAxes.pressure.valid) { + if (mRawPointerAxes.pressure) { if (mCalibration.pressureCalibration == Calibration::PressureCalibration::DEFAULT) { mCalibration.pressureCalibration = Calibration::PressureCalibration::PHYSICAL; } @@ -1255,7 +1251,7 @@ void TouchInputMapper::resolveCalibration() { } // Orientation - if (mRawPointerAxes.orientation.valid) { + if (mRawPointerAxes.orientation) { if (mCalibration.orientationCalibration == Calibration::OrientationCalibration::DEFAULT) { mCalibration.orientationCalibration = Calibration::OrientationCalibration::INTERPOLATED; } @@ -1264,7 +1260,7 @@ void TouchInputMapper::resolveCalibration() { } // Distance - if (mRawPointerAxes.distance.valid) { + if (mRawPointerAxes.distance) { if (mCalibration.distanceCalibration == Calibration::DistanceCalibration::DEFAULT) { mCalibration.distanceCalibration = Calibration::DistanceCalibration::SCALED; } @@ -2251,25 +2247,25 @@ void TouchInputMapper::cookPointerData() { case Calibration::SizeCalibration::DIAMETER: case Calibration::SizeCalibration::BOX: case Calibration::SizeCalibration::AREA: - if (mRawPointerAxes.touchMajor.valid && mRawPointerAxes.toolMajor.valid) { + if (mRawPointerAxes.touchMajor && mRawPointerAxes.toolMajor) { touchMajor = in.touchMajor; - touchMinor = mRawPointerAxes.touchMinor.valid ? in.touchMinor : in.touchMajor; + touchMinor = mRawPointerAxes.touchMinor ? in.touchMinor : in.touchMajor; toolMajor = in.toolMajor; - toolMinor = mRawPointerAxes.toolMinor.valid ? in.toolMinor : in.toolMajor; - size = mRawPointerAxes.touchMinor.valid ? avg(in.touchMajor, in.touchMinor) - : in.touchMajor; - } else if (mRawPointerAxes.touchMajor.valid) { + toolMinor = mRawPointerAxes.toolMinor ? in.toolMinor : in.toolMajor; + size = mRawPointerAxes.touchMinor ? avg(in.touchMajor, in.touchMinor) + : in.touchMajor; + } else if (mRawPointerAxes.touchMajor) { toolMajor = touchMajor = in.touchMajor; toolMinor = touchMinor = - mRawPointerAxes.touchMinor.valid ? in.touchMinor : in.touchMajor; - size = mRawPointerAxes.touchMinor.valid ? avg(in.touchMajor, in.touchMinor) - : in.touchMajor; - } else if (mRawPointerAxes.toolMajor.valid) { + mRawPointerAxes.touchMinor ? in.touchMinor : in.touchMajor; + size = mRawPointerAxes.touchMinor ? avg(in.touchMajor, in.touchMinor) + : in.touchMajor; + } else if (mRawPointerAxes.toolMajor) { touchMajor = toolMajor = in.toolMajor; touchMinor = toolMinor = - mRawPointerAxes.toolMinor.valid ? in.toolMinor : in.toolMajor; - size = mRawPointerAxes.toolMinor.valid ? avg(in.toolMajor, in.toolMinor) - : in.toolMajor; + mRawPointerAxes.toolMinor ? in.toolMinor : in.toolMajor; + size = mRawPointerAxes.toolMinor ? avg(in.toolMajor, in.toolMinor) + : in.toolMajor; } else { ALOG_ASSERT(false, "No touch or tool axes. " diff --git a/services/inputflinger/reader/mapper/TouchInputMapper.h b/services/inputflinger/reader/mapper/TouchInputMapper.h index a9a0190bce..87b72afe7c 100644 --- a/services/inputflinger/reader/mapper/TouchInputMapper.h +++ b/services/inputflinger/reader/mapper/TouchInputMapper.h @@ -61,17 +61,17 @@ static constexpr nsecs_t TOUCH_DATA_TIMEOUT = ms2ns(20); struct RawPointerAxes { RawAbsoluteAxisInfo x{}; RawAbsoluteAxisInfo y{}; - RawAbsoluteAxisInfo pressure{}; - RawAbsoluteAxisInfo touchMajor{}; - RawAbsoluteAxisInfo touchMinor{}; - RawAbsoluteAxisInfo toolMajor{}; - RawAbsoluteAxisInfo toolMinor{}; - RawAbsoluteAxisInfo orientation{}; - RawAbsoluteAxisInfo distance{}; - RawAbsoluteAxisInfo tiltX{}; - RawAbsoluteAxisInfo tiltY{}; - RawAbsoluteAxisInfo trackingId{}; - RawAbsoluteAxisInfo slot{}; + std::optional<RawAbsoluteAxisInfo> pressure{}; + std::optional<RawAbsoluteAxisInfo> touchMajor{}; + std::optional<RawAbsoluteAxisInfo> touchMinor{}; + std::optional<RawAbsoluteAxisInfo> toolMajor{}; + std::optional<RawAbsoluteAxisInfo> toolMinor{}; + std::optional<RawAbsoluteAxisInfo> orientation{}; + std::optional<RawAbsoluteAxisInfo> distance{}; + std::optional<RawAbsoluteAxisInfo> tiltX{}; + std::optional<RawAbsoluteAxisInfo> tiltY{}; + std::optional<RawAbsoluteAxisInfo> trackingId{}; + std::optional<RawAbsoluteAxisInfo> slot{}; inline int32_t getRawWidth() const { return x.maxValue - x.minValue + 1; } inline int32_t getRawHeight() const { return y.maxValue - y.minValue + 1; } diff --git a/services/inputflinger/reader/mapper/TouchpadInputMapper.cpp b/services/inputflinger/reader/mapper/TouchpadInputMapper.cpp index 128f5153a0..5c5fd3fb5f 100644 --- a/services/inputflinger/reader/mapper/TouchpadInputMapper.cpp +++ b/services/inputflinger/reader/mapper/TouchpadInputMapper.cpp @@ -240,14 +240,15 @@ TouchpadInputMapper::TouchpadInputMapper(InputDeviceContext& deviceContext, mGestureConverter(*getContext(), deviceContext, getDeviceId()), mCapturedEventConverter(*getContext(), deviceContext, mMotionAccumulator, getDeviceId()), mMetricsId(metricsIdFromInputDeviceIdentifier(deviceContext.getDeviceIdentifier())) { - RawAbsoluteAxisInfo slotAxisInfo; - deviceContext.getAbsoluteAxisInfo(ABS_MT_SLOT, &slotAxisInfo); - if (!slotAxisInfo.valid || slotAxisInfo.maxValue < 0) { + if (std::optional<RawAbsoluteAxisInfo> slotAxis = + deviceContext.getAbsoluteAxisInfo(ABS_MT_SLOT); + slotAxis && slotAxis->maxValue >= 0) { + mMotionAccumulator.configure(deviceContext, slotAxis->maxValue + 1, true); + } else { LOG(WARNING) << "Touchpad " << deviceContext.getName() << " doesn't have a valid ABS_MT_SLOT axis, and probably won't work properly."; - slotAxisInfo.maxValue = 0; + mMotionAccumulator.configure(deviceContext, 1, true); } - mMotionAccumulator.configure(deviceContext, slotAxisInfo.maxValue + 1, true); mGestureInterpreter->Initialize(GESTURES_DEVCLASS_TOUCHPAD); mGestureInterpreter->SetHardwareProperties(createHardwareProperties(deviceContext)); diff --git a/services/inputflinger/reader/mapper/gestures/GestureConverter.cpp b/services/inputflinger/reader/mapper/gestures/GestureConverter.cpp index e8e7376e92..9924d0d491 100644 --- a/services/inputflinger/reader/mapper/gestures/GestureConverter.cpp +++ b/services/inputflinger/reader/mapper/gestures/GestureConverter.cpp @@ -66,10 +66,11 @@ GestureConverter::GestureConverter(InputReaderContext& readerContext, const InputDeviceContext& deviceContext, int32_t deviceId) : mDeviceId(deviceId), mReaderContext(readerContext), - mEnableFlingStop(input_flags::enable_touchpad_fling_stop()) { - deviceContext.getAbsoluteAxisInfo(ABS_MT_POSITION_X, &mXAxisInfo); - deviceContext.getAbsoluteAxisInfo(ABS_MT_POSITION_Y, &mYAxisInfo); -} + mEnableFlingStop(input_flags::enable_touchpad_fling_stop()), + // We can safely assume that ABS_MT_POSITION_X and _Y axes will be available, as EventHub + // won't classify a device as a touchpad if they're not present. + mXAxisInfo(deviceContext.getAbsoluteAxisInfo(ABS_MT_POSITION_X).value()), + mYAxisInfo(deviceContext.getAbsoluteAxisInfo(ABS_MT_POSITION_Y).value()) {} std::string GestureConverter::dump() const { std::stringstream out; diff --git a/services/inputflinger/reader/mapper/gestures/HardwareProperties.cpp b/services/inputflinger/reader/mapper/gestures/HardwareProperties.cpp index 04655dc439..d8a1f501d1 100644 --- a/services/inputflinger/reader/mapper/gestures/HardwareProperties.cpp +++ b/services/inputflinger/reader/mapper/gestures/HardwareProperties.cpp @@ -16,6 +16,8 @@ #include "HardwareProperties.h" +#include <optional> + namespace android { namespace { @@ -33,26 +35,34 @@ unsigned short getMaxTouchCount(const InputDeviceContext& context) { HardwareProperties createHardwareProperties(const InputDeviceContext& context) { HardwareProperties props; - RawAbsoluteAxisInfo absMtPositionX; - context.getAbsoluteAxisInfo(ABS_MT_POSITION_X, &absMtPositionX); + // We can safely assume that ABS_MT_POSITION_X and _Y axes will be available, as EventHub won't + // classify a device as a touchpad if they're not present. + RawAbsoluteAxisInfo absMtPositionX = context.getAbsoluteAxisInfo(ABS_MT_POSITION_X).value(); props.left = absMtPositionX.minValue; props.right = absMtPositionX.maxValue; props.res_x = absMtPositionX.resolution; - RawAbsoluteAxisInfo absMtPositionY; - context.getAbsoluteAxisInfo(ABS_MT_POSITION_Y, &absMtPositionY); + RawAbsoluteAxisInfo absMtPositionY = context.getAbsoluteAxisInfo(ABS_MT_POSITION_Y).value(); props.top = absMtPositionY.minValue; props.bottom = absMtPositionY.maxValue; props.res_y = absMtPositionY.resolution; - RawAbsoluteAxisInfo absMtOrientation; - context.getAbsoluteAxisInfo(ABS_MT_ORIENTATION, &absMtOrientation); - props.orientation_minimum = absMtOrientation.minValue; - props.orientation_maximum = absMtOrientation.maxValue; + if (std::optional<RawAbsoluteAxisInfo> absMtOrientation = + context.getAbsoluteAxisInfo(ABS_MT_ORIENTATION); + absMtOrientation) { + props.orientation_minimum = absMtOrientation->minValue; + props.orientation_maximum = absMtOrientation->maxValue; + } else { + props.orientation_minimum = 0; + props.orientation_maximum = 0; + } - RawAbsoluteAxisInfo absMtSlot; - context.getAbsoluteAxisInfo(ABS_MT_SLOT, &absMtSlot); - props.max_finger_cnt = absMtSlot.maxValue - absMtSlot.minValue + 1; + if (std::optional<RawAbsoluteAxisInfo> absMtSlot = context.getAbsoluteAxisInfo(ABS_MT_SLOT); + absMtSlot) { + props.max_finger_cnt = absMtSlot->maxValue - absMtSlot->minValue + 1; + } else { + props.max_finger_cnt = 1; + } props.max_touch_cnt = getMaxTouchCount(context); // T5R2 ("Track 5, Report 2") is a feature of some old Synaptics touchpads that could track 5 @@ -71,9 +81,7 @@ HardwareProperties createHardwareProperties(const InputDeviceContext& context) { // are haptic. props.is_haptic_pad = false; - RawAbsoluteAxisInfo absMtPressure; - context.getAbsoluteAxisInfo(ABS_MT_PRESSURE, &absMtPressure); - props.reports_pressure = absMtPressure.valid; + props.reports_pressure = context.hasAbsoluteAxis(ABS_MT_PRESSURE); return props; } diff --git a/services/inputflinger/tests/CursorInputMapper_test.cpp b/services/inputflinger/tests/CursorInputMapper_test.cpp index 727237f287..846eced2ab 100644 --- a/services/inputflinger/tests/CursorInputMapper_test.cpp +++ b/services/inputflinger/tests/CursorInputMapper_test.cpp @@ -163,7 +163,6 @@ protected: } void createMapper() { - createDevice(); mMapper = createInputMapper<CursorInputMapper>(*mDeviceContext, mReaderConfiguration); } @@ -542,7 +541,6 @@ TEST_F(CursorInputMapperUnitTest, ProcessShouldNotRotateMotionsWhenOrientationAw // need to be rotated. mPropertyMap.addProperty("cursor.mode", "navigation"); mPropertyMap.addProperty("cursor.orientationAware", "1"); - createDevice(); ViewportFakingInputDeviceContext deviceContext(*mDevice, EVENTHUB_ID, ui::Rotation::Rotation90); mMapper = createInputMapper<CursorInputMapper>(deviceContext, mReaderConfiguration); @@ -560,7 +558,6 @@ TEST_F(CursorInputMapperUnitTest, ProcessShouldRotateMotionsWhenNotOrientationAw // Since InputReader works in the un-rotated coordinate space, only devices that are not // orientation-aware are affected by display rotation. mPropertyMap.addProperty("cursor.mode", "navigation"); - createDevice(); ViewportFakingInputDeviceContext deviceContext(*mDevice, EVENTHUB_ID, ui::Rotation::Rotation0); mMapper = createInputMapper<CursorInputMapper>(deviceContext, mReaderConfiguration); @@ -652,7 +649,6 @@ TEST_F(CursorInputMapperUnitTest, ConfigureDisplayIdWithAssociatedViewport) { mReaderConfiguration.setDisplayViewports({primaryViewport, secondaryViewport}); // Set up the secondary display as the display on which the pointer should be shown. // The InputDevice is not associated with any display. - createDevice(); ViewportFakingInputDeviceContext deviceContext(*mDevice, EVENTHUB_ID, secondaryViewport); mMapper = createInputMapper<CursorInputMapper>(deviceContext, mReaderConfiguration); @@ -673,7 +669,6 @@ TEST_F(CursorInputMapperUnitTest, DisplayViewport secondaryViewport = createSecondaryViewport(); mReaderConfiguration.setDisplayViewports({primaryViewport, secondaryViewport}); // Set up the primary display as the display on which the pointer should be shown. - createDevice(); // Associate the InputDevice with the secondary display. ViewportFakingInputDeviceContext deviceContext(*mDevice, EVENTHUB_ID, secondaryViewport); mMapper = createInputMapper<CursorInputMapper>(deviceContext, mReaderConfiguration); @@ -1032,7 +1027,6 @@ TEST_F(CursorInputMapperUnitTestWithNewBallistics, ConfigureAccelerationWithAsso mPropertyMap.addProperty("cursor.mode", "pointer"); DisplayViewport primaryViewport = createPrimaryViewport(ui::Rotation::Rotation0); mReaderConfiguration.setDisplayViewports({primaryViewport}); - createDevice(); ViewportFakingInputDeviceContext deviceContext(*mDevice, EVENTHUB_ID, primaryViewport); mMapper = createInputMapper<CursorInputMapper>(deviceContext, mReaderConfiguration); @@ -1070,7 +1064,6 @@ TEST_F(CursorInputMapperUnitTestWithNewBallistics, ConfigureAccelerationOnDispla mReaderConfiguration.setDisplayViewports({primaryViewport}); // Disable acceleration for the display. mReaderConfiguration.displaysWithMousePointerAccelerationDisabled.emplace(DISPLAY_ID); - createDevice(); // Don't associate the device with the display yet. ViewportFakingInputDeviceContext deviceContext(*mDevice, EVENTHUB_ID, diff --git a/services/inputflinger/tests/FakeEventHub.cpp b/services/inputflinger/tests/FakeEventHub.cpp index 99db99996b..31fbf209a3 100644 --- a/services/inputflinger/tests/FakeEventHub.cpp +++ b/services/inputflinger/tests/FakeEventHub.cpp @@ -101,7 +101,6 @@ void FakeEventHub::addAbsoluteAxis(int32_t deviceId, int axis, int32_t minValue, Device* device = getDevice(deviceId); RawAbsoluteAxisInfo info; - info.valid = true; info.minValue = minValue; info.maxValue = maxValue; info.flat = flat; diff --git a/services/inputflinger/tests/HardwareProperties_test.cpp b/services/inputflinger/tests/HardwareProperties_test.cpp index 643fab6d44..e87f8228c8 100644 --- a/services/inputflinger/tests/HardwareProperties_test.cpp +++ b/services/inputflinger/tests/HardwareProperties_test.cpp @@ -50,7 +50,6 @@ protected: void setupValidAxis(int axis, int32_t min, int32_t max, int32_t resolution) { EXPECT_CALL(mMockEventHub, getAbsoluteAxisInfo(EVENTHUB_ID, axis)) .WillRepeatedly(Return(std::optional<RawAbsoluteAxisInfo>{{ - .valid = true, .minValue = min, .maxValue = max, .flat = 0, diff --git a/services/inputflinger/tests/InputMapperTest.cpp b/services/inputflinger/tests/InputMapperTest.cpp index 7e96d5f5d7..7dff144f87 100644 --- a/services/inputflinger/tests/InputMapperTest.cpp +++ b/services/inputflinger/tests/InputMapperTest.cpp @@ -26,7 +26,9 @@ namespace android { using testing::_; +using testing::NiceMock; using testing::Return; +using testing::ReturnRef; void InputMapperUnitTest::SetUpWithBus(int bus) { mFakePolicy = sp<FakeInputReaderPolicy>::make(); @@ -43,23 +45,17 @@ void InputMapperUnitTest::SetUpWithBus(int bus) { EXPECT_CALL(mMockEventHub, getConfiguration(EVENTHUB_ID)).WillRepeatedly([&](int32_t) { return mPropertyMap; }); -} -void InputMapperUnitTest::createDevice() { - mDevice = std::make_unique<InputDevice>(&mMockInputReaderContext, DEVICE_ID, - /*generation=*/2, mIdentifier); - mDevice->addEmptyEventHubDevice(EVENTHUB_ID); + mDevice = std::make_unique<NiceMock<MockInputDevice>>(&mMockInputReaderContext, DEVICE_ID, + /*generation=*/2, mIdentifier); + ON_CALL((*mDevice), getConfiguration).WillByDefault(ReturnRef(mPropertyMap)); mDeviceContext = std::make_unique<InputDeviceContext>(*mDevice, EVENTHUB_ID); - std::list<NotifyArgs> args = - mDevice->configure(systemTime(), mReaderConfiguration, /*changes=*/{}); - ASSERT_THAT(args, testing::ElementsAre(testing::VariantWith<NotifyDeviceResetArgs>(_))); } void InputMapperUnitTest::setupAxis(int axis, bool valid, int32_t min, int32_t max, int32_t resolution) { EXPECT_CALL(mMockEventHub, getAbsoluteAxisInfo(EVENTHUB_ID, axis)) .WillRepeatedly(Return(valid ? std::optional<RawAbsoluteAxisInfo>{{ - .valid = true, .minValue = min, .maxValue = max, .flat = 0, diff --git a/services/inputflinger/tests/InputMapperTest.h b/services/inputflinger/tests/InputMapperTest.h index 88057dc497..fc27e4fefd 100644 --- a/services/inputflinger/tests/InputMapperTest.h +++ b/services/inputflinger/tests/InputMapperTest.h @@ -43,13 +43,6 @@ protected: virtual void SetUp() override { SetUpWithBus(0); } virtual void SetUpWithBus(int bus); - /** - * Initializes mDevice and mDeviceContext. When this happens, mDevice takes a copy of - * mPropertyMap, so tests that need to set configuration properties should do so before calling - * this. Others will most likely want to call it in their SetUp method. - */ - void createDevice(); - void setupAxis(int axis, bool valid, int32_t min, int32_t max, int32_t resolution); void expectScanCodes(bool present, std::set<int> scanCodes); @@ -67,7 +60,7 @@ protected: MockEventHubInterface mMockEventHub; sp<FakeInputReaderPolicy> mFakePolicy; MockInputReaderContext mMockInputReaderContext; - std::unique_ptr<InputDevice> mDevice; + std::unique_ptr<MockInputDevice> mDevice; std::unique_ptr<InputDeviceContext> mDeviceContext; InputReaderConfiguration mReaderConfiguration; @@ -123,11 +116,12 @@ protected: T& constructAndAddMapper(Args... args) { // ensure a device entry exists for this eventHubId mDevice->addEmptyEventHubDevice(EVENTHUB_ID); - // configure the empty device - configureDevice(/*changes=*/{}); - return mDevice->constructAndAddMapper<T>(EVENTHUB_ID, mFakePolicy->getReaderConfiguration(), - args...); + auto& mapper = + mDevice->constructAndAddMapper<T>(EVENTHUB_ID, + mFakePolicy->getReaderConfiguration(), args...); + configureDevice(/*changes=*/{}); + return mapper; } void setDisplayInfoAndReconfigure(ui::LogicalDisplayId displayId, int32_t width, int32_t height, diff --git a/services/inputflinger/tests/InputReader_test.cpp b/services/inputflinger/tests/InputReader_test.cpp index 8bb22d0963..48fd717631 100644 --- a/services/inputflinger/tests/InputReader_test.cpp +++ b/services/inputflinger/tests/InputReader_test.cpp @@ -4039,6 +4039,51 @@ TEST_F(KeyboardInputMapperTest, Process_GesureEventToSetFlagKeepTouchMode) { ASSERT_EQ(AKEY_EVENT_FLAG_FROM_SYSTEM | AKEY_EVENT_FLAG_KEEP_TOUCH_MODE, args.flags); } +/** + * When there is more than one KeyboardInputMapper for an InputDevice, each mapper should produce + * events that use the shared keyboard source across all mappers. This is to ensure that each + * input device generates key events in a consistent manner, regardless of which mapper produces + * the event. + */ +TEST_F(KeyboardInputMapperTest, UsesSharedKeyboardSource) { + mFakeEventHub->addKey(EVENTHUB_ID, KEY_HOME, 0, AKEYCODE_HOME, POLICY_FLAG_WAKE); + + // Add a mapper with SOURCE_KEYBOARD + KeyboardInputMapper& keyboardMapper = + constructAndAddMapper<KeyboardInputMapper>(AINPUT_SOURCE_KEYBOARD); + + process(keyboardMapper, ARBITRARY_TIME, 0, EV_KEY, KEY_HOME, 1); + ASSERT_NO_FATAL_FAILURE( + mFakeListener->assertNotifyKeyWasCalled(WithSource(AINPUT_SOURCE_KEYBOARD))); + process(keyboardMapper, ARBITRARY_TIME, 0, EV_KEY, KEY_HOME, 0); + ASSERT_NO_FATAL_FAILURE( + mFakeListener->assertNotifyKeyWasCalled(WithSource(AINPUT_SOURCE_KEYBOARD))); + + // Add a mapper with SOURCE_DPAD + KeyboardInputMapper& dpadMapper = + constructAndAddMapper<KeyboardInputMapper>(AINPUT_SOURCE_DPAD); + for (auto* mapper : {&keyboardMapper, &dpadMapper}) { + process(*mapper, ARBITRARY_TIME, 0, EV_KEY, KEY_HOME, 1); + ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyKeyWasCalled( + WithSource(AINPUT_SOURCE_KEYBOARD | AINPUT_SOURCE_DPAD))); + process(*mapper, ARBITRARY_TIME, 0, EV_KEY, KEY_HOME, 0); + ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyKeyWasCalled( + WithSource(AINPUT_SOURCE_KEYBOARD | AINPUT_SOURCE_DPAD))); + } + + // Add a mapper with SOURCE_GAMEPAD + KeyboardInputMapper& gamepadMapper = + constructAndAddMapper<KeyboardInputMapper>(AINPUT_SOURCE_GAMEPAD); + for (auto* mapper : {&keyboardMapper, &dpadMapper, &gamepadMapper}) { + process(*mapper, ARBITRARY_TIME, 0, EV_KEY, KEY_HOME, 1); + ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyKeyWasCalled( + WithSource(AINPUT_SOURCE_KEYBOARD | AINPUT_SOURCE_DPAD | AINPUT_SOURCE_GAMEPAD))); + process(*mapper, ARBITRARY_TIME, 0, EV_KEY, KEY_HOME, 0); + ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyKeyWasCalled( + WithSource(AINPUT_SOURCE_KEYBOARD | AINPUT_SOURCE_DPAD | AINPUT_SOURCE_GAMEPAD))); + } +} + // --- KeyboardInputMapperTest_ExternalAlphabeticDevice --- class KeyboardInputMapperTest_ExternalAlphabeticDevice : public InputMapperTest { diff --git a/services/inputflinger/tests/InterfaceMocks.h b/services/inputflinger/tests/InterfaceMocks.h index 48e0b4f516..d51c708adb 100644 --- a/services/inputflinger/tests/InterfaceMocks.h +++ b/services/inputflinger/tests/InterfaceMocks.h @@ -26,6 +26,7 @@ #include <vector> #include <EventHub.h> +#include <InputDevice.h> #include <InputReaderBase.h> #include <InputReaderContext.h> #include <NotifyArgs.h> @@ -59,7 +60,7 @@ public: MOCK_METHOD(void, requestTimeoutAtTime, (nsecs_t when), (override)); int32_t bumpGeneration() override { return ++mGeneration; } - MOCK_METHOD(void, getExternalStylusDevices, (std::vector<InputDeviceInfo> & outDevices), + MOCK_METHOD(void, getExternalStylusDevices, (std::vector<InputDeviceInfo>& outDevices), (override)); MOCK_METHOD(std::list<NotifyArgs>, dispatchExternalStylusState, (const StylusState& outState), (override)); @@ -172,7 +173,7 @@ public: MOCK_METHOD(void, requestReopenDevices, (), (override)); MOCK_METHOD(void, wake, (), (override)); - MOCK_METHOD(void, dump, (std::string & dump), (const, override)); + MOCK_METHOD(void, dump, (std::string& dump), (const, override)); MOCK_METHOD(void, monitor, (), (const, override)); MOCK_METHOD(bool, isDeviceEnabled, (int32_t deviceId), (const, override)); MOCK_METHOD(status_t, enableDevice, (int32_t deviceId), (override)); @@ -190,4 +191,75 @@ public: MOCK_METHOD(void, notifyMouseCursorFadedOnTyping, (), (override)); }; +class MockInputDevice : public InputDevice { +public: + MockInputDevice(InputReaderContext* context, int32_t id, int32_t generation, + const InputDeviceIdentifier& identifier) + : InputDevice(context, id, generation, identifier) {} + + MOCK_METHOD(uint32_t, getSources, (), (const, override)); + MOCK_METHOD(bool, isEnabled, (), ()); + + MOCK_METHOD(void, dump, (std::string& dump, const std::string& eventHubDevStr), ()); + MOCK_METHOD(void, addEmptyEventHubDevice, (int32_t eventHubId), ()); + MOCK_METHOD(std::list<NotifyArgs>, addEventHubDevice, + (nsecs_t when, int32_t eventHubId, const InputReaderConfiguration& readerConfig), + ()); + MOCK_METHOD(void, removeEventHubDevice, (int32_t eventHubId), ()); + MOCK_METHOD(std::list<NotifyArgs>, configure, + (nsecs_t when, const InputReaderConfiguration& readerConfig, + ConfigurationChanges changes), + ()); + MOCK_METHOD(std::list<NotifyArgs>, reset, (nsecs_t when), ()); + MOCK_METHOD(std::list<NotifyArgs>, process, (const RawEvent* rawEvents, size_t count), ()); + MOCK_METHOD(std::list<NotifyArgs>, timeoutExpired, (nsecs_t when), ()); + MOCK_METHOD(std::list<NotifyArgs>, updateExternalStylusState, (const StylusState& state), ()); + + MOCK_METHOD(InputDeviceInfo, getDeviceInfo, (), ()); + MOCK_METHOD(int32_t, getKeyCodeState, (uint32_t sourceMask, int32_t keyCode), ()); + MOCK_METHOD(int32_t, getScanCodeState, (uint32_t sourceMask, int32_t scanCode), ()); + MOCK_METHOD(int32_t, getSwitchState, (uint32_t sourceMask, int32_t switchCode), ()); + MOCK_METHOD(int32_t, getKeyCodeForKeyLocation, (int32_t locationKeyCode), (const)); + MOCK_METHOD(bool, markSupportedKeyCodes, + (uint32_t sourceMask, const std::vector<int32_t>& keyCodes, uint8_t* outFlags), ()); + MOCK_METHOD(std::list<NotifyArgs>, vibrate, + (const VibrationSequence& sequence, ssize_t repeat, int32_t token), ()); + MOCK_METHOD(std::list<NotifyArgs>, cancelVibrate, (int32_t token), ()); + MOCK_METHOD(bool, isVibrating, (), ()); + MOCK_METHOD(std::vector<int32_t>, getVibratorIds, (), ()); + MOCK_METHOD(std::list<NotifyArgs>, cancelTouch, (nsecs_t when, nsecs_t readTime), ()); + MOCK_METHOD(bool, enableSensor, + (InputDeviceSensorType sensorType, std::chrono::microseconds samplingPeriod, + std::chrono::microseconds maxBatchReportLatency), + ()); + + MOCK_METHOD(void, disableSensor, (InputDeviceSensorType sensorType), ()); + MOCK_METHOD(void, flushSensor, (InputDeviceSensorType sensorType), ()); + + MOCK_METHOD(std::optional<int32_t>, getBatteryEventHubId, (), (const)); + + MOCK_METHOD(bool, setLightColor, (int32_t lightId, int32_t color), ()); + MOCK_METHOD(bool, setLightPlayerId, (int32_t lightId, int32_t playerId), ()); + MOCK_METHOD(std::optional<int32_t>, getLightColor, (int32_t lightId), ()); + MOCK_METHOD(std::optional<int32_t>, getLightPlayerId, (int32_t lightId), ()); + + MOCK_METHOD(int32_t, getMetaState, (), ()); + MOCK_METHOD(void, updateMetaState, (int32_t keyCode), ()); + + MOCK_METHOD(void, addKeyRemapping, (int32_t fromKeyCode, int32_t toKeyCode), ()); + + MOCK_METHOD(void, setKeyboardType, (KeyboardType keyboardType), ()); + + MOCK_METHOD(void, bumpGeneration, (), ()); + + MOCK_METHOD(const PropertyMap&, getConfiguration, (), (const, override)); + + MOCK_METHOD(NotifyDeviceResetArgs, notifyReset, (nsecs_t when), ()); + + MOCK_METHOD(std::optional<ui::LogicalDisplayId>, getAssociatedDisplayId, (), ()); + + MOCK_METHOD(void, updateLedState, (bool reset), ()); + + MOCK_METHOD(size_t, getMapperCount, (), ()); +}; } // namespace android diff --git a/services/inputflinger/tests/KeyboardInputMapper_test.cpp b/services/inputflinger/tests/KeyboardInputMapper_test.cpp index d3e8dee4a0..88c25d302a 100644 --- a/services/inputflinger/tests/KeyboardInputMapper_test.cpp +++ b/services/inputflinger/tests/KeyboardInputMapper_test.cpp @@ -55,7 +55,6 @@ protected: void SetUp() override { InputMapperUnitTest::SetUp(); - createDevice(); // set key-codes expected in tests for (const auto& [scanCode, outKeycode] : mKeyCodeMap) { @@ -66,6 +65,8 @@ protected: mFakePolicy = sp<FakeInputReaderPolicy>::make(); EXPECT_CALL(mMockInputReaderContext, getPolicy).WillRepeatedly(Return(mFakePolicy.get())); + ON_CALL((*mDevice), getSources).WillByDefault(Return(AINPUT_SOURCE_KEYBOARD)); + mMapper = createInputMapper<KeyboardInputMapper>(*mDeviceContext, mReaderConfiguration, AINPUT_SOURCE_KEYBOARD); } diff --git a/services/inputflinger/tests/MultiTouchInputMapper_test.cpp b/services/inputflinger/tests/MultiTouchInputMapper_test.cpp index d4d3c3857f..9a6b266b21 100644 --- a/services/inputflinger/tests/MultiTouchInputMapper_test.cpp +++ b/services/inputflinger/tests/MultiTouchInputMapper_test.cpp @@ -109,7 +109,6 @@ protected: mFakePolicy->addDisplayViewport(DISPLAY_ID, DISPLAY_WIDTH, DISPLAY_HEIGHT, ui::ROTATION_0, /*isActive=*/true, "local:0", NO_PORT, ViewportType::INTERNAL); - createDevice(); mMapper = createInputMapper<MultiTouchInputMapper>(*mDeviceContext, mFakePolicy->getReaderConfiguration()); } diff --git a/services/inputflinger/tests/MultiTouchMotionAccumulator_test.cpp b/services/inputflinger/tests/MultiTouchMotionAccumulator_test.cpp index b441a23803..9ddb8c138d 100644 --- a/services/inputflinger/tests/MultiTouchMotionAccumulator_test.cpp +++ b/services/inputflinger/tests/MultiTouchMotionAccumulator_test.cpp @@ -23,10 +23,7 @@ class MultiTouchMotionAccumulatorTest : public InputMapperUnitTest { protected: static constexpr size_t SLOT_COUNT = 8; - void SetUp() override { - InputMapperUnitTest::SetUp(); - createDevice(); - } + void SetUp() override { InputMapperUnitTest::SetUp(); } MultiTouchMotionAccumulator mMotionAccumulator; diff --git a/services/inputflinger/tests/RotaryEncoderInputMapper_test.cpp b/services/inputflinger/tests/RotaryEncoderInputMapper_test.cpp index 366b3dcdd0..a796c49dbe 100644 --- a/services/inputflinger/tests/RotaryEncoderInputMapper_test.cpp +++ b/services/inputflinger/tests/RotaryEncoderInputMapper_test.cpp @@ -138,7 +138,6 @@ TEST_F(RotaryEncoderInputMapperTest, ConfigureDisplayIdWithAssociatedViewport) { mReaderConfiguration.setDisplayViewports({primaryViewport, secondaryViewport}); // Set up the secondary display as the associated viewport of the mapper. - createDevice(); ViewportFakingInputDeviceContext deviceContext(*mDevice, EVENTHUB_ID, secondaryViewport); mMapper = createInputMapper<RotaryEncoderInputMapper>(deviceContext, mReaderConfiguration); @@ -159,7 +158,6 @@ TEST_F(RotaryEncoderInputMapperTest, ConfigureDisplayIdNoAssociatedViewport) { mFakePolicy->addDisplayViewport(createPrimaryViewport()); // Set up the mapper with no associated viewport. - createDevice(); mMapper = createInputMapper<RotaryEncoderInputMapper>(*mDeviceContext, mReaderConfiguration); // Ensure input events are generated without display ID @@ -174,7 +172,6 @@ TEST_F(RotaryEncoderInputMapperTest, ConfigureDisplayIdNoAssociatedViewport) { } TEST_F(RotaryEncoderInputMapperTest, ProcessRegularScroll) { - createDevice(); mMapper = createInputMapper<RotaryEncoderInputMapper>(*mDeviceContext, mReaderConfiguration); std::list<NotifyArgs> args; @@ -191,7 +188,6 @@ TEST_F(RotaryEncoderInputMapperTest, ProcessHighResScroll) { vd_flags::high_resolution_scroll(true); EXPECT_CALL(mMockEventHub, hasRelativeAxis(EVENTHUB_ID, REL_WHEEL_HI_RES)) .WillRepeatedly(Return(true)); - createDevice(); mMapper = createInputMapper<RotaryEncoderInputMapper>(*mDeviceContext, mReaderConfiguration); std::list<NotifyArgs> args; @@ -208,7 +204,6 @@ TEST_F(RotaryEncoderInputMapperTest, HighResScrollIgnoresRegularScroll) { vd_flags::high_resolution_scroll(true); EXPECT_CALL(mMockEventHub, hasRelativeAxis(EVENTHUB_ID, REL_WHEEL_HI_RES)) .WillRepeatedly(Return(true)); - createDevice(); mMapper = createInputMapper<RotaryEncoderInputMapper>(*mDeviceContext, mReaderConfiguration); std::list<NotifyArgs> args; diff --git a/services/inputflinger/tests/SwitchInputMapper_test.cpp b/services/inputflinger/tests/SwitchInputMapper_test.cpp index 4020e78701..ebbf10b8db 100644 --- a/services/inputflinger/tests/SwitchInputMapper_test.cpp +++ b/services/inputflinger/tests/SwitchInputMapper_test.cpp @@ -33,7 +33,6 @@ class SwitchInputMapperTest : public InputMapperUnitTest { protected: void SetUp() override { InputMapperUnitTest::SetUp(); - createDevice(); mMapper = createInputMapper<SwitchInputMapper>(*mDeviceContext, mFakePolicy->getReaderConfiguration()); } diff --git a/services/inputflinger/tests/TouchpadInputMapper_test.cpp b/services/inputflinger/tests/TouchpadInputMapper_test.cpp index 1afb4f090a..fc8a7dafb0 100644 --- a/services/inputflinger/tests/TouchpadInputMapper_test.cpp +++ b/services/inputflinger/tests/TouchpadInputMapper_test.cpp @@ -109,7 +109,6 @@ protected: .WillRepeatedly([]() -> base::Result<std::vector<int32_t>> { return base::ResultError("Axis not supported", NAME_NOT_FOUND); }); - createDevice(); mMapper = createInputMapper<TouchpadInputMapper>(*mDeviceContext, mReaderConfiguration); } }; diff --git a/services/inputflinger/tests/VibratorInputMapper_test.cpp b/services/inputflinger/tests/VibratorInputMapper_test.cpp index aa4a6bb382..6e3344c345 100644 --- a/services/inputflinger/tests/VibratorInputMapper_test.cpp +++ b/services/inputflinger/tests/VibratorInputMapper_test.cpp @@ -36,7 +36,6 @@ class VibratorInputMapperTest : public InputMapperUnitTest { protected: void SetUp() override { InputMapperUnitTest::SetUp(); - createDevice(); EXPECT_CALL(mMockEventHub, getDeviceClasses(EVENTHUB_ID)) .WillRepeatedly(testing::Return(InputDeviceClass::VIBRATOR)); EXPECT_CALL(mMockEventHub, getVibratorIds(EVENTHUB_ID)) diff --git a/services/inputflinger/tests/fuzzers/MapperHelpers.h b/services/inputflinger/tests/fuzzers/MapperHelpers.h index bf56d3aac2..fea0d9a1c1 100644 --- a/services/inputflinger/tests/fuzzers/MapperHelpers.h +++ b/services/inputflinger/tests/fuzzers/MapperHelpers.h @@ -129,7 +129,6 @@ public: } if (mFdp->ConsumeBool()) { return std::optional<RawAbsoluteAxisInfo>({ - .valid = mFdp->ConsumeBool(), .minValue = mFdp->ConsumeIntegral<int32_t>(), .maxValue = mFdp->ConsumeIntegral<int32_t>(), .flat = mFdp->ConsumeIntegral<int32_t>(), diff --git a/services/inputflinger/tests/fuzzers/TouchpadInputFuzzer.cpp b/services/inputflinger/tests/fuzzers/TouchpadInputFuzzer.cpp index c620032eef..ebbb311512 100644 --- a/services/inputflinger/tests/fuzzers/TouchpadInputFuzzer.cpp +++ b/services/inputflinger/tests/fuzzers/TouchpadInputFuzzer.cpp @@ -34,7 +34,6 @@ void setAxisInfo(ThreadSafeFuzzedDataProvider& fdp, FuzzEventHub& eventHub, int3 if (fdp.ConsumeBool()) { eventHub.setAbsoluteAxisInfo(id, axis, RawAbsoluteAxisInfo{ - .valid = fdp.ConsumeBool(), .minValue = fdp.ConsumeIntegral<int32_t>(), .maxValue = fdp.ConsumeIntegral<int32_t>(), .flat = fdp.ConsumeIntegral<int32_t>(), 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<ITransactionCompletedListener>& l return; } SFTRACE_FORMAT_INSTANT("callReleaseBufferCallback %s - %" PRIu64, getDebugName(), framenumber); - ReleaseCallbackId callbackId{buffer->getId(), framenumber}; - const sp<Fence>& 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<CallbackHandle> Layer::findCallbackHandle() { @@ -2900,7 +2897,6 @@ void Layer::onLayerDisplayed(ftl::SharedFuture<FenceResult> 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<gui::BufferReleaseChannel::ProducerEndpoint>& 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<gui::BufferReleaseChannel::ProducerEndpoint> 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<gui::BufferReleaseChannel::ProducerEndpoint>& channel); // Creates a new handle each time, so we only expect // this to be called once. diff --git a/services/surfaceflinger/Scheduler/MessageQueue.cpp b/services/surfaceflinger/Scheduler/MessageQueue.cpp index 6a67ac5d42..2e1f938126 100644 --- a/services/surfaceflinger/Scheduler/MessageQueue.cpp +++ b/services/surfaceflinger/Scheduler/MessageQueue.cpp @@ -188,12 +188,13 @@ void MessageQueue::scheduleConfigure() { postMessage(sp<ConfigureHandler>::make(mCompositor)); } -void MessageQueue::scheduleFrame() { +void MessageQueue::scheduleFrame(Duration workDurationSlack) { SFTRACE_CALL(); std::lock_guard lock(mVsync.mutex); + const auto workDuration = Duration(mVsync.workDuration.get() - workDurationSlack); mVsync.scheduledFrameTimeOpt = - mVsync.registration->schedule({.workDuration = mVsync.workDuration.get().count(), + mVsync.registration->schedule({.workDuration = workDuration.ns(), .readyDuration = 0, .lastVsync = mVsync.lastCallbackTime.ns()}); } diff --git a/services/surfaceflinger/Scheduler/MessageQueue.h b/services/surfaceflinger/Scheduler/MessageQueue.h index c5fc371d3a..ba1efbe58f 100644 --- a/services/surfaceflinger/Scheduler/MessageQueue.h +++ b/services/surfaceflinger/Scheduler/MessageQueue.h @@ -74,7 +74,7 @@ public: virtual void postMessage(sp<MessageHandler>&&) = 0; virtual void postMessageDelayed(sp<MessageHandler>&&, nsecs_t uptimeDelay) = 0; virtual void scheduleConfigure() = 0; - virtual void scheduleFrame() = 0; + virtual void scheduleFrame(Duration workDurationSlack = Duration::fromNs(0)) = 0; virtual std::optional<scheduler::ScheduleResult> getScheduledFrameResult() const = 0; }; @@ -149,7 +149,7 @@ public: void postMessageDelayed(sp<MessageHandler>&&, nsecs_t uptimeDelay) override; void scheduleConfigure() override; - void scheduleFrame() override; + void scheduleFrame(Duration workDurationSlack = Duration::fromNs(0)) override; std::optional<scheduler::ScheduleResult> getScheduledFrameResult() const override; }; diff --git a/services/surfaceflinger/Scheduler/VSyncPredictor.cpp b/services/surfaceflinger/Scheduler/VSyncPredictor.cpp index ee7eda1354..04491a25c3 100644 --- a/services/surfaceflinger/Scheduler/VSyncPredictor.cpp +++ b/services/surfaceflinger/Scheduler/VSyncPredictor.cpp @@ -452,7 +452,7 @@ Duration VSyncPredictor::ensureMinFrameDurationIsKept(TimePoint expectedPresentT const auto currentPeriod = mRateMap.find(idealPeriod())->second.slope; const auto threshold = currentPeriod / 2; - const auto minFramePeriod = minFramePeriodLocked().ns(); + const auto minFramePeriod = minFramePeriodLocked(); auto prev = lastConfirmedPresentTime.ns(); for (auto& current : mPastExpectedPresentTimes) { @@ -463,10 +463,10 @@ Duration VSyncPredictor::ensureMinFrameDurationIsKept(TimePoint expectedPresentT 1e6f); } - const auto minPeriodViolation = current.ns() - prev + threshold < minFramePeriod; + const auto minPeriodViolation = current.ns() - prev + threshold < minFramePeriod.ns(); if (minPeriodViolation) { SFTRACE_NAME("minPeriodViolation"); - current = TimePoint::fromNs(prev + minFramePeriod); + current = TimePoint::fromNs(prev + minFramePeriod.ns()); prev = current.ns(); } else { break; @@ -477,7 +477,7 @@ Duration VSyncPredictor::ensureMinFrameDurationIsKept(TimePoint expectedPresentT const auto phase = Duration(mPastExpectedPresentTimes.back() - expectedPresentTime); if (phase > 0ns) { for (auto& timeline : mTimelines) { - timeline.shiftVsyncSequence(phase); + timeline.shiftVsyncSequence(phase, minFramePeriod); } mPastExpectedPresentTimes.clear(); return phase; @@ -778,8 +778,15 @@ bool VSyncPredictor::VsyncTimeline::isVSyncInPhase(Model model, nsecs_t vsync, F return vsyncSequence.seq % divisor == 0; } -void VSyncPredictor::VsyncTimeline::shiftVsyncSequence(Duration phase) { +void VSyncPredictor::VsyncTimeline::shiftVsyncSequence(Duration phase, Period minFramePeriod) { if (mLastVsyncSequence) { + const auto renderRate = mRenderRateOpt.value_or(Fps::fromPeriodNsecs(mIdealPeriod.ns())); + const auto threshold = mIdealPeriod.ns() / 2; + if (renderRate.getPeriodNsecs() - phase.ns() + threshold >= minFramePeriod.ns()) { + SFTRACE_FORMAT_INSTANT("Not-Adjusting vsync by %.2f", + static_cast<float>(phase.ns()) / 1e6f); + return; + } SFTRACE_FORMAT_INSTANT("adjusting vsync by %.2f", static_cast<float>(phase.ns()) / 1e6f); mLastVsyncSequence->vsyncTime += phase.ns(); } diff --git a/services/surfaceflinger/Scheduler/VSyncPredictor.h b/services/surfaceflinger/Scheduler/VSyncPredictor.h index 9e1c90bbef..6c8a2f29f9 100644 --- a/services/surfaceflinger/Scheduler/VSyncPredictor.h +++ b/services/surfaceflinger/Scheduler/VSyncPredictor.h @@ -104,7 +104,7 @@ private: void freeze(TimePoint lastVsync); std::optional<TimePoint> validUntil() const { return mValidUntil; } bool isVSyncInPhase(Model, nsecs_t vsync, Fps frameRate); - void shiftVsyncSequence(Duration phase); + void shiftVsyncSequence(Duration phase, Period minFramePeriod); void setRenderRate(std::optional<Fps> renderRateOpt) { mRenderRateOpt = renderRateOpt; } enum class VsyncOnTimeline { diff --git a/services/surfaceflinger/Scheduler/include/scheduler/FrameTargeter.h b/services/surfaceflinger/Scheduler/include/scheduler/FrameTargeter.h index 38cb446a81..2185bb07ec 100644 --- a/services/surfaceflinger/Scheduler/include/scheduler/FrameTargeter.h +++ b/services/surfaceflinger/Scheduler/include/scheduler/FrameTargeter.h @@ -55,15 +55,11 @@ public: std::optional<TimePoint> earliestPresentTime() const { return mEarliestPresentTime; } - // The time of the VSYNC that preceded this frame. See `presentFenceForPastVsync` for details. - TimePoint pastVsyncTime(Period minFramePeriod) const; - - // Equivalent to `presentFenceForPastVsync` unless running N VSYNCs ahead. - const FenceTimePtr& presentFenceForPreviousFrame() const { - return mPresentFences.front().fenceTime; - } + // Equivalent to `expectedSignaledPresentFence` unless running N VSYNCs ahead. + const FenceTimePtr& presentFenceForPreviousFrame() const; bool isFramePending() const { return mFramePending; } + bool wouldBackpressureHwc() const { return mWouldBackpressureHwc; } bool didMissFrame() const { return mFrameMissed; } bool didMissHwcFrame() const { return mHwcFrameMissed && !mGpuFrameMissed; } FrameTime lastSignaledFrameTime() const { return mLastSignaledFrameTime; } @@ -72,7 +68,7 @@ protected: explicit FrameTarget(const std::string& displayLabel); ~FrameTarget() = default; - bool wouldPresentEarly(Period minFramePeriod) const; + bool wouldPresentEarly(Period vsyncPeriod, Period minFramePeriod) const; // Equivalent to `pastVsyncTime` unless running N VSYNCs ahead. TimePoint previousFrameVsyncTime(Period minFramePeriod) const { @@ -81,8 +77,7 @@ protected: void addFence(sp<Fence> presentFence, FenceTimePtr presentFenceTime, TimePoint expectedPresentTime) { - mFenceWithFenceTimes.next() = {std::move(presentFence), presentFenceTime, - expectedPresentTime}; + mPresentFences.next() = {std::move(presentFence), presentFenceTime, expectedPresentTime}; } VsyncId mVsyncId; @@ -94,8 +89,9 @@ protected: TracedOrdinal<bool> mFrameMissed; TracedOrdinal<bool> mHwcFrameMissed; TracedOrdinal<bool> mGpuFrameMissed; + bool mWouldBackpressureHwc = false; - struct FenceWithFenceTime { + struct PresentFence { sp<Fence> fence = Fence::NO_FENCE; FenceTimePtr fenceTime = FenceTime::NO_FENCE; TimePoint expectedPresentTime = TimePoint(); @@ -106,9 +102,10 @@ protected: // VSYNC of at least one previous frame has not yet passed. In other words, this is NOT the // `presentFenceForPreviousFrame` if running N VSYNCs ahead, but the one that should have been // signaled by now (unless that frame missed). - FenceWithFenceTime presentFenceForPastVsync(Period minFramePeriod) const; - std::array<FenceWithFenceTime, 2> mPresentFences; - utils::RingBuffer<FenceWithFenceTime, 5> mFenceWithFenceTimes; + std::pair<bool /* wouldBackpressure */, PresentFence> expectedSignaledPresentFence( + Period vsyncPeriod, Period minFramePeriod) const; + std::array<PresentFence, 2> mPresentFencesLegacy; + utils::RingBuffer<PresentFence, 5> mPresentFences; FrameTime mLastSignaledFrameTime; @@ -120,19 +117,6 @@ private: static_assert(N > 1); return expectedFrameDuration() > (N - 1) * minFramePeriod; } - - FenceWithFenceTime pastVsyncTimePtr() const { - FenceWithFenceTime pastFenceWithFenceTime; - for (size_t i = 0; i < mFenceWithFenceTimes.size(); i++) { - const auto& fenceWithFenceTime = mFenceWithFenceTimes[i]; - // TODO(b/354007767) Fix the below condition to avoid frame drop - if (fenceWithFenceTime.expectedPresentTime > mFrameBeginTime) { - return pastFenceWithFenceTime; - } - pastFenceWithFenceTime = fenceWithFenceTime; - } - return pastFenceWithFenceTime; - } }; // Computes a display's per-frame metrics about past/upcoming targeting of present deadlines. @@ -155,7 +139,7 @@ public: void beginFrame(const BeginFrameArgs&, const IVsyncSource&); - std::optional<TimePoint> computeEarliestPresentTime(Period minFramePeriod, + std::optional<TimePoint> computeEarliestPresentTime(Period vsyncPeriod, Period minFramePeriod, Duration hwcMinWorkDuration); // TODO(b/241285191): Merge with FrameTargeter::endFrame. diff --git a/services/surfaceflinger/Scheduler/src/FrameTargeter.cpp b/services/surfaceflinger/Scheduler/src/FrameTargeter.cpp index 1d248fbbe2..21f59493d2 100644 --- a/services/surfaceflinger/Scheduler/src/FrameTargeter.cpp +++ b/services/surfaceflinger/Scheduler/src/FrameTargeter.cpp @@ -18,8 +18,10 @@ #include <common/trace.h> #include <scheduler/FrameTargeter.h> #include <scheduler/IVsyncSource.h> +#include <utils/Log.h> namespace android::scheduler { +using namespace std::chrono_literals; FrameTarget::FrameTarget(const std::string& displayLabel) : mFramePending("PrevFramePending " + displayLabel, false), @@ -27,32 +29,48 @@ FrameTarget::FrameTarget(const std::string& displayLabel) mHwcFrameMissed("PrevHwcFrameMissed " + displayLabel, false), mGpuFrameMissed("PrevGpuFrameMissed " + displayLabel, false) {} -TimePoint FrameTarget::pastVsyncTime(Period minFramePeriod) const { - // TODO(b/267315508): Generalize to N VSYNCs. - const int shift = static_cast<int>(targetsVsyncsAhead<2>(minFramePeriod)); - return mExpectedPresentTime - Period::fromNs(minFramePeriod.ns() << shift); -} +std::pair<bool /* wouldBackpressure */, FrameTarget::PresentFence> +FrameTarget::expectedSignaledPresentFence(Period vsyncPeriod, Period minFramePeriod) const { + if (!FlagManager::getInstance().allow_n_vsyncs_in_targeter()) { + const size_t i = static_cast<size_t>(targetsVsyncsAhead<2>(minFramePeriod)); + return {true, mPresentFencesLegacy[i]}; + } -FrameTarget::FenceWithFenceTime FrameTarget::presentFenceForPastVsync(Period minFramePeriod) const { - if (FlagManager::getInstance().allow_n_vsyncs_in_targeter()) { - return pastVsyncTimePtr(); + bool wouldBackpressure = true; + auto expectedPresentTime = mExpectedPresentTime; + for (size_t i = mPresentFences.size(); i != 0; --i) { + const auto& fence = mPresentFences[i - 1]; + + if (fence.expectedPresentTime + minFramePeriod < expectedPresentTime - vsyncPeriod / 2) { + wouldBackpressure = false; + } + + if (fence.expectedPresentTime <= mFrameBeginTime) { + return {wouldBackpressure, fence}; + } + + expectedPresentTime = fence.expectedPresentTime; } - const size_t i = static_cast<size_t>(targetsVsyncsAhead<2>(minFramePeriod)); - return mPresentFences[i]; + return {wouldBackpressure, PresentFence{}}; } -bool FrameTarget::wouldPresentEarly(Period minFramePeriod) const { - // TODO(b/241285475): Since this is called during `composite`, the calls to `targetsVsyncsAhead` - // should use `TimePoint::now()` in case of delays since `mFrameBeginTime`. - - // TODO(b/267315508): Generalize to N VSYNCs. - const bool allowNVsyncs = FlagManager::getInstance().allow_n_vsyncs_in_targeter(); - if (!allowNVsyncs && targetsVsyncsAhead<3>(minFramePeriod)) { +bool FrameTarget::wouldPresentEarly(Period vsyncPeriod, Period minFramePeriod) const { + if (targetsVsyncsAhead<3>(minFramePeriod)) { return true; } - const auto fence = presentFenceForPastVsync(minFramePeriod).fenceTime; - return fence->isValid() && fence->getSignalTime() != Fence::SIGNAL_TIME_PENDING; + const auto [wouldBackpressure, fence] = + expectedSignaledPresentFence(vsyncPeriod, minFramePeriod); + return wouldBackpressure && fence.fenceTime->isValid() && + fence.fenceTime->getSignalTime() != Fence::SIGNAL_TIME_PENDING; +} + +const FenceTimePtr& FrameTarget::presentFenceForPreviousFrame() const { + if (FlagManager::getInstance().allow_n_vsyncs_in_targeter()) { + return mPresentFences.back().fenceTime; + } + + return mPresentFencesLegacy.front().fenceTime; } void FrameTargeter::beginFrame(const BeginFrameArgs& args, const IVsyncSource& vsyncSource) { @@ -86,27 +104,39 @@ void FrameTargeter::beginFrame(const BeginFrameArgs& args, const IVsyncSource& v } if (!mSupportsExpectedPresentTime) { - mEarliestPresentTime = computeEarliestPresentTime(minFramePeriod, args.hwcMinWorkDuration); + mEarliestPresentTime = + computeEarliestPresentTime(vsyncPeriod, minFramePeriod, args.hwcMinWorkDuration); } SFTRACE_FORMAT("%s %" PRId64 " vsyncIn %.2fms%s", __func__, ftl::to_underlying(args.vsyncId), ticks<std::milli, float>(mExpectedPresentTime - TimePoint::now()), mExpectedPresentTime == args.expectedVsyncTime ? "" : " (adjusted)"); - FenceWithFenceTime pastPresentFence = presentFenceForPastVsync(minFramePeriod); + const auto [wouldBackpressure, fence] = + expectedSignaledPresentFence(vsyncPeriod, minFramePeriod); // In cases where the present fence is about to fire, give it a small grace period instead of // giving up on the frame. - // - // TODO(b/280667110): The grace period should depend on `sfWorkDuration` and `vsyncPeriod` being - // approximately equal, not whether backpressure propagation is enabled. - const int graceTimeForPresentFenceMs = static_cast<int>( - mBackpressureGpuComposition || !mCompositionCoverage.test(CompositionCoverage::Gpu)); + const int graceTimeForPresentFenceMs = [&] { + const bool considerBackpressure = + mBackpressureGpuComposition || !mCompositionCoverage.test(CompositionCoverage::Gpu); + + if (!FlagManager::getInstance().allow_n_vsyncs_in_targeter()) { + return static_cast<int>(considerBackpressure); + } + + if (!wouldBackpressure || !considerBackpressure) { + return 0; + } + + return static_cast<int>((std::abs(fence.expectedPresentTime.ns() - mFrameBeginTime.ns()) <= + Duration(1ms).ns())); + }(); // Pending frames may trigger backpressure propagation. const auto& isFencePending = *isFencePendingFuncPtr; - mFramePending = pastPresentFence.fenceTime != FenceTime::NO_FENCE && - isFencePending(pastPresentFence.fenceTime, graceTimeForPresentFenceMs); + mFramePending = fence.fenceTime != FenceTime::NO_FENCE && + isFencePending(fence.fenceTime, graceTimeForPresentFenceMs); // A frame is missed if the prior frame is still pending. If no longer pending, then we still // count the frame as missed if the predicted present time was further in the past than when the @@ -114,10 +144,10 @@ void FrameTargeter::beginFrame(const BeginFrameArgs& args, const IVsyncSource& v // than a typical frame duration, but should not be so small that it reports reasonable drift as // a missed frame. mFrameMissed = mFramePending || [&] { - const nsecs_t pastPresentTime = pastPresentFence.fenceTime->getSignalTime(); + const nsecs_t pastPresentTime = fence.fenceTime->getSignalTime(); if (pastPresentTime < 0) return false; mLastSignaledFrameTime = {.signalTime = TimePoint::fromNs(pastPresentTime), - .expectedPresentTime = pastPresentFence.expectedPresentTime}; + .expectedPresentTime = fence.expectedPresentTime}; const nsecs_t frameMissedSlop = vsyncPeriod.ns() / 2; return lastScheduledPresentTime.ns() < pastPresentTime - frameMissedSlop; }(); @@ -128,11 +158,14 @@ void FrameTargeter::beginFrame(const BeginFrameArgs& args, const IVsyncSource& v if (mFrameMissed) mFrameMissedCount++; if (mHwcFrameMissed) mHwcFrameMissedCount++; if (mGpuFrameMissed) mGpuFrameMissedCount++; + + mWouldBackpressureHwc = mFramePending && wouldBackpressure; } -std::optional<TimePoint> FrameTargeter::computeEarliestPresentTime(Period minFramePeriod, +std::optional<TimePoint> FrameTargeter::computeEarliestPresentTime(Period vsyncPeriod, + Period minFramePeriod, Duration hwcMinWorkDuration) { - if (wouldPresentEarly(minFramePeriod)) { + if (wouldPresentEarly(vsyncPeriod, minFramePeriod)) { return previousFrameVsyncTime(minFramePeriod) - hwcMinWorkDuration; } return {}; @@ -151,8 +184,8 @@ FenceTimePtr FrameTargeter::setPresentFence(sp<Fence> presentFence, FenceTimePtr if (FlagManager::getInstance().allow_n_vsyncs_in_targeter()) { addFence(std::move(presentFence), presentFenceTime, mExpectedPresentTime); } else { - mPresentFences[1] = mPresentFences[0]; - mPresentFences[0] = {std::move(presentFence), presentFenceTime, mExpectedPresentTime}; + mPresentFencesLegacy[1] = mPresentFencesLegacy[0]; + mPresentFencesLegacy[0] = {std::move(presentFence), presentFenceTime, mExpectedPresentTime}; } return presentFenceTime; } diff --git a/services/surfaceflinger/Scheduler/tests/FrameTargeterTest.cpp b/services/surfaceflinger/Scheduler/tests/FrameTargeterTest.cpp index 190d0621d0..bc73cbbfd5 100644 --- a/services/surfaceflinger/Scheduler/tests/FrameTargeterTest.cpp +++ b/services/surfaceflinger/Scheduler/tests/FrameTargeterTest.cpp @@ -53,12 +53,13 @@ public: const auto& target() const { return mTargeter.target(); } - bool wouldPresentEarly(Period minFramePeriod) const { - return target().wouldPresentEarly(minFramePeriod); + bool wouldPresentEarly(Period vsyncPeriod, Period minFramePeriod) const { + return target().wouldPresentEarly(vsyncPeriod, minFramePeriod); } - FrameTarget::FenceWithFenceTime presentFenceForPastVsync(Period minFramePeriod) const { - return target().presentFenceForPastVsync(minFramePeriod); + std::pair<bool /*wouldBackpressure*/, FrameTarget::PresentFence> expectedSignaledPresentFence( + Period vsyncPeriod, Period minFramePeriod) const { + return target().expectedSignaledPresentFence(vsyncPeriod, minFramePeriod); } struct Frame { @@ -173,7 +174,6 @@ TEST_F(FrameTargeterTest, inflatesExpectedPresentTime) { } TEST_F(FrameTargeterTest, recallsPastVsync) { - SET_FLAG_FOR_TEST(flags::allow_n_vsyncs_in_targeter, false); VsyncId vsyncId{111}; TimePoint frameBeginTime(1000ms); constexpr Fps kRefreshRate = 60_Hz; @@ -181,37 +181,72 @@ TEST_F(FrameTargeterTest, recallsPastVsync) { constexpr Duration kFrameDuration = 13ms; for (int n = 5; n-- > 0;) { - Frame frame(this, vsyncId++, frameBeginTime, kFrameDuration, kRefreshRate, kRefreshRate); - const auto fence = frame.end(); + FenceTimePtr fence; + { + Frame frame(this, vsyncId++, frameBeginTime, kFrameDuration, kRefreshRate, + kRefreshRate); + fence = frame.end(); + } - EXPECT_EQ(target().pastVsyncTime(kPeriod), frameBeginTime + kFrameDuration - kPeriod); - EXPECT_EQ(presentFenceForPastVsync(kPeriod).fenceTime, fence); + Frame frame(this, vsyncId++, frameBeginTime, kFrameDuration, kRefreshRate, kRefreshRate); + const auto [wouldBackpressure, presentFence] = + expectedSignaledPresentFence(kPeriod, kPeriod); + ASSERT_TRUE(wouldBackpressure); + EXPECT_EQ(presentFence.fenceTime, fence); } } -TEST_F(FrameTargeterTest, recallsPastVsyncTwoVsyncsAhead) { - SET_FLAG_FOR_TEST(flags::allow_n_vsyncs_in_targeter, false); - VsyncId vsyncId{222}; - TimePoint frameBeginTime(2000ms); - constexpr Fps kRefreshRate = 120_Hz; +TEST_F(FrameTargeterTest, wouldBackpressureAfterTime) { + SET_FLAG_FOR_TEST(flags::allow_n_vsyncs_in_targeter, true); + VsyncId vsyncId{111}; + TimePoint frameBeginTime(1000ms); + constexpr Fps kRefreshRate = 60_Hz; constexpr Period kPeriod = kRefreshRate.getPeriod(); - constexpr Duration kFrameDuration = 10ms; + constexpr Duration kFrameDuration = 13ms; - FenceTimePtr previousFence = FenceTime::NO_FENCE; + { Frame frame(this, vsyncId++, frameBeginTime, kFrameDuration, kRefreshRate, kRefreshRate); } + { + Frame frame(this, vsyncId++, frameBeginTime, kFrameDuration, kRefreshRate, kRefreshRate); - for (int n = 5; n-- > 0;) { + const auto [wouldBackpressure, presentFence] = + expectedSignaledPresentFence(kPeriod, kPeriod); + EXPECT_TRUE(wouldBackpressure); + } + { + frameBeginTime += kPeriod; Frame frame(this, vsyncId++, frameBeginTime, kFrameDuration, kRefreshRate, kRefreshRate); - const auto fence = frame.end(); + const auto [wouldBackpressure, presentFence] = + expectedSignaledPresentFence(kPeriod, kPeriod); + EXPECT_FALSE(wouldBackpressure); + } +} + +TEST_F(FrameTargeterTest, wouldBackpressureAfterTimeLegacy) { + SET_FLAG_FOR_TEST(flags::allow_n_vsyncs_in_targeter, false); + VsyncId vsyncId{111}; + TimePoint frameBeginTime(1000ms); + constexpr Fps kRefreshRate = 60_Hz; + constexpr Period kPeriod = kRefreshRate.getPeriod(); + constexpr Duration kFrameDuration = 13ms; - EXPECT_EQ(target().pastVsyncTime(kPeriod), frameBeginTime + kFrameDuration - 2 * kPeriod); - EXPECT_EQ(presentFenceForPastVsync(kPeriod).fenceTime, previousFence); + { Frame frame(this, vsyncId++, frameBeginTime, kFrameDuration, kRefreshRate, kRefreshRate); } + { + Frame frame(this, vsyncId++, frameBeginTime, kFrameDuration, kRefreshRate, kRefreshRate); - previousFence = fence; + const auto [wouldBackpressure, presentFence] = + expectedSignaledPresentFence(kPeriod, kPeriod); + EXPECT_TRUE(wouldBackpressure); + } + { + frameBeginTime += kPeriod; + Frame frame(this, vsyncId++, frameBeginTime, kFrameDuration, kRefreshRate, kRefreshRate); + const auto [wouldBackpressure, presentFence] = + expectedSignaledPresentFence(kPeriod, kPeriod); + EXPECT_TRUE(wouldBackpressure); } } -TEST_F(FrameTargeterTest, recallsPastNVsyncTwoVsyncsAhead) { - SET_FLAG_FOR_TEST(flags::allow_n_vsyncs_in_targeter, true); +TEST_F(FrameTargeterTest, recallsPastVsyncTwoVsyncsAhead) { VsyncId vsyncId{222}; TimePoint frameBeginTime(2000ms); constexpr Fps kRefreshRate = 120_Hz; @@ -219,80 +254,66 @@ TEST_F(FrameTargeterTest, recallsPastNVsyncTwoVsyncsAhead) { constexpr Duration kFrameDuration = 10ms; FenceTimePtr previousFence = FenceTime::NO_FENCE; - + FenceTimePtr currentFence = FenceTime::NO_FENCE; for (int n = 5; n-- > 0;) { Frame frame(this, vsyncId++, frameBeginTime, kFrameDuration, kRefreshRate, kRefreshRate); - const auto fence = frame.end(); - - const auto pastVsyncTime = frameBeginTime + kFrameDuration - 2 * kPeriod; - EXPECT_EQ(target().pastVsyncTime(kPeriod), pastVsyncTime); - EXPECT_EQ(presentFenceForPastVsync(kFrameDuration).fenceTime, previousFence); - - frameBeginTime += kPeriod; - previousFence = fence; + EXPECT_EQ(expectedSignaledPresentFence(kPeriod, kPeriod).second.fenceTime, previousFence); + previousFence = currentFence; + currentFence = frame.end(); } } -TEST_F(FrameTargeterTest, recallsPastVsyncTwoVsyncsAheadVrr) { - SET_FLAG_FOR_TEST(flags::vrr_config, true); - SET_FLAG_FOR_TEST(flags::allow_n_vsyncs_in_targeter, false); +TEST_F(FrameTargeterTest, recallsPastVsyncFiveVsyncsAhead) { + SET_FLAG_FOR_TEST(flags::allow_n_vsyncs_in_targeter, true); VsyncId vsyncId{222}; TimePoint frameBeginTime(2000ms); constexpr Fps kRefreshRate = 120_Hz; - constexpr Fps kPeakRefreshRate = 240_Hz; constexpr Period kPeriod = kRefreshRate.getPeriod(); - constexpr Duration kFrameDuration = 10ms; - - FenceTimePtr previousFence = FenceTime::NO_FENCE; + constexpr Duration kFrameDuration = 40ms; + FenceTimePtr firstFence = FenceTime::NO_FENCE; for (int n = 5; n-- > 0;) { - Frame frame(this, vsyncId++, frameBeginTime, kFrameDuration, kRefreshRate, - kPeakRefreshRate); + Frame frame(this, vsyncId++, frameBeginTime, kFrameDuration, kRefreshRate, kRefreshRate); const auto fence = frame.end(); - - EXPECT_EQ(target().pastVsyncTime(kPeriod), frameBeginTime + kFrameDuration - 2 * kPeriod); - EXPECT_EQ(presentFenceForPastVsync(kPeriod).fenceTime, previousFence); - - previousFence = fence; + if (firstFence == FenceTime::NO_FENCE) { + firstFence = fence; + } } + + Frame frame(this, vsyncId++, frameBeginTime, kFrameDuration, kRefreshRate, kRefreshRate); + EXPECT_EQ(expectedSignaledPresentFence(kPeriod, kPeriod).second.fenceTime, firstFence); } -TEST_F(FrameTargeterTest, recallsPastNVsyncTwoVsyncsAheadVrr) { +TEST_F(FrameTargeterTest, recallsPastVsyncTwoVsyncsAheadVrr) { SET_FLAG_FOR_TEST(flags::vrr_config, true); - SET_FLAG_FOR_TEST(flags::allow_n_vsyncs_in_targeter, true); VsyncId vsyncId{222}; TimePoint frameBeginTime(2000ms); constexpr Fps kRefreshRate = 120_Hz; - constexpr Fps kPeakRefreshRate = 240_Hz; + constexpr Fps kVsyncRate = 240_Hz; constexpr Period kPeriod = kRefreshRate.getPeriod(); + constexpr Period kVsyncPeriod = kVsyncRate.getPeriod(); constexpr Duration kFrameDuration = 10ms; FenceTimePtr previousFence = FenceTime::NO_FENCE; - + FenceTimePtr currentFence = FenceTime::NO_FENCE; for (int n = 5; n-- > 0;) { - Frame frame(this, vsyncId++, frameBeginTime, kFrameDuration, kRefreshRate, - kPeakRefreshRate); - const auto fence = frame.end(); - - const auto pastVsyncTime = frameBeginTime + kFrameDuration - 2 * kPeriod; - EXPECT_EQ(target().pastVsyncTime(kPeriod), pastVsyncTime); - EXPECT_EQ(presentFenceForPastVsync(kFrameDuration).fenceTime, previousFence); - - frameBeginTime += kPeriod; - previousFence = fence; + Frame frame(this, vsyncId++, frameBeginTime, kFrameDuration, kRefreshRate, kRefreshRate); + EXPECT_EQ(expectedSignaledPresentFence(kVsyncPeriod, kPeriod).second.fenceTime, + previousFence); + previousFence = currentFence; + currentFence = frame.end(); } } TEST_F(FrameTargeterTest, doesNotDetectEarlyPresentIfNoFence) { constexpr Period kPeriod = (60_Hz).getPeriod(); - EXPECT_EQ(presentFenceForPastVsync(kPeriod).fenceTime, FenceTime::NO_FENCE); - EXPECT_FALSE(wouldPresentEarly(kPeriod)); + EXPECT_EQ(expectedSignaledPresentFence(kPeriod, kPeriod).second.fenceTime, FenceTime::NO_FENCE); + EXPECT_FALSE(wouldPresentEarly(kPeriod, kPeriod)); } TEST_F(FrameTargeterTest, detectsEarlyPresent) { - SET_FLAG_FOR_TEST(flags::allow_n_vsyncs_in_targeter, false); VsyncId vsyncId{333}; TimePoint frameBeginTime(3000ms); constexpr Fps kRefreshRate = 60_Hz; @@ -300,20 +321,24 @@ TEST_F(FrameTargeterTest, detectsEarlyPresent) { // The target is not early while past present fences are pending. for (int n = 3; n-- > 0;) { - const Frame frame(this, vsyncId++, frameBeginTime, 10ms, kRefreshRate, kRefreshRate); - EXPECT_FALSE(wouldPresentEarly(kPeriod)); + { + const Frame frame(this, vsyncId++, frameBeginTime, 10ms, kRefreshRate, kRefreshRate); + } + EXPECT_FALSE(wouldPresentEarly(kPeriod, kPeriod)); EXPECT_FALSE(target().earliestPresentTime()); } // The target is early if the past present fence was signaled. - Frame frame(this, vsyncId++, frameBeginTime, 10ms, kRefreshRate, kRefreshRate); - const auto fence = frame.end(); - fence->signalForTest(frameBeginTime.ns()); + { + Frame frame(this, vsyncId++, frameBeginTime, 10ms, kRefreshRate, kRefreshRate); + const auto fence = frame.end(); + fence->signalForTest(frameBeginTime.ns()); + } Frame finalFrame(this, vsyncId++, frameBeginTime, 10ms, kRefreshRate, kRefreshRate); // `finalFrame` would present early, so it has an earliest present time. - EXPECT_TRUE(wouldPresentEarly(kPeriod)); + EXPECT_TRUE(wouldPresentEarly(kPeriod, kPeriod)); ASSERT_NE(std::nullopt, target().earliestPresentTime()); EXPECT_EQ(*target().earliestPresentTime(), target().expectedPresentTime() - kPeriod - kHwcMinWorkDuration); @@ -322,7 +347,6 @@ TEST_F(FrameTargeterTest, detectsEarlyPresent) { // Same as `detectsEarlyPresent`, above, but verifies that we do not set an earliest present time // when there is expected present time support. TEST_F(FrameTargeterWithExpectedPresentSupportTest, detectsEarlyPresent) { - SET_FLAG_FOR_TEST(flags::allow_n_vsyncs_in_targeter, false); VsyncId vsyncId{333}; TimePoint frameBeginTime(3000ms); constexpr Fps kRefreshRate = 60_Hz; @@ -330,26 +354,30 @@ TEST_F(FrameTargeterWithExpectedPresentSupportTest, detectsEarlyPresent) { // The target is not early while past present fences are pending. for (int n = 3; n-- > 0;) { - const Frame frame(this, vsyncId++, frameBeginTime, 10ms, kRefreshRate, kRefreshRate); - EXPECT_FALSE(wouldPresentEarly(kPeriod)); + { + const Frame frame(this, vsyncId++, frameBeginTime, 10ms, kRefreshRate, kRefreshRate); + } + EXPECT_FALSE(wouldPresentEarly(kPeriod, kPeriod)); EXPECT_FALSE(target().earliestPresentTime()); } // The target is early if the past present fence was signaled. - Frame frame(this, vsyncId++, frameBeginTime, 10ms, kRefreshRate, kRefreshRate); - const auto fence = frame.end(); - fence->signalForTest(frameBeginTime.ns()); + { + Frame frame(this, vsyncId++, frameBeginTime, 10ms, kRefreshRate, kRefreshRate); + + const auto fence = frame.end(); + fence->signalForTest(frameBeginTime.ns()); + } Frame finalFrame(this, vsyncId++, frameBeginTime, 10ms, kRefreshRate, kRefreshRate); // `finalFrame` would present early, but we have expected present time support, so it has no // earliest present time. - EXPECT_TRUE(wouldPresentEarly(kPeriod)); + EXPECT_TRUE(wouldPresentEarly(kPeriod, kPeriod)); ASSERT_EQ(std::nullopt, target().earliestPresentTime()); } TEST_F(FrameTargeterTest, detectsEarlyPresentTwoVsyncsAhead) { - SET_FLAG_FOR_TEST(flags::allow_n_vsyncs_in_targeter, false); VsyncId vsyncId{444}; TimePoint frameBeginTime(4000ms); constexpr Fps kRefreshRate = 120_Hz; @@ -357,17 +385,21 @@ TEST_F(FrameTargeterTest, detectsEarlyPresentTwoVsyncsAhead) { // The target is not early while past present fences are pending. for (int n = 3; n-- > 0;) { - const Frame frame(this, vsyncId++, frameBeginTime, 10ms, kRefreshRate, kRefreshRate); - EXPECT_FALSE(wouldPresentEarly(kPeriod)); + { + const Frame frame(this, vsyncId++, frameBeginTime, 10ms, kRefreshRate, kRefreshRate); + } + EXPECT_FALSE(wouldPresentEarly(kPeriod, kPeriod)); EXPECT_FALSE(target().earliestPresentTime()); } + { + Frame frame(this, vsyncId++, frameBeginTime, 10ms, kRefreshRate, kRefreshRate); - Frame frame(this, vsyncId++, frameBeginTime, 10ms, kRefreshRate, kRefreshRate); - const auto fence = frame.end(); - fence->signalForTest(frameBeginTime.ns()); + const auto fence = frame.end(); + fence->signalForTest(frameBeginTime.ns()); + } // The target is two VSYNCs ahead, so the past present fence is still pending. - EXPECT_FALSE(wouldPresentEarly(kPeriod)); + EXPECT_FALSE(wouldPresentEarly(kPeriod, kPeriod)); EXPECT_FALSE(target().earliestPresentTime()); { const Frame frame(this, vsyncId++, frameBeginTime, 10ms, kRefreshRate, kRefreshRate); } @@ -375,66 +407,21 @@ TEST_F(FrameTargeterTest, detectsEarlyPresentTwoVsyncsAhead) { Frame finalFrame(this, vsyncId++, frameBeginTime, 10ms, kRefreshRate, kRefreshRate); // The target is early if the past present fence was signaled. - EXPECT_TRUE(wouldPresentEarly(kPeriod)); + EXPECT_TRUE(wouldPresentEarly(kPeriod, kPeriod)); ASSERT_NE(std::nullopt, target().earliestPresentTime()); EXPECT_EQ(*target().earliestPresentTime(), target().expectedPresentTime() - kPeriod - kHwcMinWorkDuration); } -TEST_F(FrameTargeterTest, detectsEarlyPresentNVsyncsAhead) { - SET_FLAG_FOR_TEST(flags::allow_n_vsyncs_in_targeter, true); - VsyncId vsyncId{444}; - TimePoint frameBeginTime(4000ms); - Fps refreshRate = 120_Hz; - Period period = refreshRate.getPeriod(); - - // The target is not early while past present fences are pending. - for (int n = 5; n-- > 0;) { - const Frame frame(this, vsyncId++, frameBeginTime, 10ms, refreshRate, refreshRate); - EXPECT_FALSE(wouldPresentEarly(period)); - EXPECT_FALSE(target().earliestPresentTime()); - } - - Frame frame(this, vsyncId++, frameBeginTime, 10ms, refreshRate, refreshRate); - auto fence = frame.end(); - frameBeginTime += period; - fence->signalForTest(frameBeginTime.ns()); - - // The target is two VSYNCs ahead, so the past present fence is still pending. - EXPECT_FALSE(wouldPresentEarly(period)); - EXPECT_FALSE(target().earliestPresentTime()); - - { const Frame frame(this, vsyncId++, frameBeginTime, 10ms, refreshRate, refreshRate); } - - Frame oneEarlyPresentFrame(this, vsyncId++, frameBeginTime, 10ms, refreshRate, refreshRate); - // The target is early if the past present fence was signaled. - EXPECT_TRUE(wouldPresentEarly(period)); - ASSERT_NE(std::nullopt, target().earliestPresentTime()); - EXPECT_EQ(*target().earliestPresentTime(), - target().expectedPresentTime() - period - kHwcMinWorkDuration); - - fence = oneEarlyPresentFrame.end(); - frameBeginTime += period; - fence->signalForTest(frameBeginTime.ns()); - - // Change rate to track frame more than 2 vsyncs ahead - refreshRate = 144_Hz; - period = refreshRate.getPeriod(); - Frame onePresentEarlyFrame(this, vsyncId++, frameBeginTime, 16ms, refreshRate, refreshRate); - // The target is not early as last frame as the past frame is tracked for pending. - EXPECT_FALSE(wouldPresentEarly(period)); -} - TEST_F(FrameTargeterTest, detectsEarlyPresentThreeVsyncsAhead) { - SET_FLAG_FOR_TEST(flags::allow_n_vsyncs_in_targeter, false); TimePoint frameBeginTime(5000ms); constexpr Fps kRefreshRate = 144_Hz; constexpr Period kPeriod = kRefreshRate.getPeriod(); - const Frame frame(this, VsyncId{555}, frameBeginTime, 16ms, kRefreshRate, kRefreshRate); + { const Frame frame(this, VsyncId{555}, frameBeginTime, 16ms, kRefreshRate, kRefreshRate); } // The target is more than two VSYNCs ahead, but present fences are not tracked that far back. - EXPECT_TRUE(wouldPresentEarly(kPeriod)); + EXPECT_TRUE(wouldPresentEarly(kPeriod, kPeriod)); EXPECT_TRUE(target().earliestPresentTime()); EXPECT_EQ(*target().earliestPresentTime(), target().expectedPresentTime() - kPeriod - kHwcMinWorkDuration); diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index f61763880e..299f70bf97 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -2161,12 +2161,12 @@ sp<IDisplayEventConnection> SurfaceFlinger::createDisplayEventConnection( return mScheduler->createDisplayEventConnection(cycle, eventRegistration, layerHandle); } -void SurfaceFlinger::scheduleCommit(FrameHint hint) { +void SurfaceFlinger::scheduleCommit(FrameHint hint, Duration workDurationSlack) { if (hint == FrameHint::kActive) { mScheduler->resetIdleTimer(); } mPowerAdvisor->notifyDisplayUpdateImminentAndCpuReset(); - mScheduler->scheduleFrame(); + mScheduler->scheduleFrame(workDurationSlack); } void SurfaceFlinger::scheduleComposite(FrameHint hint) { @@ -2620,13 +2620,16 @@ bool SurfaceFlinger::commit(PhysicalDisplayId pacesetterId, } } - if (pacesetterFrameTarget.isFramePending()) { + if (pacesetterFrameTarget.wouldBackpressureHwc()) { if (mBackpressureGpuComposition || pacesetterFrameTarget.didMissHwcFrame()) { if (FlagManager::getInstance().vrr_config()) { mScheduler->getVsyncSchedule()->getTracker().onFrameMissed( pacesetterFrameTarget.expectedPresentTime()); } - scheduleCommit(FrameHint::kNone); + const Duration slack = FlagManager::getInstance().allow_n_vsyncs_in_targeter() + ? TimePoint::now() - pacesetterFrameTarget.frameBeginTime() + : Duration::fromNs(0); + scheduleCommit(FrameHint::kNone, slack); return false; } } @@ -2913,6 +2916,9 @@ CompositeResultsPerDisplay SurfaceFlinger::composite( // Now that the current frame has been presented above, PowerAdvisor needs the present time // of the previous frame (whose fence is signaled by now) to determine how long the HWC had // waited on that fence to retire before presenting. + // TODO(b/355238809) `presentFenceForPreviousFrame` might not always be signaled (e.g. on + // devices + // where HWC does not block on the previous present fence). Revise this assumtion. const auto& previousPresentFence = pacesetterTarget.presentFenceForPreviousFrame(); mPowerAdvisor->setSfPresentTiming(TimePoint::fromNs(previousPresentFence->getSignalTime()), @@ -5789,10 +5795,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() || @@ -6635,7 +6637,7 @@ void SurfaceFlinger::dumpOffscreenLayers(std::string& result) { } void SurfaceFlinger::dumpHwcLayersMinidump(std::string& result) const { - for (const auto& [token, display] : FTL_FAKE_GUARD(mStateLock, mDisplays)) { + for (const auto& [token, display] : mDisplays) { const auto displayId = HalDisplayId::tryCast(display->getId()); if (!displayId) { continue; diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index 89ade4ec76..157b72244f 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -273,7 +273,7 @@ public: enum class FrameHint { kNone, kActive }; // Schedule commit of transactions on the main thread ahead of the next VSYNC. - void scheduleCommit(FrameHint); + void scheduleCommit(FrameHint, Duration workDurationSlack = Duration::fromNs(0)); // As above, but also force composite regardless if transactions were committed. void scheduleComposite(FrameHint); // As above, but also force dirty geometry to repaint. 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<CallbackHandle>& 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<Fence> 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<ListenerStats, 10> 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 <android-base/thread_annotations.h> #include <binder/IBinder.h> #include <ftl/future.h> -#include <gui/BufferReleaseChannel.h> #include <gui/ITransactionCompletedListener.h> #include <ui/Fence.h> #include <ui/FenceResult.h> @@ -60,7 +59,6 @@ public: uint64_t frameNumber = 0; uint64_t previousFrameNumber = 0; ReleaseCallbackId previousReleaseCallbackId = ReleaseCallbackId::INVALID_ID; - std::shared_ptr<gui::BufferReleaseChannel::ProducerEndpoint> bufferReleaseChannel; }; class TransactionCallbackInvoker { @@ -88,13 +86,6 @@ private: std::unordered_map<sp<IBinder>, std::deque<TransactionStats>, IListenerHash> mCompletedTransactions; - struct BufferRelease { - std::shared_ptr<gui::BufferReleaseChannel::ProducerEndpoint> channel; - ReleaseCallbackId callbackId; - sp<Fence> fence; - }; - std::vector<BufferRelease> mBufferReleases; - sp<Fence> mPresentFence; }; diff --git a/services/surfaceflinger/Utils/RingBuffer.h b/services/surfaceflinger/Utils/RingBuffer.h index 198e7b2cf1..215472b388 100644 --- a/services/surfaceflinger/Utils/RingBuffer.h +++ b/services/surfaceflinger/Utils/RingBuffer.h @@ -43,8 +43,10 @@ public: } T& front() { return (*this)[0]; } + const T& front() const { return (*this)[0]; } T& back() { return (*this)[size() - 1]; } + const T& back() const { return (*this)[size() - 1]; } T& operator[](size_t index) { return mBuffer[(static_cast<size_t>(mHead + 1) + index) % mCount]; diff --git a/services/surfaceflinger/tests/unittests/FrameRateSelectionStrategyTest.cpp b/services/surfaceflinger/tests/unittests/FrameRateSelectionStrategyTest.cpp index 5c742d7360..866eb08913 100644 --- a/services/surfaceflinger/tests/unittests/FrameRateSelectionStrategyTest.cpp +++ b/services/surfaceflinger/tests/unittests/FrameRateSelectionStrategyTest.cpp @@ -28,6 +28,7 @@ namespace android { +using testing::_; using testing::DoAll; using testing::Mock; using testing::SetArgPointee; @@ -91,7 +92,7 @@ INSTANTIATE_TEST_SUITE_P(PerLayerType, FrameRateSelectionStrategyTest, PrintToStringParamName); TEST_P(FrameRateSelectionStrategyTest, SetAndGet) { - EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame()).Times(1); + EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame(_)).Times(1); const auto& layerFactory = GetParam(); auto layer = mLayers.emplace_back(layerFactory->createLayer(mFlinger)); @@ -104,7 +105,7 @@ TEST_P(FrameRateSelectionStrategyTest, SetAndGet) { } TEST_P(FrameRateSelectionStrategyTest, SetChildOverrideChildren) { - EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame()).Times(1); + EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame(_)).Times(1); const auto& layerFactory = GetParam(); auto parent = mLayers.emplace_back(layerFactory->createLayer(mFlinger)); @@ -128,7 +129,7 @@ TEST_P(FrameRateSelectionStrategyTest, SetChildOverrideChildren) { } TEST_P(FrameRateSelectionStrategyTest, SetParentOverrideChildren) { - EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame()).Times(1); + EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame(_)).Times(1); const auto& layerFactory = GetParam(); auto layer1 = mLayers.emplace_back(layerFactory->createLayer(mFlinger)); @@ -169,7 +170,7 @@ TEST_P(FrameRateSelectionStrategyTest, SetParentOverrideChildren) { } TEST_P(FrameRateSelectionStrategyTest, OverrideChildrenAndSelf) { - EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame()).Times(1); + EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame(_)).Times(1); const auto& layerFactory = GetParam(); auto layer1 = mLayers.emplace_back(layerFactory->createLayer(mFlinger)); diff --git a/services/surfaceflinger/tests/unittests/SetFrameRateTest.cpp b/services/surfaceflinger/tests/unittests/SetFrameRateTest.cpp index 9899d4290b..4705dd1cc7 100644 --- a/services/surfaceflinger/tests/unittests/SetFrameRateTest.cpp +++ b/services/surfaceflinger/tests/unittests/SetFrameRateTest.cpp @@ -35,7 +35,7 @@ #include "mock/MockVsyncController.h" namespace android { - +using testing::_; using testing::DoAll; using testing::Mock; using testing::SetArgPointee; @@ -93,7 +93,7 @@ void SetFrameRateTest::commitTransaction() { namespace { TEST_P(SetFrameRateTest, SetAndGet) { - EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame()).Times(1); + EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame(_)).Times(1); const auto& layerFactory = GetParam(); @@ -104,7 +104,7 @@ TEST_P(SetFrameRateTest, SetAndGet) { } TEST_P(SetFrameRateTest, SetAndGetParent) { - EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame()).Times(1); + EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame(_)).Times(1); const auto& layerFactory = GetParam(); @@ -129,7 +129,7 @@ TEST_P(SetFrameRateTest, SetAndGetParent) { } TEST_P(SetFrameRateTest, SetAndGetParentAllVote) { - EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame()).Times(1); + EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame(_)).Times(1); const auto& layerFactory = GetParam(); @@ -168,7 +168,7 @@ TEST_P(SetFrameRateTest, SetAndGetParentAllVote) { } TEST_P(SetFrameRateTest, SetAndGetChild) { - EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame()).Times(1); + EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame(_)).Times(1); const auto& layerFactory = GetParam(); @@ -193,7 +193,7 @@ TEST_P(SetFrameRateTest, SetAndGetChild) { } TEST_P(SetFrameRateTest, SetAndGetChildAllVote) { - EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame()).Times(1); + EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame(_)).Times(1); const auto& layerFactory = GetParam(); @@ -232,7 +232,7 @@ TEST_P(SetFrameRateTest, SetAndGetChildAllVote) { } TEST_P(SetFrameRateTest, SetAndGetChildAddAfterVote) { - EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame()).Times(1); + EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame(_)).Times(1); const auto& layerFactory = GetParam(); @@ -262,7 +262,7 @@ TEST_P(SetFrameRateTest, SetAndGetChildAddAfterVote) { } TEST_P(SetFrameRateTest, SetAndGetChildRemoveAfterVote) { - EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame()).Times(1); + EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame(_)).Times(1); const auto& layerFactory = GetParam(); @@ -293,7 +293,7 @@ TEST_P(SetFrameRateTest, SetAndGetChildRemoveAfterVote) { } TEST_P(SetFrameRateTest, SetAndGetParentNotInTree) { - EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame()).Times(1); + EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame(_)).Times(1); const auto& layerFactory = GetParam(); @@ -352,7 +352,7 @@ TEST_P(SetFrameRateTest, SetOnParentActivatesTree) { } TEST_P(SetFrameRateTest, addChildForParentWithTreeVote) { - EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame()).Times(1); + EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame(_)).Times(1); const auto& layerFactory = GetParam(); diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_CreateDisplayTest.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_CreateDisplayTest.cpp index e5f2a9138d..2d3ebb47bd 100644 --- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_CreateDisplayTest.cpp +++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_CreateDisplayTest.cpp @@ -97,7 +97,7 @@ TEST_F(CreateDisplayTest, createDisplaySetsCurrentStateForNonsecureDisplay) { // Cleanup conditions // Creating the display commits a display transaction. - EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame()).Times(1); + EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame(_)).Times(1); } TEST_F(CreateDisplayTest, createDisplaySetsCurrentStateForSecureDisplay) { @@ -129,7 +129,7 @@ TEST_F(CreateDisplayTest, createDisplaySetsCurrentStateForSecureDisplay) { // Cleanup conditions // Creating the display commits a display transaction. - EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame()).Times(1); + EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame(_)).Times(1); } TEST_F(CreateDisplayTest, createDisplaySetsCurrentStateForUniqueId) { @@ -159,7 +159,7 @@ TEST_F(CreateDisplayTest, createDisplaySetsCurrentStateForUniqueId) { // Cleanup conditions // Creating the display commits a display transaction. - EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame()).Times(1); + EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame(_)).Times(1); } // Requesting 0 tells SF not to do anything, i.e., default to refresh as physical displays diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_DestroyDisplayTest.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_DestroyDisplayTest.cpp index f8ad8e1e1b..df8f68f2b6 100644 --- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_DestroyDisplayTest.cpp +++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_DestroyDisplayTest.cpp @@ -38,7 +38,7 @@ TEST_F(DestroyDisplayTest, destroyDisplayClearsCurrentStateForDisplay) { // Call Expectations // Destroying the display commits a display transaction. - EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame()).Times(1); + EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame(_)).Times(1); // -------------------------------------------------------------------- // Invocation diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_HotplugTest.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_HotplugTest.cpp index 897f9a0319..aef467ab9d 100644 --- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_HotplugTest.cpp +++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_HotplugTest.cpp @@ -48,7 +48,7 @@ TEST_F(HotplugTest, schedulesConfigureToProcessHotplugEvents) { TEST_F(HotplugTest, schedulesFrameToCommitDisplayTransaction) { EXPECT_CALL(*mFlinger.scheduler(), scheduleConfigure()).Times(1); - EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame()).Times(1); + EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame(_)).Times(1); constexpr HWDisplayId displayId1 = 456; mFlinger.onComposerHalHotplugEvent(displayId1, DisplayHotplugEvent::DISCONNECTED); @@ -73,7 +73,7 @@ TEST_F(HotplugTest, ignoresDuplicateDisconnection) { .WillOnce(Return(Error::NONE)); // A single commit should be scheduled for both configure calls. - EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame()).Times(1); + EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame(_)).Times(1); ExternalDisplay::injectPendingHotplugEvent(this, Connection::CONNECTED); mFlinger.configure(); @@ -116,7 +116,7 @@ TEST_F(HotplugTest, rejectsHotplugIfFailedToLoadDisplayModes) { setVsyncEnabled(ExternalDisplay::HWC_DISPLAY_ID, IComposerClient::Vsync::DISABLE)) .WillOnce(Return(Error::NONE)); - EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame()).Times(1); + EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame(_)).Times(1); ExternalDisplay::injectPendingHotplugEvent(this, Connection::CONNECTED); mFlinger.configure(); diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_InitializeDisplaysTest.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_InitializeDisplaysTest.cpp index eaf468432c..5231965277 100644 --- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_InitializeDisplaysTest.cpp +++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_InitializeDisplaysTest.cpp @@ -28,7 +28,7 @@ struct InitializeDisplaysTest : DualDisplayTransactionTest<hal::PowerMode::OFF, TEST_F(InitializeDisplaysTest, initializesDisplays) { // Scheduled by the display transaction, and by powering on each display. - EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame()).Times(3); + EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame(_)).Times(3); EXPECT_CALL(static_cast<mock::VSyncTracker&>( mFlinger.scheduler()->getVsyncSchedule()->getTracker()), diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetPowerModeInternalTest.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetPowerModeInternalTest.cpp index 83e2f980ce..fed7b2e767 100644 --- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetPowerModeInternalTest.cpp +++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetPowerModeInternalTest.cpp @@ -271,7 +271,7 @@ struct DisplayPowerCase { } static void setupRepaintEverythingCallExpectations(DisplayTransactionTest* test) { - EXPECT_CALL(*test->mFlinger.scheduler(), scheduleFrame()).Times(1); + EXPECT_CALL(*test->mFlinger.scheduler(), scheduleFrame(_)).Times(1); } static void setupComposerCallExpectations(DisplayTransactionTest* test, PowerMode mode) { diff --git a/services/surfaceflinger/tests/unittests/TestableScheduler.h b/services/surfaceflinger/tests/unittests/TestableScheduler.h index f0638094c7..0814e3d6fe 100644 --- a/services/surfaceflinger/tests/unittests/TestableScheduler.h +++ b/services/surfaceflinger/tests/unittests/TestableScheduler.h @@ -62,7 +62,7 @@ public: } MOCK_METHOD(void, scheduleConfigure, (), (override)); - MOCK_METHOD(void, scheduleFrame, (), (override)); + MOCK_METHOD(void, scheduleFrame, (Duration), (override)); MOCK_METHOD(void, postMessage, (sp<MessageHandler>&&), (override)); void doFrameSignal(ICompositor& compositor, VsyncId vsyncId) { diff --git a/services/surfaceflinger/tests/unittests/TransactionApplicationTest.cpp b/services/surfaceflinger/tests/unittests/TransactionApplicationTest.cpp index e13fe4958c..fab1f6d451 100644 --- a/services/surfaceflinger/tests/unittests/TransactionApplicationTest.cpp +++ b/services/surfaceflinger/tests/unittests/TransactionApplicationTest.cpp @@ -105,7 +105,7 @@ public: void NotPlacedOnTransactionQueue(uint32_t flags) { ASSERT_TRUE(mFlinger.getTransactionQueue().isEmpty()); - EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame()).Times(1); + EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame(_)).Times(1); TransactionInfo transaction; setupSingle(transaction, flags, /*desiredPresentTime*/ systemTime(), /*isAutoTimestamp*/ true, @@ -129,7 +129,7 @@ public: void PlaceOnTransactionQueue(uint32_t flags) { ASSERT_TRUE(mFlinger.getTransactionQueue().isEmpty()); - EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame()).Times(1); + EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame(_)).Times(1); // first check will see desired present time has not passed, // but afterwards it will look like the desired present time has passed @@ -155,7 +155,7 @@ public: void BlockedByPriorTransaction(uint32_t flags) { ASSERT_TRUE(mFlinger.getTransactionQueue().isEmpty()); nsecs_t time = systemTime(); - EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame()).Times(2); + EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame(_)).Times(2); // transaction that should go on the pending thread TransactionInfo transactionA; @@ -217,7 +217,7 @@ public: TEST_F(TransactionApplicationTest, AddToPendingQueue) { ASSERT_TRUE(mFlinger.getTransactionQueue().isEmpty()); - EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame()).Times(1); + EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame(_)).Times(1); TransactionInfo transactionA; // transaction to go on pending queue setupSingle(transactionA, /*flags*/ 0, /*desiredPresentTime*/ s2ns(1), false, @@ -238,7 +238,7 @@ TEST_F(TransactionApplicationTest, AddToPendingQueue) { TEST_F(TransactionApplicationTest, Flush_RemovesFromQueue) { ASSERT_TRUE(mFlinger.getTransactionQueue().isEmpty()); - EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame()).Times(1); + EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame(_)).Times(1); TransactionInfo transactionA; // transaction to go on pending queue setupSingle(transactionA, /*flags*/ 0, /*desiredPresentTime*/ s2ns(1), false, diff --git a/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp b/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp index 8690dba041..7c678bd811 100644 --- a/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp +++ b/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp @@ -1055,6 +1055,36 @@ TEST_F(VSyncPredictorTest, timelineNotAdjustedForEarlyPresent) { EXPECT_EQ(2000, vrrTracker.nextAnticipatedVSyncTimeFrom(1400, 1000)); EXPECT_EQ(3000, vrrTracker.nextAnticipatedVSyncTimeFrom(2000, 1000)); } + +TEST_F(VSyncPredictorTest, adjustsOnlyMinFrameViolatingVrrTimeline) { + const auto refreshRate = Fps::fromPeriodNsecs(500); + auto minFrameRate = Fps::fromPeriodNsecs(1000); + hal::VrrConfig vrrConfig{.minFrameIntervalNs = + static_cast<int32_t>(minFrameRate.getPeriodNsecs())}; + ftl::NonNull<DisplayModePtr> mode = + ftl::as_non_null(createVrrDisplayMode(DisplayModeId(0), refreshRate, vrrConfig)); + VSyncPredictor vrrTracker{std::make_unique<ClockWrapper>(mClock), mode, kHistorySize, + kMinimumSamplesForPrediction, kOutlierTolerancePercent}; + vrrTracker.setRenderRate(minFrameRate, /*applyImmediately*/ false); + vrrTracker.addVsyncTimestamp(0); + + EXPECT_EQ(1000, vrrTracker.nextAnticipatedVSyncTimeFrom(700)); + EXPECT_EQ(2000, vrrTracker.nextAnticipatedVSyncTimeFrom(1000)); + auto lastConfirmedSignalTime = TimePoint::fromNs(1500); + auto lastConfirmedExpectedPresentTime = TimePoint::fromNs(1000); + vrrTracker.onFrameBegin(TimePoint::fromNs(2000), + {lastConfirmedSignalTime, lastConfirmedExpectedPresentTime}); + EXPECT_EQ(3500, vrrTracker.nextAnticipatedVSyncTimeFrom(3000, 1500)); + + minFrameRate = Fps::fromPeriodNsecs(2000); + vrrTracker.setRenderRate(minFrameRate, /*applyImmediately*/ false); + lastConfirmedSignalTime = TimePoint::fromNs(2500); + lastConfirmedExpectedPresentTime = TimePoint::fromNs(2500); + vrrTracker.onFrameBegin(TimePoint::fromNs(3000), + {lastConfirmedSignalTime, lastConfirmedExpectedPresentTime}); + // Enough time without adjusting vsync to present with new rate on time, no need of adjustment + EXPECT_EQ(5500, vrrTracker.nextAnticipatedVSyncTimeFrom(4000, 3500)); +} } // namespace android::scheduler // TODO(b/129481165): remove the #pragma below and fix conversion issues |